Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
9 changes: 9 additions & 0 deletions .changeset/smooth-laws-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@opennextjs/aws": patch
---

`InternalEvent#url` is now a full URL

BREAKING CHANGE: `InternalEvent#url` was only composed of the path and search query before.

Custom converters should be updated to populate the full URL instead.
14 changes: 2 additions & 12 deletions packages/open-next/src/adapters/edge-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ import type { OpenNextHandlerOptions } from "types/overrides";
// We import it like that so that the edge plugin can replace it
import { NextConfig } from "../adapters/config";
import { createGenericHandler } from "../core/createGenericHandler";
import {
convertBodyToReadableStream,
convertToQueryString,
} from "../core/routing/util";
import { convertBodyToReadableStream } from "../core/routing/util";

globalThis.__openNextAls = new AsyncLocalStorage();

Expand All @@ -25,13 +22,6 @@ const defaultHandler = async (
return runWithOpenNextRequestContext(
{ isISRRevalidation: false, waitUntil: options?.waitUntil },
async () => {
const host = internalEvent.headers.host
? `https://${internalEvent.headers.host}`
: "http://localhost:3000";
const initialUrl = new URL(internalEvent.rawPath, host);
initialUrl.search = convertToQueryString(internalEvent.query);
const url = initialUrl.toString();

// @ts-expect-error - This is bundled
const handler = await import("./middleware.mjs");

Expand All @@ -43,7 +33,7 @@ const defaultHandler = async (
i18n: NextConfig.i18n,
trailingSlash: NextConfig.trailingSlash,
},
url,
url: internalEvent.url,
body: convertBodyToReadableStream(
internalEvent.method,
internalEvent.body,
Expand Down
2 changes: 1 addition & 1 deletion packages/open-next/src/adapters/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ const defaultHandler = async (
internalEvent: {
...result.internalEvent,
rawPath: "/500",
url: "/500",
url: new URL("/500", new URL(result.internalEvent.url)).href,
method: "GET",
},
// On error we need to rewrite to the 500 page which is an internal rewrite
Expand Down
7 changes: 4 additions & 3 deletions packages/open-next/src/core/requestHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export async function openNextHandler(
rawPath: "/500",
method: "GET",
headers: {},
url: "/500",
url: new URL("/500", new URL(internalEvent.url)).href,
query: {},
cookies: {},
remoteAddress: "",
Expand Down Expand Up @@ -146,12 +146,13 @@ export async function openNextHandler(

const preprocessedEvent = routingResult.internalEvent;
debug("preprocessedEvent", preprocessedEvent);
const { search, pathname, hash } = new URL(preprocessedEvent.url);
const reqProps = {
method: preprocessedEvent.method,
url: preprocessedEvent.url,
url: `${pathname}${search}${hash}`,
//WORKAROUND: We pass this header to the serverless function to mimic a prefetch request which will not trigger revalidation since we handle revalidation differently
// There is 3 way we can handle revalidation:
// 1. We could just let the revalidation go as normal, but due to race condtions the revalidation will be unreliable
// 1. We could just let the revalidation go as normal, but due to race conditions the revalidation will be unreliable
// 2. We could alter the lastModified time of our cache to make next believe that the cache is fresh, but this could cause issues with stale data since the cdn will cache the stale data as if it was fresh
// 3. OUR CHOICE: We could pass a purpose prefetch header to the serverless function to make next believe that the request is a prefetch request and not trigger revalidation (This could potentially break in the future if next changes the behavior of prefetch requests)
headers: { ...headers, purpose: "prefetch" },
Expand Down
12 changes: 9 additions & 3 deletions packages/open-next/src/core/routing/matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,10 @@ export function handleRewrites<T extends RewriteDefinition>(
internalEvent: {
...event,
rawPath: rewrittenUrl,
url: `${rewrittenUrl}${convertToQueryString(finalQuery)}`,
url: new URL(
`${rewrittenUrl}${convertToQueryString(finalQuery)}`,
new URL(event.url),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it fails in e2e because event.url is incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know what test fails?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test("Missing cookies", async ({ page }) => {

test("Has cookies", async ({ page }) => {

test("Has cookies with value", async ({ page }) => {

And these 2
test("Redirect with default locale support", async ({ page }) => {
await page.goto("/redirect-with-locale/");
await page.waitForURL("/ssr/");
const el = page.getByText("SSR");
await expect(el).toBeVisible();
});
test("Redirect with locale support", async ({ page }) => {
await page.goto("/nl/redirect-with-locale/");
await page.waitForURL("/nl/ssr/");
const el = page.getByText("SSR");
await expect(el).toBeVisible();
});

).href,
},
__rewrite: rewrite,
isExternalRewrite,
Expand Down Expand Up @@ -365,7 +368,10 @@ export function fixDataPage(
...internalEvent,
rawPath: newPath,
query,
url: `${newPath}${convertToQueryString(query)}`,
url: new URL(
`${newPath}${convertToQueryString(query)}`,
new URL(internalEvent.url),
).href,
};
}
return internalEvent;
Expand Down Expand Up @@ -397,7 +403,7 @@ export function handleFallbackFalse(
event: {
...internalEvent,
rawPath: "/404",
url: "/404",
url: new URL("/404", new URL(internalEvent.url)).href,
headers: {
...internalEvent.headers,
"x-invoke-status": "404",
Expand Down
26 changes: 7 additions & 19 deletions packages/open-next/src/core/routing/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,9 @@ export async function handleMiddleware(
const hasMatch = middleMatch.some((r) => r.test(normalizedPath));
if (!hasMatch) return internalEvent;

// Retrieve the protocol:
// - In lambda, the url only contains the rawPath and the query - default to https
// - In cloudflare, the protocol is usually http in dev and https in production
const protocol = internalEvent.url.startsWith("http://") ? "http:" : "https:";

const host = headers.host
? `${protocol}//${headers.host}`
: "http://localhost:3000";

const initialUrl = new URL(normalizedPath, host);
const initialUrl = new URL(normalizedPath, new URL(internalEvent.url));
initialUrl.search = convertToQueryString(internalEvent.query);
const url = initialUrl.toString();
const url = initialUrl.href;

const middleware = await middlewareLoader();

Expand Down Expand Up @@ -131,12 +122,7 @@ export async function handleMiddleware(
// the redirected url and end the response.
if (statusCode >= 300 && statusCode < 400) {
resHeaders.location =
responseHeaders
.get("location")
?.replace(
"http://localhost:3000",
`${protocol}//${internalEvent.headers.host}`,
) ?? resHeaders.location;
responseHeaders.get("location") ?? resHeaders.location;
// res.setHeader("Location", location);
return {
body: emptyReadableStream(),
Expand All @@ -162,7 +148,7 @@ export async function handleMiddleware(
isExternalRewrite = true;
} else {
const rewriteUrlObject = new URL(rewriteUrl);
newUrl = rewriteUrlObject.pathname;
newUrl = rewriteUrlObject.href;

// Reset the query params if the middleware is a rewrite
if (middlewareQueryString.__nextDataReq) {
Expand Down Expand Up @@ -199,7 +185,9 @@ export async function handleMiddleware(
responseHeaders: resHeaders,
url: newUrl,
rawPath: rewritten
? (newUrl ?? internalEvent.rawPath)
? newUrl
? new URL(newUrl).pathname
: internalEvent.rawPath
: internalEvent.rawPath,
type: internalEvent.type,
headers: { ...internalEvent.headers, ...reqHeaders },
Expand Down
2 changes: 1 addition & 1 deletion packages/open-next/src/core/routingHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export default async function routingHandler(
internalEvent = {
...internalEvent,
rawPath: "/404",
url: "/404",
url: new URL("/404", new URL(internalEvent.url)).href,
headers: {
...internalEvent.headers,
"x-middleware-response-cache-control":
Expand Down
5 changes: 3 additions & 2 deletions packages/open-next/src/overrides/converters/aws-apigw-v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,14 @@ async function convertFromAPIGatewayProxyEvent(
event: APIGatewayProxyEvent,
): Promise<InternalEvent> {
const { path, body, httpMethod, requestContext, isBase64Encoded } = event;
const headers = normalizeAPIGatewayProxyEventHeaders(event);
return {
type: "core",
method: httpMethod,
rawPath: path,
url: path + normalizeAPIGatewayProxyEventQueryParams(event),
url: `https://${headers["x-forwarded-host"] ?? "on"}${path}${normalizeAPIGatewayProxyEventQueryParams(event)}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not 100% correct either, we could create an helper function that would retrieve the host from the headers.
If this header is not set (which can be the case), we should fallback to the host header

Copy link
Contributor Author

@vicb vicb Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean an helper to headers["x-forwarded-host"] ?? headers.on ?? "on" ?
For all aws converters?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my original idea, but i think there is a better solution (#752 (comment)). In the converters we keep headers.host ?? "on", but in the routing handler if we have a x-forwarded-host header, we change both the host header and the internalEvent.url

body: Buffer.from(body ?? "", isBase64Encoded ? "base64" : "utf8"),
headers: normalizeAPIGatewayProxyEventHeaders(event),
headers,
remoteAddress: requestContext.identity.sourceIp,
query: removeUndefinedFromQuery(
event.multiValueQueryStringParameters ?? {},
Expand Down
5 changes: 3 additions & 2 deletions packages/open-next/src/overrides/converters/aws-apigw-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,14 @@ async function convertFromAPIGatewayProxyEventV2(
event: APIGatewayProxyEventV2,
): Promise<InternalEvent> {
const { rawPath, rawQueryString, requestContext } = event;
const headers = normalizeAPIGatewayProxyEventV2Headers(event);
return {
type: "core",
method: requestContext.http.method,
rawPath,
url: rawPath + (rawQueryString ? `?${rawQueryString}` : ""),
url: `https://${headers["x-forwarded-host"] ?? "on"}${rawPath}${rawQueryString ? `?${rawQueryString}` : ""}`,
body: normalizeAPIGatewayProxyEventV2Body(event),
headers: normalizeAPIGatewayProxyEventV2Headers(event),
headers,
remoteAddress: requestContext.http.sourceIp,
query: removeUndefinedFromQuery(convertToQuery(rawQueryString)),
cookies:
Expand Down
18 changes: 12 additions & 6 deletions packages/open-next/src/overrides/converters/aws-cloudfront.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,23 +82,29 @@ function normalizeCloudFrontRequestEventHeaders(
async function convertFromCloudFrontRequestEvent(
event: CloudFrontRequestEvent,
): Promise<InternalEvent> {
const { method, uri, querystring, body, headers, clientIp } =
event.Records[0].cf.request;

const {
method,
uri,
querystring,
body,
headers: cfHeaders,
clientIp,
} = event.Records[0].cf.request;
const headers = normalizeCloudFrontRequestEventHeaders(cfHeaders);
return {
type: "core",
method,
rawPath: uri,
url: uri + (querystring ? `?${querystring}` : ""),
url: `https://${headers["x-forwarded-host"] ?? "on"}${uri}${querystring ? `?${querystring}` : ""}`,
body: Buffer.from(
body?.data ?? "",
body?.encoding === "base64" ? "base64" : "utf8",
),
headers: normalizeCloudFrontRequestEventHeaders(headers),
headers,
remoteAddress: clientIp,
query: convertToQuery(querystring),
cookies:
headers.cookie?.reduce(
cfHeaders.cookie?.reduce(
(acc, cur) => {
const { key = "", value } = cur;
acc[key] = value;
Expand Down
13 changes: 1 addition & 12 deletions packages/open-next/src/overrides/converters/edge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,7 @@ const converter: Converter<InternalEvent, InternalResult | MiddlewareResult> = {
},
convertTo: async (result) => {
if ("internalEvent" in result) {
let url = result.internalEvent.url;
if (!result.isExternalRewrite) {
if (result.origin) {
url = `${result.origin.protocol}://${result.origin.host}${
result.origin.port ? `:${result.origin.port}` : ""
}${url}`;
} else {
url = `https://${result.internalEvent.headers.host}${url}`;
}
}

const request = new Request(url, {
const request = new Request(result.internalEvent.url, {
body: result.internalEvent.body,
method: result.internalEvent.method,
headers: {
Expand Down
4 changes: 2 additions & 2 deletions packages/open-next/src/overrides/converters/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ const converter: Converter = {
});
});

const url = new URL(req.url!, `http://${req.headers.host}`);
const url = new URL(req.url!, `http://${req.headers.host ?? "on"}`);
const query = Object.fromEntries(url.searchParams.entries());
return {
type: "core",
method: req.method ?? "GET",
rawPath: url.pathname,
url: url.pathname + url.search,
url: url.href,
body,
headers: Object.fromEntries(
Object.entries(req.headers ?? {})
Expand Down
8 changes: 1 addition & 7 deletions packages/open-next/src/overrides/wrappers/cloudflare-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,7 @@ const handler: WrapperHandler<InternalEvent, InternalResult> =
}

const internalEvent = await converter.convertFrom(request);

// TODO:
// The edge converter populate event.url with the url including the origin.
// This is required for middleware to keep track of the protocol (i.e. http with wrangler dev).
// However the server expects that the origin is not included.
const url = new URL(internalEvent.url);
(internalEvent.url as string) = url.href.slice(url.origin.length);
const url = new URL(request.url);

const { promise: promiseResponse, resolve: resolveResponse } =
Promise.withResolvers<Response>();
Expand Down
1 change: 1 addition & 0 deletions packages/open-next/src/types/open-next.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export type BaseEventOrResult<T extends string = string> = {
export type InternalEvent = {
readonly method: string;
readonly rawPath: string;
// Full URL - starts with "https://on/" when the host is not available
readonly url: string;
readonly body?: Buffer;
readonly headers: Record<string, string>;
Expand Down
10 changes: 5 additions & 5 deletions packages/tests-unit/tests/converters/aws-apigw-v1.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe("convertFrom", () => {
type: "core",
method: "POST",
rawPath: "/",
url: "/",
url: "https://on/",
body: Buffer.from('{"message":"Hello, world!"}'),
headers: {
"content-type": "application/json",
Expand Down Expand Up @@ -126,7 +126,7 @@ describe("convertFrom", () => {
type: "core",
method: "POST",
rawPath: "/",
url: "/",
url: "https://on/",
body: Buffer.from('{"message":"Hello, world!"}'),
headers: {
test: "test1,test2",
Expand Down Expand Up @@ -167,7 +167,7 @@ describe("convertFrom", () => {
type: "core",
method: "POST",
rawPath: "/",
url: "/?test=test",
url: "https://on/?test=test",
body: Buffer.from('{"message":"Hello, world!"}'),
headers: {},
remoteAddress: "::1",
Expand Down Expand Up @@ -208,7 +208,7 @@ describe("convertFrom", () => {
type: "core",
method: "POST",
rawPath: "/",
url: "/",
url: "https://on/",
body: Buffer.from('{"message":"Hello, world!"}'),
headers: {
"content-type": "application/json",
Expand Down Expand Up @@ -251,7 +251,7 @@ describe("convertFrom", () => {
type: "core",
method: "GET",
rawPath: "/",
url: "/",
url: "https://on/",
body: Buffer.from("Hello, world!"),
headers: {
"content-type": "application/json",
Expand Down
8 changes: 4 additions & 4 deletions packages/tests-unit/tests/converters/aws-apigw-v2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ describe("convertFrom", () => {
type: "core",
method: "POST",
rawPath: "/",
url: "/",
url: "https://on/",
body: Buffer.from('{"message":"Hello, world!"}'),
headers: {
"content-type": "application/json",
Expand Down Expand Up @@ -163,7 +163,7 @@ describe("convertFrom", () => {
type: "core",
method: "POST",
rawPath: "/",
url: "/",
url: "https://on/",
body: Buffer.from('{"message":"Hello, world!"}'),
headers: {
"content-type": "application/json",
Expand Down Expand Up @@ -204,7 +204,7 @@ describe("convertFrom", () => {
type: "core",
method: "POST",
rawPath: "/",
url: "/?hello=world&foo=1&foo=2",
url: "https://on/?hello=world&foo=1&foo=2",
body: Buffer.from('{"message":"Hello, world!"}'),
headers: {
"content-type": "application/json",
Expand Down Expand Up @@ -246,7 +246,7 @@ describe("convertFrom", () => {
type: "core",
method: "POST",
rawPath: "/",
url: "/",
url: "https://on/",
body: Buffer.from('{"message":"Hello, world!"}'),
headers: {
"content-type": "application/json",
Expand Down
Loading