From a545f57437768e12c75fd4cccb19d0ae68f9a89b Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 18 Nov 2020 17:20:07 -0500 Subject: [PATCH] Improved spec testing --- src/connection_string.ts | 38 ++++-- test/unit/core/connection_string.test.js | 166 +++++++++++++---------- 2 files changed, 127 insertions(+), 77 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 7cbc9131e89..30817abe49c 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -797,7 +797,10 @@ function parseURI(uri: string): { srv: boolean; url: URL; hosts: string[] } { throw new MongoParseError('Password contains unescaped characters'); } - const authString = username ? (password ? `${username}:${password}` : `${username}`) : ''; + let authString = ''; + if (typeof username === 'string') authString += username; + if (typeof password === 'string') authString += `:${password}`; + const srv = protocol.includes('srv'); const hostList = hosts.split(','); const url = new URL(`${protocol.toLowerCase()}://${authString}@dummyHostname${rest}`); @@ -922,12 +925,14 @@ export function parseOptions( mongoOptions.dbName = decodeURIComponent( url.pathname[0] === '/' ? url.pathname.slice(1) : url.pathname ); + mongoOptions.credentials = new MongoCredentials({ + ...mongoOptions.credentials, + source: mongoOptions.dbName, + username: typeof url.username === 'string' ? decodeURIComponent(url.username) : undefined, + password: typeof url.password === 'string' ? decodeURIComponent(url.password) : undefined + }); const urlOptions = new Map(); - - if (url.username) urlOptions.set('username', [url.username]); - if (url.password) urlOptions.set('password', [url.password]); - for (const key of url.searchParams.keys()) { const loweredKey = key.toLowerCase(); const values = [...url.searchParams.getAll(key)]; @@ -1036,13 +1041,18 @@ function toHostArray(hostString: string) { socketPath = decodeURIComponent(hostString); } + let ipv6SanitizedHostName; + if (parsedHost.hostname.startsWith('[') && parsedHost.hostname.endsWith(']')) { + ipv6SanitizedHostName = parsedHost.hostname.substring(1, parsedHost.hostname.length - 1); + } + const result: Host = socketPath ? { host: socketPath, type: 'unix' } : { - host: parsedHost.hostname, + host: decodeURIComponent(ipv6SanitizedHostName ?? parsedHost.hostname), port: parsedHost.port ? parseInt(parsedHost.port) : 27017, type: 'tcp' }; @@ -1095,16 +1105,26 @@ export const OPTIONS: Record = { if (!mechanism) { throw new TypeError(`authMechanism one of ${mechanisms}, got ${value}`); } - let db; // some mechanisms have '$external' as the Auth Source + let source = options.credentials.source; // some mechanisms have '$external' as the Auth Source if ( mechanism === 'PLAIN' || mechanism === 'GSSAPI' || mechanism === 'MONGODB-AWS' || mechanism === 'MONGODB-X509' ) { - db = '$external'; + source = '$external'; + } + + let password: string | undefined = options.credentials.password; + if (mechanism === 'MONGODB-X509' && password === '') { + password = undefined; } - return new MongoCredentials({ ...options.credentials, mechanism, db }); + return new MongoCredentials({ + ...options.credentials, + mechanism, + source, + password: password as any + }); } }, authMechanismProperties: { diff --git a/test/unit/core/connection_string.test.js b/test/unit/core/connection_string.test.js index 74de8d9bc38..40d9ff0d9c5 100644 --- a/test/unit/core/connection_string.test.js +++ b/test/unit/core/connection_string.test.js @@ -214,88 +214,118 @@ describe('Connection String', function () { }); describe('spec tests', function () { + /** @type {import('../../spec/connection-string/valid-auth.json')[]} */ const suites = loadSpecTests('connection-string').concat(loadSpecTests('auth')); suites.forEach(suite => { describe(suite.name, function () { suite.tests.forEach(test => { - it(test.description, { - metadata: { requires: { topology: 'single' } }, - test: function (done) { - if (skipTests.indexOf(test.description) !== -1) { - return this.skip(); - } - - const valid = test.valid; - parseConnectionString(test.uri, { caseTranslate: false }, function (err, result) { - if (valid === false) { - expect(err).to.exist; - expect(err).to.be.instanceOf(MongoParseError); - expect(result).to.not.exist; - } else { - expect(err).to.not.exist; - expect(result).to.exist; - - // remove data we don't track - if (test.auth && test.auth.password === '') { - test.auth.password = null; - } - - if (test.hosts != null) { - test.hosts = test.hosts.map(host => { - delete host.type; - host.host = punycode.toASCII(host.host); - return host; - }); - - // remove values that require no validation - test.hosts.forEach(host => { - Object.keys(host).forEach(key => { - if (host[key] == null) delete host[key]; - }); - }); - - expect(result.hosts).to.containSubset(test.hosts); - } - - if (test.auth) { - if (test.auth.db != null) { - expect(result.auth).to.have.property('db'); - expect(result.auth.db).to.eql(test.auth.db); - } - - if (test.auth.username != null) { - expect(result.auth).to.have.property('username'); - expect(result.auth.username).to.eql(test.auth.username); - } - - if (test.auth.password != null) { - expect(result.auth).to.have.property('password'); - expect(result.auth.password).to.eql(test.auth.password); - } - } - - if (test.options != null) { - // it's possible we have options which are not explicitly included in the spec test - expect(result.options).to.deep.include(test.options); - } - } - - done(); - }); - } - }); + // it(test.description, { + // metadata: { requires: { topology: 'single' } }, + // test: function (done) { + // if (skipTests.indexOf(test.description) !== -1) { + // return this.skip(); + // } + + // const valid = test.valid; + // parseConnectionString(test.uri, { caseTranslate: false }, function (err, result) { + // if (valid === false) { + // expect(err).to.exist; + // expect(err).to.be.instanceOf(MongoParseError); + // expect(result).to.not.exist; + // } else { + // expect(err).to.not.exist; + // expect(result).to.exist; + + // // remove data we don't track + // if (test.auth && test.auth.password === '') { + // test.auth.password = null; + // } + + // if (test.hosts != null) { + // test.hosts = test.hosts.map(host => { + // delete host.type; + // host.host = punycode.toASCII(host.host); + // return host; + // }); + + // // remove values that require no validation + // test.hosts.forEach(host => { + // Object.keys(host).forEach(key => { + // if (host[key] == null) delete host[key]; + // }); + // }); + + // expect(result.hosts).to.containSubset(test.hosts); + // } + + // if (test.auth) { + // if (test.auth.db != null) { + // expect(result.auth).to.have.property('db'); + // expect(result.auth.db).to.eql(test.auth.db); + // } + + // if (test.auth.username != null) { + // expect(result.auth).to.have.property('username'); + // expect(result.auth.username).to.eql(test.auth.username); + // } + + // if (test.auth.password != null) { + // expect(result.auth).to.have.property('password'); + // expect(result.auth.password).to.eql(test.auth.password); + // } + // } + + // if (test.options != null) { + // // it's possible we have options which are not explicitly included in the spec test + // expect(result.options).to.deep.include(test.options); + // } + // } + + // done(); + // }); + // } + // }); it(`${test.description} -- new MongoOptions parser`, function () { if (skipTests.includes(test.description)) { return this.skip(); } + const message = `"${test.uri}"`; + const valid = test.valid; if (valid) { - expect(parseOptions(test.uri), `"${test.uri}"`).to.be.ok; + const options = parseOptions(test.uri); + expect(options, message).to.be.ok; + + if (test.hosts) { + for (const [index, { host, port }] of test.hosts.entries()) { + expect(options.hosts[index].host, message).to.equal(host); + if (typeof port === 'number') expect(options.hosts[index].port).to.equal(port); + } + } + + if (test.auth) { + if (test.auth.db != null) { + expect(options.credentials.source, message).to.equal(test.auth.db); + } + + if (test.auth.username != null) { + expect(options.credentials.username, message).to.equal(test.auth.username); + } + + if (test.auth.password != null) { + expect(options.credentials.password, message).to.equal(test.auth.password); + } + } + + // Going to have to get clever here... + // if (test.options) { + // expect(options, message).to.deep.include(test.options); + // } } else { - expect(() => parseOptions(test.uri), `"${test.uri}"`).to.throw(); + expect(() => parseOptions(test.uri), message).to.throw(); } }); });