From 9870d94d191d272f850093adc1dad71cab1556d9 Mon Sep 17 00:00:00 2001 From: Maksim Orlovich Date: Thu, 12 Jul 2018 11:57:37 -0500 Subject: [PATCH 1/2] Ensure HTTPCookie domain matching conforms to RFC6265 Fix an issue when HTTPCookie domain starts with ".", cookies are not being sent with HTTP requests. Also, force domain matching to be in all lower case. --- Foundation/HTTPCookie.swift | 2 +- Foundation/HTTPCookieStorage.swift | 32 +++++++++++--- TestFoundation/TestHTTPCookieStorage.swift | 49 ++++++++++++++++++++++ 3 files changed, 77 insertions(+), 6 deletions(-) diff --git a/Foundation/HTTPCookie.swift b/Foundation/HTTPCookie.swift index 12f739b407..13bff60266 100644 --- a/Foundation/HTTPCookie.swift +++ b/Foundation/HTTPCookie.swift @@ -246,7 +246,7 @@ open class HTTPCookie : NSObject { _path = path _name = name _value = value - _domain = canonicalDomain + _domain = canonicalDomain.lowercased() if let secureString = properties[.secure] as? String, !secureString.isEmpty diff --git a/Foundation/HTTPCookieStorage.swift b/Foundation/HTTPCookieStorage.swift index e083b592d6..e64defa367 100644 --- a/Foundation/HTTPCookieStorage.swift +++ b/Foundation/HTTPCookieStorage.swift @@ -267,8 +267,8 @@ open class HTTPCookieStorage: NSObject { into a set of header fields to add to a request. */ open func cookies(for url: URL) -> [HTTPCookie]? { - guard let host = url.host else { return nil } - return Array(self.syncQ.sync(execute: {allCookies}).values.filter{ $0.domain == host }) + guard let host = url.host?.lowercased() else { return nil } + return Array(self.syncQ.sync(execute: {allCookies}).values.filter{ $0.validFor(host: host) }) } /*! @@ -293,17 +293,17 @@ open class HTTPCookieStorage: NSObject { guard cookieAcceptPolicy != .never else { return } //if the urls don't have a host, we cannot do anything - guard let urlHost = url?.host else { return } + guard let urlHost = url?.host?.lowercased() else { return } if mainDocumentURL != nil && cookieAcceptPolicy == .onlyFromMainDocumentDomain { - guard let mainDocumentHost = mainDocumentURL?.host else { return } + guard let mainDocumentHost = mainDocumentURL?.host?.lowercased() else { return } //the url.host must be a suffix of manDocumentURL.host, this is based on Darwin's behaviour guard mainDocumentHost.hasSuffix(urlHost) else { return } } //save only those cookies whose domain matches with the url.host - let validCookies = cookies.filter { urlHost == $0.domain } + let validCookies = cookies.filter { $0.validFor(host: urlHost) } for cookie in validCookies { setCookie(cookie) } @@ -334,6 +334,28 @@ public extension Notification.Name { } extension HTTPCookie { + internal func validFor(host: String) -> Bool { + // RFC6265 - HTTP State Management Mechanism + // https://tools.ietf.org/html/rfc6265#section-5.1.3 + // + // 5.1.3. Domain Matching + // A string domain-matches a given domain string if at least one of the + // following conditions hold: + // + // 1) The domain string and the string are identical. (Note that both + // the domain string and the string will have been canonicalized to + // lower case at this point.) + // + // 2) All of the following conditions hold: + // * The domain string is a suffix of the string. + // * The last character of the string that is not included in the + // domain string is a %x2E (".") character. + // * The string is a host name (i.e., not an IP address). + + let dotlessDomain = domain.first == "." ? String(domain.suffix(domain.count - 1)) : domain + return dotlessDomain == host || (domain.first == "." && host.hasSuffix(domain)) + } + internal func persistableDictionary() -> [String: Any] { var properties: [String: Any] = [:] properties[HTTPCookiePropertyKey.name.rawValue] = name diff --git a/TestFoundation/TestHTTPCookieStorage.swift b/TestFoundation/TestHTTPCookieStorage.swift index 4289782f4a..98072a4b3d 100644 --- a/TestFoundation/TestHTTPCookieStorage.swift +++ b/TestFoundation/TestHTTPCookieStorage.swift @@ -26,6 +26,7 @@ class TestHTTPCookieStorage: XCTestCase { ("test_cookiesForURLWithMainDocumentURL", test_cookiesForURLWithMainDocumentURL), ("test_cookieInXDGSpecPath", test_cookieInXDGSpecPath), ("test_descriptionCookie", test_descriptionCookie), + ("test_cookieDomainMatching", test_cookieDomainMatching), ] } @@ -89,6 +90,11 @@ class TestHTTPCookieStorage: XCTestCase { descriptionCookie(with: .groupContainer("test")) } + func test_cookieDomainMatching() { + cookieDomainMatching(with: .shared) + cookieDomainMatching(with: .groupContainer("test")) + } + func getStorage(for type: _StorageType) -> HTTPCookieStorage { switch type { case .shared: @@ -272,6 +278,49 @@ class TestHTTPCookieStorage: XCTestCase { XCTAssertEqual(storage.description, "") } + func cookieDomainMatching(with storageType: _StorageType) { + let storage = getStorage(for: storageType) + + let simpleCookie1 = HTTPCookie(properties: [ // swift.org domain only + .name: "TestCookie1", + .value: "TestValue1", + .path: "/", + .domain: "swift.org", + ])! + + storage.setCookie(simpleCookie1) + + let simpleCookie2 = HTTPCookie(properties: [ // *.swift.org + .name: "TestCookie2", + .value: "TestValue2", + .path: "/", + .domain: ".SWIFT.org", + ])! + + storage.setCookie(simpleCookie2) + + let simpleCookie3 = HTTPCookie(properties: [ // bugs.swift.org + .name: "TestCookie3", + .value: "TestValue3", + .path: "/", + .domain: "bugs.swift.org", + ])! + + storage.setCookie(simpleCookie3) + XCTAssertEqual(storage.cookies!.count, 3) + + let swiftOrgUrl = URL(string: "https://swift.ORG")! + let ciSwiftOrgUrl = URL(string: "https://CI.swift.ORG")! + let bugsSwiftOrgUrl = URL(string: "https://BUGS.swift.org")! + let exampleComUrl = URL(string: "http://www.example.com")! + let superSwiftOrgUrl = URL(string: "https://superswift.org")! + XCTAssertEqual(storage.cookies(for: swiftOrgUrl)!.count, 2) + XCTAssertEqual(storage.cookies(for: ciSwiftOrgUrl)!.count, 1) + XCTAssertEqual(storage.cookies(for: bugsSwiftOrgUrl)!.count, 2) + XCTAssertEqual(storage.cookies(for: exampleComUrl)!.count, 0) + XCTAssertEqual(storage.cookies(for: superSwiftOrgUrl)!.count, 0) + } + func test_cookieInXDGSpecPath() { #if !os(Android) && !DARWIN_COMPATIBILITY_TESTS // No XDG on native Foundation //Test without setting the environment variable From a637cd9ce70914b0d6f9a52f3478f8f0a3bdc473 Mon Sep 17 00:00:00 2001 From: Maksim Orlovich Date: Fri, 13 Jul 2018 12:36:24 -0500 Subject: [PATCH 2/2] Improve domain matching routine per the recommendation on PR. - Also update unit tests to be more accurate in testing domain matching --- Foundation/HTTPCookieStorage.swift | 4 ++-- TestFoundation/TestHTTPCookieStorage.swift | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Foundation/HTTPCookieStorage.swift b/Foundation/HTTPCookieStorage.swift index e64defa367..b246a91588 100644 --- a/Foundation/HTTPCookieStorage.swift +++ b/Foundation/HTTPCookieStorage.swift @@ -352,8 +352,8 @@ extension HTTPCookie { // domain string is a %x2E (".") character. // * The string is a host name (i.e., not an IP address). - let dotlessDomain = domain.first == "." ? String(domain.suffix(domain.count - 1)) : domain - return dotlessDomain == host || (domain.first == "." && host.hasSuffix(domain)) + guard domain.hasPrefix(".") else { return host == domain } + return host == domain.dropFirst() || host.hasSuffix(domain) } internal func persistableDictionary() -> [String: Any] { diff --git a/TestFoundation/TestHTTPCookieStorage.swift b/TestFoundation/TestHTTPCookieStorage.swift index 98072a4b3d..bbbe1fcaa2 100644 --- a/TestFoundation/TestHTTPCookieStorage.swift +++ b/TestFoundation/TestHTTPCookieStorage.swift @@ -314,11 +314,11 @@ class TestHTTPCookieStorage: XCTestCase { let bugsSwiftOrgUrl = URL(string: "https://BUGS.swift.org")! let exampleComUrl = URL(string: "http://www.example.com")! let superSwiftOrgUrl = URL(string: "https://superswift.org")! - XCTAssertEqual(storage.cookies(for: swiftOrgUrl)!.count, 2) - XCTAssertEqual(storage.cookies(for: ciSwiftOrgUrl)!.count, 1) - XCTAssertEqual(storage.cookies(for: bugsSwiftOrgUrl)!.count, 2) - XCTAssertEqual(storage.cookies(for: exampleComUrl)!.count, 0) - XCTAssertEqual(storage.cookies(for: superSwiftOrgUrl)!.count, 0) + XCTAssertEqual(Set(storage.cookies(for: swiftOrgUrl)!), Set([simpleCookie1, simpleCookie2])) + XCTAssertEqual(storage.cookies(for: ciSwiftOrgUrl)!, [simpleCookie2]) + XCTAssertEqual(Set(storage.cookies(for: bugsSwiftOrgUrl)!), Set([simpleCookie2, simpleCookie3])) + XCTAssertEqual(storage.cookies(for: exampleComUrl)!, []) + XCTAssertEqual(storage.cookies(for: superSwiftOrgUrl)!, []) } func test_cookieInXDGSpecPath() {