From e4484ac300d9fef60217187fdbcffb8e8b05934f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Moreau?= <cem.moreau@gmail.com>
Date: Sun, 4 Feb 2018 20:16:55 +0100
Subject: [PATCH] [enh] #1216 Improve UPnP behavior

---
 app/modules/bma/index.ts          |  2 +-
 app/modules/bma/lib/network.ts    |  2 +-
 app/modules/bma/lib/upnp.ts       | 14 ++++++++-----
 app/modules/ws2p/index.ts         |  9 ++++----
 app/modules/ws2p/lib/ws2p-upnp.ts | 26 ++++++++++++++---------
 package.json                      |  2 +-
 yarn.lock                         | 34 ++++++++++---------------------
 7 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/app/modules/bma/index.ts b/app/modules/bma/index.ts
index 9dc1beaac..0509226ad 100644
--- a/app/modules/bma/index.ts
+++ b/app/modules/bma/index.ts
@@ -230,7 +230,7 @@ export class BMAPI extends stream.Transform {
     }
     if (this.server.conf.upnp) {
       try {
-        this.upnpAPI = await upnp(this.server.conf.port, this.server.conf.remoteport, this.logger);
+        this.upnpAPI = await upnp(this.server.conf.port, this.server.conf.remoteport, this.logger, this.server.conf);
         this.upnpAPI.startRegular();
         const gateway = await this.upnpAPI.findGateway();
         if (gateway) {
diff --git a/app/modules/bma/lib/network.ts b/app/modules/bma/lib/network.ts
index 3e9e026f9..60e17dada 100644
--- a/app/modules/bma/lib/network.ts
+++ b/app/modules/bma/lib/network.ts
@@ -332,7 +332,7 @@ function listInterfaces() {
 }
 
 async function upnpConf (noupnp:boolean, logger:any) {
-  const client = require('nnupnp').createClient();
+  const client = require('nat-upnp').createClient();
   // Look for 2 random ports
   const publicPort = await getAvailablePort(client)
   const privatePort = publicPort
diff --git a/app/modules/bma/lib/upnp.ts b/app/modules/bma/lib/upnp.ts
index 06b6bf41e..ea599e5a0 100644
--- a/app/modules/bma/lib/upnp.ts
+++ b/app/modules/bma/lib/upnp.ts
@@ -1,12 +1,14 @@
 import {BMAConstants} from "./constants"
-const upnp = require('nnupnp');
+import {ConfDTO} from "../../../lib/dto/ConfDTO"
+
+const upnp = require('nat-upnp');
 const Q = require('q');
 
-export const Upnp = async function (localPort:number, remotePort:number, logger:any) {
+export const Upnp = async function (localPort:number, remotePort:number, logger:any, conf:ConfDTO) {
   "use strict";
 
   logger.info('UPnP: configuring...');
-  const api = new UpnpApi(localPort, remotePort, logger)
+  const api = new UpnpApi(localPort, remotePort, logger, conf)
   try {
     await api.openPort()
   } catch (e) {
@@ -32,19 +34,21 @@ export class UpnpApi {
   constructor(
     private localPort:number,
     private remotePort:number,
-    private logger:any
+    private logger:any,
+    private conf:ConfDTO
   ) {}
 
   openPort() {
     "use strict";
     return Q.Promise((resolve:any, reject:any) => {
+      const suffix = this.conf.pair.pub.substr(0, 6)
       this.logger.trace('UPnP: mapping external port %s to local %s...', this.remotePort, this.localPort);
       const client = upnp.createClient();
       client.portMapping({
         'public': this.remotePort,
         'private': this.localPort,
         'ttl': BMAConstants.UPNP_TTL,
-        'description': 'duniter:bma:upnp'
+        'description': 'duniter:bma:' + suffix
       }, (err:any) => {
         client.close();
         if (err) {
diff --git a/app/modules/ws2p/index.ts b/app/modules/ws2p/index.ts
index c91451e46..64c4acd0b 100644
--- a/app/modules/ws2p/index.ts
+++ b/app/modules/ws2p/index.ts
@@ -1,11 +1,12 @@
 "use strict";
-import { WS2PConstants } from './lib/constants';
+import {WS2PConstants} from './lib/constants';
 import {ConfDTO, WS2PConfDTO} from "../../lib/dto/ConfDTO"
 import {Server} from "../../../server"
 import * as stream from 'stream';
 import {WS2PCluster} from "./lib/WS2PCluster"
 import {WS2PUpnp} from "./lib/ws2p-upnp"
 import {CommonConstants} from "../../lib/common-libs/constants"
+
 const constants = require("../../lib/constants");
 
 const nuuid = require('node-uuid')
@@ -109,7 +110,7 @@ export const WS2PDependency = {
     },
 
     service: {
-      input: (server:Server, conf:WS2PConfDTO, logger:any) => {
+      input: (server:Server, conf:ConfDTO, logger:any) => {
         const api = new WS2PAPI(server, conf, logger)
         server.ws2pCluster = api.getCluster()
         server.addEndpointsDefinitions(async () => api.getEndpoint())
@@ -176,7 +177,7 @@ export class WS2PAPI extends stream.Transform {
 
   constructor(
     private server:Server,
-    private conf:WS2PConfDTO,
+    private conf:ConfDTO,
     private logger:any) {
     super({ objectMode: true })
     this.cluster = WS2PCluster.plugOn(server)
@@ -212,7 +213,7 @@ export class WS2PAPI extends stream.Transform {
           this.upnpAPI.stopRegular();
         }
         try {
-          this.upnpAPI = new WS2PUpnp(this.logger)
+          this.upnpAPI = new WS2PUpnp(this.logger, this.conf)
           const { host, port, available } = await this.upnpAPI.startRegular()
           if (available) {
             // Defaults UPnP to true if not defined and available
diff --git a/app/modules/ws2p/lib/ws2p-upnp.ts b/app/modules/ws2p/lib/ws2p-upnp.ts
index 81b0dae12..a6b6a7a5f 100644
--- a/app/modules/ws2p/lib/ws2p-upnp.ts
+++ b/app/modules/ws2p/lib/ws2p-upnp.ts
@@ -1,5 +1,7 @@
 import {WS2PConstants} from "./constants"
-const upnp = require('nnupnp');
+import {ConfDTO} from "../../../lib/dto/ConfDTO"
+
+const upnp = require('nat-upnp');
 
 export interface UPnPBinding {
   remotehost:string
@@ -13,9 +15,7 @@ export class WS2PUpnp {
   private interval:NodeJS.Timer|null
   private client = upnp.createClient()
 
-  constructor(
-    private logger:any
-  ) {}
+  constructor(private logger:any, private conf:ConfDTO) {}
 
   async checkUPnPisAvailable() {
     try {
@@ -38,6 +38,12 @@ export class WS2PUpnp {
     return this.currentConfig
   }
 
+  getUpnpDescription() {
+    const uuid = (this.conf.ws2p && this.conf.ws2p.uuid) || "no-uuid-yet"
+    const suffix = this.conf.pair.pub.substr(0, 6) + ":" + uuid
+    return 'duniter:ws2p:' + suffix
+  }
+
   /**
    * Always open the same port during an execution of Duniter.
    * @returns { host:string, port:number }
@@ -45,7 +51,7 @@ export class WS2PUpnp {
   openPort() {
     return new Promise<{ host:string, port:number }>(async (resolve:any, reject:any) => {
       if (!this.currentConfig) {
-        this.currentConfig = await WS2PUpnp.getAvailablePort(this.client)
+        this.currentConfig = await this.getAvailablePort(this.client)
       }
       this.logger.trace('WS2P: mapping external port %s to local %s using UPnP...', this.currentConfig.port, [this.currentConfig.host, this.currentConfig.port].join(':'))
       const client = upnp.createClient()
@@ -53,7 +59,7 @@ export class WS2PUpnp {
         'public': this.currentConfig.port,
         'private': this.currentConfig.port,
         'ttl': WS2PConstants.WS2P_UPNP_TTL,
-        'description': 'duniter:ws2p:upnp'
+        'description': this.getUpnpDescription()
       }, (err:any) => {
         client.close()
         if (err) {
@@ -101,7 +107,7 @@ export class WS2PUpnp {
     })
   }
 
-  static async getAvailablePort(client:any) {
+  private async getAvailablePort(client:any) {
     const localIP = await WS2PUpnp.getLocalIP(client)
     const remoteIP = await WS2PUpnp.getRemoteIP(client)
     const mappings:{
@@ -111,10 +117,10 @@ export class WS2PUpnp {
       public: {
         port:number
       }
+      description:string
     }[] = await WS2PUpnp.getUPnPMappings(client)
-    const externalPortsUsed = mappings.map((m) => {
-      return m.public.port
-    })
+    const thisDesc = this.getUpnpDescription()
+    const externalPortsUsed = mappings.filter((m) => m.description !== thisDesc).map((m) => m.public.port)
     let availablePort = WS2PConstants.WS2P_PORTS_START
     while (externalPortsUsed.indexOf(availablePort) !== -1
       && availablePort <= WS2PConstants.WS2P_PORTS_END) {
diff --git a/package.json b/package.json
index 17de76510..7405a3800 100644
--- a/package.json
+++ b/package.json
@@ -80,7 +80,7 @@
     "morgan": "1.8.1",
     "multimeter": "0.1.1",
     "naclb": "1.3.10",
-    "nnupnp": "1.0.2",
+    "nat-upnp": "^1.1.1",
     "node-pre-gyp": "0.6.34",
     "node-uuid": "1.4.8",
     "optimist": "0.6.1",
diff --git a/yarn.lock b/yarn.lock
index b31aab9d3..27b57764e 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -234,10 +234,6 @@ assert-plus@^0.2.0:
   version "0.2.0"
   resolved "https://registry.yarnpkg.com/assert-plus/-/assert-plus-0.2.0.tgz#d74e1b87e7affc0db8aadb7021f3fe48101ab234"
 
-async@0.1.22:
-  version "0.1.22"
-  resolved "https://registry.yarnpkg.com/async/-/async-0.1.22.tgz#0fc1aaa088a0e3ef0ebe2d8831bab0dcf8845061"
-
 async@2.2.0:
   version "2.2.0"
   resolved "https://registry.yarnpkg.com/async/-/async-2.2.0.tgz#c324eba010a237e4fbd55a12dee86367d5c0ef32"
@@ -248,7 +244,7 @@ async@^1.4.0:
   version "1.5.2"
   resolved "https://registry.yarnpkg.com/async/-/async-1.5.2.tgz#ec6a61ae56480c0c3cb241c95618e20892f9672a"
 
-async@^2.0.0:
+async@^2.0.0, async@^2.1.5:
   version "2.6.0"
   resolved "https://registry.yarnpkg.com/async/-/async-2.6.0.tgz#61a29abb6fcc026fea77e56d1c6ec53a795951f4"
   dependencies:
@@ -1926,10 +1922,6 @@ invert-kv@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/invert-kv/-/invert-kv-1.0.0.tgz#104a8e4aaca6d3d8cd157a8ef8bfab2d7a3ffdb6"
 
-ip@0.0.1:
-  version "0.0.1"
-  resolved "https://registry.yarnpkg.com/ip/-/ip-0.0.1.tgz#bbc68d7cc448560a63fbe99237a01bc50fdca7ec"
-
 ip@^1.1.4:
   version "1.1.5"
   resolved "https://registry.yarnpkg.com/ip/-/ip-1.1.5.tgz#bdded70114290828c0a039e72ef25f5aaec4354a"
@@ -2615,6 +2607,15 @@ nan@~2.7.0:
   version "2.7.0"
   resolved "https://registry.yarnpkg.com/nan/-/nan-2.7.0.tgz#d95bf721ec877e08db276ed3fc6eb78f9083ad46"
 
+nat-upnp@^1.1.1:
+  version "1.1.1"
+  resolved "https://registry.yarnpkg.com/nat-upnp/-/nat-upnp-1.1.1.tgz#b18365e4faf44652549bb593c69e6b690df22043"
+  dependencies:
+    async "^2.1.5"
+    ip "^1.1.4"
+    request "^2.79.0"
+    xml2js "~0.1.14"
+
 natives@^1.1.0:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/natives/-/natives-1.1.0.tgz#e9ff841418a6b2ec7a495e939984f78f163e6e31"
@@ -2627,15 +2628,6 @@ negotiator@0.6.1:
   version "0.6.1"
   resolved "https://registry.yarnpkg.com/negotiator/-/negotiator-0.6.1.tgz#2b327184e8992101177b28563fb5e7102acd0ca9"
 
-nnupnp@1.0.2:
-  version "1.0.2"
-  resolved "https://registry.yarnpkg.com/nnupnp/-/nnupnp-1.0.2.tgz#1f76e283a0c8fc3a70ae84db762d999f650e0929"
-  dependencies:
-    async "0.1.22"
-    ip "0.0.1"
-    request "2.10.0"
-    xml2js "0.1.14"
-
 node-pre-gyp@0.6.23:
   version "0.6.23"
   resolved "https://registry.yarnpkg.com/node-pre-gyp/-/node-pre-gyp-0.6.23.tgz#155bf3683abcfcde008aedab1248891a0773db95"
@@ -3263,10 +3255,6 @@ request-promise@4.2.0:
     request-promise-core "1.1.1"
     stealthy-require "^1.0.0"
 
-request@2.10.0:
-  version "2.10.0"
-  resolved "https://registry.yarnpkg.com/request/-/request-2.10.0.tgz#9911b5ef669b6500da2ae0b133fa1cfc92b1c48a"
-
 request@2.40.0:
   version "2.40.0"
   resolved "https://registry.yarnpkg.com/request/-/request-2.40.0.tgz#4dd670f696f1e6e842e66b4b5e839301ab9beb67"
@@ -4252,7 +4240,7 @@ xml-escape@~1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/xml-escape/-/xml-escape-1.0.0.tgz#00963d697b2adf0c185c4e04e73174ba9b288eb2"
 
-xml2js@0.1.14:
+xml2js@~0.1.14:
   version "0.1.14"
   resolved "https://registry.yarnpkg.com/xml2js/-/xml2js-0.1.14.tgz#5274e67f5a64c5f92974cd85139e0332adc6b90c"
   dependencies:
-- 
GitLab