Various Cookie fixes (#1706)

* Various Cookie fixes

- Add support for additional Set-Cookie formats that web servers can return
- Properly handle HTTP header parsing to extract values since values can contain colons
- Make sure to set cookies on redirect requests
- Use setValue instead of addValue when applying cookies to requests otherwise,
  Cookie header might contain: cookie1=value1,cookie1=value1; cookie2=value2
- New unit tests for cookie formats and redirect with Set-Cookie

(cherry picked from commit 97a93b5)

* Remove two-digit year cookie format support & unit test (fails on Ubuntu 14.04) (#1707)

Foundation/HTTPCookie.swift

@@ -93,6 +93,38 @@ open class HTTPCookie : NSObject {
  let _version: Int
  var _properties: [HTTPCookiePropertyKey : Any]
 
+ // See: https://tools.ietf.org/html/rfc2616#section-3.3.1
+
+ // Sun, 06 Nov 1994 08:49:37 GMT ; RFC 822, updated by RFC 1123
+ static let _formatter1: DateFormatter = {
+ let formatter = DateFormatter()
+ formatter.locale = Locale(identifier: "en_US_POSIX")
+ formatter.dateFormat = "EEE, dd MMM yyyy HH:mm:ss O"
+ formatter.timeZone = TimeZone(abbreviation: "GMT")
+ return formatter
+ }()
+
+ // Sun Nov 6 08:49:37 1994 ; ANSI C's asctime() format
+ static let _formatter2: DateFormatter = {
+ let formatter = DateFormatter()
+ formatter.locale = Locale(identifier: "en_US_POSIX")
+ formatter.dateFormat = "EEE MMM d HH:mm:ss yyyy"
+ formatter.timeZone = TimeZone(abbreviation: "GMT")
+ return formatter
+ }()
+
+ // Sun, 06-Nov-1994 08:49:37 GMT ; Tomcat servers sometimes return cookies in this format
+ static let _formatter3: DateFormatter = {
+ let formatter = DateFormatter()
+ formatter.locale = Locale(identifier: "en_US_POSIX")
+ formatter.dateFormat = "EEE, dd-MMM-yyyy HH:mm:ss O"
+ formatter.timeZone = TimeZone(abbreviation: "GMT")
+ return formatter
+ }()
+
+ static let _allFormatters: [DateFormatter]
+ = [_formatter1, _formatter2, _formatter3]
+
  static let _attributes: [HTTPCookiePropertyKey]
  = [.name, .value, .originURL, .version, .domain,
  .path, .secure, .expires, .comment, .commentURL,
@@ -292,12 +324,8 @@ open class HTTPCookie : NSObject {
  if let date = expiresProperty as? Date {
  expDate = date
  } else if let dateString = expiresProperty as? String {
- let formatter = DateFormatter()
- formatter.locale = Locale(identifier: "en_US_POSIX")
- formatter.dateFormat = "EEE, dd MMM yyyy HH:mm:ss O" // per RFC 6265 '<rfc1123-date, defined in [RFC2616], Section 3.3.1>'
- let timeZone = TimeZone(abbreviation: "GMT")
- formatter.timeZone = timeZone
- expDate = formatter.date(from: dateString)
+ let results = HTTPCookie._allFormatters.compactMap { $0.date(from: dateString) }
+ expDate = results.first
  }
  }
  _expiresDate = expDate
@@ -418,7 +446,7 @@ open class HTTPCookie : NSObject {
  let name = pair.components(separatedBy: "=")[0]
  var value = pair.components(separatedBy: "\(name)=")[1] //a value can have an "="
  if canonicalize(name) == .expires {
- value = value.insertComma(at: 3) //re-insert the comma
+ value = value.unmaskCommas() //re-insert the comma
  }
  properties[canonicalize(name)] = value
  }
@@ -439,7 +467,7 @@ open class HTTPCookie : NSObject {
  //we pass this to a map()
  private class func removeCommaFromDate(_ value: String) -> String {
  if value.hasPrefix("Expires") || value.hasPrefix("expires") {
- return value.removeCommas()
+ return value.maskCommas()
  }
  return value
  }
@@ -623,12 +651,12 @@ fileprivate extension String {
  return self.trimmingCharacters(in: NSCharacterSet.whitespacesAndNewlines)
  }
 
- func removeCommas() -> String {
- return self.replacingOccurrences(of: ",", with: "")
+ func maskCommas() -> String {
+ return self.replacingOccurrences(of: ",", with: "&comma")
  }
 
- func insertComma(at index:Int) -> String {
- return  String(self.prefix(index)) + "," + String(self.suffix(self.count-index))
+ func unmaskCommas() -> String {
+ return self.replacingOccurrences(of: "&comma", with: ",")
  }
 }
 

Foundation/URLSession/Configuration.swift

@@ -115,7 +115,7 @@ internal extension URLSession._Configuration {
  if let cookieStorage = self.httpCookieStorage, let url = request.url, let cookies = cookieStorage.cookies(for: url) {
  let cookiesHeaderFields = HTTPCookie.requestHeaderFields(with: cookies)
  if let cookieValue = cookiesHeaderFields["Cookie"], cookieValue != "" {
- request.addValue(cookieValue, forHTTPHeaderField: "Cookie")
+ request.setValue(cookieValue, forHTTPHeaderField: "Cookie")
  }
  }
  }

Foundation/URLSession/http/HTTPURLProtocol.swift

@@ -120,7 +120,9 @@ internal class _HTTPURLProtocol: _NativeProtocol {
  httpHeaders = hh
  }
 
- if let hh = self.task?.originalRequest?.allHTTPHeaderFields {
+ // In case this is a redirect, take the headers from the current (redirect) request.
+ if let hh = self.task?.currentRequest?.allHTTPHeaderFields ??
+ self.task?.originalRequest?.allHTTPHeaderFields {
  if httpHeaders == nil {
  httpHeaders = hh
  } else {
@@ -211,8 +213,9 @@ internal class _HTTPURLProtocol: _NativeProtocol {
  }
  }
  case .noDelegate, .dataCompletionHandler, .downloadCompletionHandler:
- // Follow the redirect.
- startNewTransfer(with: request)
+ // Follow the redirect. Need to configure new request with cookies, etc.
+ let configuredRequest = session._configuration.configure(request: request)
+ startNewTransfer(with: configuredRequest)
  }
  }
 

Foundation/URLSession/libcurl/EasyHandle.swift

@@ -540,9 +540,10 @@ fileprivate extension _EasyHandle {
  fileprivate func setCookies(headerData data: Data) {
  guard let config = _config, config.httpCookieAcceptPolicy != HTTPCookie.AcceptPolicy.never else { return }
  guard let headerData = String(data: data, encoding: String.Encoding.utf8) else { return }
- //Convert headerData from a string to a dictionary.
- //Ignore headers like 'HTTP/1.1 200 OK\r\n' which do not have a key value pair.
- let headerComponents = headerData.split { $0 == ":" }
+ // Convert headerData from a string to a dictionary.
+ // Ignore headers like 'HTTP/1.1 200 OK\r\n' which do not have a key value pair.
+ // Value can have colons (ie, date), so only split at the first one, ie header:value
+ let headerComponents = headerData.split(separator: ":", maxSplits: 1)
  var headers: [String: String] = [:]
  //Trim the leading and trailing whitespaces (if any) before adding the header information to the dictionary.
  if headerComponents.count > 1 {

TestFoundation/HTTPServer.swift

@@ -435,6 +435,10 @@ public class TestURLSessionServer {
  let text = request.getCommaSeparatedHeaders()
  return _HTTPResponse(response: .OK, headers: "Content-Length: \(text.data(using: .utf8)!.count)", body: text)
  }
+
+ if uri == "/redirectSetCookies" {
+ return _HTTPResponse(response: .REDIRECT, headers: "Location: /setCookies\r\nSet-Cookie: redirect=true; Max-Age=7776000; path=/", body: "")
+ }
 
  if uri == "/UnitedStates" {
  let value = capitals[String(uri.dropFirst())]!

TestFoundation/TestHTTPCookie.swift

@@ -17,7 +17,8 @@ class TestHTTPCookie: XCTestCase {
  ("test_cookiesWithResponseHeader0cookies", test_cookiesWithResponseHeader0cookies),
  ("test_cookiesWithResponseHeader2cookies", test_cookiesWithResponseHeader2cookies),
  ("test_cookiesWithResponseHeaderNoDomain", test_cookiesWithResponseHeaderNoDomain),
- ("test_cookiesWithResponseHeaderNoPathNoDomain", test_cookiesWithResponseHeaderNoPathNoDomain)
+ ("test_cookiesWithResponseHeaderNoPathNoDomain", test_cookiesWithResponseHeaderNoPathNoDomain),
+ ("test_cookieExpiresDateFormats", test_cookieExpiresDateFormats),
  ]
  }
 
@@ -168,4 +169,26 @@ class TestHTTPCookie: XCTestCase {
  XCTAssertEqual(cookies[0].domain, "example.com")
  XCTAssertEqual(cookies[0].path, "/")
  }
+
+ func test_cookieExpiresDateFormats() {
+ let testDate = Date(timeIntervalSince1970: 1577881800)
+ let cookieString =
+ """
+ format1=true; expires=Wed, 01 Jan 2020 12:30:00 GMT; path=/; domain=swift.org; secure; httponly,
+ format2=true; expires=Wed Jan 1 12:30:00 2020; path=/; domain=swift.org; secure; httponly,
+ format3=true; expires=Wed, 01-Jan-2020 12:30:00 GMT; path=/; domain=swift.org; secure; httponly
+ """
+
+ let header = ["header1":"value1",
+ "Set-Cookie": cookieString,
+ "header2":"value2",
+ "header3":"value3"]
+ let cookies = HTTPCookie.cookies(withResponseHeaderFields: header, for: URL(string: "https://swift.org")!)
+ XCTAssertEqual(cookies.count, 3)
+ cookies.forEach { cookie in
+ XCTAssertEqual(cookie.expiresDate, testDate)
+ XCTAssertEqual(cookie.domain, "swift.org")
+ XCTAssertEqual(cookie.path, "/")
+ }
+ }
 }

TestFoundation/TestURLSession.swift

@@ -42,6 +42,7 @@ class TestURLSession : LoopbackServerTest {
  ("test_cookiesStorage", test_cookiesStorage),
  ("test_setCookies", test_setCookies),
  ("test_dontSetCookies", test_dontSetCookies),
+ ("test_redirectionWithSetCookies", test_redirectionWithSetCookies),
  ]
  }
 
@@ -575,6 +576,31 @@ class TestURLSession : LoopbackServerTest {
  XCTAssertEqual(cookies?.count, 1)
  }
 
+ func test_redirectionWithSetCookies() {
+ let config = URLSessionConfiguration.default
+ config.timeoutIntervalForRequest = 5
+ if let storage = config.httpCookieStorage, let cookies = storage.cookies {
+ for cookie in cookies {
+ storage.deleteCookie(cookie)
+ }
+ }
+ let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/redirectSetCookies"
+ let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil)
+ var expect = expectation(description: "POST \(urlString)")
+ var req = URLRequest(url: URL(string: urlString)!)
+ var task = session.dataTask(with: req) { (data, _, error) -> Void in
+ defer { expect.fulfill() }
+ XCTAssertNotNil(data)
+ XCTAssertNil(error as? URLError, "error = \(error as! URLError)")
+ guard let data = data else { return }
+ let headers = String(data: data, encoding: String.Encoding.utf8) ?? ""
+ print("headers here = \(headers)")
+ XCTAssertNotNil(headers.range(of: "Cookie: redirect=true"))
+ }
+ task.resume()
+ waitForExpectations(timeout: 30)
+ }
+
  func test_setCookies() {
  let config = URLSessionConfiguration.default
  config.timeoutIntervalForRequest = 5