fix middleware not using the initial query

Author: conico974

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

@@ -10,16 +10,8 @@ import type { InternalEvent, InternalResult } from "types/open-next.js";
 import { emptyReadableStream } from "utils/stream.js";
 
 import { localizePath } from "./i18n/index.js";
-//NOTE: we should try to avoid importing stuff from next as much as possible
-// every release of next could break this
-// const { run } = require("next/dist/server/web/sandbox");
-// const { getCloneableBody } = require("next/dist/server/body-streams");
-// const {
-//   signalFromNodeResponse,
-// } = require("next/dist/server/web/spec-extension/adapters/next-request");
 import {
   convertBodyToReadableStream,
-  convertToQueryString,
   getMiddlewareMatch,
   isExternal,
 } from "./util.js";
@@ -45,14 +37,16 @@ function defaultMiddlewareLoader() {
   return import("./middleware.mjs");
 }
 
-// NOTE: As of Nextjs 13.4.13+, the middleware is handled outside the next-server.
-// OpenNext will run the middleware in a sandbox and set the appropriate req headers
-// and res.body prior to processing the next-server.
-// @returns undefined | res.end()
-
-//    if res.end() is return, the parent needs to return and not process next server
+/**
+ *
+ * @param internalEvent the internal event
+ * @param initialSearch the initial query string as it was received in the handler
+ * @param middlewareLoader Only used for unit test
+ * @returns `Promise<MiddlewareEvent | InternalResult>`
+ */
 export async function handleMiddleware(
   internalEvent: InternalEvent,
+  initialSearch: string,
   middlewareLoader: MiddlewareLoader = defaultMiddlewareLoader,
 ): Promise<MiddlewareEvent | InternalResult> {
   const headers = internalEvent.headers;
@@ -73,7 +67,7 @@ export async function handleMiddleware(
   if (!hasMatch) return internalEvent;
 
   const initialUrl = new URL(normalizedPath, internalEvent.url);
-  initialUrl.search = convertToQueryString(internalEvent.query);
+  initialUrl.search = initialSearch;
   const url = initialUrl.href;
 
   const middleware = await middlewareLoader();

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

@@ -100,7 +100,13 @@ export default async function routingHandler(
       return redirect;
     }
 
-    const eventOrResult = await handleMiddleware(internalEvent);
+    const eventOrResult = await handleMiddleware(
+      internalEvent,
+      // We need to pass the initial search without any decoding
+      // TODO: we'd need to refactor InternalEvent to include the initial querystring directly
+      // Should be done in another PR because it is a breaking change
+      new URL(event.url).search,
+    );
     const isResult = "statusCode" in eventOrResult;
     if (isResult) {
       return eventOrResult;

packages/tests-unit/tests/core/routing/middleware.test.ts

@@ -84,7 +84,7 @@ describe("handleMiddleware", () => {
         "x-prerender-revalidate": "preview",
       },
     });
-    const result = await handleMiddleware(event, middlewareLoader);
+    const result = await handleMiddleware(event, "", middlewareLoader);
 
     expect(middlewareLoader).not.toHaveBeenCalled();
     expect(result).toEqual(event);
@@ -103,7 +103,7 @@ describe("handleMiddleware", () => {
         location: "/redirect",
       }),
     });
-    const result = await handleMiddleware(event, middlewareLoader);
+    const result = await handleMiddleware(event, "", middlewareLoader);
 
     expect(middlewareLoader).toHaveBeenCalled();
     expect(result.statusCode).toEqual(302);
@@ -122,7 +122,7 @@ describe("handleMiddleware", () => {
         location: "/redirect",
       }),
     });
-    const result = await handleMiddleware(event, middlewareLoader);
+    const result = await handleMiddleware(event, "", middlewareLoader);
 
     expect(middlewareLoader).toHaveBeenCalled();
     expect(result.statusCode).toEqual(302);
@@ -137,7 +137,7 @@ describe("handleMiddleware", () => {
         location: "/redirect",
       }),
     });
-    const result = await handleMiddleware(event, middlewareLoader);
+    const result = await handleMiddleware(event, "", middlewareLoader);
 
     expect(middlewareLoader).toHaveBeenCalled();
     expect(result.statusCode).toEqual(302);
@@ -152,7 +152,7 @@ describe("handleMiddleware", () => {
         location: "http://external/redirect",
       }),
     });
-    const result = await handleMiddleware(event, middlewareLoader);
+    const result = await handleMiddleware(event, "", middlewareLoader);
 
     expect(middlewareLoader).toHaveBeenCalled();
     expect(result.statusCode).toEqual(302);
@@ -170,7 +170,7 @@ describe("handleMiddleware", () => {
         "x-middleware-rewrite": "http://localhost/rewrite",
       }),
     });
-    const result = await handleMiddleware(event, middlewareLoader);
+    const result = await handleMiddleware(event, "", middlewareLoader);
 
     expect(middlewareLoader).toHaveBeenCalled();
     expect(result).toEqual({
@@ -196,7 +196,7 @@ describe("handleMiddleware", () => {
         "x-middleware-rewrite": "http://localhost/rewrite?newKey=value",
       }),
     });
-    const result = await handleMiddleware(event, middlewareLoader);
+    const result = await handleMiddleware(event, "", middlewareLoader);
 
     expect(middlewareLoader).toHaveBeenCalled();
     expect(result).toEqual({
@@ -225,7 +225,7 @@ describe("handleMiddleware", () => {
         "x-middleware-rewrite": "http://external/rewrite",
       }),
     });
-    const result = await handleMiddleware(event, middlewareLoader);
+    const result = await handleMiddleware(event, "", middlewareLoader);
 
     expect(middlewareLoader).toHaveBeenCalled();
     expect(result).toEqual({
@@ -246,7 +246,7 @@ describe("handleMiddleware", () => {
         "x-middleware-request-custom-header": "value",
       }),
     });
-    const result = await handleMiddleware(event, middlewareLoader);
+    const result = await handleMiddleware(event, "", middlewareLoader);
 
     expect(middlewareLoader).toHaveBeenCalled();
     expect(result).toEqual({
@@ -268,7 +268,7 @@ describe("handleMiddleware", () => {
       headers: new Headers(),
       body,
     });
-    const result = await handleMiddleware(event, middlewareLoader);
+    const result = await handleMiddleware(event, "", middlewareLoader);
 
     expect(middlewareLoader).toHaveBeenCalled();
     expect(result).toEqual({
@@ -291,7 +291,7 @@ describe("handleMiddleware", () => {
       }),
       body,
     });
-    const result = await handleMiddleware(event, middlewareLoader);
+    const result = await handleMiddleware(event, "", middlewareLoader);
 
     expect(middlewareLoader).toHaveBeenCalled();
     expect(result).toEqual({
@@ -312,7 +312,7 @@ describe("handleMiddleware", () => {
         host: "test.me",
       },
     });
-    await handleMiddleware(event, middlewareLoader);
+    await handleMiddleware(event, "", middlewareLoader);
     expect(middleware).toHaveBeenCalledWith(
       expect.objectContaining({
         url: "http://test.me/path",
@@ -327,7 +327,7 @@ describe("handleMiddleware", () => {
         host: "test.me/path",
       },
     });
-    await handleMiddleware(event, middlewareLoader);
+    await handleMiddleware(event, "", middlewareLoader);
     expect(middleware).toHaveBeenCalledWith(
       expect.objectContaining({
         url: "https://test.me/path",
@@ -342,11 +342,30 @@ describe("handleMiddleware", () => {
         host: "test.me",
       },
     });
-    await handleMiddleware(event, middlewareLoader);
+    await handleMiddleware(event, "", middlewareLoader);
     expect(middleware).toHaveBeenCalledWith(
       expect.objectContaining({
         url: "https://test.me/path",
       }),
     );
   });
+
+  it("should use the initial search query", async () => {
+    const event = createEvent({
+      url: "https://test.me/path?something=General%2520Banner",
+      headers: {
+        host: "test.me",
+      },
+    });
+    await handleMiddleware(
+      event,
+      "?something=General%2520Banner",
+      middlewareLoader,
+    );
+    expect(middleware).toHaveBeenCalledWith(
+      expect.objectContaining({
+        url: "https://test.me/path?something=General%2520Banner",
+      }),
+    );
+  });
 });