querystring: don't inherit from Object.prototype

This commit safely allows querystring keys that are named the same as properties that are ordinarily inherited from Object.prototype such as __proto__. Additionally, this commit provides a bit of a speed improvement (~25% in the querystring-parse 'manypairs' benchmark) when there are many unique keys. Fixes: #5642 PR-URL: #6055 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Apr 26, 2016 · 33c2861 · 33c2861
1 parent d84b00c
commit 33c2861
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 1 deletion.
diff --git a/lib/querystring.js b/lib/querystring.js
@@ -5,6 +5,12 @@
 const QueryString = exports;
 const Buffer = require('buffer').Buffer;
 
+// This constructor is used to store parsed query string values. Instantiating
+// this is faster than explicitly calling `Object.create(null)` to get a
+// "clean" empty object (tested with v8 v4.9).
+function ParsedQueryString() {}
+ParsedQueryString.prototype = Object.create(null);
+
 
 // a safe fast alternative to decodeURIComponent
 QueryString.unescapeBuffer = function(s, decodeSpaces) {
@@ -216,7 +222,7 @@ QueryString.parse = QueryString.decode = function(qs, sep, eq, options) {
   sep = sep || '&';
   eq = eq || '=';
 
-  const obj = {};
+  const obj = new ParsedQueryString();
 
   if (typeof qs !== 'string' || qs.length === 0) {
     return obj;

diff --git a/test/parallel/test-querystring.js b/test/parallel/test-querystring.js
@@ -9,6 +9,12 @@ var qs = require('querystring');
 // {{{
 // [ wonkyQS, canonicalQS, obj ]
 var qsTestCases = [
+  ['__proto__=1',
+   '__proto__=1',
+   JSON.parse('{"__proto__":"1"}')],
+  ['__defineGetter__=asdf',
+   '__defineGetter__=asdf',
+   JSON.parse('{"__defineGetter__":"asdf"}')],
   ['foo=918854443121279438895193',
    'foo=918854443121279438895193',
    {'foo': '918854443121279438895193'}],