http: drop no-op Vec::clear and dead Option branches in client state

[robobun]

Drops Zig-port leftovers in src/http/. Each is either a no-op or a constant-folded branch; the diff is behavior-preserving by construction. Net -19 lines.

Changes

• src/http/HTTPThread.rs RequestBodyBuffer::to_array_list: Vec::with_capacity(n) returns len() == 0 by definition; the immediately following .clear() is a no-op. Collapse to Vec::with_capacity(self.allocated_slice().len()). (http: pool the HTTP/1.1 request build buffer instead of reallocating per request #32350 will supersede this by deleting the function entirely, but that PR has been open since June; this change is correct on its own and the conflict is trivial.)

• src/http/lib.rs HTTPClient::to_result: inside the else if let Some(metadata) = ... arm, certificate_info is the None initialised two lines above (the if let that would set it Some is the other arm of the same if/else), so certificate_info.is_some() is a constant false and has_more reduces to self.state.fail.is_none() && !self.state.is_done(). The surrounding let mut x = None; if let Some(i) = y.take() { x = Some(i) } else ... reduces to let x = y.take(); if x.is_none() { ... }.

• src/http/InternalState.rs reset: the explicit self.compressed_body = init_empty() / self.response_message_buffer = init_empty() / self.cloned_metadata = None / self.certificate_info = None assignments all immediately precede *self = InternalState{...}, whose drop glue drops every field of the old value (InternalState has no Drop impl; MutableString wraps a plain Vec<u8>, Option<HTTPResponseMetadata> and Option<CertificateInfo> drop via their fields' Drop glue). The removed code's own comments already acknowledged this. original_request_body.deinit() stays: HTTPRequestBody deliberately has no Drop (see HTTPRequestBody.rs:51-54), so its intrusive refcount release must be explicit.

Verification

cargo check -p bun_http and cargo clippy -p bun_http are clean. Targeted test runs with the debug build:

• test/js/web/fetch/fetch-redirect.test.ts (12/12, exercises InternalState::reset on every hop)
• test/js/web/fetch/fetch-gzip.test.ts (60/60, exercises decompressor drop via reset)
• test/js/web/fetch/fetch.tls.test.ts -t checkServerIdentity (10/10, exercises both certificate_info arms of to_result)

fetch.test.ts / fetch-leak.test.ts have the same set of pre-existing failures with and without this diff.

No new test is added: each change is a boolean/drop identity, so no input can distinguish before from after.

[robobun]

^{Updated 4:21 AM PT - Jul 20th, 2026}

❌ @robobun, your commit c79b535 has some failures in Build #76207 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 34808

That installs a local version of the PR into your bun-34808 executable, so you can run:

bun-34808 --bun

[robobun]

Dead-code cleanup found during a code audit of src/http/. Each change is a boolean or drop identity (proof in the PR body), so there is no input that can distinguish before from after and no fail-before test is possible. Same class as #34789 / #34792 / #34804 / #34805 / #34806.

Verified no regressions: cargo check/clippy clean, fetch-redirect 12/12, fetch-gzip 60/60, fetch.tls checkServerIdentity 10/10. Self-review raised zero surviving concerns; the one inline nit (two remaining sibling pre-assigns in reset()) is addressed in c79b535.

CI build 76207: the ten failing tests are all classified as known flakes by the CI tooling and none is in the HTTP client (install migration, workspace filter, cron --hot, process orphan reaping, Bun.serve websocket idle CPU, node:http server leak check, REPL EPIPE, es-module-lexer, child_process signal delivery, and one concurrent proxy-stress race). The three edited functions are fetch-client internals; none of these tests reaches them in a way a provable identity could alter.

Ready for a maintainer to merge on the strength of the analysis.

[coderabbitai]

Walkthrough

Changes

HTTP state and result handling

Layer / File(s)	Summary
Buffer and state cleanup src/http/HTTPThread.rs, src/http/InternalState.rs	Directly allocates the request body vector and simplifies internal-state reset cleanup using assignment drop behavior.
Certificate result construction src/http/lib.rs	Transfers certificate metadata and adds an early metadata result path with revised completion and streaming flags.

Possibly related PRs

• oven-sh/bun#34342: Related TLS certificate-info reporting and checkServerIdentity waiting behavior.
• oven-sh/bun#34425: Related HTTPClientResult completion and has_more semantics.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title accurately summarizes the cleanup of no-op Vec::clear calls and dead Option branches in src/http.
Description check	✅ Passed	The PR description covers what changed and how it was verified, matching the template's required sections.

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

LGTM — three provable identities (no-op Vec::clear, constant-folded certificate_info.is_some(), redundant field assigns before *self = ...). The inline nit about the two remaining MutableString::init_empty() lines in reset() is non-blocking.

Extended reasoning...

Overview

Three localized dead-code removals in src/http/:

• HTTPThread.rs RequestBodyBuffer::to_array_list: drop .clear() on a fresh Vec::with_capacity (len is already 0).
• lib.rs HTTPClient::to_result: collapse let mut x = None; if let Some(i) = y.take() { x = Some(i) } else if ... into let x = y.take(); if x.is_none() { if ... }, and constant-fold the always-false certificate_info.is_some() inside the metadata arm.
• InternalState.rs reset: drop two = None assignments that immediately precede *self = InternalState{...}, whose drop glue drops those fields anyway.

Net -15 lines, no logic added.

Security risks

None. No input handling, allocation sizing, or lifetime/ownership semantics change — each edit is a boolean/drop identity.

Level of scrutiny

The touched files are hot HTTP-client internals, so I traced each change carefully:

• to_result: enumerated all three cases (certificate_info Some/None × cloned_metadata Some/None); the .take() side effects and the fall-through result (lines 4665-4682) are identical before/after.
• reset: nothing between the removed lines and *self = ... reads cloned_metadata/certificate_info (only original_request_body.deinit() runs), and InternalState has no Drop impl, so field drop-glue timing is the only difference — unobservable.
• to_array_list: Vec::with_capacity guarantees len() == 0.

Other factors

The PR body verified against fetch-redirect (exercises reset per hop), fetch-gzip (decompressor drop via reset), and fetch.tls checkServerIdentity (both certificate_info arms). The one bug-hunter finding is a completeness nit (two sibling redundant assignments left in reset()) — correct observation but non-blocking; the PR is still net-positive and behavior-preserving as-is.

[claude]

LGTM — the follow-up commit addressed the earlier note about the two MutableString pre-assigns in InternalState::reset, so the function now matches its own comment.

What was reviewed:

• to_array_list: Vec::with_capacity(n) has len() == 0, so the removed .clear() is a no-op.
• to_result: traced both arms — in the old else if branch certificate_info was the untouched None, so .is_some() constant-folds to false; the fall-through result at lib.rs:4665-4682 still receives certificate_info correctly.
• InternalState::reset: confirmed no Drop impl on InternalState or HTTPRequestBody, so *self = ... drop glue covers every removed pre-assign and original_request_body.deinit() correctly stays explicit.

Extended reasoning...

Overview

Three dead-code / constant-fold cleanups in the HTTP client (src/http/): a no-op Vec::clear() after with_capacity in HTTPThread.rs, a constant-false certificate_info.is_some() inside an else arm plus the surrounding let mut x = None; if let Some(i) = y.take() { x = Some(i) } → let x = y.take() collapse in lib.rs::to_result, and redundant field pre-assignments before *self = InternalState { ... } in InternalState.rs::reset. Net -19 lines. Each is a boolean or drop-glue identity with a proof in the PR body.

Security risks

None. No input handling, allocation sizing, or trust boundary is touched — this is dead-store removal and control-flow simplification with identical effects on every path.

Level of scrutiny

The HTTP client is a hot, memory-sensitive path, but the changes here are mechanical identities that hold by language semantics rather than by runtime behavior. I verified each independently: Vec::with_capacity returns len 0 by definition; in the old to_result the else if let arm can only be reached with certificate_info still at its None initializer, so the removed || certificate_info.is_some() is a constant false; grepped for impl Drop for InternalState / HTTPRequestBody and found none, so *self = ... drop glue drops every old field and the removed pre-assigns are pure redundancy while original_request_body.deinit() correctly stays.

Other factors

I previously flagged that the two MutableString::init_empty() pre-assigns in reset were the same pattern and should go too; commit c79b535 removed them along with the stale // allocator param dropped comment, so that feedback is resolved. The author ran the redirect, gzip, and TLS checkServerIdentity suites (which exercise reset and both certificate_info arms) with no new failures. No test is added because no input can distinguish before from after — that reasoning holds for each change.