Repository: algorand/js-algorand-sdk
Pull request #599 (2022-06-26 18:55:20)

fix: safe intDecoding
This PR fixes a couple of bugs with safe mode for integer decoding during json parse:
- safe mode threw errors if `10**15 > value < Number.MAX_SAFE_INTEGER `. It would have shown in the unit test if we had tested for edge cases.
- safe mode did not decode integers as numbers. It didn't alter the output of JSONbig in any way.
It is again only an issue for those values that are `10**15 > value < Number.MAX_SAFE_INTEGER `


Patch:
diff --git a/src/utils/utils.ts b/src/utils/utils.ts
index 611a78c..c504a09 100644
--- a/src/utils/utils.ts
+++ b/src/utils/utils.ts
@@ -10,7 +10,6 @@ export interface JSONOptions {
 /**
  * Parse JSON with additional options.
  * @param str - The JSON string to parse.
- * @param options - Parsing options.
  * @param options - Options object to configure how integers in
  *   this request's JSON response will be decoded. Use the `intDecoding`
  *   property with one of the following options:
@@ -28,34 +27,32 @@ export interface JSONOptions {
 export function parseJSON(str: string, options?: JSONOptions) {
   const intDecoding =
     options && options.intDecoding ? options.intDecoding : IntDecoding.DEFAULT;
-  const parsed = JSONbig.parse(str, (_, value) => {
+  return JSONbig.parse(str, (_, value) => {
     if (
       value != null &&
       typeof value === 'object' &&
       Object.getPrototypeOf(value) == null
     ) {
-      // for some reason the Objects returned by JSONbig.parse have a null prototype, so we
-      // need to fix that.
+      // JSONbig.parse objects are created with Object.create(null) and thus have a null prototype
+      // let us remedy that
       Object.setPrototypeOf(value, Object.prototype);
     }
 
     if (typeof value === 'bigint') {
+      if (intDecoding === 'safe' && value > Number.MAX_SAFE_INTEGER) {
+        throw new Error(
+          `Integer exceeds maximum safe integer: ${value.toString()}. Try parsing with a different intDecoding option.`
+        );
+      }
       if (
         intDecoding === 'bigint' ||
         (intDecoding === 'mixed' && value > Number.MAX_SAFE_INTEGER)
       ) {
         return value;
       }
-
       // JSONbig.parse converts number to BigInts if they are >= 10**15. This is smaller than
       // Number.MAX_SAFE_INTEGER, so we can convert some BigInts back to normal numbers.
-      if (intDecoding === 'default' || intDecoding === 'mixed') {
-        return Number(value);
-      }
-
-      throw new Error(
-        `Integer exceeds maximum safe integer: ${value.toString()}. Try parsing with a different intDecoding option.`
-      );
+      return Number(value);
     }
 
     if (typeof value === 'number') {
@@ -66,7 +63,6 @@ export function parseJSON(str: string, options?: JSONOptions) {
 
     return value;
   });
-  return parsed;
 }
 
 /**


Test patch:
diff --git a/tests/2.Encoding.js b/tests/2.Encoding.js
index 25d011c..57d1452 100644
--- a/tests/2.Encoding.js
+++ b/tests/2.Encoding.js
@@ -354,17 +354,18 @@ describe('encoding', () => {
     });
 
     it('should parse number', () => {
-      const input = '17';
-
-      for (const intDecoding of ['default', 'safe', 'mixed', 'bigint']) {
-        const actual = utils.parseJSON(input, { intDecoding });
-        const expected = intDecoding === 'bigint' ? 17n : 17;
-
-        assert.deepStrictEqual(
-          actual,
-          expected,
-          `Error when intDecoding = ${intDecoding}`
-        );
+      const inputs = ['17', '9007199254740991'];
+      for (const input of inputs) {
+        for (const intDecoding of ['default', 'safe', 'mixed', 'bigint']) {
+          const actual = utils.parseJSON(input, { intDecoding });
+          const expected =
+            intDecoding === 'bigint' ? BigInt(input) : Number(input);
+          assert.deepStrictEqual(
+            actual,
+            expected,
+            `Error when intDecoding = ${intDecoding}`
+          );
+        }
       }
     });