Skip to content

Commit

Permalink
Refactor redirect logic to be reusable for async/await (#522)
Browse files Browse the repository at this point in the history 
* refactor RedirectHandler
- `redirectState` is no longer a property of `HTTPClient.Request`. RedirectHandler now stores this state directly and therefore no longer optional.
- we no longer count the number of allowed redirects down. Instead the number of redirects is dervied from `self.visited.count` and we compare it to the maxRedirect to check if we git the limit.

* `HTTPClient.Configuration.RedirectConfiguration.Configuration` is now called `HTTPClient.Configuration.RedirectConfiguration.Mode`
only two `Configuration`s left in the type name

* add redirect logger test
  • Loading branch information
@dnadoba
dnadoba authored Dec 8, 2021
1 parent 3d7c42e commit c4feafd
Show file tree
Hide file tree
Showing 6 changed files with 312 additions and 113 deletions.
80 changes: 55 additions & 25 deletions Sources/AsyncHTTPClient/HTTPClient.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -504,11 +504,43 @@ public class HTTPClient {
/// - delegate: Delegate to process response parts.
/// - eventLoop: NIO Event Loop preference.
/// - deadline: Point in time by which the request must complete.
public func execute<Delegate: HTTPClientResponseDelegate>(request: Request,
delegate: Delegate,
eventLoop eventLoopPreference: EventLoopPreference,
deadline: NIODeadline? = nil,
logger originalLogger: Logger?) -> Task<Delegate.Response> {
/// - logger: The logger to use for this request.
public func execute<Delegate: HTTPClientResponseDelegate>(
request: Request,
delegate: Delegate,
eventLoop eventLoopPreference: EventLoopPreference,
deadline: NIODeadline? = nil,
logger originalLogger: Logger?
) -> Task<Delegate.Response> {
self._execute(
request: request,
delegate: delegate,
eventLoop: eventLoopPreference,
deadline: deadline,
logger: originalLogger,
redirectState: RedirectState(
self.configuration.redirectConfiguration.mode,
initialURL: request.url.absoluteString
)
)
}

/// Execute arbitrary HTTP request and handle response processing using provided delegate.
///
/// - parameters:
/// - request: HTTP request to execute.
/// - delegate: Delegate to process response parts.
/// - eventLoop: NIO Event Loop preference.
/// - deadline: Point in time by which the request must complete.
/// - logger: The logger to use for this request.
func _execute<Delegate: HTTPClientResponseDelegate>(
request: Request,
delegate: Delegate,
eventLoop eventLoopPreference: EventLoopPreference,
deadline: NIODeadline? = nil,
logger originalLogger: Logger?,
redirectState: RedirectState?
) -> Task<Delegate.Response> {
let logger = (originalLogger ?? HTTPClient.loggingDisabled).attachingRequestInformation(request, requestID: globalRequestID.add(1))
let taskEL: EventLoop
switch eventLoopPreference.preference {
Expand Down Expand Up @@ -543,22 +575,20 @@ public class HTTPClient {
return failedTask
}

let redirectHandler: RedirectHandler<Delegate.Response>?
switch self.configuration.redirectConfiguration.configuration {
case .follow(let max, let allowCycles):
var request = request
if request.redirectState == nil {
request.redirectState = .init(count: max, visited: allowCycles ? nil : Set())
let redirectHandler: RedirectHandler<Delegate.Response>? = {
guard let redirectState = redirectState else { return nil }

return .init(request: request, redirectState: redirectState) { newRequest, newRedirectState in
self._execute(
request: newRequest,
delegate: delegate,
eventLoop: eventLoopPreference,
deadline: deadline,
logger: logger,
redirectState: newRedirectState
)
}
redirectHandler = RedirectHandler<Delegate.Response>(request: request) { newRequest in
self.execute(request: newRequest,
delegate: delegate,
eventLoop: eventLoopPreference,
deadline: deadline)
}
case .disallow:
redirectHandler = nil
}
}()

let task = Task<Delegate.Response>(eventLoop: taskEL, logger: logger)
do {
Expand Down Expand Up @@ -804,21 +834,21 @@ extension HTTPClient.Configuration {

/// Specifies redirect processing settings.
public struct RedirectConfiguration {
enum Configuration {
enum Mode {
/// Redirects are not followed.
case disallow
/// Redirects are followed with a specified limit.
case follow(max: Int, allowCycles: Bool)
}

var configuration: Configuration
var mode: Mode

init() {
self.configuration = .follow(max: 5, allowCycles: false)
self.mode = .follow(max: 5, allowCycles: false)
}

init(configuration: Configuration) {
self.configuration = configuration
init(configuration: Mode) {
self.mode = configuration
}

/// Redirects are not followed.
Expand Down
119 changes: 33 additions & 86 deletions Sources/AsyncHTTPClient/HTTPHandler.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,9 @@ extension HTTPClient {
/// Request-specific TLS configuration, defaults to no request-specific TLS configuration.
public var tlsConfiguration: TLSConfiguration?

struct RedirectState {
var count: Int
var visited: Set<URL>?
}

/// Parsed, validated and deconstructed URL.
let deconstructedURL: DeconstructedURL

var redirectState: RedirectState?

/// Create HTTP request.
///
/// - parameters:
Expand Down Expand Up @@ -190,7 +183,6 @@ extension HTTPClient {
public init(url: URL, method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil, tlsConfiguration: TLSConfiguration?) throws {
self.deconstructedURL = try DeconstructedURL(url: url)

self.redirectState = nil
self.url = url
self.method = method
self.headers = headers
Expand Down Expand Up @@ -642,87 +634,42 @@ internal struct TaskCancelEvent {}

internal struct RedirectHandler<ResponseType> {
let request: HTTPClient.Request
let execute: (HTTPClient.Request) -> HTTPClient.Task<ResponseType>

func redirectTarget(status: HTTPResponseStatus, headers: HTTPHeaders) -> URL? {
switch status {
case .movedPermanently, .found, .seeOther, .notModified, .useProxy, .temporaryRedirect, .permanentRedirect:
break
default:
return nil
}

guard let location = headers.first(name: "Location") else {
return nil
}

guard let url = URL(string: location, relativeTo: request.url) else {
return nil
}

guard self.request.deconstructedURL.scheme.supportsRedirects(to: url.scheme) else {
return nil
}

if url.isFileURL {
return nil
}

return url.absoluteURL
let redirectState: RedirectState
let execute: (HTTPClient.Request, RedirectState) -> HTTPClient.Task<ResponseType>

func redirectTarget(status: HTTPResponseStatus, responseHeaders: HTTPHeaders) -> URL? {
responseHeaders.extractRedirectTarget(
status: status,
originalURL: self.request.url,
originalScheme: self.request.deconstructedURL.scheme
)
}

func redirect(status: HTTPResponseStatus, to redirectURL: URL, promise: EventLoopPromise<ResponseType>) {
var nextState: HTTPClient.Request.RedirectState?
if var state = request.redirectState {
guard state.count > 0 else {
return promise.fail(HTTPClientError.redirectLimitReached)
}

state.count -= 1

if var visited = state.visited {
guard !visited.contains(redirectURL) else {
return promise.fail(HTTPClientError.redirectCycleDetected)
}

visited.insert(redirectURL)
state.visited = visited
}

nextState = state
}

let originalRequest = self.request

var convertToGet = false
if status == .seeOther, self.request.method != .HEAD {
convertToGet = true
} else if status == .movedPermanently || status == .found, self.request.method == .POST {
convertToGet = true
}

var method = originalRequest.method
var headers = originalRequest.headers
var body = originalRequest.body

if convertToGet {
method = .GET
body = nil
headers.remove(name: "Content-Length")
headers.remove(name: "Content-Type")
}

if !originalRequest.url.hasTheSameOrigin(as: redirectURL) {
headers.remove(name: "Origin")
headers.remove(name: "Cookie")
headers.remove(name: "Authorization")
headers.remove(name: "Proxy-Authorization")
}

func redirect(
status: HTTPResponseStatus,
to redirectURL: URL,
promise: EventLoopPromise<ResponseType>
) {
do {
var newRequest = try HTTPClient.Request(url: redirectURL, method: method, headers: headers, body: body)
newRequest.redirectState = nextState
self.execute(newRequest).futureResult.whenComplete { result in
var redirectState = self.redirectState
try redirectState.redirect(to: redirectURL.absoluteString)

let (method, headers, body) = transformRequestForRedirect(
from: request.url,
method: self.request.method,
headers: self.request.headers,
body: self.request.body,
to: redirectURL,
status: status
)

let newRequest = try HTTPClient.Request(
url: redirectURL,
method: method,
headers: headers,
body: body
)
self.execute(newRequest, redirectState).futureResult.whenComplete { result in
promise.futureResult.eventLoop.execute {
promise.completeWith(result)
}
Expand Down
Loading

0 comments on commit c4feafd

Please sign in to comment.