diff --git a/.changeset/afraid-sloths-shave.md b/.changeset/afraid-sloths-shave.md new file mode 100644 index 00000000..e73687a9 --- /dev/null +++ b/.changeset/afraid-sloths-shave.md @@ -0,0 +1,5 @@ +--- +"@opennextjs/cloudflare": patch +--- + +fix: Ensure request.url reflects the actual host protocol in route handlers diff --git a/examples/e2e/app-pages-router/e2e/host.test.ts b/examples/e2e/app-pages-router/e2e/host.test.ts index e5f6f3f0..8abe57ca 100644 --- a/examples/e2e/app-pages-router/e2e/host.test.ts +++ b/examples/e2e/app-pages-router/e2e/host.test.ts @@ -1,12 +1,10 @@ import { expect, test } from "@playwright/test"; /** - * Tests that the request.url is the deployed host and not localhost + * Tests that the request.url is correct * - * This test is skipped since e2e tests for the cloudflare adapter - * run only locally to the baseURL doesn't match */ -test.skip("Request.url is host", async ({ baseURL, page }) => { +test("Request.url is host", async ({ baseURL, page }) => { await page.goto("/api/host"); const el = page.getByText(`{"url":"${baseURL}/api/host"}`); diff --git a/examples/e2e/app-router/e2e/host.test.ts b/examples/e2e/app-router/e2e/host.test.ts index e5f6f3f0..8abe57ca 100644 --- a/examples/e2e/app-router/e2e/host.test.ts +++ b/examples/e2e/app-router/e2e/host.test.ts @@ -1,12 +1,10 @@ import { expect, test } from "@playwright/test"; /** - * Tests that the request.url is the deployed host and not localhost + * Tests that the request.url is correct * - * This test is skipped since e2e tests for the cloudflare adapter - * run only locally to the baseURL doesn't match */ -test.skip("Request.url is host", async ({ baseURL, page }) => { +test("Request.url is host", async ({ baseURL, page }) => { await page.goto("/api/host"); const el = page.getByText(`{"url":"${baseURL}/api/host"}`); diff --git a/examples/playground14/e2e/base.spec.ts b/examples/playground14/e2e/base.spec.ts index 9a075b5f..77d74ad5 100644 --- a/examples/playground14/e2e/base.spec.ts +++ b/examples/playground14/e2e/base.spec.ts @@ -45,10 +45,10 @@ test.describe("playground/base", () => { await expect(res.json()).resolves.toEqual(expect.objectContaining({ TEST_ENV_VAR: "TEST_VALUE" })); }); - test("returns correct information about the request from a route handler", async ({ page }) => { + test("returns correct information about the request from a route handler", async ({ page, baseURL }) => { const res = await page.request.get("/api/request"); // Next.js can fall back to `localhost:3000` or `n` if it doesn't get the host - neither of these are expected. - const expectedURL = expect.stringMatching(/https?:\/\/localhost:(?!3000)\d+\/api\/request/); + const expectedURL = `${baseURL}/api/request`; await expect(res.json()).resolves.toEqual({ nextUrl: expectedURL, url: expectedURL }); }); diff --git a/examples/playground14/e2e/isr.spec.ts b/examples/playground14/e2e/isr.spec.ts index 7927822d..1a85540b 100644 --- a/examples/playground14/e2e/isr.spec.ts +++ b/examples/playground14/e2e/isr.spec.ts @@ -1,6 +1,4 @@ import { test, expect, type APIResponse } from "@playwright/test"; -import type { BinaryLike } from "node:crypto"; -import { createHash } from "node:crypto"; test.describe("playground/isr", () => { test("Generated pages exist", async ({ page }) => { diff --git a/examples/playground15/e2e/base.spec.ts b/examples/playground15/e2e/base.spec.ts index 7b2de3ab..f67b1504 100644 --- a/examples/playground15/e2e/base.spec.ts +++ b/examples/playground15/e2e/base.spec.ts @@ -45,10 +45,10 @@ test.describe("playground/base", () => { await expect(res.json()).resolves.toEqual(expect.objectContaining({ TEST_ENV_VAR: "TEST_VALUE" })); }); - test("returns correct information about the request from a route handler", async ({ page }) => { + test("returns correct information about the request from a route handler", async ({ page, baseURL }) => { const res = await page.request.get("/api/request"); // Next.js can fall back to `localhost:3000` or `n` if it doesn't get the host - neither of these are expected. - const expectedURL = expect.stringMatching(/https?:\/\/localhost:(?!3000)\d+\/api\/request/); + const expectedURL = `${baseURL}/api/request`; await expect(res.json()).resolves.toEqual({ nextUrl: expectedURL, url: expectedURL }); }); diff --git a/packages/cloudflare/src/cli/build/patches/plugins/next-server.spec.ts b/packages/cloudflare/src/cli/build/patches/plugins/next-server.spec.ts index 426c99f4..de12d23e 100644 --- a/packages/cloudflare/src/cli/build/patches/plugins/next-server.spec.ts +++ b/packages/cloudflare/src/cli/build/patches/plugins/next-server.spec.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "vitest"; import { computePatchDiff } from "../../utils/test-patch.js"; import { + attachRequestMetaRule, buildIdRule, createCacheHandlerRule, createComposableCacheHandlersRule, @@ -106,6 +107,64 @@ class NextNodeServer extends _baseserver.default { } } } + getPrerenderManifest() { + var _this_renderOpts, _this_serverOptions; + if (this._cachedPreviewManifest) { + return this._cachedPreviewManifest; + } + if (((_this_renderOpts = this.renderOpts) == null ? void 0 : _this_renderOpts.dev) || ((_this_serverOptions = this.serverOptions) == null ? void 0 : _this_serverOptions.dev) || process.env.NODE_ENV === 'development' || process.env.NEXT_PHASE === _constants.PHASE_PRODUCTION_BUILD) { + this._cachedPreviewManifest = { + version: 4, + routes: {}, + dynamicRoutes: {}, + notFoundRoutes: [], + preview: { + previewModeId: require('crypto').randomBytes(16).toString('hex'), + previewModeSigningKey: require('crypto').randomBytes(32).toString('hex'), + previewModeEncryptionKey: require('crypto').randomBytes(32).toString('hex') + } + }; + return this._cachedPreviewManifest; + } + this._cachedPreviewManifest = (0, _loadmanifest.loadManifest)((0, _path.join)(this.distDir, _constants.PRERENDER_MANIFEST)); + return this._cachedPreviewManifest; + } + getRoutesManifest() { + return (0, _tracer.getTracer)().trace(_constants2.NextNodeServerSpan.getRoutesManifest, ()=>{ + const manifest = (0, _loadmanifest.loadManifest)((0, _path.join)(this.distDir, _constants.ROUTES_MANIFEST)); + let rewrites = manifest.rewrites ?? { + beforeFiles: [], + afterFiles: [], + fallback: [] + }; + if (Array.isArray(rewrites)) { + rewrites = { + beforeFiles: [], + afterFiles: rewrites, + fallback: [] + }; + } + return { + ...manifest, + rewrites + }; + }); + } + attachRequestMeta(req, parsedUrl, isUpgradeReq) { + var _req_headers_xforwardedproto; + // Injected in base-server.ts + const protocol = ((_req_headers_xforwardedproto = req.headers['x-forwarded-proto']) == null ? void 0 : _req_headers_xforwardedproto.includes('https')) ? 'https' : 'http'; + // When there are hostname and port we build an absolute URL + const initUrl = this.fetchHostname && this.port ? \`\${protocol}://\${this.fetchHostname}:\${this.port}\${req.url}\` : this.nextConfig.experimental.trustHostHeader ? \`https://\${req.headers.host || "localhost"}\${req.url}\` : req.url; + (0, _requestmeta.addRequestMeta)(req, 'initURL', initUrl); + (0, _requestmeta.addRequestMeta)(req, 'initQuery', { + ...parsedUrl.query + }); + (0, _requestmeta.addRequestMeta)(req, 'initProtocol', protocol); + if (!isUpgradeReq) { + (0, _requestmeta.addRequestMeta)(req, 'clonableBody', (0, _bodystreams.getCloneableBody)(req.originalRequest)); + } + } // ... }`; @@ -212,6 +271,48 @@ class NextNodeServer extends _baseserver.default { test("disable node middleware", () => { expect(computePatchDiff("next-server.js", nextServerCode, disableNodeMiddlewareRule)) .toMatchInlineSnapshot(` + "Index: next-server.js + =================================================================== + --- next-server.js + +++ next-server.js + @@ -1,5 +1,4 @@ + - + class NextNodeServer extends _baseserver.default { + constructor(options){ + // Initialize super class + super(options); + @@ -79,23 +78,10 @@ + pages: (0, _findpagesdir.findDir)(dir, "pages") ? true : false + }; + } + async loadNodeMiddleware() { + - if (!process.env.NEXT_MINIMAL) { + - try { + - var _functionsConfig_functions; + - const functionsConfig = this.renderOpts.dev ? {} : require((0, _path.join)(this.distDir, 'server', _constants.FUNCTIONS_CONFIG_MANIFEST)); + - if (this.renderOpts.dev || (functionsConfig == null ? void 0 : (_functionsConfig_functions = functionsConfig.functions) == null ? void 0 : _functionsConfig_functions['/_middleware'])) { + - // if used with top level await, this will be a promise + - return require((0, _path.join)(this.distDir, 'server', 'middleware.js')); + - } + - } catch (err) { + - if ((0, _iserror.default)(err) && err.code !== 'ENOENT' && err.code !== 'MODULE_NOT_FOUND') { + - throw err; + - } + - } + - } + - } + + // patched by open next + +} + getPrerenderManifest() { + var _this_renderOpts, _this_serverOptions; + if (this._cachedPreviewManifest) { + return this._cachedPreviewManifest; + " + `); + }); + + test("attachRequestMeta", () => { + expect(computePatchDiff("next-server.js", nextServerCode, attachRequestMetaRule)).toMatchInlineSnapshot(` "Index: next-server.js =================================================================== --- next-server.js @@ -222,31 +323,17 @@ class NextNodeServer extends _baseserver.default { constructor(options){ // Initialize super class super(options); - @@ -79,21 +78,8 @@ - pages: (0, _findpagesdir.findDir)(dir, "pages") ? true : false - }; - } - async loadNodeMiddleware() { - - if (!process.env.NEXT_MINIMAL) { - - try { - - var _functionsConfig_functions; - - const functionsConfig = this.renderOpts.dev ? {} : require((0, _path.join)(this.distDir, 'server', _constants.FUNCTIONS_CONFIG_MANIFEST)); - - if (this.renderOpts.dev || (functionsConfig == null ? void 0 : (_functionsConfig_functions = functionsConfig.functions) == null ? void 0 : _functionsConfig_functions['/_middleware'])) { - - // if used with top level await, this will be a promise - - return require((0, _path.join)(this.distDir, 'server', 'middleware.js')); - - } - - } catch (err) { - - if ((0, _iserror.default)(err) && err.code !== 'ENOENT' && err.code !== 'MODULE_NOT_FOUND') { - - throw err; - - } - - } - - } - - } - + // patched by open next - +} - // ... - } - \\ No newline at end of file + @@ -143,9 +142,9 @@ + // Injected in base-server.ts + const protocol = ((_req_headers_xforwardedproto = req.headers['x-forwarded-proto']) == null ? void 0 : _req_headers_xforwardedproto.includes('https')) ? 'https' : 'http'; + // When there are hostname and port we build an absolute URL + const initUrl = this.fetchHostname && this.port ? \`\${protocol}://\${this.fetchHostname}:\${this.port}\${req.url}\` : this.nextConfig.experimental.trustHostHeader ? \`https://\${req.headers.host || "localhost"}\${req.url}\` : req.url; + - (0, _requestmeta.addRequestMeta)(req, 'initURL', initUrl); + + (0, _requestmeta.addRequestMeta)(req, 'initURL', req[Symbol.for("NextInternalRequestMeta")]?.initProtocol === "http:" && initUrl.startsWith("https://") ? \`http://\${initUrl.slice(8)}\`: initUrl); + (0, _requestmeta.addRequestMeta)(req, 'initQuery', { + ...parsedUrl.query + }); + (0, _requestmeta.addRequestMeta)(req, 'initProtocol', protocol); " `); }); diff --git a/packages/cloudflare/src/cli/build/patches/plugins/next-server.ts b/packages/cloudflare/src/cli/build/patches/plugins/next-server.ts index ef2dcde6..99355f18 100644 --- a/packages/cloudflare/src/cli/build/patches/plugins/next-server.ts +++ b/packages/cloudflare/src/cli/build/patches/plugins/next-server.ts @@ -42,6 +42,8 @@ export function patchNextServer(updater: ContentUpdater, buildOpts: BuildOptions // Node middleware are not supported on Cloudflare yet contents = patchCode(contents, disableNodeMiddlewareRule); + contents = patchCode(contents, attachRequestMetaRule); + return contents; }, }, @@ -118,3 +120,48 @@ fix: |- globalThis[handlersSetSymbol] = new Set(globalThis[handlersMapSymbol].values()); `; } + +/** + * `attachRequestMeta` sets `initUrl` to always be with `https` cause this.fetchHostname && this.port is undefined in our case. + * this.nextConfig.experimental.trustHostHeader is also true. + * + * This patch checks if the original protocol was "http:" and rewrites the `initUrl` to reflect the actual host protocol. + * It will make `request.url` in route handlers end up with the correct protocol. + * + * Note: We cannot use the already defined `initURL` we passed in as requestMetaData to NextServer's request handler as pages router + * data routes would fail. It would miss the `_next/data` part in the path in that case. + * + * Therefor we just replace the protocol if necessary in the value from this template string: + * https://github.com/vercel/next.js/blob/ea08bf27/packages/next/src/server/next-server.ts#L1920 + * + * Affected lines: + * https://github.com/vercel/next.js/blob/ea08bf27/packages/next/src/server/next-server.ts#L1916-L1923 + * + * Callstack: handleRequest-> handleRequestImpl -> attachRequestMeta + * + */ +export const attachRequestMetaRule = ` +rule: + kind: identifier + regex: ^initUrl$ + inside: + kind: arguments + all: + - has: {kind: identifier, regex: ^req$} + - has: {kind: string, regex: initURL} + inside: + kind: call_expression + all: + - has: {kind: parenthesized_expression, regex: '0'} + - has: { regex: _requestmeta.addRequestMeta} + inside: + kind: expression_statement + inside: + kind: statement_block + inside: + kind: method_definition + has: + kind: property_identifier + regex: ^attachRequestMeta$ +fix: + 'req[Symbol.for("NextInternalRequestMeta")]?.initProtocol === "http:" && initUrl.startsWith("https://") ? \`http://\${initUrl.slice(8)}\`: initUrl'`;