Merge branch 'main' into best-practices-doc-alt

pull/17804/head
Paolo Ricciuti 2 months ago committed by GitHub
commit 4b9d107eb2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -1,5 +0,0 @@
---
'svelte': patch
---
fix: hydrate if blocks correctly

@ -1,108 +0,0 @@
name: ecosystem-ci gate
on:
pull_request:
types: [opened, reopened, synchronize, ready_for_review]
issue_comment:
types: [created]
permissions: {}
jobs:
gate:
if: github.repository == 'sveltejs/svelte' && ((github.event_name == 'pull_request' && github.event.pull_request.head.ref == 'changeset-release/main') || (github.event_name == 'issue_comment' && github.event.issue.pull_request && startsWith(github.event.comment.body, '/ecosystem-ci run')))
runs-on: ubuntu-latest
permissions:
pull-requests: read
issues: read
contents: read
steps:
- name: Evaluate gate
id: evaluate
uses: actions/github-script@v8
with:
script: |
const allowed_roles = new Set(['admin', 'maintain', 'write'])
const pull_number = context.payload.pull_request
? context.payload.pull_request.number
: context.issue.number
const { data: pr } = await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number,
})
const is_release_pr = pr.head.ref === 'changeset-release/main'
if (!is_release_pr) {
core.notice(`PR #${pull_number} is not a release PR (${pr.head.ref}). Gate not required.`)
core.setOutput('should_fail', 'false')
core.setOutput('reason', 'Gate is only required for changeset-release/main')
return
}
const { data: commits } = await github.rest.pulls.listCommits({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number,
per_page: 250,
})
const last_commit = commits[commits.length - 1]
const last_commit_time = new Date(last_commit.commit.committer.date)
const comments = await github.paginate(github.rest.issues.listComments, {
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pull_number,
per_page: 100,
})
let has_valid_command = false
for (const comment of comments) {
if (!(comment.body || '').trim().startsWith('/ecosystem-ci run')) {
continue
}
const comment_time = new Date(comment.created_at)
if (comment_time <= last_commit_time) {
continue
}
let permission
try {
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({
owner: context.repo.owner,
repo: context.repo.repo,
username: comment.user.login,
})
permission = data.permission
} catch {
permission = 'none'
}
if (allowed_roles.has(permission)) {
has_valid_command = true
break
}
}
if (has_valid_command) {
core.setOutput('should_fail', 'false')
core.setOutput('reason', 'Valid maintainer /ecosystem-ci run command found after latest commit')
return
}
core.setOutput('should_fail', 'true')
core.setOutput('reason', 'Release PRs require a maintainer to run /ecosystem-ci after the latest commit')
- name: Enforce gate
if: steps.evaluate.outputs.should_fail == 'true'
run: |
echo "${{ steps.evaluate.outputs.reason }}"
exit 1
- name: Gate satisfied
if: steps.evaluate.outputs.should_fail != 'true'
run: echo "${{ steps.evaluate.outputs.reason }}"

@ -10,6 +10,10 @@ on:
description: 'Commit SHA to build'
required: true
type: string
pr:
description: 'PR number to comment on'
required: true
type: number
permissions: {}
@ -127,26 +131,16 @@ jobs:
return;
}
const { data: pulls } = await github.rest.repos.listPullRequestsAssociatedWithCommit({
owner: context.repo.owner,
repo: context.repo.repo,
commit_sha: '${{ inputs.sha }}',
});
const open = pulls.filter(p => p.state === 'open');
if (open.length === 0) {
core.setFailed(`No open PR found for commit ${{ inputs.sha }}`);
return;
}
if (open.length > 1) {
const nums = open.map(p => `#${p.number}`).join(', ');
core.setFailed(`Multiple open PRs found for commit ${{ inputs.sha }}: ${nums}`);
// For workflow_dispatch, use the explicitly provided PR number.
// We can't use listPullRequestsAssociatedWithCommit because fork
// commits don't exist in the base repo, so the API returns nothing.
const pr = Number('${{ inputs.pr }}');
if (!pr || isNaN(pr)) {
core.setFailed('workflow_dispatch requires a valid pr input');
return;
}
core.setOutput('number', open[0].number);
core.setOutput('number', pr);
- name: Post or update comment
uses: actions/github-script@v8

@ -1,5 +1,25 @@
# svelte
## 5.53.5
### Patch Changes
- fix: escape `innerText` and `textContent` bindings of `contenteditable` ([`0df5abcae223058ceb95491470372065fb87951d`](https://github.com/sveltejs/svelte/commit/0df5abcae223058ceb95491470372065fb87951d))
- fix: sanitize `transformError` values prior to embedding in HTML comments ([`0298e979371bb583855c9810db79a70a551d22b9`](https://github.com/sveltejs/svelte/commit/0298e979371bb583855c9810db79a70a551d22b9))
## 5.53.4
### Patch Changes
- fix: set server context after async transformError ([#17799](https://github.com/sveltejs/svelte/pull/17799))
- fix: hydrate if blocks correctly ([#17784](https://github.com/sveltejs/svelte/pull/17784))
- fix: handle default parameters scope leaks ([#17788](https://github.com/sveltejs/svelte/pull/17788))
- fix: prevent flushed effects from running again ([#17787](https://github.com/sveltejs/svelte/pull/17787))
## 5.53.3
### Patch Changes

@ -2,7 +2,7 @@
"name": "svelte",
"description": "Cybernetically enhanced web apps",
"license": "MIT",
"version": "5.53.3",
"version": "5.53.5",
"type": "module",
"types": "./types/index.d.ts",
"engines": {

@ -123,9 +123,13 @@ export function build_element_attributes(node, context, transform) {
expression = transform(expression, attribute.metadata.expression);
if (is_content_editable_binding(attribute.name)) {
if (attribute.name === 'innerHTML') {
// innerHTML is the only binding we don't escape
content = expression;
} else if (attribute.name === 'value' && node.name === 'textarea') {
} else if (
is_content_editable_binding(attribute.name) ||
(attribute.name === 'value' && node.name === 'textarea')
) {
content = b.call('$.escape', expression);
} else if (attribute.name === 'group' && attribute.expression.type !== 'SequenceExpression') {
const value_attribute = /** @type {AST.Attribute | undefined} */ (

@ -1098,25 +1098,19 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
},
FunctionExpression(node, { state, next }) {
const scope = state.scope.child();
const scope = state.scope.child(true);
scopes.set(node, scope);
if (node.id) {
scopes.set(node.id, state.scope); // so that declarations within with the same name are not confused with the function name
scope.declare(node.id, 'normal', 'function');
}
if (node.id) scope.declare(node.id, 'normal', 'function');
add_params(scope, node.params);
next({ scope });
},
FunctionDeclaration(node, { state, next }) {
if (node.id) {
scopes.set(node.id, state.scope); // so that declarations within with the same name are not confused with the function name
state.scope.declare(node.id, 'normal', 'function', node);
}
if (node.id) state.scope.declare(node.id, 'normal', 'function', node);
const scope = state.scope.child();
const scope = state.scope.child(true);
scopes.set(node, scope);
add_params(scope, node.params);
@ -1124,7 +1118,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
},
ArrowFunctionExpression(node, { state, next }) {
const scope = state.scope.child();
const scope = state.scope.child(true);
scopes.set(node, scope);
add_params(scope, node.params);
@ -1142,8 +1136,11 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
parent?.type === 'FunctionExpression' ||
parent?.type === 'ArrowFunctionExpression'
) {
// We already created a new scope for the function
context.next();
// The scopes created for the function nodes above handle the function identifier and
// parameters, but the block statement itself holds the non-porous function scope
const scope = context.state.scope.child();
scopes.set(node, scope);
context.next({ scope });
} else {
create_block_scope(node, context);
}

@ -248,6 +248,7 @@ export function run(thunks) {
promise.finally(() => {
blocker.settled = true;
unset_context();
});
for (const fn of thunks.slice(1)) {

@ -1,6 +1,5 @@
/** @import { Fork } from 'svelte' */
/** @import { Derived, Effect, Reaction, Source, Value } from '#client' */
/** @import { Boundary } from '../dom/blocks/boundary' */
import {
BLOCK_EFFECT,
BRANCH_EFFECT,
@ -14,7 +13,6 @@ import {
ROOT_EFFECT,
MAYBE_DIRTY,
DERIVED,
BOUNDARY_EFFECT,
EAGER_EFFECT,
HEAD_EFFECT,
ERROR_VALUE,
@ -225,6 +223,10 @@ export class Batch {
flush_queued_effects(render_effects);
flush_queued_effects(effects);
// Clear effects. Those that are still needed will be rescheduled through unskipping the skipped branches.
this.#dirty_effects.clear();
this.#maybe_dirty_effects.clear();
previous_batch = null;
this.#deferred?.resolve();
@ -423,6 +425,7 @@ export class Batch {
batch_values = previous_batch_values;
}
this.#skipped_branches.clear();
batches.delete(this);
}

@ -293,13 +293,14 @@ export class Renderer {
}
child.promise = /** @type {Promise<unknown>} */ (result).then((transformed) => {
child.#out.push(`<!--${HYDRATION_START_FAILED}${JSON.stringify(transformed)}-->`);
set_ssr_context(parent_context);
child.#out.push(Renderer.#serialize_failed_boundary(transformed));
failed_snippet(child, transformed, noop);
child.#out.push(BLOCK_CLOSE);
});
child.promise.catch(noop);
} else {
child.#out.push(`<!--${HYDRATION_START_FAILED}${JSON.stringify(result)}-->`);
child.#out.push(Renderer.#serialize_failed_boundary(result));
failed_snippet(child, result, noop);
child.#out.push(BLOCK_CLOSE);
}
@ -481,6 +482,21 @@ export class Renderer {
return this.#out.length;
}
/**
* Creates the hydration comment that marks the start of a failed boundary.
* The error is JSON-serialized and embedded inside an HTML comment for the client
* to parse during hydration. The JSON is escaped to prevent `-->` or `<!--` sequences
* from breaking out of the comment (XSS). Uses unicode escapes which `JSON.parse()`
* handles transparently.
* @param {unknown} error
* @returns {string}
*/
static #serialize_failed_boundary(error) {
var json = JSON.stringify(error);
var escaped = json.replace(/>/g, '\\u003e').replace(/</g, '\\u003c');
return `<!--${HYDRATION_START_FAILED}${escaped}-->`;
}
/**
* Only available on the server and when compiling with the `server` option.
* Takes a component and returns an object with `body` and `head` properties on it, which you can use to populate the HTML when server-rendering your app.
@ -700,9 +716,7 @@ export class Renderer {
// Render the failed snippet instead of the partial children content
const failed_renderer = new Renderer(item.global, item);
failed_renderer.type = item.type;
failed_renderer.#out.push(
`<!--${HYDRATION_START_FAILED}${JSON.stringify(transformed)}-->`
);
failed_renderer.#out.push(Renderer.#serialize_failed_boundary(transformed));
failed(failed_renderer, transformed, noop);
failed_renderer.#out.push(BLOCK_CLOSE);
await failed_renderer.#collect_content_async(content);

@ -225,6 +225,118 @@ test('select merges scoped css hash with static class', () => {
);
});
describe('boundary hydration comment escaping', () => {
const failed_snippet = (renderer: Renderer, error: unknown) => {
renderer.push(`<p>${(error as { message: string }).message}</p>`);
};
const transform = (error: unknown) => ({ message: (error as Error).message });
const payloads = [
{
name: 'escapes -->',
input: '--><img src=x onerror=alert(1)><!--',
expected: '{"message":"--\\u003e\\u003cimg src=x onerror=alert(1)\\u003e\\u003c!--"}'
},
{
name: 'escapes <!--',
input: '<!--<script>alert(1)</script>',
expected: '{"message":"\\u003c!--\\u003cscript\\u003ealert(1)\\u003c/script\\u003e"}'
},
{ name: 'escapes <!-->', input: '<!-->', expected: '{"message":"\\u003c!--\\u003e"}' },
{ name: 'escapes <!--->', input: '<!--->', expected: '{"message":"\\u003c!---\\u003e"}' },
{
name: 'escapes multiple -->',
input: '-->one-->two-->',
expected: '{"message":"--\\u003eone--\\u003etwo--\\u003e"}'
},
{ name: 'escapes --->', input: '--->', expected: '{"message":"---\\u003e"}' },
{ name: 'no double-encoding', input: '--\\u003e', expected: '{"message":"--\\\\u003e"}' },
{
name: 'the terrifying special pointy boy',
input: '--!>ooh, what an exotic closing comment tag',
expected: '{"message":"--!\\u003eooh, what an exotic closing comment tag"}'
}
];
type RenderFn = (input: string) => Promise<string> | string;
const paths: Array<{ path: string; async: boolean; render: RenderFn }> = [
{
path: 'sync children, sync transformError',
async: false,
render: (input) => {
const component = (renderer: Renderer) => {
renderer.boundary({ failed: failed_snippet }, () => {
throw new Error(input);
});
};
return Renderer.render(
component as unknown as Component,
{ transformError: transform } as any
).body;
}
},
{
path: 'sync children, async transformError',
async: true,
render: async (input) => {
const component = (renderer: Renderer) => {
renderer.boundary({ failed: failed_snippet }, () => {
throw new Error(input);
});
};
return (
await Renderer.render(
component as unknown as Component,
{
transformError: (error: unknown) => Promise.resolve(transform(error))
} as any
)
).body;
}
},
{
path: 'async children throw',
async: true,
render: async (input) => {
const component = (renderer: Renderer) => {
renderer.boundary({ failed: failed_snippet }, async () => {
await Promise.resolve();
throw new Error(input);
});
};
return (
await Renderer.render(
component as unknown as Component,
{
transformError: transform
} as any
)
).body;
}
}
];
describe.each(paths)('$path', ({ async: needs_async, render }) => {
if (needs_async) {
beforeAll(() => enable_async_mode_flag());
afterAll(() => disable_async_mode_flag());
}
test.each(payloads)('$name', async ({ input, expected }) => {
const body = await render(input);
// Extract the content between <!--[? and the first -->
// If escaping is broken, an unescaped --> in the JSON will truncate
// the match and the content won't equal the expected escaped JSON.
const match = body.match(/<!--\[\?(.+?)-->/);
expect(match, 'expected a hydration comment in output').toBeTruthy();
expect(match![1]).toBe(expected);
});
});
});
describe('async', () => {
beforeAll(() => {
enable_async_mode_flag();

@ -4,5 +4,5 @@
* The current version, as set in package.json.
* @type {string}
*/
export const VERSION = '5.53.3';
export const VERSION = '5.53.5';
export const PUBLIC_VERSION = '5';

@ -0,0 +1,82 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
html: `
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>pending a</p>
`,
async test({ assert, target }) {
const [a, b, resolve_a, resolve_b] = target.querySelectorAll('button');
resolve_a.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>page a</p>
`
);
b.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>pending b</p>
`
);
a.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>pending a</p>
`
);
resolve_b.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>pending a</p>
`
);
resolve_a.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>page a</p>
`
);
await new Promise((r) => setTimeout(r, 100));
}
});

@ -0,0 +1,52 @@
<script>
let page = $state('a');
/** @type {Array<() => void>} */
const a = [];
/** @type {Array<() => void>} */
const b = [];
function gate(next) {
const deferred = Promise.withResolvers();
if (next === 'a') a.push(deferred.resolve);
else b.push(deferred.resolve);
return deferred.promise;
}
function nav(next) {
page = next;
}
const to_render = $derived(page === 'a' ? snippet_a : snippet_b);
</script>
<button onclick={() => nav('a')}>a</button>
<button onclick={() => nav('b')}>b</button>
<button onclick={() => a.shift()?.()}>resolve a</button>
<button onclick={() => b.shift()?.()}>resolve b</button>
{#snippet snippet_a()}
<svelte:boundary>
{@const _a = await gate('a')}
<p>page a</p>
{#snippet pending()}
<p>pending a</p>
{/snippet}
</svelte:boundary>
{/snippet}
{#snippet snippet_b()}
<svelte:boundary>
{@const _b = await gate('b')}
<p>page b</p>
{#snippet pending()}
<p>pending b</p>
{/snippet}
</svelte:boundary>
{/snippet}
{@render to_render()}

@ -0,0 +1,15 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
mode: ['hydrate', 'async-server', 'client'],
ssrHtml: '<p>caught: error (hello)</p>',
transformError: () => {
return Promise.resolve('error');
},
async test({ assert, target }) {
await tick();
assert.htmlEqual(target.innerHTML, '<p>caught: error (hello)</p>');
}
});

@ -0,0 +1,14 @@
<script>
import { get } from "./main.svelte";
let { error } = $props();
const context = get()
</script>
{#if error}
<p>caught: {await error} ({context})</p>
{:else}
{(() => {
throw 'catch me';
})()}
{/if}

@ -0,0 +1,19 @@
<script module>
import { createContext } from "svelte";
import Child from "./child.svelte";
const [ get, set ] = createContext();
export {get};
</script>
<script>
set('hello');
</script>
<svelte:boundary>
{#snippet failed(error)}
<Child {error} />
{/snippet}
<Child />
</svelte:boundary>

@ -0,0 +1,7 @@
import { test } from '../../test';
export default test({
test({ assert, logs }) {
assert.deepEqual(logs, [42, 43]);
}
});

@ -0,0 +1,12 @@
<script>
let value = $state(42);
function shadow(output = value) {
const value = 1337;
return output;
}
console.log(shadow());
value += 1;
console.log(shadow());
</script>

@ -0,0 +1,8 @@
import { test } from '../../test';
export default test({
props: {
query: '--!><img src=x onerror=alert(1)><!--'
},
transformError: (error) => ({ message: /** @type {Error} */ (error).message })
});

@ -0,0 +1,15 @@
<script>
let { query } = $props();
function search(q) {
throw new Error(q);
}
</script>
<svelte:boundary>
<p>{search(query)}</p>
{#snippet failed(error)}
<p class="error">{error.message}</p>
{/snippet}
</svelte:boundary>

@ -0,0 +1,8 @@
import { test } from '../../test';
export default test({
props: {
query: '--><img src=x onerror=alert(1)><!--'
},
transformError: (error) => ({ message: /** @type {Error} */ (error).message })
});

@ -0,0 +1,15 @@
<script>
let { query } = $props();
function search(q) {
throw new Error(q);
}
</script>
<svelte:boundary>
<p>{search(query)}</p>
{#snippet failed(error)}
<p class="error">{error.message}</p>
{/snippet}
</svelte:boundary>

@ -0,0 +1,8 @@
import { test } from '../../test';
export default test({
props: {
query: '<!--<script>alert(1)</script>'
},
transformError: (error) => ({ message: /** @type {Error} */ (error).message })
});

@ -0,0 +1,15 @@
<script>
let { query } = $props();
function search(q) {
throw new Error(q);
}
</script>
<svelte:boundary>
<p>{search(query)}</p>
{#snippet failed(error)}
<p class="error">{error.message}</p>
{/snippet}
</svelte:boundary>

@ -0,0 +1,8 @@
import { test } from '../../test';
export default test({
props: {
query: '<!--><!--->-->'
},
transformError: (error) => ({ message: /** @type {Error} */ (error).message })
});

@ -0,0 +1,15 @@
<script>
let { query } = $props();
function search(q) {
throw new Error(q);
}
</script>
<svelte:boundary>
<p>{search(query)}</p>
{#snippet failed(error)}
<p class="error">{error.message}</p>
{/snippet}
</svelte:boundary>

@ -0,0 +1 @@
<!--[--><div contenteditable="">&lt;script>alert('pwnd')&lt;/script></div> <div contenteditable="">&lt;script>alert('pwnd')&lt;/script></div> <div contenteditable=""><script>alert('pwnd')</script></div><!--]-->

@ -0,0 +1,7 @@
<script>
let data = $state("<scri"+"pt>alert('pwnd')</scr"+"ipt>");
</script>
<div contenteditable bind:innerText={data}></div>
<div contenteditable bind:textContent={data}></div>
<div contenteditable bind:innerHTML={data}></div>
Loading…
Cancel
Save