Fix middleware fetch (#823)

* fix middleware for directly returned fetch

* return middleware response even for empty request

* add e2e test

* changeset

* lint fix

* fix unit test

* Apply suggestions from code review

Co-authored-by: Victor Berchet <victor@suumit.com>

---------

Co-authored-by: Victor Berchet <victor@suumit.com>

Author: conico974

.changeset/mean-icons-tap.md

@@ -0,0 +1,6 @@
+---
+"@opennextjs/aws": patch
+---
+
+fix issue when returning fetch from the middleware
+Also fix an issue that prevented retunning response with an empty body in the middleware

examples/app-pages-router/middleware.ts

@@ -43,6 +43,19 @@ export function middleware(request: NextRequest) {
       },
     });
   }
+
+  if (path === "/head" && request.method === "HEAD") {
+    return new NextResponse(null, {
+      headers: {
+        "x-from-middleware": "true",
+      },
+    });
+  }
+
+  if (path === "/fetch") {
+    // This one test both that we don't modify immutable headers
+    return fetch(new URL("/api/hello", request.url));
+  }
   const rHeaders = new Headers(request.headers);
   const r = NextResponse.next({
     request: {

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

@@ -111,20 +111,23 @@ export async function handleMiddleware(
   const reqHeaders: Record<string, string> = {};
   const resHeaders: Record<string, string | string[]> = {};
 
-  responseHeaders.delete("x-middleware-override-headers");
-  /*  Next will set the header `x-middleware-set-cookie` when you `set-cookie` in the middleware.
-   *  We can delete it here since it will be set in `set-cookie` aswell. Next removes this header in the response themselves.
-   * `x-middleware-next` is set when you invoke `NextResponse.next()`. We can delete it here aswell.
-   */
-  responseHeaders.delete("x-middleware-set-cookie");
-  responseHeaders.delete("x-middleware-next");
+  // These are internal headers used by Next.js, we don't want to expose them to the client
+  const filteredHeaders = [
+    "x-middleware-override-headers",
+    "x-middleware-set-cookie",
+    "x-middleware-next",
+    "x-middleware-rewrite",
+    // We need to drop `content-encoding` because it will be decoded
+    "content-encoding",
+  ];
 
   const xMiddlewareKey = "x-middleware-request-";
   responseHeaders.forEach((value, key) => {
     if (key.startsWith(xMiddlewareKey)) {
       const k = key.substring(xMiddlewareKey.length);
       reqHeaders[k] = value;
     } else {
+      if (filteredHeaders.includes(key.toLowerCase())) return;
       if (key.toLowerCase() === "set-cookie") {
         resHeaders[key] = resHeaders[key]
           ? [...resHeaders[key], value]
@@ -135,21 +138,6 @@ export async function handleMiddleware(
     }
   });
 
-  // If the middleware returned a Redirect, we set the `Location` header with
-  // the redirected url and end the response.
-  if (statusCode >= 300 && statusCode < 400) {
-    resHeaders.location =
-      responseHeaders.get("location") ?? resHeaders.location;
-    // res.setHeader("Location", location);
-    return {
-      body: emptyReadableStream(),
-      type: internalEvent.type,
-      statusCode: statusCode,
-      headers: resHeaders,
-      isBase64Encoded: false,
-    } satisfies InternalResult;
-  }
-
   // If the middleware returned a Rewrite, set the `url` to the pathname of the rewrite
   // NOTE: the header was added to `req` from above
   const rewriteUrl = responseHeaders.get("x-middleware-rewrite");
@@ -177,11 +165,11 @@ export async function handleMiddleware(
     }
   }
 
-  // If the middleware returned a `NextResponse`, pipe the body to res. This will return
-  // the body immediately to the client.
-  if (result.body) {
+  // If the middleware wants to directly return a response (i.e. not using `NextResponse.next()` or `NextResponse.rewrite()`)
+  // we return the response directly
+  if (!rewriteUrl && !responseHeaders.get("x-middleware-next")) {
     // transfer response body to res
-    const body = result.body as ReadableStream<Uint8Array>;
+    const body = (result.body as ReadableStream) ?? emptyReadableStream();
 
     return {
       type: internalEvent.type,

packages/tests-e2e/tests/appPagesRouter/middleware.test.ts

@@ -0,0 +1,20 @@
+import { expect, test } from "@playwright/test";
+
+test("should return correctly on HEAD request with an empty body", async ({
+  request,
+}) => {
+  const response = await request.head("/head");
+  expect(response.status()).toBe(200);
+  const body = await response.text();
+  expect(body).toBe("");
+  expect(response.headers()["x-from-middleware"]).toBe("true");
+});
+
+test("should return correctly for directly returning a fetch response", async ({
+  request,
+}) => {
+  const response = await request.get("/fetch");
+  expect(response.status()).toBe(200);
+  const body = await response.json();
+  expect(body).toEqual({ hello: "world" });
+});

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

@@ -177,9 +177,7 @@ describe("handleMiddleware", () => {
       ...event,
       rawPath: "/rewrite",
       url: "http://localhost/rewrite",
-      responseHeaders: {
-        "x-middleware-rewrite": "http://localhost/rewrite",
-      },
+      responseHeaders: {},
       isExternalRewrite: false,
     });
   });
@@ -203,9 +201,7 @@ describe("handleMiddleware", () => {
       ...event,
       rawPath: "/rewrite",
       url: "http://localhost/rewrite?newKey=value",
-      responseHeaders: {
-        "x-middleware-rewrite": "http://localhost/rewrite?newKey=value",
-      },
+      responseHeaders: {},
       query: {
         __nextDataReq: "1",
         newKey: "value",
@@ -232,9 +228,7 @@ describe("handleMiddleware", () => {
       ...event,
       rawPath: "/rewrite",
       url: "http://external/rewrite",
-      responseHeaders: {
-        "x-middleware-rewrite": "http://external/rewrite",
-      },
+      responseHeaders: {},
       isExternalRewrite: true,
     });
   });
@@ -244,6 +238,7 @@ describe("handleMiddleware", () => {
     middleware.mockResolvedValue({
       headers: new Headers({
         "x-middleware-request-custom-header": "value",
+        "x-middleware-next": "1",
       }),
     });
     const result = await handleMiddleware(event, "", middlewareLoader);