vadimdemedes/mongorito

Serialize doesn't check type of value

EdenCoder opened this issue · 7 comments

when you pass null to lib/serialize.js it will fail to map the object, we need to check if the value is null.

'use strict';

const mapObject = require('map-obj');

module.exports = obj => {
	return mapObject(obj, (key, value) => {
		if (typeof value.toBSON === 'function') {
			value = value.toBSON();
		}

		if (Array.isArray(value) && value[0] && typeof value[0].toBSON === 'function') {
			value = value.map(item => item.toBSON());
		}

		return [key, value];
	});
};

Should become

'use strict';

const mapObject = require('map-obj');

module.exports = obj => {
	return mapObject(obj, (key, value) => {
                if (!value) return [key, value];

		if (typeof value.toBSON === 'function') {
			value = value.toBSON();
		}

		if (Array.isArray(value) && value[0] && typeof value[0].toBSON === 'function') {
			value = value.map(item => item.toBSON());
		}

		return [key, value];
	});
};

I can create a pull request if it's easier, I already did for the string to object ID issue within findById

Under which condition would serialize() receive null as an input?

Migrating from mongorito@2.x to mongorito@3.x, there are cases in mongorito v2 where null would be passed as a parameter, for example an empty array is inserted as null.

Not sure I entirely understand the case you're describing. Could you submit a PR, which contains only the failing test to clearly demonstrate the problem? Would be very helpful!

@vadimdemedes

When you insert an empty array into a field, it's set in the db as null

@vadimdemedes when a property in mongo is set to null we might get null in serialize

const user = User.findOne({})
user.set('someKey', 'someVal');
user.save()  // if user.anotherKey was null, an exception is raised.

#182 should fix it. Waiting for @hlandao to fix linter errors and will publish a patch update immediately.

@vadimdemedes this is still breaking some of my production builds, have resorted to editing that file everywhere...

Any chance you can merge that?