Skip to content

Various Cookie fixes #1706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 40 additions & 12 deletions Foundation/HTTPCookie.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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: ",")
}
}

2 changes: 1 addition & 1 deletion Foundation/URLSession/Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
}
Expand Down
9 changes: 6 additions & 3 deletions Foundation/URLSession/http/HTTPURLProtocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
}

Expand Down
7 changes: 4 additions & 3 deletions Foundation/URLSession/libcurl/EasyHandle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions TestFoundation/HTTPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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())]!
Expand Down
25 changes: 24 additions & 1 deletion TestFoundation/TestHTTPCookie.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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),
]
}

Expand Down Expand Up @@ -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, "/")
}
}
}
26 changes: 26 additions & 0 deletions TestFoundation/TestURLSession.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class TestURLSession : LoopbackServerTest {
("test_cookiesStorage", test_cookiesStorage),
("test_setCookies", test_setCookies),
("test_dontSetCookies", test_dontSetCookies),
("test_redirectionWithSetCookies", test_redirectionWithSetCookies),
]
}

Expand Down Expand Up @@ -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
Expand Down