Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(server-islands): only encode ETAGO delimiter #11513

Merged
merged 8 commits into from
Jan 20, 2025
Merged

feat(server-islands): only encode ETAGO delimiter #11513

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
87aa1f6
server-islands: only encode ETAGO delimiter & opening HTML comment
kurtextrem Jul 20, 2024
d1bfe8b
Merge branch 'main' into perf/server-islands-json
florian-lefebvre Jan 8, 2025
ec0579c
feat: add test
florian-lefebvre Jan 8, 2025
4dc842d
Update fifty-socks-end.md
florian-lefebvre Jan 8, 2025
4b004f9
Update .changeset/fifty-socks-end.md
florian-lefebvre Jan 8, 2025
1ab9ad1
chore: casing
florian-lefebvre Jan 8, 2025
c561a5b
Merge branch 'main' into perf/server-islands-json
florian-lefebvre Jan 8, 2025
a0d1205
feat: update string
florian-lefebvre Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fifty-socks-end.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Updates the server islands encoding logic to only escape the script end tag open delimiter and opening HTML comment syntax
16 changes: 11 additions & 5 deletions packages/astro/src/runtime/server/render/server-islands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,19 @@ export function containsServerDirective(props: Record<string | number, any>) {
return 'server:component-directive' in props;
}

const SCRIPT_RE = /<\/script/giu;
const COMMENT_RE = /<!--/gu;
const SCRIPT_REPLACER = '<\\/script';
const COMMENT_REPLACER = '\\u003C!--';

/**
* Encodes the script end-tag open (ETAGO) delimiter and opening HTML comment syntax for JSON inside a `<script>` tag.
* @see https://mathiasbynens.be/notes/etago
*/
function safeJsonStringify(obj: any) {
return JSON.stringify(obj)
.replace(/\u2028/g, '\\u2028')
.replace(/\u2029/g, '\\u2029')
.replace(/</g, '\\u003c')
.replace(/>/g, '\\u003e')
.replace(/\//g, '\\u002f');
.replace(SCRIPT_RE, SCRIPT_REPLACER)
.replace(COMMENT_RE, COMMENT_REPLACER);
}

function createSearchParams(componentExport: string, encryptedProps: string, slots: string) {
Expand Down
4 changes: 3 additions & 1 deletion packages/astro/test/fixtures/server-islands/ssr/src/pages/index.astro
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
---
import Island from '../components/Island.astro';

const xssMe ="</script><script>alert('xss')</script>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have a couple more examples in the test, particularly one that uses a comment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really familiar with what could through, examples welcome

Copy link
Contributor Author

@kurtextrem kurtextrem Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably something like '"<!--"</script>'. Thank you for taking this over.

---
<html>
<head>
<title>Testing</title>
</head>
<body>
<h1>Testing</h1>
<Island server:defer />
<Island server:defer message={xssMe} />
</body>
</html>
7 changes: 7 additions & 0 deletions packages/astro/test/server-islands.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ describe('Server islands', () => {
assert.equal(serverIslandEl.length, 0);
});

it('HTML escapes scripts', async () => {
const res = await fixture.fetch('/');
assert.equal(res.status, 200);
const html = await res.text();
assert.equal(html.includes("</script><script>alert('xss')</script>"), false);
});

it('island is not indexed', async () => {
const res = await fixture.fetch('/_server-islands/Island', {
method: 'POST',
Expand Down
Loading