From 3cc53cbf1cefc3590afc2f04df9e4aa32ea1085c Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Mon, 20 Jul 2020 14:12:19 -0400
Subject: [PATCH] fix: ipv6 is not supported when using dns service discovery

Both server class and monitoring class split the address string on ":" and
assumed it would return address and port tuple. To handle IPv6 addresses
we need to use the number at the end of the address and keep the ipv6
address from being split. I consolidated the logic into ServerDescription
getters.

NODE-2671
---
 src/sdam/monitor.ts                       |  8 ++++----
 src/sdam/server.ts                        |  8 ++------
 src/sdam/server_description.ts            | 12 +++++++++++-
 test/unit/sdam/monitoring.test.js         |  8 ++++----
 test/unit/sdam/server_description.test.js |  6 ++++++
 5 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/src/sdam/monitor.ts b/src/sdam/monitor.ts
index a08df4ebd7..158a287a89 100644
--- a/src/sdam/monitor.ts
+++ b/src/sdam/monitor.ts
@@ -15,6 +15,7 @@ import {
   ServerHeartbeatSucceededEvent,
   ServerHeartbeatFailedEvent
 } from './events';
+import { Server } from './server';
 
 const kServer = Symbol('server');
 const kMonitorId = Symbol('monitorId');
@@ -47,7 +48,7 @@ class Monitor extends EventEmitter {
   [kCancellationToken]: any;
   [kMonitorId]: any;
 
-  constructor(server: any, options: any) {
+  constructor(server: Server, options: any) {
     super(options);
     this[kServer] = server;
     this[kConnection] = undefined;
@@ -73,12 +74,11 @@ class Monitor extends EventEmitter {
     });
 
     // TODO: refactor this to pull it directly from the pool, requires new ConnectionPool integration
-    const addressParts = server.description.address.split(':');
     const connectOptions = Object.assign(
       {
         id: '<monitor>',
-        host: addressParts[0],
-        port: parseInt(addressParts[1], 10),
+        host: server.description.host,
+        port: server.description.port,
         connectionType: Connection
       },
       server.s.options,
diff --git a/src/sdam/server.ts b/src/sdam/server.ts
index 8866289e07..185367312c 100644
--- a/src/sdam/server.ts
+++ b/src/sdam/server.ts
@@ -102,11 +102,7 @@ class Server extends EventEmitter {
 
     // create the connection pool
     // NOTE: this used to happen in `connect`, we supported overriding pool options there
-    const addressParts = this.description.address.split(':');
-    const poolOptions = Object.assign(
-      { host: addressParts[0], port: parseInt(addressParts[1], 10) },
-      options
-    );
+    const poolOptions = Object.assign({ host: description.host, port: description.port }, options);
 
     this.s.pool = new ConnectionPool(poolOptions);
     relayEvents(
@@ -150,7 +146,7 @@ class Server extends EventEmitter {
     });
   }
 
-  get description() {
+  get description(): ServerDescription {
     return this.s.description;
   }
 
diff --git a/src/sdam/server_description.ts b/src/sdam/server_description.ts
index e31d44e282..da54734be2 100644
--- a/src/sdam/server_description.ts
+++ b/src/sdam/server_description.ts
@@ -43,7 +43,7 @@ const ISMASTER_FIELDS = [
  * Internal type, not meant to be directly instantiated
  */
 class ServerDescription {
-  address: any;
+  address: string;
   error: any;
   roundTripTime: any;
   lastUpdateTime: any;
@@ -135,6 +135,16 @@ class ServerDescription {
     return WRITABLE_SERVER_TYPES.has(this.type);
   }
 
+  get host() {
+    const chopLength = `:${this.port}`.length;
+    return this.address.slice(0, -chopLength);
+  }
+
+  get port() {
+    const port = this.address.split(':').pop();
+    return port ? Number.parseInt(port, 10) : port;
+  }
+
   /**
    * Determines if another `ServerDescription` is equal to this one per the rules defined
    * in the {@link https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst#serverdescription|SDAM spec}
diff --git a/test/unit/sdam/monitoring.test.js b/test/unit/sdam/monitoring.test.js
index 0bfa6ff377..68accbfd00 100644
--- a/test/unit/sdam/monitoring.test.js
+++ b/test/unit/sdam/monitoring.test.js
@@ -4,14 +4,13 @@ const { ServerType } = require('../../../src/sdam/common');
 const { Topology } = require('../../../src/sdam/topology');
 const { Monitor } = require('../../../src/sdam/monitor');
 const { expect } = require('chai');
+const { ServerDescription } = require('../../../src/sdam/server_description');
 
 class MockServer {
   constructor(options) {
     this.s = {};
-    this.description = {
-      type: ServerType.Unknown,
-      address: `${options.host}:${options.port}`
-    };
+    this.description = new ServerDescription(`${options.host}:${options.port}`);
+    this.description.type = ServerType.Unknown;
   }
 }
 
@@ -192,6 +191,7 @@ describe('monitoring', function() {
       });
 
       const server = new MockServer(mockServer.address());
+      server.description = new ServerDescription(server.description.address);
       const monitor = new Monitor(server, {
         heartbeatFrequencyMS: 250,
         minHeartbeatFrequencyMS: 50
diff --git a/test/unit/sdam/server_description.test.js b/test/unit/sdam/server_description.test.js
index e897ee5423..001a6fb650 100644
--- a/test/unit/sdam/server_description.test.js
+++ b/test/unit/sdam/server_description.test.js
@@ -41,4 +41,10 @@ describe('ServerDescription', function() {
       });
     });
   });
+
+  it('should sensibly parse an ipv6 address', function() {
+    const description = new ServerDescription('abcd:f::abcd:abcd:abcd:abcd:27017');
+    expect(description.host).to.equal('abcd:f::abcd:abcd:abcd:abcd');
+    expect(description.port).to.equal(27017);
+  });
 });