diff --git a/.changeset/hip-rocks-hammer.md b/.changeset/hip-rocks-hammer.md new file mode 100644 index 0000000000..9bd4860efd --- /dev/null +++ b/.changeset/hip-rocks-hammer.md @@ -0,0 +1,5 @@ +--- +'@shopify/hydrogen': patch +--- + +Fix bug where `storefrontRedirect` would return an error on soft page navigations. Also change the redirect status code from 301 to 302. diff --git a/packages/hydrogen/src/routing/redirect.test.ts b/packages/hydrogen/src/routing/redirect.test.ts index 8cfbe52baa..42ffbd8d15 100644 --- a/packages/hydrogen/src/routing/redirect.test.ts +++ b/packages/hydrogen/src/routing/redirect.test.ts @@ -36,7 +36,7 @@ describe('storefrontRedirect', () => { }), ).resolves.toEqual( new Response(null, { - status: 301, + status: 302, headers: {location: shopifyDomain + '/some-page'}, }), ); @@ -46,6 +46,33 @@ describe('storefrontRedirect', () => { }); }); + it('works with soft navigations', async () => { + queryMock.mockResolvedValueOnce({ + urlRedirects: {edges: [{node: {target: shopifyDomain + '/some-page'}}]}, + }); + + await expect( + storefrontRedirect({ + storefront: storefrontMock, + request: new Request( + 'https://domain.com/some-page?_data=%2Fcollections%2Fbackcountry', + ), + }), + ).resolves.toEqual( + new Response(null, { + status: 200, + headers: { + 'X-Remix-Redirect': shopifyDomain + '/some-page', + 'X-Remix-Status': '302', + }, + }), + ); + + expect(queryMock).toHaveBeenCalledWith(expect.anything(), { + variables: {query: 'path:/some-page'}, + }); + }); + it('queries the SFAPI and returns 404 on mismatch', async () => { queryMock.mockResolvedValueOnce({ urlRedirects: {edges: []}, diff --git a/packages/hydrogen/src/routing/redirect.ts b/packages/hydrogen/src/routing/redirect.ts index fb6534f862..0fd502a45c 100644 --- a/packages/hydrogen/src/routing/redirect.ts +++ b/packages/hydrogen/src/routing/redirect.ts @@ -33,10 +33,21 @@ export async function storefrontRedirect( } = options; const {pathname, search} = new URL(request.url); - const redirectFrom = pathname + search; + const searchParams = new URLSearchParams(search); + + const isSoftNavigation = searchParams.has('_data'); + + searchParams.delete('_data'); + const filteredSearchParams = searchParams.toString(); + + const redirectFrom = + pathname + (filteredSearchParams ? '?' + isSoftNavigation : ''); if (pathname === '/admin' && !noAdminRedirect) { - return redirect(`${storefront.getShopifyDomain()}/admin`); + return createRedirectResponse( + `${storefront.getShopifyDomain()}/admin`, + isSoftNavigation, + ); } try { @@ -49,13 +60,13 @@ export async function storefrontRedirect( const location = urlRedirects?.edges?.[0]?.node?.target; if (location) { - return new Response(null, {status: 301, headers: {location}}); + return createRedirectResponse(location, isSoftNavigation); } const redirectTo = getRedirectUrl(request.url); if (redirectTo) { - return redirect(redirectTo); + return createRedirectResponse(redirectTo, isSoftNavigation); } } catch (error) { console.error( @@ -67,17 +78,17 @@ export async function storefrontRedirect( return response; } -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. - try { - return ( - new URL(requestUrl).origin === new URL(redirectUrl, requestUrl).origin - ); - } catch (e) { - return false; +function createRedirectResponse(location: string, isSoftNavigation: boolean) { + if (isSoftNavigation) { + return new Response(null, { + status: 200, + headers: {'X-Remix-Redirect': location, 'X-Remix-Status': '302'}, + }); + } else { + return new Response(null, { + status: 302, + headers: {location: location}, + }); } }