Skip to content

Commit 97a93b5

Browse files
maksimorlovichmaksimorlovich
committed
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
1 parent 68c0749 commit 97a93b5

File tree

7 files changed

+112
-19
lines changed

7 files changed

+112
-19
lines changed

‎Foundation/HTTPCookie.swift

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,47 @@ open class HTTPCookie : NSObject {
9393
let _version: Int
9494
var _properties: [HTTPCookiePropertyKey : Any]
9595

96+
// See: https://tools.ietf.org/html/rfc2616#section-3.3.1
97+
98+
// Sun, 06 Nov 1994 08:49:37 GMT ; RFC 822, updated by RFC 1123
99+
static let _formatter1: DateFormatter = {
100+
let formatter = DateFormatter()
101+
formatter.locale = Locale(identifier: "en_US_POSIX")
102+
formatter.dateFormat = "EEE, dd MMM yyyy HH:mm:ss O"
103+
formatter.timeZone = TimeZone(abbreviation: "GMT")
104+
return formatter
105+
}()
106+
107+
// Sunday, 06-Nov-94 08:49:37 GMT ; RFC 850, obsoleted by RFC 1036
108+
static let _formatter2: DateFormatter = {
109+
let formatter = DateFormatter()
110+
formatter.locale = Locale(identifier: "en_US_POSIX")
111+
formatter.dateFormat = "EEEE, dd-MMM-yy HH:mm:ss O"
112+
formatter.timeZone = TimeZone(abbreviation: "GMT")
113+
return formatter
114+
}()
115+
116+
// Sun Nov 6 08:49:37 1994 ; ANSI C's asctime() format
117+
static let _formatter3: DateFormatter = {
118+
let formatter = DateFormatter()
119+
formatter.locale = Locale(identifier: "en_US_POSIX")
120+
formatter.dateFormat = "EEE MMM d HH:mm:ss yyyy"
121+
formatter.timeZone = TimeZone(abbreviation: "GMT")
122+
return formatter
123+
}()
124+
125+
// Sun, 06-Nov-1994 08:49:37 GMT ; Tomcat servers sometimes return cookies in this format
126+
static let _formatter4: DateFormatter = {
127+
let formatter = DateFormatter()
128+
formatter.locale = Locale(identifier: "en_US_POSIX")
129+
formatter.dateFormat = "EEE, dd-MMM-yyyy HH:mm:ss O"
130+
formatter.timeZone = TimeZone(abbreviation: "GMT")
131+
return formatter
132+
}()
133+
134+
static let _allFormatters: [DateFormatter]
135+
= [_formatter1, _formatter2, _formatter3, _formatter4]
136+
96137
static let _attributes: [HTTPCookiePropertyKey]
97138
= [.name, .value, .originURL, .version, .domain,
98139
.path, .secure, .expires, .comment, .commentURL,
@@ -292,12 +333,8 @@ open class HTTPCookie : NSObject {
292333
if let date = expiresProperty as? Date {
293334
expDate = date
294335
} else if let dateString = expiresProperty as? String {
295-
let formatter = DateFormatter()
296-
formatter.locale = Locale(identifier: "en_US_POSIX")
297-
formatter.dateFormat = "EEE, dd MMM yyyy HH:mm:ss O" // per RFC 6265 '<rfc1123-date, defined in [RFC2616], Section 3.3.1>'
298-
let timeZone = TimeZone(abbreviation: "GMT")
299-
formatter.timeZone = timeZone
300-
expDate = formatter.date(from: dateString)
336+
let results = HTTPCookie._allFormatters.compactMap { $0.date(from: dateString) }
337+
expDate = results.first
301338
}
302339
}
303340
_expiresDate = expDate
@@ -418,7 +455,7 @@ open class HTTPCookie : NSObject {
418455
let name = pair.components(separatedBy: "=")[0]
419456
var value = pair.components(separatedBy: "\(name)=")[1] //a value can have an "="
420457
if canonicalize(name) == .expires {
421-
value = value.insertComma(at: 3) //re-insert the comma
458+
value = value.unmaskCommas() //re-insert the comma
422459
}
423460
properties[canonicalize(name)] = value
424461
}
@@ -439,7 +476,7 @@ open class HTTPCookie : NSObject {
439476
//we pass this to a map()
440477
private class func removeCommaFromDate(_ value: String) -> String {
441478
if value.hasPrefix("Expires") || value.hasPrefix("expires") {
442-
return value.removeCommas()
479+
return value.maskCommas()
443480
}
444481
return value
445482
}
@@ -623,12 +660,12 @@ fileprivate extension String {
623660
return self.trimmingCharacters(in: .whitespacesAndNewlines)
624661
}
625662

626-
func removeCommas() -> String {
627-
return self.replacingOccurrences(of: ",", with: "")
663+
func maskCommas() -> String {
664+
return self.replacingOccurrences(of: ",", with: "&comma")
628665
}
629666

630-
func insertComma(at index:Int) -> String {
631-
return String(self.prefix(index)) + "," + String(self.suffix(self.count-index))
667+
func unmaskCommas() -> String {
668+
return self.replacingOccurrences(of: "&comma", with: ",")
632669
}
633670
}
634671

‎Foundation/URLSession/Configuration.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ internal extension URLSession._Configuration {
111111
if let cookieStorage = self.httpCookieStorage, let url = request.url, let cookies = cookieStorage.cookies(for: url) {
112112
let cookiesHeaderFields = HTTPCookie.requestHeaderFields(with: cookies)
113113
if let cookieValue = cookiesHeaderFields["Cookie"], cookieValue != "" {
114-
request.addValue(cookieValue, forHTTPHeaderField: "Cookie")
114+
request.setValue(cookieValue, forHTTPHeaderField: "Cookie")
115115
}
116116
}
117117
}

‎Foundation/URLSession/http/HTTPURLProtocol.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,9 @@ internal class _HTTPURLProtocol: _NativeProtocol {
219219
}
220220
}
221221
case .noDelegate, .dataCompletionHandler, .downloadCompletionHandler:
222-
// Follow the redirect.
223-
startNewTransfer(with: request)
222+
// Follow the redirect. Need to configure new request with cookies, etc.
223+
let configuredRequest = session._configuration.configure(request: request)
224+
startNewTransfer(with: configuredRequest)
224225
}
225226
}
226227

‎Foundation/URLSession/libcurl/EasyHandle.swift

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

‎TestFoundation/HTTPServer.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,10 @@ public class TestURLSessionServer {
477477
let text = request.getCommaSeparatedHeaders()
478478
return _HTTPResponse(response: .OK, headers: "Content-Length: \(text.data(using: .utf8)!.count)", body: text)
479479
}
480+
481+
if uri == "/redirectSetCookies" {
482+
return _HTTPResponse(response: .REDIRECT, headers: "Location: /setCookies\r\nSet-Cookie: redirect=true; Max-Age=7776000; path=/", body: "")
483+
}
480484

481485
if uri == "/UnitedStates" {
482486
let value = capitals[String(uri.dropFirst())]!

‎TestFoundation/TestHTTPCookie.swift

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ class TestHTTPCookie: XCTestCase {
1717
("test_cookiesWithResponseHeader0cookies", test_cookiesWithResponseHeader0cookies),
1818
("test_cookiesWithResponseHeader2cookies", test_cookiesWithResponseHeader2cookies),
1919
("test_cookiesWithResponseHeaderNoDomain", test_cookiesWithResponseHeaderNoDomain),
20-
("test_cookiesWithResponseHeaderNoPathNoDomain", test_cookiesWithResponseHeaderNoPathNoDomain)
20+
("test_cookiesWithResponseHeaderNoPathNoDomain", test_cookiesWithResponseHeaderNoPathNoDomain),
21+
("test_cookieExpiresDateFormats", test_cookieExpiresDateFormats),
2122
]
2223
}
2324

@@ -168,4 +169,27 @@ class TestHTTPCookie: XCTestCase {
168169
XCTAssertEqual(cookies[0].domain, "example.com")
169170
XCTAssertEqual(cookies[0].path, "/")
170171
}
172+
173+
func test_cookieExpiresDateFormats() {
174+
let testDate = Date(timeIntervalSince1970: 1577881800)
175+
let cookieString =
176+
"""
177+
format1=true; expires=Wed, 01 Jan 2020 12:30:00 GMT; path=/; domain=swift.org; secure; httponly,
178+
format2=true; expires=Wednesday, 01-Jan-20 12:30:00 GMT; path=/; domain=swift.org; secure; httponly,
179+
format3=true; expires=Wed Jan 1 12:30:00 2020; path=/; domain=swift.org; secure; httponly,
180+
format4=true; expires=Wed, 01-Jan-2020 12:30:00 GMT; path=/; domain=swift.org; secure; httponly
181+
"""
182+
183+
let header = ["header1":"value1",
184+
"Set-Cookie": cookieString,
185+
"header2":"value2",
186+
"header3":"value3"]
187+
let cookies = HTTPCookie.cookies(withResponseHeaderFields: header, for: URL(string: "https://swift.org")!)
188+
XCTAssertEqual(cookies.count, 4)
189+
cookies.forEach { cookie in
190+
XCTAssertEqual(cookie.expiresDate, testDate)
191+
XCTAssertEqual(cookie.domain, "swift.org")
192+
XCTAssertEqual(cookie.path, "/")
193+
}
194+
}
171195
}

‎TestFoundation/TestURLSession.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class TestURLSession : LoopbackServerTest {
4444
("test_dontSetCookies", test_dontSetCookies),
4545
("test_initURLSessionConfiguration", test_initURLSessionConfiguration),
4646
("test_basicAuthRequest", test_basicAuthRequest),
47+
("test_redirectionWithSetCookies", test_redirectionWithSetCookies),
4748
]
4849
}
4950

@@ -579,6 +580,31 @@ class TestURLSession : LoopbackServerTest {
579580
XCTAssertEqual(cookies?.count, 1)
580581
}
581582

583+
func test_redirectionWithSetCookies() {
584+
let config = URLSessionConfiguration.default
585+
config.timeoutIntervalForRequest = 5
586+
if let storage = config.httpCookieStorage, let cookies = storage.cookies {
587+
for cookie in cookies {
588+
storage.deleteCookie(cookie)
589+
}
590+
}
591+
let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/redirectSetCookies"
592+
let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil)
593+
var expect = expectation(description: "POST \(urlString)")
594+
var req = URLRequest(url: URL(string: urlString)!)
595+
var task = session.dataTask(with: req) { (data, _, error) -> Void in
596+
defer { expect.fulfill() }
597+
XCTAssertNotNil(data)
598+
XCTAssertNil(error as? URLError, "error = \(error as! URLError)")
599+
guard let data = data else { return }
600+
let headers = String(data: data, encoding: String.Encoding.utf8) ?? ""
601+
print("headers here = \(headers)")
602+
XCTAssertNotNil(headers.range(of: "Cookie: redirect=true"))
603+
}
604+
task.resume()
605+
waitForExpectations(timeout: 30)
606+
}
607+
582608
func test_setCookies() {
583609
let config = URLSessionConfiguration.default
584610
config.timeoutIntervalForRequest = 5

0 commit comments

Comments
 (0)