diff --git a/.changeset/preserve-query-param-encoding.md b/.changeset/preserve-query-param-encoding.md new file mode 100644 index 000000000..57a0dae1c --- /dev/null +++ b/.changeset/preserve-query-param-encoding.md @@ -0,0 +1,9 @@ +--- +"@opennextjs/aws": patch +--- + +Preserve percent-encoding of query parameters in `getQueryFromSearchParams` + +`getQueryFromSearchParams` iterated over `URLSearchParams.entries()`, which decodes the values. Since `convertToQueryString` does not re-encode them (by design, see #817), any query value containing reserved characters (`&`, `=`, `+`, spaces, ...) was corrupted when the query string was rebuilt for `req.url`. The parser now keeps the values encoded so they round-trip correctly through `convertToQueryString`. + +Fixes opennextjs/opennextjs-cloudflare#1134. diff --git a/packages/open-next/src/overrides/converters/utils.ts b/packages/open-next/src/overrides/converters/utils.ts index 252b10975..5a1712d68 100644 --- a/packages/open-next/src/overrides/converters/utils.ts +++ b/packages/open-next/src/overrides/converters/utils.ts @@ -27,9 +27,23 @@ export function extractHostFromHeaders( /** * Get the query object from an URLSearchParams * + * The values are kept percent-encoded so that they can be reused as-is by + * `convertToQueryString` (which does not re-encode). Iterating over + * `searchParams.entries()` would decode the values, and since + * `convertToQueryString` does not re-encode them, values containing + * reserved characters (`&`, `=`, `+`, spaces, ...) would corrupt the + * rebuilt query string. + * * @param searchParams * @returns */ export function getQueryFromSearchParams(searchParams: URLSearchParams) { - return getQueryFromIterator(searchParams.entries()); + const querystring = searchParams.toString(); + if (querystring === "") return {}; + return getQueryFromIterator( + querystring.split("&").map((part) => { + const [key, value] = part.split("="); + return [key, value] as const; + }), + ); } diff --git a/packages/tests-unit/tests/converters/utils.test.ts b/packages/tests-unit/tests/converters/utils.test.ts index 9ca7b71bb..4241cae82 100644 --- a/packages/tests-unit/tests/converters/utils.test.ts +++ b/packages/tests-unit/tests/converters/utils.test.ts @@ -1,4 +1,7 @@ -import { removeUndefinedFromQuery } from "@opennextjs/aws/overrides/converters/utils.js"; +import { + getQueryFromSearchParams, + removeUndefinedFromQuery, +} from "@opennextjs/aws/overrides/converters/utils.js"; describe("removeUndefinedFromQuery", () => { it("should remove undefined from query", () => { @@ -29,3 +32,43 @@ describe("removeUndefinedFromQuery", () => { expect(result).toEqual({}); }); }); + +describe("getQueryFromSearchParams", () => { + it("returns an empty object when there are no params", () => { + expect(getQueryFromSearchParams(new URLSearchParams(""))).toEqual({}); + }); + + it("returns a single param", () => { + expect(getQueryFromSearchParams(new URLSearchParams("key=value"))).toEqual({ + key: "value", + }); + }); + + it("groups repeated keys into an array", () => { + expect( + getQueryFromSearchParams(new URLSearchParams("key=value1&key=value2")), + ).toEqual({ + key: ["value1", "value2"], + }); + }); + + // https://github.com/opennextjs/opennextjs-cloudflare/issues/1134 + it("keeps reserved characters percent-encoded so the value survives convertToQueryString", () => { + const raw = + "authorizationURL=https%3A%2F%2Fexample.com%2Fauth%3Fresponse_type%3Dcode%2Bid_token%26state%3Dabc&oauthState=xyz"; + const query = getQueryFromSearchParams(new URLSearchParams(raw)); + + // The encoded value must be preserved (not decoded), otherwise the nested + // "&"/"="/"+"/space would break the rebuilt query string. + expect(query).toEqual({ + authorizationURL: + "https%3A%2F%2Fexample.com%2Fauth%3Fresponse_type%3Dcode%2Bid_token%26state%3Dabc", + oauthState: "xyz", + }); + + // The value round-trips back to the original once decoded. + expect(decodeURIComponent(query.authorizationURL as string)).toBe( + "https://example.com/auth?response_type=code+id_token&state=abc", + ); + }); +});