Repository: denoland/deno
Pull request: fix(ext/node): querystring fallback to default decoder

Enables the full `tests/node_compat/test/parallel/test-querystring.js` test

Changed files:
filename: ext/node/polyfills/querystring.js
patch: @@ -79,10 +79,10 @@ function addKeyVal(
   decode,
 ) {
   if (key.length > 0 && keyEncoded) {
-    key = decode(key);
+    key = decodeStr(key, decode);
   }
   if (value.length > 0 && valEncoded) {
-    value = decode(value);
+    value = decodeStr(value, decode);
   }
 
   if (obj[key] === undefined) {
@@ -115,7 +115,7 @@ export function parse(
   str,
   sep = "&",
   eq = "=",
-  { decodeURIComponent = unescape, maxKeys = 1000 } = {},
+  { decodeURIComponent, maxKeys = 1000 } = {},
 ) {
   const obj = Object.create(null);
 
@@ -139,7 +139,7 @@ export function parse(
     pairs = maxKeys > 0 ? maxKeys : -1;
   }
 
-  let decode = unescape;
+  let decode = QS.unescape;
   if (decodeURIComponent) {
     decode = decodeURIComponent;
   }
@@ -466,6 +466,14 @@ function qsUnescape(s) {
   }
 }
 
+function decodeStr(s, decoder) {
+  try {
+    return decoder(s);
+  } catch {
+    return QS.unescape(s);
+  }
+}
+
 /**
  * Performs decoding of URL percent-encoded characters on the given `str`.
  * Used by `querystring.parse()` and is generally not expected to be used directly.
@@ -475,7 +483,7 @@ function qsUnescape(s) {
  */
 export const unescape = qsUnescape;
 
-export default {
+const QS = {
   parse,
   stringify,
   decode,
@@ -484,3 +492,5 @@ export default {
   escape,
   unescapeBuffer,
 };
+
+export default QS;

filename: tests/node_compat/config.jsonc
patch: @@ -120,7 +120,6 @@
       "test-path-normalize.js",
       "test-path-relative.js",
       "test-path-resolve.js",
-      "test-querystring.js",
       "test-readline-interface.js",
       "test-stdin-from-file-spawn.js",
       "test-stream-duplex-destroy.js",

filename: tests/node_compat/test/parallel/test-querystring.js
patch: @@ -2,8 +2,8 @@
 // deno-lint-ignore-file
 
 // Copyright Joyent and Node contributors. All rights reserved. MIT license.
-// Taken from Node 16.12.0
-// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually
+// Taken from Node 23.9.0
+// This file is automatically generated by `tests/node_compat/runner/setup.ts`. Do not modify this file manually.
 
 // Copyright Joyent, Inc. and other Node contributors.
 //
@@ -35,7 +35,7 @@ const inspect = require('util').inspect;
 const qs = require('querystring');
 
 function createWithNoPrototype(properties) {
-  const noProto = Object.create(null);
+  const noProto = { __proto__: null };
   properties.forEach((property) => {
     noProto[property.key] = property.value;
   });
@@ -154,6 +154,7 @@ const qsWeirdObjects = [
   [{ a: [], b: [] }, '', {}],
   [{ a: 1, b: [] }, 'a=1', { 'a': '1' }],
 ];
+// }}}
 
 const vm = require('vm');
 const foreignObject = vm.runInNewContext('({"foo": ["bar", "baz"]})');
@@ -435,16 +436,15 @@ check(qs.parse('%\u0100=%\u0101'), { '%Ā': '%ā' });
     { 'a=a': '', 'b=b': '', 'c=c': '' });
 }
 
-// TODO(wafuwafu13): Enable this
-// // Test QueryString.unescape
-// {
-//   function errDecode(str) {
-//     throw new Error('To jump to the catch scope');
-//   }
+// Test QueryString.unescape
+{
+  function errDecode(str) {
+    throw new Error('To jump to the catch scope');
+  }
 
-//   check(qs.parse('a=a', null, null, { decodeURIComponent: errDecode }),
-//         { a: 'a' });
-// }
+  check(qs.parse('a=a', null, null, { decodeURIComponent: errDecode }),
+        { a: 'a' });
+}
 
 // Test custom encode
 {
@@ -472,18 +472,16 @@ qsUnescapeTestCases.forEach((testCase) => {
   assert.strictEqual(qs.unescapeBuffer(testCase[0]).toString(), testCase[1]);
 });
 
-// TODO(wafuwafu13): Enable this
-// // Test overriding .unescape
-// {
-//   const prevUnescape = qs.unescape;
-//   qs.unescape = (str) => {
-//     return str.replace(/o/g, '_');
-//   };
-//   check(
-//     qs.parse('foo=bor'),
-//     createWithNoPrototype([{ key: 'f__', value: 'b_r' }]));
-//   qs.unescape = prevUnescape;
-// }
-
+// Test overriding .unescape
+{
+  const prevUnescape = qs.unescape;
+  qs.unescape = (str) => {
+    return str.replace(/o/g, '_');
+  };
+  check(
+    qs.parse('foo=bor'),
+    createWithNoPrototype([{ key: 'f__', value: 'b_r' }]));
+  qs.unescape = prevUnescape;
+}
 // Test separator and "equals" parsing order
 check(qs.parse('foo&bar', '&', '&'), { foo: '', bar: '' });