fix(mqtt): Fix segfault on mqtt hostname resolution

Hypfer · Jul 28, 2020 · b2410ff · b2410ff
1 parent a5e086e
commit b2410ff
Showing 1 changed file with 29 additions and 15 deletions.
diff --git a/lib/MqttClient.js b/lib/MqttClient.js
@@ -157,7 +157,7 @@ class MqttClient {
     /**
      * @private
      */
-    connect() {
+    async connect() {
         if (!this.client || (this.client && this.client.connected === false && this.client.reconnecting === false)) {
             const options = {};
             if (this.clientId) {
@@ -187,33 +187,47 @@ class MqttClient {
             };
 
             if (Tools.IS_VALID_IP(this.server)) {
-                this.connectHelper(options);
+                this.connectHelper(options, this.server);
             } else {
+                let ip;
+                let err;
+
                 /*
-                    Due to a bug somewhere (most likely the mqtt library), the process segfaults
-                    if name resolution fails. At least when calling mqtt.connect() directly.
-                    This also happens with multiple nodejs versions (12, 14)
+                    Due to a bug somewhere in some roborock firmwares, the process segfaults on
+                    some getaddrinfo syscalls.
+                    This happens with multiple nodejs versions (12,14)
 
-                    That's why we're checking the dns resolution first and don't connect if we can't resolve it.
-                    Surprisingly, this works fine.
+                    dns.resolve() seems to work differently which is why we're doing the name resolution here
                  */
-                dns.lookup(this.server, err => {
-                    if (!err) {
-                        this.connectHelper(options);
+                try {
+                    ip = await dns.promises.resolve4(this.server);
+                } catch (e) {
+                    if (e && e.code === "ENOTFOUND") {
+                        try { //tryHarder
+                            ip = await dns.promises.resolve6(this.server);
+                        } catch (e) {
+                            err = e;
+                        }
                     } else {
-                        Logger.error("MQTT connect failed due to error in name resolution:", err.toString());
+                        err = e;
                     }
-                });
+                }
+
+                if (!err) {
+                    this.connectHelper(options, ip);
+                } else {
+                    Logger.error("MQTT connect failed due to error in name resolution:", err);
+                }
             }
         }
     }
 
     /**
      * @private
      */
-    connectHelper(options) {
+    connectHelper(options, ip) {
         this.client = mqtt.connect(
-            (this.usetls ? "mqtts://" : "mqtt://") + this.server + ":" + this.port,
+            (this.usetls ? "mqtts://" : "mqtt://") + ip + ":" + this.port,
             options
         );
 
@@ -558,7 +572,7 @@ class MqttClient {
                         if (Array.isArray(zones) && zones.length) {
                             const zone_ids = [...this.configuration.getZones().values()]
                                 .filter(zone => zones.includes(zone.name) ||
-                                                zones.includes(zone.id))
+                                    zones.includes(zone.id))
                                 .map(zone => zone.id);
                             await this.vacuum.startCleaningZonesById(zone_ids);
                         } else {