[querystring] avoid Object.prototype conflicts including __proto__

lib/querystring.js

@@ -4,6 +4,7 @@
 
 const QueryString = exports;
 const Buffer = require('buffer').Buffer;
+const defineProperty = Object.defineProperty;
 
 
 // a safe fast alternative to decodeURIComponent
@@ -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;
@@ -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;
     }
   }
 

test/parallel/test-querystring.js

@@ -9,6 +9,9 @@ var qs = require('querystring');
 // {{{
 // [ wonkyQS, canonicalQS, obj ]
 var qsTestCases = [
+  ['__proto__=1',
+   '__proto__=1',
+   JSON.parse('{"__proto__":"1"}')],
   ['foo=918854443121279438895193',
    'foo=918854443121279438895193',
    {'foo': '918854443121279438895193'}],