opensearch-project / opensearch-js

Node.js Client for OpenSearch
https://opensearch.org/docs/latest/clients/javascript/
Apache License 2.0
188 stars 122 forks source link

Fix typo cause it will always trigger JSON11 parse #889

Closed Hailong-am closed 3 weeks ago

Hailong-am commented 3 weeks ago

Description

Fix typo cause it will always trigger JSON11 parse.

if the json string has number in it, numeralsAreNumbers will always be true.

(val < Number.MAX_SAFE_INTEGER || val > Number.MAX_SAFE_INTEGER)

should changed to (val < Number.MIN_SAFE_INTEGER || val > Number.MAX_SAFE_INTEGER)

https://github.com/opensearch-project/opensearch-js/blob/92884503d80269ba08099040ce20c6aac9a2c4ab/lib/Serializer.js#L101-L107

then it will always trigger JSON11 parse

https://github.com/opensearch-project/opensearch-js/blob/92884503d80269ba08099040ce20c6aac9a2c4ab/lib/Serializer.js#L128-L137

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

Check List

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

AMoo-Miki commented 3 weeks ago

Thanks, could you please add a test?

Since we have positive tests, we should add negative tests.

Hailong-am commented 3 weeks ago

Thanks, could you please add a test?

Since we have positive tests, we should add negative tests.

Thanks, a negative tests added. @dblock @AMoo-Miki

AMoo-Miki commented 2 weeks ago

OSD needs this in 2.x. It would be great if we had a release for it too.

nhtruong commented 1 week ago

I'll cut a release today

nhtruong commented 1 week ago

@AMoo-Miki https://www.npmjs.com/package/@opensearch-project/opensearch/v/2.13.0