Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/afraid-sloths-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@opennextjs/cloudflare": patch
---

fix: Ensure request.url reflects the actual host protocol in route handlers
6 changes: 2 additions & 4 deletions examples/e2e/app-pages-router/e2e/host.test.ts
Original file line number Diff line number Diff line change
@@ -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"}`);
Expand Down
6 changes: 2 additions & 4 deletions examples/e2e/app-router/e2e/host.test.ts
Original file line number Diff line number Diff line change
@@ -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"}`);
Expand Down
4 changes: 2 additions & 2 deletions examples/playground14/e2e/base.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});

Expand Down
2 changes: 0 additions & 2 deletions examples/playground14/e2e/isr.spec.ts
Original file line number Diff line number Diff line change
@@ -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 }) => {
Expand Down
4 changes: 2 additions & 2 deletions examples/playground15/e2e/base.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});

Expand Down
137 changes: 112 additions & 25 deletions packages/cloudflare/src/cli/build/patches/plugins/next-server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, expect, test } from "vitest";

import { computePatchDiff } from "../../utils/test-patch.js";
import {
attachRequestMetaRule,
buildIdRule,
createCacheHandlerRule,
createComposableCacheHandlersRule,
Expand Down Expand Up @@ -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));
}
}
// ...
}`;

Expand Down Expand Up @@ -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
Expand All @@ -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);
"
`);
});
Expand Down
47 changes: 47 additions & 0 deletions packages/cloudflare/src/cli/build/patches/plugins/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},
},
Expand Down Expand Up @@ -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'`;