add support for redirect limits (swift-server#113)

Author: artemredkin

Sources/AsyncHTTPClient/HTTPClient.swift

@@ -227,15 +227,20 @@ public class HTTPClient{
  channelEL:EventLoop?=nil,
  deadline:NIODeadline?=nil)->Task<Delegate.Response>{
 letredirectHandler:RedirectHandler<Delegate.Response>?
-ifself.configuration.followRedirects {
+switchself.configuration.redirectConfiguration.configuration {
+case.follow(let max,let allowCycles):
+varrequest= request
+if request.redirectState ==nil{
+ request.redirectState =.init(count: max, visited: allowCycles ?nil:Set())
+}
  redirectHandler =RedirectHandler<Delegate.Response>(request: request){ newRequest in
 self.execute(request: newRequest,
  delegate: delegate,
  eventLoop: delegateEL,
  channelEL: channelEL,
  deadline: deadline)
 }
-}else{
+case.disallow:
  redirectHandler =nil
 }
 
@@ -325,7 +330,7 @@ public class HTTPClient{
  /// - `305: Use Proxy`
  /// - `307: Temporary Redirect`
  /// - `308: Permanent Redirect`
-publicvarfollowRedirects:Bool
+publicvarredirectConfiguration:RedirectConfiguration
  /// Default client timeout, defaults to no timeouts.
 publicvartimeout:Timeout
  /// Upstream proxy, defaults to no proxy.
@@ -336,27 +341,27 @@ public class HTTPClient{
 publicvarignoreUncleanSSLShutdown:Bool
 
 publicinit(tlsConfiguration:TLSConfiguration?=nil,
-followRedirects:Bool=false,
+redirectConfiguration:RedirectConfiguration?=nil,
  timeout:Timeout=Timeout(),
  proxy:Proxy?=nil,
  ignoreUncleanSSLShutdown:Bool=false,
  decompression:Decompression=.disabled){
 self.tlsConfiguration = tlsConfiguration
-self.followRedirects=followRedirects
+self.redirectConfiguration=redirectConfiguration ??RedirectConfiguration()
 self.timeout = timeout
 self.proxy = proxy
 self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown
 self.decompression = decompression
 }
 
 publicinit(certificateVerification:CertificateVerification,
-followRedirects:Bool=false,
+redirectConfiguration:RedirectConfiguration?=nil,
  timeout:Timeout=Timeout(),
  proxy:Proxy?=nil,
  ignoreUncleanSSLShutdown:Bool=false,
  decompression:Decompression=.disabled){
 self.tlsConfiguration =TLSConfiguration.forClient(certificateVerification: certificateVerification)
-self.followRedirects=followRedirects
+self.redirectConfiguration=redirectConfiguration ??RedirectConfiguration()
 self.timeout = timeout
 self.proxy = proxy
 self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown
@@ -439,6 +444,38 @@ extension HTTPClient.Configuration{
 self.read = read
 }
 }
+
+ /// Specifies redirect processing settings.
+publicstructRedirectConfiguration{
+enumConfiguration{
+ /// Redirects are not followed.
+case disallow
+ /// Redirects are followed with a specified limit.
+case follow(max:Int, allowCycles:Bool)
+}
+
+varconfiguration:Configuration
+
+init(){
+self.configuration =.follow(max:5, allowCycles:false)
+}
+
+init(configuration:Configuration){
+self.configuration = configuration
+}
+
+ /// Redirects are not followed.
+publicstaticletdisallow=RedirectConfiguration(configuration:.disallow)
+
+ /// Redirects are followed with a specified limit.
+ ///
+ /// - parameters:
+ /// - max: The maximum number of allowed redirects.
+ /// - allowCycles: Whether cycles are allowed.
+ ///
+ /// - warning: Cycle detection will keep all visited URLs in memory which means a malicious server could use this as a denial-of-service vector.
+publicstaticfunc follow(max:Int, allowCycles:Bool)->RedirectConfiguration{return.init(configuration:.follow(max: max, allowCycles: allowCycles))}
+}
 }
 
 privateextensionChannelPipeline{
@@ -488,6 +525,8 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible{
 case invalidProxyResponse
 case contentLengthMissing
 case proxyAuthenticationRequired
+case redirectLimitReached
+case redirectCycleDetected
 }
 
 privatevarcode:Code
@@ -526,4 +565,8 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible{
 publicstaticletcontentLengthMissing=HTTPClientError(code:.contentLengthMissing)
  /// Proxy Authentication Required.
 publicstaticletproxyAuthenticationRequired=HTTPClientError(code:.proxyAuthenticationRequired)
+ /// Redirect Limit reached.
+publicstaticletredirectLimitReached=HTTPClientError(code:.redirectLimitReached)
+ /// Redirect Cycle detected.
+publicstaticletredirectCycleDetected=HTTPClientError(code:.redirectCycleDetected)
 }

Sources/AsyncHTTPClient/HTTPHandler.swift

@@ -100,6 +100,13 @@ extension HTTPClient{
  /// Request body, defaults to no body.
 publicvarbody:Body?
 
+structRedirectState{
+varcount:Int
+varvisited:Set<URL>?
+}
+
+varredirectState:RedirectState?
+
  /// Create HTTP request.
  ///
  /// - parameters:
@@ -152,6 +159,8 @@ extension HTTPClient{
 self.host = host
 self.headers = headers
 self.body = body
+
+self.redirectState =nil
 }
 
  /// Whether request will be executed using secure socket.
@@ -813,6 +822,26 @@ internal struct RedirectHandler<ResponseType>{
 }
 
 func redirect(status:HTTPResponseStatus, to redirectURL:URL, promise:EventLoopPromise<ResponseType>){
+varnextState:HTTPClient.Request.RedirectState?
+ifvar state = request.redirectState {
+guard state.count >0else{
+return promise.fail(HTTPClientError.redirectLimitReached)
+}
+
+ state.count -=1
+
+ifvar visited = state.visited {
+guard !visited.contains(redirectURL)else{
+return promise.fail(HTTPClientError.redirectCycleDetected)
+}
+
+ visited.insert(redirectURL)
+ state.visited = visited
+}
+
+ nextState = state
+}
+
 letoriginalRequest=self.request
 
 varconvertToGet=false
@@ -841,7 +870,8 @@ internal struct RedirectHandler<ResponseType>{
 }
 
 do{
-letnewRequest=tryHTTPClient.Request(url: redirectURL, method: method, headers: headers, body: body)
+varnewRequest=tryHTTPClient.Request(url: redirectURL, method: method, headers: headers, body: body)
+ newRequest.redirectState = nextState
 returnself.execute(newRequest).futureResult.cascade(to: promise)
 }catch{
 return promise.fail(error)

Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift

@@ -284,6 +284,16 @@ internal final class HttpBinHandler: ChannelInboundHandler{
  headers.add(name:"Location", value:"http://127.0.0.1:\(port)/echohostheader")
 self.resps.append(HTTPResponseBuilder(status:.found, headers: headers))
 return
+case"/redirect/infinite1":
+varheaders=HTTPHeaders()
+ headers.add(name:"Location", value:"/redirect/infinite2")
+self.resps.append(HTTPResponseBuilder(status:.found, headers: headers))
+return
+case"/redirect/infinite2":
+varheaders=HTTPHeaders()
+ headers.add(name:"Location", value:"/redirect/infinite1")
+self.resps.append(HTTPResponseBuilder(status:.found, headers: headers))
+return
  // Since this String is taken from URL.path, the percent encoding has been removed
 case"/percent encoded":
 if req.method !=.GET {

Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift

@@ -59,6 +59,8 @@ extension HTTPClientTests{
 ("testEventLoopArgument", testEventLoopArgument),
 ("testDecompression", testDecompression),
 ("testDecompressionLimit", testDecompressionLimit),
+("testLoopDetectionRedirectLimit", testLoopDetectionRedirectLimit),
+("testCountRedirectLimit", testCountRedirectLimit),
 ]
 }
 }

Tests/AsyncHTTPClientTests/HTTPClientTests.swift

@@ -131,7 +131,7 @@ class HTTPClientTests: XCTestCase{
 lethttpBin=HTTPBin(ssl:false)
 lethttpsBin=HTTPBin(ssl:true)
 lethttpClient=HTTPClient(eventLoopGroupProvider:.createNew,
- configuration:HTTPClient.Configuration(certificateVerification:.none,followRedirects:true))
+ configuration:HTTPClient.Configuration(certificateVerification:.none,redirectConfiguration:.follow(max:10, allowCycles:true)))
 
 defer{
 XCTAssertNoThrow(try httpClient.syncShutdown())
@@ -149,7 +149,7 @@ class HTTPClientTests: XCTestCase{
 func testHttpHostRedirect()throws{
 lethttpBin=HTTPBin(ssl:false)
 lethttpClient=HTTPClient(eventLoopGroupProvider:.createNew,
- configuration:HTTPClient.Configuration(certificateVerification:.none,followRedirects:true))
+ configuration:HTTPClient.Configuration(certificateVerification:.none,redirectConfiguration:.follow(max:10, allowCycles:true)))
 
 defer{
 XCTAssertNoThrow(try httpClient.syncShutdown())
@@ -526,7 +526,7 @@ class HTTPClientTests: XCTestCase{
 lethttpBin=HTTPBin()
 leteventLoopGroup=MultiThreadedEventLoopGroup(numberOfThreads:5)
 lethttpClient=HTTPClient(eventLoopGroupProvider:.shared(eventLoopGroup),
- configuration:HTTPClient.Configuration(followRedirects:true))
+ configuration:HTTPClient.Configuration(redirectConfiguration:.follow(max:10, allowCycles:true)))
 defer{
 XCTAssertNoThrow(try httpClient.syncShutdown())
 XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully())
@@ -568,6 +568,7 @@ class HTTPClientTests: XCTestCase{
 func testDecompression()throws{
 lethttpBin=HTTPBin(compress:true)
 lethttpClient=HTTPClient(eventLoopGroupProvider:.createNew, configuration:.init(decompression:.enabled(limit:.none)))
+
 defer{
 XCTAssertNoThrow(try httpClient.syncShutdown())
 XCTAssertNoThrow(try httpBin.shutdown())
@@ -603,6 +604,7 @@ class HTTPClientTests: XCTestCase{
 func testDecompressionLimit()throws{
 lethttpBin=HTTPBin(compress:true)
 lethttpClient=HTTPClient(eventLoopGroupProvider:.createNew, configuration:.init(decompression:.enabled(limit:.ratio(10))))
+
 defer{
 XCTAssertNoThrow(try httpClient.syncShutdown())
 XCTAssertNoThrow(try httpBin.shutdown())
@@ -626,4 +628,34 @@ class HTTPClientTests: XCTestCase{
 XCTFail("Unexptected error: \(error)")
 }
 }
+
+func testLoopDetectionRedirectLimit()throws{
+lethttpBin=HTTPBin(ssl:true)
+lethttpClient=HTTPClient(eventLoopGroupProvider:.createNew,
+ configuration:HTTPClient.Configuration(certificateVerification:.none, redirectConfiguration:.follow(max:5, allowCycles:false)))
+
+defer{
+XCTAssertNoThrow(try httpClient.syncShutdown())
+XCTAssertNoThrow(try httpBin.shutdown())
+}
+
+XCTAssertThrowsError(try httpClient.get(url:"https://localhost:\(httpBin.port)/redirect/infinite1").wait(),"Should fail with redirect limit"){ error in
+XCTAssertEqual(error as!HTTPClientError,HTTPClientError.redirectCycleDetected)
+}
+}
+
+func testCountRedirectLimit()throws{
+lethttpBin=HTTPBin(ssl:true)
+lethttpClient=HTTPClient(eventLoopGroupProvider:.createNew,
+ configuration:HTTPClient.Configuration(certificateVerification:.none, redirectConfiguration:.follow(max:5, allowCycles:true)))
+
+defer{
+XCTAssertNoThrow(try httpClient.syncShutdown())
+XCTAssertNoThrow(try httpBin.shutdown())
+}
+
+XCTAssertThrowsError(try httpClient.get(url:"https://localhost:\(httpBin.port)/redirect/infinite1").wait(),"Should fail with redirect limit"){ error in
+XCTAssertEqual(error as!HTTPClientError,HTTPClientError.redirectLimitReached)
+}
+}
 }