diff --git a/.changeset/fuzzy-camels-check.md b/.changeset/fuzzy-camels-check.md new file mode 100644 index 000000000..287c1923c --- /dev/null +++ b/.changeset/fuzzy-camels-check.md @@ -0,0 +1,5 @@ +--- +"@opennextjs/aws": patch +--- + +Fix external redirect trailing diff --git a/packages/open-next/src/core/routing/matcher.ts b/packages/open-next/src/core/routing/matcher.ts index ad8989fed..2ec344aa0 100644 --- a/packages/open-next/src/core/routing/matcher.ts +++ b/packages/open-next/src/core/routing/matcher.ts @@ -229,7 +229,7 @@ export function handleRewrites( } rewrittenUrl = isExternalRewrite ? `${protocol}//${rewrittenHost}${rewrittenPath}` - : `/${rewrittenPath}`; + : `${rewrittenPath}`; // Should we merge the query params or use only the ones from the rewrite? finalQuery = { ...query, diff --git a/packages/open-next/src/core/routing/util.ts b/packages/open-next/src/core/routing/util.ts index a29b96efd..934372beb 100644 --- a/packages/open-next/src/core/routing/util.ts +++ b/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) ?? "", }; } diff --git a/packages/tests-unit/tests/core/routing/matcher.test.ts b/packages/tests-unit/tests/core/routing/matcher.test.ts index 360a6947f..55a74550f 100644 --- a/packages/tests-unit/tests/core/routing/matcher.test.ts +++ b/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/(?:/)?$", }, ]); diff --git a/packages/tests-unit/tests/core/routing/util.test.ts b/packages/tests-unit/tests/core/routing/util.test.ts index 3e9feb4c4..6f2d9d421 100644 --- a/packages/tests-unit/tests/core/routing/util.test.ts +++ b/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,8 +128,8 @@ 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: "", }); }); @@ -137,7 +137,7 @@ describe("getUrlParts", () => { 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",