fix(NODE-3998): metadata duplication in handshake (#3615)

mongodb · Mar 31, 2023 · 6d894d6 · 6d894d6
1 parent b038cbb
commit 6d894d6
Show file tree

Hide file tree

Showing 11 changed files with 223 additions and 87 deletions.
diff --git a/src/cmap/auth/auth_provider.ts b/src/cmap/auth/auth_provider.ts
@@ -1,13 +1,9 @@
 import type { Document } from '../../bson';
 import { MongoRuntimeError } from '../../error';
-import type { ClientMetadataOptions } from '../../utils';
 import type { HandshakeDocument } from '../connect';
 import type { Connection, ConnectionOptions } from '../connection';
 import type { MongoCredentials } from './mongo_credentials';
 
-/** @internal */
-export type AuthContextOptions = ConnectionOptions & ClientMetadataOptions;
-
 /**
  * Context used during authentication
  * @internal
@@ -20,7 +16,7 @@ export class AuthContext {
   /** If the context is for reauthentication. */
   reauthenticating = false;
   /** 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;
@@ -30,7 +26,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
@@ -17,7 +17,7 @@ import {
   MongoRuntimeError,
   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';
@@ -213,7 +213,7 @@ export async function prepareHandshakeDocument(
   const handshakeDoc: HandshakeDocument = {
     [serverApi?.version ? 'hello' : LEGACY_HELLO_COMMAND]: 1,
     helloOk: true,
-    client: options.metadata || makeClientMetadata(options),
+    client: options.metadata,
     compression: compressors
   };
 

diff --git a/src/connection_string.ts b/src/connection_string.ts
@@ -15,7 +15,6 @@ import {
   MongoParseError
 } from './error';
 import {
-  DriverInfo,
   MongoClient,
   MongoClientOptions,
   MongoOptions,
@@ -543,6 +542,8 @@ export function parseOptions(
     loggerClientOptions
   );
 
+  mongoOptions.metadata = makeClientMetadata(mongoOptions);
+
   return mongoOptions;
 }
 
@@ -644,10 +645,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',
@@ -798,15 +796,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/index.ts b/src/index.ts
@@ -197,7 +197,7 @@ export type {
   ResumeToken,
   UpdateDescription
 } from './change_stream';
-export type { AuthContext, AuthContextOptions } from './cmap/auth/auth_provider';
+export type { AuthContext } from './cmap/auth/auth_provider';
 export type {
   AuthMechanismProperties,
   MongoCredentials,

diff --git a/src/mongo_client.ts b/src/mongo_client.ts
@@ -700,6 +700,7 @@ export interface MongoOptions
       >
     >,
     SupportedNodeConnectionOptions {
+  appName?: string;
   hosts: HostAddress[];
   srvHost?: string;
   credentials?: MongoCredentials;

diff --git 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 { ReadConcern } from './read_concern';
@@ -513,7 +513,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;
@@ -526,7 +529,6 @@ export interface ClientMetadata {
     version: string;
   };
   platform: string;
-  version?: string;
   application?: {
     name: string;
   };
@@ -545,44 +547,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;

diff --git a/test/integration/connection-monitoring-and-pooling/connection.test.ts b/test/integration/connection-monitoring-and-pooling/connection.test.ts
@@ -3,7 +3,9 @@ import { expect } from 'chai';
 import {
   connect,
   Connection,
+  ConnectionOptions,
   LEGACY_HELLO_COMMAND,
+  makeClientMetadata,
   MongoClient,
   MongoServerError,
   ns,
@@ -31,12 +33,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));
 
@@ -53,12 +56,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));
 
@@ -84,12 +89,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));
 

diff --git 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('../../mongodb');
 
 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
@@ -370,11 +370,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;