From 642c4a44a56ed02c2bf795bf04d7aebbc847e150 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Tue, 2 Jan 2024 10:24:11 +0530 Subject: [PATCH] fix(ext/node): querystring stringify without encode callback (#21740) Fixes https://github.com/denoland/deno/issues/21734 Changes: - Use default encode when options do not provide a encode callback. - Remove internal TS for `node:querystring`. Its not helping catching bugs like this because of invalid type assumptions and use of `any`, more of a maintenance burden. --- cli/tests/unit_node/querystring_test.ts | 21 +++++ ext/node/lib.rs | 2 +- .../{querystring.ts => querystring.js} | 90 ++++++------------- tools/core_import_map.json | 2 +- 4 files changed, 52 insertions(+), 63 deletions(-) rename ext/node/polyfills/{querystring.ts => querystring.js} (87%) diff --git a/cli/tests/unit_node/querystring_test.ts b/cli/tests/unit_node/querystring_test.ts index d750bed950..ce2747db2b 100644 --- a/cli/tests/unit_node/querystring_test.ts +++ b/cli/tests/unit_node/querystring_test.ts @@ -28,3 +28,24 @@ Deno.test({ }); }, }); + +// https://github.com/denoland/deno/issues/21734 +Deno.test({ + name: "stringify options no encode", + fn() { + assertEquals( + stringify( + { + a: "hello", + b: 5, + c: true, + d: ["foo", "bar"], + }, + "&", + "=", + {}, + ), + "a=hello&b=5&c=true&d=foo&d=bar", + ); + }, +}); diff --git a/ext/node/lib.rs b/ext/node/lib.rs index ac35f8f26b..2aac49754d 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -523,7 +523,7 @@ deno_core::extension!(deno_node, "perf_hooks.ts" with_specifier "node:perf_hooks", "process.ts" with_specifier "node:process", "punycode.ts" with_specifier "node:punycode", - "querystring.ts" with_specifier "node:querystring", + "querystring.js" with_specifier "node:querystring", "readline.ts" with_specifier "node:readline", "repl.ts" with_specifier "node:repl", "stream.ts" with_specifier "node:stream", diff --git a/ext/node/polyfills/querystring.ts b/ext/node/polyfills/querystring.js similarity index 87% rename from ext/node/polyfills/querystring.ts rename to ext/node/polyfills/querystring.js index 14bc3ea23d..5eb6a077aa 100644 --- a/ext/node/polyfills/querystring.ts +++ b/ext/node/polyfills/querystring.js @@ -22,7 +22,7 @@ export const encode = stringify; * replaces encodeURIComponent() * @see https://www.ecma-international.org/ecma-262/5.1/#sec-15.1.3.4 */ -function qsEscape(str: unknown): string { +function qsEscape(str) { if (typeof str !== "string") { if (typeof str === "object") { str = String(str); @@ -30,7 +30,7 @@ function qsEscape(str: unknown): string { str += ""; } } - return encodeStr(str as string, noEscape, hexTable); + return encodeStr(str, noEscape, hexTable); } /** @@ -42,29 +42,6 @@ function qsEscape(str: unknown): string { */ export const escape = qsEscape; -export interface ParsedUrlQuery { - [key: string]: string | string[] | undefined; -} - -export interface ParsedUrlQueryInput { - [key: string]: - | string - | number - | boolean - | ReadonlyArray - | ReadonlyArray - | ReadonlyArray - | null - | undefined; -} - -interface ParseOptions { - /** The function to use when decoding percent-encoded characters in the query string. */ - decodeURIComponent?: (string: string) => string; - /** Specifies the maximum number of keys to parse. */ - maxKeys?: number; -} - // deno-fmt-ignore const isHexTable = new Int8Array([ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0 - 15 @@ -85,7 +62,7 @@ const isHexTable = new Int8Array([ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // ... 256 ]); -function charCodes(str: string): number[] { +function charCodes(str) { const ret = new Array(str.length); for (let i = 0; i < str.length; ++i) { ret[i] = str.charCodeAt(i); @@ -94,12 +71,12 @@ function charCodes(str: string): number[] { } function addKeyVal( - obj: ParsedUrlQuery, - key: string, - value: string, - keyEncoded: boolean, - valEncoded: boolean, - decode: (encodedURIComponent: string) => string, + obj, + key, + value, + keyEncoded, + valEncoded, + decode, ) { if (key.length > 0 && keyEncoded) { key = decode(key); @@ -115,10 +92,10 @@ function addKeyVal( // A simple Array-specific property check is enough here to // distinguish from a string value and is faster and still safe // since we are generating all of the values being assigned. - if ((curValue as string[]).pop) { - (curValue as string[])[curValue!.length] = value; + if (curValue.pop) { + curValue[curValue.length] = value; } else { - obj[key] = [curValue as string, value]; + obj[key] = [curValue, value]; } } } @@ -135,12 +112,12 @@ function addKeyVal( * @see Tested in test-querystring.js */ export function parse( - str: string, + str, sep = "&", eq = "=", - { decodeURIComponent = unescape, maxKeys = 1000 }: ParseOptions = {}, -): ParsedUrlQuery { - const obj: ParsedUrlQuery = Object.create(null); + { decodeURIComponent = unescape, maxKeys = 1000 } = {}, +) { + const obj = Object.create(null); if (typeof str !== "string" || str.length === 0) { return obj; @@ -298,11 +275,6 @@ export function parse( return obj; } -interface StringifyOptions { - /** The function to use when converting URL-unsafe characters to percent-encoding in the query string. */ - encodeURIComponent: (string: string) => string; -} - /** * These characters do not need escaping when generating query strings: * ! - . _ ~ @@ -323,8 +295,7 @@ const noEscape = new Int8Array([ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 0, // 112 - 127 ]); -// deno-lint-ignore no-explicit-any -function stringifyPrimitive(v: any): string { +function stringifyPrimitive(v) { if (typeof v === "string") { return v; } @@ -341,15 +312,13 @@ function stringifyPrimitive(v: any): string { } function encodeStringifiedCustom( - // deno-lint-ignore no-explicit-any - v: any, - encode: (string: string) => string, -): string { + v, + encode, +) { return encode(stringifyPrimitive(v)); } -// deno-lint-ignore no-explicit-any -function encodeStringified(v: any, encode: (string: string) => string): string { +function encodeStringified(v, encode) { if (typeof v === "string") { return (v.length ? encode(v) : ""); } @@ -378,15 +347,14 @@ function encodeStringified(v: any, encode: (string: string) => string): string { * @see Tested in `test-querystring.js` */ export function stringify( - // deno-lint-ignore no-explicit-any - obj: Record, - sep?: string, - eq?: string, - options?: StringifyOptions, -): string { + obj, + sep, + eq, + options, +) { sep ||= "&"; eq ||= "="; - const encode = options ? options.encodeURIComponent : qsEscape; + const encode = options ? (options.encodeURIComponent || qsEscape) : qsEscape; const convert = options ? encodeStringifiedCustom : encodeStringified; if (obj !== null && typeof obj === "object") { @@ -448,7 +416,7 @@ const unhexTable = new Int8Array([ /** * A safe fast alternative to decodeURIComponent */ -export function unescapeBuffer(s: string, decodeSpaces = false): Buffer { +export function unescapeBuffer(s, decodeSpaces = false) { const out = Buffer.alloc(s.length); let index = 0; let outIndex = 0; @@ -490,7 +458,7 @@ export function unescapeBuffer(s: string, decodeSpaces = false): Buffer { return hasHex ? out.slice(0, outIndex) : out; } -function qsUnescape(s: string): string { +function qsUnescape(s) { try { return decodeURIComponent(s); } catch { diff --git a/tools/core_import_map.json b/tools/core_import_map.json index 6dff4c9306..8122a2c84d 100644 --- a/tools/core_import_map.json +++ b/tools/core_import_map.json @@ -181,7 +181,7 @@ "node:perf_hooks": "../ext/node/polyfills/perf_hooks.ts", "node:process": "../ext/node/polyfills/process.ts", "node:punycode": "../ext/node/polyfills/punycode.ts", - "node:querystring": "../ext/node/polyfills/querystring.ts", + "node:querystring": "../ext/node/polyfills/querystring.js", "node:readline": "../ext/node/polyfills/readline.ts", "ext:deno_node/readline/promises.ts": "../ext/node/polyfills/readline/promises.ts", "ext:deno_node/repl.ts": "../ext/node/polyfills/repl.ts",