feat!(NODE-4706): validate Timestamp ctor argument (#536)

Author: nbbeeken

.evergreen/config.yml

@@ -139,7 +139,7 @@ tasks:
       - func: "run typescript"
         vars:
           TS_VERSION: "4.0.2"
-          TRY_COMPILING_LIBRARY: "true"
+          TRY_COMPILING_LIBRARY: "false"
   - name: check-typescript-current
     commands:
       - func: fetch source

docs/upgrade-to-v5.md

@@ -134,3 +134,36 @@ BSON.deserialize(BSON.serialize({ d: -0 }))
 ### Capital "D" ObjectID export removed
 
 For clarity the deprecated and duplicate export `ObjectID` has been removed. `ObjectId` matches the class name and is equal in every way to the capital "D" export.
+
+### Timestamp constructor validation
+
+The `Timestamp` type no longer accepts two number arguments for the low and high bits of the int64 value.
+
+Supported constructors are as follows:
+
+```typescript
+class Timestamp {
+  constructor(int: bigint);
+  constructor(long: Long);
+  constructor(value: { t: number; i: number });
+}
+```
+
+Any code that use the two number argument style of constructing a Timestamp will need to be migrated to one of the supported constructors. We recommend using the `{ t: number; i: number }` style input, representing the timestamp and increment respectively.
+
+```typescript
+// in 4.x BSON
+new Timestamp(1, 2); // as an int64: 8589934593
+// in 5.x BSON
+new Timestamp({ t: 2, i: 1 }); // as an int64: 8589934593
+```
+
+Additionally, the `t` and `i` fields of `{ t: number; i: number }` are now validated more strictly to ensure your Timestamps are being constructed as expected.
+
+For example:
+```typescript
+new Timestamp({ t: -2, i: 1 });
+// Will throw, both fields need to be positive
+new Timestamp({ t: 2, i: 0xFFFF_FFFF + 1 });
+// Will throw, both fields need to be less than or equal to the unsigned int32 max value
+```

package-lock.json

@@ -30,7 +30,7 @@
     "@babel/plugin-external-helpers": "^7.18.6",
     "@babel/preset-env": "^7.19.4",
     "@istanbuljs/nyc-config-typescript": "^1.0.2",
-    "@microsoft/api-extractor": "^7.33.5",
+    "@microsoft/api-extractor": "^7.33.6",
     "@rollup/plugin-babel": "^6.0.2",
     "@rollup/plugin-commonjs": "^23.0.2",
     "@rollup/plugin-json": "^5.0.1",
@@ -63,7 +63,7 @@
     "standard-version": "^9.5.0",
     "ts-node": "^10.9.1",
     "tsd": "^0.24.1",
-    "typescript": "^4.8.4",
+    "typescript": "^4.9.3",
     "typescript-cached-transpile": "0.0.6",
     "uuid": "^9.0.0",
     "v8-profiler-next": "^1.9.0"

package.json

@@ -1,7 +1,6 @@
 import type { Document } from './bson';
 import type { EJSONOptions } from './extended_json';
 import type { ObjectId } from './objectid';
-import { isObjectLike } from './parser/utils';
 
 /** @public */
 export interface DBRefLike {
@@ -13,10 +12,14 @@ export interface DBRefLike {
 /** @internal */
 export function isDBRefLike(value: unknown): value is DBRefLike {
   return (
-    isObjectLike(value) &&
+    value != null &&
+    typeof value === 'object' &&
+    '$id' in value &&
     value.$id != null &&
+    '$ref' in value &&
     typeof value.$ref === 'string' &&
-    (value.$db == null || typeof value.$db === 'string')
+    // If '$db' is defined it MUST be a string, otherwise it should be absent
+    (!('$db' in value) || ('$db' in value && typeof value.$db === 'string'))
   );
 }
 

src/db_ref.ts

@@ -10,7 +10,7 @@ import { Long } from './long';
 import { MaxKey } from './max_key';
 import { MinKey } from './min_key';
 import { ObjectId } from './objectid';
-import { isDate, isObjectLike, isRegExp } from './parser/utils';
+import { isDate, isRegExp } from './parser/utils';
 import { BSONRegExp } from './regexp';
 import { BSONSymbol } from './symbol';
 import { Timestamp } from './timestamp';
@@ -36,7 +36,10 @@ type BSONType =
 
 export function isBSONType(value: unknown): value is BSONType {
   return (
-    isObjectLike(value) && Reflect.has(value, '_bsontype') && typeof value._bsontype === 'string'
+    value != null &&
+    typeof value === 'object' &&
+    '_bsontype' in value &&
+    typeof value._bsontype === 'string'
   );
 }
 

src/extended_json.ts

@@ -1,5 +1,4 @@
 import type { EJSONOptions } from './extended_json';
-import { isObjectLike } from './parser/utils';
 import type { Timestamp } from './timestamp';
 
 interface LongWASMHelpers {
@@ -327,7 +326,12 @@ export class Long {
    * Tests if the specified object is a Long.
    */
   static isLong(value: unknown): value is Long {
-    return isObjectLike(value) && value['__isLong__'] === true;
+    return (
+      value != null &&
+      typeof value === 'object' &&
+      '__isLong__' in value &&
+      value.__isLong__ === true
+    );
   }
 
   /**

src/long.ts

@@ -530,18 +530,21 @@ function deserializeObject(
       value = promoteValues ? symbol : new BSONSymbol(symbol);
       index = index + stringSize;
     } else if (elementType === constants.BSON_DATA_TIMESTAMP) {
-      const lowBits =
-        buffer[index++] |
-        (buffer[index++] << 8) |
-        (buffer[index++] << 16) |
-        (buffer[index++] << 24);
-      const highBits =
-        buffer[index++] |
-        (buffer[index++] << 8) |
-        (buffer[index++] << 16) |
-        (buffer[index++] << 24);
-
-      value = new Timestamp(lowBits, highBits);
+      // We intentionally **do not** use bit shifting here
+      // Bit shifting in javascript coerces numbers to **signed** int32s
+      // We need to keep i, and t unsigned
+      const i =
+        buffer[index++] +
+        buffer[index++] * (1 << 8) +
+        buffer[index++] * (1 << 16) +
+        buffer[index++] * (1 << 24);
+      const t =
+        buffer[index++] +
+        buffer[index++] * (1 << 8) +
+        buffer[index++] * (1 << 16) +
+        buffer[index++] * (1 << 24);
+
+      value = new Timestamp({ i, t });
     } else if (elementType === constants.BSON_DATA_MIN_KEY) {
       value = new MinKey();
     } else if (elementType === constants.BSON_DATA_MAX_KEY) {

src/parser/deserializer.ts

@@ -32,16 +32,6 @@ export function isMap(d: unknown): d is Map<unknown, unknown> {
   return Object.prototype.toString.call(d) === '[object Map]';
 }
 
-// To ensure that 0.4 of node works correctly
 export function isDate(d: unknown): d is Date {
-  return isObjectLike(d) && Object.prototype.toString.call(d) === '[object Date]';
-}
-
-/**
- * @internal
- * this is to solve the `'someKey' in x` problem where x is unknown.
- * https://github.com/typescript-eslint/typescript-eslint/issues/1071#issuecomment-541955753
- */
-export function isObjectLike(candidate: unknown): candidate is Record<string, unknown> {
-  return typeof candidate === 'object' && candidate !== null;
+  return Object.prototype.toString.call(d) === '[object Date]';
 }