From 53a650fddd736b236f2fecd0d41df0c6c4059159 Mon Sep 17 00:00:00 2001 From: daxpedda Date: Sat, 2 Mar 2024 10:17:22 +0100 Subject: [PATCH 1/2] Don't allow invalid Unicode scalar values in `char` --- CHANGELOG.md | 3 +++ crates/cli-support/src/js/binding.rs | 22 +++++++++++++++++++--- crates/cli-support/src/js/mod.rs | 13 +++++++++++++ src/convert/impls.rs | 1 + tests/wasm/char.js | 4 ++++ tests/wasm/char.rs | 6 ++++++ 6 files changed, 46 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4c87dd8df4..2c5a62321a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,9 @@ * Make .wasm output deterministic when using `--reference-types`. [#3851](https://github.com/rustwasm/wasm-bindgen/pull/3851) +* Don't allow invalid Unicode scalar values in `char`. + [#3866](https://github.com/rustwasm/wasm-bindgen/pull/3866) + -------------------------------------------------------------------------------- ## [0.2.91](https://github.com/rustwasm/wasm-bindgen/compare/0.2.90...0.2.91) diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs index 4c27d7584e4..ebe4e966c43 100644 --- a/crates/cli-support/src/js/binding.rs +++ b/crates/cli-support/src/js/binding.rs @@ -502,6 +502,11 @@ impl<'a, 'b> JsBuilder<'a, 'b> { self.prelude(&format!("_assertNonNull({});", arg)); } + fn assert_char(&mut self, arg: &str) { + self.cx.expose_assert_char(); + self.prelude(&format!("_assertChar({});", arg)); + } + fn assert_optional_bigint(&mut self, arg: &str) { if !self.cx.config.debug { return; @@ -739,7 +744,11 @@ fn instruction( Instruction::I32FromStringFirstChar => { let val = js.pop(); - js.push(format!("{}.codePointAt(0)", val)); + let i = js.tmp(); + js.prelude(&format!("const char{i} = {val}.codePointAt(0);")); + let val = format!("char{i}"); + js.assert_char(&val); + js.push(val); } Instruction::I32FromExternrefOwned => { @@ -816,11 +825,18 @@ fn instruction( Instruction::I32FromOptionChar => { let val = js.pop(); + let i = js.tmp(); js.cx.expose_is_like_none(); - js.push(format!( - "isLikeNone({0}) ? 0xFFFFFF : {0}.codePointAt(0)", + js.prelude(&format!( + "const char{i} = isLikeNone({0}) ? 0xFFFFFF : {0}.codePointAt(0);", val )); + let val = format!("char{i}"); + js.cx.expose_assert_char(); + js.prelude(&format!( + "if ({val} !== 0xFFFFFF) {{ _assertChar({val}); }}" + )); + js.push(val); } Instruction::I32FromOptionEnum { hole } => { diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 9a3899c9266..618b4282dc9 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -2116,6 +2116,19 @@ impl<'a> Context<'a> { ); } + fn expose_assert_char(&mut self) { + if !self.should_write_global("assert_char") { + return; + } + self.global( + " + function _assertChar(c) { + if (typeof(c) !== 'number' || c >= 0x110000 || (c >= 0xD800 && c < 0xE000)) throw new Error(`expected a number argument that is a valid Unicode scalar value, found ${c}`); + } + ", + ); + } + fn expose_make_mut_closure(&mut self) -> Result<(), Error> { if !self.should_write_global("make_mut_closure") { return Ok(()); diff --git a/src/convert/impls.rs b/src/convert/impls.rs index 8d2bf4baf6a..4ecef7821ef 100644 --- a/src/convert/impls.rs +++ b/src/convert/impls.rs @@ -188,6 +188,7 @@ impl FromWasmAbi for char { #[inline] unsafe fn from_abi(js: u32) -> char { + // SAFETY: Checked in bindings. char::from_u32_unchecked(js) } } diff --git a/tests/wasm/char.js b/tests/wasm/char.js index d145333e381..c9e676023e0 100644 --- a/tests/wasm/char.js +++ b/tests/wasm/char.js @@ -2,6 +2,7 @@ const wasm = require('wasm-bindgen-test.js'); const assert = require('assert'); exports.js_identity = a => a; +exports.js_option_identity = a => a; exports.js_works = () => { assert.strictEqual(wasm.letter(), 'a'); @@ -14,4 +15,7 @@ exports.js_works = () => { assert.strictEqual(wasm.rust_js_identity('㊻'), '㊻'); wasm.rust_letter('a'); wasm.rust_face('😀'); + + assert.throws(() => wasm.rust_js_identity('\uD83D'), /expected a number argument that is a valid Unicode scalar value, found 55357/); + assert.throws(() => wasm.rust_js_option_identity('\uD83D'), /expected a number argument that is a valid Unicode scalar value, found 55357/); }; diff --git a/tests/wasm/char.rs b/tests/wasm/char.rs index f852b8d8145..b23dd94306d 100644 --- a/tests/wasm/char.rs +++ b/tests/wasm/char.rs @@ -4,6 +4,7 @@ use wasm_bindgen_test::*; #[wasm_bindgen(module = "tests/wasm/char.js")] extern "C" { fn js_identity(c: char) -> char; + fn js_option_identity(c: Option) -> Option; fn js_works(); } @@ -17,6 +18,11 @@ pub fn rust_js_identity(c: char) -> char { js_identity(c) } +#[wasm_bindgen] +pub fn rust_js_option_identity(c: Option) -> Option { + js_option_identity(c) +} + #[wasm_bindgen] pub fn letter() -> char { 'a' From 03d0533ddf85f74a1798f3666829c4d6a159727c Mon Sep 17 00:00:00 2001 From: daxpedda Date: Sat, 2 Mar 2024 14:48:49 +0100 Subject: [PATCH 2/2] Account for empty string --- crates/cli-support/src/js/mod.rs | 2 +- tests/wasm/char.js | 12 +++++++++--- tests/wasm/char.rs | 9 ++++----- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 618b4282dc9..35bba115fdc 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -2123,7 +2123,7 @@ impl<'a> Context<'a> { self.global( " function _assertChar(c) { - if (typeof(c) !== 'number' || c >= 0x110000 || (c >= 0xD800 && c < 0xE000)) throw new Error(`expected a number argument that is a valid Unicode scalar value, found ${c}`); + if (typeof(c) === 'number' && (c >= 0x110000 || (c >= 0xD800 && c < 0xE000))) throw new Error(`expected a valid Unicode scalar value, found ${c}`); } ", ); diff --git a/tests/wasm/char.js b/tests/wasm/char.js index c9e676023e0..b1ee622808f 100644 --- a/tests/wasm/char.js +++ b/tests/wasm/char.js @@ -2,11 +2,11 @@ const wasm = require('wasm-bindgen-test.js'); const assert = require('assert'); exports.js_identity = a => a; -exports.js_option_identity = a => a; exports.js_works = () => { assert.strictEqual(wasm.letter(), 'a'); assert.strictEqual(wasm.face(), '😀'); + assert.strictEqual(wasm.rust_identity(''), '\u0000'); assert.strictEqual(wasm.rust_identity('Ղ'), 'Ղ'); assert.strictEqual(wasm.rust_identity('ҝ'), 'ҝ'); assert.strictEqual(wasm.rust_identity('Δ'), 'Δ'); @@ -16,6 +16,12 @@ exports.js_works = () => { wasm.rust_letter('a'); wasm.rust_face('😀'); - assert.throws(() => wasm.rust_js_identity('\uD83D'), /expected a number argument that is a valid Unicode scalar value, found 55357/); - assert.throws(() => wasm.rust_js_option_identity('\uD83D'), /expected a number argument that is a valid Unicode scalar value, found 55357/); + assert.strictEqual(wasm.rust_option_identity(undefined), undefined); + assert.strictEqual(wasm.rust_option_identity(null), undefined); + assert.strictEqual(wasm.rust_option_identity(''), '\u0000'); + assert.strictEqual(wasm.rust_option_identity('\u0000'), '\u0000'); + + assert.throws(() => wasm.rust_identity(55357), /c.codePointAt is not a function/); + assert.throws(() => wasm.rust_identity('\uD83D'), /expected a valid Unicode scalar value, found 55357/); + assert.throws(() => wasm.rust_option_identity('\uD83D'), /expected a valid Unicode scalar value, found 55357/); }; diff --git a/tests/wasm/char.rs b/tests/wasm/char.rs index b23dd94306d..d680a1cae43 100644 --- a/tests/wasm/char.rs +++ b/tests/wasm/char.rs @@ -4,7 +4,6 @@ use wasm_bindgen_test::*; #[wasm_bindgen(module = "tests/wasm/char.js")] extern "C" { fn js_identity(c: char) -> char; - fn js_option_identity(c: Option) -> Option; fn js_works(); } @@ -14,13 +13,13 @@ pub fn rust_identity(c: char) -> char { } #[wasm_bindgen] -pub fn rust_js_identity(c: char) -> char { - js_identity(c) +pub fn rust_option_identity(c: Option) -> Option { + c } #[wasm_bindgen] -pub fn rust_js_option_identity(c: Option) -> Option { - js_option_identity(c) +pub fn rust_js_identity(c: char) -> char { + js_identity(c) } #[wasm_bindgen]