Conversation
Adds RedirectConfiguration.custom(_:), letting callers intercept every redirect-eligible response and decide whether/how to follow it instead of being limited to disallow/follow(max:allowCycles:). The candidate request handed to the handler has already gone through the same method/header rewrite rules `.follow` applies (POST->GET on 303, stripping Authorization/Cookie/Origin/Proxy-Authorization cross-origin), so callers only need to make further adjustments — e.g. stripping additional sensitive headers before a cross-host redirect is followed. Wired into the Swift Concurrency execute(_:deadline:logger:) family only; the delegate-based execute(request:delegate:...) API fails fast with .invalidRedirectConfiguration since it has no HTTPClientRequest to hand the handler. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tegy Replaces the bare-closure `.custom(_:)` from the previous commit with HTTPClientRedirectStrategy, a protocol callers can conform their own types to (not just closures), addressing the two gaps a maintainer flagged on the upstream issue thread: pluggable strategies as actual types, and access to more than a bare redirect count — the strategy now receives the full per-request history alongside the candidate request and response. - HTTPClientRedirectContext bundles redirectRequest/response/history/ redirectCount into one value instead of four positional parameters. - HTTPClientRedirectStrategy.redirectDecision(for:) can throw, so a strategy can fail the whole execute() call with a custom error instead of only following/refusing. - RedirectConfiguration.strategy(_:) is the primary entry point; .custom(_:) remains as a closure-based convenience over it via an internal ClosureRedirectStrategy adapter. - Mode.custom renamed to Mode.strategy to match. Still scoped to the Swift Concurrency execute(_:deadline:logger:) family only; the delegate-based API continues to fail fast with .invalidRedirectConfiguration. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous two CI runs on this branch failed before any job started
("reference to workflow should be either a valid branch, tag, or
commit") because beta's swift-ci.yaml referenced a since-deleted
request-dl/.github branch. That's now fixed on beta (f360eae); this
empty commit just re-triggers the pull_request check with no code
changes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Swift disallows @available on an enum case that carries an associated value, so `Mode.strategy(any HTTPClientRedirectStrategy)` failed to compile under the API digester's build (which surfaces this check that normal `swift build` doesn't enforce). Store the payload as `any Sendable` instead — the same type-erasure trick already used for `Configuration._tracer` — and downcast at the one call site that needs the concrete protocol. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e API `.strategy`/`.custom` redirect handlers were only wired up for the Concurrency `execute(_:deadline:logger:)` family; the delegate-based `execute(request:delegate:...)` API fast-failed with `.invalidRedirectConfiguration` instead. That's fine on its own, but a caller whose whole pipeline is built on the delegate-based API (streaming responses through a custom `HTTPClientResponseDelegate`, as request-dl-nio's `Internals.Client` does) had no way to use `.strategy` at all -- not even to make the same follow/rewrite/refuse decisions `.follow` already supports there. `RedirectState` becomes an enum (`.follow`/`.strategy`) instead of a struct that only ever modeled `.follow`, and `RedirectHandler.redirect` now builds a `HTTPClientRedirectContext` and calls the configured strategy for `.strategy` too, converting between the delegate API's legacy `HTTPClient.Request`/`Body` and the Concurrency API's `HTTPClientRequest`/`Body` (`RedirectStrategyLegacyBridge.swift`) -- reusing a strategy's untouched `redirectRequest.body` verbatim via a new internal `Body.Mode.legacyBody` case, and lossless synchronous draining for `.bytes`/`.byteBuffer` bodies a strategy replaces it with. `.doNotFollow` is not supported on this path: honoring it would mean resuming normal response delivery for a response the delegate-based state machine already committed to treating as a redirect candidate, which that state machine has no route back from (`.follow`'s own redirect-limit/cycle errors fail the whole task the same way, they never resume delivery either). It fails with `.invalidRedirectConfiguration` instead, same as before this change, with a doc comment pointing at the Concurrency API for callers that need it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AsyncHTTPClient doesn't call HTTPClient.Request/execute(request:delegate:...) "legacy" anywhere -- it's the older of the two execute APIs, but it isn't deprecated and isn't being phased out, so labeling it that way in the new bridge code (file name, the Body.Mode case, doc comments) was misleading. Renamed throughout to name it by what it actually is: the delegate-based API, as opposed to the Concurrency one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Previously, a .strategy redirect configuration deciding .doNotFollow on the delegate-based execute(request:delegate:...) API always failed the task instead of delivering the response that triggered the redirect -- the state machine (RequestBag+StateMachine.swift) decided whether a response was a redirect candidate, and either discarded its body after counting (<=3KB) or cancelled the connection before reading any of it (known >3KB), before the strategy's own decision was ever computed. RedirectHandler.earlyStrategyDecision(head:) now asks the strategy the moment the response head arrives, before any body byte is touched -- HTTPClientRedirectContext never carries a body, so nothing about it is needed at that point. `.doNotFollow` then simply proceeds like any ordinary, non-redirect-candidate response, regardless of size. A `.follow(_:)` decision is carried on a copy of the handler (`precomputed`) into the existing size-based buffer-or-cancel mechanism, which still reuses the HTTP/1.1 connection for small bodies exactly as before -- this only changes *when* the strategy is asked, not what happens once it decides to follow. The strategy call had to move outside receiveResponseHead's own mutating access to the state machine: it's arbitrary caller code that can synchronously reenter the task (e.g. call task.cancel()), which would otherwise overlap two accesses to the same storage and trap under Swift's exclusivity enforcement. RequestBag.receiveResponseHead0 now computes the decision first and hands the state machine only the answer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Adds a pluggable
HTTPClientRedirectStrategyredirect configuration (.strategy(_:)/.custom(_:)), which gets a chance to inspect every redirect-eligible response before it's followed -- adjust the outgoing request, refuse the redirect, or fail the whole request with a custom error. Complements the fixed-limit.follow(max:allowCycles:).Supported on both execute APIs:
execute(_:deadline:logger:)family (HTTPClientRequest/HTTPClientResponse) -- the original implementation.execute(request:delegate:...)family (HTTPClient.Request/HTTPClientResponseDelegate) -- added on top, since a caller whose whole pipeline is built on the delegate-based API (streaming responses through a customHTTPClientResponseDelegate) otherwise had no way to use.strategyat all, not even for the same follow/rewrite/refuse decisions.followalready supports there.RedirectHandler.redirectbuilds aHTTPClientRedirectContextand drives the configured strategy, converting between the two APIs' request/body types (RedirectStrategyDelegateBridge.swift) -- a strategy's untouchedredirectRequest.bodyis reused verbatim,.bytes/.byteBufferreplacements are drained losslessly and synchronously, and a genuinely new streaming body throws a clearHTTPClientError.redirectStrategyBodyNotSupportedsince nothing at redirect-decision time can drain one.Known limitation, documented in code:
.doNotFollowfails the task (.invalidRedirectConfiguration) on the delegate-based path specifically. Honoring it there would mean resuming normal response delivery for a response the delegate-based state machine already committed to treating as a redirect candidate, which that state machine has no route back from today (.follow's own redirect-limit/cycle errors fail the whole task the same way). Works fully on the Concurrency API.See discussion: swift-server#923
Test plan
swift build/swift build --build-testscleanswift-format lint --recursive --strictclean across the whole treeswift testsuite passing.doNotFollow), redirect-count/history tracking, cycle detection, and (delegate API only) that configuring.strategydoesn't reject requests that never redirect🤖 Generated with Claude Code