From b42a1b417e8a4e222000336b0fe9e94053d30d98 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 30 Jul 2021 13:03:33 -0400 Subject: [PATCH] fix(NODE-3058): accept null or undefined anywhere we permit nullish values (#2921) --- .eslintrc.json | 21 ++++++++++++++++++--- src/cmap/auth/scram.ts | 2 +- src/cmap/command_monitoring_events.ts | 8 ++++---- src/cmap/connection.ts | 6 +++--- src/cmap/stream_description.ts | 2 +- src/collection.ts | 4 ++-- src/connection_string.ts | 10 +++++----- src/cursor/abstract_cursor.ts | 10 ++++------ src/cursor/aggregation_cursor.ts | 2 +- src/cursor/find_cursor.ts | 4 ++-- src/error.ts | 2 +- src/explain.ts | 2 +- src/operations/command.ts | 4 ++-- src/operations/count.ts | 2 +- src/operations/delete.ts | 4 ++-- src/operations/find.ts | 4 ++-- src/operations/insert.ts | 2 +- src/operations/update.ts | 6 +++--- src/read_preference.ts | 2 +- src/sdam/topology_description.ts | 2 +- src/sessions.ts | 4 ++-- src/sort.ts | 2 +- src/utils.ts | 2 +- 23 files changed, 60 insertions(+), 47 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 3eefba936b..e0b4725e73 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -27,7 +27,7 @@ "tsdoc/syntax": "warn", "no-console": "error", - "eqeqeq": ["error", "always", { "null": "ignore" }], + "eqeqeq": ["error", "always", {"null": "ignore"}], "strict": ["error", "global"], "@typescript-eslint/no-explicit-any": "off", @@ -36,7 +36,19 @@ "error", { "selector": "TSEnumDeclaration", - "message": "Don't declare enums" + "message": "Do not declare enums" + }, + { + "selector": "BinaryExpression[operator=/[=!]==/] Identifier[name='undefined']", + "message": "Do not strictly check undefined" + }, + { + "selector": "BinaryExpression[operator=/[=!]==/] Literal[raw='null']", + "message": "Do not strictly check null" + }, + { + "selector": "BinaryExpression[operator=/[=!]==?/] Literal[value='undefined']", + "message": "Do not strictly check typeof undefined (NOTE: currently this rule only detects the usage of 'undefined' string literal so this could be a misfire)" } ] }, @@ -44,7 +56,10 @@ "files": ["*.d.ts"], "rules": { "prettier/prettier": "off", - "@typescript-eslint/no-empty-interface": "off" + "@typescript-eslint/no-empty-interface": "off", + "@typescript-eslint/no-misused-new": "off", + "@typescript-eslint/ban-types": "off", + "@typescript-eslint/no-unused-vars": "off" } }, { diff --git a/src/cmap/auth/scram.ts b/src/cmap/auth/scram.ts index a309d038c7..327bab94bc 100644 --- a/src/cmap/auth/scram.ts +++ b/src/cmap/auth/scram.ts @@ -311,7 +311,7 @@ const hiLengthMap = { function HI(data: string, salt: Buffer, iterations: number, cryptoMethod: CryptoMethod) { // omit the work if already generated const key = [data, salt.toString('base64'), iterations].join('_'); - if (_hiCache[key] !== undefined) { + if (_hiCache[key] != null) { return _hiCache[key]; } diff --git a/src/cmap/command_monitoring_events.ts b/src/cmap/command_monitoring_events.ts index 0140baf699..35e138ca29 100644 --- a/src/cmap/command_monitoring_events.ts +++ b/src/cmap/command_monitoring_events.ts @@ -212,7 +212,7 @@ function extractCommand(command: WriteProtocolMessageType): Document { // up-convert legacy find command result = { find: collectionName(command) }; Object.keys(LEGACY_FIND_QUERY_MAP).forEach(key => { - if (typeof command.query[key] !== 'undefined') { + if (command.query[key] != null) { result[LEGACY_FIND_QUERY_MAP[key]] = deepCopy(command.query[key]); } }); @@ -220,7 +220,7 @@ function extractCommand(command: WriteProtocolMessageType): Document { Object.keys(LEGACY_FIND_OPTIONS_MAP).forEach(key => { const legacyKey = key as keyof typeof LEGACY_FIND_OPTIONS_MAP; - if (typeof command[legacyKey] !== 'undefined') { + if (command[legacyKey] != null) { result[LEGACY_FIND_OPTIONS_MAP[legacyKey]] = deepCopy(command[legacyKey]); } }); @@ -232,7 +232,7 @@ function extractCommand(command: WriteProtocolMessageType): Document { } }); - if (typeof command.pre32Limit !== 'undefined') { + if (command.pre32Limit != null) { result.limit = command.pre32Limit; } @@ -286,7 +286,7 @@ function extractReply(command: WriteProtocolMessageType, reply?: Document) { } // is this a legacy find command? - if (command.query && typeof command.query.$query !== 'undefined') { + if (command.query && command.query.$query != null) { return { ok: 1, cursor: { diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index d5ef9f6945..685aa0c808 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -343,9 +343,9 @@ export class Connection extends TypedEventEmitter { options: CommandOptions | undefined, callback: Callback ): void { - if (typeof ns.db === 'undefined' || typeof ns === 'string') { + if (!(ns instanceof MongoDBNamespace)) { // TODO(NODE-3483): Replace this with a MongoCommandError - throw new MongoDriverError('Namespace cannot be a string'); + throw new MongoDriverError('Must provide a MongoDBNamespace instance'); } const readPreference = getReadPreference(cmd, options); @@ -453,7 +453,7 @@ export class Connection extends TypedEventEmitter { /** @internal */ query(ns: MongoDBNamespace, cmd: Document, options: QueryOptions, callback: Callback): void { - const isExplain = typeof cmd.$explain !== 'undefined'; + const isExplain = cmd.$explain != null; const readPreference = options.readPreference ?? ReadPreference.primary; const batchSize = options.batchSize || 0; const limit = options.limit; diff --git a/src/cmap/stream_description.ts b/src/cmap/stream_description.ts index fe92cb218d..5109b7e35d 100644 --- a/src/cmap/stream_description.ts +++ b/src/cmap/stream_description.ts @@ -51,7 +51,7 @@ export class StreamDescription { receiveResponse(response: Document): void { this.type = parseServerType(response); RESPONSE_FIELDS.forEach(field => { - if (typeof response[field] !== 'undefined') { + if (typeof response[field] != null) { this[field] = response[field]; } diff --git a/src/collection.ts b/src/collection.ts index f7d1840c3b..e40d5a2fb0 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -699,7 +699,7 @@ export class Collection { options?: FindOptions | Callback, callback?: Callback ): Promise | void { - if (callback !== undefined && typeof callback !== 'function') { + if (callback != null && typeof callback !== 'function') { throw new MongoInvalidArgumentError( 'Third parameter to `findOne()` must be a callback or undefined' ); @@ -1088,7 +1088,7 @@ export class Collection { options?: CountDocumentsOptions | Callback, callback?: Callback ): Promise | void { - if (typeof filter === 'undefined') { + if (filter == null) { (filter = {}), (options = {}), (callback = undefined); } else if (typeof filter === 'function') { (callback = filter as Callback), (filter = {}), (options = {}); diff --git a/src/connection_string.ts b/src/connection_string.ts index e4f771543f..f38a7ee089 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -177,7 +177,7 @@ function toRecord(value: string): Record { const keyValuePairs = value.split(','); for (const keyValue of keyValuePairs) { const [key, value] = keyValue.split(':'); - if (typeof value === 'undefined') { + if (value == null) { throw new MongoParseError('Cannot have undefined values in key value pairs'); } try { @@ -219,7 +219,7 @@ export function parseOptions( mongoClient: MongoClient | MongoClientOptions | undefined = undefined, options: MongoClientOptions = {} ): MongoOptions { - if (typeof mongoClient !== 'undefined' && !(mongoClient instanceof MongoClient)) { + if (mongoClient != null && !(mongoClient instanceof MongoClient)) { options = mongoClient; mongoClient = undefined; } @@ -284,7 +284,7 @@ export function parseOptions( } const objectOptions = new CaseInsensitiveMap( - Object.entries(options).filter(([, v]) => (v ?? null) !== null) + Object.entries(options).filter(([, v]) => v != null) ); const allOptions = new CaseInsensitiveMap(); @@ -418,7 +418,7 @@ function setOption( mongoOptions[name] = getUint(name, values[0]); break; case 'string': - if (values[0] === undefined) { + if (values[0] == null) { break; } mongoOptions[name] = String(values[0]); @@ -1051,6 +1051,6 @@ export const OPTIONS = { export const DEFAULT_OPTIONS = new CaseInsensitiveMap( Object.entries(OPTIONS) - .filter(([, descriptor]) => typeof descriptor.default !== 'undefined') + .filter(([, descriptor]) => descriptor.default != null) .map(([k, d]) => [k, d.default]) ); diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts index 1be6e37114..6a62fc94b8 100644 --- a/src/cursor/abstract_cursor.ts +++ b/src/cursor/abstract_cursor.ts @@ -150,7 +150,7 @@ export abstract class AbstractCursor< this[kOptions].batchSize = options.batchSize; } - if (typeof options.comment !== 'undefined') { + if (options.comment != null) { this[kOptions].comment = options.comment; } @@ -225,9 +225,7 @@ export abstract class AbstractCursor< return { next: () => this.next().then(value => - value !== null && value !== undefined - ? { value, done: false } - : { value: undefined, done: true } + value != null ? { value, done: false } : { value: undefined, done: true } ) }; } @@ -817,7 +815,7 @@ function makeCursorStream(cursor: AbstractCursor { - needToClose = err ? !cursor.closed : result !== null; + needToClose = err ? !cursor.closed : result != null; if (err) { // NOTE: This is questionable, but we have a test backing the behavior. It seems the @@ -839,7 +837,7 @@ function makeCursorStream(cursor: AbstractCursor extends AbstractCursor ): Promise | void { if (typeof verbosity === 'function') (callback = verbosity), (verbosity = true); - if (verbosity === undefined) verbosity = true; + if (verbosity == null) verbosity = true; return executeOperation( this.topology, diff --git a/src/cursor/find_cursor.ts b/src/cursor/find_cursor.ts index 4f48815f9b..3be14b4f11 100644 --- a/src/cursor/find_cursor.ts +++ b/src/cursor/find_cursor.ts @@ -52,7 +52,7 @@ export class FindCursor extends AbstractCursor { this[kFilter] = filter || {}; this[kBuiltOptions] = options; - if (typeof options.sort !== 'undefined') { + if (options.sort != null) { this[kBuiltOptions].sort = formatSort(options.sort); } } @@ -155,7 +155,7 @@ export class FindCursor extends AbstractCursor { callback?: Callback ): Promise | void { if (typeof verbosity === 'function') (callback = verbosity), (verbosity = true); - if (verbosity === undefined) verbosity = true; + if (verbosity == null) verbosity = true; return executeOperation( this.topology, diff --git a/src/error.ts b/src/error.ts index 1a04516d74..14be0e12e3 100644 --- a/src/error.ts +++ b/src/error.ts @@ -898,7 +898,7 @@ export function isResumableError(error?: MongoError, wireVersion?: number): bool return true; } - if (typeof wireVersion !== 'undefined' && wireVersion >= 9) { + if (wireVersion != null && wireVersion >= 9) { // DRIVERS-1308: For 4.4 drivers running against 4.4 servers, drivers will add a special case to treat the CursorNotFound error code as resumable if (error && error instanceof MongoError && error.code === 43) { return true; diff --git a/src/explain.ts b/src/explain.ts index 9d2c6d34cd..0d08e694a6 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -40,7 +40,7 @@ export class Explain { } static fromOptions(options?: ExplainOptions): Explain | undefined { - if (options?.explain === undefined) return; + if (options?.explain == null) return; const explain = options.explain; if (typeof explain === 'boolean' || typeof explain === 'string') { diff --git a/src/operations/command.ts b/src/operations/command.ts index 0b11e444cd..9912f055d3 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -97,14 +97,14 @@ export abstract class CommandOperation extends AbstractOperation { if (this.hasAspect(Aspect.EXPLAINABLE)) { this.explain = Explain.fromOptions(options); - } else if (options?.explain !== undefined) { + } else if (options?.explain != null) { throw new MongoInvalidArgumentError(`Option "explain" is not supported on this command`); } } get canRetryWrite(): boolean { if (this.hasAspect(Aspect.EXPLAINABLE)) { - return this.explain === undefined; + return this.explain == null; } return true; } diff --git a/src/operations/count.ts b/src/operations/count.ts index 232f5c38ef..c50d881b5f 100644 --- a/src/operations/count.ts +++ b/src/operations/count.ts @@ -47,7 +47,7 @@ export class CountOperation extends CommandOperation { cmd.skip = options.skip; } - if (typeof options.hint !== 'undefined') { + if (options.hint != null) { cmd.hint = options.hint; } diff --git a/src/operations/delete.ts b/src/operations/delete.ts index 83124aea13..c8c15b74bd 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -64,7 +64,7 @@ export class DeleteOperation extends CommandOperation { return false; } - return this.statements.every(op => (typeof op.limit !== 'undefined' ? op.limit > 0 : true)); + return this.statements.every(op => (op.limit != null ? op.limit > 0 : true)); } execute(server: Server, session: ClientSession, callback: Callback): void { @@ -80,7 +80,7 @@ export class DeleteOperation extends CommandOperation { command.let = options.let; } - if (options.explain !== undefined && maxWireVersion(server) < 3) { + if (options.explain != null && maxWireVersion(server) < 3) { return callback ? callback( new MongoCompatibilityError(`Server ${server.name} does not support explain on delete`) diff --git a/src/operations/find.ts b/src/operations/find.ts index aa7d8a1230..4f2b15b904 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -106,7 +106,7 @@ export class FindOperation extends CommandOperation { const serverWireVersion = maxWireVersion(server); const options = this.options; - if (typeof options.allowDiskUse !== 'undefined' && serverWireVersion < 4) { + if (options.allowDiskUse != null && serverWireVersion < 4) { callback( new MongoCompatibilityError('Option "allowDiskUse" is not supported on MongoDB < 3.2') ); @@ -338,7 +338,7 @@ function makeLegacyFindCommand( findCommand.$maxTimeMS = options.maxTimeMS; } - if (typeof options.explain !== 'undefined') { + if (options.explain != null) { findCommand.$explain = true; } diff --git a/src/operations/insert.ts b/src/operations/insert.ts index 65ee2faf70..726cb921bc 100644 --- a/src/operations/insert.ts +++ b/src/operations/insert.ts @@ -37,7 +37,7 @@ export class InsertOperation extends CommandOperation { command.bypassDocumentValidation = options.bypassDocumentValidation; } - if (typeof options.comment !== 'undefined') { + if (options.comment != null) { command.comment = options.comment; } diff --git a/src/operations/update.ts b/src/operations/update.ts index b30c45716a..fa69973411 100644 --- a/src/operations/update.ts +++ b/src/operations/update.ts @@ -159,7 +159,7 @@ export class UpdateOneOperation extends UpdateOperation { ): void { super.execute(server, session, (err, res) => { if (err || !res) return callback(err); - if (typeof this.explain !== 'undefined') return callback(undefined, res); + if (this.explain != null) return callback(undefined, res); if (res.code) return callback(new MongoServerError(res)); if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0])); @@ -196,7 +196,7 @@ export class UpdateManyOperation extends UpdateOperation { ): void { super.execute(server, session, (err, res) => { if (err || !res) return callback(err); - if (typeof this.explain !== 'undefined') return callback(undefined, res); + if (this.explain != null) return callback(undefined, res); if (res.code) return callback(new MongoServerError(res)); if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0])); @@ -250,7 +250,7 @@ export class ReplaceOneOperation extends UpdateOperation { ): void { super.execute(server, session, (err, res) => { if (err || !res) return callback(err); - if (typeof this.explain !== 'undefined') return callback(undefined, res); + if (this.explain != null) return callback(undefined, res); if (res.code) return callback(new MongoServerError(res)); if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0])); diff --git a/src/read_preference.ts b/src/read_preference.ts index a0bfbd7504..d0ffd6e3c5 100644 --- a/src/read_preference.ts +++ b/src/read_preference.ts @@ -86,7 +86,7 @@ export class ReadPreference { if (!ReadPreference.isValid(mode)) { throw new MongoInvalidArgumentError(`Invalid read preference mode ${JSON.stringify(mode)}`); } - if (options === undefined && typeof tags === 'object' && !Array.isArray(tags)) { + if (options == null && typeof tags === 'object' && !Array.isArray(tags)) { options = tags; tags = undefined; } else if (tags && !Array.isArray(tags)) { diff --git a/src/sdam/topology_description.ts b/src/sdam/topology_description.ts index fab332fed4..1dd5956780 100644 --- a/src/sdam/topology_description.ts +++ b/src/sdam/topology_description.ts @@ -113,7 +113,7 @@ export class TopologyDescription { break; } - if (this.logicalSessionTimeoutMinutes === undefined) { + if (this.logicalSessionTimeoutMinutes == null) { // First server with a non null logicalSessionsTimeout this.logicalSessionTimeoutMinutes = server.logicalSessionTimeoutMinutes; continue; diff --git a/src/sessions.ts b/src/sessions.ts index 08eba0ec4d..ada596af48 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -854,7 +854,7 @@ export function applySession( Object.assign(command.readConcern, { afterClusterTime: session.operationTime }); } else if (session[kSnapshotEnabled]) { command.readConcern = command.readConcern || { level: ReadConcernLevel.snapshot }; - if (session[kSnapshotTime] !== undefined) { + if (session[kSnapshotTime] != null) { Object.assign(command.readConcern, { atClusterTime: session[kSnapshotTime] }); } } @@ -897,7 +897,7 @@ export function updateSessionFromResponse(session: ClientSession, document: Docu session.transaction._recoveryToken = document.recoveryToken; } - if (session?.[kSnapshotEnabled] && session[kSnapshotTime] === undefined) { + if (session?.[kSnapshotEnabled] && session[kSnapshotTime] == null) { // find and aggregate commands return atClusterTime on the cursor // distinct includes it in the response body const atClusterTime = document.cursor?.atClusterTime || document.atClusterTime; diff --git a/src/sort.ts b/src/sort.ts index baf63cf62d..6b766b5456 100644 --- a/src/sort.ts +++ b/src/sort.ts @@ -51,7 +51,7 @@ function prepareDirection(direction: any = 1): SortDirectionForCmd { /** @internal */ function isMeta(t: SortDirection): t is { $meta: string } { - return typeof t === 'object' && t !== null && '$meta' in t && typeof t.$meta === 'string'; + return typeof t === 'object' && t != null && '$meta' in t && typeof t.$meta === 'string'; } /** @internal */ diff --git a/src/utils.ts b/src/utils.ts index 8da2f6885d..de4b899da7 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -687,7 +687,7 @@ export function maxWireVersion(topologyOrServer?: Connection | Topology | Server if ( topologyOrServer.description && 'maxWireVersion' in topologyOrServer.description && - 'undefined' !== typeof topologyOrServer.description.maxWireVersion + topologyOrServer.description.maxWireVersion != null ) { return topologyOrServer.description.maxWireVersion; }