Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] Fix a typo while inspecting values for large numerals in OSD and the JS client #8875

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/8839.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Fix a typo while inspecting values for large numerals in OSD and the JS client ([#8839](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8839))
84 changes: 81 additions & 3 deletions packages/osd-std/src/json.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import JSON11 from 'json11';
import { stringify, parse } from './json';

describe('json', () => {
Expand Down Expand Up @@ -90,9 +91,55 @@ describe('json', () => {
expect(stringify(input, replacer, 2)).toEqual(JSON.stringify(input, replacer, 2));
});

it('can handle long numerals while parsing', () => {
const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n;
it('can handle positive long numerals while parsing', () => {
const longPositiveA = BigInt(Number.MAX_SAFE_INTEGER) * 2n;
const longPositiveB = BigInt(Number.MAX_SAFE_INTEGER) * 2n + 1n;
const text =
`{` +
// The space before and after the values, and the lack of spaces before comma are intentional
`"\\":${longPositiveA}": "[ ${longPositiveB.toString()}, ${longPositiveA.toString()} ]", ` +
`"positive": ${longPositiveA.toString()}, ` +
`"array": [ ${longPositiveB.toString()}, ${longPositiveA.toString()} ], ` +
`"negative": ${longPositiveB.toString()},` +
`"number": 102931203123987` +
`}`;

const result = parse(text);
expect(result.positive).toBe(longPositiveA);
expect(result.negative).toBe(longPositiveB);
expect(result.array).toEqual([longPositiveB, longPositiveA]);
expect(result['":' + longPositiveA]).toBe(
`[ ${longPositiveB.toString()}, ${longPositiveA.toString()} ]`
);
expect(result.number).toBe(102931203123987);
});

it('can handle negative long numerals while parsing', () => {
const longNegativeA = BigInt(Number.MIN_SAFE_INTEGER) * 2n;
const longNegativeB = BigInt(Number.MIN_SAFE_INTEGER) * 2n - 1n;
const text =
`{` +
// The space before and after the values, and the lack of spaces before comma are intentional
`"\\":${longNegativeA}": "[ ${longNegativeB.toString()}, ${longNegativeA.toString()} ]", ` +
`"positive": ${longNegativeA.toString()}, ` +
`"array": [ ${longNegativeB.toString()}, ${longNegativeA.toString()} ], ` +
`"negative": ${longNegativeB.toString()},` +
`"number": 102931203123987` +
`}`;

const result = parse(text);
expect(result.positive).toBe(longNegativeA);
expect(result.negative).toBe(longNegativeB);
expect(result.array).toEqual([longNegativeB, longNegativeA]);
expect(result['":' + longNegativeA]).toBe(
`[ ${longNegativeB.toString()}, ${longNegativeA.toString()} ]`
);
expect(result.number).toBe(102931203123987);
});

it('can handle mixed long numerals while parsing', () => {
const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n + 1n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n - 1n;
const text =
`{` +
// The space before and after the values, and the lack of spaces before comma are intentional
Expand All @@ -113,6 +160,37 @@ describe('json', () => {
expect(result.number).toBe(102931203123987);
});

it('does not use JSON11 when not needed', () => {
const spyParse = jest.spyOn(JSON11, 'parse');

const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n + 1n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n - 1n;
const text =
`{` +
`"\\":${longPositive}": "[ ${longNegative.toString()}, ${longPositive.toString()} ]", ` +
`"number": 102931203123987` +
`}`;
parse(text);

expect(spyParse).not.toHaveBeenCalled();
});

it('uses JSON11 when dealing with long numerals', () => {
const spyParse = jest.spyOn(JSON11, 'parse');

const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n + 1n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n - 1n;
const text =
`{` +
`"\\":${longPositive}": "[ ${longNegative.toString()}, ${longPositive.toString()} ]", ` +
`"positive": ${longPositive.toString()}, ` +
`"number": 102931203123987` +
`}`;
parse(text);

expect(spyParse).toHaveBeenCalled();
});

it('can handle BigInt values while stringifying', () => {
const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n;
Expand Down
2 changes: 1 addition & 1 deletion packages/osd-std/src/json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export const parse = (
numeralsAreNumbers &&
typeof val === 'number' &&
isFinite(val) &&
(val < Number.MAX_SAFE_INTEGER || val > Number.MAX_SAFE_INTEGER)
(val < Number.MIN_SAFE_INTEGER || val > Number.MAX_SAFE_INTEGER)
) {
numeralsAreNumbers = false;
}
Expand Down
9 changes: 9 additions & 0 deletions scripts/postinstall.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@ const run = async () => {
},
])
);
//ToDo: Remove when opensearch-js is released to include https://github.com/opensearch-project/opensearch-js/pull/889
promises.push(
patchFile('node_modules/@opensearch-project/opensearch/lib/Serializer.js', [
{
from: 'val < Number.MAX_SAFE_INTEGER',
to: 'val < Number.MIN_SAFE_INTEGER',
},
])
);

//Axios's type definition is far too advanced for OSD
promises.push(
Expand Down
Loading