diff --git a/.changeset/tidy-bats-prove.md b/.changeset/tidy-bats-prove.md new file mode 100644 index 00000000000..8c7f5d51eda --- /dev/null +++ b/.changeset/tidy-bats-prove.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Fix `theme dev` preview occasionally rendering the live theme by preserving the Shopify `_shopify_essential` cookie in redirects diff --git a/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts b/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts index 4329aa35ccc..efacdc852ea 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts @@ -250,6 +250,67 @@ describe('dev proxy', () => { expect(ctx.session.sessionCookies).toHaveProperty('_shopify_essential', ':AZFbAlZ..yAAH:') }) + test('captures _shopify_essential from Set-Cookie into session on 3xx responses', async () => { + const localCtx = { + ...ctx, + session: {storeFqdn: 'my-store.myshopify.com', sessionCookies: {}}, + } as unknown as DevServerContext + + const redirectResponse = new Response('should-not-be-read', { + status: 302, + headers: { + Location: 'https://my-store.myshopify.com/foo?bar=1', + 'Set-Cookie': + '_shopify_essential=ABC123; Domain=my-store.myshopify.com; Path=/; Max-Age=31536000; secure; HttpOnly; SameSite=Lax', + }, + }) + + const patchedResponse = await patchRenderingResponse(localCtx, redirectResponse) + + expect(patchedResponse.status).toBe(302) + expect(localCtx.session.sessionCookies).toHaveProperty('_shopify_essential', 'ABC123') + }) + + test('rewrites Location header from store domain to local path on 3xx responses', async () => { + const localCtx = { + ...ctx, + session: {storeFqdn: 'my-store.myshopify.com', sessionCookies: {}}, + } as unknown as DevServerContext + + const redirectResponse = new Response('should-not-be-read', { + status: 302, + headers: { + Location: 'https://my-store.myshopify.com/foo?bar=1', + }, + }) + + const patchedResponse = await patchRenderingResponse(localCtx, redirectResponse) + + expect(patchedResponse.status).toBe(302) + expect(patchedResponse.headers.get('Location')).toBe('/foo?bar=1') + }) + + test('returns 3xx responses without reading or patching the body', async () => { + const localCtx = { + ...ctx, + session: {storeFqdn: 'my-store.myshopify.com', sessionCookies: {}}, + } as unknown as DevServerContext + + const body = 'link' + const redirectResponse = new Response(body, { + status: 301, + headers: { + Location: 'https://my-store.myshopify.com/foo', + }, + }) + + const patchedResponse = await patchRenderingResponse(localCtx, redirectResponse) + + expect(patchedResponse.status).toBe(301) + // CDN injection would rewrite the href; body should be passed through unchanged. + await expect(patchedResponse.text()).resolves.toBe(body) + }) + test('handles 304 Not Modified responses without crashing', async () => { // Create 304 response with no body as per HTTP spec const notModifiedResponse = new Response(null, { diff --git a/packages/theme/src/cli/utilities/theme-environment/proxy.ts b/packages/theme/src/cli/utilities/theme-environment/proxy.ts index 6f4b765da5f..2dfa7f95527 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.ts +++ b/packages/theme/src/cli/utilities/theme-environment/proxy.ts @@ -194,13 +194,13 @@ export async function patchRenderingResponse( rawResponse: Response, patchCallback?: (html: string) => string | undefined, ): Promise { + const response = patchProxiedResponseHeaders(ctx, rawResponse) + // 3xx responses should be returned - if (rawResponse.status >= 300 && rawResponse.status < 400) { - return rawResponse + if (response.status >= 300 && response.status < 400) { + return response } - const response = patchProxiedResponseHeaders(ctx, rawResponse) - // Only set HTML content-type for actual HTML responses, preserve JSON content-type: const originalContentType = rawResponse.headers.get('content-type') const isJsonResponse = originalContentType?.includes('application/json') diff --git a/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts b/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts index 87e32234bea..3b75a96391c 100644 --- a/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts @@ -205,6 +205,58 @@ describe('render', () => { ) }) + test('does not auto-follow 3xx responses on GET (preserves intermediate Set-Cookie)', async () => { + // Given + vi.mocked(fetch).mockResolvedValue( + new Response('redirected-body-should-not-be-returned', { + status: 302, + headers: {Location: 'https://store.myshopify.com/products/1'}, + }), + ) + + // When + const response = await render(session, context) + + // Then + expect(response.status).toEqual(302) + expect(response.headers.get('Location')).toEqual('https://store.myshopify.com/products/1') + expect(fetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + method: 'GET', + redirect: 'manual', + }), + ) + }) + + test('does not auto-follow 3xx responses on POST (replaceTemplates branch)', async () => { + // Given + vi.mocked(fetch).mockResolvedValue( + new Response('redirected-body-should-not-be-returned', { + status: 302, + headers: {Location: 'https://store.myshopify.com/products/1'}, + }), + ) + + // When + const response = await render(session, { + ...context, + method: 'POST', + replaceTemplates: {'sections/header.liquid': '
hello
'}, + }) + + // Then + expect(response.status).toEqual(302) + expect(response.headers.get('Location')).toEqual('https://store.myshopify.com/products/1') + expect(fetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + method: 'POST', + redirect: 'manual', + }), + ) + }) + test('renders using query parameters', async () => { // Given vi.mocked(fetch).mockResolvedValue(new Response()) diff --git a/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.ts b/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.ts index 4a61e423558..efd4d654402 100644 --- a/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.ts +++ b/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.ts @@ -23,6 +23,7 @@ export async function render(session: DevServerSession, context: DevServerRender response = await fetch(url, { method: 'POST', body: bodyParams, + redirect: 'manual', headers: { ...headers, ...defaultHeaders(), @@ -36,6 +37,7 @@ export async function render(session: DevServerSession, context: DevServerRender // eslint-disable-next-line no-restricted-globals response = await fetch(url, { method: context.method, + redirect: 'manual', headers: { ...headers, ...defaultHeaders(),