From b40abc6e8bf72d95bf1c4c25a7ef60f1bc6653bf Mon Sep 17 00:00:00 2001 From: Monti-27 Date: Fri, 13 Mar 2026 21:55:52 +0530 Subject: [PATCH 1/7] implement url search params --- core/runtime/src/url.rs | 557 +++++++++++++++++++++++++++++++--- core/runtime/src/url/tests.rs | 171 +++++++++++ tests/wpt/src/lib.rs | 2 - 3 files changed, 692 insertions(+), 38 deletions(-) diff --git a/core/runtime/src/url.rs b/core/runtime/src/url.rs index e34b555987e..0f459b93033 100644 --- a/core/runtime/src/url.rs +++ b/core/runtime/src/url.rs @@ -14,18 +14,496 @@ #[cfg(test)] mod tests; +use boa_engine::builtins::iterable::create_iter_result_object; +use boa_engine::builtins::object::OrdinaryObject; use boa_engine::class::Class; +use boa_engine::interop::JsClass; +use boa_engine::object::{ + ObjectInitializer, + builtins::{JsArray, TypedJsFunction}, +}; +use boa_engine::property::Attribute; use boa_engine::realm::Realm; use boa_engine::value::Convert; use boa_engine::{ - Context, Finalize, JsData, JsResult, JsString, JsValue, Trace, boa_class, boa_module, js_error, + Context, Finalize, JsData, JsObject, JsResult, JsString, JsSymbol, JsValue, Trace, boa_class, + boa_module, js_error, js_string, native_function::NativeFunction, }; use std::fmt::Display; +/// A callback function for the `URLSearchParams.prototype.forEach` method. +pub type SearchParamsForEachCallback = TypedJsFunction<(JsString, JsString, JsObject), ()>; + +#[derive(Debug, Clone, Copy)] +enum UrlSearchParamsIteratorKind { + Key, + Value, + KeyAndValue, +} + +fn to_usv_string(string: &JsString) -> JsString { + JsString::from(string.to_std_string_lossy()) +} + +fn to_usv_string_value(value: &JsValue, context: &mut Context) -> JsResult { + value + .to_string(context) + .map(|string| to_usv_string(&string)) +} + +fn parse_search_params(input: &JsString) -> Vec<(JsString, JsString)> { + let input = input.to_std_string_lossy(); + let input = input.strip_prefix('?').unwrap_or(&input); + + url::form_urlencoded::parse(input.as_bytes()) + .map(|(name, value)| { + ( + JsString::from(name.as_ref()), + JsString::from(value.as_ref()), + ) + }) + .collect() +} + +fn serialize_search_params(params: &[(JsString, JsString)]) -> String { + let mut serializer = url::form_urlencoded::Serializer::new(String::new()); + + for (name, value) in params { + let name = name.to_std_string_lossy(); + let value = value.to_std_string_lossy(); + serializer.append_pair(&name, &value); + } + + serializer.finish() +} + +fn has_callable_iterator(object: &JsObject, context: &mut Context) -> JsResult { + let method = object.get(JsSymbol::iterator(), context)?; + if method.is_null_or_undefined() { + return Ok(false); + } + + if method.as_callable().is_none() { + return Err( + js_error!(TypeError: "URLSearchParams constructor requires @@iterator to be callable"), + ); + } + + Ok(true) +} + +fn array_from(value: &JsValue, context: &mut Context) -> JsResult { + let array = context + .global_object() + .get(js_string!("Array"), context)? + .to_object(context)?; + let from = array + .get(js_string!("from"), context)? + .as_object() + .ok_or_else(|| js_error!(Error: "Array.from should be callable"))?; + + let value = from.call(&array.clone().into(), std::slice::from_ref(value), context)?; + JsArray::from_object(value.to_object(context)?) +} + +fn collect_sequence_pairs( + init: &JsValue, + context: &mut Context, +) -> JsResult> { + let items = array_from(init, context)?; + let length = usize::try_from(items.length(context)?) + .map_err(|_| js_error!(RangeError: "URLSearchParams sequence is too large"))?; + let mut pairs = Vec::with_capacity(length); + + for index in 0..length { + let item = items.get(index, context)?; + let Some(item_object) = item.as_object() else { + return Err(js_error!( + TypeError: "URLSearchParams constructor expects each sequence item to be an iterable pair" + )); + }; + + if !has_callable_iterator(&item_object, context)? { + return Err(js_error!( + TypeError: "URLSearchParams constructor expects each sequence item to be an iterable pair" + )); + } + + let pair = array_from(&item, context)?; + if pair.length(context)? != 2 { + return Err(js_error!( + TypeError: "URLSearchParams constructor expects each sequence item to contain exactly two values" + )); + } + + let name = to_usv_string_value(&pair.get(0, context)?, context)?; + let value = to_usv_string_value(&pair.get(1, context)?, context)?; + pairs.push((name, value)); + } + + Ok(pairs) +} + +fn collect_record_pairs( + object: &JsObject, + context: &mut Context, +) -> JsResult> { + let keys = object.own_property_keys(context)?; + let mut pairs = Vec::new(); + + for key in keys { + let enumerable = OrdinaryObject::property_is_enumerable( + &object.clone().into(), + &[key.clone().into()], + context, + )? + .to_boolean(); + + if !enumerable { + continue; + } + + let name = to_usv_string_value(&JsValue::from(key.clone()), context)?; + let value = to_usv_string_value(&object.get(key, context)?, context)?; + pairs.push((name, value)); + } + + Ok(pairs) +} + +/// The `URLSearchParams` class represents the query portion of a URL. +#[derive(Debug, JsData, Trace, Finalize)] +pub struct UrlSearchParams { + list: Vec<(JsString, JsString)>, + url: Option>, +} + +impl UrlSearchParams { + fn from_url(url: JsObject, context: &mut Context) -> JsResult> { + Self::from_data( + Self { + list: Vec::new(), + url: Some(url), + }, + context, + )? + .downcast::() + .map_err(|_| js_error!(Error: "URLSearchParams class should be registered")) + } + + fn pairs(&self) -> Vec<(JsString, JsString)> { + if let Some(url) = &self.url { + let url = url.borrow(); + return url + .data() + .inner + .query_pairs() + .map(|(name, value)| { + ( + JsString::from(name.as_ref()), + JsString::from(value.as_ref()), + ) + }) + .collect(); + } + + self.list.clone() + } + + fn update(&mut self, pairs: Vec<(JsString, JsString)>) { + if let Some(url) = &self.url { + let mut url = url.borrow_mut(); + let url = url.data_mut(); + + if pairs.is_empty() { + url.inner.set_query(None); + } else { + let query = serialize_search_params(&pairs); + url.inner.set_query(Some(&query)); + } + return; + } + + self.list = pairs; + } +} + +#[derive(Debug, JsData, Trace, Finalize)] +struct UrlSearchParamsIterator { + search_params: JsObject, + next_index: usize, + #[unsafe_ignore_trace] + kind: UrlSearchParamsIteratorKind, + done: bool, +} + +impl UrlSearchParamsIterator { + fn create( + search_params: JsObject, + kind: UrlSearchParamsIteratorKind, + context: &mut Context, + ) -> JsValue { + ObjectInitializer::with_native_data_and_proto( + Self { + search_params, + next_index: 0, + kind, + done: false, + }, + context + .intrinsics() + .objects() + .iterator_prototypes() + .iterator(), + context, + ) + .function( + NativeFunction::from_fn_ptr(Self::next), + js_string!("next"), + 0, + ) + .function( + NativeFunction::from_fn_ptr(Self::iterator), + JsSymbol::iterator(), + 0, + ) + .property( + JsSymbol::to_string_tag(), + js_string!("URLSearchParams Iterator"), + Attribute::CONFIGURABLE, + ) + .build() + .into() + } + + #[allow(clippy::unnecessary_wraps)] + fn iterator(this: &JsValue, _: &[JsValue], _: &mut Context) -> JsResult { + Ok(this.clone()) + } + + fn next(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { + let object = this + .as_object() + .ok_or_else(|| js_error!(TypeError: "`this` is not a URLSearchParams iterator"))?; + let mut iterator = object + .downcast_mut::() + .ok_or_else(|| js_error!(TypeError: "`this` is not a URLSearchParams iterator"))?; + + if iterator.done { + return Ok(create_iter_result_object( + JsValue::undefined(), + true, + context, + )); + } + + let pair = iterator + .search_params + .borrow() + .data() + .pairs() + .get(iterator.next_index) + .cloned(); + + let Some((name, value)) = pair else { + iterator.done = true; + return Ok(create_iter_result_object( + JsValue::undefined(), + true, + context, + )); + }; + + iterator.next_index += 1; + + let result: JsValue = match iterator.kind { + UrlSearchParamsIteratorKind::Key => name.into(), + UrlSearchParamsIteratorKind::Value => value.into(), + UrlSearchParamsIteratorKind::KeyAndValue => { + JsArray::from_iter([name.into(), value.into()], context).into() + } + }; + + Ok(create_iter_result_object(result, false, context)) + } +} + +#[boa_class(rename = "URLSearchParams")] +#[boa(rename_all = "camelCase")] +impl UrlSearchParams { + #[boa(constructor)] + fn constructor(init: JsValue, context: &mut Context) -> JsResult { + let list = if init.is_undefined() || init.is_null() { + Vec::new() + } else if let Some(object) = init.as_object() { + if let Some(other) = object.downcast_ref::() { + other.pairs() + } else if has_callable_iterator(&object, context)? { + collect_sequence_pairs(&init, context)? + } else { + collect_record_pairs(&object, context)? + } + } else { + parse_search_params(&to_usv_string_value(&init, context)?) + }; + + Ok(Self { list, url: None }) + } + + #[boa(getter)] + fn size(&self) -> usize { + self.pairs().len() + } + + fn append(&mut self, name: Convert, value: Convert) { + let mut pairs = self.pairs(); + pairs.push((to_usv_string(&name.0), to_usv_string(&value.0))); + self.update(pairs); + } + + fn delete(&mut self, name: Convert, value: Option>) { + let name = to_usv_string(&name.0); + let value = value.as_ref().map(|value| to_usv_string(&value.0)); + let mut pairs = self.pairs(); + + match value { + Some(value) => { + pairs.retain(|(existing_name, existing_value)| { + existing_name != &name || existing_value != &value + }); + } + None => { + pairs.retain(|(existing_name, _)| existing_name != &name); + } + } + + self.update(pairs); + } + + fn entries(this: JsClass, context: &mut Context) -> JsValue { + UrlSearchParamsIterator::create( + this.inner(), + UrlSearchParamsIteratorKind::KeyAndValue, + context, + ) + } + + #[boa(method)] + fn for_each( + this: JsClass, + callback: SearchParamsForEachCallback, + this_arg: Option, + context: &mut Context, + ) -> JsResult<()> { + let object = this.inner().upcast(); + let this_arg = this_arg.unwrap_or_default(); + let mut index = 0usize; + + loop { + let pair = { + let params = this.borrow(); + params.pairs().get(index).cloned() + }; + let Some((name, value)) = pair else { + break; + }; + + callback.call_with_this(&this_arg, context, (value, name, object.clone()))?; + index += 1; + } + + Ok(()) + } + + fn get(&self, name: Convert) -> JsValue { + let name = to_usv_string(&name.0); + self.pairs() + .into_iter() + .find_map(|(existing_name, value)| (existing_name == name).then_some(value.into())) + .unwrap_or_else(JsValue::null) + } + + fn get_all(&self, name: Convert) -> Vec { + let name = to_usv_string(&name.0); + self.pairs() + .into_iter() + .filter_map(|(existing_name, value)| (existing_name == name).then_some(value)) + .collect() + } + + fn has(&self, name: Convert, value: Option>) -> bool { + let name = to_usv_string(&name.0); + let value = value.as_ref().map(|value| to_usv_string(&value.0)); + match value { + Some(value) => self + .pairs() + .into_iter() + .any(|(existing_name, existing_value)| { + existing_name == name && existing_value == value + }), + None => self + .pairs() + .into_iter() + .any(|(existing_name, _)| existing_name == name), + } + } + + #[boa(symbol = "iterator")] + fn iterator(this: JsClass, context: &mut Context) -> JsValue { + Self::entries(this, context) + } + + fn keys(this: JsClass, context: &mut Context) -> JsValue { + UrlSearchParamsIterator::create(this.inner(), UrlSearchParamsIteratorKind::Key, context) + } + + fn set(&mut self, name: Convert, value: Convert) { + let name = to_usv_string(&name.0); + let value = to_usv_string(&value.0); + let mut found = false; + let mut result = Vec::with_capacity(self.pairs().len() + 1); + + for (existing_name, existing_value) in self.pairs() { + if existing_name == name { + if !found { + result.push((existing_name, value.clone())); + found = true; + } + } else { + result.push((existing_name, existing_value)); + } + } + + if !found { + result.push((name, value)); + } + + self.update(result); + } + + fn sort(&mut self) { + let mut pairs = self.pairs(); + pairs.sort_by(|(left, _), (right, _)| left.cmp(right)); + self.update(pairs); + } + + fn to_string(&self) -> JsString { + JsString::from(serialize_search_params(&self.pairs())) + } + + fn values(this: JsClass, context: &mut Context) -> JsValue { + UrlSearchParamsIterator::create(this.inner(), UrlSearchParamsIteratorKind::Value, context) + } +} + /// The `URL` class represents a (properly parsed) Uniform Resource Locator. -#[derive(Debug, Clone, JsData, Trace, Finalize)] +#[derive(Debug, JsData, Trace, Finalize)] #[boa_gc(unsafe_no_drop)] -pub struct Url(#[unsafe_ignore_trace] url::Url); +pub struct Url { + #[unsafe_ignore_trace] + inner: url::Url, + search_params: Option>, +} impl Url { /// Register the `URL` class into the realm. Pass `None` for the realm to @@ -40,19 +518,22 @@ impl Url { impl Display for Url { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) + write!(f, "{}", self.inner) } } impl From for Url { fn from(url: url::Url) -> Self { - Self(url) + Self { + inner: url, + search_params: None, + } } } impl From for url::Url { fn from(url: Url) -> url::Url { - url.0 + url.inner } } @@ -68,149 +549,152 @@ impl Url { if let Some(Convert(ref base)) = base { let base_url = url::Url::parse(base) .map_err(|e| js_error!(TypeError: "Failed to parse base URL: {}", e))?; - if base_url.cannot_be_a_base() { - return Err(js_error!(TypeError: "Base URL {} cannot be a base", base)); - } let url = base_url .join(url) .map_err(|e| js_error!(TypeError: "Failed to parse URL: {}", e))?; - Ok(Self(url)) + Ok(Self::from(url)) } else { let url = url::Url::parse(url) .map_err(|e| js_error!(TypeError: "Failed to parse URL: {}", e))?; - Ok(Self(url)) + Ok(Self::from(url)) } } #[boa(getter)] fn hash(&self) -> JsString { - JsString::from(url::quirks::hash(&self.0)) + JsString::from(url::quirks::hash(&self.inner)) } #[boa(setter)] #[boa(rename = "hash")] fn set_hash(&mut self, value: Convert) { - url::quirks::set_hash(&mut self.0, &value.0); + url::quirks::set_hash(&mut self.inner, &value.0); } #[boa(getter)] fn hostname(&self) -> JsString { - JsString::from(url::quirks::hostname(&self.0)) + JsString::from(url::quirks::hostname(&self.inner)) } #[boa(setter)] #[boa(rename = "hostname")] fn set_hostname(&mut self, value: Convert) { - let _ = url::quirks::set_hostname(&mut self.0, &value.0); + let _ = url::quirks::set_hostname(&mut self.inner, &value.0); } #[boa(getter)] fn host(&self) -> JsString { - JsString::from(url::quirks::host(&self.0)) + JsString::from(url::quirks::host(&self.inner)) } #[boa(setter)] #[boa(rename = "host")] fn set_host(&mut self, value: Convert) { - let _ = url::quirks::set_host(&mut self.0, &value.0); + let _ = url::quirks::set_host(&mut self.inner, &value.0); } #[boa(getter)] fn href(&self) -> JsString { - JsString::from(url::quirks::href(&self.0)) + JsString::from(url::quirks::href(&self.inner)) } #[boa(setter)] #[boa(rename = "href")] fn set_href(&mut self, value: Convert) -> JsResult<()> { - url::quirks::set_href(&mut self.0, &value.0) + url::quirks::set_href(&mut self.inner, &value.0) .map_err(|e| js_error!(TypeError: "Failed to set href: {}", e)) } #[boa(getter)] fn origin(&self) -> JsString { - JsString::from(url::quirks::origin(&self.0)) + JsString::from(url::quirks::origin(&self.inner)) } #[boa(getter)] fn password(&self) -> JsString { - JsString::from(url::quirks::password(&self.0)) + JsString::from(url::quirks::password(&self.inner)) } #[boa(setter)] #[boa(rename = "password")] fn set_password(&mut self, value: Convert) { - let _ = url::quirks::set_password(&mut self.0, &value.0); + let _ = url::quirks::set_password(&mut self.inner, &value.0); } #[boa(getter)] fn pathname(&self) -> JsString { - JsString::from(url::quirks::pathname(&self.0)) + JsString::from(url::quirks::pathname(&self.inner)) } #[boa(setter)] #[boa(rename = "pathname")] fn set_pathname(&mut self, value: Convert) { - let () = url::quirks::set_pathname(&mut self.0, &value.0); + let () = url::quirks::set_pathname(&mut self.inner, &value.0); } #[boa(getter)] fn port(&self) -> JsString { - JsString::from(url::quirks::port(&self.0)) + JsString::from(url::quirks::port(&self.inner)) } #[boa(setter)] #[boa(rename = "port")] fn set_port(&mut self, value: Convert) { - let _ = url::quirks::set_port(&mut self.0, &value.0.to_std_string_lossy()); + let _ = url::quirks::set_port(&mut self.inner, &value.0.to_std_string_lossy()); } #[boa(getter)] fn protocol(&self) -> JsString { - JsString::from(url::quirks::protocol(&self.0)) + JsString::from(url::quirks::protocol(&self.inner)) } #[boa(setter)] #[boa(rename = "protocol")] fn set_protocol(&mut self, value: Convert) { - let _ = url::quirks::set_protocol(&mut self.0, &value.0); + let _ = url::quirks::set_protocol(&mut self.inner, &value.0); } #[boa(getter)] fn search(&self) -> JsString { - JsString::from(url::quirks::search(&self.0)) + JsString::from(url::quirks::search(&self.inner)) } #[boa(setter)] #[boa(rename = "search")] fn set_search(&mut self, value: Convert) { - url::quirks::set_search(&mut self.0, &value.0); + url::quirks::set_search(&mut self.inner, &value.0); } #[boa(getter)] - fn search_params() -> JsResult<()> { - Err(js_error!(Error: "URL.searchParams is not implemented")) + fn search_params(this: JsClass, context: &mut Context) -> JsResult { + if let Some(existing) = this.borrow().search_params.clone() { + return Ok(existing.into()); + } + + let params = UrlSearchParams::from_url(this.inner(), context)?; + this.borrow_mut().search_params = Some(params.clone()); + Ok(params.into()) } #[boa(getter)] fn username(&self) -> JsString { - JsString::from(self.0.username()) + JsString::from(self.inner.username()) } #[boa(setter)] #[boa(rename = "username")] fn set_username(&mut self, value: Convert) { - let _ = self.0.set_username(&value.0); + let _ = self.inner.set_username(&value.0); } fn to_string(&self) -> JsString { - JsString::from(format!("{}", self.0)) + JsString::from(format!("{}", self.inner)) } #[boa(rename = "toJSON")] fn to_json(&self) -> JsString { - JsString::from(format!("{}", self.0)) + JsString::from(format!("{}", self.inner)) } #[boa(static)] @@ -244,4 +728,5 @@ impl Url { #[boa_module] pub mod js_module { type Url = super::Url; + type UrlSearchParams = super::UrlSearchParams; } diff --git a/core/runtime/src/url/tests.rs b/core/runtime/src/url/tests.rs index 21cfc836955..575efd830ca 100644 --- a/core/runtime/src/url/tests.rs +++ b/core/runtime/src/url/tests.rs @@ -68,6 +68,12 @@ fn url_base() { assert_eq(url.hash, "#fragment"); "##, ), + TestAction::run( + r##" + url = new URL("http://example.org/new/path?new-query#new-fragment", "about:blank"); + assert_eq(url.href, "http://example.org/new/path?new-query#new-fragment"); + "##, + ), ]); } @@ -107,6 +113,171 @@ fn url_static_methods() { assert(!URL.canParse("http//:example.org/new/path?new-query#new-fragment")); assert(!URL.canParse("http://example.org/new/path?new-query#new-fragment", "http:")); assert(URL.canParse("/new/path?new-query#new-fragment", "http://example.org/")); + assert(URL.canParse("http://example.org/new/path?new-query#new-fragment", "about:blank")); + "##, + ), + ]); +} + +#[test] +fn url_search_params_constructor_and_methods() { + run_test_actions([ + TestAction::run(TEST_HARNESS), + TestAction::run( + r##" + const params = new URLSearchParams([["b", "2"], ["a", "1"], ["a", "3"]]); + assert_eq(params.size, 3); + assert_eq(params.get("a"), "1"); + assert_eq(params.get("missing"), null); + assert_eq(params.getAll("a").join(","), "1,3"); + assert(params.has("b")); + assert(!params.has("b", "4")); + + params.append("c", "5"); + params.delete("a", "1"); + params.set("b", "4"); + params.sort(); + + assert_eq(params.toString(), "a=3&b=4&c=5"); + assert_eq([...params].map(([k, v]) => `${k}=${v}`).join("&"), "a=3&b=4&c=5"); + "##, + ), + TestAction::run( + r##" + const fromObject = new URLSearchParams({ a: "1", b: "2" }); + assert_eq(fromObject.toString(), "a=1&b=2"); + + const fromString = new URLSearchParams("?x=1&x=2"); + assert_eq(fromString.getAll("x").join(","), "1,2"); + "##, + ), + TestAction::run( + r##" + const fromIterableObject = new URLSearchParams({ + *[Symbol.iterator]() { + yield ["i", "1"]; + yield new Set(["j", "2"]); + }, + ignored: "value", + }); + assert_eq(fromIterableObject.toString(), "i=1&j=2"); + "##, + ), + TestAction::run( + r##" + const record = Object.create({ inherited: "x" }); + Object.defineProperty(record, "hidden", { value: "2", enumerable: false }); + record.a = "1"; + assert_eq(new URLSearchParams(record).toString(), "a=1"); + "##, + ), + ]); +} + +#[test] +fn url_search_params_is_live_and_cached() { + run_test_actions([ + TestAction::run(TEST_HARNESS), + TestAction::run( + r##" + const url = new URL("https://example.com/?a=1&b=2"); + const params1 = url.searchParams; + const params2 = url.searchParams; + + assert(params1 === params2, "searchParams must be the same object"); + assert_eq(params1.get("a"), "1"); + + params1.append("c", "3"); + assert_eq(url.search, "?a=1&b=2&c=3"); + + url.search = "?x=9"; + assert_eq(params1.get("x"), "9"); + assert_eq(params1.get("a"), null); + + params1.delete("x"); + assert_eq(url.href, "https://example.com/"); + "##, + ), + TestAction::run( + r##" + const liveUrl = new URL("http://a.b/c?a=1&b=2&c=3"); + const liveSeen = []; + + for (const entry of liveUrl.searchParams) { + if (entry[0] === "a") { + liveUrl.search = "x=1&y=2&z=3"; + } + liveSeen.push(entry.join("=")); + } + + assert_eq(liveSeen.join("&"), "a=1&y=2&z=3"); + "##, + ), + TestAction::run( + r##" + const forEachUrl = new URL("http://localhost/query?param0=0¶m1=1¶m2=2"); + const forEachSeen = []; + + forEachUrl.searchParams.forEach((value, key) => { + forEachSeen.push(`${key}=${value}`); + if (key === "param0") { + forEachUrl.searchParams.delete("param1"); + } + }); + + assert_eq(forEachSeen.join("&"), "param0=0¶m2=2"); + "##, + ), + ]); +} + +#[test] +fn url_search_params_constructor_errors() { + run_test_actions([ + TestAction::run(TEST_HARNESS), + TestAction::run( + r##" + var threw = false; + try { + new URLSearchParams(new String("ab")); + } catch (e) { + threw = e instanceof TypeError; + } + assert(threw, "string wrapper objects must use the iterable branch"); + "##, + ), + TestAction::run( + r##" + var threw = false; + try { + new URLSearchParams([[1, 2, 3]]); + } catch (e) { + threw = e instanceof TypeError; + } + assert(threw, "pairs with length != 2 must throw"); + "##, + ), + TestAction::run( + r##" + var threw = false; + try { + new URLSearchParams({ a: "1", [Symbol.iterator]: 1 }); + } catch (e) { + threw = e instanceof TypeError; + } + assert(threw, "non-callable @@iterator must throw"); + "##, + ), + TestAction::run( + r##" + const key = Symbol("k"); + var threw = false; + try { + new URLSearchParams({ [key]: "1", a: "2" }); + } catch (e) { + threw = e instanceof TypeError; + } + assert(threw, "enumerable symbol keys must throw during record conversion"); "##, ), ]); diff --git a/tests/wpt/src/lib.rs b/tests/wpt/src/lib.rs index 956cbae045b..d24ed44dba0 100644 --- a/tests/wpt/src/lib.rs +++ b/tests/wpt/src/lib.rs @@ -416,8 +416,6 @@ fn url( #[base_dir = "${WPT_ROOT}"] #[files("url/url-*.any.js")] #[exclude("idlharness")] - // "Base URL about:blank cannot be a base" - #[exclude("url-searchparams.any.js")] // "fetch is not defined" #[exclude("url-origin.any.js")] #[exclude("url-setters.any.js")] From 6168ca1e517c9307ba30a36288abf3217c43e9e1 Mon Sep 17 00:00:00 2001 From: Monti-27 Date: Fri, 13 Mar 2026 22:48:47 +0530 Subject: [PATCH 2/7] fix url search params --- core/runtime/src/url.rs | 291 +++++++++++++++++++++++++++------- core/runtime/src/url/tests.rs | 50 ++++++ 2 files changed, 283 insertions(+), 58 deletions(-) diff --git a/core/runtime/src/url.rs b/core/runtime/src/url.rs index 0f459b93033..090aa143123 100644 --- a/core/runtime/src/url.rs +++ b/core/runtime/src/url.rs @@ -17,7 +17,7 @@ mod tests; use boa_engine::builtins::iterable::create_iter_result_object; use boa_engine::builtins::object::OrdinaryObject; use boa_engine::class::Class; -use boa_engine::interop::JsClass; +use boa_engine::interop::{JsClass, TryFromJsArgument}; use boa_engine::object::{ ObjectInitializer, builtins::{JsArray, TypedJsFunction}, @@ -26,8 +26,8 @@ use boa_engine::property::Attribute; use boa_engine::realm::Realm; use boa_engine::value::Convert; use boa_engine::{ - Context, Finalize, JsData, JsObject, JsResult, JsString, JsSymbol, JsValue, Trace, boa_class, - boa_module, js_error, js_string, native_function::NativeFunction, + Context, Finalize, JsData, JsError, JsObject, JsResult, JsString, JsSymbol, JsValue, Trace, + boa_class, boa_module, js_error, js_string, native_function::NativeFunction, }; use std::fmt::Display; @@ -77,68 +77,223 @@ fn serialize_search_params(params: &[(JsString, JsString)]) -> String { serializer.finish() } -fn has_callable_iterator(object: &JsObject, context: &mut Context) -> JsResult { - let method = object.get(JsSymbol::iterator(), context)?; +#[derive(Debug, Clone)] +struct OptionalArg(Option); + +impl<'a> TryFromJsArgument<'a> for OptionalArg { + fn try_from_js_argument( + _: &'a JsValue, + rest: &'a [JsValue], + _: &mut Context, + ) -> JsResult<(Self, &'a [JsValue])> { + match rest.split_first() { + Some((first, rest)) => Ok((Self(Some(first.clone())), rest)), + None => Ok((Self(None), rest)), + } + } +} + +#[derive(Debug, Clone)] +struct SyncIterator { + iterator: JsObject, + next: JsObject, + done: bool, +} + +#[derive(Debug, Clone)] +struct IteratorStep { + done: bool, + value: JsValue, +} + +fn get_method(object: &JsObject, key: K, context: &mut Context) -> JsResult> +where + K: Into, +{ + let method = object.get(key, context)?; if method.is_null_or_undefined() { - return Ok(false); + return Ok(None); } - if method.as_callable().is_none() { - return Err( - js_error!(TypeError: "URLSearchParams constructor requires @@iterator to be callable"), - ); + let Some(method) = method.as_object().filter(JsObject::is_callable) else { + return Err(js_error!( + TypeError: "value returned for property of object is not a function" + )); + }; + + Ok(Some(method.clone())) +} + +impl SyncIterator { + fn from_method( + value: &JsValue, + iterator_method: &JsObject, + context: &mut Context, + ) -> JsResult { + let iterator = iterator_method.call(value, &[], context)?; + let Some(iterator) = iterator.as_object() else { + return Err(js_error!(TypeError: "returned iterator is not an object")); + }; + + let next = iterator.get(js_string!("next"), context)?; + let Some(next) = next.as_object().filter(JsObject::is_callable) else { + return Err(js_error!( + TypeError: "value returned for property of object is not a function" + )); + }; + + Ok(Self { + iterator: iterator.clone(), + next: next.clone(), + done: false, + }) + } + + fn next_result( + &mut self, + value: Option<&JsValue>, + context: &mut Context, + ) -> JsResult { + let result = self + .next + .call( + &self.iterator.clone().into(), + value.map_or(&[], std::slice::from_ref), + context, + ) + .inspect_err(|_| { + self.done = true; + })?; + + let Some(result) = result.as_object() else { + self.done = true; + return Err(js_error!(TypeError: "next value should be an object")); + }; + + let done = result + .get(js_string!("done"), context) + .inspect_err(|_| { + self.done = true; + })? + .to_boolean(); + let value = result.get(js_string!("value"), context).inspect_err(|_| { + self.done = true; + })?; + + self.done = done; + + Ok(IteratorStep { done, value }) + } + + fn step_value(&mut self, context: &mut Context) -> JsResult> { + let result = self.next_result(None, context)?; + Ok((!result.done).then_some(result.value)) + } + + fn close(&self, completion: JsResult, context: &mut Context) -> JsResult { + let return_method = match get_method(&self.iterator, js_string!("return"), context) { + Ok(Some(return_method)) => { + return_method.call(&self.iterator.clone().into(), &[], context) + } + Ok(None) => return completion, + Err(err) => { + completion?; + return Err(err); + } + }; + + let completion = completion?; + let return_value = return_method?; + + if return_value.is_object() { + Ok(completion) + } else { + Err(js_error!(TypeError: "inner result was not an object")) + } } +} - Ok(true) +fn close_iterator_with_error( + iterator: &SyncIterator, + error: JsError, + context: &mut Context, +) -> JsResult { + match iterator.close(Err(error), context) { + Ok(_) => unreachable!("iterator close with error completion should not succeed"), + Err(err) => Err(err), + } } -fn array_from(value: &JsValue, context: &mut Context) -> JsResult { - let array = context - .global_object() - .get(js_string!("Array"), context)? - .to_object(context)?; - let from = array - .get(js_string!("from"), context)? - .as_object() - .ok_or_else(|| js_error!(Error: "Array.from should be callable"))?; - - let value = from.call(&array.clone().into(), std::slice::from_ref(value), context)?; - JsArray::from_object(value.to_object(context)?) +fn collect_sequence_item_pair( + item: &JsObject, + context: &mut Context, +) -> JsResult<(JsString, JsString)> { + let Some(iterator_method) = get_method(item, JsSymbol::iterator(), context)? else { + return Err(js_error!( + TypeError: "URLSearchParams constructor expects each sequence item to be an iterable pair" + )); + }; + + let mut pair_iterator = + SyncIterator::from_method(&item.clone().into(), &iterator_method, context)?; + + let Some(name) = pair_iterator.step_value(context)? else { + return Err(js_error!( + TypeError: "URLSearchParams constructor expects each sequence item to contain exactly two values" + )); + }; + let name = match to_usv_string_value(&name, context) { + Ok(name) => name, + Err(err) => return close_iterator_with_error(&pair_iterator, err, context), + }; + + let Some(value) = pair_iterator.step_value(context)? else { + return Err(js_error!( + TypeError: "URLSearchParams constructor expects each sequence item to contain exactly two values" + )); + }; + let value = match to_usv_string_value(&value, context) { + Ok(value) => value, + Err(err) => return close_iterator_with_error(&pair_iterator, err, context), + }; + + if pair_iterator.step_value(context)?.is_some() { + return close_iterator_with_error( + &pair_iterator, + js_error!( + TypeError: "URLSearchParams constructor expects each sequence item to contain exactly two values" + ), + context, + ); + } + + Ok((name, value)) } fn collect_sequence_pairs( init: &JsValue, + iterator_method: &JsObject, context: &mut Context, ) -> JsResult> { - let items = array_from(init, context)?; - let length = usize::try_from(items.length(context)?) - .map_err(|_| js_error!(RangeError: "URLSearchParams sequence is too large"))?; - let mut pairs = Vec::with_capacity(length); + let mut items = SyncIterator::from_method(init, iterator_method, context)?; + let mut pairs = Vec::new(); - for index in 0..length { - let item = items.get(index, context)?; + while let Some(item) = items.step_value(context)? { let Some(item_object) = item.as_object() else { - return Err(js_error!( - TypeError: "URLSearchParams constructor expects each sequence item to be an iterable pair" - )); + return close_iterator_with_error( + &items, + js_error!( + TypeError: "URLSearchParams constructor expects each sequence item to be an iterable pair" + ), + context, + ); }; - if !has_callable_iterator(&item_object, context)? { - return Err(js_error!( - TypeError: "URLSearchParams constructor expects each sequence item to be an iterable pair" - )); - } - - let pair = array_from(&item, context)?; - if pair.length(context)? != 2 { - return Err(js_error!( - TypeError: "URLSearchParams constructor expects each sequence item to contain exactly two values" - )); - } - - let name = to_usv_string_value(&pair.get(0, context)?, context)?; - let value = to_usv_string_value(&pair.get(1, context)?, context)?; - pairs.push((name, value)); + let pair = match collect_sequence_item_pair(&item_object, context) { + Ok(pair) => pair, + Err(err) => return close_iterator_with_error(&items, err, context), + }; + pairs.push(pair); } Ok(pairs) @@ -336,10 +491,8 @@ impl UrlSearchParams { let list = if init.is_undefined() || init.is_null() { Vec::new() } else if let Some(object) = init.as_object() { - if let Some(other) = object.downcast_ref::() { - other.pairs() - } else if has_callable_iterator(&object, context)? { - collect_sequence_pairs(&init, context)? + if let Some(iterator_method) = get_method(&object, JsSymbol::iterator(), context)? { + collect_sequence_pairs(&init, &iterator_method, context)? } else { collect_record_pairs(&object, context)? } @@ -361,9 +514,19 @@ impl UrlSearchParams { self.update(pairs); } - fn delete(&mut self, name: Convert, value: Option>) { + fn delete( + &mut self, + name: Convert, + value: OptionalArg, + context: &mut Context, + ) -> JsResult<()> { let name = to_usv_string(&name.0); - let value = value.as_ref().map(|value| to_usv_string(&value.0)); + let value = value + .0 + .as_ref() + .map(|value| value.try_js_into::>(context)) + .transpose()? + .map(|value| to_usv_string(&value.0)); let mut pairs = self.pairs(); match value { @@ -378,6 +541,7 @@ impl UrlSearchParams { } self.update(pairs); + Ok(()) } fn entries(this: JsClass, context: &mut Context) -> JsValue { @@ -431,10 +595,21 @@ impl UrlSearchParams { .collect() } - fn has(&self, name: Convert, value: Option>) -> bool { + fn has( + &self, + name: Convert, + value: OptionalArg, + context: &mut Context, + ) -> JsResult { let name = to_usv_string(&name.0); - let value = value.as_ref().map(|value| to_usv_string(&value.0)); - match value { + let value = value + .0 + .as_ref() + .map(|value| value.try_js_into::>(context)) + .transpose()? + .map(|value| to_usv_string(&value.0)); + + Ok(match value { Some(value) => self .pairs() .into_iter() @@ -445,7 +620,7 @@ impl UrlSearchParams { .pairs() .into_iter() .any(|(existing_name, _)| existing_name == name), - } + }) } #[boa(symbol = "iterator")] diff --git a/core/runtime/src/url/tests.rs b/core/runtime/src/url/tests.rs index 575efd830ca..d6f53f26d9e 100644 --- a/core/runtime/src/url/tests.rs +++ b/core/runtime/src/url/tests.rs @@ -171,6 +171,31 @@ fn url_search_params_constructor_and_methods() { assert_eq(new URLSearchParams(record).toString(), "a=1"); "##, ), + TestAction::run( + r##" + const originalFrom = Array.from; + Array.from = () => { + throw new Error("patched"); + }; + + try { + const params = new URLSearchParams([["a", "1"], new Set(["b", "2"])]); + assert_eq(params.toString(), "a=1&b=2"); + } finally { + Array.from = originalFrom; + } + "##, + ), + TestAction::run( + r##" + const customParams = new URLSearchParams("x=1"); + customParams[Symbol.iterator] = function* () { + yield ["a", "b"]; + }; + + assert_eq(new URLSearchParams(customParams).toString(), "a=b"); + "##, + ), ]); } @@ -231,6 +256,31 @@ fn url_search_params_is_live_and_cached() { ]); } +#[test] +fn url_search_params_optional_value_argument() { + run_test_actions([ + TestAction::run(TEST_HARNESS), + TestAction::run( + r##" + const params = new URLSearchParams("a=b&a=undefined&b=c"); + + assert(params.has("a", undefined)); + assert(!params.has("a", "missing")); + + params.delete("a", undefined); + assert_eq(params.toString(), "a=b&b=c"); + "##, + ), + TestAction::run( + r##" + const deleteAllParams = new URLSearchParams("a=b&a=undefined&a=d"); + deleteAllParams.delete("a"); + assert_eq(deleteAllParams.toString(), ""); + "##, + ), + ]); +} + #[test] fn url_search_params_constructor_errors() { run_test_actions([ From 91b48b668eb1e87e441c5fac73ebf65231c790db Mon Sep 17 00:00:00 2001 From: Monti-27 Date: Fri, 13 Mar 2026 22:55:51 +0530 Subject: [PATCH 3/7] add url search params tests --- core/runtime/src/url/tests.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/runtime/src/url/tests.rs b/core/runtime/src/url/tests.rs index d6f53f26d9e..4933f02921e 100644 --- a/core/runtime/src/url/tests.rs +++ b/core/runtime/src/url/tests.rs @@ -278,6 +278,14 @@ fn url_search_params_optional_value_argument() { assert_eq(deleteAllParams.toString(), ""); "##, ), + TestAction::run( + r##" + const url = new URL("https://example.com/?a=undefined&a=x"); + assert(url.searchParams.has("a", undefined)); + url.searchParams.delete("a", undefined); + assert_eq(url.search, "?a=x"); + "##, + ), ]); } From 2fa3af502c8b71c0575989bcbaf7b15a6e96fac2 Mon Sep 17 00:00:00 2001 From: Monti-27 Date: Tue, 17 Mar 2026 15:00:11 +0530 Subject: [PATCH 4/7] update url search params init --- core/runtime/src/url.rs | 161 +++++++++++++++++++++++----------------- tests/wpt/src/lib.rs | 1 - 2 files changed, 91 insertions(+), 71 deletions(-) diff --git a/core/runtime/src/url.rs b/core/runtime/src/url.rs index 090aa143123..cd0af7ab8e8 100644 --- a/core/runtime/src/url.rs +++ b/core/runtime/src/url.rs @@ -29,7 +29,9 @@ use boa_engine::{ Context, Finalize, JsData, JsError, JsObject, JsResult, JsString, JsSymbol, JsValue, Trace, boa_class, boa_module, js_error, js_string, native_function::NativeFunction, }; +use std::cell::RefCell; use std::fmt::Display; +use std::rc::Rc; /// A callback function for the `URLSearchParams.prototype.forEach` method. pub type SearchParamsForEachCallback = TypedJsFunction<(JsString, JsString, JsObject), ()>; @@ -330,11 +332,12 @@ fn collect_record_pairs( #[derive(Debug, JsData, Trace, Finalize)] pub struct UrlSearchParams { list: Vec<(JsString, JsString)>, - url: Option>, + #[unsafe_ignore_trace] + url: Option>>, } impl UrlSearchParams { - fn from_url(url: JsObject, context: &mut Context) -> JsResult> { + fn from_url(url: Rc>, context: &mut Context) -> JsResult> { Self::from_data( Self { list: Vec::new(), @@ -350,8 +353,6 @@ impl UrlSearchParams { if let Some(url) = &self.url { let url = url.borrow(); return url - .data() - .inner .query_pairs() .map(|(name, value)| { ( @@ -368,13 +369,12 @@ impl UrlSearchParams { fn update(&mut self, pairs: Vec<(JsString, JsString)>) { if let Some(url) = &self.url { let mut url = url.borrow_mut(); - let url = url.data_mut(); if pairs.is_empty() { - url.inner.set_query(None); + url.set_query(None); } else { let query = serialize_search_params(&pairs); - url.inner.set_query(Some(&query)); + url.set_query(Some(&query)); } return; } @@ -676,7 +676,7 @@ impl UrlSearchParams { #[boa_gc(unsafe_no_drop)] pub struct Url { #[unsafe_ignore_trace] - inner: url::Url, + inner: Rc>, search_params: Option>, } @@ -689,26 +689,54 @@ impl Url { pub fn register(realm: Option, context: &mut Context) -> JsResult<()> { js_module::boa_register(realm, context) } -} -impl Display for Url { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.inner) + /// Create a native `Url` value from Rust code. + pub fn new(Convert(ref url): Convert, base: Option>) -> JsResult { + Ok(Self { + inner: Rc::new(RefCell::new(Self::parse_url( + url, + base.as_ref().map(|base| base.0.as_str()), + )?)), + search_params: None, + }) } -} -impl From for Url { - fn from(url: url::Url) -> Self { - Self { - inner: url, - search_params: None, + /// Create a JavaScript `URL` object from native `Url` data. + pub fn from_data(mut data: Self, context: &mut Context) -> JsResult { + if data.search_params.is_none() { + data.search_params = Some(UrlSearchParams::from_url(data.inner.clone(), context)?); } + + ::from_data(data, context) + } + + fn parse_url(url: &str, base: Option<&str>) -> JsResult { + if let Some(base) = base { + let base_url = url::Url::parse(base) + .map_err(|e| js_error!(TypeError: "Failed to parse base URL: {}", e))?; + + base_url + .join(url) + .map_err(|e| js_error!(TypeError: "Failed to parse URL: {}", e)) + } else { + url::Url::parse(url).map_err(|e| js_error!(TypeError: "Failed to parse URL: {}", e)) + } + } + + fn from_parsed(url: url::Url, context: &mut Context) -> JsResult { + let inner = Rc::new(RefCell::new(url)); + let search_params = UrlSearchParams::from_url(inner.clone(), context)?; + + Ok(Self { + inner, + search_params: Some(search_params), + }) } } -impl From for url::Url { - fn from(url: Url) -> url::Url { - url.inner +impl Display for Url { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.inner.borrow()) } } @@ -720,156 +748,148 @@ impl Url { /// # Errors /// Any errors that might occur during URL parsing. #[boa(constructor)] - pub fn new(Convert(ref url): Convert, base: Option>) -> JsResult { - if let Some(Convert(ref base)) = base { - let base_url = url::Url::parse(base) - .map_err(|e| js_error!(TypeError: "Failed to parse base URL: {}", e))?; - - let url = base_url - .join(url) - .map_err(|e| js_error!(TypeError: "Failed to parse URL: {}", e))?; - Ok(Self::from(url)) - } else { - let url = url::Url::parse(url) - .map_err(|e| js_error!(TypeError: "Failed to parse URL: {}", e))?; - Ok(Self::from(url)) - } + pub fn constructor( + Convert(ref url): Convert, + base: Option>, + context: &mut Context, + ) -> JsResult { + Self::from_parsed( + Self::parse_url(url, base.as_ref().map(|base| base.0.as_str()))?, + context, + ) } #[boa(getter)] fn hash(&self) -> JsString { - JsString::from(url::quirks::hash(&self.inner)) + JsString::from(url::quirks::hash(&self.inner.borrow())) } #[boa(setter)] #[boa(rename = "hash")] fn set_hash(&mut self, value: Convert) { - url::quirks::set_hash(&mut self.inner, &value.0); + url::quirks::set_hash(&mut self.inner.borrow_mut(), &value.0); } #[boa(getter)] fn hostname(&self) -> JsString { - JsString::from(url::quirks::hostname(&self.inner)) + JsString::from(url::quirks::hostname(&self.inner.borrow())) } #[boa(setter)] #[boa(rename = "hostname")] fn set_hostname(&mut self, value: Convert) { - let _ = url::quirks::set_hostname(&mut self.inner, &value.0); + let _ = url::quirks::set_hostname(&mut self.inner.borrow_mut(), &value.0); } #[boa(getter)] fn host(&self) -> JsString { - JsString::from(url::quirks::host(&self.inner)) + JsString::from(url::quirks::host(&self.inner.borrow())) } #[boa(setter)] #[boa(rename = "host")] fn set_host(&mut self, value: Convert) { - let _ = url::quirks::set_host(&mut self.inner, &value.0); + let _ = url::quirks::set_host(&mut self.inner.borrow_mut(), &value.0); } #[boa(getter)] fn href(&self) -> JsString { - JsString::from(url::quirks::href(&self.inner)) + JsString::from(url::quirks::href(&self.inner.borrow())) } #[boa(setter)] #[boa(rename = "href")] fn set_href(&mut self, value: Convert) -> JsResult<()> { - url::quirks::set_href(&mut self.inner, &value.0) + url::quirks::set_href(&mut self.inner.borrow_mut(), &value.0) .map_err(|e| js_error!(TypeError: "Failed to set href: {}", e)) } #[boa(getter)] fn origin(&self) -> JsString { - JsString::from(url::quirks::origin(&self.inner)) + JsString::from(url::quirks::origin(&self.inner.borrow())) } #[boa(getter)] fn password(&self) -> JsString { - JsString::from(url::quirks::password(&self.inner)) + JsString::from(url::quirks::password(&self.inner.borrow())) } #[boa(setter)] #[boa(rename = "password")] fn set_password(&mut self, value: Convert) { - let _ = url::quirks::set_password(&mut self.inner, &value.0); + let _ = url::quirks::set_password(&mut self.inner.borrow_mut(), &value.0); } #[boa(getter)] fn pathname(&self) -> JsString { - JsString::from(url::quirks::pathname(&self.inner)) + JsString::from(url::quirks::pathname(&self.inner.borrow())) } #[boa(setter)] #[boa(rename = "pathname")] fn set_pathname(&mut self, value: Convert) { - let () = url::quirks::set_pathname(&mut self.inner, &value.0); + let () = url::quirks::set_pathname(&mut self.inner.borrow_mut(), &value.0); } #[boa(getter)] fn port(&self) -> JsString { - JsString::from(url::quirks::port(&self.inner)) + JsString::from(url::quirks::port(&self.inner.borrow())) } #[boa(setter)] #[boa(rename = "port")] fn set_port(&mut self, value: Convert) { - let _ = url::quirks::set_port(&mut self.inner, &value.0.to_std_string_lossy()); + let _ = url::quirks::set_port(&mut self.inner.borrow_mut(), &value.0.to_std_string_lossy()); } #[boa(getter)] fn protocol(&self) -> JsString { - JsString::from(url::quirks::protocol(&self.inner)) + JsString::from(url::quirks::protocol(&self.inner.borrow())) } #[boa(setter)] #[boa(rename = "protocol")] fn set_protocol(&mut self, value: Convert) { - let _ = url::quirks::set_protocol(&mut self.inner, &value.0); + let _ = url::quirks::set_protocol(&mut self.inner.borrow_mut(), &value.0); } #[boa(getter)] fn search(&self) -> JsString { - JsString::from(url::quirks::search(&self.inner)) + JsString::from(url::quirks::search(&self.inner.borrow())) } #[boa(setter)] #[boa(rename = "search")] fn set_search(&mut self, value: Convert) { - url::quirks::set_search(&mut self.inner, &value.0); + url::quirks::set_search(&mut self.inner.borrow_mut(), &value.0); } #[boa(getter)] - fn search_params(this: JsClass, context: &mut Context) -> JsResult { - if let Some(existing) = this.borrow().search_params.clone() { - return Ok(existing.into()); - } - - let params = UrlSearchParams::from_url(this.inner(), context)?; - this.borrow_mut().search_params = Some(params.clone()); - Ok(params.into()) + fn search_params(&self) -> JsValue { + self.search_params + .clone() + .expect("URL.searchParams should be initialized during construction") + .into() } #[boa(getter)] fn username(&self) -> JsString { - JsString::from(self.inner.username()) + JsString::from(self.inner.borrow().username()) } #[boa(setter)] #[boa(rename = "username")] fn set_username(&mut self, value: Convert) { - let _ = self.inner.set_username(&value.0); + let _ = self.inner.borrow_mut().set_username(&value.0); } fn to_string(&self) -> JsString { - JsString::from(format!("{}", self.inner)) + JsString::from(format!("{}", self.inner.borrow())) } #[boa(rename = "toJSON")] fn to_json(&self) -> JsString { - JsString::from(format!("{}", self.inner)) + JsString::from(format!("{}", self.inner.borrow())) } #[boa(static)] @@ -879,7 +899,7 @@ impl Url { #[boa(static)] fn can_parse(url: Convert, base: Option>) -> bool { - Url::new(url, base).is_ok() + Self::parse_url(&url.0, base.as_ref().map(|base| base.0.as_str())).is_ok() } #[boa(static)] @@ -888,9 +908,10 @@ impl Url { base: Option>, context: &mut Context, ) -> JsResult { - Url::new(url, base).map_or(Ok(JsValue::null()), |u| { - Url::from_data(u, context).map(JsValue::from) - }) + Self::parse_url(&url.0, base.as_ref().map(|base| base.0.as_str())) + .map_or(Ok(JsValue::null()), |url| { + Self::from_data(Self::from_parsed(url, context)?, context).map(JsValue::from) + }) } #[boa(static)] diff --git a/tests/wpt/src/lib.rs b/tests/wpt/src/lib.rs index d24ed44dba0..16bda6e8574 100644 --- a/tests/wpt/src/lib.rs +++ b/tests/wpt/src/lib.rs @@ -1,7 +1,6 @@ //! Integration tests running the Web Platform Tests (WPT) for the `boa_runtime` crate. #![allow(unused_crate_dependencies)] -use boa_engine::class::Class; use boa_engine::interop::ContextData; use boa_engine::parser::source::UTF16Input; use boa_engine::property::Attribute; From a594311f4748c1f5461953ea54b30877a73f8538 Mon Sep 17 00:00:00 2001 From: Monti-27 Date: Wed, 18 Mar 2026 00:31:59 +0530 Subject: [PATCH 5/7] fix url review comments --- core/engine/src/object/operations.rs | 2 +- core/runtime/src/url.rs | 57 +++++++++++++++++----------- 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/core/engine/src/object/operations.rs b/core/engine/src/object/operations.rs index 4956adc5cad..ff36ee30ef0 100644 --- a/core/engine/src/object/operations.rs +++ b/core/engine/src/object/operations.rs @@ -791,7 +791,7 @@ impl JsObject { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-getmethod - pub(crate) fn get_method(&self, key: K, context: &mut Context) -> JsResult> + pub fn get_method(&self, key: K, context: &mut Context) -> JsResult> where K: Into, { diff --git a/core/runtime/src/url.rs b/core/runtime/src/url.rs index cd0af7ab8e8..b55cd741975 100644 --- a/core/runtime/src/url.rs +++ b/core/runtime/src/url.rs @@ -7,6 +7,10 @@ //! - [MDN documentation][mdn] //! - [WHATWG `URL` specification][spec] //! +//! Implemented sections in this file: +//! - `URL.searchParams`: +//! - `URLSearchParams` constructor and methods: +//! //! [spec]: https://url.spec.whatwg.org/ //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/API/URL #![allow(clippy::needless_pass_by_value)] @@ -79,6 +83,10 @@ fn serialize_search_params(params: &[(JsString, JsString)]) -> String { serializer.finish() } +/// Captures whether an optional argument was actually supplied. +/// +/// This is used for overloads where an explicit `undefined` must not be treated +/// the same as an omitted argument. #[derive(Debug, Clone)] struct OptionalArg(Option); @@ -108,24 +116,6 @@ struct IteratorStep { value: JsValue, } -fn get_method(object: &JsObject, key: K, context: &mut Context) -> JsResult> -where - K: Into, -{ - let method = object.get(key, context)?; - if method.is_null_or_undefined() { - return Ok(None); - } - - let Some(method) = method.as_object().filter(JsObject::is_callable) else { - return Err(js_error!( - TypeError: "value returned for property of object is not a function" - )); - }; - - Ok(Some(method.clone())) -} - impl SyncIterator { fn from_method( value: &JsValue, @@ -193,7 +183,7 @@ impl SyncIterator { } fn close(&self, completion: JsResult, context: &mut Context) -> JsResult { - let return_method = match get_method(&self.iterator, js_string!("return"), context) { + let return_method = match self.iterator.get_method(js_string!("return"), context) { Ok(Some(return_method)) => { return_method.call(&self.iterator.clone().into(), &[], context) } @@ -230,7 +220,7 @@ fn collect_sequence_item_pair( item: &JsObject, context: &mut Context, ) -> JsResult<(JsString, JsString)> { - let Some(iterator_method) = get_method(item, JsSymbol::iterator(), context)? else { + let Some(iterator_method) = item.get_method(JsSymbol::iterator(), context)? else { return Err(js_error!( TypeError: "URLSearchParams constructor expects each sequence item to be an iterable pair" )); @@ -486,12 +476,19 @@ impl UrlSearchParamsIterator { #[boa_class(rename = "URLSearchParams")] #[boa(rename_all = "camelCase")] impl UrlSearchParams { + /// WHATWG URL: + /// + /// This implements the constructor branches for: + /// - empty / null init + /// - sequence input via @@iterator + /// - record input via enumerable own properties + /// - string input parsed as application/x-www-form-urlencoded #[boa(constructor)] fn constructor(init: JsValue, context: &mut Context) -> JsResult { let list = if init.is_undefined() || init.is_null() { Vec::new() } else if let Some(object) = init.as_object() { - if let Some(iterator_method) = get_method(&object, JsSymbol::iterator(), context)? { + if let Some(iterator_method) = object.get_method(JsSymbol::iterator(), context)? { collect_sequence_pairs(&init, &iterator_method, context)? } else { collect_record_pairs(&object, context)? @@ -520,6 +517,8 @@ impl UrlSearchParams { value: OptionalArg, context: &mut Context, ) -> JsResult<()> { + // WHATWG URL: + // The second argument only participates when it was actually supplied. let name = to_usv_string(&name.0); let value = value .0 @@ -601,6 +600,8 @@ impl UrlSearchParams { value: OptionalArg, context: &mut Context, ) -> JsResult { + // WHATWG URL: + // The second argument only participates when it was actually supplied. let name = to_usv_string(&name.0); let value = value .0 @@ -691,6 +692,9 @@ impl Url { } /// Create a native `Url` value from Rust code. + /// + /// # Errors + /// Returns an error if `url` cannot be parsed against the optional `base`. pub fn new(Convert(ref url): Convert, base: Option>) -> JsResult { Ok(Self { inner: Rc::new(RefCell::new(Self::parse_url( @@ -702,6 +706,10 @@ impl Url { } /// Create a JavaScript `URL` object from native `Url` data. + /// + /// # Errors + /// Returns an error if the object or its eagerly-created `searchParams` + /// view cannot be allocated. pub fn from_data(mut data: Self, context: &mut Context) -> JsResult { if data.search_params.is_none() { data.search_params = Some(UrlSearchParams::from_url(data.inner.clone(), context)?); @@ -865,11 +873,14 @@ impl Url { } #[boa(getter)] - fn search_params(&self) -> JsValue { + fn search_params(&self) -> JsObject { + // WHATWG URL: + // `searchParams` is created during URL construction and the getter + // returns that same live object. self.search_params .clone() .expect("URL.searchParams should be initialized during construction") - .into() + .upcast() } #[boa(getter)] From 7f33963c2d5c8442984d164f1a519e993369f280 Mon Sep 17 00:00:00 2001 From: Monti-27 Date: Thu, 19 Mar 2026 23:14:03 +0530 Subject: [PATCH 6/7] add url coverage tests --- core/runtime/src/url/tests.rs | 231 ++++++++++++++++++++++++++++++++++ 1 file changed, 231 insertions(+) diff --git a/core/runtime/src/url/tests.rs b/core/runtime/src/url/tests.rs index 4933f02921e..bae904e02e1 100644 --- a/core/runtime/src/url/tests.rs +++ b/core/runtime/src/url/tests.rs @@ -116,6 +116,17 @@ fn url_static_methods() { assert(URL.canParse("http://example.org/new/path?new-query#new-fragment", "about:blank")); "##, ), + TestAction::run( + r##" + const parsed = URL.parse("https://example.org/?a=1"); + assert(parsed instanceof URL); + assert_eq(parsed.searchParams.get("a"), "1"); + + parsed.searchParams.append("b", "2"); + assert_eq(parsed.href, "https://example.org/?a=1&b=2"); + assert_eq(URL.parse("http//:example.org/"), null); + "##, + ), ]); } @@ -256,6 +267,57 @@ fn url_search_params_is_live_and_cached() { ]); } +#[test] +fn url_search_params_iterators() { + run_test_actions([ + TestAction::run(TEST_HARNESS), + TestAction::run( + r##" + const params = new URLSearchParams([["a", "1"], ["b", "2"]]); + const entries = params.entries(); + + assert(entries[Symbol.iterator]() === entries, "iterator should return itself"); + assert_eq(Object.prototype.toString.call(entries), "[object URLSearchParams Iterator]"); + + assert_eq(entries.next().value.join("="), "a=1"); + assert_eq(entries.next().value.join("="), "b=2"); + + const done = entries.next(); + assert(done.done); + assert_eq(done.value, undefined); + + const doneAgain = entries.next(); + assert(doneAgain.done); + assert_eq(doneAgain.value, undefined); + + assert_eq([...params.keys()].join(","), "a,b"); + assert_eq([...params.values()].join(","), "1,2"); + "##, + ), + TestAction::run( + r##" + const next = new URLSearchParams("a=1").entries().next; + + let primitiveThisThrew = false; + try { + next.call(1); + } catch (e) { + primitiveThisThrew = e instanceof TypeError; + } + assert(primitiveThisThrew, "calling iterator next with primitive this must throw"); + + let wrongObjectThrew = false; + try { + next.call({}); + } catch (e) { + wrongObjectThrew = e instanceof TypeError; + } + assert(wrongObjectThrew, "calling iterator next with a plain object must throw"); + "##, + ), + ]); +} + #[test] fn url_search_params_optional_value_argument() { run_test_actions([ @@ -293,6 +355,12 @@ fn url_search_params_optional_value_argument() { fn url_search_params_constructor_errors() { run_test_actions([ TestAction::run(TEST_HARNESS), + TestAction::run( + r##" + assert_eq(new URLSearchParams().toString(), ""); + assert_eq(new URLSearchParams(null).toString(), ""); + "##, + ), TestAction::run( r##" var threw = false; @@ -338,5 +406,168 @@ fn url_search_params_constructor_errors() { assert(threw, "enumerable symbol keys must throw during record conversion"); "##, ), + TestAction::run( + r##" + var cleanup = []; + var threw = false; + + try { + new URLSearchParams({ + [Symbol.iterator]() { + return { + next() { + return { value: 1, done: false }; + }, + return() { + cleanup.push("outer"); + return {}; + }, + }; + }, + }); + } catch (e) { + threw = e instanceof TypeError; + } + + assert(threw, "non-object sequence items must throw"); + assert_eq(cleanup.join(","), "outer"); + "##, + ), + TestAction::run( + r##" + var cleanup = []; + var threw = false; + + try { + new URLSearchParams({ + [Symbol.iterator]() { + return { + next() { + return { + value: { + [Symbol.iterator]() { + let step = 0; + return { + next() { + step += 1; + if (step === 1) { + return { value: "a", done: false }; + } + if (step === 2) { + return { + value: { + toString() { + throw new Error("boom"); + }, + }, + done: false, + }; + } + return { done: true }; + }, + return() { + cleanup.push("pair"); + return {}; + }, + }; + }, + }, + done: false, + }; + }, + return() { + cleanup.push("outer"); + return {}; + }, + }; + }, + }); + } catch (e) { + threw = e instanceof Error && e.message === "boom"; + } + + assert(threw, "errors from pair value coercion must propagate"); + assert_eq(cleanup.join(","), "pair,outer"); + "##, + ), + TestAction::run( + r##" + var threw = false; + + try { + new URLSearchParams({ + [Symbol.iterator]() { + return 1; + }, + }); + } catch (e) { + threw = e instanceof TypeError; + } + + assert(threw, "iterator methods must return an object"); + "##, + ), + TestAction::run( + r##" + var threw = false; + + try { + new URLSearchParams({ + [Symbol.iterator]() { + return { next: 1 }; + }, + }); + } catch (e) { + threw = e instanceof TypeError; + } + + assert(threw, "iterator next must be callable"); + "##, + ), + TestAction::run( + r##" + var threw = false; + + try { + new URLSearchParams({ + [Symbol.iterator]() { + return { + next() { + return 1; + }, + }; + }, + }); + } catch (e) { + threw = e instanceof TypeError; + } + + assert(threw, "iterator next result must be an object"); + "##, + ), + TestAction::run( + r##" + var threw = false; + + try { + new URLSearchParams({ + [Symbol.iterator]() { + return { + next() { + return { value: 1, done: false }; + }, + return() { + return 1; + }, + }; + }, + }); + } catch (e) { + threw = e instanceof TypeError; + } + + assert(threw, "iterator return must produce an object"); + "##, + ), ]); } From de17de0091100b6773e766636567fc35292ec186 Mon Sep 17 00:00:00 2001 From: Monti-27 Date: Thu, 19 Mar 2026 23:34:03 +0530 Subject: [PATCH 7/7] split url coverage tests --- core/runtime/src/url/tests.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/core/runtime/src/url/tests.rs b/core/runtime/src/url/tests.rs index bae904e02e1..876be762220 100644 --- a/core/runtime/src/url/tests.rs +++ b/core/runtime/src/url/tests.rs @@ -406,6 +406,13 @@ fn url_search_params_constructor_errors() { assert(threw, "enumerable symbol keys must throw during record conversion"); "##, ), + ]); +} + +#[test] +fn url_search_params_constructor_iterator_cleanup_errors() { + run_test_actions([ + TestAction::run(TEST_HARNESS), TestAction::run( r##" var cleanup = []; @@ -490,6 +497,13 @@ fn url_search_params_constructor_errors() { assert_eq(cleanup.join(","), "pair,outer"); "##, ), + ]); +} + +#[test] +fn url_search_params_constructor_iterator_protocol_errors() { + run_test_actions([ + TestAction::run(TEST_HARNESS), TestAction::run( r##" var threw = false;