Skip to content

Commit

Permalink
querystring: avoid conflicts with Object.prototype and __proto__
Browse files Browse the repository at this point in the history
In order to solve nodejs#5642 , keeping an eye and actually most likely
improving performance for nodejs#728, this PR is meant to show
a better way to deal with inheritance.

As most common CS related problem say, "hashes" are faster than O(N*?)
searches in an array and this is the reason I've proposed a solution
based on `in` operator first.
  • Loading branch information
WebReflection committed Mar 14, 2016
1 parent 0ea3899 commit b121816
Showing 1 changed file with 61 additions and 26 deletions.
87 changes: 61 additions & 26 deletions lib/querystring.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

const QueryString = exports;
const Buffer = require('buffer').Buffer;
const defineProperty = Object.defineProperty;


// a safe fast alternative to decodeURIComponent
Expand Down Expand Up @@ -267,20 +268,37 @@ QueryString.parse = QueryString.decode = function(qs, sep, eq, options) {
key = decodeStr(key, decode);
if (valEncoded)
value = decodeStr(value, decode);
// Use a key array lookup instead of using hasOwnProperty(), which is
// slower
if (keys.indexOf(key) === -1) {
obj[key] = value;
keys[keys.length] = key;
// most common case is to define unique keys per query string
// using the `in` operator we speed up the assignment of unique values
// instead of performing an indexOf each time.
// On top of that, we are sure the property is not conflicting
// with those in Object.prototype so it's also safer.
// However, if the object had already a key, even as own property
// we need to check the indexOf instantly after.
if (key in obj) {
// Use a key array lookup instead of using hasOwnProperty(),
// which is slower
if (keys.indexOf(key) === -1) {
keys[keys.length] = key;
defineProperty(obj, key, {
configurable: true,
enumerable: true,
writable: true,
value: value
});
} else {
const curValue = obj[key];
// `instanceof Array` is used instead of Array.isArray() because it
// is ~15-20% faster with v8 4.7 and is safe to use because we are
// using it with values being created within this function
if (curValue instanceof Array)
curValue[curValue.length] = value;
else
obj[key] = [curValue, value];
}
} else {
const curValue = obj[key];
// `instanceof Array` is used instead of Array.isArray() because it
// is ~15-20% faster with v8 4.7 and is safe to use because we are
// using it with values being created within this function
if (curValue instanceof Array)
curValue[curValue.length] = value;
else
obj[key] = [curValue, value];
keys[keys.length] = key;
obj[key] = value;
}
if (--pairs === 0)
break;
Expand Down Expand Up @@ -370,20 +388,37 @@ QueryString.parse = QueryString.decode = function(qs, sep, eq, options) {
key = decodeStr(key, decode);
if (valEncoded)
value = decodeStr(value, decode);
// Use a key array lookup instead of using hasOwnProperty(), which is
// slower
if (keys.indexOf(key) === -1) {
obj[key] = value;
keys[keys.length] = key;
// most common case is to define unique keys per query string
// using the `in` operator we speed up the assignment of unique values
// instead of performing an indexOf each time.
// On top of that, we are sure the property is not conflicting
// with those in Object.prototype so it's also safer.
// However, if the object had already a key, even as own property
// we need to check the indexOf instantly after.
if (key in obj) {
// Use a key array lookup instead of using hasOwnProperty(),
// which is slower
if (keys.indexOf(key) === -1) {
keys[keys.length] = key;
defineProperty(obj, key, {
configurable: true,
enumerable: true,
writable: true,
value: value
});
} else {
const curValue = obj[key];
// `instanceof Array` is used instead of Array.isArray() because it
// is ~15-20% faster with v8 4.7 and is safe to use because we are
// using it with values being created within this function
if (curValue instanceof Array)
curValue[curValue.length] = value;
else
obj[key] = [curValue, value];
}
} else {
const curValue = obj[key];
// `instanceof Array` is used instead of Array.isArray() because it
// is ~15-20% faster with v8 4.7 and is safe to use because we are
// using it with values being created within this function
if (curValue instanceof Array)
curValue[curValue.length] = value;
else
obj[key] = [curValue, value];
keys[keys.length] = key;
obj[key] = value;
}
}

Expand Down

0 comments on commit b121816

Please sign in to comment.