-
-
Notifications
You must be signed in to change notification settings - Fork 161
fix(runtime): add Symbol.toStringTag to Web/runtime built-ins #10632
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| ### Fixed | ||
|
|
||
| - **Web/runtime built-ins lack `Symbol.toStringTag` (#10555).** | ||
| `Object.prototype.toString.call(new URLSearchParams())` was `[object | ||
| Object]` instead of `[object URLSearchParams]`, and `x[Symbol.toStringTag]` | ||
| read back `undefined` — the standard cross-realm type check `kindOf`, | ||
| `isURLSearchParams`, `isFormData`, `isBlob`, and lodash's `baseGetTag` use. | ||
| axios 1.19.0 decides how to serialize a request body this way: a | ||
| `URLSearchParams` body was sent as JSON instead of | ||
| `application/x-www-form-urlencoded`. | ||
|
|
||
| Covers `URL`, `URLSearchParams`, `Headers`, `Request`, `Response`, | ||
| `FormData`, `Blob`, `File`, `AbortController`, `AbortSignal`, | ||
| `TextEncoder`, `TextDecoder`, `EventTarget`, `Event`, `CustomEvent` — both | ||
| the brand string and the real `x[Symbol.toStringTag]` value, plus a | ||
| correctly-shaped (`writable: false, enumerable: false, configurable: true`) | ||
| own descriptor on each constructor's `.prototype`. `Map`/`Promise`/ | ||
| `ArrayBuffer`/`DataView` are deliberately out of scope (their brand string | ||
| was already correct via a different, structural mechanism — only their own | ||
| property is missing, a separate fix). `Uint8Array` already had a correct | ||
| accessor. | ||
|
|
||
| Root cause: two representations, two gaps. The Web Fetch family and | ||
| `TextEncoder`/`TextDecoder` are small-integer registry handles with no | ||
| brand/property case in `js_object_to_string` or | ||
| `js_object_get_symbol_property`. `URL`/`URLSearchParams` and | ||
| `AbortController`/`AbortSignal`/`EventTarget`/`Event`/`CustomEvent` are | ||
| real objects whose instances are never `[[Prototype]]`-linked to their | ||
| `.prototype` object, so a property installed only there (the issue's | ||
| suggested shape) would never be reached from an instance. | ||
|
|
||
| Fix: a new `web_builtin_to_string_tag` in `perry-runtime` answers both | ||
| `Object.prototype.toString` and `x[Symbol.toStringTag]` from one place | ||
| (reusing the existing `fetch_handle_kind_probe`/structural/class-id | ||
| detectors), and a real descriptor is *also* installed on each | ||
| constructor's `.prototype` for reflection. The directly-affected path | ||
| measured ~4.7x fewer instructions, not slower (the new check runs early | ||
| and short-circuits several later brand checks a `Headers` handle used to | ||
| fall through). | ||
|
|
||
| Validation: new `test_gap_10555_symbol_tostringtag_web_builtins` (brand | ||
| string, property value, and full descriptor shape for all 13 types, plus | ||
| JSON/typeof/non-writability checks) fails on the baseline and matches Node | ||
| on the fix. Two existing unit tests updated (not a regression — the fix | ||
| makes `TextEncoder`'s sentinel handle id meaningful, which one test had | ||
| assumed was generic); `test_gap_url*`/`test_gap_headers*`/ | ||
| `test_gap_fetch*`/`test_gap_events_import_4995` (12 tests) unaffected. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -518,6 +518,34 @@ unsafe fn web_stream_symbol_property(obj_f64: f64, sym_f64: f64) -> Option<f64> | |
| Some(f64::from_bits(TAG_UNDEFINED)) | ||
| } | ||
|
|
||
| /// `Symbol.toStringTag` for Perry's Web/runtime built-ins that have no | ||
| /// registered prototype-chain or class-id hook reachable from the generic | ||
| /// resolvers below (#10555) -- see `web_builtin_to_string_tag`'s doc comment | ||
| /// for the full inventory and why each kind needs this. An own override | ||
| /// (`Object.defineProperty(x, Symbol.toStringTag, …)`) still wins: the | ||
| /// side-table read is a pointer-KEYED lookup, safe even for the | ||
| /// handle-backed kinds since it never dereferences `obj_f64` as a pointer. | ||
| unsafe fn web_builtin_to_string_tag_symbol_property(obj_f64: f64, sym_f64: f64) -> Option<f64> { | ||
| let sym_key = sym_key_from_f64(sym_f64); | ||
| if sym_key == 0 { | ||
| return None; | ||
| } | ||
| let to_string_tag = well_known_symbol("toStringTag"); | ||
| if to_string_tag.is_null() { | ||
| return None; | ||
| } | ||
| let ts_f64 = f64::from_bits(crate::value::JSValue::pointer(to_string_tag as *const u8).bits()); | ||
| if sym_key != sym_key_from_f64(ts_f64) { | ||
| return None; | ||
| } | ||
| if let Some(v) = own_symbol_property(obj_f64, sym_f64) { | ||
| return Some(v); | ||
| } | ||
| let tag = crate::object::web_builtin_to_string_tag(obj_f64)?; | ||
| let str_ptr = js_string_from_bytes(tag.as_ptr(), tag.len() as u32); | ||
| Some(f64::from_bits(STRING_TAG | (str_ptr as u64 & POINTER_MASK))) | ||
|
Comment on lines
+544
to
+546
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift Honor configurable prototype tag changes. This fallback returns a fixed tag before the later prototype lookup at Line 960. It therefore ignores a redefined or deleted configurable property such as Resolve the effective prototype property before this fallback. Only synthesize the fixed tag when that lookup misses. Add coverage for redefine and delete operations on a supported prototype. The PR objective requires standard 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| #[no_mangle] | ||
| pub unsafe extern "C" fn js_object_get_symbol_property(obj_f64: f64, sym_f64: f64) -> f64 { | ||
| #[cfg(feature = "regex-engine")] | ||
|
|
@@ -668,6 +696,9 @@ pub unsafe extern "C" fn js_object_get_symbol_property(obj_f64: f64, sym_f64: f6 | |
| if let Some(v) = web_stream_symbol_property(obj_f64, sym_f64) { | ||
| return v; | ||
| } | ||
| if let Some(v) = web_builtin_to_string_tag_symbol_property(obj_f64, sym_f64) { | ||
| return v; | ||
| } | ||
| // #1213: Timeout/Immediate handles expose `Symbol.dispose` so | ||
| // `using t = setTimeout(...)` and `t[Symbol.dispose]()` clear the timer. | ||
| // The handle is a small id NaN-boxed as POINTER; the symbol-keyed read | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Let an own
Symbol.toStringTagoverride the fallback brand.Line 336 returns the fixed Web built-in tag before
object_to_string_tag_propertyruns. For example, afterObject.defineProperty(url, Symbol.toStringTag, { value: "Custom" }),Object.prototype.toString.call(url)still returns[object URL]instead of[object Custom]. Check the property first and use the built-in tag only as the fallback.Proposed fix
if let Some(tag) = web_builtin_to_string_tag(value) { - let formatted = format!("[object {}]", tag); + let tag = object_to_string_tag_property(value).unwrap_or_else(|| tag.to_owned()); + let formatted = format!("[object {}]", tag); let bytes = formatted.as_bytes(); let str_ptr = crate::string::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32); return f64::from_bits(STRING_TAG | (str_ptr as u64 & POINTER_MASK)); }🤖 Prompt for AI Agents