Fix external redirect trailing (#598)

* Fix External redirects issues without trailing slash #591

* fix some incorrect unit test redirect

* Create fuzzy-camels-check.md

Author: conico974

.changeset/fuzzy-camels-check.md

@@ -0,0 +1,5 @@
+---
+"@opennextjs/aws": patch
+---
+
+Fix external redirect trailing

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

@@ -229,7 +229,7 @@ export function handleRewrites<T extends RewriteDefinition>(
     }
     rewrittenUrl = isExternalRewrite
       ? `${protocol}//${rewrittenHost}${rewrittenPath}`
-      : `/${rewrittenPath}`;
+      : `${rewrittenPath}`;
     // Should we merge the query params or use only the ones from the rewrite?
     finalQuery = {
       ...query,

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

@@ -47,7 +47,7 @@ export function getUrlParts(url: string, isExternal: boolean) {
     const match = url.match(regex);
     return {
       hostname: "",
-      pathname: match?.[1] ?? url,
+      pathname: match?.[1] ? `/${match[1]}` : url,
       protocol: "",
       queryString: match?.[2] ?? "",
     };
@@ -61,7 +61,7 @@ export function getUrlParts(url: string, isExternal: boolean) {
   return {
     protocol: match[1] ?? "https:",
     hostname: match[2],
-    pathname: match[3],
+    pathname: match[3] ?? "",
     queryString: match[4]?.slice(1) ?? "",
   };
 }

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

@@ -240,13 +240,31 @@ describe("handleRedirects", () => {
       {
         source: "/:path+",
         destination: "/new/:path+",
-        internal: true,
+        locale: false,
         statusCode: 308,
-        regex: "^(?:/((?:[^/]+?)(?:/(?:[^/]+?))*))/$",
+        regex: "^(?!/_next)(?:/((?:[^/]+?)(?:/(?:[^/]+?))*))(?:/)?$",
       },
     ]);
 
-    expect(result).toBeUndefined();
+    expect(result.headers.Location).toBe("/new/api-route");
+  });
+
+  it("should redirect matching nested path", () => {
+    const event = createEvent({
+      url: "/api-route/secret",
+    });
+
+    const result = handleRedirects(event, [
+      {
+        source: "/:path+",
+        destination: "/new/:path+",
+        locale: false,
+        statusCode: 308,
+        regex: "^(?!/_next)(?:/((?:[^/]+?)(?:/(?:[^/]+?))*))(?:/)?$",
+      },
+    ]);
+
+    expect(result.headers.Location).toBe("/new/api-route/secret");
   });
 
   it("should not redirect unmatched path", () => {
@@ -258,9 +276,9 @@ describe("handleRedirects", () => {
       {
         source: "/foo/",
         destination: "/bar",
-        internal: true,
-        statusCode: 308,
-        regex: "^(?:/((?:[^/]+?)(?:/(?:[^/]+?))*))/$",
+        locale: false,
+        statusCode: 307,
+        regex: "^(?!/_next)/foo/(?:/)?$",
       },
     ]);
 

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

@@ -119,7 +119,7 @@ describe("getUrlParts", () => {
     it("returns url parts for /", () => {
       expect(getUrlParts("/", false)).toEqual({
         hostname: "",
-        pathname: "", // TODO: This behaviour is inconsistent with external pathname
+        pathname: "/",
         protocol: "",
         queryString: "",
       });
@@ -128,16 +128,16 @@ describe("getUrlParts", () => {
     it("returns url parts", () => {
       expect(getUrlParts("/relative", false)).toEqual({
         hostname: "",
-        pathname: "relative",
-        protocol: "", // TODO: This behaviour is inconsistent with external pathname
+        pathname: "/relative",
+        protocol: "",
         queryString: "",
       });
     });
 
     it("returns url parts with query string", () => {
       expect(getUrlParts("/relative/path?query=1", false)).toEqual({
         hostname: "",
-        pathname: "relative/path", // TODO: This behaviour is inconsistent with external pathname
+        pathname: "/relative/path",
         protocol: "",
         queryString: "query=1",
       });
@@ -162,6 +162,16 @@ describe("getUrlParts", () => {
       });
     });
 
+    // For reference https://github.com/opennextjs/opennextjs-aws/issues/591
+    it("returns url parts for / without trailing slash", () => {
+      expect(getUrlParts("http://localhost", true)).toEqual({
+        hostname: "localhost",
+        pathname: "",
+        protocol: "http:",
+        queryString: "",
+      });
+    });
+
     it("returns url parts", () => {
       expect(getUrlParts("https://localhost/relative", true)).toEqual({
         hostname: "localhost",