Make handling of long numerals an option that is disabled by default (#…

…557) Also: * Strengthen the tests * update USER_GUIDE.md Signed-off-by: Miki <[email protected]>
opensearch-project · Jul 14, 2023 · 08069bc · 08069bc
1 parent 97c808a
commit 08069bc
Show file tree

Hide file tree

Showing 8 changed files with 74 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 ### Added
 ### Dependencies
 ### Changed
+- Make handling of long numerals an option that is disabled by default ([#557](https://github.com/opensearch-project/opensearch-js/pull/557))
 ### Deprecated
 ### Removed
 ### Fixed

diff --git a/USER_GUIDE.md b/USER_GUIDE.md
@@ -5,6 +5,7 @@
     - [Authenticate with Amazon OpenSearch Service](#authenticate-with-amazon-opensearch-service)
       - [Using AWS V2 SDK](#using-aws-v2-sdk)
       - [Using AWS V3 SDK](#using-aws-v3-sdk)
+    - [Enable Handling of Long Numerals](#enable-handling-of-long-numerals) 
   - [Create an Index](#create-an-index)
   - [Add a Document to the Index](#add-a-document-to-the-index)
   - [Search for the Document](#search-for-the-document)
@@ -107,6 +108,21 @@ const client = new Client({
 });
 ```
 
+### Enable Handling of Long Numerals
+
+JavaScript can safely work with integers from -(2<sup>53</sup> - 1) to 2<sup>53</sup> - 1. However, 
+serialized JSON texts from other languages can potentially have numeric values beyond that range and the native
+serialization and deserialization methods of JavaScript's JSON, incapable of parsing them with precision; these
+values get rounded to fit the IEEE-754 representation.
+
+The `Client` can be configured to appropriately deserialize long numerals as `BigInt` values and vice versa:
+
+```javascript
+const client = new Client({
+  enableLongNumeralSupport: true,
+});
+```
+
 ## Create an Index
 
 ```javascript

diff --git a/index.js b/index.js
@@ -128,6 +128,7 @@ class Client extends OpenSearchAPI {
               proxy: null,
               enableMetaHeader: true,
               disablePrototypePoisoningProtection: false,
+              enableLongNumeralSupport: false,
             },
             opts
           );
@@ -151,6 +152,7 @@ class Client extends OpenSearchAPI {
       this[kEventEmitter] = new EventEmitter();
       this.serializer = new options.Serializer({
         disablePrototypePoisoningProtection: options.disablePrototypePoisoningProtection,
+        enableLongNumeralSupport: options.enableLongNumeralSupport,
       });
       this.connectionPool = new options.ConnectionPool({
         pingTimeout: options.pingTimeout,

diff --git a/lib/Serializer.d.ts b/lib/Serializer.d.ts
@@ -29,6 +29,7 @@
 
 export interface SerializerOptions {
   disablePrototypePoisoningProtection: boolean | 'proto' | 'constructor';
+  enableLongNumeralSupport: boolean;
 }
 
 export default class Serializer {

diff --git a/lib/Serializer.js b/lib/Serializer.js
@@ -95,6 +95,7 @@ class Serializer {
     this[kJsonOptions] = {
       protoAction: disable === true || disable === 'proto' ? 'ignore' : 'error',
       constructorAction: disable === true || disable === 'constructor' ? 'ignore' : 'error',
+      enableLongNumeralSupport: opts.enableLongNumeralSupport === true,
     };
   }
 
@@ -258,10 +259,12 @@ class Serializer {
       }
       return val;
     };
+    const shouldHandleLongNumerals =
+      isBigIntSupported && this[kJsonOptions].enableLongNumeralSupport;
     try {
-      json = JSON.stringify(object, isBigIntSupported ? checkForBigInts : null);
+      json = JSON.stringify(object, shouldHandleLongNumerals ? checkForBigInts : null);
 
-      if (isBigIntSupported && !numeralsAreNumbers) {
+      if (shouldHandleLongNumerals && !numeralsAreNumbers) {
         const temp = this._stringifyWithBigInt(object, json);
         if (temp) json = temp;
       }
@@ -286,14 +289,16 @@ class Serializer {
 
       return val;
     };
+    const shouldHandleLongNumerals =
+      isBigIntSupported && this[kJsonOptions].enableLongNumeralSupport;
     try {
       object = sjson.parse(
         json,
-        isBigIntSupported ? checkForLargeNumerals : null,
+        shouldHandleLongNumerals ? checkForLargeNumerals : null,
         this[kJsonOptions]
       );
 
-      if (isBigIntSupported && !numeralsAreNumbers) {
+      if (shouldHandleLongNumerals && !numeralsAreNumbers) {
         const temp = this._parseWithBigInt(json);
         if (temp) {
           object = temp;

diff --git a/test/fixtures/longnumerals-dataset.ndjson b/test/fixtures/longnumerals-dataset.ndjson
@@ -1,3 +1,5 @@
 {"number":18014398509481982,"description":"-18014398509481982 , -1 , 1 , 18014398509481982"}
-{"number":-18014398509481982,"description":"෴18014398509481982"}
+{"number":-18014398509481982,"description":"෴߷֍෴18014398509481982"}
+{"description":"[\"෴߷֍෴18014398509481982\"]", "number":18014398509481982}
+{"description":"18014398509481982", "number":18014398509481982}
 {"number":9007199254740891,"description":"Safer than [18014398509481982]"}
diff --git a/test/integration/serializer/longnumerals.test.js b/test/integration/serializer/longnumerals.test.js
@@ -39,6 +39,7 @@ const { Client } = require('../../../');
 const INDEX = `test-serializer-${process.pid}`;
 const client = new Client({
   node: process.env.TEST_OPENSEARCH_SERVER || 'http://localhost:9200',
+  enableLongNumeralSupport: true,
 });
 
 beforeEach(async () => {
@@ -77,14 +78,16 @@ test('long numerals', async (t) => {
       },
     },
   });
-  t.equal(results.length, 3);
+  t.equal(results.length, 5);
   const object = {};
   for (const result of results) {
     object[result.description] = result.number;
   }
   t.same(object, {
     '-18014398509481982 , -1 , 1 , 18014398509481982': 18014398509481982n,
-    '෴18014398509481982': -18014398509481982n,
+    '෴߷֍෴18014398509481982': -18014398509481982n,
     'Safer than [18014398509481982]': 9007199254740891,
+    18014398509481982: 18014398509481982n,
+    '["෴߷֍෴18014398509481982"]': 18014398509481982n,
   });
 });
diff --git a/test/unit/serializer.test.js b/test/unit/serializer.test.js
@@ -43,9 +43,9 @@ test('Basic', (t) => {
   t.same(s.deserialize(json), obj);
 });
 
-test('Long numerals', (t) => {
-  t.plan(7);
-  const s = new Serializer();
+test('Long numerals enabled', (t) => {
+  t.plan(3);
+  const s = new Serializer({ enableLongNumeralSupport: true });
   const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n; // eslint-disable-line no-undef
   const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n; // eslint-disable-line no-undef
   const json =
@@ -55,20 +55,49 @@ test('Long numerals', (t) => {
     `"positive": ${longPositive.toString()}, ` +
     `"array": [ ${longNegative.toString()}, ${longPositive.toString()} ], ` +
     `"negative": ${longNegative.toString()},` +
+    `"false-positive-1": "෴${longNegative.toString()}", ` +
+    `"false-positive-2": "[ ߷${longPositive.toString()} ]", ` +
+    `"false-positive-3": "\\": ֍${longPositive.toString()}\\"", ` +
+    `"false-positive-4": "෴߷֍${longPositive.toString()}", ` +
     `"hardcoded": 102931203123987` +
     `}`;
   const obj = s.deserialize(json);
   const res = s.serialize(obj);
-  t.equal(obj.positive, longPositive);
-  t.equal(obj.negative, longNegative);
-  t.same(obj.array, [longNegative, longPositive]);
+  t.same(obj, {
+    hardcoded: 102931203123987,
+    'false-positive-4': `෴߷֍${longPositive.toString()}`,
+    'false-positive-3': `": ֍${longPositive.toString()}"`,
+    'false-positive-2': `[ ߷${longPositive.toString()} ]`,
+    'false-positive-1': `෴${longNegative.toString()}`,
+    negative: longNegative,
+    array: [longNegative, longPositive],
+    positive: longPositive,
+    ['":' + longPositive]: `[ ${longNegative.toString()}, ${longPositive.toString()} ]`,
+  });
   // The space before and after the values, and the lack of spaces before comma are intentional
-  t.equal(obj['":' + longPositive], `[ ${longNegative.toString()}, ${longPositive.toString()} ]`);
-  t.equal(obj.hardcoded, 102931203123987);
   t.equal(res.replace(/\s+/g, ''), json.replace(/\s+/g, ''));
   t.match(res, `"[ ${longNegative.toString()}, ${longPositive.toString()} ]"`);
 });
 
+test('long numerals not enabled', (t) => {
+  t.plan(5);
+  const s = new Serializer({ enableLongNumeralSupport: false });
+  const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 3n; // eslint-disable-line no-undef
+  const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 3n; // eslint-disable-line no-undef
+  const json =
+    `{` +
+    `"positive": ${longPositive.toString()}, ` +
+    `"negative": ${longNegative.toString()}` +
+    `}`;
+  const obj = s.deserialize(json);
+  const res = s.serialize(obj);
+  t.not(obj.positive, longPositive);
+  t.not(obj.negative, longNegative);
+  t.equal(typeof obj.positive, 'number');
+  t.equal(typeof obj.negative, 'number');
+  t.not(res.replace(/\s+/g, ''), json.replace(/\s+/g, ''));
+});
+
 test('ndserialize', (t) => {
   t.plan(1);
   const s = new Serializer();