Skip to content

Commit

Permalink
Prevent encoded props from double decoding (#1360)
Browse files Browse the repository at this point in the history
* Prevent encoded props from double decoding in the RSC flight inlined in the initial page load

* Removed unused htmlDecode
  • Loading branch information
blittle authored and benjaminsehl committed May 30, 2022
1 parent 490e359 commit 333dd7a
Show file tree
Hide file tree
Showing 8 changed files with 19 additions and 30 deletions.
5 changes: 5 additions & 0 deletions .changeset/popular-rivers-judge.md
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 2 additions & 2 deletions packages/hydrogen/src/framework/Hydration/rsc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
}
}

Expand Down
9 changes: 0 additions & 9 deletions packages/hydrogen/src/utilities/html-encoding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,3 @@ export function htmlEncode(html: string) {
.replace(/"/g, '"')
.replace(/'/g, ''');
}

export function htmlDecode(text: string) {
return text
.replace(/&lt;/g, '<')
.replace(/&gt;/g, '>')
.replace(/&quot;/g, '"')
.replace(/&#39;/g, "'")
.replace(/&amp;/g, '&');
}
2 changes: 1 addition & 1 deletion packages/hydrogen/src/utilities/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
17 changes: 1 addition & 16 deletions packages/hydrogen/src/utilities/tests/html-encoding.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {htmlDecode, htmlEncode} from '../html-encoding';
import {htmlEncode} from '../html-encoding';

describe('html encoding', () => {
describe('htmlEncode', () => {
Expand All @@ -12,19 +12,4 @@ M2:{&quot;id&quot;:&quot;ShopifyProvider-MTY2NjM5NzU1NQ&quot;,&quot;name&quot;:&
expect(htmlEncode(`how's it going?`)).toBe(`how&#39;s it going?`);
});
});

describe('htmlDecode', () => {
it('decodes things properly', () => {
expect(htmlDecode('&lt;div&gt;')).toBe('<div>');
expect(
htmlDecode(`S1:&quot;react.suspense&quot;
M2:{&quot;id&quot;:&quot;ShopifyProvider-MTY2NjM5NzU1NQ&quot;,&quot;name&quot;:&quot;ShopifyProviderClient&quot;}`)
).toBe(`S1:"react.suspense"
M2:{"id":"ShopifyProvider-MTY2NjM5NzU1NQ","name":"ShopifyProviderClient"}`);
});

it('does not double-decode ampersands', () => {
expect(htmlDecode(`drink: g&amp;gt;`)).toBe(`drink: g&gt;`);
});
});
});
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
export default function Passthrough({children}) {
return children;
export default function Passthrough({children, prop2}) {
return (
<>
<div>{children}</div>
<div dangerouslySetInnerHTML={{__html: prop2.escapedValue}} />
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ export default function Escaping() {
return (
<Passthrough
prop="</script><script>document.body = ''</script>"
// eslint-disable-next-line prettier/prettier
prop2={{escapedValue: '&quot;fiddle&quot;'}}
// eslint-disable-next-line react/no-children-prop
children="</script><script>alert('hi')</script>"
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ export default async function testCases({
expect(await page.textContent('body')).toContain(
"</script><script>alert('hi')</script>"
);
expect(await page.textContent('body')).toContain(`"fiddle"`);
});

it('adds style tags for CSS modules', async () => {
Expand Down

0 comments on commit 333dd7a

Please sign in to comment.