querystring: validate surrogate pairs in encodeStr

The surrogate pair path in `encodeStr` only checked that a second code
unit existed; it did not verify that the leading unit was a high
surrogate (0xD800-0xDBFF) or that the trailing unit was a low surrogate
(0xDC00-0xDFFF). A lone high surrogate followed by a non-surrogate (or
a lone low surrogate) was silently combined and emitted as a
syntactically valid four-byte UTF-8 sequence representing an entirely
different code point, producing malformed output that does not
roundtrip through `querystring.parse`.

Match `encodeURIComponent` semantics by throwing `ERR_INVALID_URI` when
either half of the pair is not a valid surrogate.

Co-Authored-By: Claude <[email protected]>
Co-Authored-By: DeepView Autofix <[email protected]>
Co-Authored-By: Nikita Skovoroda <[email protected]>
Signed-off-by: Nikita Skovoroda <[email protected]>

Author: 3 people

lib/internal/querystring.js

@@ -93,13 +93,15 @@ function encodeStr(str, noEscapeTable, hexTable) {
     // This branch should never happen because all URLSearchParams entries
     // should already be converted to USVString. But, included for
     // completion's sake anyway.
-    if (i >= len)
+    if (i >= len || c >= 0xDC00)
       throw new ERR_INVALID_URI();
 
-    const c2 = StringPrototypeCharCodeAt(str, i) & 0x3FF;
+    const c2 = StringPrototypeCharCodeAt(str, i);
+    if (c2 < 0xDC00 || c2 >= 0xE000)
+      throw new ERR_INVALID_URI();
 
     lastPos = i + 1;
-    c = 0x10000 + (((c & 0x3FF) << 10) | c2);
+    c = 0x10000 + (((c & 0x3FF) << 10) | (c2 & 0x3FF));
     out += hexTable[0xF0 | (c >> 18)] +
            hexTable[0x80 | ((c >> 12) & 0x3F)] +
            hexTable[0x80 | ((c >> 6) & 0x3F)] +

test/parallel/test-querystring-escape.js

@@ -10,17 +10,26 @@ assert.strictEqual(qs.escape({}), '%5Bobject%20Object%5D');
 assert.strictEqual(qs.escape([5, 10]), '5%2C10');
 assert.strictEqual(qs.escape('Ŋōđĕ'), '%C5%8A%C5%8D%C4%91%C4%95');
 assert.strictEqual(qs.escape('testŊōđĕ'), 'test%C5%8A%C5%8D%C4%91%C4%95');
-assert.strictEqual(qs.escape(`${String.fromCharCode(0xD800 + 1)}test`),
-                   '%F0%90%91%B4est');
+assert.strictEqual(
+  qs.escape(`${String.fromCharCode(0xD800 + 1)}${String.fromCharCode(0xDC00)}`),
+  '%F0%90%90%80');
 
-assert.throws(
-  () => qs.escape(String.fromCharCode(0xD800 + 1)),
-  {
-    code: 'ERR_INVALID_URI',
-    name: 'URIError',
-    message: 'URI malformed'
-  }
-);
+for (const invalid of [
+  String.fromCharCode(0xD800 + 1),
+  `${String.fromCharCode(0xD800 + 1)}test`,
+  `${String.fromCharCode(0xDC00)}test`,
+  `${String.fromCharCode(0xD800 + 1)}${String.fromCharCode(0x41)}`,
+  `${String.fromCharCode(0xD800 + 1)}${String.fromCharCode(0xD800)}`,
+]) {
+  assert.throws(
+    () => qs.escape(invalid),
+    {
+      code: 'ERR_INVALID_URI',
+      name: 'URIError',
+      message: 'URI malformed',
+    },
+  );
+}
 
 // Using toString for objects
 assert.strictEqual(