fix(core): handle external redirect destinations with query parameters correctly (#1744)

Author: dphang

packages/libs/core/src/match.ts

@@ -22,18 +22,18 @@ export function matchPath(path: string, source: string): Match {
  */
 export function compileDestination(
   destination: string,
-  params: object
+  params: any
 ): string | null {
   try {
     const destinationLowerCase = destination.toLowerCase();
     if (
       destinationLowerCase.startsWith("https://") ||
       destinationLowerCase.startsWith("http://")
     ) {
-      // Handle external URLs
-      const { origin, pathname } = new URL(destination);
+      // Handle external URL redirects
+      const { origin, pathname, search } = new URL(destination);
       const toPath = compile(pathname, { encode: encodeURIComponent });
-      const compiledDestination = `${origin}${toPath(params)}`;
+      const compiledDestination = `${origin}${toPath(params)}${search}`;
 
       // Remove trailing slash if original destination didn't have it
       if (!destination.endsWith("/") && compiledDestination.endsWith("/")) {

packages/libs/core/src/route/redirect.ts

@@ -147,8 +147,8 @@ export async function getLanguageRedirectPath(
 
 /**
  * Get the redirect of the given path, if it exists.
- * @param path
- * @param manifest
+ * @param request
+ * @param routesManifest
  */
 export function getRedirectPath(
   request: Request,

packages/libs/core/tests/route/redirect.test.ts

@@ -36,6 +36,11 @@ describe("Redirector Tests", () => {
             destination: "https://example.com",
             statusCode: 308
           },
+          {
+            source: "/external-2",
+            destination: "https://example.com/?a=b",
+            statusCode: 308
+          },
           {
             source: "/invalid-destination",
             destination: "ftp://example.com",
@@ -46,16 +51,17 @@ describe("Redirector Tests", () => {
     });
 
     it.each`
-      path                      | expectedRedirect         | expectedStatusCode
-      ${"/a"}                   | ${"/b"}                  | ${308}
-      ${"/c"}                   | ${"/d"}                  | ${302}
-      ${"/old-blog/abc"}        | ${"/news/abc"}           | ${308}
-      ${"/old-users/1234"}      | ${"/users/1234"}         | ${307}
-      ${"/old-users/abc"}       | ${null}                  | ${null}
-      ${"/external"}            | ${"https://example.com"} | ${308}
-      ${"/invalid-destination"} | ${null}                  | ${null}
-      ${"/en/a"}                | ${"/en/b"}               | ${308}
-      ${"/fr/a"}                | ${"/fr/b"}               | ${308}
+      path                      | expectedRedirect              | expectedStatusCode
+      ${"/a"}                   | ${"/b"}                       | ${308}
+      ${"/c"}                   | ${"/d"}                       | ${302}
+      ${"/old-blog/abc"}        | ${"/news/abc"}                | ${308}
+      ${"/old-users/1234"}      | ${"/users/1234"}              | ${307}
+      ${"/old-users/abc"}       | ${null}                       | ${null}
+      ${"/external"}            | ${"https://example.com"}      | ${308}
+      ${"/external-2"}          | ${"https://example.com/?a=b"} | ${308}
+      ${"/invalid-destination"} | ${null}                       | ${null}
+      ${"/en/a"}                | ${"/en/b"}                    | ${308}
+      ${"/fr/a"}                | ${"/fr/b"}                    | ${308}
     `(
       "redirects path $path to $expectedRedirect",
       ({ path, expectedRedirect, expectedStatusCode }) => {