feat(fetch_winhttp): add a WinHTTP-backed fetch transport for Windows by sandersaares · Pull Request #687 · microsoft/oxidizer

Sander Saares (sandersaares) · 2026-08-21T13:11:07Z

[Copilot speaking]

Motivation

fetch had no transport that uses the operating system's own HTTP stack on Windows. Every request went through a bundled userspace client, which means the process carries its own TLS stack and certificate policy rather than deferring to the machine's configuration, and it cannot benefit from platform features such as system proxy discovery or the OS HTTP/3 implementation.

fetch_winhttp implements the fetch transport contract on top of WinHTTP, so a Windows application can use the platform stack while keeping the same runtime-neutral fetch API and the same telemetry, timeout, and body-streaming semantics as any other transport.

What this delivers

A new Windows-only crate providing a WinHTTP-backed fetch transport, reached through an extension trait:

let deps = WinHttpDeps::builder(clock, global_pool, sink)
    .tls(tls_config)
    .build();

let client = HttpClient::builder_winhttp(deps).build();

The transport is fully asynchronous and runtime-neutral. It never blocks a caller thread on network I/O; WinHTTP completions arrive on OS threads and wake the awaiting future directly.

Architecture

HttpClient  ──►  fetch custom transport factory
                          │
                          │  one instance per (core × pool slot)
                          ▼
                 WinHttpTransport ──► WinHttpSession (HINTERNET, async)
                          │                  │
                          │                  └─ status callback (registered once)
                          ▼                              │
                    per request:                         │ completions
              connect handle ─► request handle           │
                          │                              │
                          └──► RequestContext ◄──────────┘
                               (pinned, owns the single
                                outstanding operation)

Each materialized transport instance owns its own WinHTTP session with global connection pooling disabled, so independently built clients, cloned builders, and separate pool slots never share pooled connections. Cloning an HttpClient shares that client's transport resources, matching the generic contract.

Request contexts are rented from a per-instance pool, pinned, and handed to WinHTTP as a raw context pointer. Ownership transfers to WinHTTP once the context is installed and returns only on the final HANDLE_CLOSING callback, which is also what closes the connect handle and releases the session. Every request handle has at most one outstanding asynchronous operation, and completions may be inline and reentrant, so no Rust borrow survives across a submit call.

Behavior

Protocols. HTTP/1.1, HTTP/2, and HTTP/3 are all first-class, including HTTP/3 without silent TCP fallback when it is explicitly required. The negotiated protocol is reported on the response.
Streaming in both directions. Request bodies stream through WinHttpWriteData with no buffering added by the transport; known lengths above u32::MAX and unknown lengths are both supported on every protocol. Response bodies are read lazily into pooled buffers and are not fetched until the caller polls, preserving backpressure.
Trailers. Response trailers exposed by WinHTTP are preserved as body trailer frames. WinHTTP has no request-trailer API, so an outgoing trailer frame fails the request rather than being silently dropped.
Timeouts. The native connect, send, and receive timers are left unlimited so the canonical fetch timeout semantics govern the request. The response timeout covers connection establishment, upload, and response headers; the body idle timeout applies after headers. The connect timeout is enforced against the injected tick::Clock, so it is deterministic under test. Name resolution has no separate deadline; it is covered by the connect timeout along with the rest of connection establishment. The native WinHTTP resolve/connect/send/receive timers stay unlimited so the canonical fetch timeouts govern the request.
Failure handling. HttpClientBuilder::build() stays infallible. A session that cannot initialize yields a permanently failed transport that returns an initialization error without issuing any network I/O. 4xx and 5xx remain successful responses.
Telemetry. Request and error counts plus rich failure logs are emitted through observed, with cold-connect attribution kept to logs rather than metric dimensions.

Generic transport options that WinHTTP cannot represent — generic TLS configuration, finite connection limits, and bounded connection lifetimes — are ignored rather than approximated, and the full fidelity mapping is documented in the crate's design document.

Platform floor

Windows 11 build 22000 or Windows Server 2025. The transport depends on header query flags for byte-exact header bytes and for trailers that were introduced in that release, and it does not carry runtime capability probes or downlevel fallbacks. Windows Server 2022 is not supported.

Scope

The crate is empty on non-Windows targets and takes no dependency on the windows crate there. fetch's custom-transport architecture, CI topology, and stabilization decisions are unchanged; feedback gathered while implementing against fetch is recorded descriptively rather than acted on here.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Add mockable WinHTTP bindings, typed handle ownership, option conversions, protocol queries, and recoverability-aware error mapping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Initialize isolated WinHTTP sessions per core and pool slot, retain deterministic failed handlers, and emit bounded transport telemetry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Finish automatic chunked uploads with the required null zero-length WinHTTP write and add HTTP/1.1, HTTP/2, and HTTP/3 localhost integration coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Document the supported Windows baseline, automatic proxy and ignored-option behavior, HTTP/1.1 trailer limitation, and the implementation and test structure validated against real WinHTTP. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Keep exact native callback-address validation while allowing Miri's distinct function-pointer shims to exercise the full session failure lifecycle. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Make the WinHTTP bindings boundary explicitly unsafe, document operation-specific safety requirements, and add local proofs at call sites. Clarify binding and request-body type names and synchronize the implementation guide with the retained buffer model. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Reconcile every caller-supplied Content-Length against the true body length instead of only above the DWORD boundary, and reject a caller-supplied Transfer-Encoding rather than letting a second framing directive reach the wire. Reserve response read capacity only after WinHTTP reports availability, so a trickling peer cannot amplify a small response into many full pool blocks. Correct the callback ownership contract: sequential submission is guaranteed by construction and the atomic tag only publishes the payload and resolves the claim race, with correctness additionally resting on HANDLE_CLOSING never overlapping another callback. State the arm precondition in terms of the event reaching its terminal state, which is what events_once actually requires. Replace the notification mask change-detector with a test deriving its expectation from the operations and statuses the callback protocol consumes, backed by an exhaustive OperationKind::ALL. Cap retries in the standard-pipeline test so a failure cannot block forever on a frozen clock. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Split the two oversized modules so each file owns one concern, and move tests next to the code they exercise. `request.rs` now holds only the request driver and its failure attribution. Response header parsing moves to `response_headers.rs`, where it gains direct tests against the byte-level parser instead of being reachable only through a fully mocked request lifecycle. Context installation and per-operation handle ownership move to `operation.rs`. `options.rs` now holds only the public option types. Numeric, string and option-value conversions move to `convert.rs`, the synchronous WinHTTP query layer moves to `query.rs`, the SDK constant re-exports move to `bindings/mod.rs`, and the shared mock harness moves to `testing.rs`. `security_flags()` moves to `tls.rs`. Tests stranded in `options.rs` are relocated to the modules they cover, giving `context.rs` and `callback.rs` behavioural coverage they previously lacked. The four error constructors that bind an error label to its recovery information now live only in `error.rs`; the duplicated copies that had already begun to drift are removed. `RequestTranslationError`, `ResponseHeadersError`, `RequestBodyFramingError` and `RequestBodyError` become `ohno` clusters. The HTTP version rejection raised from a request message is now distinguishable from the one raised from the configured protocol set. Integration tests are organised by concern rather than by protocol version: `protocols.rs`, `tls.rs`, `transport_policy.rs` and `lifecycle.rs`. Response recording moves to `common/recording.rs` and frame collection is shared rather than duplicated per binary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Correct the module tree to name every module and describe the scope each one actually has, and add the integration-test layout so both the source and test structure are discoverable from the implementation guide. Fix two false claims about errors. The design guide promised that every error states the underlying Win32 code; three families carry none, and the promise is now scoped to failures that a WinHTTP call actually produced. The implementation guide counted two error families that do not wrap a native error; there are three, plus two outcomes that construct no transport error at all. Document two contractual behaviours that were unstated. The version on a request message does not select the wire version: HTTP/0.9 and HTTP/1.0 are rejected and every other value is ignored in favour of the configured version set and negotiation. The emitted event, counter and field names are a surface that dashboards bind to, so they are now enumerated in the design guide, with the mechanics of producing them left in the implementation guide. State in the crate documentation that a caller-supplied `Transfer-Encoding` is rejected, that a `Content-Length` header must state the exact body length, and that redirects, cookies and automatic authentication are disabled and cannot be re-enabled. Renumber the platform-support heading into the section it belongs to, rewrite the testing chapter in the indicative, and repair the dangling and ambiguous cross-references between the two guides. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

The transport previously ignored `fetch`'s `connection_idle_timeout` on the grounds that WinHTTP offered no control over pooled connection lifetime. It does: a session option bounds how long an idle connection stays eligible for reuse, backed by a single connection-manager field that governs HTTP/1.1, HTTP/2 and HTTP/3 alike. The option is absent from the public SDK, so `bindings` declares it directly - the only constant in the crate not re-exported from the generated bindings. Its documentation stands in for the reference page the option does not have, and records the contract read from the Windows source: session scope, a five-second minimum enforced by rejection rather than clamping, no upper bound, and a process-wide keep-alive pool that the option disables in agreement with the session's own explicit disabling. `Limited(d)` rounds up to whole milliseconds and rises to the minimum when shorter, so an aggressive idle policy cannot turn into an unbuildable transport. `Unlimited` takes the largest representable window, there being no reserved "never expire" encoding; the platform compares a 64-bit elapsed-time delta, so that value is an ordinary window rather than one that overflows. `fetch`'s default already matches the platform's own, making the default path a behavioral no-op, and a test pins that equality since the documented contract now rests on it. Also records two unexploited platform capabilities as future opportunities: per-connection identity through connection GUIDs, which would make `connection_lifetime`, cold-connect attribution and `ConnectionInfo` reachable, and the native performance knobs worth revisiting when profiling identifies a bottleneck. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Corrects two documentation claims that promised behaviour the transport does not deliver: the connection idle timeout description, and the Content-Length reconciliation contract, which cannot detect an untruthful header on an unknown-length body because the header itself supplies the declared length. Rewrites the safety contracts across the crate so every unsafe block discharges a stated precondition. The Bindings trait rule now forbids only an exclusive context borrow across a submission, matching what a reentrant callback taking its own shared borrow actually requires. READ_COMPLETE is carved out of the payload-readability requirement because its decoder compares addresses without reading through the pointer. Makes the test dispatch wrappers unsafe with a full contract and moves them into a single source in testing.rs, replacing three divergent copies. Fixes a mock that built a mutable slice over uninitialised buffer capacity. Removes proxy-resolution notification flags from the callback mask, which no binding method can ever produce, and renames the constant to reflect that it is narrower than the native ALL_COMPLETIONS. Replaces four tests that could not fail with tests that exercise the behaviour they name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

…viour An invalid_request no longer claims the server saw nothing. A body frame the transport cannot send is reached only once the body yields it, so a request trailer is rejected after the headers and every preceding data frame have gone out. Only rejections decided from request metadata precede transmission. The error-label table now describes the condition each label covers instead of naming a handful of native codes as though the set were complete, and states that the recognized code set is not contractual and that a label follows the reported code rather than the underlying cause. The recoverability rationale covers the same families, including responses that exceed a limit WinHTTP enforces. Crate documentation states that an unlimited connection idle timeout is approximated by the longest window the platform can express rather than an unbounded one. Cold-connect telemetry is documented as marking a connection attempt, which is what the transport attributes: a request that fails while still connecting carries the field, and that is the case the attribution exists to identify. The response reader no longer claims retained memory always tracks delivered payload. A zero availability figure carries no size information, so that read is speculative and reserves up to the preferred size however few bytes come back. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

The `observed` crate family now reports no severity for events that declare no log signal. `fetch.winhttp.request` is metric-only by design, so its expectations move to `ExpectedEvent::without_severity`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

State the Windows Server floor explicitly rather than leaving server operators to infer it from a client build number. The WinHTTP response-header query capabilities the transport depends on are documented as introduced in build 22000, so Windows Server 2025 is the earliest supported server release and Windows Server 2022 is excluded. Trim the crate-level docs to the behavior that changes how callers write code, and drop the option-fidelity detail that design.md already carries. Restate the connection lifetime and request framing contracts without recounting what implementing them would take. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Copilot

Pull request overview

Adds a Windows-only WinHTTP transport for fetch, including asynchronous request handling, streaming bodies, protocol negotiation, TLS controls, telemetry, and lifecycle management.

Changes:

Implements the WinHTTP FFI, request lifecycle, callbacks, framing, and response handling.
Adds comprehensive unit and localhost integration coverage for HTTP/1.1, HTTP/2, HTTP/3, TLS, and pooling.
Updates dependencies, documentation, and workspace configuration.

Reviewed changes

Copilot reviewed 39 out of 42 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`.spelling`	Adds transport terminology.
`Cargo.toml`	Adds WinHTTP test dependencies.
`Cargo.lock`	Resolves the expanded dependency graph.
`clippy.toml`	Recognizes WinHTTP terminology.
`crates/fetch_winhttp/Cargo.toml`	Configures Windows dependencies and tests.
`crates/fetch_winhttp/README.md`	Documents the implemented transport.
`crates/fetch_winhttp/src/lib.rs`	Exposes the Windows-only public API.
`crates/fetch_winhttp/src/builder.rs`	Integrates WinHTTP with `HttpClient`.
`crates/fetch_winhttp/src/transport.rs`	Implements the custom transport service.
`crates/fetch_winhttp/src/request.rs`	Drives request and response setup.
`crates/fetch_winhttp/src/session.rs`	Configures WinHTTP sessions.
`crates/fetch_winhttp/src/callback.rs`	Dispatches native asynchronous callbacks.
`crates/fetch_winhttp/src/context.rs`	Stores callback-visible request state.
`crates/fetch_winhttp/src/operation.rs`	Manages operation and context ownership.
`crates/fetch_winhttp/src/convert.rs`	Converts HTTP values to native forms.
`crates/fetch_winhttp/src/options.rs`	Defines transport-specific options.
`crates/fetch_winhttp/src/tls.rs`	Maps TLS relaxations to WinHTTP flags.
`crates/fetch_winhttp/src/query.rs`	Queries native response metadata.
`crates/fetch_winhttp/src/response_headers.rs`	Parses headers and trailers.
`crates/fetch_winhttp/src/error.rs`	Classifies native failures.
`crates/fetch_winhttp/src/error_labels.rs`	Defines stable error labels.
`crates/fetch_winhttp/src/handle.rs`	Adds RAII native handle wrappers.
`crates/fetch_winhttp/src/telemetry.rs`	Emits request and failure telemetry.
`crates/fetch_winhttp/src/testing.rs`	Provides callback lifecycle test utilities.
`crates/fetch_winhttp/src/body/mod.rs`	Wires body readers and writers.
`crates/fetch_winhttp/src/body/read.rs`	Implements lazy response streaming.
`crates/fetch_winhttp/src/body/write.rs`	Implements upload streaming and framing.
`crates/fetch_winhttp/src/bindings/mod.rs`	Centralizes WinHTTP bindings and constants.
`crates/fetch_winhttp/src/bindings/abstractions.rs`	Defines the native binding contract.
`crates/fetch_winhttp/src/bindings/facade.rs`	Dispatches real or mocked bindings.
`crates/fetch_winhttp/src/bindings/real.rs`	Wraps live WinHTTP calls.
`crates/fetch_winhttp/tests/common/mod.rs`	Shares integration-test helpers.
`crates/fetch_winhttp/tests/common/server.rs`	Implements HTTP/1.1 and HTTP/2 fixtures.
`crates/fetch_winhttp/tests/common/http3_server.rs`	Implements the HTTP/3 fixture.
`crates/fetch_winhttp/tests/common/recording.rs`	Defines request recording plans.
`crates/fetch_winhttp/tests/lifecycle.rs`	Tests pooling and cancellation.
`crates/fetch_winhttp/tests/protocols.rs`	Tests protocol negotiation and framing.
`crates/fetch_winhttp/tests/tls.rs`	Tests TLS validation controls.
`crates/fetch_winhttp/tests/transport_policy.rs`	Tests redirects, cookies, decoding, and framing.
`crates/fetch_winhttp/tests/non_windows.rs`	Keeps non-Windows test runs populated.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…ndows targets The crate-level cfg(windows) attribute stripped the crate and integration test documentation along with the items, so missing_docs fired on every non-Windows build. Declare the documentation before the attribute and redirect the links that target stripped items to the published docs. Also give the empty expected split an element type, which the pinned nightly toolchain cannot otherwise infer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

…wTotalLength maximum Also correct the transport policy suite's summary, which described content decoding as absent when gzip and deflate are decoded transparently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

The error-path benchmark aimed a request at a loopback port that had been bound and released, on the assumption that loopback answers such a connection with an immediate reset. It does not: WinHTTP retried the connect for slightly over two seconds per iteration, so the benchmark measured a retry delay rather than the transport, and Criterion needed 202 seconds to collect its samples. Add a ResetServer fixture that accepts the connection and discards it with a zero linger interval, which sends an RST. The transport now fails on a reset that is already in flight, which is the failure the benchmark was meant to cover, and the iteration cost drops from 2.03 s to 775 us. Also bring context.rs to the canonical format the pinned nightly produces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Copilot

Pull request overview

Copilot reviewed 62 out of 66 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/mutants.rs:71

Adding the facade and implementation to one mutation group does not make cargo-mutants run both packages' tests. Without explicit --test-package arguments, each fetch_winhttp_impl mutant is still scored only against that package's tests, so the facade integration tests cannot catch it and the reported score is inaccurate. Pass every group member as --test-package as well as --package.

Response body downloads scaled heap allocations with the payload: every frame allocated a boxed read future, and the read loop sized each read from WinHttpQueryDataAvailable, which fragmented a megabyte into hundreds of tiny frames. Reads now go through WinHttpReadDataEx. A response that declares how many bytes remain gets WINHTTP_READ_DATA_EX_FLAG_FILL_BUFFER and a read sized from that remainder, so the transfer arrives in few large frames. A response that declares nothing is read without the flag and takes whatever has arrived, preserving trickled streaming. The declaration only sizes reads: end-of-stream stays exclusively the zero-length READ_COMPLETE, and a declaration is withheld when a content or transfer encoding is present because WinHTTP decodes the body. The availability query is gone. The erased read future is now rented from a thread-local plurality::MultiPool, so a streaming download reuses one slot instead of allocating per frame. A one megabyte download drops from 237 allocations and 669 KB to 35 allocations and 11 KB, and from 3.66 ms to 1.53 ms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Copilot

Pull request overview

Copilot reviewed 62 out of 66 changed files in this pull request and generated 2 comments.

+pub(crate) fn connection_idle_timeout_millis(idle_timeout: &ConnectionIdleTimeout) -> u32 {
+    match idle_timeout {
+        ConnectionIdleTimeout::Unlimited => u32::MAX,
+        ConnectionIdleTimeout::Limited(duration) => dword_millis(*duration).max(CONNECTION_IDLE_TIMEOUT_MINIMUM_MS),


+// This policy applies to the merge-group run only. The Anvil-generated pull
+// request recipe (`justfiles/anvil/checks/mutants-diff.just`) has no hook for
+// expressing it and is deliberately left unmodified; AB#7802888 tracks adding
+// the capability at the Anvil level.


Evgenii (Vaiz) · 2026-08-28T05:54:52Z

+}
+
+#[cfg(test)]
+pub(crate) fn parse_header_buffer(buffer: &[u16], returned_bytes: u32) -> Result<String, ConversionError> {


🤖 Clawpilot here! Automated review comment from an AI agent -- not a human reviewer. Please verify before acting.

AI reviewer: Claude Opus 5

parse_header_buffer is #[cfg(test)] and has no production caller. Its only callers are four assertions in this file's own test module. It repeats the validation that query::query_header_units performs inline at query.rs:233-246: byte count to UTF-16 units, reject a returned length past the buffer with ReturnedLengthOutOfBoundsError, reject an interior zero with InteriorZeroCodeUnitError.

The tests that look like they cover this validation cover a copy of it. If query_header_units drifts -- bounds check inverted, interior-zero check moved after the decode, a different error type -- all four assertions still pass and the production path is unprotected for those cases. The two sides already express the bounds check differently (buffer.get(..units) here, compare-then-truncate there), which is the drift surface.

Lift the shared validation out of query_header_units into one function both call, then delete parse_header_buffer.

Evgenii (Vaiz) · 2026-08-28T05:56:52Z

-  post-send wait for the first response byte, excluding connect and send - so the transport
-  does not remap `ResponseTimeout` onto a native timer; it only sets the native
-  receive-response timer as a looser liveness backstop (implementation.md §10.4).
+  receiving the response headers. Expiration surfaces as `HttpError::timeout`.


🤖 Clawpilot here! Automated review comment from an AI agent -- not a human reviewer. Please verify before acting.

AI reviewer: Grok 4.5

This rewrite drops the sentence that said who enforces ResponseTimeout. The previous text read "fetch enforces this above the transport (the same way fetch_hyper relies on it)" and explained that WinHTTP has no native timer with matching semantics. What remains is the bullet lead-in "read per-request from the request extensions", under the heading "Which timeouts the transport honors".

The transport does not read it. ResponseTimeout appears nowhere in fetch_winhttp or fetch_winhttp_impl; the only consumer is fetch/src/client.rs:348. The identical parenthetical on the BodyTimeout bullet below is accurate, because RequestDriver::new does read that one. implementation.md §10.4 still says ResponseTimeout "needs no transport mapping", so the two documents now disagree and a reader will look for transport code that does not exist.

Restore the enforcement owner in this bullet: fetch enforces ResponseTimeout by wrapping the pipeline, while the transport owns only the connect deadline and BodyTimeout. The seatbelt bullet below already uses that shape.

Evgenii (Vaiz) · 2026-08-28T06:05:16Z

+
+        match content_length {
+            Some(length) => {
+                let total_length = match u32::try_from(length) {


🤖 Clawpilot here! Automated review comment from an AI agent -- not a human reviewer. Please verify before acting.

AI reviewer: Claude Opus 5

A known body length of 0 sets total_length = 0, which is exactly
WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH. Nothing else separates the two cases:
reconcile_content_length returns early when the caller sent no Content-Length, and
automatic_chunking is false on this arm. Only the > u32::MAX arm inserts a header.

This is reachable by default. fetch's empty body reports content_length() == Some(0)
(asserted in transport.rs), and RequestDriver::new passes that value straight in. An
empty POST, PUT or PATCH with no explicit header then reaches WinHttpSendRequest
with the ignore sentinel, so the request carries neither Content-Length nor
Transfer-Encoding. RFC 9110 §8.6 asks for Content-Length when the method anticipates a
body, and gateways commonly reject a POST framed that way.

This is the complement of the earlier sentinel discussion, not a repeat of it: that thread
asked about u32::MAX and the answer was that the sentinel is 0, so u32::MAX cannot
collide. Zero is the length that does.

Handle 0 explicitly: insert Content-Length: 0 when the length is 0 and the caller
supplied none, mirroring the > u32::MAX arm, or make the field Option<u32> so the two
meanings stop sharing an encoding. The test comment at line 479 -- "the unknown-length
sentinel is zero, so it cannot collide with one" -- holds for every length except 0;
please correct it and add a Some(0) case to
request_body_framing_maps_known_and_unknown_lengths.

The sentinel collision is confirmed in the source. What WinHTTP puts on the wire for
dwTotalLength == 0 was not executed -- please confirm that part.

Evgenii (Vaiz) · 2026-08-28T06:05:49Z

+  own length; where the length was adopted from the header it compares the value with
+  itself and normalizes. Values that survive are collapsed into one canonical decimal
+  header, so duplicates and non-canonical spellings such as `007` never reach the wire.
+- When the caller supplied no `Content-Length` and the length fits a `DWORD`, none is


🤖 Clawpilot here! Automated review comment from an AI agent -- not a human reviewer. Please verify before acting.

AI reviewer: Claude Opus 5

This does not hold at the boundary. WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH is 0, so a known
length of zero passes the ignore sentinel and WinHttpSendRequest emits no Content-Length at
all. This is the sentence a later maintainer reads to conclude the empty-body case is covered,
so it needs correcting whichever way the code goes (see the related comment on body/write.rs).

Suggested wording once the behaviour is settled: "...none is inserted for a positive length:
WinHttpSendRequest emits the header from dwTotalLength. A length of zero is the sentinel
value itself, so Content-Length: 0 is inserted instead."

Evgenii (Vaiz) · 2026-08-28T06:07:00Z

🤖 Clawpilot here! Automated review comment from an AI agent -- not a human reviewer. Please verify before acting.

AI reviewer: GPT-5.6 Sol

crates/fetch_winhttp/docs/design.md:29 says the crate is #[cfg(windows)] in its entirety.
This PR makes that false: crates/fetch_winhttp/src/linux.rs is gated on not(windows) and
compiles is_supported() plus a test as a coverage anchor, fetch_winhttp_impl has the same,
and tests/non_windows.rs exercises that build. The actual contract is an empty public surface
that still compiles off Windows.

The line is outside this PR's diff, so this is a top-level note rather than an inline comment.
Please state the implemented contract: the transport and public API are Windows-only, while the
crates compile with no transport API on other targets.

martin-kolinek · 2026-08-28T06:46:15Z

+        // operation owns, so it stays allocated and unaliased until that read
+        // completes, which is what this write and the completion below do.
+        unsafe {
+            lent.write(b'q');


🤖: This write has no exposed provenance to pick up, so it is UB by with_exposed_provenance_mut's own contract: "The provenance of the returned pointer is that of some pointer that was previously exposed by passing it to expose_provenance, or a ptr as usize cast. ... If there is no previously 'exposed' provenance that justifies the way the returned pointer will be used, the program has undefined behavior."

The address at line 991 comes from the mock at line 1160, read_record.lent_addresses.lock().unwrap().push(buffer.as_ptr().addr()), and the only other producer of that value is buffer.as_ptr().addr() at line 117. .addr() is the strict-provenance accessor and deliberately does not expose. Nothing else exposes this allocation either — operation.rs exposes the request context pointer, not the pooled block — so with_exposed_provenance_mut has nothing to pick up and <*mut u8>::write's "dst must be valid for writes" is unmet: the region is live, in bounds, aligned and unaliased, but the pointer carries no provenance for it. The SAFETY comment argues the other clause ("stays allocated and unaliased until that read completes"), which is true and necessary but does not imply the exposure clause.

This is reachable in CI, not just in theory: .github/workflows/nightly.yml runs cargo miri test --all-features --workspace --lib --tests with MIRIFLAGS: -Zmiri-strict-provenance, where an int-to-pointer cast is a hard error. This test carries no #[cfg_attr(miri, ignore)] and is not in .miri-tree-borrows-skip, so the nightly job breaks. The PR gate's Miri run uses permissive provenance, which accepts the cast heuristically — which is why "the body module is clean under Miri" holds here and will not hold there.

One-word fix at the recording site: record buffer.as_ptr().expose_provenance() instead of .addr() at line 1160, and have this SAFETY comment cite that exposure as what justifies the write. The recorded value is otherwise only compared (line 1043), so nothing else needs to change.

(Rest of read_once audited while here and all fine: remaining == Some(0) falls to the _ arm so no zero-length read is issued; capacity <= desired <= remaining means a fill-buffer read never waits on bytes the peer does not owe; remaining_capacity() == 0 forces the reserve so capacity != 0 on the no-fill path; saturating_sub plus EOF-only-from-a-zero-length-READ_COMPLETE handles both an over-running and an under-running declaration; and there is no panic path between lending the span and moving the BytesBuf into the operation buffer.)

martin-kolinek · 2026-08-28T06:46:22Z

+        // send. Without one, any wait would be open-ended, so the read takes
+        // whatever has already arrived.
+        let (desired, fill_buffer) = match self.remaining {
+            Some(remaining) if remaining != 0 => (usize::try_from(remaining).unwrap_or(usize::MAX).min(DESIRED_READ_SIZE), true),


🤖: BodyTimeout is an idle timeout — it promises to fire only when the body makes no progress — but a declared remainder now sets WINHTTP_READ_DATA_EX_FLAG_FILL_BUFFER with desired up to DESIRED_READ_SIZE (256 KiB, line 31), so the read does not complete until that whole region is filled (or EOF). request.rs:148-152 turns a caller's BodyTimeout into HttpBodyOptions::timeout, and the timeout layer only sees Pending between completions.

A peer that streams a Content-Length response continuously at less than 256 KiB per timeout interval therefore trips body_timeout even though the connection was never idle for a moment. That converts the documented idle policy into a minimum-throughput requirement, and it applies to ordinary Content-Length responses — exactly the shape this change optimizes for. A 1 MiB body over a 1 Mbit/s link with a 1 s BodyTimeout fails; before this push, the availability-sized reads completed on every delivery and the same transfer succeeded.

The commit message covers the allocation and latency win (237→35 allocations, 3.66→1.53 ms) but does not mention the interaction with the body timeout, so this looks unconsidered rather than accepted.

Either preserve observable progress when a body timeout is active (e.g. omit the fill flag, or bound desired when body_options carries a timeout), or enforce idleness at a layer that can observe partial fills. Worth a test that streams a declared-length body in sub-DESIRED_READ_SIZE increments with a BodyTimeout set and asserts it completes.

Evgenii (Vaiz)

yolo

martin-kolinek · 2026-08-28T09:35:56Z

🤖 Required: enable server-certificate revocation checking.

WinHTTP does not check certificate revocation by default. Microsoft explicitly calls out that revocation checking must be requested by the application: https://learn.microsoft.com/en-us/windows/win32/winhttp/winhttp-security-considerations

For every strict HTTPS request, please set WINHTTP_OPTION_ENABLE_FEATURE with WINHTTP_ENABLE_SSL_REVOCATION on the request handle before WinHttpSendRequest. The existing accept_invalid_certs flags do not enable this check, and the implementation currently only classifies revocation-related failures after WinHTTP reports them.

This should be a default transport security invariant, not an optional tuning knob. Please add binding/mock coverage proving the option is applied at the correct handle and lifecycle stage, and preserve the existing distinction between a revoked certificate and temporary revocation-server unavailability in error classification.

The transport reads a response body differently depending on whether the headers declare its length: a declared remainder sizes each read and lets it demand a full region, while an undeclared one makes every read take whatever has already arrived. The request side was covered in both shapes, but the response side was measured only with a declared length, so the second read path had no benchmark at all. Add `get_unknown_high`, serving the same payload as `get_known_high` through a chunked response so `hyper` withholds `Content-Length`. Rename the existing download scenarios to `get_known_low`/`get_known_high` to match the naming the upload scenarios already use. Both download scenarios now assert the response shape they are named for before measuring. Which shape the fixture serves is decided by `hyper` from the frames the plan scripts rather than stated by the plan, so without the check a change to either could quietly turn the undeclared scenario into a second measurement of the declared one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Copilot

Pull request overview

Copilot reviewed 62 out of 66 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

docs/packaging-guidelines.md:57

Cargo does not publish a dangling target entry here: since Cargo 1.80 it strips explicit targets whose sources are excluded from the normalized package manifest and warns during packaging. Describing the entry as present but inert is therefore inaccurate; document the stripping behavior (and warning) instead.
Cargo.toml:21
This changes the published contents of every crate inheriting workspace.package.include, not just the new WinHTTP crates: all existing examples and benchmarks will be stripped. That workspace-wide packaging migration is not disclosed in the PR description, which presents the change as adding a Windows transport. Please either scope the special allowlist to the new crates or explicitly document the repository-wide artifact change and its consumer impact.

# We ship only what a dependent has to build: the library sources, its
# compile-time inputs, and the crate metadata. Examples and benchmarks are
# development code that no dependent ever builds, so they stay out.

…io sends The download scenarios verify the response shape they are named for, but the upload scenarios asserted nothing, so nothing would catch `post_unknown_high` quietly becoming a second measurement of `post_known_high` if `fetch` changed how it derives a length from a body. Add the counterpart check. A repeating fixture retains no requests, because it would otherwise accumulate every body a high leg uploads, so the check sends one request to a throwaway fixture serving a scripted sequence, which does retain them, and inspects the framing the transport put on the wire. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Copilot

Pull request overview

Copilot reviewed 62 out of 66 changed files in this pull request and generated no new comments.

Sessions opened with `WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY` pay Windows proxy discovery, and that discovery turns out to dominate the transport's own cost: building a client spent most of its time there, and it remained a third of a minimal request thereafter. The target scenario is service-to-service traffic, which reaches its peers directly and gains nothing from a proxy. Open sessions with `WINHTTP_ACCESS_TYPE_NO_PROXY` instead, so requests always connect directly and Windows proxy configuration is never consulted. Callers who need a proxy are not served by this transport; supporting one would be a feature in its own right, with its own configuration surface. Measured on localhost fixtures, wall clock per iteration: client build 3.49 ms -> 419 us (-88%) first (cold) request 6.88 ms -> 1.96 ms (-72%) minimal GET 433 us -> 309 us (-29%) h1 plaintext 434 us -> 298 us (-31%) h1 over TLS 476 us -> 333 us (-31%) h2 over TLS 533 us -> 405 us (-22%) h3 over QUIC 693 us -> 555 us (-21%) Allocation counts are unchanged throughout, confirming the saving is operating system work rather than anything on the transport's own path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Copilot

Pull request overview

Copilot reviewed 62 out of 66 changed files in this pull request and generated 1 comment.

+//! - Requests always connect directly to the origin. Proxies are not supported,
+//!   Windows proxy configuration is not consulted, and there is no setting that
+//!   changes this.


Sander Saares (sandersaares) and others added 25 commits August 21, 2026 15:59

docs(fetch_winhttp): align implementation design

0c56a7f

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

feat(fetch_winhttp): add public API shell

7090167

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

feat(fetch_winhttp): add callback ownership core

17c614c

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

feat(fetch_winhttp): add headers-only request lifecycle

b78dea9

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

feat(fetch_winhttp): add streaming request uploads

0b46fbe

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

feat(fetch_winhttp): stream response bodies

706f7a1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

feat(fetch_winhttp): complete transport semantics

bb2ef4f

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

fix(fetch_winhttp): align builder and contract docs

d494f56

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

refactor(fetch_winhttp): harden internal contracts

9ec274f

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

refactor(fetch_winhttp): clarify callback ownership

2bcba84

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

chore(fetch_winhttp): resync lockfile after rebase onto main

56dae03

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dae6b35-0aaf-4113-a32f-db88840f7979

Sander Saares (sandersaares) requested a balanced review from Copilot August 21, 2026 13:16

Copilot started reviewing on behalf of Sander Saares (sandersaares) August 21, 2026 13:18 View session

Copilot AI reviewed Aug 21, 2026

View reviewed changes

Comment thread crates/fetch_winhttp_impl/src/body/write.rs

Comment thread crates/fetch_winhttp_impl/src/request.rs

Comment thread crates/fetch_winhttp/tests/transport_policy.rs Outdated

Comment thread crates/fetch_winhttp_impl/src/builder.rs

Sander Saares (sandersaares) and others added 2 commits August 21, 2026 16:47

Copilot AI review requested due to automatic review settings August 27, 2026 14:33

Copilot started reviewing on behalf of Sander Saares (sandersaares) August 27, 2026 14:33 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

martin-kolinek approved these changes Aug 27, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 28, 2026 05:32

Copilot started reviewing on behalf of Sander Saares (sandersaares) August 28, 2026 05:33 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Evgenii (Vaiz) reviewed Aug 28, 2026

View reviewed changes

martin-kolinek reviewed Aug 28, 2026

View reviewed changes

Evgenii (Vaiz) approved these changes Aug 28, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 28, 2026 10:20

Copilot started reviewing on behalf of Sander Saares (sandersaares) August 28, 2026 10:20 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 28, 2026 10:33

Copilot started reviewing on behalf of Sander Saares (sandersaares) August 28, 2026 10:33 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 28, 2026 15:36

Copilot started reviewing on behalf of Sander Saares (sandersaares) August 28, 2026 15:37 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Comment thread crates/fetch_winhttp/src/lib.rs

Comment on lines +28 to +30

//! - Requests always connect directly to the origin. Proxies are not supported,

//! Windows proxy configuration is not consulted, and there is no setting that

//! changes this.

Uh oh!

Conversation

Sander Saares (sandersaares) commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

What this delivers

Architecture

Behavior

Platform floor

Scope

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!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Evgenii (Vaiz) Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Evgenii (Vaiz) Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Evgenii (Vaiz) Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Evgenii (Vaiz) Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Evgenii (Vaiz) commented Aug 28, 2026

Uh oh!

martin-kolinek Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

martin-kolinek Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Evgenii (Vaiz) left a comment

Choose a reason for hiding this comment

Uh oh!

martin-kolinek commented Aug 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Sander Saares (sandersaares) commented Aug 21, 2026 •

edited

Loading