From 8751291e4504de89c8aea0d2dbfaefbe7a6e22d0 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Tue, 29 Sep 2026 08:51:17 +0800 Subject: [PATCH] fix(forms): honor own properties in named access Replace the native-member name allowlist with real own-property checks using the incoming V8 name. Keep definition restrictions separate from named-property visibility so Reflect.defineProperty returns false while Object.defineProperty throws for supported names. Cover own data and accessor properties across parsed, detached, adopted, and child-document forms, including collisions with prototype members. --- .../element/forms/form_element.rs | 86 +++++---------- .../tests/dom_xhr/forms/named_lookup.rs | 101 ++++++++++++++++++ 2 files changed, 125 insertions(+), 62 deletions(-) diff --git a/moli-renderer-v8/src/native_bridge/element/forms/form_element.rs b/moli-renderer-v8/src/native_bridge/element/forms/form_element.rs index 4bd57e7827..c03e9a37c9 100644 --- a/moli-renderer-v8/src/native_bridge/element/forms/form_element.rs +++ b/moli-renderer-v8/src/native_bridge/element/forms/form_element.rs @@ -2,7 +2,6 @@ use super::*; use crate::custom_elements::is_form_associated_custom_element_handle; use crate::native_bridge::bridge::wrapped_handle_value_for_receiver; use crate::native_bridge::element::{html_element_getter_receiver, html_element_setter_receiver}; -use crate::util::throw_type_error; use moli_webapi_declare::DataPropertyDescriptorDeclaration; pub(in crate::native_bridge) fn form_action_getter_function<'s>( @@ -558,10 +557,11 @@ pub(in crate::native_bridge) fn form_named_getter<'s>( let Ok(key) = v8::Local::::try_from(key) else { return v8::Intercepted::kNo; }; + if object_has_expando_named_property(scope, args.holder(), key.into()) { + return v8::Intercepted::kNo; + } let key = key.to_rust_string_lossy(scope); - if is_array_index_property_name(&key) - || object_has_expando_named_property(scope, args.holder(), &key) - { + if is_array_index_property_name(&key) { return v8::Intercepted::kNo; } let Some(context) = args.holder().get_creation_context(scope) else { @@ -620,10 +620,11 @@ pub(in crate::native_bridge) fn form_named_descriptor<'s>( let Ok(key) = v8::Local::::try_from(key) else { return v8::Intercepted::kNo; }; + if object_has_expando_named_property(scope, args.holder(), key.into()) { + return v8::Intercepted::kNo; + } let key = key.to_rust_string_lossy(scope); - if is_array_index_property_name(&key) - || object_has_expando_named_property(scope, args.holder(), &key) - { + if is_array_index_property_name(&key) { return v8::Intercepted::kNo; } let Some(context) = args.holder().get_creation_context(scope) else { @@ -685,10 +686,11 @@ pub(in crate::native_bridge) fn form_named_deleter<'s>( let Ok(key) = v8::Local::::try_from(key) else { return v8::Intercepted::kNo; }; + if object_has_expando_named_property(scope, args.holder(), key.into()) { + return v8::Intercepted::kNo; + } let key = key.to_rust_string_lossy(scope); - if is_array_index_property_name(&key) - || object_has_expando_named_property(scope, args.holder(), &key) - { + if is_array_index_property_name(&key) { return v8::Intercepted::kNo; } if !form_has_named_item_or_past_name(unsafe { &*runtime_ptr }, handle, &key) { @@ -703,7 +705,7 @@ pub(in crate::native_bridge) fn form_named_definer<'s>( key: v8::Local<'_, v8::Name>, _desc: &v8::PropertyDescriptor, args: v8::PropertyCallbackArguments<'s>, - _rv: v8::ReturnValue<'_, v8::Boolean>, + mut rv: v8::ReturnValue<'_, v8::Boolean>, ) -> v8::Intercepted { let Ok((runtime_ptr, handle)) = node_runtime_and_handle_from_object_or_detached(scope, args.holder()) @@ -714,69 +716,28 @@ pub(in crate::native_bridge) fn form_named_definer<'s>( return v8::Intercepted::kNo; }; let key = key.to_rust_string_lossy(scope); - if is_array_index_property_name(&key) - || object_has_expando_named_property(scope, args.holder(), &key) - { + if is_array_index_property_name(&key) { return v8::Intercepted::kNo; } if !form_has_named_item_or_past_name(unsafe { &*runtime_ptr }, handle, &key) { return v8::Intercepted::kNo; } - throw_type_error(scope, "Cannot redefine an HTMLFormElement named property."); + // LegacyOverrideBuiltIns rejects defining any supported name, even when + // a real own property makes that name invisible to getters and deleters. + // Let V8 distinguish Object.defineProperty (throws) from Reflect (false). + rv.set_bool(false); v8::Intercepted::kYes } fn object_has_expando_named_property( scope: &mut v8::PinScope<'_, '_>, object: v8::Local<'_, v8::Object>, - key: &str, + key: v8::Local<'_, v8::Name>, ) -> bool { - if form_native_property_can_be_overridden(key) { - return false; - } - let Some(key) = v8_string(scope, key) else { - return false; - }; // Enumerating own keys invokes the indexed interceptor, which resolves every // form control. A real-property check neither enumerates virtual properties // nor evaluates an own accessor getter. - object - .has_real_named_property(scope, key.into()) - .unwrap_or(false) -} - -fn form_native_property_can_be_overridden(key: &str) -> bool { - matches!( - key, - "addEventListener" - | "removeEventListener" - | "dispatchEvent" - | "nodeType" - | "nodeName" - | "ownerDocument" - | "namespaceURI" - | "prefix" - | "localName" - | "title" - | "lang" - | "dir" - | "acceptCharset" - | "action" - | "autocomplete" - | "enctype" - | "encoding" - | "method" - | "name" - | "noValidate" - | "target" - | "elements" - | "length" - | "submit" - | "reset" - | "requestSubmit" - | "checkValidity" - | "reportValidity" - ) + object.has_real_named_property(scope, key).unwrap_or(false) } fn form_has_named_item_or_past_name( @@ -961,10 +922,11 @@ pub(in crate::native_bridge) fn form_named_query<'s>( let Ok(key) = v8::Local::::try_from(key) else { return v8::Intercepted::kNo; }; + if object_has_expando_named_property(scope, args.holder(), key.into()) { + return v8::Intercepted::kNo; + } let key = key.to_rust_string_lossy(scope); - if is_array_index_property_name(&key) - || object_has_expando_named_property(scope, args.holder(), &key) - { + if is_array_index_property_name(&key) { return v8::Intercepted::kNo; } if !form_has_named_item_or_past_name(unsafe { &*runtime_ptr }, handle, &key) { diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/forms/named_lookup.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/forms/named_lookup.rs index 27c60ab28a..b19e21c6e9 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_xhr/forms/named_lookup.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/forms/named_lookup.rs @@ -1,5 +1,106 @@ use super::*; +#[test] +fn form_named_lookup_own_properties_take_precedence_over_controls_and_prototypes() { + let mut vm = new_parsed_test_vm( + "https://form-lookup-own-properties.test/", + "
", + ); + let result = vm.eval(r#" + (() => { + const detachedDocument = document.implementation.createHTMLDocument('forms'); + const adopted = detachedDocument.createElement('form'); + document.body.appendChild(document.adoptNode(adopted)); + const childDocument = document.body.appendChild(document.createElement('iframe')).contentDocument; + const forms = [ + document.getElementById('parsed'), + document.createElement('form'), + detachedDocument.body.appendChild(detachedDocument.createElement('form')), + adopted, + childDocument.body.appendChild(childDocument.createElement('form')) + ]; + const names = [ + 'addEventListener', 'removeEventListener', 'dispatchEvent', + 'nodeType', 'nodeName', 'ownerDocument', 'namespaceURI', 'prefix', 'localName', + 'title', 'lang', 'dir', 'acceptCharset', 'action', 'autocomplete', 'enctype', + 'encoding', 'method', 'name', 'noValidate', 'target', 'elements', 'length', + 'submit', 'reset', 'requestSubmit', 'checkValidity', 'reportValidity', 'ordinaryKey' + ]; + for (const [mode, form] of forms.entries()) { + const doc = form.ownerDocument; + for (const name of names) { + const label = mode + ': ' + name; + if (Object.hasOwn(form, name)) throw Error(label + ': native instance property'); + let reads = 0; + const getter = () => { ++reads; return 'user-defined'; }; + Object.defineProperty(form, name, {get: getter, configurable: true, enumerable: true}); + const input = doc.createElement('input'); + input.name = name; + form.appendChild(input); + if (!(name in form) || reads !== 0) throw Error(label + ': query invoked accessor'); + const descriptor = Object.getOwnPropertyDescriptor(form, name); + if (descriptor.get !== getter || !descriptor.enumerable || reads !== 0) + throw Error(label + ': own descriptor'); + if (form[name] !== 'user-defined' || reads !== 1) throw Error(label + ': own accessor lost'); + if (!Reflect.deleteProperty(form, name) || reads !== 1) throw Error(label + ': own deletion'); + if (form[name] !== input) throw Error(label + ': named control must override prototype'); + if (Reflect.deleteProperty(form, name)) throw Error(label + ': named deletion'); + input.remove(); + if (Object.hasOwn(form, name)) throw Error(label + ': removed control still visible'); + + Object.defineProperty(form, name, {value: 'own-data', configurable: true, writable: true}); + form.appendChild(input); + if (form[name] !== 'own-data' || Object.getOwnPropertyDescriptor(form, name).value !== 'own-data') + throw Error(label + ': own data lost'); + if (!Reflect.deleteProperty(form, name) || form[name] !== input) + throw Error(label + ': data deletion must expose control'); + input.remove(); + } + } + return 'ok'; + })() + "#).expect("real own properties must win regardless of their names or wrapper construction path"); + assert_eq!(result, "ok"); +} + +#[test] +fn form_named_lookup_define_property_uses_supported_names_not_visibility() { + let mut vm = new_parsed_test_vm( + "https://form-lookup-define-property.test/", + "", + ); + let result = vm.eval(r#" + (() => { + for (const doc of [document, document.implementation.createHTMLDocument('forms')]) { + const form = doc.body.appendChild(doc.createElement('form')); + for (const name of ['submit', 'ordinaryKey']) { + Object.defineProperty(form, name, {value: 'own', configurable: true, writable: true}); + const input = form.appendChild(doc.createElement('input')); + input.name = name; + if (Reflect.defineProperty(form, name, {value: 'replacement'})) + throw Error(name + ': hidden supported name must reject definition'); + let threw = false; + try { Object.defineProperty(form, name, {value: 'replacement'}); } + catch (error) { threw = error instanceof TypeError; } + if (!threw || form[name] !== 'own') throw Error(name + ': definition failure changed own property'); + if (!delete form[name] || form[name] !== input) throw Error(name + ': reveal named property'); + if (Reflect.defineProperty(form, name, {value: 'replacement'})) + throw Error(name + ': visible supported name must reject definition'); + input.removeAttribute('name'); + if (Reflect.defineProperty(form, name, {value: 'replacement'})) + throw Error(name + ': past name must reject definition'); + input.remove(); + if (!Reflect.defineProperty(form, name, {value: 'replacement', configurable: true})) + throw Error(name + ': unsupported name must allow definition'); + if (!delete form[name]) throw Error(name + ': ordinary deletion'); + } + } + return 'ok'; + })() + "#).expect("defineProperty must reject supported names even when an own property hides them"); + assert_eq!(result, "ok"); +} + #[test] fn form_named_lookup_misses_do_not_enumerate_or_traverse_controls() { let mut vm = new_parsed_test_vm(