From 8a596936d2cad9e39ccbbc2bf7e6498cc50d1e64 Mon Sep 17 00:00:00 2001
From: David Kondrad <david.a.kondrad@jci.com>
Date: Tue, 24 Dec 2019 12:07:11 -0500
Subject: [PATCH] dynamic events: validate handler before executing (#4105)

---
 .../wrappers/Element/EventHandler.ts          |  2 +-
 .../samples/event-handler-dynamic/expected.js |  3 +-
 .../event-handler-dynamic-hash/_config.js     | 56 +++++++++++++++++++
 .../event-handler-dynamic-hash/main.svelte    | 23 ++++++++
 .../event-handler-dynamic-invalid/_config.js  | 28 ++++++++++
 .../event-handler-dynamic-invalid/main.svelte | 13 +++++
 .../_config.js                                | 16 ++++++
 .../main.svelte                               |  7 +++
 .../_config.js                                | 16 ++++++
 .../main.svelte                               | 11 ++++
 .../_config.js                                | 16 ++++++
 .../main.svelte                               | 13 +++++
 .../_config.js                                | 19 +++++++
 .../main.svelte                               | 20 +++++++
 .../event-handler-dynamic-multiple/_config.js | 14 +++++
 .../main.svelte                               | 11 ++++
 .../samples/event-handler-dynamic/_config.js  | 10 +++-
 17 files changed, 274 insertions(+), 4 deletions(-)
 create mode 100644 test/runtime/samples/event-handler-dynamic-hash/_config.js
 create mode 100644 test/runtime/samples/event-handler-dynamic-hash/main.svelte
 create mode 100644 test/runtime/samples/event-handler-dynamic-invalid/_config.js
 create mode 100644 test/runtime/samples/event-handler-dynamic-invalid/main.svelte
 create mode 100644 test/runtime/samples/event-handler-dynamic-modifier-once/_config.js
 create mode 100644 test/runtime/samples/event-handler-dynamic-modifier-once/main.svelte
 create mode 100644 test/runtime/samples/event-handler-dynamic-modifier-prevent-default/_config.js
 create mode 100644 test/runtime/samples/event-handler-dynamic-modifier-prevent-default/main.svelte
 create mode 100644 test/runtime/samples/event-handler-dynamic-modifier-self/_config.js
 create mode 100644 test/runtime/samples/event-handler-dynamic-modifier-self/main.svelte
 create mode 100644 test/runtime/samples/event-handler-dynamic-modifier-stop-propagation/_config.js
 create mode 100644 test/runtime/samples/event-handler-dynamic-modifier-stop-propagation/main.svelte
 create mode 100644 test/runtime/samples/event-handler-dynamic-multiple/_config.js
 create mode 100644 test/runtime/samples/event-handler-dynamic-multiple/main.svelte

diff --git a/src/compiler/compile/render_dom/wrappers/Element/EventHandler.ts b/src/compiler/compile/render_dom/wrappers/Element/EventHandler.ts
index 75fa17db2f..03183ee576 100644
--- a/src/compiler/compile/render_dom/wrappers/Element/EventHandler.ts
+++ b/src/compiler/compile/render_dom/wrappers/Element/EventHandler.ts
@@ -30,7 +30,7 @@ export default class EventHandlerWrapper {
 
 		if (this.node.reassigned) {
 			block.maintain_context = true;
-			return x`function () { ${snippet}.apply(this, arguments); }`;
+			return x`function () { if (@is_function(${snippet})) ${snippet}.apply(this, arguments); }`;
 		}
 		return snippet;
 	}
diff --git a/test/js/samples/event-handler-dynamic/expected.js b/test/js/samples/event-handler-dynamic/expected.js
index 34c33151bf..42c6b2951a 100644
--- a/test/js/samples/event-handler-dynamic/expected.js
+++ b/test/js/samples/event-handler-dynamic/expected.js
@@ -6,6 +6,7 @@ import {
 	element,
 	init,
 	insert,
+	is_function,
 	listen,
 	noop,
 	run_all,
@@ -46,7 +47,7 @@ function create_fragment(ctx) {
 				listen(button0, "click", /*updateHandler1*/ ctx[2]),
 				listen(button1, "click", /*updateHandler2*/ ctx[3]),
 				listen(button2, "click", function () {
-					/*clickHandler*/ ctx[0].apply(this, arguments);
+					if (is_function(/*clickHandler*/ ctx[0])) /*clickHandler*/ ctx[0].apply(this, arguments);
 				})
 			];
 		},
diff --git a/test/runtime/samples/event-handler-dynamic-hash/_config.js b/test/runtime/samples/event-handler-dynamic-hash/_config.js
new file mode 100644
index 0000000000..e60e561524
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-hash/_config.js
@@ -0,0 +1,56 @@
+export default {
+	html: `
+	<p>
+		<button>set handler 1</button>
+		<button>set handler 2</button>
+	</p>
+	<p>0</p>
+	<button>click</button>
+	`,
+
+	async test({ assert, component, target, window }) {
+		const [updateButton1, updateButton2, button] = target.querySelectorAll(
+			'button'
+		);
+
+		const event = new window.MouseEvent('click');
+		let err = "";
+		window.addEventListener('error', (e) => {
+			e.preventDefault();
+			err = e.message;
+		});
+
+		await button.dispatchEvent(event);
+		assert.equal(err, "", err);
+		assert.htmlEqual(target.innerHTML, `
+			<p>
+				<button>set handler 1</button>
+				<button>set handler 2</button>
+			</p>
+			<p>0</p>
+			<button>click</button>
+		`);
+
+		await updateButton1.dispatchEvent(event);
+		await button.dispatchEvent(event);
+		assert.htmlEqual(target.innerHTML, `
+			<p>
+				<button>set handler 1</button>
+				<button>set handler 2</button>
+			</p>
+			<p>1</p>
+			<button>click</button>
+		`);
+
+		await updateButton2.dispatchEvent(event);
+		await button.dispatchEvent(event);
+		assert.htmlEqual(target.innerHTML, `
+			<p>
+				<button>set handler 1</button>
+				<button>set handler 2</button>
+			</p>
+			<p>2</p>
+			<button>click</button>
+		`);
+	},
+};
diff --git a/test/runtime/samples/event-handler-dynamic-hash/main.svelte b/test/runtime/samples/event-handler-dynamic-hash/main.svelte
new file mode 100644
index 0000000000..c81af02006
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-hash/main.svelte
@@ -0,0 +1,23 @@
+<script>
+	let clickHandler = {};
+	let number = 0;
+	
+	function updateHandler1(){
+		clickHandler.f = () => number = 1;
+	}
+
+	function updateHandler2(){
+		clickHandler.f = () => number = 2;
+	}
+	
+
+</script>
+
+<p>
+<button on:click={updateHandler1}>set handler 1</button>
+<button on:click={updateHandler2}>set handler 2</button>
+</p>
+
+<p>{ number }</p>
+
+<button on:click={clickHandler.f}>click</button>
\ No newline at end of file
diff --git a/test/runtime/samples/event-handler-dynamic-invalid/_config.js b/test/runtime/samples/event-handler-dynamic-invalid/_config.js
new file mode 100644
index 0000000000..ba1777f945
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-invalid/_config.js
@@ -0,0 +1,28 @@
+export default {
+	html: `<button>undef</button>
+	<button>null</button>
+	<button>invalid</button>`,
+
+	async test({ assert, component, target, window }) {
+		const [buttonUndef, buttonNull, buttonInvalid] = target.querySelectorAll(
+			'button'
+		);
+
+		const event = new window.MouseEvent('click');
+		let err = "";
+		window.addEventListener('error', (e) => {
+			e.preventDefault();
+			err = e.message;
+		});
+
+		// All three should not throw if proper checking is done in runtime code
+		await buttonUndef.dispatchEvent(event);
+		assert.equal(err, "", err);
+
+		await buttonNull.dispatchEvent(event);
+		assert.equal(err, "", err);
+
+		await buttonInvalid.dispatchEvent(event);
+		assert.equal(err, "", err);
+	},
+};
diff --git a/test/runtime/samples/event-handler-dynamic-invalid/main.svelte b/test/runtime/samples/event-handler-dynamic-invalid/main.svelte
new file mode 100644
index 0000000000..f4e8c5fdb7
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-invalid/main.svelte
@@ -0,0 +1,13 @@
+<script>
+	let handlerUndef;
+	let handlerNull;
+	let handlerInvalid;
+
+	handlerUndef = undefined;
+	handlerNull = null;
+	handlerInvalid = 42;
+</script>
+
+<button on:click={handlerUndef}>undef</button>
+<button on:click={handlerNull}>null</button>
+<button on:click={handlerInvalid}>invalid</button>
diff --git a/test/runtime/samples/event-handler-dynamic-modifier-once/_config.js b/test/runtime/samples/event-handler-dynamic-modifier-once/_config.js
new file mode 100644
index 0000000000..41daf374c8
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-modifier-once/_config.js
@@ -0,0 +1,16 @@
+export default {
+	html: `
+		<button>0</button>
+	`,
+
+	async test({ assert, component, target, window }) {
+		const button = target.querySelector('button');
+		const event = new window.MouseEvent('click');
+
+		await button.dispatchEvent(event);
+		assert.equal(component.count, 1);
+
+		await button.dispatchEvent(event);
+		assert.equal(component.count, 1);
+	}
+};
diff --git a/test/runtime/samples/event-handler-dynamic-modifier-once/main.svelte b/test/runtime/samples/event-handler-dynamic-modifier-once/main.svelte
new file mode 100644
index 0000000000..d363d708ba
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-modifier-once/main.svelte
@@ -0,0 +1,7 @@
+<script>
+	let f;
+	export let count = 0;
+	f = () => count += 1;
+</script>
+
+<button on:click|once="{f}">{count}</button>
\ No newline at end of file
diff --git a/test/runtime/samples/event-handler-dynamic-modifier-prevent-default/_config.js b/test/runtime/samples/event-handler-dynamic-modifier-prevent-default/_config.js
new file mode 100644
index 0000000000..4fa032bf37
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-modifier-prevent-default/_config.js
@@ -0,0 +1,16 @@
+export default {
+	html: `
+		<button>click me</button>
+	`,
+
+	async test({ assert, component, target, window }) {
+		const button = target.querySelector('button');
+		const event = new window.MouseEvent('click', {
+			cancelable: true
+		});
+
+		await button.dispatchEvent(event);
+
+		assert.ok(component.default_was_prevented);
+	}
+};
diff --git a/test/runtime/samples/event-handler-dynamic-modifier-prevent-default/main.svelte b/test/runtime/samples/event-handler-dynamic-modifier-prevent-default/main.svelte
new file mode 100644
index 0000000000..49d42f3305
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-modifier-prevent-default/main.svelte
@@ -0,0 +1,11 @@
+<script>
+	export let default_was_prevented;
+	let f;
+
+	function handle_click(event) {
+		default_was_prevented = event.defaultPrevented;
+	}
+	f = handle_click;
+</script>
+
+<button on:click|preventDefault={f}>click me</button>
\ No newline at end of file
diff --git a/test/runtime/samples/event-handler-dynamic-modifier-self/_config.js b/test/runtime/samples/event-handler-dynamic-modifier-self/_config.js
new file mode 100644
index 0000000000..6d7d29e482
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-modifier-self/_config.js
@@ -0,0 +1,16 @@
+export default {
+	html: `
+		<div>
+			<button>click me</button>
+		</div>
+	`,
+
+	async test({ assert, component, target, window }) {
+		const button = target.querySelector('button');
+		const event = new window.MouseEvent('click');
+
+		await button.dispatchEvent(event);
+
+		assert.ok(!component.inner_clicked);
+	},
+};
diff --git a/test/runtime/samples/event-handler-dynamic-modifier-self/main.svelte b/test/runtime/samples/event-handler-dynamic-modifier-self/main.svelte
new file mode 100644
index 0000000000..b57d88ec02
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-modifier-self/main.svelte
@@ -0,0 +1,13 @@
+<script>
+	export let inner_clicked;
+	let f;
+
+	function handle_click(event) {
+		inner_clicked = true;
+	}
+	f = handle_click;
+</script>
+
+<div on:click|self={f}>
+	<button>click me</button>
+</div>
diff --git a/test/runtime/samples/event-handler-dynamic-modifier-stop-propagation/_config.js b/test/runtime/samples/event-handler-dynamic-modifier-stop-propagation/_config.js
new file mode 100644
index 0000000000..8517429e5c
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-modifier-stop-propagation/_config.js
@@ -0,0 +1,19 @@
+export default {
+	html: `
+		<div>
+			<button>click me</button>
+		</div>
+	`,
+
+	async test({ assert, component, target, window }) {
+		const button = target.querySelector('button');
+		const event = new window.MouseEvent('click', {
+			bubbles: true
+		});
+
+		await button.dispatchEvent(event);
+
+		assert.ok(component.inner_clicked);
+		assert.ok(!component.outer_clicked);
+	}
+};
diff --git a/test/runtime/samples/event-handler-dynamic-modifier-stop-propagation/main.svelte b/test/runtime/samples/event-handler-dynamic-modifier-stop-propagation/main.svelte
new file mode 100644
index 0000000000..bad7359927
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-modifier-stop-propagation/main.svelte
@@ -0,0 +1,20 @@
+<script>
+	export let inner_clicked;
+	export let outer_clicked;
+	let f1;
+	let f2;
+
+	function handle_inner_click(event) {
+		inner_clicked = true;
+	}
+
+	function handle_outer_click(event) {
+		outer_clicked = true;
+	}
+	f1 = handle_inner_click;
+	f2 = handle_outer_click;
+</script>
+
+<div on:click={f2}>
+	<button on:click|stopPropagation={f1}>click me</button>
+</div>
\ No newline at end of file
diff --git a/test/runtime/samples/event-handler-dynamic-multiple/_config.js b/test/runtime/samples/event-handler-dynamic-multiple/_config.js
new file mode 100644
index 0000000000..cf17c61f60
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-multiple/_config.js
@@ -0,0 +1,14 @@
+export default {
+	html: `
+		<button>click me</button>
+	`,
+
+	async test({ assert, component, target, window }) {
+		const button = target.querySelector('button');
+		const event = new window.MouseEvent('click');
+
+		await button.dispatchEvent(event);
+		assert.equal(component.clickHandlerOne, 1);
+		assert.equal(component.clickHandlerTwo, 1);
+	}
+};
diff --git a/test/runtime/samples/event-handler-dynamic-multiple/main.svelte b/test/runtime/samples/event-handler-dynamic-multiple/main.svelte
new file mode 100644
index 0000000000..2dbbe61ea6
--- /dev/null
+++ b/test/runtime/samples/event-handler-dynamic-multiple/main.svelte
@@ -0,0 +1,11 @@
+<script>
+	export let clickHandlerOne = 0;
+	export let clickHandlerTwo = 0;
+	let f1;
+	let f2;
+
+	f1 = () => clickHandlerOne++;
+	f2 = () => clickHandlerTwo++;
+</script>
+
+<button on:click='{f1}' on:click='{f2}'>click me</button>
diff --git a/test/runtime/samples/event-handler-dynamic/_config.js b/test/runtime/samples/event-handler-dynamic/_config.js
index 41cfd6e729..e60e561524 100644
--- a/test/runtime/samples/event-handler-dynamic/_config.js
+++ b/test/runtime/samples/event-handler-dynamic/_config.js
@@ -14,8 +14,14 @@ export default {
 		);
 
 		const event = new window.MouseEvent('click');
+		let err = "";
+		window.addEventListener('error', (e) => {
+			e.preventDefault();
+			err = e.message;
+		});
 
 		await button.dispatchEvent(event);
+		assert.equal(err, "", err);
 		assert.htmlEqual(target.innerHTML, `
 			<p>
 				<button>set handler 1</button>
@@ -24,7 +30,7 @@ export default {
 			<p>0</p>
 			<button>click</button>
 		`);
-		
+
 		await updateButton1.dispatchEvent(event);
 		await button.dispatchEvent(event);
 		assert.htmlEqual(target.innerHTML, `
@@ -35,7 +41,7 @@ export default {
 			<p>1</p>
 			<button>click</button>
 		`);
-		
+
 		await updateButton2.dispatchEvent(event);
 		await button.dispatchEvent(event);
 		assert.htmlEqual(target.innerHTML, `