diff --git a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift index a1047fa85..8fc0b3759 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,100 @@ 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 case .follow(var followState)? = 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 followState.redirect(to: redirectURL.absoluteString) + currentRedirectState = .follow(followState) + + let newRequest = currentRequest.followingRedirect( + from: preparedRequest.url, + to: redirectURL, status: response.status, - originalURL: preparedRequest.url, - originalScheme: preparedRequest.poolKey.scheme + config: followState.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 .strategy(let anyStrategy): + let strategy = anyStrategy as! any HTTPClientRedirectStrategy + 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 + } + + // Pre-build the request the same way `.follow` would, applying the standard + // 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 `.strategy` mode. + let candidateRequest = currentRequest.followingRedirect( + from: preparedRequest.url, + to: redirectURL, + status: response.status, + config: .init( + max: 0, + allowCycles: true, + retainHTTPMethodAndBodyOn301: false, + retainHTTPMethodAndBodyOn302: false + ) + ) - currentRequest = newRequest + let context = HTTPClientRedirectContext( + redirectRequest: candidateRequest, + response: HTTPResponseHead( + version: response.version, + status: response.status, + headers: response.headers + ), + history: history, + redirectCount: customRedirectCount + ) + + switch try strategy.redirectDecision(for: context) { + 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/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/AsyncAwait/HTTPClientRequest+Prepared.swift b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRequest+Prepared.swift index 70d158460..f540bc15b 100644 --- a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRequest+Prepared.swift +++ b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRequest+Prepared.swift @@ -114,6 +114,14 @@ extension HTTPClientRequest.Prepared.Body { ) case .byteBuffer(let byteBuffer): self = .byteBuffer(byteBuffer) + case .delegateBody: + // Only ever produced by the delegate-based redirect-strategy bridge + // (`RedirectStrategyDelegateBridge.swift`), which converts it back into a + // `HTTPClient.Body` (`asDelegateBody()`) rather than routing it through the + // Concurrency API's own request preparation. + fatalError( + "`.delegateBody` never reaches `HTTPClientRequest.Prepared` -- it is unwrapped via `asDelegateBody()` instead." + ) #if UnstableHTTPAPIsSupport case .httpClientRequestBody(let length, let requestBody): self = .httpClientRequestBody(length, requestBody) @@ -132,6 +140,10 @@ extension RequestBodyLength { self = .known(Int64(buffer.readableBytes)) case .sequence(let length, _, _), .asyncSequence(let length, _): self = length + case .delegateBody: + fatalError( + "`.delegateBody` never reaches `HTTPClientRequest.Prepared` -- it is unwrapped via `asDelegateBody()` instead." + ) #if UnstableHTTPAPIsSupport case .httpClientRequestBody(let length, _): self = length diff --git a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRequest.swift b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRequest.swift index 17563a122..d6ee28cf0 100644 --- a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRequest.swift +++ b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRequest.swift @@ -101,6 +101,17 @@ extension HTTPClientRequest { ) case byteBuffer(ByteBuffer) + /// Wraps a delegate-based-API ``HTTPClient/Body`` verbatim, untouched. + /// + /// Exists solely so the delegate-based `execute(request:delegate:...)` API can offer a + /// redirect-eligible request's body to a ``HTTPClientRedirectStrategy`` (which is typed + /// in terms of this Concurrency-API `Body`, not that one) without re-encoding it -- see + /// `RedirectStrategyDelegateBridge.swift`. `canBeConsumedMultipleTimes` is conservatively + /// `false`: a delegate-API `Body`'s `stream` closure isn't guaranteed replayable. Never + /// produced by any public factory, and never asked to iterate -- it is unwrapped back + /// into a `HTTPClient.Body` (`asDelegateBody()`) before it would ever need to stream. + case delegateBody(HTTPClient.Body) + #if UnstableHTTPAPIsSupport case httpClientRequestBody( length: RequestBodyLength, @@ -398,6 +409,7 @@ extension Optional where Wrapped == HTTPClientRequest.Body { case .byteBuffer: return true case .sequence(_, let canBeConsumedMultipleTimes, _): return canBeConsumedMultipleTimes case .asyncSequence: return false + case .delegateBody: return false #if UnstableHTTPAPIsSupport case .httpClientRequestBody: return false // TODO: I think this should be TRUE #endif @@ -441,6 +453,11 @@ extension HTTPClientRequest.Body: AsyncSequence { return .init(storage: .byteBuffer(makeCompleteBody(AsyncIterator.allocator))) case .byteBuffer(let byteBuffer): return .init(storage: .byteBuffer(byteBuffer)) + case .delegateBody: + // Only ever produced by the delegate-based redirect-strategy bridge, which unwraps + // it back into a `HTTPClient.Body` (`asDelegateBody()`) instead of ever asking this + // AsyncSequence conformance to iterate it. + fatalError("`.delegateBody` is never iterated -- it is unwrapped via `asDelegateBody()` instead.") #if UnstableHTTPAPIsSupport case .httpClientRequestBody: fatalError("Unimplemented") diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index cc8792497..0bcca2fca 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -759,6 +759,27 @@ public final class HTTPClient: Sendable { ] ) + if case .strategy = self.configuration.redirectConfiguration.mode, + #unavailable(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0) + { + // `.strategy(_:)`/`.custom(_:)` require this same availability floor to construct, so + // this is unreachable in practice -- kept as defense in depth rather than silently + // falling back to "no redirects are followed" (what `RedirectState.init?` returning + // `nil` here would otherwise mean). On an available OS, `.strategy` is now handled by + // `RedirectHandler` alongside `.follow`, driving the strategy through the + // delegate-based path via `RedirectStrategyDelegateBridge.swift`. + logger.debug( + "`.strategy` redirect configuration requires a newer OS than this process is running on, 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 +1333,18 @@ 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 pluggable ``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. @@ -1397,6 +1425,33 @@ extension HTTPClient.Configuration { public static func follow(configuration: FollowConfiguration) -> RedirectConfiguration { .init(configuration: .follow(configuration)) } + + /// 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`/`history` passed to the strategy to enforce your own policy. + /// - note: Supported by both the Swift Concurrency `execute(_:deadline:logger:)` family and + /// the delegate-based `execute(request:delegate:...)` API. On the latter, `.follow(_:)` + /// reissues through the delegate API's own request/body types, and a body your strategy + /// replaces with a new streaming (`.stream`/`AsyncSequence`-backed) representation throws + /// ``HTTPClientError/redirectStrategyBodyNotSupported`` -- nothing at redirect-decision + /// time is in a position to drain one. `context.redirectRequest` unchanged, or a + /// `.bytes`/`.byteBuffer` replacement, works either way. + @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 @Sendable (HTTPClientRedirectContext) throws -> HTTPClientRedirectDecision + ) -> RedirectConfiguration { + .strategy(ClosureRedirectStrategy(handler: handler)) + } } /// Connection pool configuration. @@ -1525,6 +1580,7 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { case invalidDNSOverridesConfiguration case invalidLocalAddress case invalidProxyConfiguration + case redirectStrategyBodyNotSupported case internalStateFailure(file: String, line: UInt) } @@ -1622,6 +1678,9 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { return "Invalid local address" case .invalidProxyConfiguration: return "The proxy configuration is not valid" + case .redirectStrategyBodyNotSupported: + return + "A redirect strategy returned a streaming request body over the delegate-based execute API, which can't drain it at redirect-decision time. Return a `.bytes`/`.byteBuffer` body, reuse `context.redirectRequest` unchanged, or use the Concurrency `execute(_:deadline:logger:)` API instead." case .internalStateFailure(let file, let line): return "An internal state failure has occurred (File: \(file), line: \(line)). Please open an issue with a reproducer if possible" @@ -1731,6 +1790,10 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { /// The proxy configuration is not valid. public static let invalidProxyConfiguration = HTTPClientError(code: .invalidProxyConfiguration) + /// A ``HTTPClientRedirectStrategy`` returned a request body the delegate-based + /// `execute(request:delegate:...)` API can't drain at redirect-decision time. + public static let redirectStrategyBodyNotSupported = HTTPClientError(code: .redirectStrategyBodyNotSupported) + /// A state machine has reached an unsupported state, that wasn't considered when implementing. public static func internalStateFailure(file: String = #fileID, line: UInt = #line) -> HTTPClientError { HTTPClientError(code: .internalStateFailure(file: file, line: line)) diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index b6fdbfeac..6bb265bf2 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -1059,6 +1059,18 @@ internal struct RedirectHandler { let redirectState: RedirectState let execute: (HTTPClient.Request, RedirectState) -> HTTPClient.Task + /// Set only on a copy returned from `earlyStrategyDecision(head:)`, carrying what that + /// already decided so `redirect(head:to:promise:)` -- called later, once the state machine + /// gets around to it -- reissues/fails accordingly instead of asking the strategy again. + /// `.follow` mode never sets this; its decision (redirect-limit/cycle check) has nothing to + /// precompute and stays exactly as cheap to make late as early. + var precomputed: Precomputed? + + enum Precomputed { + case launch(HTTPClient.Request, RedirectState) + case fail(Error) + } + func redirectTarget(status: HTTPResponseStatus, responseHeaders: HTTPHeaders) -> URL? { responseHeaders.extractRedirectTarget( status: status, @@ -1067,14 +1079,154 @@ internal struct RedirectHandler { ) } + /// What `earlyStrategyDecision(head:)` decided, so `receiveResponseHead` (`RequestBag + /// +StateMachine.swift`) can act on it without re-deriving anything. + enum EarlyStrategyDecision { + /// `.follow` mode, or `head` isn't redirect-eligible at all -- proceed exactly as + /// before this existed. + case notApplicable + /// The response that triggered this should be delivered as the final response, same as + /// any ordinary (non-redirect-candidate) response -- nothing about its body has been + /// touched yet, so this works regardless of size. + case doNotFollow + /// Either what to launch, or what error to fail with -- both already computed, both + /// requiring the same redirect-eligible response's body to be drained-or-cancelled the + /// same way `.follow` does, hence going through the same `redirectURL`-tagged path. + case decided(RedirectHandler, URL) + } + + /// Computes a `.strategy` redirect configuration's decision for `head`, before any response + /// body byte has been read. Safe to call regardless of body size or whether it's even known + /// (`Content-Length` present, absent, or huge) -- `HTTPClientRedirectContext` never carries + /// a body, so nothing here needs one. (Adding a body field to that type later would break + /// this assumption.) + /// + /// - Important: `strategy.redirectDecision(for:)` is arbitrary caller code that may + /// synchronously reenter this task (e.g. call `task.cancel()`). Call this from outside any + /// exclusive access to the state machine that will go on to consume the result -- + /// `RequestBag.receiveResponseHead0` calls this first and hands the state machine only the + /// already-computed answer, never the handler itself, for exactly this reason. + func earlyStrategyDecision(head: HTTPResponseHead) -> EarlyStrategyDecision { + guard case .strategy(let anyStrategyState) = self.redirectState else { + return .notApplicable + } + + guard let redirectURL = self.redirectTarget(status: head.status, responseHeaders: head.headers) else { + return .notApplicable + } + + guard #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) else { + // Unreachable in practice -- see `RedirectState.init?`'s own guard for why. Once the + // state machine eventually calls `redirect(head:to:promise:)` on this (unmodified) + // handler, `redirectByStrategy` fails it the same way `_execute`'s own top-level + // guard would have. + return .notApplicable + } + + let strategyState = anyStrategyState as! RedirectState.Strategy + + do { + let (method, headers, body) = transformRequestForRedirect( + from: request.url, + method: self.request.method, + headers: self.request.headers, + body: self.request.body, + to: redirectURL, + status: head.status, + // Matches the Concurrency API's own `.strategy` loop: `max`/`allowCycles` don't + // apply to `.strategy` mode (there's no built-in limit), only the + // `retainHTTPMethodAndBodyOn30{1,2}` flags feed into this rewrite. + config: .init( + max: 0, + allowCycles: true, + retainHTTPMethodAndBodyOn301: false, + retainHTTPMethodAndBodyOn302: false + ) + ) + + let candidateRequest = try HTTPClient.Request( + url: redirectURL, + method: method, + headers: headers, + body: body, + tlsConfiguration: self.request.tlsConfiguration + ) + + let history = + strategyState.history + [ + HTTPClientRequestResponse( + request: HTTPClientRequest(delegateRequest: self.request), + responseHead: head + ) + ] + + let context = HTTPClientRedirectContext( + redirectRequest: HTTPClientRequest(delegateRequest: candidateRequest), + response: head, + history: history, + redirectCount: strategyState.redirectCount + ) + + switch try strategyState.strategy.redirectDecision(for: context) { + case .doNotFollow: + return .doNotFollow + + case .follow(let newRequest): + let newDelegateRequest = try newRequest.asDelegateRequest() + + let newStrategyState = RedirectState.Strategy( + strategy: strategyState.strategy, + history: history, + redirectCount: strategyState.redirectCount + 1 + ) + + var decided = self + decided.precomputed = .launch(newDelegateRequest, .strategy(newStrategyState)) + return .decided(decided, redirectURL) + } + } catch { + var decided = self + decided.precomputed = .fail(error) + return .decided(decided, redirectURL) + } + } + + /// Reissues or fails according to whatever `earlyStrategyDecision(head:)` already decided + /// (`.strategy` mode), or makes `.follow` mode's decision now, for the first time (there is + /// nothing to precompute there -- the redirect-limit/cycle check is exactly as cheap late as + /// early, and always ends in "redirect" or "fail the task", never "delivered normally"). func redirect( + head: HTTPResponseHead, + to redirectURL: URL, + promise: EventLoopPromise + ) -> HTTPClient.Task? { + if let precomputed = self.precomputed { + switch precomputed { + case .launch(let newRequest, let newRedirectState): + return self.launch(newRequest, newRedirectState, promise: promise) + case .fail(let error): + promise.fail(error) + return nil + } + } + + switch self.redirectState { + case .follow(let followState): + return self.redirectByFollowing(followState, status: head.status, to: redirectURL, promise: promise) + case .strategy: + return self.redirectByStrategy(promise: promise) + } + } + + private func redirectByFollowing( + _ followState: RedirectState.Follow, status: HTTPResponseStatus, to redirectURL: URL, promise: EventLoopPromise ) -> HTTPClient.Task? { do { - var redirectState = self.redirectState - try redirectState.redirect(to: redirectURL.absoluteString) + var followState = followState + try followState.redirect(to: redirectURL.absoluteString) let (method, headers, body) = transformRequestForRedirect( from: request.url, @@ -1083,7 +1235,7 @@ internal struct RedirectHandler { body: self.request.body, to: redirectURL, status: status, - config: self.redirectState.config + config: followState.config ) let newRequest = try HTTPClient.Request( @@ -1094,20 +1246,39 @@ internal struct RedirectHandler { tlsConfiguration: self.request.tlsConfiguration ) - let newTask = self.execute(newRequest, redirectState) - - newTask.futureResult.whenComplete { result in - promise.futureResult.eventLoop.execute { - promise.completeWith(result) - } - } - - return newTask + return self.launch(newRequest, .follow(followState), promise: promise) } catch { promise.fail(error) return nil } } + + /// Reached only when `earlyStrategyDecision(head:)` couldn't run the strategy at all -- + /// i.e. an OS below the availability floor `.strategy(_:)`/`.custom(_:)` require to + /// construct in the first place. Unreachable in practice; see `RedirectState.init?`'s own + /// guard for why. Every other path through `.strategy` mode is decided by + /// `earlyStrategyDecision(head:)` at head-received time and carried here via + /// `precomputed`, above, before this method would ever be reached. + private func redirectByStrategy(promise: EventLoopPromise) -> HTTPClient.Task? { + promise.fail(HTTPClientError.invalidRedirectConfiguration) + return nil + } + + private func launch( + _ newRequest: HTTPClient.Request, + _ newRedirectState: RedirectState, + promise: EventLoopPromise + ) -> HTTPClient.Task { + let newTask = self.execute(newRequest, newRedirectState) + + newTask.futureResult.whenComplete { result in + promise.futureResult.eventLoop.execute { + promise.completeWith(result) + } + } + + return newTask + } } extension RequestBodyLength { diff --git a/Sources/AsyncHTTPClient/RedirectState.swift b/Sources/AsyncHTTPClient/RedirectState.swift index 9665c03d4..5490aa0eb 100644 --- a/Sources/AsyncHTTPClient/RedirectState.swift +++ b/Sources/AsyncHTTPClient/RedirectState.swift @@ -22,11 +22,48 @@ import struct Foundation.URL typealias RedirectMode = HTTPClient.Configuration.RedirectConfiguration.Mode -struct RedirectState { - var config: HTTPClient.Configuration.RedirectConfiguration.FollowConfiguration +// `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) { + 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 .strategy: + hasher.combine(2) + } + } +} - /// All visited URLs. - private var visited: [String] +/// Tracks how the delegate-based `execute(request:delegate:...)` API should handle the next +/// redirect for one logical request, for whichever mode `HTTPClient.Configuration +/// .RedirectConfiguration.Mode` was configured with. +enum RedirectState { + case follow(Follow) + + /// Always a `Strategy` value underneath -- type-erased as `any Sendable` for the same reason + /// `Mode.strategy` itself is: `Strategy` is only available on OSes new enough for Swift + /// Concurrency, and Swift disallows `@available` on an enum case with an associated value. See + /// `Mode.strategy`'s own doc comment. + case strategy(any Sendable) } extension RedirectState { @@ -40,26 +77,66 @@ extension RedirectState { switch configuration { case .disallow: return nil + case .follow(let config): - self.init(config: config, visited: [initialURL]) + self = .follow(Follow(config: config, visited: [initialURL])) + + case .strategy(let anyStrategy): + guard #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) else { + // `.strategy(_:)`/`.custom(_:)` require this same availability floor to + // construct, so this is unreachable in practice. Falling back to "don't track + // redirect state" would silently degrade to "no redirects are ever followed" + // instead -- `_execute`'s own availability guard is what actually surfaces this + // as `.invalidRedirectConfiguration` rather than a silent behavior change. + return nil + } + + let strategy = anyStrategy as! any HTTPClientRedirectStrategy + self = .strategy(Strategy(strategy: strategy, history: [], redirectCount: 0)) } } } extension RedirectState { - /// Call this method when you are about to do a redirect to the given `redirectURL`. - /// This method records that URL into `self`. - /// - Parameter redirectURL: the new URL to redirect the request to - /// - Throws: if it reaches the redirect limit or detects a redirect cycle if and `allowCycles` is false - mutating func redirect(to redirectURL: String) throws { - guard self.visited.count <= config.max else { - throw HTTPClientError.redirectLimitReached + struct Follow: Sendable { + var config: HTTPClient.Configuration.RedirectConfiguration.FollowConfiguration + + /// All visited URLs. + private var visited: [String] + + fileprivate init(config: HTTPClient.Configuration.RedirectConfiguration.FollowConfiguration, visited: [String]) + { + self.config = config + self.visited = visited } - guard config.allowCycles || !self.visited.contains(redirectURL) else { - throw HTTPClientError.redirectCycleDetected + /// Call this method when you are about to do a redirect to the given `redirectURL`. + /// This method records that URL into `self`. + /// - Parameter redirectURL: the new URL to redirect the request to + /// - Throws: if it reaches the redirect limit or detects a redirect cycle if and `allowCycles` is false + mutating func redirect(to redirectURL: String) throws { + guard self.visited.count <= config.max else { + throw HTTPClientError.redirectLimitReached + } + + guard config.allowCycles || !self.visited.contains(redirectURL) else { + throw HTTPClientError.redirectCycleDetected + } + self.visited.append(redirectURL) } - self.visited.append(redirectURL) + } +} + +@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) +extension RedirectState { + struct Strategy: Sendable { + let strategy: any HTTPClientRedirectStrategy + + /// Every request/response pair sent so far for this logical request, oldest first. + var history: [HTTPClientRequestResponse] + + /// How many redirects have already been followed for this logical request. + var redirectCount: Int } } diff --git a/Sources/AsyncHTTPClient/RedirectStrategyDelegateBridge.swift b/Sources/AsyncHTTPClient/RedirectStrategyDelegateBridge.swift new file mode 100644 index 000000000..ddcb34cea --- /dev/null +++ b/Sources/AsyncHTTPClient/RedirectStrategyDelegateBridge.swift @@ -0,0 +1,93 @@ +//===----------------------------------------------------------------------===// +// +// 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 NIOCore + +#if canImport(FoundationEssentials) +import struct FoundationEssentials.URL +#else +import struct Foundation.URL +#endif + +/// Converts between the `HTTPClient.Request`/`HTTPClient.Body` the delegate-based +/// `execute(request:delegate:...)` API is built on, and the `HTTPClientRequest`/`HTTPClientRequest +/// .Body` a ``HTTPClientRedirectStrategy`` is offered and returns -- so a `.strategy` redirect +/// configuration, which is only natively wired up for the Swift Concurrency `execute(_:deadline: +/// logger:)` family, can also drive the delegate-based path's own redirect following (see +/// `RedirectHandler` in `HTTPHandler.swift`). +@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) +extension HTTPClientRequest { + /// Builds the request a strategy is offered from the `HTTPClient.Request` the delegate-based + /// path was about to follow a redirect with. + /// + /// `localAddress` has no equivalent field on `HTTPClient.Request`, so a strategy that sets it + /// on the request it returns has no effect when reissued through `asDelegateRequest()` -- + /// there is nowhere on that side to put it. + init(delegateRequest request: HTTPClient.Request) { + self.init(url: request.url.absoluteString) + self.method = request.method + self.headers = request.headers + self.body = request.body.map { HTTPClientRequest.Body(.delegateBody($0)) } + self.tlsConfiguration = request.tlsConfiguration + } + + /// Reissues a strategy's `.follow(_:)` decision through the delegate-based path. + /// + /// - Throws: Whatever `HTTPClient.Request.init(url:method:headers:body:tlsConfiguration:)` + /// throws for an invalid `url`, or ``HTTPClientError/redirectStrategyBodyNotSupported`` if + /// `body` is a streaming representation (`.stream`/`AsyncSequence`-backed, or the unstable + /// upload-writer body) that didn't originate from `init(delegateRequest:)` -- those can only + /// be drained by actually opening a connection and writing to it, which nothing at + /// redirect-decision time is in a position to do. A strategy that only returns + /// `context.redirectRequest` unchanged, or replaces its body with `.bytes`/`.byteBuffer`, + /// never hits this. + func asDelegateRequest() throws -> HTTPClient.Request { + try HTTPClient.Request( + url: self.url, + method: self.method, + headers: self.headers, + body: self.body.map { try $0.asDelegateBody() }, + tlsConfiguration: self.tlsConfiguration + ) + } +} + +@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) +extension HTTPClientRequest.Body { + fileprivate func asDelegateBody() throws -> HTTPClient.Body { + switch self.mode { + case .delegateBody(let body): + // The common case: a strategy that didn't touch `redirectRequest.body` at all, so + // this is still the exact body `init(delegateRequest:)` wrapped -- reused verbatim, + // no re-encoding, no loss of whatever streaming behavior it originally had. + return body + + case .byteBuffer(let buffer): + return .byteBuffer(buffer) + + case .sequence(_, _, let makeCompleteBody): + // `makeCompleteBody` is a plain synchronous closure (unlike `.asyncSequence`), so + // this is a lossless, non-streaming drain -- not an approximation. + return .byteBuffer(makeCompleteBody(ByteBufferAllocator())) + + case .asyncSequence: + throw HTTPClientError.redirectStrategyBodyNotSupported + + #if UnstableHTTPAPIsSupport + case .httpClientRequestBody: + throw HTTPClientError.redirectStrategyBodyNotSupported + #endif + } + } +} diff --git a/Sources/AsyncHTTPClient/RequestBag+StateMachine.swift b/Sources/AsyncHTTPClient/RequestBag+StateMachine.swift index 8f5cd37ce..f49734028 100644 --- a/Sources/AsyncHTTPClient/RequestBag+StateMachine.swift +++ b/Sources/AsyncHTTPClient/RequestBag+StateMachine.swift @@ -328,11 +328,32 @@ extension RequestBag.StateMachine { case redirect(HTTPRequestExecutor, RedirectHandler, HTTPResponseHead, URL) } + /// The redirect handler awaiting this request's response head, if any -- for computing a + /// `.strategy` configuration's decision (`RedirectHandler.earlyStrategyDecision(head:)`) + /// *before* calling `receiveResponseHead`, since that call is arbitrary caller code that may + /// reenter this task synchronously, which `receiveResponseHead`'s exclusive access to + /// `self.state` cannot tolerate. + var redirectHandlerAwaitingResponseHead: RedirectHandler? { + guard case .executing(_, _, .initialized(let handler)) = self.state else { + return nil + } + return handler + } + /// The response head has been received. /// - /// - Parameter head: The response' head + /// - Parameters: + /// - head: The response' head + /// - earlyDecision: What `redirectHandlerAwaitingResponseHead?.earlyStrategyDecision(head:)` + /// already decided for a `.strategy` redirect configuration, computed by the caller + /// before this call (see that method's own doc comment for why it can't be computed in + /// here). `.notApplicable` for `.follow` mode, no configured handler, or a non-redirect + /// response -- proceeds exactly as if this parameter didn't exist. /// - Returns: Whether the response should be forwarded to the delegate. Will be `false` if the request follows a redirect. - mutating func receiveResponseHead(_ head: HTTPResponseHead) -> ReceiveResponseHeadAction { + mutating func receiveResponseHead( + _ head: HTTPResponseHead, + earlyDecision: RedirectHandler.EarlyStrategyDecision + ) -> ReceiveResponseHeadAction { switch self.state { case .initialized, .queued, .deadlineExceededWhileQueued: preconditionFailure("How can we receive a response, if the request hasn't started yet.") @@ -341,26 +362,40 @@ extension RequestBag.StateMachine { preconditionFailure("If we receive a response, we must not have received something else before") } - if let redirectHandler = redirectHandler, - let redirectURL = redirectHandler.redirectTarget( - status: head.status, - responseHeaders: head.headers - ) - { - // If we will redirect, we need to consume the response's body ASAP, to be able to - // reuse the existing connection. We will consume a response body, if the body is - // smaller than 3kb. - switch head.contentLength { - case .some(0...(HTTPClient.maxBodySizeRedirectResponse)), .none: - self.state = .redirected(executor, redirectHandler, 0, head, redirectURL) - return .signalBodyDemand(executor) - case .some: - self.state = .finished(error: HTTPClientError.cancelled) - return .redirect(executor, redirectHandler, head, redirectURL) - } - } else { + switch earlyDecision { + case .doNotFollow: + // Nothing about the body has been touched -- deliver this response exactly like + // any ordinary, non-redirect-candidate one, regardless of its size. self.state = .executing(executor, requestState, .buffering(.init(), next: .askExecutorForMore)) return .forwardResponseHead(head) + + case .decided(let decidedHandler, let redirectURL): + return self.redirectOrForward( + executor: executor, + requestState: requestState, + redirectHandler: decidedHandler, + redirectURL: redirectURL, + head: head + ) + + case .notApplicable: + if let redirectHandler = redirectHandler, + let redirectURL = redirectHandler.redirectTarget( + status: head.status, + responseHeaders: head.headers + ) + { + return self.redirectOrForward( + executor: executor, + requestState: requestState, + redirectHandler: redirectHandler, + redirectURL: redirectURL, + head: head + ) + } else { + self.state = .executing(executor, requestState, .buffering(.init(), next: .askExecutorForMore)) + return .forwardResponseHead(head) + } } case .redirected: preconditionFailure("This state can only be reached after we have received a HTTP head") @@ -373,6 +408,30 @@ extension RequestBag.StateMachine { } } + /// Shared by `.follow` mode (which has always decided by the time this is reached) and a + /// `.strategy` mode `.follow(_:)` decision (already computed early, carried on + /// `redirectHandler`): reads up to `HTTPClient.maxBodySizeRedirectResponse` of body to try to + /// reuse the connection, or cancels immediately for a known-larger body. + private mutating func redirectOrForward( + executor: HTTPRequestExecutor, + requestState: RequestStreamState, + redirectHandler: RedirectHandler, + redirectURL: URL, + head: HTTPResponseHead + ) -> ReceiveResponseHeadAction { + // If we will redirect, we need to consume the response's body ASAP, to be able to + // reuse the existing connection. We will consume a response body, if the body is + // smaller than 3kb. + switch head.contentLength { + case .some(0...(HTTPClient.maxBodySizeRedirectResponse)), .none: + self.state = .redirected(executor, redirectHandler, 0, head, redirectURL) + return .signalBodyDemand(executor) + case .some: + self.state = .finished(error: HTTPClientError.cancelled) + return .redirect(executor, redirectHandler, head, redirectURL) + } + } + enum ReceiveResponseBodyAction { case none case forwardResponsePart(ByteBuffer) diff --git a/Sources/AsyncHTTPClient/RequestBag.swift b/Sources/AsyncHTTPClient/RequestBag.swift index 1122aa8e0..6d596d5a6 100644 --- a/Sources/AsyncHTTPClient/RequestBag.swift +++ b/Sources/AsyncHTTPClient/RequestBag.swift @@ -284,8 +284,16 @@ final class RequestBag: Sendabl self.delegate.didVisitURL(task: self.task, self.loopBoundState.value.request, head) self.loopBoundState.value.endRequestSpan(response: head) + // Computed here, outside the state machine's exclusive access to its own storage -- + // `earlyStrategyDecision(head:)` runs arbitrary caller code (`redirectDecision(for:)`) + // that may synchronously reenter this task (e.g. `task.cancel()`), which the `mutating + // func receiveResponseHead` call below cannot tolerate overlapping with. + let earlyDecision = + self.loopBoundState.value.state.redirectHandlerAwaitingResponseHead? + .earlyStrategyDecision(head: head) ?? .notApplicable + // runs most likely on channel eventLoop - switch self.loopBoundState.value.state.receiveResponseHead(head) { + switch self.loopBoundState.value.state.receiveResponseHead(head, earlyDecision: earlyDecision) { case .none: break @@ -294,7 +302,7 @@ final class RequestBag: Sendabl case .redirect(let executor, let handler, let head, let newURL): self.loopBoundState.value.redirectTask = handler.redirect( - status: head.status, + head: head, to: newURL, promise: self.task.promise ) @@ -320,7 +328,7 @@ final class RequestBag: Sendabl case .redirect(let executor, let handler, let head, let newURL): self.loopBoundState.value.redirectTask = handler.redirect( - status: head.status, + head: head, to: newURL, promise: self.task.promise ) @@ -359,7 +367,7 @@ final class RequestBag: Sendabl case .redirect(let handler, let head, let newURL): self.loopBoundState.value.redirectTask = handler.redirect( - status: head.status, + head: head, to: newURL, promise: self.task.promise ) diff --git a/Sources/AsyncHTTPClient/TracingSupport.swift b/Sources/AsyncHTTPClient/TracingSupport.swift index ca7cae82c..07396372a 100644 --- a/Sources/AsyncHTTPClient/TracingSupport.swift +++ b/Sources/AsyncHTTPClient/TracingSupport.swift @@ -42,6 +42,8 @@ struct TracingSupport { Int(length) case .byteBuffer(let byteBuffer): byteBuffer.readableBytes + case .delegateBody(let delegateBody): + delegateBody.contentLength.map(Int.init) case .asyncSequence(.unknown, _), .sequence(.unknown, _, _), nil: nil #if UnstableHTTPAPIsSupport diff --git a/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift b/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift index 12d1c0e14..2f01c96bf 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,177 @@ final class AsyncAwaitEndToEndTests: XCTestCase { } } + // MARK: - Pluggable redirect strategies + + 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 { 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(context.redirectRequest.url, "http://localhost:\(port)/ok") + + var rewritten = context.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 { context in + XCTAssertEqual(context.history.count, context.redirectCount + 1) + observedCounts.withLockedValue { $0.append(context.redirectCount) } + guard context.redirectCount < 3 else { + return .doNotFollow + } + return .follow(context.redirectRequest) + } + + let client = HTTPClient(eventLoopGroupProvider: .singleton, configuration: config) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + // /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( + 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) + } + } + + /// 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/HTTPClientTestUtils.swift b/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift index ea80cee6e..770f5e65a 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift @@ -1037,6 +1037,19 @@ internal final class HTTPBinHandler: ChannelInboundHandler { headers.add(name: "Location", value: targetURL) self.resps.append(HTTPResponseBuilder(status: .found, headers: headers)) return + case "/redirect/302-with-body": + // `size` lets a test exercise a redirect-eligible response whose body is + // announced (`Content-Length`) as larger than `HTTPClient + // .maxBodySizeRedirectResponse` (3KB) -- none of the other `/redirect/*` + // endpoints here can produce that, since they're all bodyless. + let size = Int(self.value(for: "size", from: urlComponents.query ?? "")) ?? 4096 + var headers = self.responseHeaders + headers.add(name: "location", value: "/ok") + headers.replaceOrAdd(name: "content-length", value: "\(size)") + var builder = HTTPResponseBuilder(status: .found, headers: headers) + builder.body = ByteBuffer(repeating: UInt8(ascii: "x"), count: size) + self.resps.append(builder) + return case "/percent%20encoded": if req.method != .GET { self.resps.append(HTTPResponseBuilder(status: .methodNotAllowed)) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 83bbaf3ae..5d00b3ad4 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -448,6 +448,126 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { XCTAssertEqual("1234", data.data) } + func testCustomRedirectConfigurationDoesNotAffectNonRedirectingRequestsOverDelegateBasedExecute() throws { + // `.custom`/`.strategy` only kick in once the delegate-based `execute(request:delegate:...)` + // API (exercised here via `.get`) actually encounters a redirect-eligible response -- + // configuring one doesn't reject every request outright, matching the Concurrency + // `execute(_:deadline:logger:)` API, which never consults its redirect mode for a request + // that never redirects either. + let localClient = HTTPClient( + eventLoopGroupProvider: .shared(self.clientGroup), + configuration: HTTPClient.Configuration( + redirectConfiguration: .custom { _ in .doNotFollow } + ) + ) + defer { XCTAssertNoThrow(try localClient.syncShutdown()) } + + let response = try localClient.get(url: self.defaultHTTPBinURLPrefix + "ok").wait() + XCTAssertEqual(response.status, .ok) + } + + func testCustomRedirectHandlerCanRefuseRedirectOverDelegateBasedExecute() throws { + // `.doNotFollow` is decided the moment the response head arrives + // (`RedirectHandler.earlyStrategyDecision(head:)`), before any byte of its body has been + // read -- so the 302 itself becomes the final response, delivered exactly like any + // ordinary (non-redirect-candidate) response, same as the Concurrency API. + let localClient = HTTPClient( + eventLoopGroupProvider: .shared(self.clientGroup), + configuration: HTTPClient.Configuration( + redirectConfiguration: .custom { _ in .doNotFollow } + ) + ) + defer { XCTAssertNoThrow(try localClient.syncShutdown()) } + + let response = try localClient.get(url: "http://localhost:\(self.defaultHTTPBin.port)/redirect/302").wait() + + XCTAssertEqual(response.status, .found) + XCTAssertEqual(response.history.count, 1) + } + + func testCustomRedirectHandlerCanRewriteRedirectRequestOverDelegateBasedExecute() throws { + let port = self.defaultHTTPBin.port + let localClient = HTTPClient( + eventLoopGroupProvider: .shared(self.clientGroup), + configuration: HTTPClient.Configuration( + 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(context.redirectRequest.url, "http://localhost:\(port)/ok") + + var rewritten = context.redirectRequest + rewritten.url = "http://localhost:\(port)/echo-uri" + return .follow(rewritten) + } + ) + ) + defer { XCTAssertNoThrow(try localClient.syncShutdown()) } + + let response = try localClient.get(url: "http://localhost:\(port)/redirect/302").wait() + + XCTAssertEqual(response.status, .ok) + XCTAssertEqual(response.headers.first(name: "X-Calling-URI"), "/echo-uri") + XCTAssertEqual( + response.history.map(\.request.url.absoluteString), + ["http://localhost:\(port)/redirect/302", "http://localhost:\(port)/echo-uri"] + ) + } + + func testCustomRedirectHandlerCanRefuseRedirectWithLargeBodyOverDelegateBasedExecute() throws { + // End-to-end version of `RequestBagTests + // .testRedirectStrategyDoesNotFollowWithLargeAnnouncedBodyDeliversResponseNormally`: + // `/redirect/302-with-body` announces a body bigger than `HTTPClient + // .maxBodySizeRedirectResponse` (3KB), which used to be cancelled before any byte was + // read, before the strategy even got a chance to decide. + let localClient = HTTPClient( + eventLoopGroupProvider: .shared(self.clientGroup), + configuration: HTTPClient.Configuration( + redirectConfiguration: .custom { _ in .doNotFollow } + ) + ) + defer { XCTAssertNoThrow(try localClient.syncShutdown()) } + + let response = try localClient.get( + url: "http://localhost:\(self.defaultHTTPBin.port)/redirect/302-with-body?size=8192" + ).wait() + + XCTAssertEqual(response.status, .found) + XCTAssertEqual(response.history.count, 1) + XCTAssertEqual(response.body?.readableBytes, 8192) + } + + func testCustomRedirectHandlerOverDelegateBasedExecuteInvokesStrategyExactlyOncePerRedirect() throws { + // The end-to-end guard against `earlyStrategyDecision(head:)` and the later + // `redirect(head:to:promise:)` both invoking the strategy for the same redirect: if + // `precomputed` weren't threaded through correctly, this would observe duplicate/repeated + // counts instead of a clean [0, 1, 2]. + let port = self.defaultHTTPBin.port + let observedCounts = NIOLockedValueBox<[Int]>([]) + let localClient = HTTPClient( + eventLoopGroupProvider: .shared(self.clientGroup), + configuration: HTTPClient.Configuration( + redirectConfiguration: .custom { context in + observedCounts.withLockedValue { $0.append(context.redirectCount) } + guard context.redirectCount < 2 else { + return .doNotFollow + } + return .follow(context.redirectRequest) + } + ) + ) + defer { XCTAssertNoThrow(try localClient.syncShutdown()) } + + let response = try localClient.get( + url: "http://localhost:\(port)/redirect/infinite1" + ).wait() + + XCTAssertEqual(response.status, .found) + XCTAssertEqual(observedCounts.withLockedValue { $0 }, [0, 1, 2]) + } + func testHttpRedirect() throws { let httpsBin = HTTPBin(.http1_1(ssl: true)) let localClient = HTTPClient( diff --git a/Tests/AsyncHTTPClientTests/RequestBagTests.swift b/Tests/AsyncHTTPClientTests/RequestBagTests.swift index 297e81704..3061cd042 100644 --- a/Tests/AsyncHTTPClientTests/RequestBagTests.swift +++ b/Tests/AsyncHTTPClientTests/RequestBagTests.swift @@ -964,6 +964,161 @@ final class RequestBagTests: XCTestCase { XCTAssertTrue(redirectTriggered) } + /// A `.strategy` redirect configuration deciding `.doNotFollow` for a redirect-eligible + /// response whose body is announced (`Content-Length`) as larger than `HTTPClient + /// .maxBodySizeRedirectResponse` (3KB) -- the case that, before `earlyStrategyDecision(head:)` + /// existed, was cancelled before any byte of the body was ever read, since the state machine + /// used to decide "redirect or not" only by size, not by asking the strategy first. Now the + /// strategy is asked immediately at head time, before touching the body at all, so + /// `.doNotFollow` delivers this response exactly like `testRaceBetweenConnectionCloseAndDemandMoreData`'s + /// plain, non-redirect one -- regardless of size. + @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) + func testRedirectStrategyDoesNotFollowWithLargeAnnouncedBodyDeliversResponseNormally() { + let embeddedEventLoop = EmbeddedEventLoop() + defer { XCTAssertNoThrow(try embeddedEventLoop.syncShutdownGracefully()) } + let logger = Logger(label: "test") + + var maybeRequest: HTTPClient.Request? + XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "https://swift.org")) + guard let request = maybeRequest else { return XCTFail("Expected to have a request") } + + struct RefusingStrategy: HTTPClientRedirectStrategy { + func redirectDecision(for context: HTTPClientRedirectContext) throws -> HTTPClientRedirectDecision { + .doNotFollow + } + } + + let delegate = UploadCountingDelegate(eventLoop: embeddedEventLoop) + var maybeRequestBag: RequestBag? + XCTAssertNoThrow( + maybeRequestBag = try RequestBag( + request: request, + eventLoopPreference: .delegate(on: embeddedEventLoop), + task: .init(eventLoop: embeddedEventLoop, logger: logger), + redirectHandler: .init( + request: request, + redirectState: RedirectState( + .strategy(RefusingStrategy()), + initialURL: request.url.absoluteString + )!, + execute: { _, _ in + XCTFail("`.doNotFollow` must not redirect") + return HTTPClient.Task( + eventLoop: embeddedEventLoop, + logger: logger + ) + } + ), + connectionDeadline: .now() + .seconds(30), + requestOptions: .forTests(), + delegate: delegate + ) + ) + guard let bag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag.") } + + let executor = MockRequestExecutor(eventLoop: embeddedEventLoop) + executor.runRequest(bag) + + let responseHead = HTTPResponseHead( + version: .http1_1, + status: .found, + headers: ["content-length": "\(4 * 1024)", "location": "https://swift.org/sswg"] + ) + bag.receiveResponseHead(responseHead) + XCTAssertFalse(executor.isCancelled) + XCTAssertFalse(executor.signalledDemandForResponseBody) + XCTAssertNoThrow(try XCTUnwrap(delegate.backpressurePromise).succeed(())) + XCTAssertTrue(executor.signalledDemandForResponseBody) + executor.resetResponseStreamDemandSignal() + + XCTAssertEqual(delegate.hitDidReceiveBodyPart, 0) + bag.receiveResponseBodyParts([ByteBuffer(repeating: 0, count: 4 * 1024)]) + XCTAssertFalse(executor.isCancelled) + XCTAssertEqual(delegate.hitDidReceiveBodyPart, 1) + XCTAssertNoThrow(try XCTUnwrap(delegate.backpressurePromise).succeed(())) + executor.resetResponseStreamDemandSignal() + + bag.receiveResponseEnd([], trailers: nil) + XCTAssertEqual(delegate.hitDidReceiveResponse, 1) + + XCTAssertFalse(executor.isCancelled) + XCTAssertEqual(delegate.receivedHead?.status, .found) + } + + /// `strategy.redirectDecision(for:)` is arbitrary caller code, and a strategy that + /// synchronously reaches back into its own task (e.g. to abort rather than redirect) is a + /// realistic thing to write. `earlyStrategyDecision(head:)` must be safe to call this way -- + /// specifically, it must be computed *before* `RequestBag`'s exclusive access to its own + /// state machine storage is taken, or a reentrant `task.cancel()` from inside the strategy + /// traps under Swift's exclusivity enforcement instead of just failing the task. + @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) + func testRedirectStrategyReentrantlyCancellingTheTaskDoesNotTrap() { + let embeddedEventLoop = EmbeddedEventLoop() + defer { XCTAssertNoThrow(try embeddedEventLoop.syncShutdownGracefully()) } + let logger = Logger(label: "test") + + var maybeRequest: HTTPClient.Request? + XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "https://swift.org")) + guard let request = maybeRequest else { return XCTFail("Expected to have a request") } + + let task = HTTPClient.Task(eventLoop: embeddedEventLoop, logger: logger) + + struct SelfCancellingStrategy: HTTPClientRedirectStrategy { + let task: HTTPClient.Task + + func redirectDecision(for context: HTTPClientRedirectContext) throws -> HTTPClientRedirectDecision { + self.task.cancel() + return .doNotFollow + } + } + + let delegate = UploadCountingDelegate(eventLoop: embeddedEventLoop) + var maybeRequestBag: RequestBag? + XCTAssertNoThrow( + maybeRequestBag = try RequestBag( + request: request, + eventLoopPreference: .delegate(on: embeddedEventLoop), + task: task, + redirectHandler: .init( + request: request, + redirectState: RedirectState( + .strategy(SelfCancellingStrategy(task: task)), + initialURL: request.url.absoluteString + )!, + execute: { _, _ in + XCTFail("Should not redirect") + return HTTPClient.Task( + eventLoop: embeddedEventLoop, + logger: logger + ) + } + ), + connectionDeadline: .now() + .seconds(30), + requestOptions: .forTests(), + delegate: delegate + ) + ) + guard let bag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag.") } + + let executor = MockRequestExecutor(eventLoop: embeddedEventLoop) + executor.runRequest(bag) + + // Must not trap. The strategy cancels `task` from inside `redirectDecision(for:)`, which + // reenters this same task while `earlyStrategyDecision(head:)` is being computed -- + // proving it runs outside the state machine's own exclusive access. + bag.receiveResponseHead( + .init( + version: .http1_1, + status: .found, + headers: ["content-length": "\(4 * 1024)", "location": "https://swift.org/sswg"] + ) + ) + + XCTAssertThrowsError(try task.futureResult.wait()) { + XCTAssertEqual($0 as? HTTPClientError, .cancelled) + } + } + func testWeDontLeakTheRequestIfTheRequestWriterWasCapturedByAPromise() { final class LeakDetector: Sendable {} diff --git a/Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift b/Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift index e80d49ed8..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: + case .disallow, .strategy: Issue.record("Unexpected value") } @@ -130,7 +130,7 @@ struct HTTPClientConfigurationPropsTests { switch config.redirectConfiguration.mode { case .disallow: break - case .follow: + case .follow, .strategy: Issue.record("Unexpected value") } }