diff --git a/.changeset/large-pillows-try.md b/.changeset/large-pillows-try.md new file mode 100644 index 000000000..a2053843f --- /dev/null +++ b/.changeset/large-pillows-try.md @@ -0,0 +1,7 @@ +--- +'@cloudflare/next-on-pages': patch +--- + +Prevent middleware redirects applying search params. + +When a middleware function results in a redirect, the location header specified in the response is the full destination, including any search params, as written by the developer. Previously, we always applied search params to redirects found during the routing process, no matter what. This meant that we accidentally were applying search params to middleware redirects, which would alter the intended final destination. This change prevents that from happening. diff --git a/packages/next-on-pages/templates/_worker.js/handleRequest.ts b/packages/next-on-pages/templates/_worker.js/handleRequest.ts index 7cbc81aae..64ff3f635 100644 --- a/packages/next-on-pages/templates/_worker.js/handleRequest.ts +++ b/packages/next-on-pages/templates/_worker.js/handleRequest.ts @@ -71,13 +71,17 @@ async function generateResponse( output: VercelBuildOutput ): Promise { // Redirect user to external URL for redirects. - if (headers.normal.has('location')) { - // Apply the search params to the location header. - const location = headers.normal.get('location') ?? '/'; - const paramsStr = [...searchParams.keys()].length - ? `?${searchParams.toString()}` - : ''; - headers.normal.set('location', `${location}${paramsStr}`); + const locationHeader = headers.normal.get('location'); + if (locationHeader) { + // Apply the search params to the location header if it was not from middleware. + // Middleware that returns a redirect will specify the destination, including any search params + // that they want to include. Therefore, we should not be appending search params to those. + if (locationHeader !== headers.middlewareLocation) { + const paramsStr = [...searchParams.keys()].length + ? `?${searchParams.toString()}` + : ''; + headers.normal.set('location', `${locationHeader ?? '/'}${paramsStr}`); + } return new Response(null, { status, headers: headers.normal }); } diff --git a/packages/next-on-pages/templates/_worker.js/routes-matcher.ts b/packages/next-on-pages/templates/_worker.js/routes-matcher.ts index 81ce90b22..a479c2e9d 100644 --- a/packages/next-on-pages/templates/_worker.js/routes-matcher.ts +++ b/packages/next-on-pages/templates/_worker.js/routes-matcher.ts @@ -1,5 +1,5 @@ import { parse } from 'cookie'; -import type { MatchPCREResult } from './utils'; +import type { MatchPCREResult, MatchedSetHeaders } from './utils'; import { isLocaleTrailingSlashRegex, parseAcceptLanguage } from './utils'; import { applyHeaders, @@ -32,7 +32,7 @@ export class RoutesMatcher { /** Status for the response object */ public status: number | undefined; /** Headers for the response object */ - public headers: { normal: Headers; important: Headers }; + public headers: MatchedSetHeaders; /** Search params for the response object */ public searchParams: URLSearchParams; /** Custom response body from middleware */ @@ -184,6 +184,7 @@ export class RoutesMatcher { } applyHeaders(this.headers.normal, resp.headers); + this.headers.middlewareLocation = resp.headers.get('location'); } /** diff --git a/packages/next-on-pages/templates/_worker.js/utils/routing.ts b/packages/next-on-pages/templates/_worker.js/utils/routing.ts index 7a1a42df2..b05ad38b3 100644 --- a/packages/next-on-pages/templates/_worker.js/utils/routing.ts +++ b/packages/next-on-pages/templates/_worker.js/utils/routing.ts @@ -6,10 +6,27 @@ import { applySearchParams, } from './http'; +export type MatchedSetHeaders = { + /** + * The headers present on a source route. + * Gets applied to the final response before the response headers from running a function. + */ + normal: Headers; + /** + * The *important* headers - the ones present on a source route that specifies `important: true`. + * Gets applied to the final response after the response headers from running a function. + */ + important: Headers; + /** + * Tracks if a location header is found, and what the value is, after running a middleware function. + */ + middlewareLocation?: string | null; +}; + export type MatchedSet = { path: string; status: number | undefined; - headers: { normal: Headers; important: Headers }; + headers: MatchedSetHeaders; searchParams: URLSearchParams; body: BodyInit | undefined | null; }; diff --git a/packages/next-on-pages/tests/templates/requestTestData/middleware.ts b/packages/next-on-pages/tests/templates/requestTestData/middleware.ts index 9da820b99..b1f9c0c49 100644 --- a/packages/next-on-pages/tests/templates/requestTestData/middleware.ts +++ b/packages/next-on-pages/tests/templates/requestTestData/middleware.ts @@ -105,7 +105,7 @@ export const testSet: TestSet = { status: 307, data: '', headers: { - location: 'http://localhost/somewhere-else?redirect=', + location: 'http://localhost/somewhere-else', }, }, },