Skip to content

Commit

Permalink
feat!(NODE-4410): only enumerate own properties (#527)
Browse files Browse the repository at this point in the history
Co-authored-by: Bailey Pearson <bailey.pearson@mongodb.com>
  • Loading branch information
nbbeeken and baileympearson committed Dec 5, 2022
1 parent be74b30 commit 5103e4d
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 37 deletions.
12 changes: 12 additions & 0 deletions docs/upgrade-to-v5.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,18 @@ The following deprecated methods have been removed:
- `ObjectId.get_inc`
- The `static getInc()` is private since invoking it increments the next `ObjectId` index, so invoking would impact the creation of subsequent ObjectIds.

### BSON Element names are now fetched only from object's own properties

`BSON.serialize`, `EJSON.stringify` and `BSON.calculateObjectSize` now only inspect own properties and do not consider properties defined on the prototype of the input.

```typescript
const object = { a: 1 };
Object.setPrototypeOf(object, { b: 2 });
BSON.deserialize(BSON.serialize(object));
// now returns { a: 1 } in v5.0
// would have returned { a: 1, b: 2 } in v4.x
```

### Negative Zero is now serialized to Double

BSON serialize will now preserve negative zero values as a floating point number.
Expand Down
2 changes: 1 addition & 1 deletion src/extended_json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ function serializeDocument(doc: any, options: EJSONSerializeOptions) {
if (typeof bsontype === 'undefined') {
// It's a regular object. Recursively serialize its property values.
const _doc: Document = {};
for (const name in doc) {
for (const name of Object.keys(doc)) {
options.seenObjects.push({ propertyName: name, obj: null });
try {
const value = serializeValue(doc[name], options);
Expand Down
2 changes: 1 addition & 1 deletion src/parser/calculate_size.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export function calculateObjectSize(
}

// Calculate size
for (const key in object) {
for (const key of Object.keys(object)) {
totalLength += calculateElement(key, object[key], serializeFunctions, false, ignoreUndefined);
}
}
Expand Down
13 changes: 3 additions & 10 deletions src/parser/deserializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,23 +314,16 @@ function deserializeObject(
(buffer[index + 1] << 8) |
(buffer[index + 2] << 16) |
(buffer[index + 3] << 24);
let arrayOptions = options;
let arrayOptions: DeserializeOptions = options;

// Stop index
const stopIndex = index + objectSize;

// All elements of array to be returned as raw bson
if (fieldsAsRaw && fieldsAsRaw[name]) {
arrayOptions = {};
for (const n in options) {
(
arrayOptions as {
[key: string]: DeserializeOptions[keyof DeserializeOptions];
}
)[n] = options[n as keyof DeserializeOptions];
}
arrayOptions['raw'] = true;
arrayOptions = { ...options, raw: true };
}

if (!globalUTFValidation) {
arrayOptions = { ...arrayOptions, validation: { utf8: shouldValidateKey } };
}
Expand Down
2 changes: 1 addition & 1 deletion src/parser/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ export function serializeInto(
}

// Iterate over all the keys
for (const key in object) {
for (const key of Object.keys(object)) {
let value = object[key];
// Is there an override value
if (typeof value?.toBSON === 'function') {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
'use strict';

const BSON = require('../register-bson');
import * as BSON from '../register-bson';
const EJSON = BSON.EJSON;
const vm = require('vm');
import * as vm from 'node:vm';

// BSON types
const Binary = BSON.Binary;
Expand Down Expand Up @@ -30,6 +28,7 @@ function getOldBSON() {
try {
// do a dynamic resolve to avoid exception when running browser tests
const file = require.resolve('bson');
// eslint-disable-next-line @typescript-eslint/no-var-requires
const oldModule = require(file).BSON;
const funcs = new oldModule.BSON();
oldModule.serialize = funcs.serialize;
Expand All @@ -49,7 +48,7 @@ describe('Extended JSON', function () {

before(function () {
const buffer = Buffer.alloc(64);
for (var i = 0; i < buffer.length; i++) buffer[i] = i;
for (let i = 0; i < buffer.length; i++) buffer[i] = i;
const date = new Date();
date.setTime(1488372056737);
doc = {
Expand Down Expand Up @@ -80,15 +79,15 @@ describe('Extended JSON', function () {

it('should correctly extend an existing mongodb module', function () {
// TODO(NODE-4377): doubleNumberIntFit should be a double not a $numberLong
var json =
const 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"}}';

expect(json).to.equal(EJSON.stringify(doc, null, 0, { relaxed: false }));
});

it('should correctly deserialize using the default relaxed mode', function () {
// Deserialize the document using non strict mode
var doc1 = EJSON.parse(EJSON.stringify(doc, null, 0));
let doc1 = EJSON.parse(EJSON.stringify(doc, null, 0));

// Validate the values
expect(300).to.equal(doc1.int32Number);
Expand All @@ -109,23 +108,23 @@ describe('Extended JSON', function () {

it('should correctly serialize, and deserialize using built-in BSON', function () {
// Create a doc
var doc1 = {
const doc1 = {
int32: new Int32(10)
};

// Serialize the document
var text = EJSON.stringify(doc1, null, 0, { relaxed: false });
const text = EJSON.stringify(doc1, null, 0, { relaxed: false });
expect(text).to.equal('{"int32":{"$numberInt":"10"}}');

// Deserialize the json in strict and non strict mode
var doc2 = EJSON.parse(text, { relaxed: false });
let doc2 = EJSON.parse(text, { relaxed: false });
expect(doc2.int32._bsontype).to.equal('Int32');
doc2 = EJSON.parse(text);
expect(doc2.int32).to.equal(10);
});

it('should correctly serialize bson types when they are values', function () {
var serialized = EJSON.stringify(new ObjectId('591801a468f9e7024b6235ea'), { relaxed: false });
let serialized = EJSON.stringify(new ObjectId('591801a468f9e7024b6235ea'), { relaxed: false });
expect(serialized).to.equal('{"$oid":"591801a468f9e7024b6235ea"}');
serialized = EJSON.stringify(new ObjectID('591801a468f9e7024b6235ea'), { relaxed: false });
expect(serialized).to.equal('{"$oid":"591801a468f9e7024b6235ea"}');
Expand Down Expand Up @@ -183,8 +182,8 @@ describe('Extended JSON', function () {
expect(EJSON.parse('null')).to.be.null;
expect(EJSON.parse('[null]')[0]).to.be.null;

var input = '{"result":[{"_id":{"$oid":"591801a468f9e7024b623939"},"emptyField":null}]}';
var parsed = EJSON.parse(input);
const input = '{"result":[{"_id":{"$oid":"591801a468f9e7024b623939"},"emptyField":null}]}';
const parsed = EJSON.parse(input);

expect(parsed).to.deep.equal({
result: [{ _id: new ObjectId('591801a468f9e7024b623939'), emptyField: null }]
Expand Down Expand Up @@ -334,14 +333,14 @@ describe('Extended JSON', function () {
it('should work for function-valued and array-valued replacer parameters', function () {
const doc = { a: new Int32(10), b: new Int32(10) };

var replacerArray = ['a', '$numberInt'];
var serialized = EJSON.stringify(doc, replacerArray, 0, { relaxed: false });
const replacerArray = ['a', '$numberInt'];
let serialized = EJSON.stringify(doc, replacerArray, 0, { relaxed: false });
expect(serialized).to.equal('{"a":{"$numberInt":"10"}}');

serialized = EJSON.stringify(doc, replacerArray);
expect(serialized).to.equal('{"a":10}');

var replacerFunc = function (key, value) {
const replacerFunc = function (key, value) {
return key === 'b' ? undefined : value;
};
serialized = EJSON.stringify(doc, replacerFunc, 0, { relaxed: false });
Expand All @@ -352,11 +351,13 @@ describe('Extended JSON', function () {
});

if (!usingOldBSON) {
it.skip('skipping 4.x/1.x interop tests', () => {});
it.skip('skipping 4.x/1.x interop tests', () => {
// ignore
});
} else {
it('should interoperate 4.x with 1.x versions of this library', function () {
const buffer = Buffer.alloc(64);
for (var i = 0; i < buffer.length; i++) {
for (let i = 0; i < buffer.length; i++) {
buffer[i] = i;
}
const [oldBsonObject, newBsonObject] = [OldBSON, BSON].map(bsonModule => {
Expand Down Expand Up @@ -454,7 +455,9 @@ describe('Extended JSON', function () {
// by mongodb-core, then remove this test case and uncomment the MinKey checks in the test case above
it('should interop with MinKey 1.x and 4.x, except the case that #310 breaks', function () {
if (!usingOldBSON) {
it.skip('interop tests', () => {});
it.skip('interop tests', () => {
// ignore
});
return;
}

Expand Down Expand Up @@ -516,7 +519,7 @@ describe('Extended JSON', function () {
const serialized = EJSON.stringify(original);
expect(serialized).to.equal('{"__proto__":{"a":42}}');
const deserialized = EJSON.parse(serialized);
expect(deserialized).to.have.deep.ownPropertyDescriptor('__proto__', {
expect(deserialized).to.have.ownPropertyDescriptor('__proto__', {
configurable: true,
enumerable: true,
writable: true,
Expand All @@ -527,7 +530,8 @@ describe('Extended JSON', function () {

context('circular references', () => {
it('should throw a helpful error message for input with circular references', function () {
const obj = {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const obj: any = {
some: {
property: {
array: []
Expand All @@ -542,7 +546,8 @@ Converting circular structure to EJSON:
});

it('should throw a helpful error message for input with circular references, one-level nested', function () {
const obj = {};
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const obj: any = {};
obj.obj = obj;
expect(() => EJSON.serialize(obj)).to.throw(`\
Converting circular structure to EJSON:
Expand All @@ -551,7 +556,8 @@ Converting circular structure to EJSON:
});

it('should throw a helpful error message for input with circular references, one-level nested inside base object', function () {
const obj = {};
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const obj: any = {};
obj.obj = obj;
expect(() => EJSON.serialize({ foo: obj })).to.throw(`\
Converting circular structure to EJSON:
Expand All @@ -560,7 +566,8 @@ Converting circular structure to EJSON:
});

it('should throw a helpful error message for input with circular references, pointing back to base object', function () {
const obj = { foo: {} };
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const obj: any = { foo: {} };
obj.foo.obj = obj;
expect(() => EJSON.serialize(obj)).to.throw(`\
Converting circular structure to EJSON:
Expand Down Expand Up @@ -785,4 +792,13 @@ Converting circular structure to EJSON:
expect(parsedUUID).to.deep.equal(expectedResult);
});
});

it('should only enumerate own property keys from input objects', () => {
const input = { a: 1 };
Object.setPrototypeOf(input, { b: 2 });
const string = EJSON.stringify(input);
expect(string).to.not.include(`"b":`);
const result = JSON.parse(string);
expect(result).to.deep.equal({ a: 1 });
});
});
10 changes: 10 additions & 0 deletions test/node/parser/calculate_size.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import * as BSON from '../../register-bson';
import { expect } from 'chai';

describe('calculateSize()', () => {
it('should only enumerate own property keys from input objects', () => {
const input = { a: 1 };
Object.setPrototypeOf(input, { b: 2 });
expect(BSON.calculateObjectSize(input)).to.equal(12);
});
});
18 changes: 18 additions & 0 deletions test/node/parser/deserializer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import * as BSON from '../../register-bson';
import { expect } from 'chai';

describe('deserializer()', () => {
describe('when the fieldsAsRaw options is present and has a value that corresponds to a key in the object', () => {
it('ignores non-own properties set on the options object', () => {
const bytes = BSON.serialize({ someKey: [1] });
const options = { fieldsAsRaw: { someKey: true } };
Object.setPrototypeOf(options, { promoteValues: false });
const result = BSON.deserialize(bytes, options);
expect(result).to.have.property('someKey').that.is.an('array');
expect(
result.someKey[0],
'expected promoteValues option set on options object prototype to be ignored, but it was not'
).to.not.have.property('_bsontype', 'Int32');
});
});
});
17 changes: 17 additions & 0 deletions test/node/parser/serializer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import * as BSON from '../../register-bson';
import { bufferFromHexArray } from '../tools/utils';
import { expect } from 'chai';

describe('serialize()', () => {
it('should only enumerate own property keys from input objects', () => {
const input = { a: 1 };
Object.setPrototypeOf(input, { b: 2 });
const bytes = BSON.serialize(input);
expect(bytes).to.deep.equal(
bufferFromHexArray([
'106100', // type int32, a\x00
'01000000' // int32LE = 1
])
);
});
});

0 comments on commit 5103e4d

Please sign in to comment.