Break loop on magical key?

[lukeed]

Right now, as of 2.1.0, if a key named __proto__, constructor, or prototype is found, that assignment is skipped: https://github.com/lukeed/dset/blob/master/src/index.js#L6

But, in preparing for the 3.0 version, I'm left wondering if those keys should break the loop entirely? The reasoning is that if those keys are showing up, it's (most likely) a malicious call in the first place, so we might as well throw away the entire operation.

Without breaking, something like this is still possible:

let user = { age: 123 };
dset(user, '__proto__.isAdmin', true);
//=> skip "__proto__" key
//=> assign "isAdmin" key to root object

console.log(user);
//=> { age: 123, isAdmin: true }

While still mutated, this is still considered safe because only the given user object is affect. New objects (or new User objects) are not polluted – aka, do not inherit – a new isAdmin = true base property.

The above is effectively the same thing as this:

let user = { age: 123 };

dset(user, 'isAdmin', true);
//=> assign "isAdmin" key to root object

console.log(user);
//=> { age: 123, isAdmin: true }

Breaking on "__proto__" key (and the others) would change the 1st snippet such that nothing changes on the user object. Its final value would be { age: 123 } instead of the current { age: 123, isAdmin: true }.

[maraisr]

Just wanted to voice my opinion of this matter;

To me this feels like the right approach, meaning that dset(obj, 'a.__proto__.x'. 123) to produce { a: {} }, rather than hoist. Meaning we ignore all property south of a restricted word.

[lukeed]

FWIW, it looks like lodash/set does something equivalent to "breaking the loop" when they encounter a magical property too:

var input1 = {};
lodash(input1, '__proto__.foo', 123);
console.log(input1);  // => {}
// (dset currently) { foo: 123 }

var input2 = {};
lodash(input2, 'a.prototype.foo', 123);
console.log(input2);  // => {a: {} }
// (dset currently) {a: {foo: 123 }}