Transfer scheme and host to new request when redirected

Package.swift

@@ -31,7 +31,7 @@ let package = Package(
  ),
  .testTarget(
  name: "NIOHTTPClientTests",
- dependencies: ["NIOHTTPClient"]
+ dependencies: ["NIOHTTPClient", "NIOFoundationCompat"]
  ),
  ]
 )

Sources/NIOHTTPClient/HTTPClientProxyHandler.swift

@@ -74,7 +74,7 @@ internal final class HTTPClientProxyHandler: ChannelDuplexHandler, RemovableChan
  switch res{
  case .head(let head):
  switch head.status.code{
- case 200..<300:
+ case 200..<300:
  // Any 2xx (Successful) response indicates that the sender (and all
  // inbound proxies) will switch to tunnel mode immediately after the
  // blank line that concludes the successful response's header section
@@ -116,7 +116,7 @@ internal final class HTTPClientProxyHandler: ChannelDuplexHandler, RemovableChan
  private func handleConnect(context: ChannelHandlerContext) -> EventLoopFuture<Void>{
  return self.onConnect(context.channel).flatMap{
  self.readState = .connected
-
+
  // forward any buffered reads
  while !self.readBuffer.isEmpty{
  context.fireChannelRead(self.readBuffer.removeFirst())

Sources/NIOHTTPClient/HTTPCookie.swift

@@ -68,7 +68,7 @@ public struct HTTPCookie{
  formatter.locale = Locale(identifier: "en_US")
  formatter.timeZone = TimeZone(identifier: "GMT")
  formatter.dateFormat = "EEE, dd MMM yyyy HH:mm:ss z"
- self.expires = parseComponentValue(component).flatMap{formatter.date(from: $0) }
+ self.expires = self.parseComponentValue(component).flatMap{formatter.date(from: $0) }
  continue
  }
 

Sources/NIOHTTPClient/HTTPHandler.swift

@@ -140,7 +140,7 @@ internal class ResponseAccumulator: HTTPClientResponseDelegate{
  case .body(let head, var body):
  var part = part
  body.writeBuffer(&part)
- state = .body(head, body)
+self.state = .body(head, body)
  case .end:
  preconditionFailure("request already processed")
  case .error:
@@ -171,7 +171,7 @@ internal class ResponseAccumulator: HTTPClientResponseDelegate{
 /// This delegate is strongly held by the HTTPTaskHandler
 /// for the duration of the HTTPRequest processing and will be
 /// released together with the HTTPTaskHandler when channel is closed
-public protocol HTTPClientResponseDelegate: class{
+public protocol HTTPClientResponseDelegate: AnyObject{
  associatedtype Response
 
  func didTransmitRequestBody(task: HTTPClient.Task<Response>)
@@ -361,13 +361,13 @@ internal class TaskHandler<T: HTTPClientResponseDelegate>: ChannelInboundHandler
  if (event as? IdleStateHandler.IdleStateEvent) == .read{
  self.state = .end
  let error = HTTPClientError.readTimeout
- delegate.didReceiveError(task: self.task, error)
- promise.fail(error)
+self.delegate.didReceiveError(task: self.task, error)
+self.promise.fail(error)
  } else if (event as? TaskCancelEvent) != nil{
  self.state = .end
  let error = HTTPClientError.cancelled
- delegate.didReceiveError(task: self.task, error)
- promise.fail(error)
+self.delegate.didReceiveError(task: self.task, error)
+self.promise.fail(error)
  } else{
  context.fireUserInboundEventTriggered(event)
  }
@@ -380,8 +380,8 @@ internal class TaskHandler<T: HTTPClientResponseDelegate>: ChannelInboundHandler
  default:
  self.state = .end
  let error = HTTPClientError.remoteConnectionClosed
- delegate.didReceiveError(task: self.task, error)
- promise.fail(error)
+self.delegate.didReceiveError(task: self.task, error)
+self.promise.fail(error)
  }
  }
 
@@ -408,7 +408,7 @@ internal class TaskHandler<T: HTTPClientResponseDelegate>: ChannelInboundHandler
 
 internal struct RedirectHandler<T>{
  let request: HTTPClient.Request
- let execute: ((HTTPClient.Request) -> HTTPClient.Task<T>)
+ let execute: (HTTPClient.Request) -> HTTPClient.Task<T>
 
  func redirectTarget(status: HTTPResponseStatus, headers: HTTPHeaders) -> URL?{
  switch status{
@@ -443,6 +443,18 @@ internal struct RedirectHandler<T>{
  var request = self.request
  request.url = redirectURL
 
+ if let redirectHost = redirectURL.host{
+ request.host = redirectHost
+ } else{
+ preconditionFailure("redirectURL doesn't contain a host")
+ }
+
+ if let redirectScheme = redirectURL.scheme{
+ request.scheme = redirectScheme
+ } else{
+ preconditionFailure("redirectURL doesn't contain a scheme")
+ }
+
  var convertToGet = false
  if status == .seeOther, request.method != .HEAD{
  convertToGet = true

Sources/NIOHTTPClient/SwiftNIOHTTP.swift

@@ -115,7 +115,6 @@ public class HTTPClient{
 
  public func execute<T: HTTPClientResponseDelegate>(request: Request, delegate: T, timeout: Timeout? = nil) -> Task<T.Response>{
  let timeout = timeout ?? configuration.timeout
-
  let promise: EventLoopPromise<T.Response> = self.eventLoopGroup.next().makePromise()
 
  let redirectHandler: RedirectHandler<T.Response>?
@@ -151,12 +150,12 @@ public class HTTPClient{
  let taskHandler = TaskHandler(task: task, delegate: delegate, promise: promise, redirectHandler: redirectHandler)
  return channel.pipeline.addHandler(taskHandler)
  }
- }
+}
 
  if let connectTimeout = timeout.connect{
  bootstrap = bootstrap.connectTimeout(connectTimeout)
  }
-
+
  let address = self.resolveAddress(request: request, proxy: self.configuration.proxy)
  bootstrap.connect(host: address.host, port: address.port)
  .map{channel in
@@ -172,7 +171,7 @@ public class HTTPClient{
  return task
  }
 
- private func resolveAddress(request: Request, proxy: Proxy?) -> (host: String, port: Int) {
+ private func resolveAddress(request: Request, proxy: Proxy?) -> (host: String, port: Int){
  switch self.configuration.proxy{
  case .none:
  return (request.host, request.port)
@@ -216,7 +215,7 @@ public class HTTPClient{
 private extension ChannelPipeline{
  func addProxyHandler(for request: HTTPClient.Request, decoder: ByteToMessageHandler<HTTPResponseDecoder>, encoder: HTTPRequestEncoder, tlsConfiguration: TLSConfiguration?) -> EventLoopFuture<Void>{
  let handler = HTTPClientProxyHandler(host: request.host, port: request.port, onConnect:{channel in
-return channel.pipeline.removeHandler(decoder).flatMap{
+ channel.pipeline.removeHandler(decoder).flatMap{
  return channel.pipeline.addHandler(
  ByteToMessageHandler(HTTPResponseDecoder(leftOverBytesStrategy: .forwardBytes)),
  position: .after(encoder)

Tests/NIOHTTPClientTests/HTTPClientTestUtils.swift

@@ -34,7 +34,7 @@ class TestHTTPDelegate: HTTPClientResponseDelegate{
  case .body(let head, var body):
  var buffer = buffer
  body.writeBuffer(&buffer)
- state = .body(head, body)
+self.state = .body(head, body)
  default:
  preconditionFailure("expecting head or body")
  }
@@ -94,11 +94,11 @@ internal class HttpBin{
  }
 
  init(ssl: Bool = false, simulateProxy: HTTPProxySimulator.Option? = nil){
- self.serverChannel = try! ServerBootstrap(group: group)
+ self.serverChannel = try! ServerBootstrap(group: self.group)
  .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
  .childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
  .childChannelInitializer{channel in
-return channel.pipeline.configureHTTPServerPipeline(withPipeliningAssistance: true, withErrorHandling: true).flatMap{
+ channel.pipeline.configureHTTPServerPipeline(withPipeliningAssistance: true, withErrorHandling: true).flatMap{
  if let simulateProxy = simulateProxy{
  return channel.pipeline.addHandler(HTTPProxySimulator(option: simulateProxy), position: .first)
  } else{
@@ -216,13 +216,27 @@ internal final class HttpBinHandler: ChannelInboundHandler{
  case "/redirect/302":
  var headers = HTTPHeaders()
  headers.add(name: "Location", value: "/ok")
- resps.append(HTTPResponseBuilder(status: .found, headers: headers))
+self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
  return
  case "/redirect/https":
- let port = value(for: "port", from: url.query!)
+ let port = self.value(for: "port", from: url.query!)
  var headers = HTTPHeaders()
  headers.add(name: "Location", value: "https://localhost:\(port)/ok")
- resps.append(HTTPResponseBuilder(status: .found, headers: headers))
+ self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
+ return
+ case "/redirect/loopback":
+ let port = self.value(for: "port", from: url.query!)
+ var headers = HTTPHeaders()
+ headers.add(name: "Location", value: "http://127.0.0.1:\(port)/echohostheader")
+ self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
+ return
+ case "/echohostheader":
+ var builder = HTTPResponseBuilder(status: .ok)
+ let hostValue = req.headers["Host"].first ?? ""
+ var buff = context.channel.allocator.buffer(capacity: hostValue.utf8.count)
+ buff.writeString(hostValue)
+ builder.add(buff)
+ self.resps.append(builder)
  return
  case "/wait":
  return
@@ -246,14 +260,14 @@ internal final class HttpBinHandler: ChannelInboundHandler{
  return
  }
  case .body(let body):
- var response = resps.removeFirst()
+ var response = self.resps.removeFirst()
  response.add(body)
- resps.prepend(response)
+self.resps.prepend(response)
  case .end:
  if self.resps.isEmpty{
  return
  }
- let response = resps.removeFirst()
+ let response = self.resps.removeFirst()
  context.write(wrapOutboundOut(.head(response.head)), promise: nil)
  if let body = response.body{
  let data = body.withUnsafeReadableBytes{
@@ -288,7 +302,7 @@ internal final class HttpBinHandler: ChannelInboundHandler{
  }
 }
 
-fileprivate let cert = """
+private let cert = """
 -----BEGIN CERTIFICATE-----
 MIICmDCCAYACCQCPC8JDqMh1zzANBgkqhkiG9w0BAQsFADANMQswCQYDVQQGEwJ1
 czAgFw0xODEwMzExNTU1MjJaGA8yMTE4MTAwNzE1NTUyMlowDTELMAkGA1UEBhMC
@@ -307,7 +321,7 @@ Au4LoEYwT730QKC/VQxxEVZobjn9/sTrq9CZlbPYHxX4fz6e00sX7H9i49vk9zQ5
 -----END CERTIFICATE-----
 """
 
-fileprivate let key = """
+private let key = """
 -----BEGIN PRIVATE KEY-----
 MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQDiC+TGmbSP/nWW
 N1tjyNfnWCU5ATjtIOfdtP6ycx8JSeqkvyNXG21kNUn14jTTU8BglGL2hfVpCbMi

Tests/NIOHTTPClientTests/SwiftNIOHTTPTests+XCTest.swift

@@ -33,6 +33,7 @@ extension SwiftHTTPTests{
  ("testGetHttps", testGetHttps),
  ("testPostHttps", testPostHttps),
  ("testHttpRedirect", testHttpRedirect),
+ ("testHttpHostRedirect", testHttpHostRedirect),
  ("testMultipleContentLengthHeaders", testMultipleContentLengthHeaders),
  ("testStreaming", testStreaming),
  ("testRemoteClose", testRemoteClose),

Tests/NIOHTTPClientTests/SwiftNIOHTTPTests.swift

@@ -14,6 +14,7 @@
 
 import Foundation
 import NIO
+import NIOFoundationCompat
 @testable import NIOHTTP1
 @testable import NIOHTTPClient
 import NIOSSL
@@ -171,6 +172,30 @@ class SwiftHTTPTests: XCTestCase{
  XCTAssertEqual(response.status, .ok)
  }
 
+ func testHttpHostRedirect() throws{
+ let httpBin = HttpBin(ssl: false)
+ let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
+ configuration: HTTPClient.Configuration(certificateVerification: .none, followRedirects: true))
+
+ defer{
+ try! httpClient.syncShutdown()
+ httpBin.shutdown()
+ }
+
+ let response = try httpClient.get(url: "http://localhost:\(httpBin.port)/redirect/loopback?port=\(httpBin.port)").wait()
+ guard var body = response.body else{
+ XCTFail("The target page should have a body containing the value of the Host header")
+ return
+ }
+ guard let responseData = body.readData(length: body.readableBytes) else{
+ XCTFail("Read data shouldn't be nil since we passed body.readableBytes to body.readData")
+ return
+ }
+ let decoder = JSONDecoder()
+ let hostName = try decoder.decode([String: String].self, from: responseData)["data"]
+ XCTAssert(hostName == "127.0.0.1")
+ }
+
  func testMultipleContentLengthHeaders() throws{
  let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
  defer{