Skip to content
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
20 changes: 19 additions & 1 deletion crates/perry-codegen/src/lower_string_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
};

Expand Down
5 changes: 5 additions & 0 deletions crates/perry-codegen/src/runtime_decls/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
27 changes: 27 additions & 0 deletions crates/perry-runtime/src/builtins/numbers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)`,
Expand Down
108 changes: 108 additions & 0 deletions crates/perry/tests/issue_string_method_nullish_receiver.rs
Original file line number Diff line number Diff line change
@@ -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() {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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");
}
Loading