fix(fetch): reject non-null window option in Request constructor#5210
fix(fetch): reject non-null window option in Request constructor#5210HiteshShonak wants to merge 3 commits intoboa-dev:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Boa’s Fetch Request implementation to align with the Fetch Standard requirement that the window option must be null, and adds a regression test to ensure non-null values throw.
Changes:
- Extend
RequestInitwith awindowfield and throw aTypeErrorwhenwindowis non-null. - Add a regression test that checks several non-null
windowvalues throw, whilenullis accepted.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| core/runtime/src/fetch/request.rs | Adds window to RequestInit and validates it during request construction. |
| core/runtime/src/fetch/tests/request.rs | Adds a regression test for Request constructor window option behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Test262 conformance changes
Tested main commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5210 +/- ##
===========================================
+ Coverage 47.24% 59.68% +12.44%
===========================================
Files 476 589 +113
Lines 46892 63480 +16588
===========================================
+ Hits 22154 37890 +15736
- Misses 24738 25590 +852 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zhuzhu81998
left a comment
There was a problem hiding this comment.
i dont think boa is doing anything about window option at all. and this pr does not change that.
From the Fetch spec, window is supposed to be null, and WPT has tests that expect a TypeError when it’s non-null. Node.js also throws here. |
#4338 didnt implement everything from fetch. |
yeah fair, i was just trying to stop silently accepting invalid values, but fine to wait till fetch is more complete |
|
Agreed with @zhuzhu81998, this should be part of a PR that properly implements |
This Pull Request fixes/closes #5209.
It changes the following:
windowfield toRequestInitand reject any non-null value with aTypeError.nullis still accepted.Testing:
cargo test -p boa_runtime request -- --nocaptureSpec reference: https://fetch.spec.whatwg.org/#dom-request