Repository navigation
HTTP: cancel unread lazy fetch bodies for HEAD responses - #44227
vladislav-miroshnikov wants to merge 1 commit into
Conversation
|
@Jarred-Sumner, could you check the pending-producer cleanup here? This keeps the consumer guard from #44014 and only adds cancellation for an unmaterialized FetchResponseBody. The adjacent response.text() HEAD regression passes with the new upstream-abort test. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: oven-sh/bun/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. WalkthroughWhen a HEAD response has a Locked body without a readable stream, ChangesHEAD response body cancellation
Suggested reviewers: Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The HEAD-response cleanup change and its regression coverage present no identified blocker to merging after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change addresses upstream requests left open by discarded HEAD response bodies. The reviewed path preserves active body readers and confines cancellation to the response’s own fetch operation. No security issue was identified in that path, though security coverage is not complete. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
What does this PR do?
Fixes #43970. An untouched fetch Response can still have a pending body producer when a HEAD response discards it, so there is no ReadableStream to cancel and the upstream request stays open. Take and cancel that pending FetchResponseBody producer while preserving the pending-consumer guard and the existing stream cancellation path.
This covers the untouched-fetch case explicitly left outside #43973; materialized stream behavior is unchanged.
How did you verify your code works?
The new regression reproduces the missing upstream abort on Bun 1.4.2 and passes on the patched main debug build. It sends three HEAD requests, waits for the upstream abort events and checks a later GET. The existing serve, reused-response and pending-promise-abort suites pass 368 tests with 1 skip, including the held response.text() HEAD case. Local HTTP tests run without the host proxy variables.