fixed #1388 : took code from 736c95a870 and fixed it as https://github.com/PurpleI2P/i2pd/issues/1388#issuecomment-528495918 tells

daemon/UPnP.cpp

@@ -79,43 +79,58 @@ namespace transport
 
 	void UPnP::Discover ()
 	{
-#if MINIUPNPC_API_VERSION >= 14
-		int nerror = 0;
-		m_Devlist = upnpDiscover (2000, m_MulticastIf, m_Minissdpdpath, 0, 0, 2, &nerror);
-#elif ( MINIUPNPC_API_VERSION >= 8 || defined(UPNPDISCOVER_SUCCESS) )
-		int nerror = 0;
-		m_Devlist = upnpDiscover (2000, m_MulticastIf, m_Minissdpdpath, 0, 0, &nerror);
+		bool isError;
+        int err;
+
+#if ((MINIUPNPC_API_VERSION >= 8) || defined (UPNPDISCOVER_SUCCESS))
+        err = UPNPDISCOVER_SUCCESS;
+
+#if (MINIUPNPC_API_VERSION >= 14)
+		m_Devlist = upnpDiscover (UPNP_RESPONSE_TIMEOUT, NULL, NULL, 0, 0, 2, &err);
 #else
-		m_Devlist = upnpDiscover (2000, m_MulticastIf, m_Minissdpdpath, 0);
+		m_Devlist = upnpDiscover (UPNP_RESPONSE_TIMEOUT, NULL, NULL, 0, 0, &err);
 #endif
+
+		isError = err != UPNPDISCOVER_SUCCESS;
+#else  // MINIUPNPC_API_VERSION >= 8
+        err = 0;
+        m_Devlist = upnpDiscover (UPNP_RESPONSE_TIMEOUT, NULL, NULL, 0);
+		isError = m_Devlist == NULL;
+#endif // MINIUPNPC_API_VERSION >= 8
 		{
-			// notify satrting thread
+			// notify starting thread
 			std::unique_lock<std::mutex> l(m_StartedMutex);
 			m_Started.notify_all ();
 		}
 
-		int r;
-		r = UPNP_GetValidIGD (m_Devlist, &m_upnpUrls, &m_upnpData, m_NetworkAddr, sizeof (m_NetworkAddr));
-		if (r == 1)
+		if (isError)
+		{
+            LogPrint (eLogError, "UPnP: unable to discover Internet Gateway Devices: error ", err);
+			return;
+		}
+
+		err = UPNP_GetValidIGD (m_Devlist, &m_upnpUrls, &m_upnpData, m_NetworkAddr, sizeof (m_NetworkAddr));
+		if (err == UPNP_IGD_VALID_CONNECTED)
 		{
-			r = UPNP_GetExternalIPAddress (m_upnpUrls.controlURL, m_upnpData.first.servicetype, m_externalIPAddress);
-			if(r != UPNPCOMMAND_SUCCESS)
+			err = UPNP_GetExternalIPAddress (m_upnpUrls.controlURL, m_upnpData.first.servicetype, m_externalIPAddress);
+			if(err != UPNPCOMMAND_SUCCESS)
 			{
-				LogPrint (eLogError, "UPnP: UPNP_GetExternalIPAddress() returned ", r);
+				LogPrint (eLogError, "UPnP: unable to get external address: error ", err);
 				return;
 			}
 			else
 			{
+				LogPrint (eLogError, "UPnP: found Internet Gateway Device ", m_upnpUrls.controlURL);
 				if (!m_externalIPAddress[0])
 				{
-					LogPrint (eLogError, "UPnP: GetExternalIPAddress() failed.");
+                    LogPrint (eLogError, "UPnP: found Internet Gateway Device doesn't know our external address");
 					return;
 				}
 			}
 		}
 		else
 		{
-			LogPrint (eLogError, "UPnP: GetValidIGD() failed.");
+            LogPrint (eLogError, "UPnP: unable to find valid Internet Gateway Device: error ", err);
 			return;
 		}
 
@@ -126,6 +141,20 @@ namespace transport
 		PortMapping ();
 	}
 
+	int UPnP::CheckMapping (const char* port, const char* type)
+	{
+		int err = UPNPCOMMAND_SUCCESS;
+
+#if (MINIUPNPC_API_VERSION >= 10)
+		err = UPNP_GetSpecificPortMappingEntry(m_upnpUrls.controlURL, m_upnpData.first.servicetype, port, type, NULL, NULL, NULL, NULL, NULL, NULL);
+#elif ((MINIUPNPC_API_VERSION >= 8) || defined (UPNPDISCOVER_SUCCESS))
+		err = UPNP_GetSpecificPortMappingEntry(m_upnpUrls.controlURL, m_upnpData.first.servicetype, port, type, NULL, NULL, NULL, NULL, NULL);
+#else
+		err = UPNP_GetSpecificPortMappingEntry(m_upnpUrls.controlURL, m_upnpData.first.servicetype, port, type, NULL, NULL);
+#endif
+		return err;
+	}
+
 	void UPnP::PortMapping ()
 	{
 		const auto& a = context.GetRouterInfo().GetAddresses();
@@ -140,47 +169,64 @@ namespace transport
 			if (ecode != boost::asio::error::operation_aborted)
 			PortMapping ();
 		});
-
-	}
-
-	void UPnP::CloseMapping ()
-	{
-		const auto& a = context.GetRouterInfo().GetAddresses();
-		for (const auto& address : a)
-		{
-			if (!address->host.is_v6 () && address->port)
-			CloseMapping (address);
-		}
 	}
 
 	void UPnP::TryPortMapping (std::shared_ptr<i2p::data::RouterInfo::Address> address)
 	{
 		std::string strType (GetProto (address)), strPort (std::to_string (address->port));
-		int r;
 		std::string strDesc; i2p::config::GetOption("upnp.name", strDesc);
-#ifdef UPNPDISCOVER_SUCCESS
-		r = UPNP_AddPortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strPort.c_str (), m_NetworkAddr, strDesc.c_str (), strType.c_str (), 0, "0");
+		int err = UPNPCOMMAND_SUCCESS;
+
+		// check for existing mapping
+		err = CheckMapping (strPort.c_str (), strType.c_str ());
+		if (err != UPNPCOMMAND_SUCCESS) // if mapping not found
+		{
+            LogPrint (eLogDebug, "UPnP: possibly port ", strPort, " is not forwarded: return code ", err);
+
+#if ((MINIUPNPC_API_VERSION >= 8) || defined (UPNPDISCOVER_SUCCESS))
+			err = UPNP_AddPortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strPort.c_str (), m_NetworkAddr, strDesc.c_str (), strType.c_str (), NULL, NULL);
 #else
-		r = UPNP_AddPortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strPort.c_str (), m_NetworkAddr, strDesc.c_str (), strType.c_str (), 0);
+			err = UPNP_AddPortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strPort.c_str (), m_NetworkAddr, strDesc.c_str (), strType.c_str (), NULL);
 #endif
-		if (r!=UPNPCOMMAND_SUCCESS)
+			if (err != UPNPCOMMAND_SUCCESS)
 			{
-			LogPrint (eLogError, "UPnP: AddPortMapping (", m_NetworkAddr, ":", strPort, ") failed with code ", r);
+				LogPrint (eLogError, "UPnP: port forwarding to ", m_NetworkAddr, ":", strPort, " failed: return code ", err);
 				return;
 			}
 			else
 			{
-			LogPrint (eLogDebug, "UPnP: Port Mapping successful. (", m_NetworkAddr ,":", strPort, " type ", strType, " -> ", m_externalIPAddress ,":", strPort ,")");
+				LogPrint (eLogInfo, "UPnP: port successfully forwarded (", m_externalIPAddress ,":", strPort, " type ", strType, " -> ", m_NetworkAddr ,":", strPort ,")");
 				return;
 			}
 		}
+		else
+		{
+            LogPrint (eLogDebug, "UPnP: external forward from ", m_NetworkAddr, ":", strPort, " exists on current Internet Gateway Device");
+			return;
+		}
+	}
+
+	void UPnP::CloseMapping ()
+	{
+		const auto& a = context.GetRouterInfo().GetAddresses();
+		for (const auto& address : a)
+		{
+			if (!address->host.is_v6 () && address->port)
+			CloseMapping (address);
+		}
+	}
 
 	void UPnP::CloseMapping (std::shared_ptr<i2p::data::RouterInfo::Address> address)
 	{
 		std::string strType (GetProto (address)), strPort (std::to_string (address->port));
-		int r = 0;
-		r = UPNP_DeletePortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strType.c_str (), 0);
-		LogPrint (eLogError, "UPnP: DeletePortMapping() returned : ", r);
+		int err = UPNPCOMMAND_SUCCESS;
+		
+		err = CheckMapping (strPort.c_str (), strType.c_str ());
+		if (err == UPNPCOMMAND_SUCCESS)
+		{
+			err = UPNP_DeletePortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strType.c_str (), NULL);
+			LogPrint (eLogError, "UPnP: DeletePortMapping() returned : ", err);
+		}
 	}
 
 	void UPnP::Close ()

daemon/UPnP.h

@@ -19,6 +19,16 @@ namespace i2p
 {
 namespace transport
 {
+	const int UPNP_RESPONSE_TIMEOUT = 2000; // in milliseconds
+
+	enum
+	{
+		UPNP_IGD_NONE = 0,
+		UPNP_IGD_VALID_CONNECTED = 1,
+		UPNP_IGD_VALID_NOT_CONNECTED = 2,
+		UPNP_IGD_INVALID = 3
+	};
+
 	class UPnP
 	{
 		public:
@@ -33,6 +43,7 @@ namespace transport
 		private:
 
 		void Discover ();
+		int  CheckMapping (const char* port, const char* type);
 		void PortMapping ();
 		void TryPortMapping (std::shared_ptr<i2p::data::RouterInfo::Address> address);
 		void CloseMapping ();
@@ -53,8 +64,6 @@ namespace transport
 		struct IGDdatas m_upnpData;
 
 		// For miniupnpc
-        char * m_MulticastIf = 0;
-        char * m_Minissdpdpath = 0;
 		struct UPNPDev * m_Devlist = 0;
 		char m_NetworkAddr[64];
 		char m_externalIPAddress[40];
@@ -68,6 +77,7 @@ namespace transport {
 	/* class stub */
 	class UPnP {
 		public:
+
 		UPnP () {};
 		~UPnP () {};
 		void Start () { LogPrint(eLogWarning, "UPnP: this module was disabled at compile-time"); }