Fix adapter request creation when double slashes exist (#5336)

remix-run · Feb 1, 2023 · 611c68b · 611c68b
1 parent 518009d
commit 611c68b
Show file tree

Hide file tree

Showing 9 changed files with 173 additions and 6 deletions.
diff --git a/.changeset/nine-apricots-breathe.md b/.changeset/nine-apricots-breathe.md
@@ -0,0 +1,8 @@
+---
+"@remix-run/architect": patch
+"@remix-run/express": patch
+"@remix-run/netlify": patch
+"@remix-run/vercel": patch
+---
+
+Fix Fetch Request creation for incoming URLs with double slashes
diff --git a/packages/remix-architect/__tests__/server-test.ts b/packages/remix-architect/__tests__/server-test.ts
@@ -101,6 +101,38 @@ describe("architect createRequestHandler", () => {
         });
     });
 
+    it("handles root // requests", async () => {
+      mockedCreateRequestHandler.mockImplementation(() => async (req) => {
+        return new Response(`URL: ${new URL(req.url).pathname}`);
+      });
+
+      // We don't have a real app to test, but it doesn't matter. We won't ever
+      // call through to the real createRequestHandler
+      // @ts-expect-error
+      await lambdaTester(createRequestHandler({ build: undefined }))
+        .event(createMockEvent({ rawPath: "//" }))
+        .expectResolve((res) => {
+          expect(res.statusCode).toBe(200);
+          expect(res.body).toBe("URL: //");
+        });
+    });
+
+    it("handles nested // requests", async () => {
+      mockedCreateRequestHandler.mockImplementation(() => async (req) => {
+        return new Response(`URL: ${new URL(req.url).pathname}`);
+      });
+
+      // We don't have a real app to test, but it doesn't matter. We won't ever
+      // call through to the real createRequestHandler
+      // @ts-expect-error
+      await lambdaTester(createRequestHandler({ build: undefined }))
+        .event(createMockEvent({ rawPath: "//foo//bar" }))
+        .expectResolve((res) => {
+          expect(res.statusCode).toBe(200);
+          expect(res.body).toBe("URL: //foo//bar");
+        });
+    });
+
     it("handles null body", async () => {
       mockedCreateRequestHandler.mockImplementation(() => async () => {
         return new Response(null, { status: 200 });

diff --git a/packages/remix-architect/server.ts b/packages/remix-architect/server.ts
@@ -62,7 +62,7 @@ export function createRemixRequest(event: APIGatewayProxyEventV2): NodeRequest {
   let host = event.headers["x-forwarded-host"] || event.headers.host;
   let search = event.rawQueryString.length ? `?${event.rawQueryString}` : "";
   let scheme = process.env.ARC_SANDBOX ? "http" : "https";
-  let url = new URL(event.rawPath + search, `${scheme}://${host}`);
+  let url = new URL(`${scheme}://${host}${event.rawPath}${search}`);
   let isFormData = event.headers["content-type"]?.includes(
     "multipart/form-data"
   );

diff --git a/packages/remix-express/__tests__/server-test.ts b/packages/remix-express/__tests__/server-test.ts
@@ -64,6 +64,30 @@ describe("express createRequestHandler", () => {
       expect(res.headers["x-powered-by"]).toBe("Express");
     });
 
+    it("handles root // URLs", async () => {
+      mockedCreateRequestHandler.mockImplementation(() => async (req) => {
+        return new Response("URL: " + new URL(req.url).pathname);
+      });
+
+      let request = supertest(createApp());
+      let res = await request.get("//");
+
+      expect(res.status).toBe(200);
+      expect(res.text).toBe("URL: //");
+    });
+
+    it("handles nested // URLs", async () => {
+      mockedCreateRequestHandler.mockImplementation(() => async (req) => {
+        return new Response("URL: " + new URL(req.url).pathname);
+      });
+
+      let request = supertest(createApp());
+      let res = await request.get("//foo//bar");
+
+      expect(res.status).toBe(200);
+      expect(res.text).toBe("URL: //foo//bar");
+    });
+
     it("handles null body", async () => {
       mockedCreateRequestHandler.mockImplementation(() => async () => {
         return new Response(null, { status: 200 });

diff --git a/packages/remix-express/server.ts b/packages/remix-express/server.ts
@@ -93,8 +93,7 @@ export function createRemixRequest(
   req: express.Request,
   res: express.Response
 ): NodeRequest {
-  let origin = `${req.protocol}://${req.get("host")}`;
-  let url = new URL(req.url, origin);
+  let url = new URL(`${req.protocol}://${req.get("host")}${req.url}`);
 
   // Abort action/loaders once we can no longer write a response
   let controller = new NodeAbortController();

diff --git a/packages/remix-netlify/__tests__/server-test.ts b/packages/remix-netlify/__tests__/server-test.ts
@@ -38,7 +38,9 @@ function createMockEvent(event: Partial<HandlerEvent> = {}): HandlerEvent {
     rawQuery: "",
     path: "/",
     httpMethod: "GET",
-    headers: {},
+    headers: {
+      host: "localhost:3000",
+    },
     multiValueHeaders: {},
     queryStringParameters: null,
     multiValueQueryStringParameters: null,
@@ -74,6 +76,80 @@ describe("netlify createRequestHandler", () => {
         });
     });
 
+    it("handles root // requests", async () => {
+      mockedCreateRequestHandler.mockImplementation(() => async (req) => {
+        return new Response(`URL: ${new URL(req.url).pathname}`);
+      });
+
+      // We don't have a real app to test, but it doesn't matter. We won't ever
+      // call through to the real createRequestHandler
+      // @ts-expect-error
+      await lambdaTester(createRequestHandler({ build: undefined }))
+        .event(createMockEvent({ rawUrl: "http://localhost:3000//" }))
+        .expectResolve((res) => {
+          expect(res.statusCode).toBe(200);
+          expect(res.body).toBe("URL: //");
+        });
+    });
+
+    it("handles nested // requests", async () => {
+      mockedCreateRequestHandler.mockImplementation(() => async (req) => {
+        return new Response(`URL: ${new URL(req.url).pathname}`);
+      });
+
+      // We don't have a real app to test, but it doesn't matter. We won't ever
+      // call through to the real createRequestHandler
+      // @ts-expect-error
+      await lambdaTester(createRequestHandler({ build: undefined }))
+        .event(createMockEvent({ rawUrl: "http://localhost:3000//foo//bar" }))
+        .expectResolve((res) => {
+          expect(res.statusCode).toBe(200);
+          expect(res.body).toBe("URL: //foo//bar");
+        });
+    });
+
+    it("handles root // requests (development)", async () => {
+      let oldEnv = process.env.NODE_ENV;
+      process.env.NODE_ENV = "development";
+
+      mockedCreateRequestHandler.mockImplementation(() => async (req) => {
+        return new Response(`URL: ${new URL(req.url).pathname}`);
+      });
+
+      // We don't have a real app to test, but it doesn't matter. We won't ever
+      // call through to the real createRequestHandler
+      // @ts-expect-error
+      await lambdaTester(createRequestHandler({ build: undefined }))
+        .event(createMockEvent({ path: "//" }))
+        .expectResolve((res) => {
+          expect(res.statusCode).toBe(200);
+          expect(res.body).toBe("URL: //");
+        });
+
+      process.env.NODE_ENV = oldEnv;
+    });
+
+    it("handles nested // requests (development)", async () => {
+      let oldEnv = process.env.NODE_ENV;
+      process.env.NODE_ENV = "development";
+
+      mockedCreateRequestHandler.mockImplementation(() => async (req) => {
+        return new Response(`URL: ${new URL(req.url).pathname}`);
+      });
+
+      // We don't have a real app to test, but it doesn't matter. We won't ever
+      // call through to the real createRequestHandler
+      // @ts-expect-error
+      await lambdaTester(createRequestHandler({ build: undefined }))
+        .event(createMockEvent({ path: "//foo//bar" }))
+        .expectResolve((res) => {
+          expect(res.statusCode).toBe(200);
+          expect(res.body).toBe("URL: //foo//bar");
+        });
+
+      process.env.NODE_ENV = oldEnv;
+    });
+
     it("handles null body", async () => {
       mockedCreateRequestHandler.mockImplementation(() => async () => {
         return new Response(null, { status: 200 });

diff --git a/packages/remix-netlify/server.ts b/packages/remix-netlify/server.ts
@@ -63,7 +63,7 @@ export function createRemixRequest(event: HandlerEvent): NodeRequest {
   } else {
     let origin = event.headers.host;
     let rawPath = getRawPath(event);
-    url = new URL(rawPath, `http://${origin}`);
+    url = new URL(`http://${origin}${rawPath}`);
   }
 
   // Note: No current way to abort these for Netlify, but our router expects

diff --git a/packages/remix-vercel/__tests__/server-test.ts b/packages/remix-vercel/__tests__/server-test.ts
@@ -68,6 +68,34 @@ describe("vercel createRequestHandler", () => {
       expect(res.text).toBe("URL: /foo/bar");
     });
 
+    it("handles root // requests", async () => {
+      mockedCreateRequestHandler.mockImplementation(() => async (req) => {
+        return new Response(`URL: ${new URL(req.url).pathname}`);
+      });
+
+      let request = supertest(createApp());
+      // note: vercel's createServerWithHelpers requires a x-now-bridge-request-id
+      let res = await request.get("//").set({ "x-now-bridge-request-id": "2" });
+
+      expect(res.status).toBe(200);
+      expect(res.text).toBe("URL: //");
+    });
+
+    it("handles nested // requests", async () => {
+      mockedCreateRequestHandler.mockImplementation(() => async (req) => {
+        return new Response(`URL: ${new URL(req.url).pathname}`);
+      });
+
+      let request = supertest(createApp());
+      // note: vercel's createServerWithHelpers requires a x-now-bridge-request-id
+      let res = await request
+        .get("//foo//bar")
+        .set({ "x-now-bridge-request-id": "2" });
+
+      expect(res.status).toBe(200);
+      expect(res.text).toBe("URL: //foo//bar");
+    });
+
     it("handles null body", async () => {
       mockedCreateRequestHandler.mockImplementation(() => async () => {
         return new Response(null, { status: 200 });

diff --git a/packages/remix-vercel/server.ts b/packages/remix-vercel/server.ts
@@ -82,7 +82,7 @@ export function createRemixRequest(
   let host = req.headers["x-forwarded-host"] || req.headers["host"];
   // doesn't seem to be available on their req object!
   let protocol = req.headers["x-forwarded-proto"] || "https";
-  let url = new URL(req.url!, `${protocol}://${host}`);
+  let url = new URL(`${protocol}://${host}${req.url}`);
 
   // Abort action/loaders once we can no longer write a response
   let controller = new NodeAbortController();