Skip to content

Commit 51dc885

Browse files
artemredkinartemredkin
authored
add support for redirect limits (#113)
1 parent c1c3da3 commit 51dc885

File tree

5 files changed

+128
-11
lines changed

5 files changed

+128
-11
lines changed

Diff for: ‎Sources/AsyncHTTPClient/HTTPClient.swift

+50-7
Original file line numberDiff line numberDiff line change
@@ -227,15 +227,20 @@ public class HTTPClient {
227227
channelEL: EventLoop? = nil,
228228
deadline: NIODeadline? = nil) -> Task<Delegate.Response> {
229229
let redirectHandler: RedirectHandler<Delegate.Response>?
230-
if self.configuration.followRedirects {
230+
switch self.configuration.redirectConfiguration.configuration {
231+
case .follow(let max, let allowCycles):
232+
var request = request
233+
if request.redirectState == nil {
234+
request.redirectState = .init(count: max, visited: allowCycles ? nil : Set())
235+
}
231236
redirectHandler = RedirectHandler<Delegate.Response>(request: request) { newRequest in
232237
self.execute(request: newRequest,
233238
delegate: delegate,
234239
eventLoop: delegateEL,
235240
channelEL: channelEL,
236241
deadline: deadline)
237242
}
238-
} else {
243+
case .disallow:
239244
redirectHandler = nil
240245
}
241246

@@ -325,7 +330,7 @@ public class HTTPClient {
325330
/// - `305: Use Proxy`
326331
/// - `307: Temporary Redirect`
327332
/// - `308: Permanent Redirect`
328-
public var followRedirects: Bool
333+
public var redirectConfiguration: RedirectConfiguration
329334
/// Default client timeout, defaults to no timeouts.
330335
public var timeout: Timeout
331336
/// Upstream proxy, defaults to no proxy.
@@ -336,27 +341,27 @@ public class HTTPClient {
336341
public var ignoreUncleanSSLShutdown: Bool
337342

338343
public init(tlsConfiguration: TLSConfiguration? = nil,
339-
followRedirects: Bool = false,
344+
redirectConfiguration: RedirectConfiguration? = nil,
340345
timeout: Timeout = Timeout(),
341346
proxy: Proxy? = nil,
342347
ignoreUncleanSSLShutdown: Bool = false,
343348
decompression: Decompression = .disabled) {
344349
self.tlsConfiguration = tlsConfiguration
345-
self.followRedirects = followRedirects
350+
self.redirectConfiguration = redirectConfiguration ?? RedirectConfiguration()
346351
self.timeout = timeout
347352
self.proxy = proxy
348353
self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown
349354
self.decompression = decompression
350355
}
351356

352357
public init(certificateVerification: CertificateVerification,
353-
followRedirects: Bool = false,
358+
redirectConfiguration: RedirectConfiguration? = nil,
354359
timeout: Timeout = Timeout(),
355360
proxy: Proxy? = nil,
356361
ignoreUncleanSSLShutdown: Bool = false,
357362
decompression: Decompression = .disabled) {
358363
self.tlsConfiguration = TLSConfiguration.forClient(certificateVerification: certificateVerification)
359-
self.followRedirects = followRedirects
364+
self.redirectConfiguration = redirectConfiguration ?? RedirectConfiguration()
360365
self.timeout = timeout
361366
self.proxy = proxy
362367
self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown
@@ -439,6 +444,38 @@ extension HTTPClient.Configuration {
439444
self.read = read
440445
}
441446
}
447+
448+
/// Specifies redirect processing settings.
449+
public struct RedirectConfiguration {
450+
enum Configuration {
451+
/// Redirects are not followed.
452+
case disallow
453+
/// Redirects are followed with a specified limit.
454+
case follow(max: Int, allowCycles: Bool)
455+
}
456+
457+
var configuration: Configuration
458+
459+
init() {
460+
self.configuration = .follow(max: 5, allowCycles: false)
461+
}
462+
463+
init(configuration: Configuration) {
464+
self.configuration = configuration
465+
}
466+
467+
/// Redirects are not followed.
468+
public static let disallow = RedirectConfiguration(configuration: .disallow)
469+
470+
/// Redirects are followed with a specified limit.
471+
///
472+
/// - parameters:
473+
/// - max: The maximum number of allowed redirects.
474+
/// - allowCycles: Whether cycles are allowed.
475+
///
476+
/// - warning: Cycle detection will keep all visited URLs in memory which means a malicious server could use this as a denial-of-service vector.
477+
public static func follow(max: Int, allowCycles: Bool) -> RedirectConfiguration { return .init(configuration: .follow(max: max, allowCycles: allowCycles)) }
478+
}
442479
}
443480

444481
private extension ChannelPipeline {
@@ -488,6 +525,8 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible {
488525
case invalidProxyResponse
489526
case contentLengthMissing
490527
case proxyAuthenticationRequired
528+
case redirectLimitReached
529+
case redirectCycleDetected
491530
}
492531

493532
private var code: Code
@@ -526,4 +565,8 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible {
526565
public static let contentLengthMissing = HTTPClientError(code: .contentLengthMissing)
527566
/// Proxy Authentication Required.
528567
public static let proxyAuthenticationRequired = HTTPClientError(code: .proxyAuthenticationRequired)
568+
/// Redirect Limit reached.
569+
public static let redirectLimitReached = HTTPClientError(code: .redirectLimitReached)
570+
/// Redirect Cycle detected.
571+
public static let redirectCycleDetected = HTTPClientError(code: .redirectCycleDetected)
529572
}

Diff for: ‎Sources/AsyncHTTPClient/HTTPHandler.swift

+31-1
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,13 @@ extension HTTPClient {
100100
/// Request body, defaults to no body.
101101
public var body: Body?
102102

103+
struct RedirectState {
104+
var count: Int
105+
var visited: Set<URL>?
106+
}
107+
108+
var redirectState: RedirectState?
109+
103110
/// Create HTTP request.
104111
///
105112
/// - parameters:
@@ -152,6 +159,8 @@ extension HTTPClient {
152159
self.host = host
153160
self.headers = headers
154161
self.body = body
162+
163+
self.redirectState = nil
155164
}
156165

157166
/// Whether request will be executed using secure socket.
@@ -813,6 +822,26 @@ internal struct RedirectHandler<ResponseType> {
813822
}
814823

815824
func redirect(status: HTTPResponseStatus, to redirectURL: URL, promise: EventLoopPromise<ResponseType>) {
825+
var nextState: HTTPClient.Request.RedirectState?
826+
if var state = request.redirectState {
827+
guard state.count > 0 else {
828+
return promise.fail(HTTPClientError.redirectLimitReached)
829+
}
830+
831+
state.count -= 1
832+
833+
if var visited = state.visited {
834+
guard !visited.contains(redirectURL) else {
835+
return promise.fail(HTTPClientError.redirectCycleDetected)
836+
}
837+
838+
visited.insert(redirectURL)
839+
state.visited = visited
840+
}
841+
842+
nextState = state
843+
}
844+
816845
let originalRequest = self.request
817846

818847
var convertToGet = false
@@ -841,7 +870,8 @@ internal struct RedirectHandler<ResponseType> {
841870
}
842871

843872
do {
844-
let newRequest = try HTTPClient.Request(url: redirectURL, method: method, headers: headers, body: body)
873+
var newRequest = try HTTPClient.Request(url: redirectURL, method: method, headers: headers, body: body)
874+
newRequest.redirectState = nextState
845875
return self.execute(newRequest).futureResult.cascade(to: promise)
846876
} catch {
847877
return promise.fail(error)

Diff for: ‎Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift

+10
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,16 @@ internal final class HttpBinHandler: ChannelInboundHandler {
284284
headers.add(name: "Location", value: "http://127.0.0.1:\(port)/echohostheader")
285285
self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
286286
return
287+
case "/redirect/infinite1":
288+
var headers = HTTPHeaders()
289+
headers.add(name: "Location", value: "/redirect/infinite2")
290+
self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
291+
return
292+
case "/redirect/infinite2":
293+
var headers = HTTPHeaders()
294+
headers.add(name: "Location", value: "/redirect/infinite1")
295+
self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
296+
return
287297
// Since this String is taken from URL.path, the percent encoding has been removed
288298
case "/percent encoded":
289299
if req.method != .GET {

Diff for: ‎Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift

+2
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ extension HTTPClientTests {
5959
("testEventLoopArgument", testEventLoopArgument),
6060
("testDecompression", testDecompression),
6161
("testDecompressionLimit", testDecompressionLimit),
62+
("testLoopDetectionRedirectLimit", testLoopDetectionRedirectLimit),
63+
("testCountRedirectLimit", testCountRedirectLimit),
6264
]
6365
}
6466
}

Diff for: ‎Tests/AsyncHTTPClientTests/HTTPClientTests.swift

+35-3
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ class HTTPClientTests: XCTestCase {
131131
let httpBin = HTTPBin(ssl: false)
132132
let httpsBin = HTTPBin(ssl: true)
133133
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
134-
configuration: HTTPClient.Configuration(certificateVerification: .none, followRedirects: true))
134+
configuration: HTTPClient.Configuration(certificateVerification: .none, redirectConfiguration: .follow(max: 10, allowCycles: true)))
135135

136136
defer {
137137
XCTAssertNoThrow(try httpClient.syncShutdown())
@@ -149,7 +149,7 @@ class HTTPClientTests: XCTestCase {
149149
func testHttpHostRedirect() throws {
150150
let httpBin = HTTPBin(ssl: false)
151151
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
152-
configuration: HTTPClient.Configuration(certificateVerification: .none, followRedirects: true))
152+
configuration: HTTPClient.Configuration(certificateVerification: .none, redirectConfiguration: .follow(max: 10, allowCycles: true)))
153153

154154
defer {
155155
XCTAssertNoThrow(try httpClient.syncShutdown())
@@ -526,7 +526,7 @@ class HTTPClientTests: XCTestCase {
526526
let httpBin = HTTPBin()
527527
let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 5)
528528
let httpClient = HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup),
529-
configuration: HTTPClient.Configuration(followRedirects: true))
529+
configuration: HTTPClient.Configuration(redirectConfiguration: .follow(max: 10, allowCycles: true)))
530530
defer {
531531
XCTAssertNoThrow(try httpClient.syncShutdown())
532532
XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully())
@@ -568,6 +568,7 @@ class HTTPClientTests: XCTestCase {
568568
func testDecompression() throws {
569569
let httpBin = HTTPBin(compress: true)
570570
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew, configuration: .init(decompression: .enabled(limit: .none)))
571+
571572
defer {
572573
XCTAssertNoThrow(try httpClient.syncShutdown())
573574
XCTAssertNoThrow(try httpBin.shutdown())
@@ -603,6 +604,7 @@ class HTTPClientTests: XCTestCase {
603604
func testDecompressionLimit() throws {
604605
let httpBin = HTTPBin(compress: true)
605606
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew, configuration: .init(decompression: .enabled(limit: .ratio(10))))
607+
606608
defer {
607609
XCTAssertNoThrow(try httpClient.syncShutdown())
608610
XCTAssertNoThrow(try httpBin.shutdown())
@@ -626,4 +628,34 @@ class HTTPClientTests: XCTestCase {
626628
XCTFail("Unexptected error: \(error)")
627629
}
628630
}
631+
632+
func testLoopDetectionRedirectLimit() throws {
633+
let httpBin = HTTPBin(ssl: true)
634+
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
635+
configuration: HTTPClient.Configuration(certificateVerification: .none, redirectConfiguration: .follow(max: 5, allowCycles: false)))
636+
637+
defer {
638+
XCTAssertNoThrow(try httpClient.syncShutdown())
639+
XCTAssertNoThrow(try httpBin.shutdown())
640+
}
641+
642+
XCTAssertThrowsError(try httpClient.get(url: "https://localhost:\(httpBin.port)/redirect/infinite1").wait(), "Should fail with redirect limit") { error in
643+
XCTAssertEqual(error as! HTTPClientError, HTTPClientError.redirectCycleDetected)
644+
}
645+
}
646+
647+
func testCountRedirectLimit() throws {
648+
let httpBin = HTTPBin(ssl: true)
649+
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
650+
configuration: HTTPClient.Configuration(certificateVerification: .none, redirectConfiguration: .follow(max: 5, allowCycles: true)))
651+
652+
defer {
653+
XCTAssertNoThrow(try httpClient.syncShutdown())
654+
XCTAssertNoThrow(try httpBin.shutdown())
655+
}
656+
657+
XCTAssertThrowsError(try httpClient.get(url: "https://localhost:\(httpBin.port)/redirect/infinite1").wait(), "Should fail with redirect limit") { error in
658+
XCTAssertEqual(error as! HTTPClientError, HTTPClientError.redirectLimitReached)
659+
}
660+
}
629661
}

0 commit comments

Comments
 (0)