diff --git a/changelog.d/10644-buffer-prototype-shape.md b/changelog.d/10644-buffer-prototype-shape.md new file mode 100644 index 0000000000..882862efac --- /dev/null +++ b/changelog.d/10644-buffer-prototype-shape.md @@ -0,0 +1,3 @@ +### Fixed + +- `Buffer.prototype`'s own property set no longer contains 36 bogus entries (including two bare string literals, `"function"` and `"undefined"`, plus `DataView`/`Uint8Array.prototype`/`Object.prototype` methods that belong further up the prototype chain) and now includes Node's internal `Slice`/`Write` methods and the deprecated `offset`/`parent` accessors, matching `Object.getOwnPropertyNames(Buffer.prototype)` on Node 26.5.1 exactly (96 own properties, 95 enumerable via `for-in`). diff --git a/crates/perry-runtime/src/buffer/copy_write.rs b/crates/perry-runtime/src/buffer/copy_write.rs index 4fbed786f1..20315d64b8 100644 --- a/crates/perry-runtime/src/buffer/copy_write.rs +++ b/crates/perry-runtime/src/buffer/copy_write.rs @@ -105,6 +105,12 @@ pub extern "C" fn js_buffer_write_len( let bytes_to_write = match encoding { 1 => decode_hex(str_bytes), 2 | 3 => decode_base64(str_bytes), + // #10426: encoding tag 6 (utf16le/ucs2) fell through to the + // default (raw UTF-8 bytes) arm — dormant in the pre-existing + // `buf.write(str, offset, 'utf16le')` path and would have made + // the new `ucs2Write` method equally wrong. `from::utf16le_string_bytes` + // is the same UTF-16LE encoder `Buffer.from(str, 'utf16le')` uses. + 6 => super::from::utf16le_string_bytes(str_bytes), _ => str_bytes.to_vec(), }; diff --git a/crates/perry-runtime/src/buffer/from.rs b/crates/perry-runtime/src/buffer/from.rs index 16ed61c8dd..50725ce64d 100644 --- a/crates/perry-runtime/src/buffer/from.rs +++ b/crates/perry-runtime/src/buffer/from.rs @@ -110,7 +110,11 @@ fn latin1_string_bytes(str_bytes: &[u8]) -> Vec { out } -fn utf16le_string_bytes(str_bytes: &[u8]) -> Vec { +/// #10426: made `pub(crate)` so `copy_write.rs`'s `js_buffer_write_len` +/// can reuse it for the new `ucs2Write` method (and to close the same gap +/// in the pre-existing generic `buf.write(str, offset, 'utf16le')` path, +/// which silently wrote raw UTF-8 bytes instead of UTF-16LE before this). +pub(crate) fn utf16le_string_bytes(str_bytes: &[u8]) -> Vec { let decoded = String::from_utf8_lossy(str_bytes); let mut out = Vec::with_capacity(decoded.len() * 2); for unit in decoded.encode_utf16() { diff --git a/crates/perry-runtime/src/object/buffer_dispatch.rs b/crates/perry-runtime/src/object/buffer_dispatch.rs index ae1fcc2a72..0e5a90280c 100644 --- a/crates/perry-runtime/src/object/buffer_dispatch.rs +++ b/crates/perry-runtime/src/object/buffer_dispatch.rs @@ -243,8 +243,43 @@ buffer_method_names!( "getBigUint64", "setBigInt64", "setBigUint64", + // #10426: Node's internal fixed-encoding slice/write pair, present as + // real own methods on `Buffer.prototype` (`buf.write(str, off, enc)` / + // `buf.toString(enc, start, end)` dispatch through these internally in + // Node; Perry exposes the same names so duck-typed reads and direct + // calls both work, matching `Object.getOwnPropertyNames(Buffer.prototype)`). + "asciiSlice", + "asciiWrite", + "base64Slice", + "base64Write", + "base64urlSlice", + "base64urlWrite", + "hexSlice", + "hexWrite", + "latin1Slice", + "latin1Write", + "ucs2Slice", + "ucs2Write", + "utf8Slice", + "utf8Write", ); +/// Fixed encoding tag for one of Node's internal `Buffer.prototype` +/// `Slice`/`Write` methods (#10426). Tags match +/// `js_encoding_tag_from_value`'s numbering (0=utf8 … 6=utf16le/ucs2). +fn fixed_slice_write_encoding(method_name: &str) -> Option { + Some(match method_name { + "utf8Slice" | "utf8Write" => 0, + "hexSlice" | "hexWrite" => 1, + "base64Slice" | "base64Write" => 2, + "base64urlSlice" | "base64urlWrite" => 3, + "latin1Slice" | "latin1Write" => 4, + "asciiSlice" | "asciiWrite" => 5, + "ucs2Slice" | "ucs2Write" => 6, + _ => return None, + }) +} + unsafe fn buffer_secret_export_format(bits: f64) -> Option { let raw = bits.to_bits(); if (raw >> 48) as u16 == 0x7FFC { @@ -680,6 +715,44 @@ pub unsafe fn dispatch_buffer_method( crate::buffer::js_buffer_copy(buf_ptr, dst_ptr, target_start, source_start, source_end) as f64 } + // #10426: Node's internal `Slice(start, end)` / + // `Write(string, offset, length)` pair — the same + // operation as `toString(encoding, start, end)` / `write(string, + // offset, length, encoding)` with the encoding fixed by the method + // name instead of an argument. + "asciiSlice" | "base64Slice" | "base64urlSlice" | "hexSlice" | "latin1Slice" + | "ucs2Slice" | "utf8Slice" => { + let enc = fixed_slice_write_encoding(method_name).unwrap_or(0); + let len = (*buf_ptr).length as i32; + let start = if !args.is_empty() { arg_i32(0) } else { 0 }; + let end = if args.len() >= 2 { arg_i32(1) } else { len }; + let str_ptr = crate::buffer::js_buffer_to_string_range(buf_ptr, enc, start, end); + f64::from_bits(JSValue::string_ptr(str_ptr).bits()) + } + "asciiWrite" | "base64Write" | "base64urlWrite" | "hexWrite" | "latin1Write" + | "ucs2Write" | "utf8Write" => { + if args.is_empty() || !is_buffer_dispatch_string(args[0]) { + throw_buffer_type_error_with_code( + "argument must be a string", + "ERR_INVALID_ARG_TYPE", + ); + } + let enc = fixed_slice_write_encoding(method_name).unwrap_or(0); + let str_bits = args[0].to_bits(); + let str_addr = if (str_bits >> 48) >= 0x7FF8 { + str_bits & 0x0000_FFFF_FFFF_FFFF + } else { + str_bits + }; + let str_ptr = str_addr as *const crate::string::StringHeader; + let offset = if args.len() >= 2 { arg_i32(1) } else { 0 }; + let max_len = if args.len() >= 3 { + arg_i32(2) + } else { + (*buf_ptr).length as i32 - offset + }; + crate::buffer::js_buffer_write_len(buf_ptr, str_ptr, offset, max_len, enc) as f64 + } "toJSON" => crate::buffer::js_buffer_to_json(buf_f64), // `buf.write(string, offset?, length?, encoding?)` — writes the // utf8/hex/base64 encoding of `string` into `buf` at `offset`. diff --git a/crates/perry-runtime/src/object/native_module/callable_exports.rs b/crates/perry-runtime/src/object/native_module/callable_exports.rs index 73fc029d8f..dfa21cbac7 100644 --- a/crates/perry-runtime/src/object/native_module/callable_exports.rs +++ b/crates/perry-runtime/src/object/native_module/callable_exports.rs @@ -426,6 +426,83 @@ extern "C" fn buffer_prototype_method_thunk(_closure: *const crate::closure::Clo f64::from_bits(crate::value::TAG_UNDEFINED) } +/// `this` for a `Buffer.prototype.offset`/`.parent` accessor read, resolved +/// through `IMPLICIT_THIS` (the same mechanism `require_webcrypto_this` in +/// `ctor_thunks.rs` uses for Web Crypto getters). `None` for a non-buffer +/// receiver — Node's real getters answer `undefined` rather than throwing +/// (`isInstance(this, Buffer) ? … : undefined`), and ordinary Buffer/typed- +/// array instance reads never reach this getter at all (they resolve +/// `.offset`/`.parent` directly — see `get_field_by_name_tail.rs`); this only +/// matters for reflection (`Object.getOwnPropertyDescriptor(Buffer.prototype, +/// "offset").get.call(x)`) and enumeration. +fn buffer_prototype_this_addr() -> Option { + let this_bits = crate::object::IMPLICIT_THIS.with(|c| c.get()); + let jv = crate::value::JSValue::from_bits(this_bits); + if !jv.is_pointer() { + return None; + } + let addr = (this_bits & crate::value::POINTER_MASK) as usize; + if addr == 0 || crate::buffer::js_buffer_is_buffer(addr as i64) == 0 { + return None; + } + Some(addr) +} + +/// #10426: `Buffer.prototype.parent` — deprecated legacy alias for +/// `.buffer` (the backing `ArrayBuffer`), still a real own accessor on +/// Node's `Buffer.prototype`. +extern "C" fn buffer_prototype_parent_getter_thunk( + _closure: *const crate::closure::ClosureHeader, +) -> f64 { + match buffer_prototype_this_addr() { + Some(addr) => { + crate::value::js_nanbox_pointer(crate::buffer::buffer_backing_array_buffer(addr) as i64) + } + None => f64::from_bits(crate::value::TAG_UNDEFINED), + } +} + +/// #10426: `Buffer.prototype.offset` — deprecated legacy alias for +/// `.byteOffset`, still a real own accessor on Node's `Buffer.prototype`. +extern "C" fn buffer_prototype_offset_getter_thunk( + _closure: *const crate::closure::ClosureHeader, +) -> f64 { + match buffer_prototype_this_addr() { + Some(addr) => crate::buffer::buffer_byte_offset(addr) as f64, + None => f64::from_bits(crate::value::TAG_UNDEFINED), + } +} + +/// Install a `Buffer.prototype` accessor (`offset`/`parent`) — `{ +/// enumerable: true, configurable: false }`, matching Node's +/// `ObjectDefineProperty(Buffer.prototype, name, { enumerable: true, get() +/// {…} })` (no `configurable: true`, so it defaults false). Mirrors +/// `install_webcrypto_proto_getter`'s shape for a `*mut ObjectHeader` proto. +fn install_buffer_prototype_getter(proto_obj: *mut ObjectHeader, name: &str, func_ptr: *const u8) { + if proto_obj.is_null() { + return; + } + crate::closure::js_register_closure_arity(func_ptr, 0); + let closure = crate::closure::js_closure_alloc(func_ptr, 0); + let value = if closure.is_null() { + f64::from_bits(crate::value::TAG_UNDEFINED) + } else { + set_bound_native_closure_name(closure, &format!("get {name}")); + crate::value::js_nanbox_pointer(closure as i64) + }; + let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + js_object_set_field_by_name(proto_obj, key, f64::from_bits(crate::value::TAG_UNDEFINED)); + super::set_builtin_accessor_descriptor( + proto_obj as usize, + name.to_string(), + super::AccessorDescriptor { + get: value.to_bits(), + set: 0, + }, + super::PropertyAttrs::new(true, true, false), + ); +} + const BUFFER_STATIC_METHODS: &[&str] = &[ "from", "alloc", @@ -440,22 +517,40 @@ const BUFFER_STATIC_METHODS: &[&str] = &[ "copyBytesFrom", ]; -/// Node exposes the WHOLE Buffer method surface on `Buffer.prototype`, and it is -/// enumerable — `for (const k in Buffer.prototype)` yields ~93 names there. -/// Perry used to install ELEVEN, which quietly broke any code that walks the -/// prototype: mysql2 sizes every outgoing packet by no-op'ing the write methods -/// of a zero-length Buffer +/// Node's actual `Buffer.prototype` own-property surface — checked +/// property-by-property against `Object.getOwnPropertyNames(Buffer.prototype)` +/// on Node 26.5.1 (96 names there; 93 real callable methods here, plus +/// `constructor` and the `offset`/`parent` accessors installed separately +/// below = 96). Perry used to install ELEVEN, which quietly broke any code +/// that walks the prototype: mysql2 sizes every outgoing packet by no-op'ing +/// the write methods of a zero-length Buffer /// (`for (const k in Buffer.prototype) if (typeof mock[k] === "function") mock[k] = noop`), /// so `writeUInt32LE` — absent from the stub list — stayed live, wrote into the /// empty measuring buffer, and killed the MySQL handshake with -/// RangeError [ERR_OUT_OF_RANGE]. Generated from the dispatcher's own -/// `is_buffer_method_name` table so the two can't drift. +/// RangeError [ERR_OUT_OF_RANGE]. +/// +/// #10426: this list is DELIBERATELY NOT the same as +/// `buffer_dispatch::is_buffer_method_name` (a comment here used to say it +/// was "generated from" that table "so the two can't drift" — that coupling +/// was the bug). `is_buffer_method_name` answers a different question — "does +/// a property read on a Buffer *instance* need to synthesize a bound-method +/// closure" — and is deliberately broad: it also recognizes names Buffer +/// instances answer only by INHERITANCE (`Uint8Array.prototype.at`/`set`/ +/// `entries`/`keys`/`values`/`copyWithin`/`toBase64`/`toHex`/`setFromBase64`/ +/// `setFromHex`), by DataView accessors on a DataView-marked buffer +/// (`getInt32`/`setFloat64`/…), and by `Object.prototype` +/// (`hasOwnProperty`/`isPrototypeOf`/`propertyIsEnumerable`/`valueOf`) so +/// duck-type probes on an INSTANCE keep working (see that table's own +/// comments). None of those belong on `Buffer.prototype` itself as OWN +/// properties — Node inherits them further up the chain — so installing this +/// list from that one copied 36 names Node never puts here (plus two bare +/// string literals, `"function"` and `"undefined"`, that had drifted in from +/// nearby prose/JS-idiom comments and were never real method names at all). const BUFFER_PROTOTYPE_METHODS: &[&str] = &[ "toString", "inspect", "slice", "subarray", - "set", "copy", "write", "toJSON", @@ -465,18 +560,9 @@ const BUFFER_PROTOTYPE_METHODS: &[&str] = &[ "indexOf", "lastIndexOf", "includes", - "at", "swap16", "swap32", "swap64", - "values", - "keys", - "entries", - "undefined", - "hasOwnProperty", - "propertyIsEnumerable", - "valueOf", - "isPrototypeOf", "toLocaleString", "readUInt8", "readUint8", @@ -540,32 +626,20 @@ const BUFFER_PROTOTYPE_METHODS: &[&str] = &[ "writeUintLE", "writeIntBE", "writeIntLE", - "toBase64", - "toHex", - "setFromBase64", - "setFromHex", - "copyWithin", - "function", - "getInt8", - "getUint8", - "getInt16", - "getUint16", - "getInt32", - "getUint32", - "getFloat32", - "getFloat64", - "setInt8", - "setUint8", - "setInt16", - "setUint16", - "setInt32", - "setUint32", - "setFloat32", - "setFloat64", - "getBigInt64", - "getBigUint64", - "setBigInt64", - "setBigUint64", + "asciiSlice", + "asciiWrite", + "base64Slice", + "base64Write", + "base64urlSlice", + "base64urlWrite", + "hexSlice", + "hexWrite", + "latin1Slice", + "latin1Write", + "ucs2Slice", + "ucs2Write", + "utf8Slice", + "utf8Write", ]; const SQLITE_DATABASE_SYNC_PROTOTYPE_METHODS: &[&str] = &[ @@ -950,6 +1024,18 @@ pub(crate) fn buffer_constructor_value() -> f64 { }) }); } + proto.with_mut_ptr(|proto: *mut ObjectHeader| { + install_buffer_prototype_getter( + proto, + "parent", + buffer_prototype_parent_getter_thunk as *const u8, + ); + install_buffer_prototype_getter( + proto, + "offset", + buffer_prototype_offset_getter_thunk as *const u8, + ); + }); let proto_value = proto.with_mut_ptr(|proto: *mut ObjectHeader| { crate::value::js_nanbox_pointer(proto as i64) }); diff --git a/test-files/test_gap_10426_buffer_prototype_shape.ts b/test-files/test_gap_10426_buffer_prototype_shape.ts new file mode 100644 index 0000000000..dda95f3468 --- /dev/null +++ b/test-files/test_gap_10426_buffer_prototype_shape.ts @@ -0,0 +1,163 @@ +// #10426: `Buffer.prototype`'s own (enumerable) property set had 36 bogus +// entries — including two bare string literals ("function"/"undefined" that +// had drifted in from nearby prose/JS-idiom comments, never real method +// names), plus DataView accessors, `Uint8Array.prototype`/TC39 base64-hex +// methods, and `Object.prototype` methods that all belong further up the +// prototype chain, not as OWN properties of Buffer.prototype — and was +// missing 16 of Node's own members: the 14 internal `Slice`/ +// `Write` methods and the deprecated `offset`/`parent` accessors. +// Root cause: `BUFFER_PROTOTYPE_METHODS` in +// crates/perry-runtime/src/object/native_module/callable_exports.rs was +// generated from the SAME name table `buffer_dispatch::is_buffer_method_name` +// uses to decide whether a property READ on a Buffer *instance* should +// synthesize a bound-method closure (deliberately broad, for duck-typed +// inherited-method reads) — conflating that broad instance-read predicate +// with the narrow set of names that should be Buffer.prototype's own keys. +import { Buffer } from "node:buffer"; + +// The issue's own repro, verbatim (own/for-in counts + full sorted list). +const own = Object.getOwnPropertyNames(Buffer.prototype).sort(); +const forin: string[] = []; +for (const k in Buffer.prototype) forin.push(k); +console.log("own", own.length, "for-in", forin.length); +console.log("has 'function':", own.includes("function"), "has 'undefined':", own.includes("undefined")); +console.log(own.join(" ")); + +// Descriptor shape for a representative sample of each own-property kind: +// a non-enumerable data method (constructor), ordinary enumerable data +// methods (old + new), and the two new accessor properties. +function describe(name: string) { + const d = Object.getOwnPropertyDescriptor(Buffer.prototype, name); + if (!d) { + console.log(name, "MISSING"); + return; + } + console.log( + name, + JSON.stringify({ + writable: d.writable, + enumerable: d.enumerable, + configurable: d.configurable, + hasGet: typeof d.get === "function", + hasSet: typeof d.set === "function", + isFn: typeof d.value === "function", + }), + ); +} +for (const name of [ + "constructor", + "write", + "toString", + "toLocaleString", + "offset", + "parent", + "utf8Slice", + "utf8Write", + "hexSlice", + "hexWrite", + "base64Slice", + "base64urlSlice", + "asciiSlice", + "latin1Slice", + "ucs2Slice", +]) { + describe(name); +} + +// The bogus keys must be gone as OWN properties (still reachable as +// duck-typed INSTANCE reads through is_buffer_method_name, checked below — +// that's a distinct, intentionally-broader mechanism this fix doesn't touch). +console.log("own has 'at':", own.includes("at")); +console.log("own has 'set':", own.includes("set")); +console.log("own has 'hasOwnProperty':", own.includes("hasOwnProperty")); +console.log("own has 'toBase64':", own.includes("toBase64")); +console.log("own has 'getInt32':", own.includes("getInt32")); + +// mysql2's own motivating idiom: no-op every function-typed key found via +// for-in on Buffer.prototype, on a zero-length Buffer. +const mock = Buffer.alloc(0); +let noopCount = 0; +for (const k of forin) { + if (typeof (mock as any)[k] === "function") { + (mock as any)[k] = () => {}; + noopCount++; + } +} +console.log("noopCount:", noopCount); + +// Functional round-trips for the 14 newly-added internal methods, on a +// fixed, deterministic byte source ("Hello"). +const src = Buffer.from([0x48, 0x65, 0x6c, 0x6c, 0x6f]); +console.log("utf8Slice:", src.utf8Slice(0, 5)); +console.log("asciiSlice:", src.asciiSlice(1, 4)); +console.log("latin1Slice:", src.latin1Slice(0, 5)); +console.log("hexSlice:", src.hexSlice(0, 5)); +console.log("base64Slice:", src.base64Slice(0, 5)); +console.log("base64urlSlice:", src.base64urlSlice(0, 5)); + +const u16 = Buffer.from("Hi", "utf16le"); +console.log("ucs2Slice:", u16.ucs2Slice(0, 4)); + +// base64url-distinguishing bytes (produces '+'/'/' in base64, '-'/'_' in +// base64url) round-tripped through both Slice and Write. +const urlish = Buffer.from([0xfb, 0xff, 0xbf]); +console.log("base64Slice (urlish):", urlish.base64Slice(0, 3)); +console.log("base64urlSlice (urlish):", urlish.base64urlSlice(0, 3)); + +const w1 = Buffer.alloc(8, 0); +console.log("utf8Write:", w1.utf8Write("Hi!", 1)); +console.log("utf8Write result:", w1.toString("hex")); + +const w2 = Buffer.alloc(8, 0); +console.log("hexWrite:", w2.hexWrite("48656c6c6f", 0)); +console.log("hexWrite result:", w2.toString("utf8", 0, 5)); + +const w3 = Buffer.alloc(8, 0); +console.log("base64Write:", w3.base64Write("SGVsbG8=", 0)); +console.log("base64Write result:", w3.toString("utf8", 0, 5)); + +const w4 = Buffer.alloc(8, 0); +console.log("base64urlWrite:", w4.base64urlWrite("SGVsbG8", 0)); +console.log("base64urlWrite result:", w4.toString("utf8", 0, 5)); + +const w5 = Buffer.alloc(8, 0); +console.log("asciiWrite:", w5.asciiWrite("Hi", 2)); +console.log("asciiWrite result:", w5.toString("ascii", 2, 4)); + +const w6 = Buffer.alloc(8, 0); +console.log("latin1Write:", w6.latin1Write("Hi", 0)); +console.log("latin1Write result:", w6.toString("latin1", 0, 2)); + +const w7 = Buffer.alloc(8, 0); +console.log("ucs2Write:", w7.ucs2Write("Hi", 0)); +console.log("ucs2Write result:", w7.toString("utf16le", 0, 4)); + +// Cross-check against the existing generic `toString(encoding, start, end)` +// / `write(string, offset, length, encoding)` paths these delegate to — must +// agree exactly. +console.log("utf8Slice === toString(utf8):", src.utf8Slice(0, 5) === src.toString("utf8", 0, 5)); +console.log("hexSlice === toString(hex):", src.hexSlice(0, 5) === src.toString("hex", 0, 5)); + +// `offset`/`parent` on a real instance (already correctly handled at the +// instance level before this fix — kept as a regression control) and a +// subarray with a non-zero byteOffset. +const backing = Buffer.alloc(16); +const view = backing.subarray(4, 10); +console.log("view.offset:", (view as any).offset, "view.byteOffset:", view.byteOffset); +console.log("view.parent === view.buffer:", (view as any).parent === view.buffer); + +// The new accessor's `get`, invoked directly off Buffer.prototype (the +// reflection path the accessor descriptor itself exists for). +const offsetGetter = Object.getOwnPropertyDescriptor(Buffer.prototype, "offset")!.get!; +console.log("offset getter via .call(view):", offsetGetter.call(view)); +console.log("offset getter via .call({}) (non-buffer this):", offsetGetter.call({})); + +// Regression: instance-level duck-typed reads for names removed from the +// OWN-property list must still work (inherited, not own). +const b = Buffer.alloc(4); +console.log("typeof b.hasOwnProperty:", typeof b.hasOwnProperty); +console.log("b.hasOwnProperty('x'):", b.hasOwnProperty("x")); +console.log("typeof b.at:", typeof b.at); +console.log("b.at(0):", b.at(0)); +console.log("Buffer.prototype.hasOwnProperty('at'):", Buffer.prototype.hasOwnProperty("at")); +console.log("Buffer.prototype.hasOwnProperty('hasOwnProperty'):", Buffer.prototype.hasOwnProperty("hasOwnProperty"));