From 333dd7af8ff202ae84af228b41c25101f82ecc3b Mon Sep 17 00:00:00 2001 From: Bret Little Date: Fri, 27 May 2022 10:30:04 -0400 Subject: [PATCH] Prevent encoded props from double decoding (#1360) * Prevent encoded props from double decoding in the RSC flight inlined in the initial page load * Removed unused htmlDecode --- .changeset/popular-rivers-judge.md | 5 +++++ .../hydrogen/src/framework/Hydration/rsc.ts | 4 ++-- .../hydrogen/src/utilities/html-encoding.ts | 9 --------- packages/hydrogen/src/utilities/index.ts | 2 +- .../src/utilities/tests/html-encoding.test.ts | 17 +---------------- .../src/components/Passthrough.client.jsx | 9 +++++++-- .../src/routes/escaping.server.jsx | 2 ++ .../server-components/tests/e2e-test-cases.ts | 1 + 8 files changed, 19 insertions(+), 30 deletions(-) create mode 100644 .changeset/popular-rivers-judge.md diff --git a/.changeset/popular-rivers-judge.md b/.changeset/popular-rivers-judge.md new file mode 100644 index 0000000000..84fe8e5296 --- /dev/null +++ b/.changeset/popular-rivers-judge.md @@ -0,0 +1,5 @@ +--- +'@shopify/hydrogen': patch +--- + +Fix a problem where encoded html content props passed from server to client components would get double decoded, and break hydration on app load. diff --git a/packages/hydrogen/src/framework/Hydration/rsc.ts b/packages/hydrogen/src/framework/Hydration/rsc.ts index 683c89d03e..67d5067bf3 100644 --- a/packages/hydrogen/src/framework/Hydration/rsc.ts +++ b/packages/hydrogen/src/framework/Hydration/rsc.ts @@ -7,7 +7,6 @@ import { // @ts-ignore } from '@shopify/hydrogen/vendor/react-server-dom-vite'; import {RSC_PATHNAME} from '../../constants'; -import {htmlDecode} from '../../utilities'; let rscReader: ReadableStream | null; @@ -16,9 +15,10 @@ const flightChunks: string[] = []; const FLIGHT_ATTRIBUTE = 'data-flight'; function addElementToFlightChunks(el: Element) { + // We don't need to decode, because `.getAttribute` already decodes const chunk = el.getAttribute(FLIGHT_ATTRIBUTE); if (chunk) { - flightChunks.push(htmlDecode(chunk)); + flightChunks.push(chunk); } } diff --git a/packages/hydrogen/src/utilities/html-encoding.ts b/packages/hydrogen/src/utilities/html-encoding.ts index 373010895c..e406e8e44c 100644 --- a/packages/hydrogen/src/utilities/html-encoding.ts +++ b/packages/hydrogen/src/utilities/html-encoding.ts @@ -6,12 +6,3 @@ export function htmlEncode(html: string) { .replace(/"/g, '"') .replace(/'/g, '''); } - -export function htmlDecode(text: string) { - return text - .replace(/</g, '<') - .replace(/>/g, '>') - .replace(/"/g, '"') - .replace(/'/g, "'") - .replace(/&/g, '&'); -} diff --git a/packages/hydrogen/src/utilities/index.ts b/packages/hydrogen/src/utilities/index.ts index 31c1ac58c3..896d74d245 100644 --- a/packages/hydrogen/src/utilities/index.ts +++ b/packages/hydrogen/src/utilities/index.ts @@ -18,4 +18,4 @@ export {getMeasurementAsParts, getMeasurementAsString} from './measurement'; export {parseMetafieldValue} from './parseMetafieldValue'; export {fetchBuilder, graphqlRequestBody, decodeShopifyId} from './fetch'; export {getTime} from './timing'; -export {htmlEncode, htmlDecode} from './html-encoding'; +export {htmlEncode} from './html-encoding'; diff --git a/packages/hydrogen/src/utilities/tests/html-encoding.test.ts b/packages/hydrogen/src/utilities/tests/html-encoding.test.ts index 6c5a2ddc71..2729ad7db7 100644 --- a/packages/hydrogen/src/utilities/tests/html-encoding.test.ts +++ b/packages/hydrogen/src/utilities/tests/html-encoding.test.ts @@ -1,4 +1,4 @@ -import {htmlDecode, htmlEncode} from '../html-encoding'; +import {htmlEncode} from '../html-encoding'; describe('html encoding', () => { describe('htmlEncode', () => { @@ -12,19 +12,4 @@ M2:{"id":"ShopifyProvider-MTY2NjM5NzU1NQ","name":& expect(htmlEncode(`how's it going?`)).toBe(`how's it going?`); }); }); - - describe('htmlDecode', () => { - it('decodes things properly', () => { - expect(htmlDecode('<div>')).toBe('
'); - expect( - htmlDecode(`S1:"react.suspense" -M2:{"id":"ShopifyProvider-MTY2NjM5NzU1NQ","name":"ShopifyProviderClient"}`) - ).toBe(`S1:"react.suspense" -M2:{"id":"ShopifyProvider-MTY2NjM5NzU1NQ","name":"ShopifyProviderClient"}`); - }); - - it('does not double-decode ampersands', () => { - expect(htmlDecode(`drink: g&gt;`)).toBe(`drink: g>`); - }); - }); }); diff --git a/packages/playground/server-components/src/components/Passthrough.client.jsx b/packages/playground/server-components/src/components/Passthrough.client.jsx index 3e8294d367..2de610667b 100644 --- a/packages/playground/server-components/src/components/Passthrough.client.jsx +++ b/packages/playground/server-components/src/components/Passthrough.client.jsx @@ -1,3 +1,8 @@ -export default function Passthrough({children}) { - return children; +export default function Passthrough({children, prop2}) { + return ( + <> +
{children}
+
+ + ); } diff --git a/packages/playground/server-components/src/routes/escaping.server.jsx b/packages/playground/server-components/src/routes/escaping.server.jsx index 349100f126..ef0047efd7 100644 --- a/packages/playground/server-components/src/routes/escaping.server.jsx +++ b/packages/playground/server-components/src/routes/escaping.server.jsx @@ -4,6 +4,8 @@ export default function Escaping() { return ( diff --git a/packages/playground/server-components/tests/e2e-test-cases.ts b/packages/playground/server-components/tests/e2e-test-cases.ts index 02c90e3a45..6494651709 100644 --- a/packages/playground/server-components/tests/e2e-test-cases.ts +++ b/packages/playground/server-components/tests/e2e-test-cases.ts @@ -221,6 +221,7 @@ export default async function testCases({ expect(await page.textContent('body')).toContain( "" ); + expect(await page.textContent('body')).toContain(`"fiddle"`); }); it('adds style tags for CSS modules', async () => {