Skip to content

refactor: internalEvent.url is a full URL #752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 26, 2025
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
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
3 changes: 2 additions & 1 deletion packages/open-next/src/adapters/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
resolveQueue,
resolveTagCache,
} from "../core/resolve";
import { constructNextUrl } from "../core/routing/util";
import routingHandler, {
INTERNAL_HEADER_INITIAL_PATH,
INTERNAL_HEADER_RESOLVED_ROUTES,
Expand Down Expand Up @@ -90,7 +91,7 @@ const defaultHandler = async (
internalEvent: {
...result.internalEvent,
rawPath: "/500",
url: "/500",
url: constructNextUrl(result.internalEvent.url, "/500"),
method: "GET",
},
// On error we need to rewrite to the 500 page which is an internal rewrite
Expand Down
13 changes: 9 additions & 4 deletions packages/open-next/src/core/requestHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ import { runWithOpenNextRequestContext } from "utils/promise";
import type { OpenNextHandlerOptions } from "types/overrides";
import { debug, error, warn } from "../adapters/logger";
import { patchAsyncStorage } from "./patchAsyncStorage";
import { convertRes, createServerResponse } from "./routing/util";
import {
constructNextUrl,
convertRes,
createServerResponse,
} from "./routing/util";
import routingHandler, {
INTERNAL_HEADER_INITIAL_PATH,
INTERNAL_HEADER_RESOLVED_ROUTES,
Expand Down Expand Up @@ -102,7 +106,7 @@ export async function openNextHandler(
rawPath: "/500",
method: "GET",
headers: {},
url: "/500",
url: constructNextUrl(internalEvent.url, "/500"),
query: {},
cookies: {},
remoteAddress: "",
Expand Down Expand Up @@ -146,12 +150,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
32 changes: 17 additions & 15 deletions packages/open-next/src/core/routing/matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { emptyReadableStream, toReadableStream } from "utils/stream";
import { debug } from "../../adapters/logger";
import { localizePath } from "./i18n";
import {
constructNextUrl,
convertFromQueryString,
convertToQueryString,
escapeRegex,
Expand Down Expand Up @@ -171,7 +172,7 @@ export function handleRewrites<T extends RewriteDefinition>(
event: InternalEvent,
rewrites: T[],
) {
const { rawPath, headers, query, cookies } = event;
const { rawPath, headers, query, cookies, url } = event;
const localizedRawPath = localizePath(event);
const matcher = routeHasMatcher(headers, cookies, query);
const computeHas = computeParamHas(headers, cookies, query);
Expand All @@ -185,7 +186,7 @@ export function handleRewrites<T extends RewriteDefinition>(
});
let finalQuery = query;

let rewrittenUrl = rawPath;
let rewrittenUrl = url;
const isExternalRewrite = isExternal(rewrite?.destination);
debug("isExternalRewrite", isExternalRewrite);
if (rewrite) {
Expand All @@ -202,16 +203,12 @@ export function handleRewrites<T extends RewriteDefinition>(
// %2B does not get interpreted as a space in the URL thus breaking the query string.
const encodePlusQueryString = queryString.replaceAll("+", "%20");
debug("urlParts", { pathname, protocol, hostname, queryString });
const toDestinationPath = compile(escapeRegex(pathname ?? "") ?? "");
const toDestinationHost = compile(escapeRegex(hostname ?? "") ?? "");
const toDestinationQuery = compile(
escapeRegex(encodePlusQueryString ?? "") ?? "",
);
const toDestinationPath = compile(escapeRegex(pathname));
const toDestinationHost = compile(escapeRegex(hostname));
const toDestinationQuery = compile(escapeRegex(encodePlusQueryString));
const params = {
// params for the source
...getParamsFromSource(match(escapeRegex(rewrite?.source) ?? ""))(
pathToUse,
),
...getParamsFromSource(match(escapeRegex(rewrite.source)))(pathToUse),
// params for the has
...rewrite.has?.reduce((acc, cur) => {
return Object.assign(acc, computeHas(cur));
Expand All @@ -232,20 +229,22 @@ export function handleRewrites<T extends RewriteDefinition>(
}
rewrittenUrl = isExternalRewrite
? `${protocol}//${rewrittenHost}${rewrittenPath}`
: `${rewrittenPath}`;
: new URL(rewrittenPath, event.url).href;

// Should we merge the query params or use only the ones from the rewrite?
finalQuery = {
...query,
...convertFromQueryString(rewrittenQuery),
};
rewrittenUrl += convertToQueryString(finalQuery);
debug("rewrittenUrl", { rewrittenUrl, finalQuery, isUsingParams });
}

return {
internalEvent: {
...event,
rawPath: rewrittenUrl,
url: `${rewrittenUrl}${convertToQueryString(finalQuery)}`,
rawPath: new URL(rewrittenUrl).pathname,
url: rewrittenUrl,
},
__rewrite: rewrite,
isExternalRewrite,
Expand Down Expand Up @@ -365,7 +364,10 @@ export function fixDataPage(
...internalEvent,
rawPath: newPath,
query,
url: `${newPath}${convertToQueryString(query)}`,
url: new URL(
`${newPath}${convertToQueryString(query)}`,
internalEvent.url,
).href,
};
}
return internalEvent;
Expand Down Expand Up @@ -397,7 +399,7 @@ export function handleFallbackFalse(
event: {
...internalEvent,
rawPath: "/404",
url: "/404",
url: constructNextUrl(internalEvent.url, "/404"),
headers: {
...internalEvent.headers,
"x-invoke-status": "404",
Expand Down
44 changes: 12 additions & 32 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, 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 @@ -155,28 +141,24 @@ export async function handleMiddleware(
let middlewareQueryString = internalEvent.query;
let newUrl = internalEvent.url;
if (rewriteUrl) {
newUrl = rewriteUrl;
rewritten = true;
// If not a string, it should probably throw
if (isExternal(rewriteUrl, internalEvent.headers.host as string)) {
newUrl = rewriteUrl;
rewritten = true;
if (isExternal(newUrl, internalEvent.headers.host as string)) {
isExternalRewrite = true;
} else {
const rewriteUrlObject = new URL(rewriteUrl);
newUrl = rewriteUrlObject.pathname;

// Reset the query params if the middleware is a rewrite
if (middlewareQueryString.__nextDataReq) {
middlewareQueryString = {
__nextDataReq: middlewareQueryString.__nextDataReq,
};
} else {
middlewareQueryString = {};
}
middlewareQueryString = middlewareQueryString.__nextDataReq
? {
__nextDataReq: middlewareQueryString.__nextDataReq,
}
: {};

rewriteUrlObject.searchParams.forEach((v: string, k: string) => {
middlewareQueryString[k] = v;
});
rewritten = true;
}
}

Expand All @@ -198,9 +180,7 @@ export async function handleMiddleware(
return {
responseHeaders: resHeaders,
url: newUrl,
rawPath: rewritten
? (newUrl ?? internalEvent.rawPath)
: internalEvent.rawPath,
rawPath: new URL(newUrl).pathname,
type: internalEvent.type,
headers: { ...internalEvent.headers, ...reqHeaders },
body: internalEvent.body,
Expand Down
15 changes: 15 additions & 0 deletions packages/open-next/src/core/routing/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,21 @@ export function getUrlParts(url: string, isExternal: boolean) {
};
}

/**
* Creates an URL to a Next page
*
* @param baseUrl Used to get the origin
* @param path The pathname
* @returns The Next URL considering the basePath
*
* @__PURE__
*/
export function constructNextUrl(baseUrl: string, path: string) {
const nextBasePath = NextConfig.basePath;
const url = new URL(`${nextBasePath}${path}`, baseUrl);
return url.href;
}

/**
*
* @__PURE__
Expand Down
3 changes: 2 additions & 1 deletion packages/open-next/src/core/routingHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
dynamicRouteMatcher,
staticRouteMatcher,
} from "./routing/routeMatcher";
import { constructNextUrl } from "./routing/util";

export const MIDDLEWARE_HEADER_PREFIX = "x-middleware-response-";
export const MIDDLEWARE_HEADER_PREFIX_LEN = MIDDLEWARE_HEADER_PREFIX.length;
Expand Down Expand Up @@ -174,7 +175,7 @@ export default async function routingHandler(
internalEvent = {
...internalEvent,
rawPath: "/404",
url: "/404",
url: constructNextUrl(internalEvent.url, "/404"),
headers: {
...internalEvent.headers,
"x-middleware-response-cache-control":
Expand Down
7 changes: 4 additions & 3 deletions packages/open-next/src/overrides/converters/aws-apigw-v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Converter } from "types/overrides";
import { fromReadableStream } from "utils/stream";

import { debug } from "../../adapters/logger";
import { removeUndefinedFromQuery } from "./utils";
import { extractHostFromHeaders, removeUndefinedFromQuery } from "./utils";

function normalizeAPIGatewayProxyEventHeaders(
event: APIGatewayProxyEvent,
Expand Down 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://${extractHostFromHeaders(headers)}${path}${normalizeAPIGatewayProxyEventQueryParams(event)}`,
body: Buffer.from(body ?? "", isBase64Encoded ? "base64" : "utf8"),
headers: normalizeAPIGatewayProxyEventHeaders(event),
headers,
remoteAddress: requestContext.identity.sourceIp,
query: removeUndefinedFromQuery(
event.multiValueQueryStringParameters ?? {},
Expand Down
7 changes: 4 additions & 3 deletions packages/open-next/src/overrides/converters/aws-apigw-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { fromReadableStream } from "utils/stream";

import { debug } from "../../adapters/logger";
import { convertToQuery } from "../../core/routing/util";
import { removeUndefinedFromQuery } from "./utils";
import { extractHostFromHeaders, removeUndefinedFromQuery } from "./utils";

// Not sure which one is really needed as this is not documented anywhere but server actions redirect are not working without this,
// it causes a 500 error from cloudfront itself with a 'x-amzErrortype: InternalFailure' header
Expand Down 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://${extractHostFromHeaders(headers)}${rawPath}${rawQueryString ? `?${rawQueryString}` : ""}`,
body: normalizeAPIGatewayProxyEventV2Body(event),
headers: normalizeAPIGatewayProxyEventV2Headers(event),
headers,
remoteAddress: requestContext.http.sourceIp,
query: removeUndefinedFromQuery(convertToQuery(rawQueryString)),
cookies:
Expand Down
Loading