Skip to content

Commit

Permalink
fix: only upgrade symbol to string if promoteValues is true
Browse files Browse the repository at this point in the history 
We currently automatically promote BSONSymbol to a JavaScript
string, which loses type information when roundtripping the value.
Since we already have a setting to signal when a user doesn't mind
the loss of type information (`promoteValues`), this behavior is
now gated by that option.

NODE-2518
  • Loading branch information
@mbroadst
mbroadst committed Mar 26, 2020
1 parent 30f5a8f commit 067a7ba
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 57 deletions.
2 changes: 2 additions & 0 deletions lib/double.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ class Double {
return this.value;
}

// NOTE: JavaScript has +0 and -0, apparently to model limit calculations. If a user
// explicitly provided `-0` then we need to ensure the sign makes it into the output
if (Object.is(Math.sign(this.value), -0)) {
return { $numberDouble: `-${this.value.toFixed(1)}` };
}
Expand Down
5 changes: 3 additions & 2 deletions lib/parser/deserializer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const Decimal128 = require('../decimal128');
const Int32 = require('../int_32');
const DBRef = require('../db_ref');
const BSONRegExp = require('../regexp');
const BSONSymbol = require('../symbol');
const Binary = require('../binary');
const constants = require('../constants');
const validateUtf8 = require('../validate_utf8').validateUtf8;
Expand Down Expand Up @@ -414,8 +415,8 @@ function deserializeObject(buffer, index, options, isArray) {
buffer[index + stringSize - 1] !== 0
)
throw new Error('bad string length in bson');
// symbol is deprecated - upgrade to string.
object[name] = buffer.toString('utf8', index, index + stringSize - 1);
const symbol = buffer.toString('utf8', index, index + stringSize - 1);
object[name] = promoteValues ? symbol : new BSONSymbol(symbol);
index = index + stringSize;
} else if (elementType === constants.BSON_DATA_TIMESTAMP) {
const lowBits =
Expand Down
83 changes: 28 additions & 55 deletions test/node/bson_corpus_tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,35 +58,9 @@ const skipBSON = {

const skipExtendedJSON = {
'Timestamp with high-order bit set on both seconds and increment':
'Current BSON implementation of timestamp/long cannot hold these values - 1 too large.'
};

// test modifications for JavaScript
const modifiedDoubles = {
'+1.0': { canonical_extjson: '{"d":{"$numberDouble":"1"}}' },
'-1.0': { canonical_extjson: '{"d":{"$numberDouble":"-1"}}' },
'1.23456789012345677E+18': { canonical_extjson: '{"d":{"$numberDouble":"1234567890123456800"}}' },
'-1.23456789012345677E+18': {
canonical_extjson: '{"d":{"$numberDouble":"-1234567890123456800"}}'
},
'0.0': { canonical_extjson: '{"d":{"$numberDouble":"0"}}' },
'-0.0': {
canonical_extjson: '{"d":{"$numberDouble":"0"}}',
canonical_bson: '10000000016400000000000000000000'
}
};

const modifiedMultitype = {
'All BSON types': {
canonical_extjson:
'{"_id":{"$oid":"57e193d7a9cc81b4027498b5"},"Symbol":"symbol","String":"string","Int32":{"$numberInt":"42"},"Int64":{"$numberLong":"42"},"Double":{"$numberDouble":"-1"},"Binary":{"$binary":{"base64":"o0w498Or7cijeBSpkquNtg==","subType":"03"}},"BinaryUserDefined":{"$binary":{"base64":"AQIDBAU=","subType":"80"}},"Code":{"$code":"function() {}"},"CodeWithScope":{"$code":"function() {}","$scope":{}},"Subdocument":{"foo":"bar"},"Array":[{"$numberInt":"1"},{"$numberInt":"2"},{"$numberInt":"3"},{"$numberInt":"4"},{"$numberInt":"5"}],"Timestamp":{"$timestamp":{"t":42,"i":1}},"Regex":{"$regularExpression":{"pattern":"pattern","options":""}},"DatetimeEpoch":{"$date":{"$numberLong":"0"}},"DatetimePositive":{"$date":{"$numberLong":"2147483647"}},"DatetimeNegative":{"$date":{"$numberLong":"-2147483648"}},"True":true,"False":false,"DBPointer":{"$ref":"collection","$id":{"$oid":"57e193d7a9cc81b4027498b1"}},"DBRef":{"$ref":"collection","$id":{"$oid":"57fd71e96e32ab4225b723fb"},"$db":"database"},"Minkey":{"$minKey":1},"Maxkey":{"$maxKey":1},"Null":null,"Undefined":null}',
canonical_bson:
'48020000075f69640057e193d7a9cc81b4027498b50253796d626f6c000700000073796d626f6c0002537472696e670007000000737472696e670010496e743332002a00000012496e743634002a0000000000000001446f75626c6500000000000000f0bf0542696e617279001000000003a34c38f7c3abedc8a37814a992ab8db60542696e61727955736572446566696e656400050000008001020304050d436f6465000e00000066756e6374696f6e2829207b7d000f436f64655769746853636f7065001b0000000e00000066756e6374696f6e2829207b7d00050000000003537562646f63756d656e74001200000002666f6f0004000000626172000004417272617900280000001030000100000010310002000000103200030000001033000400000010340005000000001154696d657374616d7000010000002a0000000b5265676578007061747465726e0000094461746574696d6545706f6368000000000000000000094461746574696d65506f73697469766500ffffff7f00000000094461746574696d654e656761746976650000000080ffffffff085472756500010846616c73650000034442506f696e746572002b0000000224726566000b000000636f6c6c656374696f6e00072469640057e193d7a9cc81b4027498b100034442526566003d0000000224726566000b000000636f6c6c656374696f6e00072469640057fd71e96e32ab4225b723fb02246462000900000064617461626173650000ff4d696e6b6579007f4d61786b6579000a4e756c6c000a556e646566696e65640000',
converted_extjson:
'{"_id":{"$oid":"57e193d7a9cc81b4027498b5"},"Symbol":"symbol","String":"string","Int32":{"$numberInt":"42"},"Int64":{"$numberLong":"42"},"Double":{"$numberDouble":"-1"},"Binary":{"$binary":{"base64":"o0w498Or7cijeBSpkquNtg==","subType":"03"}},"BinaryUserDefined":{"$binary":{"base64":"AQIDBAU=","subType":"80"}},"Code":{"$code":"function() {}"},"CodeWithScope":{"$code":"function() {}","$scope":{}},"Subdocument":{"foo":"bar"},"Array":[{"$numberInt":"1"},{"$numberInt":"2"},{"$numberInt":"3"},{"$numberInt":"4"},{"$numberInt":"5"}],"Timestamp":{"$timestamp":{"t":42,"i":1}},"Regex":{"$regularExpression":{"pattern":"pattern","options":""}},"DatetimeEpoch":{"$date":{"$numberLong":"0"}},"DatetimePositive":{"$date":{"$numberLong":"2147483647"}},"DatetimeNegative":{"$date":{"$numberLong":"-2147483648"}},"True":true,"False":false,"DBPointer":{"$ref":"collection","$id":{"$oid":"57e193d7a9cc81b4027498b1"}},"DBRef":{"$ref":"collection","$id":{"$oid":"57fd71e96e32ab4225b723fb"},"$db":"database"},"Minkey":{"$minKey":1},"Maxkey":{"$maxKey":1},"Null":null,"Undefined":null}',
converted_bson:
'48020000075f69640057e193d7a9cc81b4027498b50253796d626f6c000700000073796d626f6c0002537472696e670007000000737472696e670010496e743332002a00000012496e743634002a0000000000000001446f75626c6500000000000000f0bf0542696e617279001000000003a34c38f7c3abedc8a37814a992ab8db60542696e61727955736572446566696e656400050000008001020304050d436f6465000e00000066756e6374696f6e2829207b7d000f436f64655769746853636f7065001b0000000e00000066756e6374696f6e2829207b7d00050000000003537562646f63756d656e74001200000002666f6f0004000000626172000004417272617900280000001030000100000010310002000000103200030000001033000400000010340005000000001154696d657374616d7000010000002a0000000b5265676578007061747465726e0000094461746574696d6545706f6368000000000000000000094461746574696d65506f73697469766500ffffff7f00000000094461746574696d654e656761746976650000000080ffffffff085472756500010846616c73650000034442506f696e746572002b0000000224726566000b000000636f6c6c656374696f6e00072469640057e193d7a9cc81b4027498b100034442526566003d0000000224726566000b000000636f6c6c656374696f6e00072469640057fd71e96e32ab4225b723fb02246462000900000064617461626173650000ff4d696e6b6579007f4d61786b6579000a4e756c6c000a556e646566696e65640000'
}
'Current BSON implementation of timestamp/long cannot hold these values - 1 too large.',
'1.23456789012345677E+18': 'NODE-2519',
'-1.23456789012345677E+18': 'NODE-2519'
};

const corpus = require('./tools/bson_corpus_test_loader');
Expand All @@ -96,22 +70,6 @@ describe('BSON Corpus', function() {
const description = scenario.description;
const valid = scenario.valid || [];

// since doubles are formatted differently in JS than in corpus, overwrite expected results
if (description === 'Double type') {
valid.forEach(v => {
if (modifiedDoubles[v.description]) {
Object.assign(v, modifiedDoubles[v.description]);
}
});
// multitype test has a double nested in object, so change those expected values too
} else if (description === 'Multiple types within the same document') {
valid.forEach(v => {
if (modifiedMultitype[v.description]) {
Object.assign(v, modifiedMultitype[v.description]);
}
});
}

describe(description, function() {
if (valid) {
describe('valid-bson', function() {
Expand All @@ -122,20 +80,35 @@ describe('BSON Corpus', function() {
}

it(v.description, function() {
if (v.description === 'All BSON types' && deprecated) {
// there is just too much variation in the specified expectation to make this work
this.skip();
return;
}

const cB = Buffer.from(v.canonical_bson, 'hex');
let dB, convB;
if (v.degenerate_bson) dB = Buffer.from(v.degenerate_bson, 'hex');
if (v.converted_bson) convB = Buffer.from(v.converted_bson, 'hex');
if (deprecated) {
const roundTripped = BSON.serialize(
BSON.deserialize(
cB,
Object.assign({}, deserializeOptions, { promoteValues: true })
),
serializeOptions
);

const roundTripped = BSON.serialize(
BSON.deserialize(cB, deserializeOptions),
serializeOptions
);
const convB = Buffer.from(v.converted_bson, 'hex');
expect(convB).to.deep.equal(roundTripped);
} else {
const roundTripped = BSON.serialize(
BSON.deserialize(cB, deserializeOptions),
serializeOptions
);

if (deprecated) expect(convB).to.deep.equal(roundTripped);
else expect(cB).to.deep.equal(roundTripped);
expect(cB).to.deep.equal(roundTripped);
}

if (dB) {
if (v.degenerate_bson) {
const dB = Buffer.from(v.degenerate_bson, 'hex');
expect(cB).to.deep.equal(
BSON.serialize(BSON.deserialize(dB, deserializeOptions), serializeOptions)
);
Expand Down

0 comments on commit 067a7ba

Please sign in to comment.