fixes shadow hydration escaping

[PH4NTOMiki]

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

fixes #3773
there are many better ways to solve this, if anyone has a suggestion.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: 0da6c8f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✔️ Deploy Preview for kit-demo canceled.

🔨 Explore the source changes: 0da6c8f

🔍 Inspect the deploy log: https://app.netlify.com/sites/kit-demo/deploys/6203ee9b50a2ee0008998ab6

[PH4NTOMiki]

I also tried using devalue package but it's the same, html becomes malformed. This way is vulnerable to XSS probably so not the best way, but just a suggestion.

[PH4NTOMiki]

Actually I found out that Next.js is using the same(similar) strategy with regex like this PR:
https://github.com/vercel/next.js/blob/694407450638b037673c6d714bfe4126aeded740/packages/next/server/htmlescape.ts

I'll improve this a little but this isn't that bad.

[dominikg]

we have a utility for that

kit/packages/kit/src/utils/escape.js

Line 19 in 59badb5

export function escape_json_string_in_html(str) {

[dominikg]

and it would be great to have a test in https://github.com/sveltejs/kit/tree/master/packages/kit/test/apps/basics/src/routes/xss so that we can catch regressions.

[PH4NTOMiki]

Thanks, I actually adopted the code from Next.js in the latest commit, but I'll implement that code now you suggested because DRY.

[dominikg]

@PH4NTOMiki i pushed a fix for the invalid double-qoute escape and added a test. hope you don't mind. this is kind of urgent 😬

[PH4NTOMiki]

@PH4NTOMiki i pushed a fix for the invalid double-qoute escape and added a test. hope you don't mind. this is kind of urgent grimacing

Yeah, of course, thank you

[benmccann]

Looks like you'll need to run pnpm format

[benmccann]

I had a couple suggestions for the jsdocs, but otherwise lgtm

I would like to get another set of eyes from @Conduitry though since he was more involved in #2534. Or Rich since he added the original escape mechanism in #882

[dominikg]

these docs describe where the escaped result can be safely used, not how the return value looks like.

see

kit/packages/kit/src/runtime/server/page/load_node.js

Line 260 in 647131e

json: `{"status":${response.status},"statusText":${s(response.statusText)},"headers":${s(headers)},"body":"${escape_json_string_in_html(body)}"}`

it's actually a string prop on a larger json thats added to a script block elsewhere later. Not fond of this complexity but it is what it is.

I'd like to merge this first and we can improve the docs in a followup PR.

[benmccann]

Something in this PR seems to have broken https://kit.svelte.dev/docs

[PH4NTOMiki]

@dominikg what do you think?
@benmccann could you roll over back to 886ad46
I think that should work?

[PH4NTOMiki]

Or should I try to PR that again?

[PH4NTOMiki]

@benmccann also, is there any change that that Vite update could be causing anything? maybe together with this, or alone?

[PH4NTOMiki]

I think it's the " fix that @dominikg pushed, 8c8e4e6 this commit, I think is the cause

[benmccann]

It works if I revert this line:

body += `<script type="application/json" data-type="svelte-props">${escape_json_in_html(s(shadow_props))}</script>`;

[PH4NTOMiki]

Isn't that the current/latest line?

[benmccann]

Yes, and it's broken