Skip to content

Commit

Permalink
[osd/std] Add fallback mechanism when recovery from false-positives i…
Browse files Browse the repository at this point in the history 
…n handling of long numerals fails (#6253)

Signed-off-by: Miki <miki@amazon.com>
  • Loading branch information
@AMoo-Miki
AMoo-Miki authored Mar 25, 2024
1 parent 27d73ab commit 312075c
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 62 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Discover] Fix lazy loading of the legacy table from getting stuck ([#6041](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6041))
- [BUG][Discover] Allow saved sort from search embeddable to load in Dashboard ([#5934](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5934))
- [osd/std] Add additional recovery from false-positives in handling of long numerals ([#5956](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5956), [#6245](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6245))
- [osd/std] Add fallback mechanism when recovery from false-positives in handling of long numerals fails ([#6253](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6253))
- [BUG][Multiple Datasource] Fix missing customApiRegistryPromise param for test connection ([#5944](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5944))
- [BUG][Multiple Datasource] Add a migration function for datasource to add migrationVersion field ([#6025](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6025))
- [BUG][MD]Expose picker using function in data source management plugin setup([#6030](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6030))
Expand Down
129 changes: 67 additions & 62 deletions packages/osd-std/src/json.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,77 +143,82 @@ const parseStringWithLongNumerals = (
* 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;
obj = parseMarkedText(markedJSON);
} catch (e) {
hadException = true;
/* There are two types of exception objects that can be raised:
* 1) a textual message with the position that we need to parse
* i. Unexpected [token|string ...] at position ...
* ii. Expected ',' or ... after ... in JSON at position ...
* iii. expected ',' or ... after ... in object at line ... column ...
* 2) a proper object with lineNumber and columnNumber which we can use
* Note: this might refer to the part of the code that threw the exception but
* we will try it anyway; the regex is specific enough to not produce
* false-positives.
*/
let { lineNumber, columnNumber } = e;

if (typeof e?.message === 'string') {
/* Check for 1-i and 1-ii
* Finding "..."෴1111"..." inside a string value, the extra quotes throw a syntax error
* and the position points to " that is assumed to be the begining of a value.
try {
do {
try {
hadException = false;
obj = parseMarkedText(markedJSON);
} catch (e) {
hadException = true;
/* There are two types of exception objects that can be raised:
* 1) a textual message with the position that we need to parse
* i. Unexpected [token|string ...] at position ...
* ii. Expected ',' or ... after ... in JSON at position ...
* iii. expected ',' or ... after ... in object at line ... column ...
* 2) a proper object with lineNumber and columnNumber which we can use
* Note: this might refer to the part of the code that threw the exception but
* we will try it anyway; the regex is specific enough to not produce
* false-positives.
*/
let match = e.message.match(/^(?:Une|E)xpected .*at position (\d+)(\D|$)/i);
if (match) {
lineNumber = 1;
// Add 1 to reach the marker
columnNumber = parseInt(match[1], 10) + 1;
} else {
/* Check for 1-iii
* Finding "...,"෴1111"..." inside a string value, the extra quotes throw a syntax error
* and the column number points to the marker after the " that is assumed to be terminating the
* value.
* PS: There are different versions of this error across browsers and platforms.
let { lineNumber, columnNumber } = e;

if (typeof e?.message === 'string') {
/* Check for 1-i and 1-ii
* Finding "..."෴1111"..." inside a string value, the extra quotes throw a syntax error
* and the position points to " that is assumed to be the begining of a value.
*/
// ToDo: Add functional tests for this path
match = e.message.match(/expected .*line (\d+) column (\d+)(\D|$)/i);
let match = e.message.match(/^(?:Un)?expected .*at position (\d+)(\D|$)/i);
if (match) {
lineNumber = parseInt(match[1], 10);
columnNumber = parseInt(match[2], 10);
lineNumber = 1;
// Add 1 to reach the marker
columnNumber = parseInt(match[1], 10) + 1;
} else {
/* Check for 1-iii
* Finding "...,"෴1111"..." inside a string value, the extra quotes throw a syntax error
* and the column number points to the marker after the " that is assumed to be terminating the
* value.
* PS: There are different versions of this error across browsers and platforms.
*/
// ToDo: Add functional tests for this path
match = e.message.match(/expected .*line (\d+) column (\d+)(\D|$)/i);
if (match) {
lineNumber = parseInt(match[1], 10);
columnNumber = parseInt(match[2], 10);
}
}
}
}

if (lineNumber < 1 || columnNumber < 2) {
/* The problem is not with this replacement.
* Note: This will never happen because the outer parse would have already thrown.
*/
// coverage:ignore-line
throw e;
}
if (lineNumber < 1 || columnNumber < 2) {
/* The problem is not with this replacement.
* Note: This will never happen because the outer parse would have already thrown.
*/
// coverage:ignore-line
throw e;
}

/* We need to skip e.lineNumber - 1 number of `\n` occurrences.
* Then, we need to go to e.columnNumber - 2 to look for `"<mark>\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}})"${marker}(-?\\d+)"`
);
if (!re.test(markedJSON)) {
/* The exception is not caused by adding the marker.
* Note: This will never happen because the outer parse would have already thrown.
/* We need to skip e.lineNumber - 1 number of `\n` occurrences.
* Then, we need to go to e.columnNumber - 2 to look for `"<mark>\d+"`; we need to `-1` to
* account for the quote but an additional `-1` is needed because columnNumber starts from 1.
*/
// coverage:ignore-line
throw e;
}
const re = new RegExp(
`^((?:.*\\n){${lineNumber - 1}}[^\\n]{${columnNumber - 2}})"${marker}(-?\\d+)"`
);
if (!re.test(markedJSON)) {
/* The exception is not caused by adding the marker.
* Note: This will never happen because the outer parse would have already thrown.
*/
// coverage:ignore-line
throw e;
}

// We have found a bad replacement; let's remove it.
markedJSON = markedJSON.replace(re, '$1$2');
}
} while (hadException);
// We have found a bad replacement; let's remove it.
markedJSON = markedJSON.replace(re, '$1$2');
}
} while (hadException);
} catch (ex) {
// If parsing of marked `text` fails, fallback to parsing the original `text`
obj = JSON.parse(text, reviver || undefined);
}

return obj;
};
Expand Down

0 comments on commit 312075c

Please sign in to comment.