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

fix(NODE-4464):stringify and parse negative zero to and from $numberDouble: -0.0 #531

Merged
merged 2 commits into from
Dec 2, 2022
Merged
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
31 changes: 18 additions & 13 deletions src/extended_json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,15 @@ function deserializeValue(value: any, options: EJSON.Options = {}) {
return value;
}

// if it's an integer, should interpret as smallest BSON integer
// that can represent it exactly. (if out of range, interpret as double.)
if (Math.floor(value) === value) {
if (value >= BSON_INT32_MIN && value <= BSON_INT32_MAX) return new Int32(value);
if (value >= BSON_INT64_MIN && value <= BSON_INT64_MAX) return Long.fromNumber(value);
if (Number.isInteger(value) && !Object.is(value, -0)) {
// interpret as being of the smallest BSON integer type that can represent the number exactly
if (value >= BSON_INT32_MIN && value <= BSON_INT32_MAX) {
return new Int32(value);
}
if (value >= BSON_INT64_MIN && value <= BSON_INT64_MAX) {
// TODO(NODE-4377): EJSON js number handling diverges from BSON
return Long.fromNumber(value);
}
}

// If the number is a non-integer or out of integer range, should interpret as BSON Double.
Expand Down Expand Up @@ -216,16 +220,17 @@ function serializeValue(value: any, options: EJSONSerializeOptions): any {
}

if (typeof value === 'number' && (!options.relaxed || !isFinite(value))) {
// it's an integer
if (Math.floor(value) === value) {
const int32Range = value >= BSON_INT32_MIN && value <= BSON_INT32_MAX,
int64Range = value >= BSON_INT64_MIN && value <= BSON_INT64_MAX;

if (Number.isInteger(value) && !Object.is(value, -0)) {
// interpret as being of the smallest BSON integer type that can represent the number exactly
if (int32Range) return { $numberInt: value.toString() };
if (int64Range) return { $numberLong: value.toString() };
if (value >= BSON_INT32_MIN && value <= BSON_INT32_MAX) {
return { $numberInt: value.toString() };
}
if (value >= BSON_INT64_MIN && value <= BSON_INT64_MAX) {
// TODO(NODE-4377): EJSON js number handling diverges from BSON
return { $numberLong: value.toString() };
}
}
return { $numberDouble: value.toString() };
return { $numberDouble: Object.is(value, -0) ? '-0.0' : value.toString() };
}

if (value instanceof RegExp || isRegExp(value)) {
Expand Down
2 changes: 2 additions & 0 deletions test/node/bson_corpus.spec.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ function nativeToREJSON(native) {
}

function normalize(cEJ) {
// TODO(NODE-3396): loses information about the original input
// ex. parse will preserve -0 but stringify will output +0
return JSON.stringify(JSON.parse(cEJ));
}

Expand Down
30 changes: 27 additions & 3 deletions test/node/double_tests.js → test/node/double.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';

const BSON = require('../register-bson');
import * as BSON from '../register-bson';
const Double = BSON.Double;

describe('BSON Double Precision', function () {
Expand Down Expand Up @@ -89,4 +87,30 @@ describe('BSON Double Precision', function () {
expect(type).to.not.equal(BSON.BSON_DATA_NUMBER);
expect(type).to.equal(BSON.BSON_DATA_INT);
});

describe('extended JSON', () => {
describe('stringify()', () => {
it('preserves negative zero in canonical format', () => {
const result = BSON.EJSON.stringify({ a: -0.0 }, { relaxed: false });
expect(result).to.equal(`{"a":{"$numberDouble":"-0.0"}}`);
});

it('loses negative zero in relaxed format', () => {
const result = BSON.EJSON.stringify({ a: -0.0 }, { relaxed: true });
expect(result).to.equal(`{"a":0}`);
});
});

describe('parse()', () => {
it('preserves negative zero in deserialization with relaxed false', () => {
const result = BSON.EJSON.parse(`{ "a": -0.0 }`, { relaxed: false });
expect(result.a).to.have.property('_bsontype', 'Double');
});

it('preserves negative zero in deserialization with relaxed true', () => {
const result = BSON.EJSON.parse(`{ "a": -0.0 }`, { relaxed: true });
expect(Object.is(result.a, -0), 'expected prop a to be negative zero').to.be.true;
});
});
});
});
3 changes: 2 additions & 1 deletion test/node/extended_json_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe('Extended JSON', function () {
});

it('should correctly extend an existing mongodb module', function () {
// Serialize the document
// TODO(NODE-4377): doubleNumberIntFit should be a double not a $numberLong
var json =
'{"_id":{"$numberInt":"100"},"gh":{"$numberInt":"1"},"binary":{"$binary":{"base64":"AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+Pw==","subType":"00"}},"date":{"$date":{"$numberLong":"1488372056737"}},"code":{"$code":"function() {}","$scope":{"a":{"$numberInt":"1"}}},"dbRef":{"$ref":"tests","$id":{"$numberInt":"1"},"$db":"test"},"decimal":{"$numberDecimal":"100"},"double":{"$numberDouble":"10.1"},"int32":{"$numberInt":"10"},"long":{"$numberLong":"200"},"maxKey":{"$maxKey":1},"minKey":{"$minKey":1},"objectId":{"$oid":"111111111111111111111111"},"objectID":{"$oid":"111111111111111111111111"},"oldObjectID":{"$oid":"111111111111111111111111"},"regexp":{"$regularExpression":{"pattern":"hello world","options":"i"}},"symbol":{"$symbol":"symbol"},"timestamp":{"$timestamp":{"t":0,"i":1000}},"int32Number":{"$numberInt":"300"},"doubleNumber":{"$numberDouble":"200.2"},"longNumberIntFit":{"$numberLong":"7036874417766400"},"doubleNumberIntFit":{"$numberLong":"19007199250000000"}}';

Expand All @@ -103,6 +103,7 @@ describe('Extended JSON', function () {
expect(doc1.int32Number._bsontype).to.equal('Int32');
expect(doc1.doubleNumber._bsontype).to.equal('Double');
expect(doc1.longNumberIntFit._bsontype).to.equal('Long');
// TODO(NODE-4377): EJSON should not try to make Longs from large ints
expect(doc1.doubleNumberIntFit._bsontype).to.equal('Long');
});

Expand Down