refactor: internalEvent.url is a full URL (#752)

Author: vicb

.changeset/smooth-laws-pull.md

@@ -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.

packages/open-next/src/adapters/edge-adapter.ts

@@ -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();
 
@@ -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");
 
@@ -43,7 +33,7 @@ const defaultHandler = async (
           i18n: NextConfig.i18n,
           trailingSlash: NextConfig.trailingSlash,
         },
-        url,
+        url: internalEvent.url,
         body: convertBodyToReadableStream(
           internalEvent.method,
           internalEvent.body,

packages/open-next/src/adapters/middleware.ts

@@ -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,
@@ -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

packages/open-next/src/core/requestHandler.ts

@@ -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,
@@ -102,7 +106,7 @@ export async function openNextHandler(
               rawPath: "/500",
               method: "GET",
               headers: {},
-              url: "/500",
+              url: constructNextUrl(internalEvent.url, "/500"),
               query: {},
               cookies: {},
               remoteAddress: "",
@@ -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" },

packages/open-next/src/core/routing/matcher.ts

@@ -14,6 +14,7 @@ import { emptyReadableStream, toReadableStream } from "utils/stream";
 import { debug } from "../../adapters/logger";
 import { localizePath } from "./i18n";
 import {
+  constructNextUrl,
   convertFromQueryString,
   convertToQueryString,
   escapeRegex,
@@ -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);
@@ -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) {
@@ -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));
@@ -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,
@@ -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;
@@ -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",

packages/open-next/src/core/routing/middleware.ts

@@ -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();
 
@@ -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(),
@@ -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;
     }
   }
 
@@ -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,

packages/open-next/src/core/routing/util.ts

@@ -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__

packages/open-next/src/core/routingHandler.ts

@@ -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;
@@ -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":

packages/open-next/src/overrides/converters/aws-apigw-v1.ts

@@ -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,
@@ -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 ?? {},

packages/open-next/src/overrides/converters/aws-apigw-v2.ts

@@ -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
@@ -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: