From 753e621b5f6c483bd8ef25d7b511a437b6f90f0c Mon Sep 17 00:00:00 2001 From: Maksim Orlovich Date: Mon, 1 Oct 2018 16:21:18 -0500 Subject: [PATCH] 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 97a93b5d4c1fca906254172896086183021ef374) * Remove two-digit year cookie format support & unit test (fails on Ubuntu 14.04) (#1707) --- Foundation/HTTPCookie.swift | 52 ++++++++++++++----- Foundation/URLSession/Configuration.swift | 2 +- .../URLSession/http/HTTPURLProtocol.swift | 9 ++-- .../URLSession/libcurl/EasyHandle.swift | 7 +-- TestFoundation/HTTPServer.swift | 4 ++ TestFoundation/TestHTTPCookie.swift | 25 ++++++++- TestFoundation/TestURLSession.swift | 26 ++++++++++ 7 files changed, 105 insertions(+), 20 deletions(-) diff --git a/Foundation/HTTPCookie.swift b/Foundation/HTTPCookie.swift index b63c9f641b..7bf46ceb1f 100644 --- a/Foundation/HTTPCookie.swift +++ b/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 '' - 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: ",") } } diff --git a/Foundation/URLSession/Configuration.swift b/Foundation/URLSession/Configuration.swift index 573f6788c0..5148d85716 100644 --- a/Foundation/URLSession/Configuration.swift +++ b/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") } } } diff --git a/Foundation/URLSession/http/HTTPURLProtocol.swift b/Foundation/URLSession/http/HTTPURLProtocol.swift index 83cc7af25d..7444a437cf 100644 --- a/Foundation/URLSession/http/HTTPURLProtocol.swift +++ b/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) } } diff --git a/Foundation/URLSession/libcurl/EasyHandle.swift b/Foundation/URLSession/libcurl/EasyHandle.swift index a4e9de83e4..a194be9e72 100644 --- a/Foundation/URLSession/libcurl/EasyHandle.swift +++ b/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 { diff --git a/TestFoundation/HTTPServer.swift b/TestFoundation/HTTPServer.swift index 608fbda244..8bcc1e8554 100644 --- a/TestFoundation/HTTPServer.swift +++ b/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())]! diff --git a/TestFoundation/TestHTTPCookie.swift b/TestFoundation/TestHTTPCookie.swift index b46e373087..aee767f6fd 100644 --- a/TestFoundation/TestHTTPCookie.swift +++ b/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, "/") + } + } } diff --git a/TestFoundation/TestURLSession.swift b/TestFoundation/TestURLSession.swift index fbb3048f7d..2c1c1a715c 100644 --- a/TestFoundation/TestURLSession.swift +++ b/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