fix(NODE-5161): metadata duplication in handshake (#3628)

Co-authored-by: Bailey Pearson <[email protected]>

Author: nbbeeken

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;

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
   };
 

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: {

src/mongo_client.ts

@@ -770,6 +770,7 @@ export interface MongoOptions
       >
     >,
     SupportedNodeConnectionOptions {
+  appName?: string;
   hosts: HostAddress[];
   srvHost?: string;
   credentials?: MongoCredentials;

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,44 +691,38 @@ 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<MongoOptions, 'appName' | 'driverInfo'>
+): 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(),
       name: process.platform,
       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;

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<ConnectionOptions> = {
+          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<ConnectionOptions> = {
+          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<ConnectionOptions> = {
+          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));
 

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));

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;