Fix redirect bug (#1629)

Shopify · Jan 12, 2024 · 810f48c · 810f48c
1 parent 357becf
commit 810f48c
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 3 deletions.
diff --git a/.changeset/blue-beds-destroy.md b/.changeset/blue-beds-destroy.md
@@ -0,0 +1,5 @@
+---
+'@shopify/hydrogen': patch
+---
+
+Fix redirect bug
diff --git a/packages/hydrogen/src/routing/redirect.test.ts b/packages/hydrogen/src/routing/redirect.test.ts
@@ -166,6 +166,16 @@ describe('storefrontRedirect', () => {
           headers: {location: '/some-page?param=https://another.com'},
         }),
       );
+
+      await expect(
+        storefrontRedirect({
+          response,
+          storefront: storefrontMock,
+          request: new Request(
+            'http://localhost:3000/missing?return_to=%01http%3A%2F%2Fexample.com',
+          ),
+        }),
+      ).resolves.toEqual(response);
     });
   });
 });
diff --git a/packages/hydrogen/src/routing/redirect.ts b/packages/hydrogen/src/routing/redirect.ts
@@ -56,7 +56,7 @@ export async function storefrontRedirect(
       searchParams.get('return_to') || searchParams.get('redirect');
 
     if (redirectTo) {
-      if (isLocalPath(redirectTo)) {
+      if (isLocalPath(request.url, redirectTo)) {
         return redirect(redirectTo);
       } else {
         console.warn(
@@ -74,12 +74,18 @@ export async function storefrontRedirect(
   return response;
 }
 
-function isLocalPath(url: string) {
+function isLocalPath(requestUrl: string, redirectUrl: string) {
   // We don't want to redirect cross domain,
   // doing so could create phishing vulnerability
   // Test for protocols, e.g. https://, http://, //
   // and uris: mailto:, tel:, javascript:, etc.
-  return !/^(([a-z+-]+:)?\/\/|[a-z+-]+:)/i.test(url.trim());
+  try {
+    return (
+      new URL(requestUrl).origin === new URL(redirectUrl, requestUrl).origin
+    );
+  } catch (e) {
+    return false;
+  }
 }
 
 const REDIRECT_QUERY = `#graphql