Add stricter string validation to neo4j.int (#985)

bigmontz · OskarDamkjaer · web-flow · commit edde4fc130c7 · 2022-09-06T14:23:04.000+02:00
`neo4j.int` could have some surprising result when used with string. For avoiding this problem,
a configuration option called `strictStringValidation` was added. When enable, `strictStringValidation` will trigger a deeper validation of the string. This option could slow down the conversion.


Co-authored-by: Oskar Damkjaer &lt;OskarDamkjaer@users.noreply.github.com&gt;
diff --git a/packages/core/src/integer.ts b/packages/core/src/integer.ts
@@ -833,10 +833,12 @@ class Integer {
    * @access private
    * @param {string} str The textual representation of the Integer
    * @param {number=} radix The radix in which the text is written (2-36), defaults to 10
+   * @param {Object} [opts={}] Configuration options
+   * @param {boolean} [opts.strictStringValidation=false] Enable strict validation generated Integer.
    * @returns {!Integer} The corresponding Integer value
    * @expose
    */
-  static fromString (str: string, radix?: number): Integer {
+  static fromString (str: string, radix?: number, { strictStringValidation }: { strictStringValidation?: boolean} = {}): Integer {
     if (str.length === 0) {
       throw newError('number format error: empty string')
     }
@@ -867,7 +869,13 @@ class Integer {
     let result = Integer.ZERO
     for (let i = 0; i < str.length; i += 8) {
       const size = Math.min(8, str.length - i)
-      const value = parseInt(str.substring(i, i + size), radix)
+      const valueString = str.substring(i, i + size)
+      const value = parseInt(valueString, radix)
+
+      if (strictStringValidation === true && !_isValidNumberFromString(valueString, value, radix)) {
+        throw newError(`number format error: "${valueString}" is NaN in radix ${radix}: ${str}`)
+      }
+
       if (size < 8) {
         const power = Integer.fromNumber(Math.pow(radix, size))
         result = result.multiply(power).add(Integer.fromNumber(value))
@@ -883,18 +891,20 @@ class Integer {
    * Converts the specified value to a Integer.
    * @access private
    * @param {!Integer|number|string|bigint|!{low: number, high: number}} val Value
+   * @param {Object} [opts={}] Configuration options
+   * @param {boolean} [opts.strictStringValidation=false] Enable strict validation generated Integer.
    * @returns {!Integer}
    * @expose
    */
-  static fromValue (val: Integerable): Integer {
+  static fromValue (val: Integerable, opts: { strictStringValidation?: boolean} = {}): Integer {
     if (val /* is compatible */ instanceof Integer) {
       return val
     }
     if (typeof val === 'number') {
       return Integer.fromNumber(val)
     }
     if (typeof val === 'string') {
-      return Integer.fromString(val)
+      return Integer.fromString(val, undefined, opts)
     }
     if (typeof val === 'bigint') {
       return Integer.fromString(val.toString())
@@ -946,6 +956,20 @@ class Integer {
   }
 }
 
+/**
+ *
+ * @private
+ * @param theString
+ * @param theNumber
+ * @param radix
+ * @return {boolean} True if valid
+ */
+function _isValidNumberFromString (theString: string, theNumber: number, radix: number): boolean {
+  return !Number.isNaN(theString) &&
+    !Number.isNaN(theNumber) &&
+    theNumber.toString(radix).toLocaleLowerCase() === theString.toLocaleLowerCase()
+}
+
 type Integerable =
   | number
   | string
@@ -1011,6 +1035,8 @@ const TWO_PWR_24 = Integer.fromInt(TWO_PWR_24_DBL)
  * Cast value to Integer type.
  * @access public
  * @param {Mixed} value - The value to use.
+ * @param {Object} [opts={}] Configuration options
+ * @param {boolean} [opts.strictStringValidation=false] Enable strict validation generated Integer.
  * @return {Integer} - An object of type Integer.
  */
 const int = Integer.fromValue
diff --git a/packages/core/test/integer.test.ts b/packages/core/test/integer.test.ts
@@ -256,6 +256,26 @@ describe('Integer', () => {
       newError('number format error: interior "-" character: 123-2')
     ))
 
+  test('Integer.fromString("7891a", undefined, { strictStringValidation: true }) toThrow invalid character', () =>
+    expect(() => Integer.fromString('7891a', undefined, { strictStringValidation: true })).toThrow(
+      newError('number format error: "7891a" is NaN in radix 10: 7891a')
+    ))
+
+  test('Integer.fromString("78a91", undefined, { strictStringValidation: true }) toThrow invalid character', () =>
+    expect(() => Integer.fromString('78a91', undefined, { strictStringValidation: true })).toThrow(
+      newError('number format error: "78a91" is NaN in radix 10: 78a91')
+    ))
+
+  test('Integer.fromString("a7891", undefined, { strictStringValidation: true }) toThrow invalid character', () =>
+    expect(() => Integer.fromString('a7891', undefined, { strictStringValidation: true })).toThrow(
+      newError('number format error: "a7891" is NaN in radix 10: a7891')
+    ))
+
+  test('Integer.fromString("7010", 2, { strictStringValidation: true }) toThrow invalid character', () =>
+    expect(() => Integer.fromString('7010', 2, { strictStringValidation: true })).toThrow(
+      newError('number format error: "7010" is NaN in radix 2: 7010')
+    ))
+
   forEachFromValueScenarios(({ input, expectedOutput }) =>
     test(`Integer.fromValue(${mayIntegerToString(input)}) toEqual ${expectedOutput}`, () =>
       expect(Integer.fromValue(input)).toEqual(expectedOutput))
@@ -266,6 +286,29 @@ describe('Integer', () => {
       expect(int(input)).toEqual(expectedOutput))
   )
 
+  test('int("7891a", { strictStringValidation: true }) toThrow invalid character', () =>
+    expect(() => int('7891a', { strictStringValidation: true })).toThrow(
+      newError('number format error: "7891a" is NaN in radix 10: 7891a')
+    ))
+
+  test('int("78a91", { strictStringValidation: true }) toThrow invalid character', () =>
+    expect(() => int('78a91', { strictStringValidation: true })).toThrow(
+      newError('number format error: "78a91" is NaN in radix 10: 78a91')
+    ))
+
+  test('int("a7891", { strictStringValidation: true }) toThrow invalid character', () =>
+    expect(() => int('a7891', { strictStringValidation: true })).toThrow(
+      newError('number format error: "a7891" is NaN in radix 10: a7891')
+    ))
+
+  test('int("7891123456789876a", { strictStringValidation: true }) toThrow invalid character', () =>
+    expect(() => int('7891123456789876a', { strictStringValidation: true })).toThrow(
+      newError('number format error: "a" is NaN in radix 10: 7891123456789876a')
+    ))
+
+  test('int("7891123456789876a") not toThrow invalid character', () =>
+    expect(() => int('7891123456789876a')).not.toThrow())
+
   forEachStaticToNumberScenarios(({ input, expectedOutput }) =>
     test(`Integer.toNumber(${mayIntegerToString(input)}) toEqual ${expectedOutput}`, () =>
       expect(Integer.toNumber(input)).toEqual(expectedOutput))
