fix(fetch): reject Request body for GET and HEAD methods by HiteshShonak · Pull Request #5201 · boa-dev/boa

HiteshShonak · 2026-03-21T03:11:33Z

This Pull Request fixes/closes #5200.

It changes the following:

into_request_builder now checks if the method is GET or HEAD and throws a TypeError if a body is provided, matching the Fetch Standard.
Added regression tests for both GET and HEAD cases.

Testing:

cargo test -p boa_runtime request -- --nocapture

Spec reference: https://fetch.spec.whatwg.org/#dom-request

Copilot

Pull request overview

This PR aligns Boa’s Request constructor with the Fetch Standard by rejecting request bodies for GET and HEAD methods, addressing issue #5200.

Changes:

Add a GET/HEAD + body validation in RequestInit::into_request_builder, returning a TypeError when violated.
Add regression tests asserting new Request(..., { method: "GET"/"HEAD", body: "x" }) throws a TypeError.

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 GET/HEAD body rejection logic during request construction.
core/runtime/src/fetch/tests/request.rs	Adds regression tests for GET/HEAD requests with an explicit body.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

core/runtime/src/fetch/request.rs

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

github-actions · 2026-03-21T03:20:34Z

Test262 conformance changes

Test result	main count	PR count	difference
Total	53,125	53,125	0
Passed	51,049	51,049	0
Ignored	1,482	1,482	0
Failed	594	594	0
Panics	0	0	0
Conformance	96.09%	96.09%	0.00%

Tested main commit: 3cec5e6bc4454077cb8d65b537d368d633137e0e
Tested PR commit: bb1141a15b73df28e12595848fe6d826dd408c15
Compare commits: 3cec5e6...bb1141a

codecov · 2026-03-21T03:31:47Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.78%. Comparing base (6ddc2b4) to head (bb1141a).
⚠️ Report is 951 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #5201       +/-   ##
===========================================
+ Coverage   47.24%   59.78%   +12.54%     
===========================================
  Files         476      590      +114     
  Lines       46892    63701    +16809     
===========================================
+ Hits        22154    38086    +15932     
- Misses      24738    25615      +877

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jedel1043 · 2026-04-06T21:12:52Z

core/runtime/src/fetch/request.rs

+            is_get_or_head_method = matches!(parts.method, http::Method::GET | http::Method::HEAD);
+            has_inherited_body = parts.extensions.get::<HasBody>().is_some() || !body.is_empty();


Can you link the specific part of the spec that handles inherited bodies? I was looking through it and I didn't see it doing this. The only thing it checked was the parent request and the RequestInit object.

In general I would like to see this being commented with the specific spec lines it's implementing, since it makes maintainance much easier.

hey, added spec comments to both places. the HasBody extension is there because boa stores body as Vec, so an empty string body and no body both look the same. needed a way to track if body was explicitly set, which is what inputBody is non-null checks in the spec: https://fetch.spec.whatwg.org/#dom-request

That sounds kinda suboptimal. What about just changing body to an Option<Vec<u8>>? That avoids having to use an extension to something that should be "native" in a way.

switched to Option<Vec<u8>> and removed the HasBody extension. let me know if this looks good

…move HasBody extension

jedel1043 · 2026-04-10T15:38:34Z

core/runtime/src/fetch/request.rs

+        let mut is_get_or_head_method = true;
+        let mut inherited_is_get_or_head_method = true;
+        let mut inherited_body = None;
+        let mut request_body: Option<Vec<u8>> = None;


I'm slightly confused about using all these flags. As far as I understand you just need inherited_body, and the rest can be inlined into the specific part of the code using them, since they're mostly used only once.

request_body doesn't need to exist here, you can put it below by using a let-if block:

let request_body = if let Some(body) = &self.body { // ... } else { input_body };

simplified this and inlined the extra flags, and moved request_body into the let-if block as suggested

jedel1043 · 2026-04-10T15:45:47Z

core/runtime/src/fetch/request.rs

+        if is_get_or_head_method
+            && (self.body.is_some()
+                || inherited_body
+                    .as_ref()
+                    .is_some_and(|body| !body.is_empty() || !inherited_is_get_or_head_method))


Why inherited_body also checks that the body is empty? I thought this would throw even if the body is empty, since the spec is checking if the body is null, not if it's empty.
This change would also considerably simplify the code:

let is_get_or_head_method = method.eq_ignore_ascii_case("GET") || method.eq_ignore_ascii_case("HEAD"); if is_get_or_head_method && (self.body.is_some() || inherited_body.is_some() { // throw error }

removed the empty-body check here and switched to the simplified null/non-null check

fix(fetch): reject Request body for GET and HEAD methods

24f439e

HiteshShonak requested a review from a team as a code owner March 21, 2026 03:11

Copilot AI review requested due to automatic review settings March 21, 2026 03:11

github-actions bot added Waiting On Review Waiting on reviews from the maintainers C-Tests Issues and PRs related to the tests. C-Runtime Issues and PRs related to Boa's runtime features and removed Waiting On Review Waiting on reviews from the maintainers labels Mar 21, 2026

github-actions bot added this to the v1.0.0 milestone Mar 21, 2026

Copilot started reviewing on behalf of HiteshShonak March 21, 2026 03:12 View session

Copilot AI reviewed Mar 21, 2026

View reviewed changes

core/runtime/src/fetch/request.rs Outdated Show resolved Hide resolved

core/runtime/src/fetch/tests/request.rs Show resolved Hide resolved

fix(fetch): handle inherited empty body in GET/HEAD check

a31e50e

github-actions bot added the Waiting On Review Waiting on reviews from the maintainers label Mar 21, 2026

Merge branch 'main' into fix/request-get-head-body

0d1ce8b

jedel1043 requested changes Apr 6, 2026

View reviewed changes

HiteshShonak added 5 commits April 7, 2026 07:37

Merge branch 'main' into fix/request-get-head-body

9ca194b

refactor(fetch): add spec comments to GET/HEAD body validation

e979d67

refactor(fetch): use Option<Vec<u8>> for request body handling and re…

bb95051

…move HasBody extension

Merge branch 'main' into fix/request-get-head-body

0e85a33

Merge branch 'main' into fix/request-get-head-body

97a7c23

jedel1043 requested changes Apr 10, 2026

View reviewed changes

HiteshShonak added 2 commits April 10, 2026 22:26

refactor(fetch): simplify GET/HEAD body validation logic

6112763

Merge branch 'main' into fix/request-get-head-body

bb1141a

		is_get_or_head_method = matches!(parts.method, http::Method::GET \| http::Method::HEAD);
		has_inherited_body = parts.extensions.get::<HasBody>().is_some() \|\| !body.is_empty();

Uh oh!

Conversation

HiteshShonak commented Mar 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test262 conformance changes

Uh oh!

codecov bot commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Mar 21, 2026 •

edited

Loading

codecov bot commented Mar 21, 2026 •

edited

Loading