Release/0.5.7#16
Merged
Merged
Conversation
…TTP edge cases
Turn on four Clippy lints crate-wide:
cast_possible_truncation, cast_possible_wrap,
cast_sign_loss, arithmetic_side_effects
This covers integer arithmetic soundness issues.
Rewrites some logic to avoid possible issues and marks others with
safety guarantees.
Enabling clippy's lints surfaced several real bugs across the WinHTTP path as well:
* URL parsing no longer truncates long components. winhttp_crack_url used
fixed 2 KiB host / 8 KiB path / 8 KiB query buffers and silently dropped
anything longer. It now reads the in-place pointers WinHTTP writes back
into the input UTF-16, so components are bounded only by the URL length.
* Response body collection returns Error::body("response body too large")
if the accumulated length would overflow usize, instead of wrapping.
Primarily matters on 32-bit targets where usize is u32.
* Retry budget can no longer spin millions of iterations after a long
suspend (sleep, debugger). The slot-advance loop is now capped
at slots-count iterations.
* WinHTTP callback bookkeeping no longer has a path where a panic during
the HANDLE_CLOSING handler could be swallowed by catch_unwind and leave
WinHttpRequestHandle::drop hanging in wait_closed_and_idle. The retained
context is released before signalling, and the active-callback counter
uses checked_add / checked_sub.
* MultiByteToWideChar wrapper clamps `written` against capacity before
set_len, closing a theoretical UB window if Win32 ever overreported.
* WinHTTP header / read / write FFI surface oversize inputs as
Error::request / Error::body instead of silently truncating via `as u32`
(header lines > u32::MAX chars, write buffers > 4 GiB, etc.).
There was a problem hiding this comment.
Pull request overview
This PR prepares the 0.5.7 release by tightening security behavior around redirects and secret redaction, improving WinHTTP URL parsing correctness, and hardening multiple code paths for 32-bit/overflow soundness and reliability.
Changes:
- Add type-level secret redaction for
Debugoutput (URL/proxy passwords, cascading intoError). - Harden redirect handling (strip sensitive headers on cross-origin redirects; surface https→http downgrade as a redirect-policy error).
- Improve 32-bit/overflow robustness (checked length arithmetic, safer casts, retry budget loop cap, WinHTTP CrackUrl buffer preservation) and update CI/test reliability settings.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/url_parse.rs | Updates RFC-clean URL test commentary to reflect dot-segment/%2e behavior expectations. |
| tests/real_world.rs | Increases test timeouts and adds redirect-policy parity tests for https→http downgrades. |
| tests/integration.rs | Adjusts streaming upload test to avoid truncating casts. |
| src/winhttp.rs | Adds cross-origin redirect header stripping, callback abort plumbing, overflow guards, and write/read size handling changes. |
| src/util.rs | Removes an attribute previously suppressing dead-code warnings around wide-string conversion. |
| src/url.rs | Redacts URL passwords in Debug, adjusts percent-decoding, and hardens dot-segment removal to treat %2e/%2E as dot tokens. |
| src/timer.rs | Simplifies timer arming by passing Duration to the threadpool timer wrapper. |
| src/threadpool.rs | Changes timer API to accept Duration and centralizes 100ns tick conversion with saturation. |
| src/retry.rs | Caps slot-advance work, improves 32-bit conversions, and uses saturating arithmetic for writer accounting. |
| src/response.rs | Prevents usize overflow while collecting response bodies (returns Error::body). |
| src/request.rs | Uses explicit secret exposure when building Basic auth from URL userinfo. |
| src/redact.rs | Introduces Redacted<T> newtype to redact secrets from Debug output. |
| src/proxy.rs | Redacts proxy Basic-auth passwords in Debug by storing them as Redacted<String>. |
| src/lib.rs | Wires in the new redact module for the native WinHTTP backend. |
| src/error.rs | Adds Error::redirect, improves Win32 error conversion, and adds tests for redaction behavior in Debug. |
| src/encoding.rs | Minor comment/implementation adjustments in decoder helpers. |
| src/client.rs | Translates silently-surfaced https→http blocked redirects into redirect-policy errors; tracks whether redirects are followed. |
| src/abi/winhttp.rs | Adds header removal helper, improves size handling at FFI boundaries, and preserves full URL part lengths in WinHttpCrackUrl. |
| src/abi/mod.rs | Adds a dword_size_of<T>() helper for Win32 DWORD size casts. |
| src/abi/encoding.rs | Adds empty-input fast-path and tighter bounds checks around Win32/ICU transcoding. |
| examples/whirl.rs | Uses saturating arithmetic for download progress counters. |
| examples/hn_live.rs | Uses saturating arithmetic and elapsed-time loop control for robustness. |
| examples/github_api.rs | Uses saturating arithmetic for display numbering. |
| CHANGELOG.md | Adds 0.5.7 release notes. |
| Cargo.toml | Bumps version to 0.5.7 and enables additional clippy overflow/cast lints. |
| Cargo.lock | Updates lockfile for the 0.5.7 release and dependency bumps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Wrap Url::password and Proxy basic-auth password in a new
Redacted<T> type so `{:?}` (and transitively Error::Debug, panic
messages, dbg!, and tracing ?-formatters) no longer leak
credentials. Display was already userinfo-free.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Cross-origin redirects no longer forward Authorization, Cookie, Cookie2, Proxy-Authorization, Proxy-Authenticate, or WWW-Authenticate to the new origin. Previously WinHTTP forwarded these unchanged (documented behavior per Security Considerations item 16), so a victim.example -> attacker.example 302 would deliver caller credentials to the attacker host. A redirect from https:// to http:// now returns an Err where is_redirect() is true, instead of silently surfacing the 3xx response. Most callers check is_success() or unwrap_or on .text() and would not have noticed the downgrade; the new shape matches reqwest.
RFC 3986 §6.2.2.2 normalizes percent-encoded unreserved chars to their literal form before §5.2.4 runs. join() previously matched only literal "." / "..", so "%2e%2e/secret" against a trusted base traversed out on the wire. Classify segments via strip_dot_token() so any mix of ".", "%2e", "%2E" collapses; "%2f" stays encoded (segment byte, not separator). Trailing slash now keyed on "last segment was a dot" so /foo/%2e behaves like /foo/.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixed
Debugoutput forUrlandProxy(andError, transitively) now redacts the URL password and proxy Basic-auth password.Displayalready omitted them where provided.winhttp_crack_urlpreviously used fixed-size buffers for URL parts - these are now preserved in full.usize.HANDLE_CLOSINGcallback bookkeeping could be swallowed and leaveWinHttpRequestHandle::drophanging inwait_closed_and_idle.Authorization,Cookie,Cookie2,Proxy-Authorization,Proxy-Authenticate, orWWW-Authenticateheaders to the new origin. WinHTTP forwards these unchanged by default and documents that stripping them is the application's responsibility (Security Considerations item 16).https://->http://redirect now returnsError::is_redirect() == trueinstead of silently surfacing the 3xx response. The Err shape matches what reqwest produces underhttps_only(true), but wrest applies it unconditionally: WinHTTP defaults to blocking the downgrade and wrest preserves that default, so the new shape is always what callers see (reqwest's default follows https->http silently).Url::join()now treats%2e/%2Eas.when collapsing dot-segments, so an attacker-controlled relative reference like%2e%2e/secretcan no longer escape a trusted base.Changed
httpbin.orgfor a locally-hosted version for reliability (doesn't affect local testing)cast_possible_truncation,cast_possible_wrap,cast_sign_loss, andarithmetic_side_effectslints (warn) to guard against integer-overflow and 32-bit soundness regressions.