Skip to content

Commit

Permalink
fix: ipv6 is not supported when using dns service discovery
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nbbeeken authored Jul 20, 2020
1 parent 95440fb commit 3cc53cb
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 15 deletions.
8 changes: 4 additions & 4 deletions src/sdam/monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
ServerHeartbeatSucceededEvent,
ServerHeartbeatFailedEvent
} from './events';
import { Server } from './server';

const kServer = Symbol('server');
const kMonitorId = Symbol('monitorId');
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down
8 changes: 2 additions & 6 deletions src/sdam/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -150,7 +146,7 @@ class Server extends EventEmitter {
});
}

get description() {
get description(): ServerDescription {
return this.s.description;
}

Expand Down
12 changes: 11 additions & 1 deletion src/sdam/server_description.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}
Expand Down
8 changes: 4 additions & 4 deletions test/unit/sdam/monitoring.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions test/unit/sdam/server_description.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

0 comments on commit 3cc53cb

Please sign in to comment.