Skip to content

Commit f1e9457

Browse files
sommeeeervicb
sommeeeer
and
vicb
authored
add(patch): Ensure request.url is with correct protocol in route handlers (#865)
* add(patch): Ensure request.url is with correct protocol in route handlers * changeset * only replace the protocol * update snapshot * Update .changeset/afraid-sloths-shave.md Co-authored-by: Victor Berchet <[email protected]> * review --------- Co-authored-by: Victor Berchet <[email protected]>
1 parent 2c1de0b commit f1e9457

File tree

8 files changed

+172
-39
lines changed

8 files changed

+172
-39
lines changed

‎.changeset/afraid-sloths-shave.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@opennextjs/cloudflare": patch
3+
---
4+
5+
fix: Ensure request.url reflects the actual host protocol in route handlers

‎examples/e2e/app-pages-router/e2e/host.test.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
import { expect, test } from "@playwright/test";
22

33
/**
4-
* Tests that the request.url is the deployed host and not localhost
4+
* Tests that the request.url is correct
55
*
6-
* This test is skipped since e2e tests for the cloudflare adapter
7-
* run only locally to the baseURL doesn't match
86
*/
9-
test.skip("Request.url is host", async ({ baseURL, page }) => {
7+
test("Request.url is host", async ({ baseURL, page }) => {
108
await page.goto("/api/host");
119

1210
const el = page.getByText(`{"url":"${baseURL}/api/host"}`);

‎examples/e2e/app-router/e2e/host.test.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
import { expect, test } from "@playwright/test";
22

33
/**
4-
* Tests that the request.url is the deployed host and not localhost
4+
* Tests that the request.url is correct
55
*
6-
* This test is skipped since e2e tests for the cloudflare adapter
7-
* run only locally to the baseURL doesn't match
86
*/
9-
test.skip("Request.url is host", async ({ baseURL, page }) => {
7+
test("Request.url is host", async ({ baseURL, page }) => {
108
await page.goto("/api/host");
119

1210
const el = page.getByText(`{"url":"${baseURL}/api/host"}`);

‎examples/playground14/e2e/base.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ test.describe("playground/base", () => {
4545
await expect(res.json()).resolves.toEqual(expect.objectContaining({ TEST_ENV_VAR: "TEST_VALUE" }));
4646
});
4747

48-
test("returns correct information about the request from a route handler", async ({ page }) => {
48+
test("returns correct information about the request from a route handler", async ({ page, baseURL }) => {
4949
const res = await page.request.get("/api/request");
5050
// Next.js can fall back to `localhost:3000` or `n` if it doesn't get the host - neither of these are expected.
51-
const expectedURL = expect.stringMatching(/https?:\/\/localhost:(?!3000)\d+\/api\/request/);
51+
const expectedURL = `${baseURL}/api/request`;
5252
await expect(res.json()).resolves.toEqual({ nextUrl: expectedURL, url: expectedURL });
5353
});
5454

‎examples/playground14/e2e/isr.spec.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
import { test, expect, type APIResponse } from "@playwright/test";
2-
import type { BinaryLike } from "node:crypto";
3-
import { createHash } from "node:crypto";
42

53
test.describe("playground/isr", () => {
64
test("Generated pages exist", async ({ page }) => {

‎examples/playground15/e2e/base.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ test.describe("playground/base", () => {
4545
await expect(res.json()).resolves.toEqual(expect.objectContaining({ TEST_ENV_VAR: "TEST_VALUE" }));
4646
});
4747

48-
test("returns correct information about the request from a route handler", async ({ page }) => {
48+
test("returns correct information about the request from a route handler", async ({ page, baseURL }) => {
4949
const res = await page.request.get("/api/request");
5050
// Next.js can fall back to `localhost:3000` or `n` if it doesn't get the host - neither of these are expected.
51-
const expectedURL = expect.stringMatching(/https?:\/\/localhost:(?!3000)\d+\/api\/request/);
51+
const expectedURL = `${baseURL}/api/request`;
5252
await expect(res.json()).resolves.toEqual({ nextUrl: expectedURL, url: expectedURL });
5353
});
5454

‎packages/cloudflare/src/cli/build/patches/plugins/next-server.spec.ts‎

Lines changed: 112 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, test } from "vitest";
22

33
import { computePatchDiff } from "../../utils/test-patch.js";
44
import {
5+
attachRequestMetaRule,
56
buildIdRule,
67
createCacheHandlerRule,
78
createComposableCacheHandlersRule,
@@ -106,6 +107,64 @@ class NextNodeServer extends _baseserver.default {
106107
}
107108
}
108109
}
110+
getPrerenderManifest() {
111+
var _this_renderOpts, _this_serverOptions;
112+
if (this._cachedPreviewManifest) {
113+
return this._cachedPreviewManifest;
114+
}
115+
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) {
116+
this._cachedPreviewManifest = {
117+
version: 4,
118+
routes: {},
119+
dynamicRoutes: {},
120+
notFoundRoutes: [],
121+
preview: {
122+
previewModeId: require('crypto').randomBytes(16).toString('hex'),
123+
previewModeSigningKey: require('crypto').randomBytes(32).toString('hex'),
124+
previewModeEncryptionKey: require('crypto').randomBytes(32).toString('hex')
125+
}
126+
};
127+
return this._cachedPreviewManifest;
128+
}
129+
this._cachedPreviewManifest = (0, _loadmanifest.loadManifest)((0, _path.join)(this.distDir, _constants.PRERENDER_MANIFEST));
130+
return this._cachedPreviewManifest;
131+
}
132+
getRoutesManifest() {
133+
return (0, _tracer.getTracer)().trace(_constants2.NextNodeServerSpan.getRoutesManifest, ()=>{
134+
const manifest = (0, _loadmanifest.loadManifest)((0, _path.join)(this.distDir, _constants.ROUTES_MANIFEST));
135+
let rewrites = manifest.rewrites ?? {
136+
beforeFiles: [],
137+
afterFiles: [],
138+
fallback: []
139+
};
140+
if (Array.isArray(rewrites)) {
141+
rewrites = {
142+
beforeFiles: [],
143+
afterFiles: rewrites,
144+
fallback: []
145+
};
146+
}
147+
return {
148+
...manifest,
149+
rewrites
150+
};
151+
});
152+
}
153+
attachRequestMeta(req, parsedUrl, isUpgradeReq) {
154+
var _req_headers_xforwardedproto;
155+
// Injected in base-server.ts
156+
const protocol = ((_req_headers_xforwardedproto = req.headers['x-forwarded-proto']) == null ? void 0 : _req_headers_xforwardedproto.includes('https')) ? 'https' : 'http';
157+
// When there are hostname and port we build an absolute URL
158+
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;
159+
(0, _requestmeta.addRequestMeta)(req, 'initURL', initUrl);
160+
(0, _requestmeta.addRequestMeta)(req, 'initQuery', {
161+
...parsedUrl.query
162+
});
163+
(0, _requestmeta.addRequestMeta)(req, 'initProtocol', protocol);
164+
if (!isUpgradeReq) {
165+
(0, _requestmeta.addRequestMeta)(req, 'clonableBody', (0, _bodystreams.getCloneableBody)(req.originalRequest));
166+
}
167+
}
109168
// ...
110169
}`;
111170

@@ -212,6 +271,48 @@ class NextNodeServer extends _baseserver.default {
212271
test("disable node middleware", () => {
213272
expect(computePatchDiff("next-server.js", nextServerCode, disableNodeMiddlewareRule))
214273
.toMatchInlineSnapshot(`
274+
"Index: next-server.js
275+
===================================================================
276+
--- next-server.js
277+
+++ next-server.js
278+
@@ -1,5 +1,4 @@
279+
-
280+
class NextNodeServer extends _baseserver.default {
281+
constructor(options){
282+
// Initialize super class
283+
super(options);
284+
@@ -79,23 +78,10 @@
285+
pages: (0, _findpagesdir.findDir)(dir, "pages") ? true : false
286+
};
287+
}
288+
async loadNodeMiddleware() {
289+
- if (!process.env.NEXT_MINIMAL) {
290+
- try {
291+
- var _functionsConfig_functions;
292+
- const functionsConfig = this.renderOpts.dev ? {} : require((0, _path.join)(this.distDir, 'server', _constants.FUNCTIONS_CONFIG_MANIFEST));
293+
- if (this.renderOpts.dev || (functionsConfig == null ? void 0 : (_functionsConfig_functions = functionsConfig.functions) == null ? void 0 : _functionsConfig_functions['/_middleware'])) {
294+
- // if used with top level await, this will be a promise
295+
- return require((0, _path.join)(this.distDir, 'server', 'middleware.js'));
296+
- }
297+
- } catch (err) {
298+
- if ((0, _iserror.default)(err) && err.code !== 'ENOENT' && err.code !== 'MODULE_NOT_FOUND') {
299+
- throw err;
300+
- }
301+
- }
302+
- }
303+
- }
304+
+ // patched by open next
305+
+}
306+
getPrerenderManifest() {
307+
var _this_renderOpts, _this_serverOptions;
308+
if (this._cachedPreviewManifest) {
309+
return this._cachedPreviewManifest;
310+
"
311+
`);
312+
});
313+
314+
test("attachRequestMeta", () => {
315+
expect(computePatchDiff("next-server.js", nextServerCode, attachRequestMetaRule)).toMatchInlineSnapshot(`
215316
"Index: next-server.js
216317
===================================================================
217318
--- next-server.js
@@ -222,31 +323,17 @@ class NextNodeServer extends _baseserver.default {
222323
constructor(options){
223324
// Initialize super class
224325
super(options);
225-
@@ -79,21 +78,8 @@
226-
pages: (0, _findpagesdir.findDir)(dir, "pages") ? true : false
227-
};
228-
}
229-
async loadNodeMiddleware() {
230-
- if (!process.env.NEXT_MINIMAL) {
231-
- try {
232-
- var _functionsConfig_functions;
233-
- const functionsConfig = this.renderOpts.dev ? {} : require((0, _path.join)(this.distDir, 'server', _constants.FUNCTIONS_CONFIG_MANIFEST));
234-
- if (this.renderOpts.dev || (functionsConfig == null ? void 0 : (_functionsConfig_functions = functionsConfig.functions) == null ? void 0 : _functionsConfig_functions['/_middleware'])) {
235-
- // if used with top level await, this will be a promise
236-
- return require((0, _path.join)(this.distDir, 'server', 'middleware.js'));
237-
- }
238-
- } catch (err) {
239-
- if ((0, _iserror.default)(err) && err.code !== 'ENOENT' && err.code !== 'MODULE_NOT_FOUND') {
240-
- throw err;
241-
- }
242-
- }
243-
- }
244-
- }
245-
+ // patched by open next
246-
+}
247-
// ...
248-
}
249-
\\ No newline at end of file
326+
@@ -143,9 +142,9 @@
327+
// Injected in base-server.ts
328+
const protocol = ((_req_headers_xforwardedproto = req.headers['x-forwarded-proto']) == null ? void 0 : _req_headers_xforwardedproto.includes('https')) ? 'https' : 'http';
329+
// When there are hostname and port we build an absolute URL
330+
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;
331+
- (0, _requestmeta.addRequestMeta)(req, 'initURL', initUrl);
332+
+ (0, _requestmeta.addRequestMeta)(req, 'initURL', req[Symbol.for("NextInternalRequestMeta")]?.initProtocol === "http:" && initUrl.startsWith("https://") ? \`http://\${initUrl.slice(8)}\`: initUrl);
333+
(0, _requestmeta.addRequestMeta)(req, 'initQuery', {
334+
...parsedUrl.query
335+
});
336+
(0, _requestmeta.addRequestMeta)(req, 'initProtocol', protocol);
250337
"
251338
`);
252339
});

‎packages/cloudflare/src/cli/build/patches/plugins/next-server.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ export function patchNextServer(updater: ContentUpdater, buildOpts: BuildOptions
4242
// Node middleware are not supported on Cloudflare yet
4343
contents = patchCode(contents, disableNodeMiddlewareRule);
4444

45+
contents = patchCode(contents, attachRequestMetaRule);
46+
4547
return contents;
4648
},
4749
},
@@ -118,3 +120,48 @@ fix: |-
118120
globalThis[handlersSetSymbol] = new Set(globalThis[handlersMapSymbol].values());
119121
`;
120122
}
123+
124+
/**
125+
* `attachRequestMeta` sets `initUrl` to always be with `https` cause this.fetchHostname && this.port is undefined in our case.
126+
* this.nextConfig.experimental.trustHostHeader is also true.
127+
*
128+
* This patch checks if the original protocol was "http:" and rewrites the `initUrl` to reflect the actual host protocol.
129+
* It will make `request.url` in route handlers end up with the correct protocol.
130+
*
131+
* Note: We cannot use the already defined `initURL` we passed in as requestMetaData to NextServer's request handler as pages router
132+
* data routes would fail. It would miss the `_next/data` part in the path in that case.
133+
*
134+
* Therefor we just replace the protocol if necessary in the value from this template string:
135+
* https://github.com/vercel/next.js/blob/ea08bf27/packages/next/src/server/next-server.ts#L1920
136+
*
137+
* Affected lines:
138+
* https://github.com/vercel/next.js/blob/ea08bf27/packages/next/src/server/next-server.ts#L1916-L1923
139+
*
140+
* Callstack: handleRequest-> handleRequestImpl -> attachRequestMeta
141+
*
142+
*/
143+
export const attachRequestMetaRule = `
144+
rule:
145+
kind: identifier
146+
regex: ^initUrl$
147+
inside:
148+
kind: arguments
149+
all:
150+
- has: {kind: identifier, regex: ^req$}
151+
- has: {kind: string, regex: initURL}
152+
inside:
153+
kind: call_expression
154+
all:
155+
- has: {kind: parenthesized_expression, regex: '0'}
156+
- has: { regex: _requestmeta.addRequestMeta}
157+
inside:
158+
kind: expression_statement
159+
inside:
160+
kind: statement_block
161+
inside:
162+
kind: method_definition
163+
has:
164+
kind: property_identifier
165+
regex: ^attachRequestMeta$
166+
fix:
167+
'req[Symbol.for("NextInternalRequestMeta")]?.initProtocol === "http:" && initUrl.startsWith("https://") ? \`http://\${initUrl.slice(8)}\`: initUrl'`;

0 commit comments

Comments
 (0)