From 0c6a70f1220f47c36e8e65df9d1c950e917fd4c9 Mon Sep 17 00:00:00 2001 From: Miki Date: Wed, 12 Jul 2023 14:18:27 -0700 Subject: [PATCH 1/5] Add upgrading NPM to all workflows running older Node.js versions (#545) (#553) Also: * Add compatibility checks for Node.js v18 * Bump `actions/setup-node` to v3 Signed-off-by: Miki --- .github/workflows/bundler.yml | 13 ++++++++++++- .github/workflows/compatibility.yml | 2 +- .github/workflows/coverage.yml | 13 ++++++++++++- .github/workflows/gh_pages.yml | 2 +- .github/workflows/integration.yml | 30 +++++++++++++++++++++++++---- .github/workflows/license.yml | 13 ++++++++++++- .github/workflows/nodejs.yml | 15 +++++++++++++-- CHANGELOG.md | 3 +++ 8 files changed, 80 insertions(+), 11 deletions(-) diff --git a/.github/workflows/bundler.yml b/.github/workflows/bundler.yml index eeb16474a..8fb07ab73 100644 --- a/.github/workflows/bundler.yml +++ b/.github/workflows/bundler.yml @@ -30,10 +30,21 @@ jobs: make cluster.clean cluster.opensearch.build cluster.opensearch.start - name: Use Node.js 14.x - uses: actions/setup-node@v1 + uses: actions/setup-node@v3 with: node-version: 14.x + # NPM started understanding yarn.lock file starting from v7 + - name: Update NPM + shell: bash + run: | + export NPM_VERSION=$(npm -v) + export IS_NPM_SUITABLE=$(node -p "parseInt(process.env.NPM_ROOT, 10) >= 7") + if [ "$IS_NPM_SUITABLE" == "false" ]; then + echo "NPM needs upgrading!" + npm i npm@7 -g + fi + - name: Install run: | npm install diff --git a/.github/workflows/compatibility.yml b/.github/workflows/compatibility.yml index 132545b61..b711236a2 100644 --- a/.github/workflows/compatibility.yml +++ b/.github/workflows/compatibility.yml @@ -50,7 +50,7 @@ jobs: make cluster.clean cluster.opensearch.build cluster.opensearch.start - name: Use Node.js 16.x - uses: actions/setup-node@v1 + uses: actions/setup-node@v3 with: node-version: 16.x diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 88a610c74..7de962d9b 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -23,10 +23,21 @@ jobs: - uses: actions/checkout@v2 - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 + uses: actions/setup-node@v3 with: node-version: ${{ matrix.node-version }} + # NPM started understanding yarn.lock file starting from v7 + - name: Update NPM + shell: bash + run: | + export NPM_VERSION=$(npm -v) + export IS_NPM_SUITABLE=$(node -p "parseInt(process.env.NPM_ROOT, 10) >= 7") + if [ "$IS_NPM_SUITABLE" == "false" ]; then + echo "NPM needs upgrading!" + npm i npm@7 -g + fi + - name: Install run: | npm install diff --git a/.github/workflows/gh_pages.yml b/.github/workflows/gh_pages.yml index ea39b7278..99f89b0cc 100644 --- a/.github/workflows/gh_pages.yml +++ b/.github/workflows/gh_pages.yml @@ -9,7 +9,7 @@ jobs: name: Update gh-pages with docs steps: - uses: actions/checkout@v3 - - uses: actions/setup-node@v1 + - uses: actions/setup-node@v3 with: ruby-version: 16.x - name: Install Tools diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 14de9f6db..a99674a15 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -17,7 +17,7 @@ jobs: strategy: matrix: - node-version: [10.x, 12.x, 14.x, 16.x] + node-version: [10.x, 12.x, 14.x, 16.x, 18.x] steps: - uses: actions/checkout@v2 @@ -34,10 +34,21 @@ jobs: make cluster.clean cluster.opensearch.build cluster.opensearch.start - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 + uses: actions/setup-node@v3 with: node-version: ${{ matrix.node-version }} + # NPM started understanding yarn.lock file starting from v7 + - name: Update NPM + shell: bash + run: | + export NPM_VERSION=$(npm -v) + export IS_NPM_SUITABLE=$(node -p "parseInt(process.env.NPM_ROOT, 10) >= 7") + if [ "$IS_NPM_SUITABLE" == "false" ]; then + echo "NPM needs upgrading!" + npm i npm@7 -g + fi + - name: Install run: | npm install @@ -52,7 +63,7 @@ jobs: strategy: matrix: - node-version: [10.x, 12.x, 14.x, 16.x] + node-version: [10.x, 12.x, 14.x, 16.x, 18.x] steps: - uses: actions/checkout@v2 @@ -70,10 +81,21 @@ jobs: make cluster.clean cluster.opensearch.build cluster.opensearch.start - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 + uses: actions/setup-node@v3 with: node-version: ${{ matrix.node-version }} + # NPM started understanding yarn.lock file starting from v7 + - name: Update NPM + shell: bash + run: | + export NPM_VERSION=$(npm -v) + export IS_NPM_SUITABLE=$(node -p "parseInt(process.env.NPM_ROOT, 10) >= 7") + if [ "$IS_NPM_SUITABLE" == "false" ]; then + echo "NPM needs upgrading!" + npm i npm@7 -g + fi + - name: Install run: | npm install diff --git a/.github/workflows/license.yml b/.github/workflows/license.yml index 19b04b3c0..c9774ad67 100644 --- a/.github/workflows/license.yml +++ b/.github/workflows/license.yml @@ -15,10 +15,21 @@ jobs: - uses: actions/checkout@v2 - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 + uses: actions/setup-node@v3 with: node-version: ${{ matrix.node-version }} + # NPM started understanding yarn.lock file starting from v7 + - name: Update NPM + shell: bash + run: | + export NPM_VERSION=$(npm -v) + export IS_NPM_SUITABLE=$(node -p "parseInt(process.env.NPM_ROOT, 10) >= 7") + if [ "$IS_NPM_SUITABLE" == "false" ]; then + echo "NPM needs upgrading!" + npm i npm@7 -g + fi + - name: Install run: | npm install diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index 0576534f0..931c90d87 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -18,17 +18,28 @@ jobs: strategy: matrix: - node-version: [10.x, 12.x, 14.x, 16.x] + node-version: [10.x, 12.x, 14.x, 16.x, 18.x] os: [ubuntu-latest, windows-latest, macOS-latest] steps: - uses: actions/checkout@v2 - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 + uses: actions/setup-node@v3 with: node-version: ${{ matrix.node-version }} + # NPM started understanding yarn.lock file starting from v7 + - name: Update NPM + shell: bash + run: | + export NPM_VERSION=$(npm -v) + export IS_NPM_SUITABLE=$(node -p "parseInt(process.env.NPM_ROOT, 10) >= 7") + if [ "$IS_NPM_SUITABLE" == "false" ]; then + echo "NPM needs upgrading!" + npm i npm@7 -g + fi + - name: Install run: | npm install diff --git a/CHANGELOG.md b/CHANGELOG.md index 004050322..3aa42792e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Added ### Dependencies ### Changed + +- Add upgrading NPM to all workflows running older Node.js versions ([#545](https://github.com/opensearch-project/opensearch-js/issues/545)) + ### Deprecated ### Removed ### Fixed From e519441ca2913e3c08765309fc3ad32dac22bbde Mon Sep 17 00:00:00 2001 From: Miki Date: Wed, 12 Jul 2023 14:57:46 -0700 Subject: [PATCH 2/5] Add serialization and deserialization of numerals larger than `Number.MAX_SAFE_INTEGER` (#544) (#554) Signed-off-by: Miki --- CHANGELOG.md | 3 + lib/Serializer.js | 243 +++++++++++++++++- test/fixtures/longnumerals-dataset.ndjson | 3 + .../serializer/longnumerals.test.js | 90 +++++++ test/unit/serializer.test.js | 26 ++ 5 files changed, 363 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/longnumerals-dataset.ndjson create mode 100644 test/integration/serializer/longnumerals.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 3aa42792e..0f8027848 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ## [Unreleased] ### Added + +- Add serialization and deserialization of numerals larger than `Number.MAX_SAFE_INTEGER` ([#544](https://github.com/opensearch-project/opensearch-js/pull/544)) + ### Dependencies ### Changed diff --git a/lib/Serializer.js b/lib/Serializer.js index 06bec7199..37a9710c2 100644 --- a/lib/Serializer.js +++ b/lib/Serializer.js @@ -35,6 +35,60 @@ const sjson = require('secure-json-parse'); const { SerializationError, DeserializationError } = require('./errors'); const kJsonOptions = Symbol('secure json parse options'); +/* In JavaScript, a `Number` is a 64-bit floating-point value which can store 16 digits. However, the + * serializer and deserializer will need to cater to numeric values generated by other languages which + * can have up to 19 digits. Native JSON parser and stringifier, incapable of handling the extra + * digits, corrupt the values, making them unusable. + * + * To work around this limitation, the deserializer converts long sequences of digits into strings and + * marks them before applying the parser. During the parsing, string values that begin with the mark + * are converted to `BigInt` values. + * Similarly, during stringification, the serializer converts `BigInt` values to marked strings and + * when done, it replaces them with plain numerals. + * + * `Number.MAX_SAFE_INTEGER`, 9,007,199,254,740,991, is the largest number that the native methods can + * parse and stringify, and any numeral greater than that would need to be translated using the + * workaround; all 17-digits or longer and only tail-end of the 16-digits need translation. It would + * be unfair to all the 16-digit numbers if the translation applied to `\d{16,}` only to cover the + * less than 10%. Hence, a RegExp is created to only match numerals too long to be a number. + * + * To make the explanation simpler, let's assume that MAX_SAFE_INTEGER is 8921 which has 4 digits. + * Starting from the right, we take each digit onwards, `[-9]`: + * 1) 7922 - 7929: 792[2-9]\d{0} + * 2) 7930 - 7999: 79[3-9]\d{1} + * 9) 9 + 1 = 10 which results in a rollover; no need to do anything. + * 8) 9000 - 9999: [9-9]\d{3} + * Finally we add anything 5 digits or longer: `\d{5,} + * + * PS, a better solution would use AST but considering its performance penalty, RegExp is the next + * the best solution. + */ +const isBigIntSupported = typeof BigInt !== 'undefined'; +const maxIntAsString = String(Number.MAX_SAFE_INTEGER); +const maxIntLength = maxIntAsString.length; +// Sub-patterns for each digit +const bigIntMatcherTokens = [`\\d{${maxIntAsString.length + 1},}`]; +for (let i = 0; i < maxIntLength; i++) { + if (maxIntAsString[i] !== '9') { + bigIntMatcherTokens.push( + maxIntAsString.substring(0, i) + + `[${parseInt(maxIntAsString[i], 10) + 1}-9]` + + `\\d{${maxIntLength - i - 1}}` + ); + } +} + +/* The matcher that looks for `": , ...}` and `[..., , ...]` + * + * The pattern starts by looking for `":` not immediately preceded by a `\`. That should be + * followed by any of the numeric sub-patterns. A comma, end of an array, end of an object, or + * the end of the input are the only acceptable elements after it. + */ +const bigIntMatcher = new RegExp( + `((?:\\[|,|(? { + if (json.indexOf(marker) === -1) { + bigIntMarker = marker; + return true; + } + }); + } while (!bigIntMarker); + + return { + bigIntMarker, + length, + }; + } + + _parseWithBigInt(json) { + const { bigIntMarker, length } = this._getSuitableBigIntMarker(json); + + let hadException; + let markedJSON = json.replace(bigIntMatcher, `$1"${bigIntMarker}$2"$3`); + + /* RegExp cannot replace AST and the process of marking adds quotes. So, any false-positive hit + * will make the JSON string unparseable. + * + * To find those instances, we try to parse and watch for the location of any errors. If an error + * is caused by the marking, we remove that single marking and try again. + */ + do { + try { + hadException = false; + JSON.parse(markedJSON); + } catch (e) { + hadException = true; + /* There are two types of exception objects that can be raised: + * 1) a proper object with lineNumber and columnNumber which we can use + * 2) a textual message with the position that we need to parse + */ + let { lineNumber, columnNumber } = e; + if (!lineNumber || !columnNumber) { + const match = + // ToDo: When support for ancient versions of Node.js are dropped, replace with + // e?.message?.match?.() + e && + e.message && + typeof e.message.match === 'function' && + e.message.match(/^Unexpected token.*at position (\d+)$/); + if (match) { + lineNumber = 1; + // The position is zero-indexed; adding 1 to normalize it for the -2 that comes later + columnNumber = parseInt(match[1], 10) + 1; + } + } + + if (lineNumber < 1 || columnNumber < 2) { + // The problem is not with this replacement; just return a failure. + return; + } + + /* We need to skip e.lineNumber - 1 number of `\n` occurrences. + * Then, we need to go to e.columnNumber - 2 to look for `"\d+"`; we need to `-1` to + * account for the quote but an additional `-1` is needed because columnNumber starts from 1. + */ + const re = new RegExp( + `^((?:.*\\n){${lineNumber - 1}}[^\\n]{${columnNumber - 2}})"${bigIntMarker}(-?\\d+)"` + ); + if (!re.test(markedJSON)) { + // The exception is not caused by adding the marker + return; + } + + // We have found a bad replacement; let's remove it. + markedJSON = markedJSON.replace(re, '$1$2'); + } + } while (hadException); + + const bigIntMarkFinder = new RegExp(`^${bigIntMarker}-?\\d+$`); + + // Exceptions will trickle up to the caller + return sjson.parse( + markedJSON, + (key, val) => + /* Convert marked values to BigInt values. + * The `startsWith` is purely for performance, to avoid running `test` if not needed. + */ + typeof val === 'string' && val.startsWith(bigIntMarker) && bigIntMarkFinder.test(val) + ? BigInt(val.substring(length)) // eslint-disable-line no-undef + : val, + this[kJsonOptions] + ); + } + + _stringifyWithBigInt(object, candidate) { + const { bigIntMarker } = this._getSuitableBigIntMarker(candidate); + + /* The matcher that looks for "" + * Because we have made sure that `bigIntMarker` was never present in the original object, we can + * carelessly assume every "" is due to our marking. + */ + const markedBigIntMatcher = new RegExp(`"${bigIntMarker}(-?\\d+)"`, 'g'); + + return ( + JSON.stringify( + object, + /* Convert BigInt values to a string and mark them. + * Can't be bothered with Number values beyond safe values because they are already corrupted. + */ + (key, val) => (typeof val === 'bigint' ? `${bigIntMarker}${val.toString()}` : val) + ) + // Replace marked substrings with just the numerals + .replace(markedBigIntMatcher, '$1') + ); + } + serialize(object) { debug('Serializing', object); let json; + let numeralsAreNumbers = true; + const checkForBigInts = (key, val) => { + if (typeof val === 'bigint') { + numeralsAreNumbers = false; + // Number() is much faster than parseInt() on BigInt values + return Number(val); + } + return val; + }; try { - json = JSON.stringify(object); + json = JSON.stringify(object, isBigIntSupported ? checkForBigInts : null); + + if (isBigIntSupported && !numeralsAreNumbers) { + const temp = this._stringifyWithBigInt(object, json); + if (temp) json = temp; + } } catch (err) { throw new SerializationError(err.message, object); } @@ -58,8 +274,31 @@ class Serializer { deserialize(json) { debug('Deserializing', json); let object; + let numeralsAreNumbers = true; + const checkForLargeNumerals = (key, val) => { + if ( + numeralsAreNumbers && + typeof val === 'number' && + (val < Number.MAX_SAFE_INTEGER || val > Number.MAX_SAFE_INTEGER) + ) { + numeralsAreNumbers = false; + } + + return val; + }; try { - object = sjson.parse(json, this[kJsonOptions]); + object = sjson.parse( + json, + isBigIntSupported ? checkForLargeNumerals : null, + this[kJsonOptions] + ); + + if (isBigIntSupported && !numeralsAreNumbers) { + const temp = this._parseWithBigInt(json); + if (temp) { + object = temp; + } + } } catch (err) { throw new DeserializationError(err.message, json); } diff --git a/test/fixtures/longnumerals-dataset.ndjson b/test/fixtures/longnumerals-dataset.ndjson new file mode 100644 index 000000000..673862301 --- /dev/null +++ b/test/fixtures/longnumerals-dataset.ndjson @@ -0,0 +1,3 @@ +{"number":18014398509481982,"description":"-18014398509481982 , -1 , 1 , 18014398509481982"} +{"number":-18014398509481982,"description":"෴18014398509481982"} +{"number":9007199254740891,"description":"Safer than [18014398509481982]"} diff --git a/test/integration/serializer/longnumerals.test.js b/test/integration/serializer/longnumerals.test.js new file mode 100644 index 000000000..2e5675d2f --- /dev/null +++ b/test/integration/serializer/longnumerals.test.js @@ -0,0 +1,90 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + */ + +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +'use strict'; + +const { createReadStream } = require('fs'); +const { join } = require('path'); +const split = require('split2'); +const { test, beforeEach, afterEach } = require('tap'); + +const { Client } = require('../../../'); + +const INDEX = `test-serializer-${process.pid}`; +const client = new Client({ + node: process.env.TEST_OPENSEARCH_SERVER || 'http://localhost:9200', +}); + +beforeEach(async () => { + await client.indices.create({ index: INDEX }); + const stream = createReadStream( + join(__dirname, '..', '..', 'fixtures', 'longnumerals-dataset.ndjson') + ); + const result = await client.helpers.bulk({ + datasource: stream.pipe(split()), + refreshOnCompletion: true, + onDocument() { + return { + index: { _index: INDEX }, + }; + }, + }); + if (result.failed > 0) { + throw new Error('Failed bulk indexing docs'); + } +}); + +afterEach(async () => { + await client.indices.delete({ index: INDEX }, { ignore: 404 }); +}); + +test('long numerals', async (t) => { + const results = await client.helpers.search({ + index: INDEX, + body: { + query: { + range: { + number: { + lt: 999999999999999999n, + }, + }, + }, + }, + }); + t.equal(results.length, 3); + const object = {}; + for (const result of results) { + object[result.description] = result.number; + } + t.same(object, { + '-18014398509481982 , -1 , 1 , 18014398509481982': 18014398509481982n, + '෴18014398509481982': -18014398509481982n, + 'Safer than [18014398509481982]': 9007199254740891, + }); +}); diff --git a/test/unit/serializer.test.js b/test/unit/serializer.test.js index ccbb9baf2..f6c6339a7 100644 --- a/test/unit/serializer.test.js +++ b/test/unit/serializer.test.js @@ -43,6 +43,32 @@ test('Basic', (t) => { t.same(s.deserialize(json), obj); }); +test('Long numerals', (t) => { + t.plan(7); + const s = new Serializer(); + 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 = + `{` + + // The space before and after the values, and the lack of spaces before comma are intentional + `"\\":${longPositive}": "[ ${longNegative.toString()}, ${longPositive.toString()} ]", ` + + `"positive": ${longPositive.toString()}, ` + + `"array": [ ${longNegative.toString()}, ${longPositive.toString()} ], ` + + `"negative": ${longNegative.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]); + // 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('ndserialize', (t) => { t.plan(1); const s = new Serializer(); From c5096752b1f371be0b60379172c97b1979209caa Mon Sep 17 00:00:00 2001 From: Miki Date: Wed, 12 Jul 2023 14:58:33 -0700 Subject: [PATCH 3/5] Version Bump: 2.3.0 (#546) (#555) Signed-off-by: Theo Truong --- CHANGELOG.md | 9 +++++++++ package.json | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f8027848..de3e56aab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,15 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ## [Unreleased] +### Added +### Dependencies +### Changed +### Deprecated +### Removed +### Fixed +### Security + +## [2.3.0] ### Added diff --git a/package.json b/package.json index 8ad84211c..3be8fb784 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ } }, "homepage": "https://www.opensearch.org/", - "version": "2.2.1", + "version": "2.3.0", "versionCanary": "7.10.0-canary.6", "keywords": [ "opensearch", From 198d3a21edf597af8b434d546531e5ce5ddfb580 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Mon, 17 Jul 2023 16:49:24 -0400 Subject: [PATCH 4/5] Make handling of long numerals an option that is disabled by default (#557) (#561) Also: * Strengthen the tests * update USER_GUIDE.md (cherry picked from commit 08069bcc74a631e1d795d93570be57497f1d3d24) Signed-off-by: Miki Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- USER_GUIDE.md | 16 +++++++ index.js | 2 + lib/Serializer.d.ts | 1 + lib/Serializer.js | 13 ++++-- test/fixtures/longnumerals-dataset.ndjson | 4 +- .../serializer/longnumerals.test.js | 7 ++- test/unit/serializer.test.js | 45 +++++++++++++++---- 7 files changed, 73 insertions(+), 15 deletions(-) diff --git a/USER_GUIDE.md b/USER_GUIDE.md index d917f93f6..0f2e52dca 100644 --- 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 -(253 - 1) to 253 - 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 index 04a83d5c7..5e5154398 100644 --- 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 index 5e9799996..92aa88cdf 100644 --- 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 index 37a9710c2..4fb5123c4 100644 --- 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 index 673862301..59bc6963c 100644 --- 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 index 2e5675d2f..7a10aafbb 100644 --- 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 index f6c6339a7..1773fce07 100644 --- 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(); From 485a01f8c59da68c98e2bb94dac6c3bdb52d10b5 Mon Sep 17 00:00:00 2001 From: Theo Truong Date: Mon, 17 Jul 2023 15:37:50 -0600 Subject: [PATCH 5/5] Version Bump: 2.3.1 Signed-off-by: Theo Truong --- CHANGELOG.md | 13 ++++++++++++- package.json | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de3e56aab..90911e5da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,16 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Fixed ### Security +## [2.3.1] +### Added +### Dependencies +### Changed +- Made the new Long Number logic added in 2.3.0 optional and disabled by default ([561](https://github.com/opensearch-project/opensearch-js/pull/561) +### Deprecated +### Removed +### Fixed +### Security + ## [2.3.0] ### Added @@ -155,4 +165,5 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) [2.1.0]: https://github.com/opensearch-project/opensearch-js/releases/tag/2.1.0 [2.2.0]: https://github.com/opensearch-project/opensearch-js/releases/tag/2.2.0 -[Unreleased]: https://github.com/opensearch-project/opensearch-js/compare/2.2.0...HEAD +[2.3.0]: https://github.com/opensearch-project/opensearch-js/releases/tag/2.3.0 +[Unreleased]: https://github.com/opensearch-project/opensearch-js/compare/2.3.0...HEAD diff --git a/package.json b/package.json index 3be8fb784..06f4bdefc 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ } }, "homepage": "https://www.opensearch.org/", - "version": "2.3.0", + "version": "2.3.1", "versionCanary": "7.10.0-canary.6", "keywords": [ "opensearch",