Merge pull request #2376

e3680b69 Rename m_listenning_port to m_listening_port (MaxXor)
a1ea475f Delete UPnP port mapping on exit (MaxXor)

2 files changed, 102 insertions, 48 deletions

diff --git a/src/p2p/net_node.h b/src/p2p/net_node.h
index 22ae5ec26..8bbaa9138 100644
--- a/src/p2p/net_node.h
+++ b/src/p2p/net_node.h
@@ -105,7 +105,7 @@ namespace nodetool
     bool init(const boost::program_options::variables_map& vm);
     bool deinit();
     bool send_stop_signal();
-    uint32_t get_this_peer_port(){return m_listenning_port;}
+    uint32_t get_this_peer_port(){return m_listening_port;}
     t_payload_net_handler& get_payload_object();
 
     template <class Archive, class t_version_type>
@@ -218,6 +218,8 @@ namespace nodetool
     bool is_peer_used(const peerlist_entry& peer);
     bool is_peer_used(const anchor_peerlist_entry& peer);
     bool is_addr_connected(const epee::net_utils::network_address& peer);
+    void add_upnp_port_mapping(uint32_t port);
+    void delete_upnp_port_mapping(uint32_t port);
     template<class t_callback>
     bool try_ping(basic_node_data& node_data, p2p_connection_context& context, t_callback cb);
     bool try_get_support_flags(const p2p_connection_context& context, std::function<void(p2p_connection_context&, const uint32_t&)> f);
@@ -287,7 +289,7 @@ namespace nodetool
 
     bool m_have_address;
     bool m_first_connection_maker_call;
-    uint32_t m_listenning_port;
+    uint32_t m_listening_port;
     uint32_t m_external_port;
     uint32_t m_ip_address;
     bool m_allow_local_ip;
diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl
index 889cfaf9d..13352c24a 100644
--- a/src/p2p/net_node.inl
+++ b/src/p2p/net_node.inl
@@ -573,55 +573,15 @@ namespace nodetool
     res = m_net_server.init_server(m_port, m_bind_ip);
     CHECK_AND_ASSERT_MES(res, false, "Failed to bind server");
 
-    m_listenning_port = m_net_server.get_binded_port();
-    MLOG_GREEN(el::Level::Info, "Net service bound to " << m_bind_ip << ":" << m_listenning_port);
+    m_listening_port = m_net_server.get_binded_port();
+    MLOG_GREEN(el::Level::Info, "Net service bound to " << m_bind_ip << ":" << m_listening_port);
     if(m_external_port)
       MDEBUG("External port defined as " << m_external_port);
 
-    // Add UPnP port mapping
-    if(m_no_igd == false) {
-      MDEBUG("Attempting to add IGD port mapping.");
-      int result;
-#if MINIUPNPC_API_VERSION > 13
-      // default according to miniupnpc.h
-      unsigned char ttl = 2;
-      UPNPDev* deviceList = upnpDiscover(1000, NULL, NULL, 0, 0, ttl, &result);
-#else
-      UPNPDev* deviceList = upnpDiscover(1000, NULL, NULL, 0, 0, &result);
-#endif
-      UPNPUrls urls;
-      IGDdatas igdData;
-      char lanAddress[64];
-      result = UPNP_GetValidIGD(deviceList, &urls, &igdData, lanAddress, sizeof lanAddress);
-      freeUPNPDevlist(deviceList);
-      if (result != 0) {
-        if (result == 1) {
-          std::ostringstream portString;
-          portString << m_listenning_port;
-
-          // Delete the port mapping before we create it, just in case we have dangling port mapping from the daemon not being shut down correctly
-          UPNP_DeletePortMapping(urls.controlURL, igdData.first.servicetype, portString.str().c_str(), "TCP", 0);
-
-          int portMappingResult;
-          portMappingResult = UPNP_AddPortMapping(urls.controlURL, igdData.first.servicetype, portString.str().c_str(), portString.str().c_str(), lanAddress, CRYPTONOTE_NAME, "TCP", 0, "0");
-          if (portMappingResult != 0) {
-            LOG_ERROR("UPNP_AddPortMapping failed, error: " << strupnperror(portMappingResult));
-          } else {
-            MLOG_GREEN(el::Level::Info, "Added IGD port mapping.");
-          }
-        } else if (result == 2) {
-          MWARNING("IGD was found but reported as not connected.");
-        } else if (result == 3) {
-          MWARNING("UPnP device was found but not recognized as IGD.");
-        } else {
-          MWARNING("UPNP_GetValidIGD returned an unknown result code.");
-        }
+    // add UPnP port mapping
+    if(!m_no_igd)
+      add_upnp_port_mapping(m_listening_port);
 
-        FreeUPNPUrls(&urls);
-      } else {
-        MINFO("No IGD was found.");
-      }
-    }
     return res;
   }
   //-----------------------------------------------------------------------------------
@@ -688,6 +648,9 @@ namespace nodetool
     kill();
     m_peerlist.deinit();
     m_net_server.deinit_server();
+    // remove UPnP port mapping
+    if(!m_no_igd)
+      delete_upnp_port_mapping(m_listening_port);
     return store_config();
   }
   //-----------------------------------------------------------------------------------
@@ -1389,7 +1352,7 @@ namespace nodetool
     node_data.local_time = local_time;
     node_data.peer_id = m_config.m_peer_id;
     if(!m_hide_my_port)
-      node_data.my_port = m_external_port ? m_external_port : m_listenning_port;
+      node_data.my_port = m_external_port ? m_external_port : m_listening_port;
     else
       node_data.my_port = 0;
     node_data.network_id = m_network_id;
@@ -1973,4 +1936,93 @@ namespace nodetool
 
     return true;
   }
+
+  template<class t_payload_net_handler>
+  void node_server<t_payload_net_handler>::add_upnp_port_mapping(uint32_t port)
+  {
+    MDEBUG("Attempting to add IGD port mapping.");
+    int result;
+#if MINIUPNPC_API_VERSION > 13
+    // default according to miniupnpc.h
+    unsigned char ttl = 2;
+    UPNPDev* deviceList = upnpDiscover(1000, NULL, NULL, 0, 0, ttl, &result);
+#else
+    UPNPDev* deviceList = upnpDiscover(1000, NULL, NULL, 0, 0, &result);
+#endif
+    UPNPUrls urls;
+    IGDdatas igdData;
+    char lanAddress[64];
+    result = UPNP_GetValidIGD(deviceList, &urls, &igdData, lanAddress, sizeof lanAddress);
+    freeUPNPDevlist(deviceList);
+    if (result != 0) {
+      if (result == 1) {
+        std::ostringstream portString;
+        portString << port;
+
+        // Delete the port mapping before we create it, just in case we have dangling port mapping from the daemon not being shut down correctly
+        UPNP_DeletePortMapping(urls.controlURL, igdData.first.servicetype, portString.str().c_str(), "TCP", 0);
+
+        int portMappingResult;
+        portMappingResult = UPNP_AddPortMapping(urls.controlURL, igdData.first.servicetype, portString.str().c_str(), portString.str().c_str(), lanAddress, CRYPTONOTE_NAME, "TCP", 0, "0");
+        if (portMappingResult != 0) {
+          LOG_ERROR("UPNP_AddPortMapping failed, error: " << strupnperror(portMappingResult));
+        } else {
+          MLOG_GREEN(el::Level::Info, "Added IGD port mapping.");
+        }
+      } else if (result == 2) {
+        MWARNING("IGD was found but reported as not connected.");
+      } else if (result == 3) {
+        MWARNING("UPnP device was found but not recognized as IGD.");
+      } else {
+        MWARNING("UPNP_GetValidIGD returned an unknown result code.");
+      }
+
+      FreeUPNPUrls(&urls);
+    } else {
+      MINFO("No IGD was found.");
+    }
+  }
+
+  template<class t_payload_net_handler>
+  void node_server<t_payload_net_handler>::delete_upnp_port_mapping(uint32_t port)
+  {
+    MDEBUG("Attempting to delete IGD port mapping.");
+    int result;
+#if MINIUPNPC_API_VERSION > 13
+    // default according to miniupnpc.h
+    unsigned char ttl = 2;
+    UPNPDev* deviceList = upnpDiscover(1000, NULL, NULL, 0, 0, ttl, &result);
+#else
+    UPNPDev* deviceList = upnpDiscover(1000, NULL, NULL, 0, 0, &result);
+#endif
+    UPNPUrls urls;
+    IGDdatas igdData;
+    char lanAddress[64];
+    result = UPNP_GetValidIGD(deviceList, &urls, &igdData, lanAddress, sizeof lanAddress);
+    freeUPNPDevlist(deviceList);
+    if (result != 0) {
+      if (result == 1) {
+        std::ostringstream portString;
+        portString << port;
+
+        int portMappingResult;
+        portMappingResult = UPNP_DeletePortMapping(urls.controlURL, igdData.first.servicetype, portString.str().c_str(), "TCP", 0);
+        if (portMappingResult != 0) {
+          LOG_ERROR("UPNP_DeletePortMapping failed, error: " << strupnperror(portMappingResult));
+        } else {
+          MLOG_GREEN(el::Level::Info, "Deleted IGD port mapping.");
+        }
+      } else if (result == 2) {
+        MWARNING("IGD was found but reported as not connected.");
+      } else if (result == 3) {
+        MWARNING("UPnP device was found but not recognized as IGD.");
+      } else {
+        MWARNING("UPNP_GetValidIGD returned an unknown result code.");
+      }
+
+      FreeUPNPUrls(&urls);
+    } else {
+      MINFO("No IGD was found.");
+    }
+  }
 }