diff --git a/packages/vinext/src/entries/pages-server-entry.ts b/packages/vinext/src/entries/pages-server-entry.ts index 74f897259..267bfd23a 100644 --- a/packages/vinext/src/entries/pages-server-entry.ts +++ b/packages/vinext/src/entries/pages-server-entry.ts @@ -312,7 +312,14 @@ export function matchPageRoute(url, request) { } function parseQuery(url) { - const qs = url.split("?")[1]; + // Per RFC 3986 only the first "?" separates path from query, so additional + // "?" chars belong to the query string (e.g. /linker?href=/about?hello=world + // has query "href=/about?hello=world"). split("?")[1] would drop everything + // after the second "?" and strip embedded query strings from values. + const queryIndex = url.indexOf("?"); + if (queryIndex === -1) return {}; + const hashIndex = url.indexOf("#", queryIndex + 1); + const qs = hashIndex === -1 ? url.slice(queryIndex + 1) : url.slice(queryIndex + 1, hashIndex); if (!qs) return {}; const p = new URLSearchParams(qs); const q = {}; diff --git a/packages/vinext/src/utils/query.ts b/packages/vinext/src/utils/query.ts index f82399888..ea859694e 100644 --- a/packages/vinext/src/utils/query.ts +++ b/packages/vinext/src/utils/query.ts @@ -55,9 +55,18 @@ export function mergeRouteParamsIntoQuery( /** * Parse a URL's query string into a Record, with multi-value keys promoted to arrays. + * + * Per RFC 3986 only the first `?` separates path from query; any further `?` + * characters are part of the query string itself (e.g. `/linker?href=/about?hello=world` + * has the query `href=/about?hello=world`). Using `indexOf("?")` instead of + * `split("?")[1]` preserves the rest of the query so values like `` + * targets keep their own query strings intact. */ export function parseQueryString(url: string): Record { - const qs = url.split("?")[1]; + const queryIndex = url.indexOf("?"); + if (queryIndex === -1) return {}; + const hashIndex = url.indexOf("#", queryIndex + 1); + const qs = hashIndex === -1 ? url.slice(queryIndex + 1) : url.slice(queryIndex + 1, hashIndex); if (!qs) return {}; const params = new URLSearchParams(qs); const query: Record = {}; diff --git a/tests/fixtures/pages-basic/pages/linker.tsx b/tests/fixtures/pages-basic/pages/linker.tsx new file mode 100644 index 000000000..ee1275f1c --- /dev/null +++ b/tests/fixtures/pages-basic/pages/linker.tsx @@ -0,0 +1,32 @@ +import Link from "next/link"; + +// Regression fixture for issue #1471: when `?href=/path?query=value` is passed +// through the URL, the Pages Router must preserve the embedded `?query=value` +// portion when rendering `` and when calling `router.push()`. +// Ported from Next.js: test/e2e/trailing-slashes/pages/linker.js +// +// `getServerSideProps` receives the parsed `query.href` value, which by RFC +// 3986 contains everything after the first `?`. The rendered `` must +// then output an `` that matches the original target verbatim. + +interface LinkerProps { + href: string; +} + +export default function Linker({ href }: LinkerProps) { + return ( +
+ + link to {href} + +
+ ); +} + +export async function getServerSideProps(context: { + query: Record; +}) { + const raw = context.query.href; + const href = typeof raw === "string" ? raw : "/"; + return { props: { href } }; +} diff --git a/tests/pages-router.test.ts b/tests/pages-router.test.ts index 66f3a8123..22b5a826a 100644 --- a/tests/pages-router.test.ts +++ b/tests/pages-router.test.ts @@ -1088,6 +1088,33 @@ describe("Pages Router integration", () => { expect(html).toContain("Server-Side Rendered"); }); + // Regression for cloudflare/vinext#1471: when a query value itself contains + // a query string (e.g. `?href=/about?hello=world`), the embedded `?hello=world` + // is part of the `href` value per RFC 3986 — only the first `?` separates the + // path from the query string. `getServerSideProps({ query })` must surface + // the full value so `` renders the complete target. + // Mirrors `test/e2e/trailing-slashes/pages/linker.js` from the Next.js suite. + it("Pages Router Link preserves an embedded query string in the href prop", async () => { + const res = await fetch(`${baseUrl}/linker?href=/about?hello=world`); + expect(res.status).toBe(200); + const html = await res.text(); + // The rendered link target must include the embedded `?hello=world`. The + // anchor uses `id="link"` to match Next.js's linker fixture; the literal + // anchor href is what `` resolves through normalizePathTrailingSlash + // and withBasePath. With trailingSlash:false and no basePath this is the + // exact source string. + expect(html).toContain('href="/about?hello=world"'); + }); + + it("Pages Router Link strips trailing slash before an embedded query string", async () => { + const res = await fetch(`${baseUrl}/linker?href=/about/?hello=world`); + expect(res.status).toBe(200); + const html = await res.text(); + // trailingSlash defaults to false — `/about/?hello=world` collapses to + // `/about?hello=world` while preserving the query. + expect(html).toContain('href="/about?hello=world"'); + }); + // Ported from Next.js: test/e2e/edge-pages-support/index.test.ts // https://github.com/vercel/next.js/blob/canary/test/e2e/edge-pages-support/index.test.ts // Closes cloudflare/vinext#1342: original query params must survive a diff --git a/tests/query.test.ts b/tests/query.test.ts index 6f3ea668e..e40275dbf 100644 --- a/tests/query.test.ts +++ b/tests/query.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vite-plus/test"; -import { appendSearchParamsToUrl, mergeRewriteQuery } from "../packages/vinext/src/utils/query.js"; +import { + appendSearchParamsToUrl, + mergeRewriteQuery, + parseQueryString, +} from "../packages/vinext/src/utils/query.js"; describe("mergeRewriteQuery", () => { it("preserves original query params when the rewrite target has none", () => { @@ -113,3 +117,39 @@ describe("appendSearchParamsToUrl", () => { expect(url).toBe("?lang=en&q=vinext"); }); }); + +describe("parseQueryString", () => { + it("returns an empty object when there is no query string", () => { + expect(parseQueryString("/about")).toEqual({}); + }); + + it("parses simple key=value pairs", () => { + expect(parseQueryString("/about?foo=bar")).toEqual({ foo: "bar" }); + }); + + it("promotes duplicate keys to arrays", () => { + expect(parseQueryString("/search?tag=a&tag=b")).toEqual({ tag: ["a", "b"] }); + }); + + // Regression for #1471: Pages Router `` strips query string from href. + // When a page is requested as `/linker?href=/about?hello=world`, the value of + // the `href` query param is `/about?hello=world` (RFC 3986 only treats the + // first `?` as a path/query separator). Splitting the URL on every `?` would + // drop the embedded query from the value and cause `` to + // render without the trailing query string. + it("preserves embedded query strings in values when the URL has multiple ?", () => { + expect(parseQueryString("/linker?href=/about?hello=world")).toEqual({ + href: "/about?hello=world", + }); + }); + + it("preserves embedded query strings whose value has a trailing-slash path", () => { + expect(parseQueryString("/linker?href=/about/?hello=world")).toEqual({ + href: "/about/?hello=world", + }); + }); + + it("stops at the URL hash fragment", () => { + expect(parseQueryString("/about?foo=bar#section")).toEqual({ foo: "bar" }); + }); +});