Skip to content

Commit df1f0ff

Browse files
fix(runtime): implement spec-compliant HeadersIterator for Headers API (#5003)
## Summary Fix `Headers.entries()`, `Headers.keys()`, and `Headers.values()` so they return proper iterator objects instead of arrays. Closes #4989 ## Problem `Headers.entries()`, `Headers.keys()`, and `Headers.values()` were not returning iterator objects. Current behavior before this change: - `entries()` returned an array - `keys()` and `values()` returned collected values without a callable `.next()` - `Headers[Symbol.iterator]()` already worked through a workaround Because of this, calls like `headers.entries().next()` failed. ## What changed - Added a `HeadersIterator` implementation for fetch headers - Updated `Headers.entries()`, `Headers.keys()`, and `Headers.values()` to return iterator objects - Kept `Headers[Symbol.iterator]()` aligned with iterator behavior - Added regression tests covering iterator behavior and `for...of` usage ## Tests Added/updated tests for: - `entries()` returns an iterator - `keys()` returns an iterator - `values()` returns an iterator - iterator results have the correct `{ value, done }` shape - `[Symbol.iterator]()` still works - `for...of` works with headers iteration Ran locally: ```bash cargo test ``` ## Specification According to the Fetch Standard, `Headers.entries()`, `Headers.keys()`, and `Headers.values()` must return iterator objects. Reference: https://fetch.spec.whatwg.org/#headers-class This change aligns Boa's `Headers` implementation with the expected iterator semantics used by other JavaScript runtimes.
1 parent 248bbed commit df1f0ff

4 files changed

Lines changed: 205 additions & 47 deletions

File tree

core/runtime/src/fetch/headers.rs

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
//! See <https://developer.mozilla.org/en-US/docs/Web/API/Headers>.
44
#![allow(clippy::needless_pass_by_value)]
55

6+
use super::headers_iterator::{HeadersIterator, IterationKind};
67
use boa_engine::interop::JsClass;
7-
use boa_engine::object::builtins::{JsArray, TypedJsFunction};
8+
use boa_engine::object::builtins::TypedJsFunction;
89
use boa_engine::value::{Convert, TryFromJs};
910
use boa_engine::{
1011
Context, Finalize, JsData, JsObject, JsResult, JsString, JsValue, Trace, boa_class, js_error,
@@ -49,7 +50,7 @@ fn to_header_value(value: impl AsRef<str>) -> JsResult<HeaderValue> {
4950
#[derive(Debug, Default, Clone, JsData, Trace, Finalize)]
5051
pub struct JsHeaders {
5152
#[unsafe_ignore_trace]
52-
headers: Rc<RefCell<HttpHeaderMap>>,
53+
pub(crate) headers: Rc<RefCell<HttpHeaderMap>>,
5354
}
5455

5556
impl TryFromJs for JsHeaders {
@@ -159,21 +160,13 @@ impl JsHeaders {
159160
}
160161

161162
/// Returns an iterator allowing to go through all key/value pairs contained in this object.
162-
// TODO: This should return a JsIterator, but not such thing exists yet.
163-
pub fn entries(&self, context: &mut Context) -> JsValue {
164-
JsArray::from_iter(
165-
self.headers
166-
.borrow()
167-
.iter()
168-
.map(|(k, v)| {
169-
let k: JsValue = JsString::from(k.as_str()).into();
170-
let v: JsValue = JsString::from(v.to_str().unwrap_or_default()).into();
171-
JsArray::from_iter([k, v], context).into()
172-
})
173-
.collect::<Vec<_>>(),
174-
context,
175-
)
176-
.into()
163+
///
164+
/// # Errors
165+
///
166+
/// Returns an error if the headers iterator cannot be created.
167+
#[boa(method)]
168+
pub fn entries(this: JsClass<Self>, context: &mut Context) -> JsResult<JsValue> {
169+
HeadersIterator::create_headers_iterator(this.inner(), IterationKind::KeyAndValue, context)
177170
}
178171

179172
/// Executes a provided function once for each key/value pair in the Headers object.
@@ -246,13 +239,13 @@ impl JsHeaders {
246239

247240
/// Returns an iterator allowing you to go through all keys of the key/value pairs
248241
/// contained in this object.
249-
#[allow(clippy::unused_self)]
250-
fn keys(&self) -> Vec<JsString> {
251-
self.headers
252-
.borrow()
253-
.keys()
254-
.map(|k| JsString::from(k.as_str()))
255-
.collect()
242+
///
243+
/// # Errors
244+
///
245+
/// Returns an error if the headers iterator cannot be created.
246+
#[boa(method)]
247+
fn keys(this: JsClass<Self>, context: &mut Context) -> JsResult<JsValue> {
248+
HeadersIterator::create_headers_iterator(this.inner(), IterationKind::Key, context)
256249
}
257250

258251
/// Sets a new value for an existing header inside a Headers object, or adds the
@@ -264,11 +257,24 @@ impl JsHeaders {
264257
Ok(())
265258
}
266259

267-
fn values(&self) -> Vec<JsString> {
268-
self.headers
269-
.borrow()
270-
.values()
271-
.map(|v| JsString::from(v.to_str().unwrap_or("")))
272-
.collect()
260+
/// Returns an iterator allowing you to go through all values of the key/value pairs
261+
/// contained in this object.
262+
///
263+
/// # Errors
264+
///
265+
/// Returns an error if the headers iterator cannot be created.
266+
#[boa(method)]
267+
fn values(this: JsClass<Self>, context: &mut Context) -> JsResult<JsValue> {
268+
HeadersIterator::create_headers_iterator(this.inner(), IterationKind::Value, context)
269+
}
270+
271+
/// `[Symbol.iterator]()` is an alias for `entries()`
272+
///
273+
/// # Errors
274+
///
275+
/// Returns an error if the headers iterator cannot be created.
276+
#[boa(symbol = "iterator")]
277+
fn symbol_iterator(this: JsClass<Self>, context: &mut Context) -> JsResult<JsValue> {
278+
HeadersIterator::create_headers_iterator(this.inner(), IterationKind::KeyAndValue, context)
273279
}
274280
}
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
//! This module implements the `HeadersIterator` object.
2+
//!
3+
//! More information:
4+
//! - [Fetch specification][spec]
5+
//!
6+
//! [spec]: https://fetch.spec.whatwg.org/#headers-class
7+
8+
use boa_engine::{
9+
Context, JsData, JsResult, JsString, JsValue, boa_class,
10+
builtins::iterable::create_iter_result_object, error::JsNativeError, interop::JsClass,
11+
object::JsObject, object::builtins::JsArray,
12+
};
13+
use boa_gc::{Finalize, Trace};
14+
15+
use super::headers::JsHeaders;
16+
17+
/// The `IterationKind` enum represents the different kinds of iteration that can be performed.
18+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
19+
pub(crate) enum IterationKind {
20+
/// Iterates over the keys.
21+
Key,
22+
/// Iterates over the values.
23+
Value,
24+
/// Iterates over the keys and values.
25+
KeyAndValue,
26+
}
27+
28+
/// The Headers Iterator object represents an iteration over a Headers object.
29+
/// It implements the iterator protocol.
30+
///
31+
/// More information:
32+
/// - [Fetch specification][spec]
33+
///
34+
/// [spec]: https://fetch.spec.whatwg.org/#headers-class
35+
#[derive(Debug, Finalize, Trace, JsData)]
36+
pub(crate) struct HeadersIterator {
37+
iterated_headers: JsObject<JsHeaders>,
38+
next_index: usize,
39+
#[unsafe_ignore_trace]
40+
iteration_kind: IterationKind,
41+
}
42+
43+
#[boa_class(rename = "Headers Iterator")]
44+
impl HeadersIterator {
45+
/// Prevent direct construction — `HeadersIterator` instances are only
46+
/// created internally via [`HeadersIterator::create_headers_iterator`].
47+
#[boa(constructor)]
48+
fn constructor() -> JsResult<Self> {
49+
Err(JsNativeError::typ()
50+
.with_message("Illegal constructor")
51+
.into())
52+
}
53+
54+
/// `%HeadersIteratorPrototype%.next()`
55+
///
56+
/// Advances the iterator and returns the next `{ value, done }` result.
57+
#[boa(method)]
58+
fn next(&mut self, context: &mut Context) -> JsValue {
59+
let item_kind = self.iteration_kind;
60+
61+
let element = self
62+
.iterated_headers
63+
.borrow()
64+
.data()
65+
.headers
66+
.borrow()
67+
.iter()
68+
.nth(self.next_index)
69+
.map(|(k, v)| {
70+
(
71+
JsValue::from(JsString::from(k.as_str())),
72+
JsValue::from(JsString::from(v.to_str().unwrap_or(""))),
73+
)
74+
});
75+
76+
if let Some((key, value)) = element {
77+
self.next_index += 1;
78+
79+
return match item_kind {
80+
IterationKind::Key => create_iter_result_object(key, false, context),
81+
IterationKind::Value => create_iter_result_object(value, false, context),
82+
IterationKind::KeyAndValue => {
83+
let result = JsArray::from_iter([key, value], context);
84+
create_iter_result_object(result.into(), false, context)
85+
}
86+
};
87+
}
88+
89+
create_iter_result_object(JsValue::undefined(), true, context)
90+
}
91+
92+
/// Returns `this`, making the iterator itself iterable (`for...of` support).
93+
#[boa(method)]
94+
#[boa(symbol = "iterator")]
95+
fn symbol_iterator(this: JsClass<Self>) -> JsValue {
96+
this.inner().into()
97+
}
98+
}
99+
100+
impl HeadersIterator {
101+
/// Creates a new iterator over the given `Headers` object.
102+
pub(crate) fn create_headers_iterator(
103+
headers: JsObject<JsHeaders>,
104+
kind: IterationKind,
105+
context: &mut Context,
106+
) -> JsResult<JsValue> {
107+
let iter = Self {
108+
iterated_headers: headers,
109+
next_index: 0,
110+
iteration_kind: kind,
111+
};
112+
113+
let proto = context
114+
.realm()
115+
.get_class::<Self>()
116+
.ok_or_else(|| boa_engine::js_error!(Error: "Headers Iterator not registered"))?
117+
.prototype();
118+
119+
let headers_iterator = JsObject::from_proto_and_data(proto, iter);
120+
Ok(headers_iterator.into())
121+
}
122+
}

core/runtime/src/fetch/mod.rs

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88
//! [mdn]: https://developer.mozilla.org/en-US/docs/Web/API/fetch
99
1010
use crate::fetch::headers::JsHeaders;
11+
use crate::fetch::headers_iterator::{HeadersIterator, IterationKind};
1112
use crate::fetch::request::{JsRequest, RequestInit};
1213
use crate::fetch::response::JsResponse;
1314
use boa_engine::class::Class;
1415
use boa_engine::object::FunctionObjectBuilder;
15-
use boa_engine::object::builtins::JsArray;
1616
use boa_engine::property::PropertyDescriptor;
1717
use boa_engine::realm::Realm;
1818
use boa_engine::{
@@ -25,6 +25,7 @@ use std::cell::RefCell;
2525
use std::rc::Rc;
2626

2727
pub mod headers;
28+
pub mod headers_iterator;
2829
pub mod request;
2930
pub mod response;
3031
pub mod tests;
@@ -188,6 +189,7 @@ pub mod js_module {
188189
type JsHeaders = super::JsHeaders;
189190
type JsRequest = super::JsRequest;
190191
type JsResponse = super::JsResponse;
192+
type HeadersIterator = super::headers_iterator::HeadersIterator;
191193

192194
/// The `fetch` function.
193195
///
@@ -207,21 +209,22 @@ pub mod js_module {
207209
}
208210
}
209211

210-
#[doc(inline)]
211-
pub use js_module::fetch;
212-
213-
fn headers_iterator(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult<JsValue> {
214-
let this_object = this.as_object();
215-
let headers = this_object
216-
.as_ref()
217-
.and_then(JsObject::downcast_ref::<JsHeaders>)
218-
.ok_or_else(|| {
219-
js_error!(TypeError: "`Headers.prototype[Symbol.iterator]` requires a `Headers` object")
220-
})?;
221-
222-
let entries = headers.entries(context);
223-
let entries_array = JsArray::from_object(entries.to_object(context)?)?;
224-
entries_array.values(context)
212+
fn headers_symbol_iterator(
213+
this: &JsValue,
214+
_: &[JsValue],
215+
context: &mut Context,
216+
) -> JsResult<JsValue> {
217+
let this_object = this.as_object().ok_or_else(
218+
|| js_error!(TypeError: "`Headers.prototype[Symbol.iterator]` requires a `Headers` object"),
219+
)?;
220+
221+
let Ok(headers) = this_object.clone().downcast::<JsHeaders>() else {
222+
return Err(
223+
js_error!(TypeError: "`Headers.prototype[Symbol.iterator]` requires a `Headers` object"),
224+
);
225+
};
226+
227+
HeadersIterator::create_headers_iterator(headers, IterationKind::KeyAndValue, context)
225228
}
226229

227230
/// Register the `fetch` function in the realm, as well as ALL supporting classes.
@@ -252,7 +255,7 @@ pub fn register<F: Fetcher>(
252255

253256
let iterator = FunctionObjectBuilder::new(
254257
context.realm(),
255-
NativeFunction::from_fn_ptr(headers_iterator),
258+
NativeFunction::from_fn_ptr(headers_symbol_iterator),
256259
)
257260
.name(js_string!("[Symbol.iterator]"))
258261
.length(0)

core/runtime/src/fetch/tests/headers.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,3 +119,30 @@ fn headers_invalid_inputs_throw_type_error_objects() {
119119
),
120120
]);
121121
}
122+
123+
#[test]
124+
fn headers_iterator_throws_on_invalid_this() {
125+
run_test_actions([
126+
TestAction::harness(),
127+
TestAction::inspect_context(register),
128+
TestAction::run(
129+
r#"
130+
try {
131+
const iterator = Headers.prototype[Symbol.iterator].call({});
132+
throw Error("expected the call above to throw");
133+
} catch (e) {
134+
assert(e instanceof TypeError);
135+
assertEq(e.message, "`Headers.prototype[Symbol.iterator]` requires a `Headers` object");
136+
}
137+
138+
try {
139+
const iterator = Headers.prototype[Symbol.iterator].call(1);
140+
throw Error("expected the call above to throw");
141+
} catch (e) {
142+
assert(e instanceof TypeError);
143+
assertEq(e.message, "`Headers.prototype[Symbol.iterator]` requires a `Headers` object");
144+
}
145+
"#,
146+
),
147+
]);
148+
}

0 commit comments

Comments
 (0)