Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Use nullary objects #58

Merged
merged 1 commit into from

3 participants

Eivind Fjeldstad Marc Harter TJ Holowaychuk
Eivind Fjeldstad

Don't allow Object prototype to be modified. Test will fail in IE < 9

TJ Holowaychuk tj merged commit 8d2276f into from
Marc Harter

nullary objects also remove toString() and valueOf() which means you can't compare values with ==

var object = Object.create(null)
object == 'thingy'
TypeError: Cannot convert object to primitive value

=== will work but i imagine having no Object.prototype anymore is causing issues for others (like in the reference above)

/cc @visionmedia

TJ Holowaychuk
Owner
tj commented

yarg :( silly javascript

Eivind Fjeldstad

An alternative could be to keep a blacklist of properties. Or even attatch the proto after the fact. Like

var obj = Object.create(null);
// parse querystring
obj.__proto__ = {}

Might be bad for performance though

Marc Harter

if I may inquire, what was the rationale for using nullary objects?

Object.create(null) is significantly slower than {}, not sure how hot this code path is in this case though:

http://jsperf.com/object-create-proto-after

TJ Holowaychuk
Owner
tj commented

__proto__ is plenty fast enough if we want to go that route, plus @wavded it's definitely much slower but at several million ops/s that doesn't matter

Marc Harter

yeah fair enough, i'm cool with __proto__

Eivind Fjeldstad

I have a feeling this is going to come up again and again. As long as Object.prototype is safe, I'm happy.

Marc Harter

Yeah I agree

TJ Holowaychuk
Owner
tj commented

0.6.3 has the fix

simo simov referenced this pull request in senchalabs/connect
Closed

BUG in bodyParser #953

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 26, 2013
  1. Eivind Fjeldstad

    use nullary objects

    eivindfjeldstad authored
This page is out of date. Refresh to see the latest.
Showing with 20 additions and 4 deletions.
  1. +14 −4 index.js
  2. +6 −0 test/parse.js
18 index.js
View
@@ -58,14 +58,24 @@ var reduce = function(arr, fn, initial) {
};
/**
+ * Create a nullary object if possible
+ */
+
+var objectCreate = function () {
+ return Object.create
+ ? Object.create(null)
+ : {};
+}
+
+/**
* Cache non-integer test regexp.
*/
var isint = /^[0-9]+$/;
function promote(parent, key) {
- if (parent[key].length == 0) return parent[key] = {};
- var t = {};
+ if (parent[key].length == 0) return parent[key] = objectCreate();
+ var t = objectCreate();
for (var i in parent[key]) t[i] = parent[key][i];
parent[key] = t;
return t;
@@ -121,7 +131,7 @@ function merge(parent, key, val){
// optimize
} else {
if (!isint.test(key) && isArray(parent.base)) {
- var t = {};
+ var t = objectCreate();
for (var k in parent.base) t[k] = parent.base[k];
parent.base = t;
}
@@ -160,7 +170,7 @@ function parseString(str){
if ('' == key) return ret;
return merge(ret, decode(key), decode(val));
- }, { base: {} }).base;
+ }, { base: objectCreate() }).base;
}
/**
6 test/parse.js
View
@@ -149,4 +149,10 @@ describe('qs.parse()', function(){
expect(qs.parse('_r=1&'))
.to.eql({ _r: '1' })
})
+
+ it('should not be possible to access Object prototype', function() {
+ qs.parse('constructor[prototype][bad]=bad');
+ qs.parse('bad[constructor][prototype][bad]=bad');
+ expect(Object.prototype.bad).to.be(undefined);
+ });
})
Something went wrong with that request. Please try again.