diff --git a/src/cmap/auth/auth_provider.ts b/src/cmap/auth/auth_provider.ts index 2a38abe9b4..3e7fcb23c6 100644 --- a/src/cmap/auth/auth_provider.ts +++ b/src/cmap/auth/auth_provider.ts @@ -7,14 +7,17 @@ import type { MongoCredentials } from './mongo_credentials'; export type AuthContextOptions = ConnectionOptions & ClientMetadataOptions; -/** Context used during authentication */ +/** + * Context used during authentication + * @internal + */ export class AuthContext { /** The connection to authenticate */ connection: Connection; /** The credentials to use for authentication */ credentials?: MongoCredentials; /** The options passed to the `connect` method */ - options: AuthContextOptions; + options: ConnectionOptions; /** A response from an initial auth attempt, only some mechanisms use this (e.g, SCRAM) */ response?: Document; @@ -24,7 +27,7 @@ export class AuthContext { constructor( connection: Connection, credentials: MongoCredentials | undefined, - options: AuthContextOptions + options: ConnectionOptions ) { this.connection = connection; this.credentials = credentials; diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 4597db2c5f..16028a31ad 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -18,7 +18,7 @@ import { MongoServerError, needsRetryableWriteLabel } from '../error'; -import { Callback, ClientMetadata, HostAddress, makeClientMetadata, ns } from '../utils'; +import { Callback, ClientMetadata, HostAddress, ns } from '../utils'; import { AuthContext, AuthProvider } from './auth/auth_provider'; import { GSSAPI } from './auth/gssapi'; import { MongoCR } from './auth/mongocr'; @@ -233,7 +233,7 @@ export function prepareHandshakeDocument( const handshakeDoc: HandshakeDocument = { [serverApi?.version ? 'hello' : LEGACY_HELLO_COMMAND]: true, helloOk: true, - client: options.metadata || makeClientMetadata(options), + client: options.metadata, compression: compressors }; diff --git a/src/connection_string.ts b/src/connection_string.ts index a1a177433d..3abbcf9228 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -16,7 +16,6 @@ import { } from './error'; import { Logger as LegacyLogger, LoggerLevel as LegacyLoggerLevel } from './logger'; import { - DriverInfo, MongoClient, MongoClientOptions, MongoOptions, @@ -534,6 +533,8 @@ export function parseOptions( loggerClientOptions ); + mongoOptions.metadata = makeClientMetadata(mongoOptions); + return mongoOptions; } @@ -635,10 +636,7 @@ interface OptionDescriptor { export const OPTIONS = { appName: { - target: 'metadata', - transform({ options, values: [value] }): DriverInfo { - return makeClientMetadata({ ...options.driverInfo, appName: String(value) }); - } + type: 'string' }, auth: { target: 'credentials', @@ -784,15 +782,8 @@ export const OPTIONS = { type: 'boolean' }, driverInfo: { - target: 'metadata', - default: makeClientMetadata(), - transform({ options, values: [value] }) { - if (!isRecord(value)) throw new MongoParseError('DriverInfo must be an object'); - return makeClientMetadata({ - driverInfo: value, - appName: options.metadata?.application?.name - }); - } + default: {}, + type: 'record' }, enableUtf8Validation: { type: 'boolean', default: true }, family: { diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 77fc4953e1..1390a6001d 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -770,6 +770,7 @@ export interface MongoOptions > >, SupportedNodeConnectionOptions { + appName?: string; hosts: HostAddress[]; srvHost?: string; credentials?: MongoCredentials; diff --git a/src/utils.ts b/src/utils.ts index 3fce528b4a..57e2ec3566 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -20,7 +20,7 @@ import { MongoRuntimeError } from './error'; import type { Explain } from './explain'; -import type { MongoClient } from './mongo_client'; +import type { MongoClient, MongoOptions } from './mongo_client'; import type { CommandOperationOptions, OperationParent } from './operations/command'; import type { Hint, OperationOptions } from './operations/operation'; import { PromiseProvider } from './promise_provider'; @@ -657,7 +657,10 @@ export function makeStateMachine(stateTable: StateTable): StateTransitionFunctio }; } -/** @public */ +/** + * @public + * @see https://github.com/mongodb/specifications/blob/master/source/mongodb-handshake/handshake.rst#hello-command + */ export interface ClientMetadata { driver: { name: string; @@ -670,7 +673,6 @@ export interface ClientMetadata { version: string; }; platform: string; - version?: string; application?: { name: string; }; @@ -689,13 +691,21 @@ export interface ClientMetadataOptions { // eslint-disable-next-line @typescript-eslint/no-var-requires const NODE_DRIVER_VERSION = require('../package.json').version; -export function makeClientMetadata(options?: ClientMetadataOptions): ClientMetadata { - options = options ?? {}; +export function makeClientMetadata( + options: Pick +): ClientMetadata { + const name = options.driverInfo.name ? `nodejs|${options.driverInfo.name}` : 'nodejs'; + const version = options.driverInfo.version + ? `${NODE_DRIVER_VERSION}|${options.driverInfo.version}` + : NODE_DRIVER_VERSION; + const platform = options.driverInfo.platform + ? `Node.js ${process.version}, ${os.endianness()}|${options.driverInfo.platform}` + : `Node.js ${process.version}, ${os.endianness()}`; const metadata: ClientMetadata = { driver: { - name: 'nodejs', - version: NODE_DRIVER_VERSION + name, + version }, os: { type: os.type(), @@ -703,30 +713,16 @@ export function makeClientMetadata(options?: ClientMetadataOptions): ClientMetad architecture: process.arch, version: os.release() }, - platform: `Node.js ${process.version}, ${os.endianness()} (unified)` + platform }; - // support optionally provided wrapping driver info - if (options.driverInfo) { - if (options.driverInfo.name) { - metadata.driver.name = `${metadata.driver.name}|${options.driverInfo.name}`; - } - - if (options.driverInfo.version) { - metadata.version = `${metadata.driver.version}|${options.driverInfo.version}`; - } - - if (options.driverInfo.platform) { - metadata.platform = `${metadata.platform}|${options.driverInfo.platform}`; - } - } - if (options.appName) { // MongoDB requires the appName not exceed a byte length of 128 - const buffer = Buffer.from(options.appName); - metadata.application = { - name: buffer.byteLength > 128 ? buffer.slice(0, 128).toString('utf8') : options.appName - }; + const name = + Buffer.byteLength(options.appName, 'utf8') <= 128 + ? options.appName + : Buffer.from(options.appName, 'utf8').subarray(0, 128).toString('utf8'); + metadata.application = { name }; } return metadata; diff --git a/test/integration/connection-monitoring-and-pooling/connection.test.ts b/test/integration/connection-monitoring-and-pooling/connection.test.ts index 226b00ab41..0945ad066d 100644 --- a/test/integration/connection-monitoring-and-pooling/connection.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection.test.ts @@ -5,7 +5,7 @@ import { connect } from '../../../src/cmap/connect'; import { Connection } from '../../../src/cmap/connection'; import { LEGACY_HELLO_COMMAND } from '../../../src/constants'; import { Topology } from '../../../src/sdam/topology'; -import { ns } from '../../../src/utils'; +import { makeClientMetadata, ns } from '../../../src/utils'; import { skipBrokenAuthTestBeforeEachHook } from '../../tools/runner/hooks/configuration'; import { assert as test, setupDatabase } from '../shared'; @@ -27,12 +27,13 @@ describe('Connection', function () { it('should execute a command against a server', { metadata: { requires: { apiVersion: false, topology: '!load-balanced' } }, test: function (done) { - const connectOptions = Object.assign( - { connectionType: Connection }, - this.configuration.options - ); + const connectOptions: Partial = { + connectionType: Connection, + ...this.configuration.options, + metadata: makeClientMetadata({ driverInfo: {} }) + }; - connect(connectOptions, (err, conn) => { + connect(connectOptions as any as ConnectionOptions, (err, conn) => { expect(err).to.not.exist; this.defer(_done => conn.destroy(_done)); @@ -49,12 +50,14 @@ describe('Connection', function () { it('should emit command monitoring events', { metadata: { requires: { apiVersion: false, topology: '!load-balanced' } }, test: function (done) { - const connectOptions = Object.assign( - { connectionType: Connection, monitorCommands: true }, - this.configuration.options - ); - - connect(connectOptions, (err, conn) => { + const connectOptions: Partial = { + connectionType: Connection, + monitorCommands: true, + ...this.configuration.options, + metadata: makeClientMetadata({ driverInfo: {} }) + }; + + connect(connectOptions as any as ConnectionOptions, (err, conn) => { expect(err).to.not.exist; this.defer(_done => conn.destroy(_done)); @@ -80,12 +83,13 @@ describe('Connection', function () { }, test: function (done) { const namespace = ns(`${this.configuration.db}.$cmd`); - const connectOptions = Object.assign( - { connectionType: Connection }, - this.configuration.options - ); + const connectOptions: Partial = { + connectionType: Connection, + ...this.configuration.options, + metadata: makeClientMetadata({ driverInfo: {} }) + }; - connect(connectOptions, (err, conn) => { + connect(connectOptions as any as ConnectionOptions, (err, conn) => { expect(err).to.not.exist; this.defer(_done => conn.destroy(_done)); diff --git a/test/integration/node-specific/topology.test.js b/test/integration/node-specific/topology.test.js index ee806b9691..88c0d33016 100644 --- a/test/integration/node-specific/topology.test.js +++ b/test/integration/node-specific/topology.test.js @@ -1,11 +1,14 @@ 'use strict'; const { expect } = require('chai'); +const { makeClientMetadata } = require('../../../src/utils'); describe('Topology', function () { it('should correctly track states of a topology', { metadata: { requires: { apiVersion: false, topology: '!load-balanced' } }, // apiVersion not supported by newTopology() test: function (done) { - const topology = this.configuration.newTopology(); + const topology = this.configuration.newTopology({ + metadata: makeClientMetadata({ driverInfo: {} }) + }); const states = []; topology.on('stateChanged', (_, newState) => states.push(newState)); diff --git a/test/tools/cmap_spec_runner.ts b/test/tools/cmap_spec_runner.ts index 634e732f3e..5900bb46a9 100644 --- a/test/tools/cmap_spec_runner.ts +++ b/test/tools/cmap_spec_runner.ts @@ -363,11 +363,8 @@ async function runCmapTest(test: CmapTest, threadContext: ThreadContext) { delete poolOptions.backgroundThreadIntervalMS; } - let metadata; - if (poolOptions.appName) { - metadata = makeClientMetadata({ appName: poolOptions.appName }); - delete poolOptions.appName; - } + const metadata = makeClientMetadata({ appName: poolOptions.appName, driverInfo: {} }); + delete poolOptions.appName; const operations = test.operations; const expectedError = test.error; diff --git a/test/unit/cmap/handshake/client_metadata.test.ts b/test/unit/cmap/handshake/client_metadata.test.ts new file mode 100644 index 0000000000..00ab7fe558 --- /dev/null +++ b/test/unit/cmap/handshake/client_metadata.test.ts @@ -0,0 +1,150 @@ +import { expect } from 'chai'; +import * as os from 'os'; + +import { makeClientMetadata } from '../../../../src/utils'; + +// eslint-disable-next-line @typescript-eslint/no-var-requires +const NODE_DRIVER_VERSION = require('../../../../package.json').version; + +describe('makeClientMetadata()', () => { + context('when driverInfo.platform is provided', () => { + it('appends driverInfo.platform to the platform field', () => { + const options = { + driverInfo: { platform: 'myPlatform' } + }; + const metadata = makeClientMetadata(options); + expect(metadata).to.deep.equal({ + driver: { + name: 'nodejs', + version: NODE_DRIVER_VERSION + }, + os: { + type: os.type(), + name: process.platform, + architecture: process.arch, + version: os.release() + }, + platform: `Node.js ${process.version}, ${os.endianness()}|myPlatform` + }); + }); + }); + + context('when driverInfo.name is provided', () => { + it('appends driverInfo.name to the driver.name field', () => { + const options = { + driverInfo: { name: 'myName' } + }; + const metadata = makeClientMetadata(options); + expect(metadata).to.deep.equal({ + driver: { + name: 'nodejs|myName', + version: NODE_DRIVER_VERSION + }, + os: { + type: os.type(), + name: process.platform, + architecture: process.arch, + version: os.release() + }, + platform: `Node.js ${process.version}, ${os.endianness()}` + }); + }); + }); + + context('when driverInfo.version is provided', () => { + it('appends driverInfo.version to the version field', () => { + const options = { + driverInfo: { version: 'myVersion' } + }; + const metadata = makeClientMetadata(options); + expect(metadata).to.deep.equal({ + driver: { + name: 'nodejs', + version: `${NODE_DRIVER_VERSION}|myVersion` + }, + os: { + type: os.type(), + name: process.platform, + architecture: process.arch, + version: os.release() + }, + platform: `Node.js ${process.version}, ${os.endianness()}` + }); + }); + }); + + context('when no custom driverInfo is provided', () => { + const metadata = makeClientMetadata({ driverInfo: {} }); + + it('does not append the driver info to the metadata', () => { + expect(metadata).to.deep.equal({ + driver: { + name: 'nodejs', + version: NODE_DRIVER_VERSION + }, + os: { + type: os.type(), + name: process.platform, + architecture: process.arch, + version: os.release() + }, + platform: `Node.js ${process.version}, ${os.endianness()}` + }); + }); + + it('does not set the application field', () => { + expect(metadata).not.to.have.property('application'); + }); + }); + + context('when app name is provided', () => { + context('when the app name is over 128 bytes', () => { + const longString = 'a'.repeat(300); + const options = { + appName: longString, + driverInfo: {} + }; + const metadata = makeClientMetadata(options); + + it('truncates the application name to <=128 bytes', () => { + expect(metadata.application?.name).to.be.a('string'); + // the above assertion fails if `metadata.application?.name` is undefined, so + // we can safely assert that it exists + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + expect(Buffer.byteLength(metadata.application!.name, 'utf8')).to.equal(128); + }); + }); + + context( + 'TODO(NODE-5150): fix appName truncation when multi-byte unicode charaters straddle byte 128', + () => { + const longString = '€'.repeat(300); + const options = { + appName: longString, + driverInfo: {} + }; + const metadata = makeClientMetadata(options); + + it('truncates the application name to 129 bytes', () => { + expect(metadata.application?.name).to.be.a('string'); + // the above assertion fails if `metadata.application?.name` is undefined, so + // we can safely assert that it exists + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + expect(Buffer.byteLength(metadata.application!.name, 'utf8')).to.equal(129); + }); + } + ); + + context('when the app name is under 128 bytes', () => { + const options = { + appName: 'myApplication', + driverInfo: {} + }; + const metadata = makeClientMetadata(options); + + it('sets the application name to the value', () => { + expect(metadata.application?.name).to.equal('myApplication'); + }); + }); + }); +}); diff --git a/test/unit/sdam/topology.test.js b/test/unit/sdam/topology.test.js index 978386697b..72b8033888 100644 --- a/test/unit/sdam/topology.test.js +++ b/test/unit/sdam/topology.test.js @@ -38,7 +38,8 @@ describe('Topology (unit)', function () { it('should correctly pass appname', function (done) { const server = new Topology([`localhost:27017`], { metadata: makeClientMetadata({ - appName: 'My application name' + appName: 'My application name', + driverInfo: {} }) }); @@ -46,7 +47,7 @@ describe('Topology (unit)', function () { done(); }); - it('should report the correct platform in client metadata', function (done) { + it('should report the correct platform in client metadata', async function () { const helloRequests = []; mockServer.setMessageHandler(request => { const doc = request.document; @@ -59,22 +60,17 @@ describe('Topology (unit)', function () { }); client = new MongoClient(`mongodb://${mockServer.uri()}/`); - client.connect(err => { - expect(err).to.not.exist; - client.db().command({ ping: 1 }, err => { - expect(err).to.not.exist; + await client.connect(); - expect(helloRequests).to.have.length.greaterThan(1); - helloRequests.forEach(helloRequest => - expect(helloRequest) - .nested.property('client.platform') - .to.match(/unified/) - ); + await client.db().command({ ping: 1 }); - done(); - }); - }); + expect(helloRequests).to.have.length.greaterThan(1); + for (const request of helloRequests) { + expect(request) + .nested.property('client.platform') + .to.match(/Node.js /); + } }); });