Create Map object that does not have __proto__ quirk #4728

Closed
GeorgeBailey opened this Issue Feb 7, 2013 · 14 comments

4 participants

@GeorgeBailey

Per StackOverflow question.

In Java, I find it straight forward to take a string and use it as a key in a LinkedHashMap. I can even translate it into JSON and back with no troubles.

I am using Node.JS/JavaScript now, and there is a special case that is not handled.

var makesSense = '{"__proto__":"foo","toString":"bar"}'
var noSense = JSON.stringify(JSON.parse('{"__proto__":"foo","toString":"bar"}'))
console.log(noSense) // outputs {"toString":"bar"}

I want a slick way to handle __proto__ and other things like it.

Why does this matter?

Surely nobody is actually going to type __proto__ by accident. But what if they were doing this on purpose. They learn I am using JavaScript, so what? - No problems except in the following situation:

  • Software has a array of strings. It just so happens one of these strings say __proto__ because someone was poking around trying to break my software.
  • Software creates a Map using those strings for a key, and fills the Map with some nice data.
  • Software later goes through the array of strings, and collects the information from the Map. The Map returns something null, and then boom: null pointer exception.
  • Software now does not work. This would qualify as something sort of like denial of service.

I know that that situation is absolutely vary rare, but I don't like it. I cannot remember all the quirks of the programming language I am using, so given enough time, I am bound to write this kind of code.

I pride myself on creating code that is not subject to tampering. So, I am attempting to eliminate these pinholes from my software.

I recommend that you allow creation of a special kind of object.

The code could be something like var myMap = new Map() instead of var myMap = {}. Creating a Map would mean the __proto__ key would be completely bypassed, as if it was not even there. __proto__, and similar quirks (like toString) would be treated like any other key.

I posted this issue on V8 as well. I really don't care whether this Map is added in V8 or if it is added in Node.JS, or both.

@bnoordhuis
Node.js Foundation member

Closing, not a node.js issue (and arguably not a V8 issue either.)

@bnoordhuis bnoordhuis closed this Feb 7, 2013
@GeorgeBailey

@bnoordhuis, It is a feature request for a new type of object. This different type of object would not have __proto__ in it, and thus would be more useful as a Map.

It is quite true that I could rely on a separate module, but it would probably not be as efficient, and would certainly require a different syntax.

@bnoordhuis
Node.js Foundation member

Golden rule of node.js core: don't extend the language. You're free to attempt something yourself with a module but we're not going to.

@GeorgeBailey

Thanks. I'll keep that in mind.

@bergus

@bnoordhuis: Actually it's not a extension of the language, but the removal of a non-standard extension. I'd love to see a config flag to just disable a public __proto__ properties on any object.

@GeorgeBailey

@bergus, It would be risky business to remove it globally. And after __proto__ you need to consider removing __defineSetter__, valueOf, and the likes.

I'd much prefer to have the ability to simply remove these for a given object only. If that was possible, I could easily make a constructor so that when you say new Map(), it removes those objects from this. (Map) As it stands now, delete x.__proto__ just doesn't work.

I think that @bnoordhuis has a good point. It really is part of the language, and has been around for longer than JSON. To allow removal of __proto__ would be an extension of the language, and thus it falls in the V8 department.

@isaacs

Actually it's not a extension of the language, but the removal of a non-standard extension.

@bergus __proto__ is standardized in the latest ES specs.

We don't modify the language. Maybe what you're looking for is the (very much slower) Object.create(null) or the much fancier stuff you get with the --harmony_collections flag.

@bergus

@GeorgeBailey: No, the difference to __defineSetter__, valueOf etc is that they are defined on Object.prototype, instead of being a property of the object itself. They can (and should) be bypassed by using Object.create(null) - Opera's JS engine for example even handles "proto" as a normal property on such prototypeless objects (haven't tested V8).

@isaacs: It was really standardized? Oh crap, I had hoped for Object.setPrototypeOf.

However, using --harmony_collections will solve the OPs actual problem. Great idea! According to MDN, Map supports ususal strings as well.

@GeorgeBailey
  1. using --harmony_collections

    That would mean a lot of syntax changes and a brand new way of mapping which is not compatible with all the programs out there. I would prefer my solution which is to simply allow removal of __proto__.

  2. I much prefer making Object.create(null) to turn __proto__ into a normal property, like Opera does.

But hey, maybe we should do both!

@isaacs

Using --harmony_collections would mean that you use a Map or WeakMap as your thing, instead of a plain old object.

If you want Object.create(null) to not have a magical __proto__ property, then take it up with TC-39, and then with V8.

I agree with you, this part of the language sucks. But Node is not the place to fix JavaScript.

@GeorgeBailey

I completely agree. I definitely want this. I am happy to suggest this to the TC-39 / ECMA people. I don't know how to do this correctly, so I can't do it now, maybe later.

@GeorgeBailey

the (very much slower) Object.create(null)

It takes twice as long, but not very long at all.

Anyone can test it on their own if they like.

  • for(var i = 0; i < 1000000; i++) Object.create(null)
  • for(var i = 0; i < 1000000; i++) ({})
@bnoordhuis
Node.js Foundation member

It takes twice as long, but not very long at all.

In my experience, it's more like 10-20x. In the grand scheme of things that's negligible outside of really hot code paths, though.

@GeorgeBailey

In my experience, it's more like 10-20x.

Yea, maybe my testing was flawed.

In the grand scheme of things that's negligible outside of really hot code paths, though.

Right on! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment