feat(NODE-4711)!: remove evalFunctions option (#539)

nbbeeken · web-flow · commit 0427eb588073 · 2022-12-10T00:02:42.000+01:00
diff --git a/docs/upgrade-to-v5.md b/docs/upgrade-to-v5.md
@@ -179,9 +179,41 @@ EJSON.parse("...",  { strict: false }); /* migrate to */ EJSON.parse("...",  { r
 // stringify
 EJSON.stringify({}, { strict: true  }); /* migrate to */ EJSON.stringify({}, { relaxed: false });
 EJSON.stringify({}, { strict: false }); /* migrate to */ EJSON.stringify({}, { relaxed: true });
+```
 
 ### The BSON default export has been removed.
 
 * If you import BSON commonjs style `const BSON = require('bson')` then the `BSON.default` property is no longer present.
 * If you import BSON esmodule style `import BSON from 'bson'` then this code will crash upon loading. **TODO: This is not the case right now but it will be after NODE-4713.**
   * This error will throw: `SyntaxError: The requested module 'bson' does not provide an export named 'default'`.
+
+### `class Code` always converts `.code` to string
+
+The `Code` class still supports the same constructor arguments as before.
+It will now convert the first argument to a string before saving it to the code property, see the following:
+
+```typescript
+const myCode = new Code(function iLoveJavascript() { console.log('I love javascript') });
+// myCode.code === "function iLoveJavascript() { console.log('I love javascript') }"
+// typeof myCode.code === 'string'
+```
+
+### `BSON.deserialize()` only returns `Code` instances
+
+The deserialize options: `evalFunctions`, `cacheFunctions`, and `cacheFunctionsCrc32` have been removed.
+The `evalFunctions` option, when enabled, would return BSON Code typed values as eval-ed javascript functions, now it will always return Code instances.
+
+See the following snippet for how to migrate:
+```typescript
+const bsonBytes = BSON.serialize(
+  { iLoveJavascript: function () { console.log('I love javascript') } },
+  { serializeFunctions: true } // serializeFunctions still works!
+);
+const result = BSON.deserialize(bsonBytes)
+// result.iLoveJavascript instanceof Code
+// result.iLoveJavascript.code === "function () { console.log('I love javascript') }"
+const iLoveJavascript = new Function(`return ${result.iLoveJavascript.code}`)();
+iLoveJavascript();
+// prints "I love javascript"
+// iLoveJavascript.name === "iLoveJavascript"
+```
diff --git a/src/code.ts b/src/code.ts
@@ -2,7 +2,7 @@ import type { Document } from './bson';
 
 /** @public */
 export interface CodeExtended {
-  $code: string | Function;
+  $code: string;
   $scope?: Document;
 }
 
@@ -16,19 +16,27 @@ export class Code {
     return 'Code';
   }
 
-  code!: string | Function;
-  scope?: Document;
+  code: string;
+
+  // a code instance having a null scope is what determines whether
+  // it is BSONType 0x0D (just code) / 0x0F (code with scope)
+  scope: Document | null;
+
   /**
    * @param code - a string or function.
    * @param scope - an optional scope for the function.
    */
-  constructor(code: string | Function, scope?: Document) {
-    this.code = code;
-    this.scope = scope;
+  constructor(code: string | Function, scope?: Document | null) {
+    this.code = code.toString();
+    this.scope = scope ?? null;
   }
 
-  toJSON(): { code: string | Function; scope?: Document } {
-    return { code: this.code, scope: this.scope };
+  toJSON(): { code: string; scope?: Document } {
+    if (this.scope != null) {
+      return { code: this.code, scope: this.scope };
+    }
+
+    return { code: this.code };
   }
 
   /** @internal */
@@ -53,7 +61,7 @@ export class Code {
   inspect(): string {
     const codeJson = this.toJSON();
     return `new Code("${String(codeJson.code)}"${
-      codeJson.scope ? `, ${JSON.stringify(codeJson.scope)}` : ''
+      codeJson.scope != null ? `, ${JSON.stringify(codeJson.scope)}` : ''
     })`;
   }
 }
diff --git a/src/parser/calculate_size.ts b/src/parser/calculate_size.ts
@@ -2,7 +2,7 @@ import { Binary } from '../binary';
 import type { Document } from '../bson';
 import * as constants from '../constants';
 import { ByteUtils } from '../utils/byte_utils';
-import { isAnyArrayBuffer, isDate, isRegExp, normalizedFunctionString } from './utils';
+import { isAnyArrayBuffer, isDate, isRegExp } from './utils';
 
 export function calculateObjectSize(
   object: Document,
@@ -189,38 +189,14 @@ function calculateElement(
         );
       }
     case 'function':
-      // WTF for 0.4.X where typeof /someregexp/ === 'function'
-      if (value instanceof RegExp || isRegExp(value) || String.call(value) === '[object RegExp]') {
+      if (serializeFunctions) {
         return (
           (name != null ? ByteUtils.utf8ByteLength(name) + 1 : 0) +
           1 +
-          ByteUtils.utf8ByteLength(value.source) +
-          1 +
-          (value.global ? 1 : 0) +
-          (value.ignoreCase ? 1 : 0) +
-          (value.multiline ? 1 : 0) +
+          4 +
+          ByteUtils.utf8ByteLength(value.toString()) +
           1
         );
-      } else {
-        if (serializeFunctions && value.scope != null && Object.keys(value.scope).length > 0) {
-          return (
-            (name != null ? ByteUtils.utf8ByteLength(name) + 1 : 0) +
-            1 +
-            4 +
-            4 +
-            ByteUtils.utf8ByteLength(normalizedFunctionString(value)) +
-            1 +
-            calculateObjectSize(value.scope, serializeFunctions, ignoreUndefined)
-          );
-        } else if (serializeFunctions) {
-          return (
-            (name != null ? ByteUtils.utf8ByteLength(name) + 1 : 0) +
-            1 +
-            4 +
-            ByteUtils.utf8ByteLength(normalizedFunctionString(value)) +
-            1
-          );
-        }
       }
   }
 
diff --git a/src/parser/deserializer.ts b/src/parser/deserializer.ts
@@ -19,16 +19,6 @@ import { validateUtf8 } from '../validate_utf8';
 
 /** @public */
 export interface DeserializeOptions {
-  /** evaluate functions in the BSON document scoped to the object deserialized. */
-  evalFunctions?: boolean;
-  /** cache evaluated functions for reuse. */
-  cacheFunctions?: boolean;
-  /**
-   * use a crc32 code for caching, otherwise use the string of the function.
-   * @deprecated this option to use the crc32 function never worked as intended
-   * due to the fact that the crc32 function itself was never implemented.
-   * */
-  cacheFunctionsCrc32?: boolean;
   /** when deserializing a Long will fit it into a Number if it's smaller than 53 bits */
   promoteLongs?: boolean;
   /** when deserializing a Binary will return it as a node.js Buffer instance. */
@@ -67,8 +57,6 @@ export interface DeserializeOptions {
 const JS_INT_MAX_LONG = Long.fromNumber(constants.JS_INT_MAX);
 const JS_INT_MIN_LONG = Long.fromNumber(constants.JS_INT_MIN);
 
-const functionCache: { [hash: string]: Function } = {};
-
 export function deserialize(
   buffer: Uint8Array,
   options: DeserializeOptions,
@@ -120,9 +108,6 @@ function deserializeObject(
   options: DeserializeOptions,
   isArray = false
 ) {
-  const evalFunctions = options['evalFunctions'] == null ? false : options['evalFunctions'];
-  const cacheFunctions = options['cacheFunctions'] == null ? false : options['cacheFunctions'];
-
   const fieldsAsRaw = options['fieldsAsRaw'] == null ? null : options['fieldsAsRaw'];
 
   // Return raw bson buffer instead of parsing it
@@ -569,18 +554,7 @@ function deserializeObject(
         shouldValidateKey
       );
 
-      // If we are evaluating the functions
-      if (evalFunctions) {
-        // If we have cache enabled let's look for the md5 of the function in the cache
-        if (cacheFunctions) {
-          // Got to do this to avoid V8 deoptimizing the call due to finding eval
-          value = isolateEval(functionString, functionCache, object);
-        } else {
-          value = isolateEval(functionString);
-        }
-      } else {
-        value = new Code(functionString);
-      }
+      value = new Code(functionString);
 
       // Update parse index position
       index = index + stringSize;
@@ -643,20 +617,7 @@ function deserializeObject(
         throw new BSONError('code_w_scope total size is too long, clips outer document');
       }
 
-      // If we are evaluating the functions
-      if (evalFunctions) {
-        // If we have cache enabled let's look for the md5 of the function in the cache
-        if (cacheFunctions) {
-          // Got to do this to avoid V8 deoptimizing the call due to finding eval
-          value = isolateEval(functionString, functionCache, object);
-        } else {
-          value = isolateEval(functionString);
-        }
-
-        value.scope = scopeObject;
-      } else {
-        value = new Code(functionString, scopeObject);
-      }
+      value = new Code(functionString, scopeObject);
     } else if (elementType === constants.BSON_DATA_DBPOINTER) {
       // Get the code string size
       const stringSize =
@@ -728,28 +689,6 @@ function deserializeObject(
   return object;
 }
 
-/**
- * Ensure eval is isolated, store the result in functionCache.
- *
- * @internal
- */
-function isolateEval(
-  functionString: string,
-  functionCache?: { [hash: string]: Function },
-  object?: Document
-) {
-  // eslint-disable-next-line @typescript-eslint/no-implied-eval
-  if (!functionCache) return new Function(functionString);
-  // Check for cache hit, eval if missing and return cached function
-  if (functionCache[functionString] == null) {
-    // eslint-disable-next-line @typescript-eslint/no-implied-eval
-    functionCache[functionString] = new Function(functionString);
-  }
-
-  // Set the object
-  return functionCache[functionString].bind(object);
-}
-
 function getValidatedString(
   buffer: Uint8Array,
   start: number,
diff --git a/src/parser/serializer.ts b/src/parser/serializer.ts
@@ -13,15 +13,7 @@ import type { MinKey } from '../min_key';
 import type { ObjectId } from '../objectid';
 import type { BSONRegExp } from '../regexp';
 import { ByteUtils } from '../utils/byte_utils';
-import {
-  isBigInt64Array,
-  isBigUInt64Array,
-  isDate,
-  isMap,
-  isRegExp,
-  isUint8Array,
-  normalizedFunctionString
-} from './utils';
+import { isBigInt64Array, isBigUInt64Array, isDate, isMap, isRegExp, isUint8Array } from './utils';
 
 /** @public */
 export interface SerializeOptions {
@@ -386,22 +378,15 @@ function serializeDouble(buffer: Uint8Array, key: string, value: Double, index:
   return index;
 }
 
-function serializeFunction(
-  buffer: Uint8Array,
-  key: string,
-  value: Function,
-  index: number,
-  _checkKeys = false,
-  _depth = 0
-) {
+function serializeFunction(buffer: Uint8Array, key: string, value: Function, index: number) {
   buffer[index++] = constants.BSON_DATA_CODE;
   // Number of written bytes
   const numberOfWrittenBytes = ByteUtils.encodeUTF8Into(buffer, key, index);
   // Encode the name
   index = index + numberOfWrittenBytes;
   buffer[index++] = 0;
   // Function string
-  const functionString = normalizedFunctionString(value);
+  const functionString = value.toString();
 
   // Write the string
   const size = ByteUtils.encodeUTF8Into(buffer, functionString, index + 4) + 1;
@@ -441,7 +426,7 @@ function serializeCode(
 
     // Serialize the function
     // Get the function string
-    const functionString = typeof value.code === 'string' ? value.code : value.code.toString();
+    const functionString = value.code;
     // Index adjustment
     index = index + 4;
     // Write string into buffer
@@ -679,7 +664,7 @@ export function serializeInto(
       } else if (value['_bsontype'] === 'Double') {
         index = serializeDouble(buffer, key, value, index);
       } else if (typeof value === 'function' && serializeFunctions) {
-        index = serializeFunction(buffer, key, value, index, checkKeys, depth);
+        index = serializeFunction(buffer, key, value, index);
       } else if (value['_bsontype'] === 'Code') {
         index = serializeCode(
           buffer,
@@ -790,7 +775,7 @@ export function serializeInto(
           ignoreUndefined
         );
       } else if (typeof value === 'function' && serializeFunctions) {
-        index = serializeFunction(buffer, key, value, index, checkKeys, depth);
+        index = serializeFunction(buffer, key, value, index);
       } else if (value['_bsontype'] === 'Binary') {
         index = serializeBinary(buffer, key, value, index);
       } else if (value['_bsontype'] === 'Symbol') {
@@ -894,7 +879,7 @@ export function serializeInto(
           ignoreUndefined
         );
       } else if (typeof value === 'function' && serializeFunctions) {
-        index = serializeFunction(buffer, key, value, index, checkKeys, depth);
+        index = serializeFunction(buffer, key, value, index);
       } else if (value['_bsontype'] === 'Binary') {
         index = serializeBinary(buffer, key, value, index);
       } else if (value['_bsontype'] === 'Symbol') {
diff --git a/src/parser/utils.ts b/src/parser/utils.ts
@@ -1,11 +1,3 @@
-/**
- * Normalizes our expected stringified form of a function across versions of node
- * @param fn - The function to stringify
- */
-export function normalizedFunctionString(fn: Function): string {
-  return fn.toString().replace('function(', 'function (');
-}
-
 export function isAnyArrayBuffer(value: unknown): value is ArrayBuffer {
   return ['[object ArrayBuffer]', '[object SharedArrayBuffer]'].includes(
     Object.prototype.toString.call(value)
diff --git a/test/node/code.test.ts b/test/node/code.test.ts
diff --git a/test/node/parser/deserializer.test.ts b/test/node/parser/deserializer.test.ts
diff --git a/test/types/bson.test-d.ts b/test/types/bson.test-d.ts
