diff --git a/crates/perry-codegen/src/lower_string_method.rs b/crates/perry-codegen/src/lower_string_method.rs index ff7d572011..6a887f6db0 100644 --- a/crates/perry-codegen/src/lower_string_method.rs +++ b/crates/perry-codegen/src/lower_string_method.rs @@ -129,8 +129,26 @@ pub(crate) fn lower_string_method( let recv_box = if is_string_expr(ctx, object) { recv_box } else { + // A nullish receiver must throw the V8 member-access TypeError — + // `undefined.charAt(0)` reads `charAt` off `undefined` FIRST (ECMA-262 + // §13.3), so it throws `Cannot read properties of undefined (reading + // 'charAt')` — NOT coerce `undefined`→`"undefined"` like the general + // `js_string_coerce`. The guarded helper does RequireObjectCoercible + + // ToString; pass the method name for the diagnostic. + let prop_idx = ctx.strings.intern(property); + let (prop_global, prop_len) = { + let entry = ctx.strings.entry(prop_idx); + ( + format!("@{}", entry.bytes_global), + entry.byte_len.to_string(), + ) + }; let blk = ctx.block(); - let coerced = blk.call(I64, "js_string_coerce", &[(DOUBLE, &recv_box)]); + let coerced = blk.call( + I64, + "js_string_coerce_method_this", + &[(DOUBLE, &recv_box), (PTR, &prop_global), (I64, &prop_len)], + ); nanbox_string_inline(blk, &coerced) }; diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 55e10fa239..7097664f13 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -373,6 +373,11 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { module.declare_function("js_math_min2", DOUBLE, &[DOUBLE, DOUBLE]); module.declare_function("js_math_max2", DOUBLE, &[DOUBLE, DOUBLE]); module.declare_function("js_string_coerce", I64, &[DOUBLE]); + // RequireObjectCoercible + ToString for inline-lowered String.prototype + // methods on a non-string receiver: a nullish `this` throws the V8 + // member-access TypeError instead of coercing undefined→"undefined". + // Args: (value, prop_name_ptr, prop_name_len). + module.declare_function("js_string_coerce_method_this", I64, &[DOUBLE, PTR, I64]); module.declare_function("js_array_slice", I64, &[I64, I32, I32]); module.declare_function("js_array_slice_values", I64, &[I64, DOUBLE, DOUBLE]); module.declare_function("js_array_shift_f64", DOUBLE, &[I64]); diff --git a/crates/perry-runtime/src/builtins/numbers.rs b/crates/perry-runtime/src/builtins/numbers.rs index 02bdb0799b..821c4ec0fc 100644 --- a/crates/perry-runtime/src/builtins/numbers.rs +++ b/crates/perry-runtime/src/builtins/numbers.rs @@ -673,6 +673,33 @@ pub extern "C" fn js_string_coerce(value: f64) -> *mut StringHeader { js_string_from_bytes(result.as_ptr(), result.len() as u32) } +/// `RequireObjectCoercible(this)` + `ToString(this)` for the inline-lowered +/// `String.prototype` methods (`charAt` / `charCodeAt` / `codePointAt` / +/// `split` / `toUpperCase` / …) when the receiver is NOT statically +/// string-typed. `codegen`'s `lower_string_method` optimistically routes any +/// receiver here, but a nullish receiver must throw the V8 member-access +/// `TypeError` — `x.charAt(0)` reads `charAt` off `x` FIRST (ECMA-262 §13.3), +/// so `undefined`/`null` throws `Cannot read properties of undefined (reading +/// 'charAt')` — NOT coerce `undefined`→`"undefined"` like the general +/// `js_string_coerce` used for `String(x)` / `'' + x`. `prop_name_*` carries +/// the static method name for the diagnostic. +#[no_mangle] +pub extern "C" fn js_string_coerce_method_this( + value: f64, + prop_name_ptr: *const u8, + prop_name_len: usize, +) -> *mut StringHeader { + let jsval = JSValue::from_bits(value.to_bits()); + if jsval.is_undefined() || jsval.is_null() { + crate::error::js_throw_type_error_property_access( + jsval.is_null() as u32, + prop_name_ptr, + prop_name_len, + ); + } + js_string_coerce(value) +} + /// Spec `ToString` (ECMA-262 §7.1.17) rejects a Symbol with a `TypeError`. /// The lenient `js_string_coerce` / `js_jsvalue_to_string` paths instead /// produce a `"Symbol(desc)"` descriptive string — correct for `String(sym)`, diff --git a/crates/perry/tests/issue_string_method_nullish_receiver.rs b/crates/perry/tests/issue_string_method_nullish_receiver.rs new file mode 100644 index 0000000000..66a80f82ae --- /dev/null +++ b/crates/perry/tests/issue_string_method_nullish_receiver.rs @@ -0,0 +1,108 @@ +//! Inline-lowered `String.prototype` methods on a nullish receiver coerced the +//! receiver to `"undefined"` / `"null"` instead of throwing the member-access +//! `TypeError`. +//! +//! `codegen`'s `lower_string_method` optimistically routes an any-typed +//! receiver (`(x: any).charAt(i)` / `.codePointAt(i)` / `.split(s)` / …) through +//! the inline string helpers, applying `ToString(this)` for a non-string value. +//! But `ToString` maps `undefined`→`"undefined"` and `null`→`"null"`, so +//! `undefined.codePointAt(0)` returned `117` (`"undefined".codePointAt(0)`) and +//! `undefined.toUpperCase()` returned `"UNDEFINED"` — where V8 throws +//! `Cannot read properties of undefined (reading 'codePointAt')`, because +//! `x.codePointAt` reads the method off `x` FIRST (ECMA-262 §13.3, evaluated +//! before the call). The general property-get path (used for e.g. `slice`, +//! `indexOf`) already threw correctly; the inline char-access/case/split path +//! skipped the `RequireObjectCoercible` guard. +//! +//! Fix: the coercion branch calls `js_string_coerce_method_this`, which does +//! `RequireObjectCoercible(this)` (throwing the V8 member-access message with +//! the static method name) before `ToString`. A statically string-typed +//! receiver still skips the guard (fast path). + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(dir: &std::path::Path, source: &str) -> String { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output).output().expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary exited non-zero: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).trim().to_string() +} + +#[test] +fn string_method_on_nullish_receiver_throws_member_access_type_error() { + let dir = tempfile::tempdir().expect("tempdir"); + let out = compile_and_run( + dir.path(), + r#" +// `any`-typed receivers read out of a data structure, mirroring the real +// failure shape (`cell.value.codePointAt(0)` where `cell.value` is undefined). +const cells: { value: any }[] = [{ value: undefined }, { value: null }, { value: "abc" }]; + +function attempt(label: string, fn: () => any): string { + try { + const r = fn(); + return label + "=NOTHROW:" + String(r); + } catch (e: any) { + return label + "=" + e.message; + } +} + +const lines = [ + attempt("u.codePointAt", () => cells[0].value.codePointAt(0)), + attempt("u.charCodeAt", () => cells[0].value.charCodeAt(0)), + attempt("u.charAt", () => cells[0].value.charAt(0)), + attempt("u.toUpperCase", () => cells[0].value.toUpperCase()), + attempt("u.split", () => cells[0].value.split("")), + attempt("n.codePointAt", () => cells[1].value.codePointAt(0)), + attempt("n.charAt", () => cells[1].value.charAt(0)), + // A real string receiver still works (no over-eager guard). + attempt("ok.charAt", () => cells[2].value.charAt(1)), + attempt("ok.codePointAt", () => cells[2].value.codePointAt(0)), +]; + +process.stdout.write(lines.join("\n") + "\n"); +"#, + ); + let expected = [ + "u.codePointAt=Cannot read properties of undefined (reading 'codePointAt')", + "u.charCodeAt=Cannot read properties of undefined (reading 'charCodeAt')", + "u.charAt=Cannot read properties of undefined (reading 'charAt')", + "u.toUpperCase=Cannot read properties of undefined (reading 'toUpperCase')", + "u.split=Cannot read properties of undefined (reading 'split')", + "n.codePointAt=Cannot read properties of null (reading 'codePointAt')", + "n.charAt=Cannot read properties of null (reading 'charAt')", + "ok.charAt=NOTHROW:b", + "ok.codePointAt=NOTHROW:97", + ] + .join("\n"); + assert_eq!(out, expected, "string method on nullish receiver"); +}