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.

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

fix: add snippet argument validation in dev #15521

Merged
merged 11 commits into from
Apr 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/bright-jeans-compare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: add snippet argument validation in dev
6 changes: 6 additions & 0 deletions documentation/docs/98-reference/.generated/shared-errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ This error would be thrown in a setup like this:

Here, `List.svelte` is using `{@render children(item)` which means it expects `Parent.svelte` to use snippets. Instead, `Parent.svelte` uses the deprecated `let:` directive. This combination of APIs is incompatible, hence the error.

### invalid_snippet_arguments

```
A snippet function was passed invalid arguments. Snippets should only be instantiated via `{@render ...}`
```

### lifecycle_outside_component

```
Expand Down
4 changes: 4 additions & 0 deletions packages/svelte/messages/shared-errors/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ This error would be thrown in a setup like this:

Here, `List.svelte` is using `{@render children(item)` which means it expects `Parent.svelte` to use snippets. Instead, `Parent.svelte` uses the deprecated `let:` directive. This combination of APIs is incompatible, hence the error.

## invalid_snippet_arguments

> A snippet function was passed invalid arguments. Snippets should only be instantiated via `{@render ...}`

## lifecycle_outside_component

> `%name%(...)` can only be used during component initialisation
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** @import { BlockStatement, Expression, Identifier, Pattern, Statement } from 'estree' */
/** @import { AssignmentPattern, BlockStatement, Expression, Identifier, Statement } from 'estree' */
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types' */
import { dev } from '../../../../state.js';
Expand All @@ -12,7 +12,7 @@ import { get_value } from './shared/declarations.js';
*/
export function SnippetBlock(node, context) {
// TODO hoist where possible
/** @type {Pattern[]} */
/** @type {(Identifier | AssignmentPattern)[]} */
const args = [b.id('$$anchor')];

/** @type {BlockStatement} */
Expand Down Expand Up @@ -66,7 +66,18 @@ export function SnippetBlock(node, context) {
}
}
}

if (dev) {
declarations.unshift(
b.stmt(
b.call(
'$.validate_snippet_args',
.../** @type {Identifier[]} */ (
args.map((arg) => (arg?.type === 'Identifier' ? arg : arg?.left))
)
)
)
);
}
body = b.block([
...declarations,
.../** @type {BlockStatement} */ (context.visit(node.body, child_state)).body
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/** @import { BlockStatement } from 'estree' */
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types.js' */
import { dev } from '../../../../state.js';
import * as b from '../../../../utils/builders.js';

/**
Expand All @@ -13,7 +14,9 @@ export function SnippetBlock(node, context) {
[b.id('$$payload'), ...node.parameters],
/** @type {BlockStatement} */ (context.visit(node.body))
);

if (dev) {
fn.body.body.unshift(b.stmt(b.call('$.validate_snippet_args', b.id('$$payload'))));
}
// @ts-expect-error - TODO remove this hack once $$render_inner for legacy bindings is gone
fn.___snippet = true;

Expand Down
15 changes: 15 additions & 0 deletions packages/svelte/src/internal/client/dev/validation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { invalid_snippet_arguments } from '../../shared/errors.js';
/**
* @param {Node} anchor
* @param {...(()=>any)[]} args
*/
export function validate_snippet_args(anchor, ...args) {
if (typeof anchor !== 'object' || !(anchor instanceof Node)) {
invalid_snippet_arguments();
}
for (let arg of args) {
if (typeof arg !== 'function') {
invalid_snippet_arguments();
}
}
}
1 change: 1 addition & 0 deletions packages/svelte/src/internal/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export {
export { check_target, legacy_api } from './dev/legacy.js';
export { trace } from './dev/tracing.js';
export { inspect } from './dev/inspect.js';
export { validate_snippet_args } from './dev/validation.js';
export { await_block as await } from './dom/blocks/await.js';
export { if_block as if } from './dom/blocks/if.js';
export { key_block as key } from './dom/blocks/key.js';
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/internal/server/blocks/snippet.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @import { Snippet } from 'svelte' */
/** @import { Payload } from '#server' */
/** @import { Payload } from '../payload' */
/** @import { Getters } from '#shared' */

/**
Expand Down
13 changes: 12 additions & 1 deletion packages/svelte/src/internal/server/dev.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
/** @import { Component, Payload } from '#server' */
/** @import { Component } from '#server' */
import { FILENAME } from '../../constants.js';
import {
is_tag_valid_with_ancestor,
is_tag_valid_with_parent
} from '../../html-tree-validation.js';
import { current_component } from './context.js';
import { invalid_snippet_arguments } from '../shared/errors.js';
import { Payload } from './payload.js';

/**
* @typedef {{
Expand Down Expand Up @@ -98,3 +100,12 @@ export function push_element(payload, tag, line, column) {
export function pop_element() {
parent = /** @type {Element} */ (parent).parent;
}

/**
* @param {Payload} payload
*/
export function validate_snippet_args(payload) {
if (typeof payload !== 'object' || !(payload instanceof Payload)) {
invalid_snippet_arguments();
}
}
57 changes: 6 additions & 51 deletions packages/svelte/src/internal/server/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @import { ComponentType, SvelteComponent } from 'svelte' */
/** @import { Component, Payload, RenderOutput } from '#server' */
/** @import { Component, RenderOutput } from '#server' */
/** @import { Store } from '#shared' */
export { FILENAME, HMR } from '../../constants.js';
import { attr, clsx, to_class, to_style } from '../shared/attributes.js';
Expand All @@ -17,43 +17,13 @@ import { EMPTY_COMMENT, BLOCK_CLOSE, BLOCK_OPEN } from './hydration.js';
import { validate_store } from '../shared/validate.js';
import { is_boolean_attribute, is_raw_text_element, is_void } from '../../utils.js';
import { reset_elements } from './dev.js';
import { Payload } from './payload.js';

// https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
// https://infra.spec.whatwg.org/#noncharacter
const INVALID_ATTR_NAME_CHAR_REGEX =
/[\s'">/=\u{FDD0}-\u{FDEF}\u{FFFE}\u{FFFF}\u{1FFFE}\u{1FFFF}\u{2FFFE}\u{2FFFF}\u{3FFFE}\u{3FFFF}\u{4FFFE}\u{4FFFF}\u{5FFFE}\u{5FFFF}\u{6FFFE}\u{6FFFF}\u{7FFFE}\u{7FFFF}\u{8FFFE}\u{8FFFF}\u{9FFFE}\u{9FFFF}\u{AFFFE}\u{AFFFF}\u{BFFFE}\u{BFFFF}\u{CFFFE}\u{CFFFF}\u{DFFFE}\u{DFFFF}\u{EFFFE}\u{EFFFF}\u{FFFFE}\u{FFFFF}\u{10FFFE}\u{10FFFF}]/u;

/**
* @param {Payload} to_copy
* @returns {Payload}
*/
export function copy_payload({ out, css, head, uid }) {
return {
out,
css: new Set(css),
head: {
title: head.title,
out: head.out,
css: new Set(head.css),
uid: head.uid
},
uid
};
}

/**
* Assigns second payload to first
* @param {Payload} p1
* @param {Payload} p2
* @returns {void}
*/
export function assign_payload(p1, p2) {
p1.out = p2.out;
p1.css = p2.css;
p1.head = p2.head;
p1.uid = p2.uid;
}

/**
* @param {Payload} payload
* @param {string} tag
Expand Down Expand Up @@ -87,16 +57,6 @@ export function element(payload, tag, attributes_fn = noop, children_fn = noop)
*/
export let on_destroy = [];

/**
* Creates an ID generator
* @param {string} prefix
* @returns {() => string}
*/
function props_id_generator(prefix) {
let uid = 1;
return () => `${prefix}s${uid++}`;
}

/**
* 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.
Expand All @@ -106,14 +66,7 @@ function props_id_generator(prefix) {
* @returns {RenderOutput}
*/
export function render(component, options = {}) {
const uid = props_id_generator(options.idPrefix ? options.idPrefix + '-' : '');
/** @type {Payload} */
const payload = {
out: '',
css: new Set(),
head: { title: '', out: '', css: new Set(), uid },
uid
};
const payload = new Payload(options.idPrefix ? options.idPrefix + '-' : '');

const prev_on_destroy = on_destroy;
on_destroy = [];
Expand Down Expand Up @@ -542,7 +495,9 @@ export { html } from './blocks/html.js';

export { push, pop } from './context.js';

export { push_element, pop_element } from './dev.js';
export { push_element, pop_element, validate_snippet_args } from './dev.js';

export { assign_payload, copy_payload } from './payload.js';

export { snapshot } from '../shared/clone.js';

Expand Down
64 changes: 64 additions & 0 deletions packages/svelte/src/internal/server/payload.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
export class Payload {
/** @type {Set<{ hash: string; code: string }>} */
css = new Set();
out = '';
uid = () => '';

head = {
/** @type {Set<{ hash: string; code: string }>} */
css: new Set(),
title: '',
out: '',
uid: () => ''
};

constructor(id_prefix = '') {
this.uid = props_id_generator(id_prefix);
this.head.uid = this.uid;
}
}

/**
* Used in legacy mode to handle bindings
* @param {Payload} to_copy
* @returns {Payload}
*/
export function copy_payload({ out, css, head, uid }) {
const payload = new Payload();

payload.out = out;
payload.css = new Set(css);
payload.uid = uid;

payload.head = {
title: head.title,
out: head.out,
css: new Set(head.css),
uid: head.uid
};

return payload;
}

/**
* Assigns second payload to first
* @param {Payload} p1
* @param {Payload} p2
* @returns {void}
*/
export function assign_payload(p1, p2) {
p1.out = p2.out;
p1.css = p2.css;
p1.head = p2.head;
p1.uid = p2.uid;
}

/**
* Creates an ID generator
* @param {string} prefix
* @returns {() => string}
*/
function props_id_generator(prefix) {
let uid = 1;
return () => `${prefix}s${uid++}`;
}
13 changes: 0 additions & 13 deletions packages/svelte/src/internal/server/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,6 @@ export interface Component {
function?: any;
}

export interface Payload {
out: string;
css: Set<{ hash: string; code: string }>;
head: {
title: string;
out: string;
uid: () => string;
css: Set<{ hash: string; code: string }>;
};
/** Function that generates a unique ID */
uid: () => string;
}

export interface RenderOutput {
/** HTML that goes into the `<head>` */
head: string;
Expand Down
15 changes: 15 additions & 0 deletions packages/svelte/src/internal/shared/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,21 @@ export function invalid_default_snippet() {
}
}

/**
* A snippet function was passed invalid arguments. Snippets should only be instantiated via `{@render ...}`
* @returns {never}
*/
export function invalid_snippet_arguments() {
if (DEV) {
const error = new Error(`invalid_snippet_arguments\nA snippet function was passed invalid arguments. Snippets should only be instantiated via \`{@render ...}\`\nhttps://svelte.dev/e/invalid_snippet_arguments`);

error.name = 'Svelte error';
throw error;
} else {
throw new Error(`https://svelte.dev/e/invalid_snippet_arguments`);
}
}

/**
* `%name%(...)` can only be used during component initialisation
* @param {string} name
Expand Down