Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
---

add(patch): Ensure request.url is with correct 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
41 changes: 41 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,42 @@ fix: |-
globalThis[handlersSetSymbol] = new Set(globalThis[handlersMapSymbol].values());
`;
}

/**
*
* https://github.com/vercel/next.js/blob/ea08bf27/packages/next/src/server/next-server.ts#L1916-L1923
*
* initUrl will always be with `https` cause this.fetchHostname && this.port is undefined in our case.
* this.nextConfig.experimental.trustHostHeader is also true.
*
* Callstack: handleRequest-> handleRequestImpl -> attachRequestMeta
*
* We actually set this `initURL` already in nextServer.getRequestHandlerWithMetadata(metadata) as seen here:
* https://github.com/opennextjs/opennextjs-aws/blob/fe913bb/packages/open-next/src/core/requestHandler.ts#L259
*
*/
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'`;