ready for review

Author: aditi-khare-mongoDB

src/int_32.ts

@@ -3,6 +3,7 @@ import { BSON_INT32_MAX, BSON_INT32_MIN } from './constants';
 import { BSONError } from './error';
 import type { EJSONOptions } from './extended_json';
 import { type InspectFn, defaultInspect } from './parser/utils';
+import { removeLeadingZeros } from './utils/string_utils';
 
 /** @public */
 export interface Int32Extended {
@@ -48,11 +49,7 @@ export class Int32 extends BSONValue {
    * @param value - the string we want to represent as an int32.
    */
   static fromString(value: string): Int32 {
-    const cleanedValue = !/[^0]+/.test(value)
-      ? value.replace(/^0+/, '0') // all zeros case
-      : value[0] === '-'
-        ? value.replace(/^-0+/, '-') // negative number with leading zeros
-        : value.replace(/^\+?0+/, ''); // positive number with leading zeros
+    const cleanedValue = removeLeadingZeros(value);
 
     const coercedValue = Number(value);
 

src/long.ts

@@ -3,6 +3,7 @@ import { BSONError } from './error';
 import type { EJSONOptions } from './extended_json';
 import { type InspectFn, defaultInspect } from './parser/utils';
 import type { Timestamp } from './timestamp';
+import { removeLeadingZeros } from './utils/string_utils';
 
 interface LongWASMHelpers {
   /** Gets the high bits of the last operation performed */
@@ -261,16 +262,16 @@ export class Long extends BSONValue {
       const validCharRangeEnd = String.fromCharCode('a'.charCodeAt(0) + (radix - 11));
       regexInputString = `[^-0-9+(a-${validCharRangeEnd})]`;
     }
-    const regex = new RegExp(regexInputString, '\i');
+    const regex = new RegExp(regexInputString, 'i');
     return regex.test(str) ? false : str;
   }
 
   /**
    * @internal
    * Returns a Long representation of the given string, written using the specified radix.
-   * This method throws an error, if the following both of the conditions are true:
+   * Throws an error if `throwsError` is set to true and any of the following conditions are true:
    *  - the string contains invalid characters for the given radix
-   *  - throwsError is true
+   *  - the string contains whitespace
    * @param str - The textual representation of the Long
    * @param unsigned - Whether unsigned or not, defaults to signed
    * @param radix - The radix in which the text is written (2-36), defaults to 10
@@ -293,15 +294,20 @@ export class Long extends BSONValue {
       unsigned = !!unsigned;
     }
     radix = radix || 10;
+
+    if (radix < 2 || 36 < radix) throw new BSONError('radix');
+
     if (throwsError && !Long.validateStringCharacters(str, radix)) {
-      throw new BSONError(`Input: ${str} contains invalid characters for radix: ${radix}`);
+      throw new BSONError(`Input: '${str}' contains invalid characters for radix: ${radix}`);
+    }
+    if (throwsError && str.trim() !== str) {
+      throw new BSONError(`Input: '${str}' contains whitespace.`);
     }
-    if (radix < 2 || 36 < radix) throw new BSONError('radix');
 
     let p;
     if ((p = str.indexOf('-')) > 0) throw new BSONError('interior hyphen');
     else if (p === 0) {
-      return Long.fromStringHelper(str.substring(1), unsigned, radix).neg();
+      return Long.fromStringHelper(str.substring(1), unsigned, radix, throwsError).neg();
     }
 
     // Do several (8) digits each time through the loop, so as to
@@ -325,16 +331,25 @@ export class Long extends BSONValue {
   }
 
   /**
-   * @internal - TODO(NODE-XXXX): fromStrictString throws on overflow
    * Returns a Long representation of the given string, written using the specified radix.
-   * If the string contains invalid characters for the given radix, this function will throw an error.
+   * Throws an error if any of the following conditions are true:
+   * - the string contains invalid characters for the given radix
+   * - the string contains whitespace
+   * - the value the string represents is too large or too small to be a Long
    * @param str - The textual representation of the Long
    * @param unsigned - Whether unsigned or not, defaults to signed
    * @param radix - The radix in which the text is written (2-36), defaults to 10
    * @returns The corresponding Long value
    */
   static fromStringStrict(str: string, unsigned?: boolean, radix?: number): Long {
-    return Long.fromStringHelper(str, unsigned, radix, true);
+    // remove leading zeros (for later string comparison and to make math faster)
+    const cleanedStr = removeLeadingZeros(str);
+    // doing this check outside of recursive function so cleanedStr value is consistent
+    const result = Long.fromStringHelper(cleanedStr, unsigned, radix, true);
+    if (result.toString(radix).toLowerCase() !== cleanedStr.toLowerCase()) {
+      throw new BSONError(`Input: ${str} is not representable as a Long`);
+    }
+    return result;
   }
 
   /**

src/utils/string_utils.ts

@@ -0,0 +1,11 @@
+/**
+ * @internal
+ * Removes leading zeros from textual representation of a number.
+ */
+export function removeLeadingZeros(str: string): string {
+  return !/[^0]+/.test(str)
+    ? str.replace(/^0+/, '0') // all zeros case
+    : str[0] === '-'
+      ? str.replace(/^-0+/, '-') // negative number with leading zeros
+      : str.replace(/^\+?0+/, '');
+}

test/node/long.test.ts

@@ -164,46 +164,73 @@ describe('Long', function () {
     });
   });
 
-  describe.only('static fromStringStrict()', function () {
+  describe('static fromStringStrict()', function () {
     const successInputs = [
-      ['basic no alphabet low radix', '1236', 8],
-      ['radix does allow given alphabet letter', 'eEe', 15],
-      ['hexadecimal letters', '126073efbcdADEF', 16],
-      ['negative hexadecimal letters', '-126073efbcdADEF', 16]
+      ['basic no alphabet low radix', '1236', true, 8],
+      ['negative basic no alphabet low radix', '-1236', false, 8],
+      ['valid upper and lower case letters in string with radix > 10', 'eEe', true, 15],
+      ['hexadecimal letters', '126073efbcdADEF', true, 16],
+      ['negative hexadecimal letters', '-1267efbcdDEF', false, 16],
+      ['negative leading zeros', '-00000032', false, 15, '-32'],
+      ['leading zeros', '00000032', false, 15, '32'],
+      ['explicit positive leading zeros', '+00000032', false, 15, '32'],
+      ['max unsigned binary input', Long.MAX_UNSIGNED_VALUE.toString(2), true, 2],
+      ['max unsigned decimal input', Long.MAX_UNSIGNED_VALUE.toString(10), true, 10],
+      ['max unsigned hex input', Long.MAX_UNSIGNED_VALUE.toString(16), true, 16],
+      ['max signed binary input', Long.MAX_VALUE.toString(2), false, 2],
+      ['max signed decimal input', Long.MAX_VALUE.toString(10), false, 10],
+      ['max signed hex input', Long.MAX_VALUE.toString(16), false, 16],
+      ['min signed binary input', Long.MIN_VALUE.toString(2), false, 2],
+      ['min signed decimal input', Long.MIN_VALUE.toString(10), false, 10],
+      ['min signed hex input', Long.MIN_VALUE.toString(16), false, 16],
+      ['signed zero', '0', false, 10],
+      ['unsigned zero', '0', true, 10]
     ];
 
     const failureInputs = [
-      ['empty string','', 2],
-      ['non a-z or numeric string', '~~', 36],
-      ['alphabet in radix < 10', 'a', 4],
-      ['radix does not allow all alphabet letters', 'eee', 14]
+      ['empty string', '', true, 2],
+      ['invalid numbers in binary string', '234', true, 2],
+      ['non a-z or numeric string', '~~', true, 36],
+      ['alphabet in radix < 10', 'a', true, 9],
+      ['radix does not allow all alphabet letters', 'eee', 14],
+      ['over max unsigned binary input', Long.MAX_UNSIGNED_VALUE.toString(2) + '1', true, 2],
+      ['over max unsigned decimal input', Long.MAX_UNSIGNED_VALUE.toString(10) + '1', true, 10],
+      ['over max unsigned hex input', Long.MAX_UNSIGNED_VALUE.toString(16) + '1', true, 16],
+      ['over max signed binary input', Long.MAX_VALUE.toString(2) + '1', false, 2],
+      ['over max signed decimal input', Long.MAX_VALUE.toString(10) + '1', false, 10],
+      ['over max signed hex input', Long.MAX_VALUE.toString(16) + '1', false, 16],
+      ['under min signed binary input', Long.MIN_VALUE.toString(2) + '1', false, 2],
+      ['under min signed decimal input', Long.MIN_VALUE.toString(10) + '1', false, 10],
+      ['under min signed hex input', Long.MIN_VALUE.toString(16) + '1', false, 16]
     ];
 
-    for (const [testName, str, radix] of successInputs) {
+    for (const [testName, str, unsigned, radix, expectedStr] of successInputs) {
       context(`when the input is ${testName}`, () => {
         it(`should return a input string`, () => {
-          expect(Long.fromStringStrict(str, true, radix).toString(radix)).to.equal(str.toLowerCase());
+          expect(Long.fromStringStrict(str, unsigned, radix).toString(radix)).to.equal(
+            expectedStr ?? str.toLowerCase()
+          );
         });
       });
     }
-    for (const [testName, str, radix] of failureInputs) {
+    for (const [testName, str, unsigned, radix] of failureInputs) {
       context(`when the input is ${testName}`, () => {
-        it(`should return false`, () => {
-          expect(() => Long.fromStringStrict(str, true, radix)).to.throw(BSONError);
+        it(`should throw BSONError`, () => {
+          expect(() => Long.fromStringStrict(str, unsigned, radix)).to.throw(BSONError);
         });
       });
     }
   });
 
   describe('static validateStringCharacters()', function () {
     const successInputs = [
-      ['multiple decimal points', '..', 30],
-      ['radix does not allow given alphabet letter', 'eEe', 15],
-      ['empty string','', 2], 
+      ['radix does allows given alphabet letter', 'eEe', 15],
+      ['empty string', '', 2],
       ['all possible hexadecimal characters', '12efabc689873dADCDEF', 16]
     ];
 
     const failureInputs = [
+      ['multiple decimal points', '..', 30],
       ['non a-z or numeric string', '~~', 36],
       ['alphabet in radix < 10', 'a', 4],
       ['radix does not allow all alphabet letters', 'eee', 14]

test/node/release.test.ts

@@ -48,6 +48,7 @@ const REQUIRED_FILES = [
   'src/utils/byte_utils.ts',
   'src/utils/node_byte_utils.ts',
   'src/utils/number_utils.ts',
+  'src/utils/string_utils.ts',
   'src/utils/web_byte_utils.ts',
   'src/utils/latin.ts',
   'src/validate_utf8.ts',