From 373930f0f790dcd578a0070446e61e01d9a0c373 Mon Sep 17 00:00:00 2001 From: brennobemoura <37243584+brennobemoura@users.noreply.github.com> Date: Thu, 3 Sep 2026 15:42:22 -0300 Subject: [PATCH 1/4] Add custom redirect handler support to RedirectConfiguration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../AsyncAwait/HTTPClient+execute.swift | 112 +++++++++++++----- Sources/AsyncHTTPClient/HTTPClient.swift | 67 ++++++++++- Sources/AsyncHTTPClient/RedirectState.swift | 35 ++++++ .../AsyncAwaitEndToEndTests.swift | 99 ++++++++++++++++ .../HTTPClientTests.swift | 18 +++ .../SwiftConfigurationTests.swift | 4 +- 6 files changed, 304 insertions(+), 31 deletions(-) diff --git a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift index a1047fa85..e2a162342 100644 --- a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift +++ b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift @@ -46,7 +46,7 @@ extension HTTPClient { request, deadline: deadline, logger: logger ?? Self.loggingDisabled, - redirectState: RedirectState(self.configuration.redirectConfiguration.mode, initialURL: request.url) + redirectMode: self.configuration.redirectConfiguration.mode ) } } @@ -88,10 +88,11 @@ extension HTTPClient { _ request: HTTPClientRequest, deadline: NIODeadline, logger: Logger, - redirectState: RedirectState? + redirectMode: HTTPClient.Configuration.RedirectConfiguration.Mode ) async throws -> HTTPClientResponse { var currentRequest = request - var currentRedirectState = redirectState + var currentRedirectState = RedirectState(redirectMode, initialURL: request.url) + var customRedirectCount = 0 var history: [HTTPClientRequestResponse] = [] // this loop is there to follow potential redirects @@ -122,39 +123,94 @@ extension HTTPClient { return response }() - guard var redirectState = currentRedirectState else { - // a `nil` redirectState means we should not follow redirects + switch redirectMode { + case .disallow: return response - } - guard - let redirectURL = response.headers.extractRedirectTarget( + case .follow: + guard var redirectState = currentRedirectState else { + // a `nil` redirectState means we should not follow redirects + return response + } + + guard + let redirectURL = response.headers.extractRedirectTarget( + status: response.status, + originalURL: preparedRequest.url, + originalScheme: preparedRequest.poolKey.scheme + ) + else { + // response does not want a redirect + return response + } + + // validate that we do not exceed any limits or are running circles + try redirectState.redirect(to: redirectURL.absoluteString) + currentRedirectState = redirectState + + let newRequest = currentRequest.followingRedirect( + from: preparedRequest.url, + to: redirectURL, status: response.status, - originalURL: preparedRequest.url, - originalScheme: preparedRequest.poolKey.scheme + config: redirectState.config ) - else { - // response does not want a redirect - return response - } - // validate that we do not exceed any limits or are running circles - try redirectState.redirect(to: redirectURL.absoluteString) - currentRedirectState = redirectState + guard newRequest.body.canBeConsumedMultipleTimes else { + // we already send the request body and it cannot be send again + return response + } - let newRequest = currentRequest.followingRedirect( - from: preparedRequest.url, - to: redirectURL, - status: response.status, - config: redirectState.config - ) + currentRequest = newRequest - guard newRequest.body.canBeConsumedMultipleTimes else { - // we already send the request body and it cannot be send again - return response - } + case .custom(let handler): + guard + let redirectURL = response.headers.extractRedirectTarget( + status: response.status, + originalURL: preparedRequest.url, + originalScheme: preparedRequest.poolKey.scheme + ) + else { + // response does not want a redirect + return response + } - currentRequest = newRequest + // Pre-build the request the same way `.follow` would, applying the standard + // method/header rewrite rules, so the handler only needs to make further + // adjustments rather than reimplement those rules itself. `max`/`allowCycles` + // are irrelevant here: only the `retainHTTPMethodAndBodyOn30{1,2}` flags feed + // into this transformation, and there's no built-in limit in `.custom` mode. + let candidateRequest = currentRequest.followingRedirect( + from: preparedRequest.url, + to: redirectURL, + status: response.status, + config: .init( + max: 0, + allowCycles: true, + retainHTTPMethodAndBodyOn301: false, + retainHTTPMethodAndBodyOn302: false + ) + ) + + let responseHead = HTTPResponseHead( + version: response.version, + status: response.status, + headers: response.headers + ) + + switch handler(candidateRequest, responseHead, customRedirectCount) { + case .doNotFollow: + return response + + case .follow(let newRequest): + guard newRequest.body.canBeConsumedMultipleTimes else { + // we already send the request body and it cannot be send again + return response + } + + customRedirectCount += 1 + currentRequest = newRequest + } + } } } diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index cc8792497..4aeff4d49 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -759,6 +759,23 @@ public final class HTTPClient: Sendable { ] ) + if #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *), + case .custom = self.configuration.redirectConfiguration.mode + { + // `.custom` redirect handlers operate on `HTTPClientRequest`/`HTTPClientResponse` and are + // only wired up for the Swift Concurrency `execute(_:deadline:logger:)` family of APIs. + logger.debug( + "`.custom` redirect configuration is not supported by the delegate-based execute API, failing request" + ) + return Task.failedTask( + eventLoop: taskEL, + error: HTTPClientError.invalidRedirectConfiguration, + logger: logger, + tracing: tracing, + makeOrGetFileIOThreadPool: self.makeOrGetFileIOThreadPool + ) + } + let failedTask: Task? = self.state.withLockedValue { state -> (Task?) in switch state { case .upAndRunning: @@ -1312,11 +1329,14 @@ extension HTTPClient.Configuration { /// Specifies redirect processing settings. public struct RedirectConfiguration: Sendable { - enum Mode: Hashable { + enum Mode { /// Redirects are not followed. case disallow /// Redirects are followed with a specified limit. case follow(FollowConfiguration) + /// Redirects are handed to a caller-supplied handler. + @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) + case custom(CustomRedirectHandler) } /// Configuration for following redirects. @@ -1353,6 +1373,38 @@ extension HTTPClient.Configuration { } } + /// The result of a ``CustomRedirectHandler`` deciding whether — and how — to follow a redirect. + @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) + public enum RedirectDecision: Sendable { + /// Follow the redirect using the given request. + case follow(HTTPClientRequest) + /// Do not follow the redirect; the response that triggered it is returned as-is. + case doNotFollow + } + + /// A handler invoked whenever a response indicates a redirect (a 3xx status with a `Location` + /// header), giving the caller the chance to inspect, modify, or refuse the redirect before it is + /// sent. + /// + /// `redirectRequest` has already gone through the same method/header rewrite rules `.follow` + /// would apply (converting `POST` to `GET` on a 303, stripping `Authorization`/`Cookie`/`Origin`/ + /// `Proxy-Authorization` on cross-origin redirects) — the handler only needs to make further + /// adjustments, not reimplement those rules from scratch. + /// + /// - Parameters: + /// - redirectRequest: The request that would be sent to follow the redirect. + /// - response: The head of the response that triggered the redirect. + /// - redirectCount: How many redirects have already been followed for this logical request. + /// There is no built-in limit for `.custom` mode — the handler is responsible for enforcing + /// its own policy (e.g. refusing past a maximum count) to avoid infinite redirect loops. + /// - Returns: Whether — and with what request — to follow the redirect. + @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) + public typealias CustomRedirectHandler = @Sendable ( + _ redirectRequest: HTTPClientRequest, + _ response: HTTPResponseHead, + _ redirectCount: Int + ) -> RedirectDecision + var mode: Mode init() { @@ -1397,6 +1449,19 @@ extension HTTPClient.Configuration { public static func follow(configuration: FollowConfiguration) -> RedirectConfiguration { .init(configuration: .follow(configuration)) } + + /// Redirects are handed to a caller-supplied handler, which decides whether and how to follow + /// each one. See ``CustomRedirectHandler``. + /// + /// - warning: There is no built-in redirect-count or cycle limit for this mode — use the + /// `redirectCount` passed to the handler to enforce your own policy. + /// - note: Only supported by the Swift Concurrency `execute(_:deadline:logger:)` family of APIs. + /// Using `.custom` with the delegate-based `execute(request:delegate:...)` API fails with + /// ``HTTPClientError/invalidRedirectConfiguration``. + @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) + public static func custom(_ handler: @escaping CustomRedirectHandler) -> RedirectConfiguration { + .init(configuration: .custom(handler)) + } } /// Connection pool configuration. diff --git a/Sources/AsyncHTTPClient/RedirectState.swift b/Sources/AsyncHTTPClient/RedirectState.swift index 9665c03d4..bfd4d89b1 100644 --- a/Sources/AsyncHTTPClient/RedirectState.swift +++ b/Sources/AsyncHTTPClient/RedirectState.swift @@ -22,6 +22,37 @@ import struct Foundation.URL typealias RedirectMode = HTTPClient.Configuration.RedirectConfiguration.Mode +// `Mode` can't derive `Equatable`/`Hashable` because `.custom` carries a closure. `.custom` values have +// no meaningful notion of equality, so — like `NaN` — a `.custom` value is never equal to any other +// value, including another `.custom`; this is consistent (if vacuously so) with the `Hashable` +// requirement that equal values hash equally. +extension HTTPClient.Configuration.RedirectConfiguration.Mode: Equatable { + static func == (lhs: Self, rhs: Self) -> Bool { + switch (lhs, rhs) { + case (.disallow, .disallow): + return true + case (.follow(let lhsConfig), .follow(let rhsConfig)): + return lhsConfig == rhsConfig + default: + return false + } + } +} + +extension HTTPClient.Configuration.RedirectConfiguration.Mode: Hashable { + func hash(into hasher: inout Hasher) { + switch self { + case .disallow: + hasher.combine(0) + case .follow(let config): + hasher.combine(1) + hasher.combine(config) + case .custom: + hasher.combine(2) + } + } +} + struct RedirectState { var config: HTTPClient.Configuration.RedirectConfiguration.FollowConfiguration @@ -42,6 +73,10 @@ extension RedirectState { return nil case .follow(let config): self.init(config: config, visited: [initialURL]) + case .custom: + // `.custom` redirects are handled entirely by the caller-supplied handler; there is no + // count/cycle state for `RedirectState` to track. + return nil } } } diff --git a/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift b/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift index 78dec4296..b2c28b796 100644 --- a/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift +++ b/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// import Logging +import NIOConcurrencyHelpers import NIOCore import NIOFoundationCompat import NIOHTTP1 @@ -768,6 +769,104 @@ final class AsyncAwaitEndToEndTests: XCTestCase { } } + // MARK: - Custom redirect handler + + func testCustomRedirectHandlerCanRewriteRedirectRequest() { + XCTAsyncTest { + let bin = HTTPBin(.http1_1(compress: false)) + defer { XCTAssertNoThrow(try bin.shutdown()) } + + let port = bin.port + var config = HTTPClient.Configuration() + config.redirectConfiguration = .custom { redirectRequest, response, redirectCount in + XCTAssertEqual(response.status, .found) + XCTAssertEqual(redirectCount, 0) + // The candidate request has already gone through the standard rewrite rules: it + // should already point at the `Location` from the /redirect/302 response. + XCTAssertEqual(redirectRequest.url, "http://localhost:\(port)/ok") + + var rewritten = redirectRequest + rewritten.url = "http://localhost:\(port)/echo-uri" + return .follow(rewritten) + } + + let client = HTTPClient(eventLoopGroupProvider: .singleton, configuration: config) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + let request = HTTPClientRequest(url: "http://localhost:\(port)/redirect/302") + guard + let response = await XCTAssertNoThrowWithResult( + try await client.execute(request, deadline: .now() + .seconds(10)) + ) + else { return } + + XCTAssertEqual(response.status, .ok) + XCTAssertEqual(response.headers.first(name: "X-Calling-URI"), "/echo-uri") + XCTAssertEqual( + response.history.map(\.request.url), + ["http://localhost:\(port)/redirect/302", "http://localhost:\(port)/echo-uri"] + ) + } + } + + func testCustomRedirectHandlerCanRefuseRedirect() { + XCTAsyncTest { + let bin = HTTPBin(.http1_1(compress: false)) + defer { XCTAssertNoThrow(try bin.shutdown()) } + + var config = HTTPClient.Configuration() + config.redirectConfiguration = .custom { _, _, _ in .doNotFollow } + + let client = HTTPClient(eventLoopGroupProvider: .singleton, configuration: config) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + let request = HTTPClientRequest(url: "http://localhost:\(bin.port)/redirect/302") + guard + let response = await XCTAssertNoThrowWithResult( + try await client.execute(request, deadline: .now() + .seconds(10)) + ) + else { return } + + // The handler refused the redirect, so the 302 itself is the final response. + XCTAssertEqual(response.status, .found) + XCTAssertEqual(response.history.count, 1) + } + } + + func testCustomRedirectHandlerReceivesIncreasingRedirectCountAndCanBoundLoops() { + XCTAsyncTest { + let bin = HTTPBin(.http1_1(compress: false)) + defer { XCTAssertNoThrow(try bin.shutdown()) } + + let observedCounts = NIOLockedValueBox<[Int]>([]) + var config = HTTPClient.Configuration() + config.redirectConfiguration = .custom { redirectRequest, _, redirectCount in + observedCounts.withLockedValue { $0.append(redirectCount) } + guard redirectCount < 3 else { + return .doNotFollow + } + return .follow(redirectRequest) + } + + let client = HTTPClient(eventLoopGroupProvider: .singleton, configuration: config) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + // /redirect/infinite1 <-> /redirect/infinite2 bounce forever. `.custom` mode has no + // built-in redirect limit (unlike `.follow`), so this exercises the handler enforcing + // its own bound using the `redirectCount` it's handed. + let request = HTTPClientRequest(url: "http://localhost:\(bin.port)/redirect/infinite1") + guard + let response = await XCTAssertNoThrowWithResult( + try await client.execute(request, deadline: .now() + .seconds(10)) + ) + else { return } + + XCTAssertEqual(response.status, .found) + XCTAssertEqual(observedCounts.withLockedValue { $0 }, [0, 1, 2, 3]) + XCTAssertEqual(response.history.count, 4) + } + } + func testShutdown() { XCTAsyncTest { let client = makeDefaultHTTPClient() diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 75371d437..74890932a 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -448,6 +448,24 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { XCTAssertEqual("1234", data.data) } + func testCustomRedirectConfigurationFailsWithDelegateBasedExecute() throws { + // `.custom` redirect handlers operate on `HTTPClientRequest`/`HTTPClientResponse` and are only + // wired up for the Swift Concurrency `execute(_:deadline:logger:)` family of APIs; the + // delegate-based `execute(request:delegate:...)` API (exercised here via `.get`) should fail + // fast rather than silently ignore the configured handler. + let localClient = HTTPClient( + eventLoopGroupProvider: .shared(self.clientGroup), + configuration: HTTPClient.Configuration( + redirectConfiguration: .custom { _, _, _ in .doNotFollow } + ) + ) + defer { XCTAssertNoThrow(try localClient.syncShutdown()) } + + XCTAssertThrowsError(try localClient.get(url: self.defaultHTTPBinURLPrefix + "ok").wait()) { + XCTAssertEqual($0 as? HTTPClientError, .invalidRedirectConfiguration) + } + } + func testHttpRedirect() throws { let httpsBin = HTTPBin(.http1_1(ssl: true)) let localClient = HTTPClient( diff --git a/Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift b/Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift index e80d49ed8..74eb9ad9c 100644 --- a/Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift +++ b/Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift @@ -67,7 +67,7 @@ struct HTTPClientConfigurationPropsTests { #expect(follow.allowCycles) #expect(follow.retainHTTPMethodAndBodyOn301) #expect(follow.retainHTTPMethodAndBodyOn302) - case .disallow: + case .disallow, .custom: Issue.record("Unexpected value") } @@ -130,7 +130,7 @@ struct HTTPClientConfigurationPropsTests { switch config.redirectConfiguration.mode { case .disallow: break - case .follow: + case .follow, .custom: Issue.record("Unexpected value") } } From 26f4fc38f7b689817aa7d7d37d2d1dc7e47db145 Mon Sep 17 00:00:00 2001 From: brennobemoura <37243584+brennobemoura@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:04:01 -0300 Subject: [PATCH 2/4] Rework custom redirect handling as a pluggable HTTPClientRedirectStrategy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../AsyncAwait/HTTPClient+execute.swift | 21 ++-- .../HTTPClientRedirectStrategy.swift | 96 +++++++++++++++++ Sources/AsyncHTTPClient/HTTPClient.swift | 68 ++++-------- Sources/AsyncHTTPClient/RedirectState.swift | 14 +-- .../AsyncAwaitEndToEndTests.swift | 101 +++++++++++++++--- .../HTTPClientTests.swift | 2 +- .../SwiftConfigurationTests.swift | 4 +- 7 files changed, 229 insertions(+), 77 deletions(-) create mode 100644 Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRedirectStrategy.swift diff --git a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift index e2a162342..2ba78eaab 100644 --- a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift +++ b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift @@ -162,7 +162,7 @@ extension HTTPClient { currentRequest = newRequest - case .custom(let handler): + case .strategy(let strategy): guard let redirectURL = response.headers.extractRedirectTarget( status: response.status, @@ -175,10 +175,10 @@ extension HTTPClient { } // Pre-build the request the same way `.follow` would, applying the standard - // method/header rewrite rules, so the handler only needs to make further + // method/header rewrite rules, so the strategy only needs to make further // adjustments rather than reimplement those rules itself. `max`/`allowCycles` // are irrelevant here: only the `retainHTTPMethodAndBodyOn30{1,2}` flags feed - // into this transformation, and there's no built-in limit in `.custom` mode. + // into this transformation, and there's no built-in limit in `.strategy` mode. let candidateRequest = currentRequest.followingRedirect( from: preparedRequest.url, to: redirectURL, @@ -191,13 +191,18 @@ extension HTTPClient { ) ) - let responseHead = HTTPResponseHead( - version: response.version, - status: response.status, - headers: response.headers + let context = HTTPClientRedirectContext( + redirectRequest: candidateRequest, + response: HTTPResponseHead( + version: response.version, + status: response.status, + headers: response.headers + ), + history: history, + redirectCount: customRedirectCount ) - switch handler(candidateRequest, responseHead, customRedirectCount) { + switch try strategy.redirectDecision(for: context) { case .doNotFollow: return response diff --git a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRedirectStrategy.swift b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRedirectStrategy.swift new file mode 100644 index 000000000..508b2ff79 --- /dev/null +++ b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRedirectStrategy.swift @@ -0,0 +1,96 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the AsyncHTTPClient open source project +// +// Copyright (c) 2026 Apple Inc. and the AsyncHTTPClient project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import NIOHTTP1 + +/// A pluggable strategy for deciding whether — and how — to follow HTTP redirects, used via +/// ``HTTPClient/Configuration/RedirectConfiguration/strategy(_:)``. +/// +/// Unlike `.disallow`/`.follow(max:allowCycles:)`, a strategy gets a chance to inspect every +/// redirect-eligible response before it's followed: adjust the outgoing request, refuse the redirect +/// outright, or fail the whole request with a custom error. +/// +/// A single strategy instance is stored on ``HTTPClient/Configuration`` and reused for every request +/// that client makes, including concurrently — if your strategy holds mutable state (e.g. an audit +/// log, a shared allow-list), synchronize it yourself (an `actor`, or a class using a lock). +/// Per-request state doesn't need that: ``HTTPClientRedirectContext/history`` already carries +/// everything tracked so far for the *current* logical request, so most policies (host allow-listing, +/// loop bounds, auditing) can be implemented statelessly by reading it fresh on each call. +@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) +public protocol HTTPClientRedirectStrategy: Sendable { + /// Decide whether — and how — to follow a redirect. + /// + /// - Parameter context: Everything known about the redirect so far. See + /// ``HTTPClientRedirectContext``. + /// - Returns: Whether — and with what request — to follow the redirect. + /// - Throws: To fail the whole `execute(...)` call with a custom error instead of following the + /// redirect or returning the response that triggered it. + func redirectDecision(for context: HTTPClientRedirectContext) throws -> HTTPClientRedirectDecision +} + +/// Everything a ``HTTPClientRedirectStrategy`` is handed to decide whether — and how — to follow one +/// redirect. +@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) +public struct HTTPClientRedirectContext: Sendable { + /// The request that would be sent to follow the redirect. It has already gone through the same + /// method/header rewrite rules `.follow` would apply (converting `POST` to `GET` on a 303, + /// stripping `Authorization`/`Cookie`/`Origin`/`Proxy-Authorization` on cross-origin redirects) — + /// you only need to make further adjustments, not reimplement those rules from scratch. + public var redirectRequest: HTTPClientRequest + + /// The head of the response that triggered the redirect. + public var response: HTTPResponseHead + + /// Every request/response pair sent so far for this logical request, oldest first, including the + /// one that produced ``response``. This is the same data that ends up in + /// ``HTTPClientResponse/history`` on the final response. + public var history: [HTTPClientRequestResponse] + + /// How many redirects have already been followed for this logical request (equivalently, + /// `history.count - 1`). There is no built-in limit for `.strategy`/`.custom` mode — enforce your + /// own policy (e.g. refusing past a maximum count) to avoid infinite redirect loops. + public var redirectCount: Int + + public init( + redirectRequest: HTTPClientRequest, + response: HTTPResponseHead, + history: [HTTPClientRequestResponse], + redirectCount: Int + ) { + self.redirectRequest = redirectRequest + self.response = response + self.history = history + self.redirectCount = redirectCount + } +} + +/// The result of a ``HTTPClientRedirectStrategy`` deciding whether — and how — to follow a redirect. +@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) +public enum HTTPClientRedirectDecision: Sendable { + /// Follow the redirect using the given request. + case follow(HTTPClientRequest) + /// Do not follow the redirect; the response that triggered it is returned as-is. + case doNotFollow +} + +/// Adapts a closure to ``HTTPClientRedirectStrategy``, backing +/// ``HTTPClient/Configuration/RedirectConfiguration/custom(_:)``. +@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) +struct ClosureRedirectStrategy: HTTPClientRedirectStrategy { + let handler: @Sendable (HTTPClientRedirectContext) throws -> HTTPClientRedirectDecision + + func redirectDecision(for context: HTTPClientRedirectContext) throws -> HTTPClientRedirectDecision { + try self.handler(context) + } +} diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 4aeff4d49..df9e70194 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -760,12 +760,12 @@ public final class HTTPClient: Sendable { ) if #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *), - case .custom = self.configuration.redirectConfiguration.mode + case .strategy = self.configuration.redirectConfiguration.mode { - // `.custom` redirect handlers operate on `HTTPClientRequest`/`HTTPClientResponse` and are - // only wired up for the Swift Concurrency `execute(_:deadline:logger:)` family of APIs. + // `.strategy`/`.custom` redirect handlers operate on `HTTPClientRequest`/`HTTPClientResponse` + // and are only wired up for the Swift Concurrency `execute(_:deadline:logger:)` family of APIs. logger.debug( - "`.custom` redirect configuration is not supported by the delegate-based execute API, failing request" + "`.strategy` redirect configuration is not supported by the delegate-based execute API, failing request" ) return Task.failedTask( eventLoop: taskEL, @@ -1334,9 +1334,9 @@ extension HTTPClient.Configuration { case disallow /// Redirects are followed with a specified limit. case follow(FollowConfiguration) - /// Redirects are handed to a caller-supplied handler. + /// Redirects are handed to a pluggable ``HTTPClientRedirectStrategy``. @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) - case custom(CustomRedirectHandler) + case strategy(any HTTPClientRedirectStrategy) } /// Configuration for following redirects. @@ -1373,38 +1373,6 @@ extension HTTPClient.Configuration { } } - /// The result of a ``CustomRedirectHandler`` deciding whether — and how — to follow a redirect. - @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) - public enum RedirectDecision: Sendable { - /// Follow the redirect using the given request. - case follow(HTTPClientRequest) - /// Do not follow the redirect; the response that triggered it is returned as-is. - case doNotFollow - } - - /// A handler invoked whenever a response indicates a redirect (a 3xx status with a `Location` - /// header), giving the caller the chance to inspect, modify, or refuse the redirect before it is - /// sent. - /// - /// `redirectRequest` has already gone through the same method/header rewrite rules `.follow` - /// would apply (converting `POST` to `GET` on a 303, stripping `Authorization`/`Cookie`/`Origin`/ - /// `Proxy-Authorization` on cross-origin redirects) — the handler only needs to make further - /// adjustments, not reimplement those rules from scratch. - /// - /// - Parameters: - /// - redirectRequest: The request that would be sent to follow the redirect. - /// - response: The head of the response that triggered the redirect. - /// - redirectCount: How many redirects have already been followed for this logical request. - /// There is no built-in limit for `.custom` mode — the handler is responsible for enforcing - /// its own policy (e.g. refusing past a maximum count) to avoid infinite redirect loops. - /// - Returns: Whether — and with what request — to follow the redirect. - @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) - public typealias CustomRedirectHandler = @Sendable ( - _ redirectRequest: HTTPClientRequest, - _ response: HTTPResponseHead, - _ redirectCount: Int - ) -> RedirectDecision - var mode: Mode init() { @@ -1450,17 +1418,27 @@ extension HTTPClient.Configuration { .init(configuration: .follow(configuration)) } - /// Redirects are handed to a caller-supplied handler, which decides whether and how to follow - /// each one. See ``CustomRedirectHandler``. + /// Redirects are handed to a pluggable strategy, which decides whether and how to follow each + /// one. See ``HTTPClientRedirectStrategy``. /// /// - warning: There is no built-in redirect-count or cycle limit for this mode — use the - /// `redirectCount` passed to the handler to enforce your own policy. + /// `redirectCount`/`history` passed to the strategy to enforce your own policy. /// - note: Only supported by the Swift Concurrency `execute(_:deadline:logger:)` family of APIs. - /// Using `.custom` with the delegate-based `execute(request:delegate:...)` API fails with - /// ``HTTPClientError/invalidRedirectConfiguration``. + /// Using `.strategy`/`.custom` with the delegate-based `execute(request:delegate:...)` API + /// fails with ``HTTPClientError/invalidRedirectConfiguration``. + @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) + public static func strategy(_ strategy: any HTTPClientRedirectStrategy) -> RedirectConfiguration { + .init(configuration: .strategy(strategy)) + } + + /// Convenience over ``strategy(_:)`` for a policy that doesn't need its own type: redirects are + /// handed to `handler`, which decides whether and how to follow each one. See + /// ``HTTPClientRedirectContext`` for what `handler` receives. @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) - public static func custom(_ handler: @escaping CustomRedirectHandler) -> RedirectConfiguration { - .init(configuration: .custom(handler)) + public static func custom( + _ handler: @escaping @Sendable (HTTPClientRedirectContext) throws -> HTTPClientRedirectDecision + ) -> RedirectConfiguration { + .strategy(ClosureRedirectStrategy(handler: handler)) } } diff --git a/Sources/AsyncHTTPClient/RedirectState.swift b/Sources/AsyncHTTPClient/RedirectState.swift index bfd4d89b1..466b4af6a 100644 --- a/Sources/AsyncHTTPClient/RedirectState.swift +++ b/Sources/AsyncHTTPClient/RedirectState.swift @@ -22,10 +22,10 @@ import struct Foundation.URL typealias RedirectMode = HTTPClient.Configuration.RedirectConfiguration.Mode -// `Mode` can't derive `Equatable`/`Hashable` because `.custom` carries a closure. `.custom` values have -// no meaningful notion of equality, so — like `NaN` — a `.custom` value is never equal to any other -// value, including another `.custom`; this is consistent (if vacuously so) with the `Hashable` -// requirement that equal values hash equally. +// `Mode` can't derive `Equatable`/`Hashable` because `.strategy` carries an existential. `.strategy` +// values have no meaningful notion of equality, so — like `NaN` — a `.strategy` value is never equal +// to any other value, including another `.strategy`; this is consistent (if vacuously so) with the +// `Hashable` requirement that equal values hash equally. extension HTTPClient.Configuration.RedirectConfiguration.Mode: Equatable { static func == (lhs: Self, rhs: Self) -> Bool { switch (lhs, rhs) { @@ -47,7 +47,7 @@ extension HTTPClient.Configuration.RedirectConfiguration.Mode: Hashable { case .follow(let config): hasher.combine(1) hasher.combine(config) - case .custom: + case .strategy: hasher.combine(2) } } @@ -73,8 +73,8 @@ extension RedirectState { return nil case .follow(let config): self.init(config: config, visited: [initialURL]) - case .custom: - // `.custom` redirects are handled entirely by the caller-supplied handler; there is no + case .strategy: + // `.strategy` redirects are handled entirely by the caller-supplied strategy; there is no // count/cycle state for `RedirectState` to track. return nil } diff --git a/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift b/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift index b2c28b796..f7d627d02 100644 --- a/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift +++ b/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift @@ -769,7 +769,7 @@ final class AsyncAwaitEndToEndTests: XCTestCase { } } - // MARK: - Custom redirect handler + // MARK: - Pluggable redirect strategies func testCustomRedirectHandlerCanRewriteRedirectRequest() { XCTAsyncTest { @@ -778,14 +778,15 @@ final class AsyncAwaitEndToEndTests: XCTestCase { let port = bin.port var config = HTTPClient.Configuration() - config.redirectConfiguration = .custom { redirectRequest, response, redirectCount in - XCTAssertEqual(response.status, .found) - XCTAssertEqual(redirectCount, 0) + config.redirectConfiguration = .custom { context in + XCTAssertEqual(context.response.status, .found) + XCTAssertEqual(context.redirectCount, 0) + XCTAssertEqual(context.history.count, 1) // The candidate request has already gone through the standard rewrite rules: it // should already point at the `Location` from the /redirect/302 response. - XCTAssertEqual(redirectRequest.url, "http://localhost:\(port)/ok") + XCTAssertEqual(context.redirectRequest.url, "http://localhost:\(port)/ok") - var rewritten = redirectRequest + var rewritten = context.redirectRequest rewritten.url = "http://localhost:\(port)/echo-uri" return .follow(rewritten) } @@ -815,7 +816,7 @@ final class AsyncAwaitEndToEndTests: XCTestCase { defer { XCTAssertNoThrow(try bin.shutdown()) } var config = HTTPClient.Configuration() - config.redirectConfiguration = .custom { _, _, _ in .doNotFollow } + config.redirectConfiguration = .custom { _ in .doNotFollow } let client = HTTPClient(eventLoopGroupProvider: .singleton, configuration: config) defer { XCTAssertNoThrow(try client.syncShutdown()) } @@ -840,20 +841,21 @@ final class AsyncAwaitEndToEndTests: XCTestCase { let observedCounts = NIOLockedValueBox<[Int]>([]) var config = HTTPClient.Configuration() - config.redirectConfiguration = .custom { redirectRequest, _, redirectCount in - observedCounts.withLockedValue { $0.append(redirectCount) } - guard redirectCount < 3 else { + config.redirectConfiguration = .custom { context in + XCTAssertEqual(context.history.count, context.redirectCount + 1) + observedCounts.withLockedValue { $0.append(context.redirectCount) } + guard context.redirectCount < 3 else { return .doNotFollow } - return .follow(redirectRequest) + return .follow(context.redirectRequest) } let client = HTTPClient(eventLoopGroupProvider: .singleton, configuration: config) defer { XCTAssertNoThrow(try client.syncShutdown()) } - // /redirect/infinite1 <-> /redirect/infinite2 bounce forever. `.custom` mode has no - // built-in redirect limit (unlike `.follow`), so this exercises the handler enforcing - // its own bound using the `redirectCount` it's handed. + // /redirect/infinite1 <-> /redirect/infinite2 bounce forever. `.custom`/`.strategy` mode + // has no built-in redirect limit (unlike `.follow`), so this exercises the handler + // enforcing its own bound using the `redirectCount` it's handed. let request = HTTPClientRequest(url: "http://localhost:\(bin.port)/redirect/infinite1") guard let response = await XCTAssertNoThrowWithResult( @@ -867,6 +869,77 @@ final class AsyncAwaitEndToEndTests: XCTestCase { } } + /// A real `HTTPClientRedirectStrategy` conformance (not a closure), proving strategies are + /// genuinely pluggable types — and using `history` (not just a count) to detect that a redirect + /// target has already been visited, the way `.follow(allowCycles: false)` does internally. + private struct VisitedURLCycleDetectingStrategy: HTTPClientRedirectStrategy { + func redirectDecision(for context: HTTPClientRedirectContext) throws -> HTTPClientRedirectDecision { + let visited = Set(context.history.map(\.request.url)) + guard !visited.contains(context.redirectRequest.url) else { + return .doNotFollow + } + return .follow(context.redirectRequest) + } + } + + func testRedirectStrategyTypeDetectsCyclesUsingHistory() { + XCTAsyncTest { + let bin = HTTPBin(.http1_1(compress: false)) + defer { XCTAssertNoThrow(try bin.shutdown()) } + + var config = HTTPClient.Configuration() + config.redirectConfiguration = .strategy(VisitedURLCycleDetectingStrategy()) + + let client = HTTPClient(eventLoopGroupProvider: .singleton, configuration: config) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + // infinite1 -> infinite2 -> infinite1 (already visited, refused). + let request = HTTPClientRequest(url: "http://localhost:\(bin.port)/redirect/infinite1") + guard + let response = await XCTAssertNoThrowWithResult( + try await client.execute(request, deadline: .now() + .seconds(10)) + ) + else { return } + + XCTAssertEqual(response.status, .found) + XCTAssertEqual( + response.history.map(\.request.url), + [ + "http://localhost:\(bin.port)/redirect/infinite1", + "http://localhost:\(bin.port)/redirect/infinite2", + ] + ) + } + } + + private struct RedirectRefusedError: Error, Equatable {} + + private struct ThrowingRedirectStrategy: HTTPClientRedirectStrategy { + func redirectDecision(for context: HTTPClientRedirectContext) throws -> HTTPClientRedirectDecision { + throw RedirectRefusedError() + } + } + + func testRedirectStrategyCanThrowToFailTheRequest() { + XCTAsyncTest { + let bin = HTTPBin(.http1_1(compress: false)) + defer { XCTAssertNoThrow(try bin.shutdown()) } + + var config = HTTPClient.Configuration() + config.redirectConfiguration = .strategy(ThrowingRedirectStrategy()) + + let client = HTTPClient(eventLoopGroupProvider: .singleton, configuration: config) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + let request = HTTPClientRequest(url: "http://localhost:\(bin.port)/redirect/302") + await XCTAssertThrowsError( + try await client.execute(request, deadline: .now() + .seconds(10)) + ) { + XCTAssertEqual($0 as? RedirectRefusedError, RedirectRefusedError()) + } + } + } + func testShutdown() { XCTAsyncTest { let client = makeDefaultHTTPClient() diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 74890932a..5ae14994e 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -456,7 +456,7 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { let localClient = HTTPClient( eventLoopGroupProvider: .shared(self.clientGroup), configuration: HTTPClient.Configuration( - redirectConfiguration: .custom { _, _, _ in .doNotFollow } + redirectConfiguration: .custom { _ in .doNotFollow } ) ) defer { XCTAssertNoThrow(try localClient.syncShutdown()) } diff --git a/Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift b/Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift index 74eb9ad9c..f2f1f8a98 100644 --- a/Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift +++ b/Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift @@ -67,7 +67,7 @@ struct HTTPClientConfigurationPropsTests { #expect(follow.allowCycles) #expect(follow.retainHTTPMethodAndBodyOn301) #expect(follow.retainHTTPMethodAndBodyOn302) - case .disallow, .custom: + case .disallow, .strategy: Issue.record("Unexpected value") } @@ -130,7 +130,7 @@ struct HTTPClientConfigurationPropsTests { switch config.redirectConfiguration.mode { case .disallow: break - case .follow, .custom: + case .follow, .strategy: Issue.record("Unexpected value") } } From 92febeb6bbd32a420aece71434e446341e3cbac6 Mon Sep 17 00:00:00 2001 From: brennobemoura <37243584+brennobemoura@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:16:48 -0300 Subject: [PATCH 3/4] Re-trigger CI now that beta's swift-ci.yaml points at a valid ref 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 From c1ecceb1f6dcbe63327c4734f103bb7393167154 Mon Sep 17 00:00:00 2001 From: brennobemoura <37243584+brennobemoura@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:44:25 -0300 Subject: [PATCH 4/4] Fix API-breakage-check build failure: enum case can't be @available MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift | 3 ++- Sources/AsyncHTTPClient/HTTPClient.swift | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift index 2ba78eaab..38982fdc7 100644 --- a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift +++ b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift @@ -162,7 +162,8 @@ extension HTTPClient { currentRequest = newRequest - case .strategy(let strategy): + case .strategy(let anyStrategy): + let strategy = anyStrategy as! any HTTPClientRedirectStrategy guard let redirectURL = response.headers.extractRedirectTarget( status: response.status, diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index df9e70194..301973538 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -1335,8 +1335,12 @@ extension HTTPClient.Configuration { /// Redirects are followed with a specified limit. case follow(FollowConfiguration) /// Redirects are handed to a pluggable ``HTTPClientRedirectStrategy``. - @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) - case strategy(any HTTPClientRedirectStrategy) + /// + /// Stored as `any Sendable` (erasure trick so this case doesn't need to be marked + /// `@available`, which Swift disallows on enum cases with associated values) — always an + /// `any HTTPClientRedirectStrategy` underneath, since `.strategy(_:)`/`.custom(_:)` are the + /// only way to construct one. + case strategy(any Sendable) } /// Configuration for following redirects.