From 7f0c573938786b9637cc297e55ff9939882dc7c5 Mon Sep 17 00:00:00 2001 From: Lily Vulcano Date: Mon, 12 Aug 2019 12:19:35 -0700 Subject: [PATCH] [5.0.3] Backport 5.1 changes to HTTPCookie parsing. Fixes https://bugs.swift.org/browse/SR-11294 --- Foundation/HTTPCookie.swift | 292 ++++++++++++++++++++-------- TestFoundation/TestHTTPCookie.swift | 12 +- TestFoundation/TestURL.swift | 2 +- 3 files changed, 223 insertions(+), 83 deletions(-) diff --git a/Foundation/HTTPCookie.swift b/Foundation/HTTPCookie.swift index a337bee981..87ea4bfbda 100644 --- a/Foundation/HTTPCookie.swift +++ b/Foundation/HTTPCookie.swift @@ -17,14 +17,6 @@ public struct HTTPCookiePropertyKey : RawRepresentable, Equatable, Hashable { public init(rawValue: String) { self.rawValue = rawValue } - - public var hashValue: Int { - return self.rawValue.hashValue - } - - public static func ==(_ lhs: HTTPCookiePropertyKey, _ rhs: HTTPCookiePropertyKey) -> Bool { - return lhs.rawValue == rhs.rawValue - } } extension HTTPCookiePropertyKey { @@ -71,6 +63,28 @@ extension HTTPCookiePropertyKey { internal static let created = HTTPCookiePropertyKey(rawValue: "Created") } +internal extension HTTPCookiePropertyKey { + static let httpOnly = HTTPCookiePropertyKey(rawValue: "HttpOnly") + + static private let _setCookieAttributes: [String: HTTPCookiePropertyKey] = { + // Only some attributes are valid in the Set-Cookie header. + let validProperties: [HTTPCookiePropertyKey] = [ + .expires, .maximumAge, .domain, .path, .secure, .comment, + .commentURL, .discard, .port, .version, .httpOnly + ] + let canonicalNames = validProperties.map { $0.rawValue.lowercased() } + return Dictionary(uniqueKeysWithValues: zip(canonicalNames, validProperties)) + }() + + init?(attributeName: String) { + let canonical = attributeName.lowercased() + switch HTTPCookiePropertyKey._setCookieAttributes[canonical] { + case let property?: self = property + case nil: return nil + } + } +} + /// `HTTPCookie` represents an http cookie. /// /// An `HTTPCookie` instance represents a single http cookie. It is @@ -125,11 +139,6 @@ open class HTTPCookie : NSObject { static let _allFormatters: [DateFormatter] = [_formatter1, _formatter2, _formatter3] - static let _attributes: [HTTPCookiePropertyKey] - = [.name, .value, .originURL, .version, .domain, - .path, .secure, .expires, .comment, .commentURL, - .discard, .maximumAge, .port] - /// Initialize a HTTPCookie object with a dictionary of parameters /// /// - Parameter properties: The dictionary of properties to be used to @@ -255,10 +264,16 @@ open class HTTPCookie : NSObject { /// - Experiment: This is a draft API currently under consideration for official import into Foundation as a suitable alternative /// - Note: Since this API is under consideration it may be either removed or revised in the near future public init?(properties: [HTTPCookiePropertyKey : Any]) { + func stringValue(_ strVal: Any?) -> String? { + if let subStr = strVal as? Substring { + return String(subStr) + } + return strVal as? String + } guard - let path = properties[.path] as? String, - let name = properties[.name] as? String, - let value = properties[.value] as? String + let path = stringValue(properties[.path]), + let name = stringValue(properties[.name]), + let value = stringValue(properties[.value]) else { return nil } @@ -313,7 +328,7 @@ open class HTTPCookie : NSObject { } var expDate: Date? = nil - // Maximum-Age is prefered over expires-Date but only version 1 cookies use Maximum-Age + // Maximum-Age is preferred over expires-Date but only version 1 cookies use Maximum-Age if let maximumAge = properties[.maximumAge] as? String, let secondsFromNow = Int(maximumAge) { if version == 1 { @@ -344,8 +359,12 @@ open class HTTPCookie : NSObject { } else { _commentURL = nil } - _HTTPOnly = false + if let httpOnlyString = properties[.httpOnly] as? String { + _HTTPOnly = httpOnlyString == "TRUE" + } else { + _HTTPOnly = false + } _properties = [ .created : Date().timeIntervalSinceReferenceDate, // Cocoa Compatibility @@ -404,34 +423,78 @@ open class HTTPCookie : NSObject { /// This method will ignore irrelevant header fields so /// you can pass a dictionary containing data other than cookie data. /// - Parameter headerFields: The response header fields to check for cookies. - /// - Parameter URL: The URL that the cookies came from - relevant to how the cookies are interpeted. + /// - Parameter URL: The URL that the cookies came from - relevant to how the cookies are interpreted. /// - Returns: An array of HTTPCookie objects open class func cookies(withResponseHeaderFields headerFields: [String : String], for URL: URL) -> [HTTPCookie] { - //HTTP Cookie parsing based on RFC 6265: https://tools.ietf.org/html/rfc6265 - //Though RFC6265 suggests that multiple cookies cannot be folded into a single Set-Cookie field, this is - //pretty common. It also suggests that commas and semicolons among other characters, cannot be a part of + // HTTP Cookie parsing based on RFC 6265: https://tools.ietf.org/html/rfc6265 + // Though RFC6265 suggests that multiple cookies cannot be folded into a single Set-Cookie field, this is + // pretty common. It also suggests that commas and semicolons among other characters, cannot be a part of // names and values. This implementation takes care of multiple cookies in the same field, however it doesn't - //support commas and semicolons in names and values(except for dates) + // support commas and semicolons in names and values(except for dates) guard let cookies: String = headerFields["Set-Cookie"] else { return [] } - let nameValuePairs = cookies.components(separatedBy: ";") //split the name/value and attribute/value pairs - .map({$0.trim()}) //trim whitespaces - .map({removeCommaFromDate($0)}) //get rid of commas in dates - .flatMap({$0.components(separatedBy: ",")}) //cookie boundaries are marked by commas - .map({$0.trim()}) //trim again - .filter({$0.caseInsensitiveCompare("HTTPOnly") != .orderedSame}) //we don't use HTTPOnly, do we? - .flatMap({createNameValuePair(pair: $0)}) //create Name and Value properties + var httpCookies: [HTTPCookie] = [] + + // Let's do old school parsing, which should allow us to handle the + // embedded commas correctly. + var idx: String.Index = cookies.startIndex + let end: String.Index = cookies.endIndex + while idx < end { + // Skip leading spaces. + while idx < end && cookies[idx].isSpace { + idx = cookies.index(after: idx) + } + let cookieStartIdx: String.Index = idx + var cookieEndIdx: String.Index = idx + + while idx < end { + // Scan to the next comma, but check that the comma is not a + // legal comma in a value, by looking ahead for the token, + // which indicates the comma was separating cookies. + let cookiesRest = cookies[idx..) -> HTTPCookie? { + private class func createHttpCookie(url: URL, cookie: String) -> HTTPCookie? { var properties: [HTTPCookiePropertyKey : Any] = [:] - for pair in pairs { - let name = pair.components(separatedBy: "=")[0] - var value = pair.components(separatedBy: "\(name)=")[1] //a value can have an "=" - if canonicalize(name) == .expires { - value = value.unmaskCommas() //re-insert the comma + let scanner = Scanner(string: cookie) + + guard let nameValuePair = scanner.scanUpToString(";") else { + // if the scanner does not read anything, there's no cookie + return nil + } + + guard case (let name?, let value?) = splitNameValue(nameValuePair) else { + return nil + } + + properties[.name] = name + properties[.value] = value + properties[.originURL] = url + + while scanner.scanString(";") != nil { + if let attribute = scanner.scanUpToString(";") { + switch splitNameValue(attribute) { + case (nil, _): + // ignore empty attribute names + break + case (let name?, nil): + switch HTTPCookiePropertyKey(attributeName: name) { + case .secure?: + properties[.secure] = "TRUE" + case .discard?: + properties[.discard] = "TRUE" + case .httpOnly?: + properties[.httpOnly] = "TRUE" + default: + // ignore unknown attributes + break + } + case (let name?, let value?): + switch HTTPCookiePropertyKey(attributeName: name) { + case .comment?: + properties[.comment] = value + case .commentURL?: + properties[.commentURL] = value + case .domain?: + properties[.domain] = value + case .maximumAge?: + properties[.maximumAge] = value + case .path?: + properties[.path] = value + case .port?: + properties[.port] = value + case .version?: + properties[.version] = value + case .expires?: + properties[.expires] = value + default: + // ignore unknown attributes + break + } + } } - properties[canonicalize(name)] = value } - // If domain wasn't provided, extract it from the URL - if properties[.domain] == nil { + if let domain = properties[.domain] as? String { + // The provided domain string has to be prepended with a dot, + // because the domain field indicates that it can be sent + // subdomains of the domain (but only if it is not an IP address). + if (!domain.hasPrefix(".") && !isIPv4Address(domain)) { + properties[.domain] = ".\(domain)" + } + } else { + // If domain wasn't provided, extract it from the URL. No dots in + // this case, only exact matching. properties[.domain] = url.host } + // Always lowercase the domain. + if let domain = properties[.domain] as? String { + properties[.domain] = domain.lowercased() + } - //the default Path is "/" - if properties[.path] == nil { + // the default Path is "/" + if let path = properties[.path] as? String, path.first == "/" { + // do nothing + } else { properties[.path] = "/" } return HTTPCookie(properties: properties) } - //we pass this to a map() - private class func removeCommaFromDate(_ value: String) -> String { - if value.hasPrefix("Expires") || value.hasPrefix("expires") { - return value.maskCommas() + private class func splitNameValue(_ pair: String) -> (name: String?, value: String?) { + let scanner = Scanner(string: pair) + + guard let name = scanner.scanUpToString("=")?.trim(), + !name.isEmpty else { + // if the scanner does not read anything, or the trimmed name is + // empty, there's no name=value + return (nil, nil) } - return value - } - //These cookie attributes are defined in RFC 6265 and 2965(which is obsolete) - //HTTPCookie supports these - private class func isCookieAttribute(_ string: String) -> Bool { - return _attributes.first(where: {$0.rawValue.caseInsensitiveCompare(string) == .orderedSame}) != nil - } + guard scanner.scanString("=") != nil else { + // if the scanner does not find =, there's no value + return (name, nil) + } - //Cookie attribute names are case-insensitive as per RFC6265: https://tools.ietf.org/html/rfc6265 - //but HTTPCookie needs only the first letter of each attribute in uppercase - private class func canonicalize(_ name: String) -> HTTPCookiePropertyKey { - let idx = _attributes.index(where: {$0.rawValue.caseInsensitiveCompare(name) == .orderedSame})! - return _attributes[idx] + let location = scanner.scanLocation + let value = String(pair[pair.index(pair.startIndex, offsetBy: location).. [String] { - if pair.caseInsensitiveCompare(HTTPCookiePropertyKey.secure.rawValue) == .orderedSame { - return ["Secure=TRUE"] - } - let name = pair.components(separatedBy: "=")[0] - let value = pair.components(separatedBy: "\(name)=")[1] - if !isCookieAttribute(name) { - return ["Name=\(name)", "Value=\(value)"] - } - return [pair] + private class func isIPv4Address(_ string: String) -> Bool { + var x = in_addr() + return inet_pton(AF_INET, string, &x) == 1 } /// Returns a dictionary representation of the receiver. @@ -573,7 +692,7 @@ open class HTTPCookie : NSObject { /// /// Cookies may be marked secure by a server (or by a javascript). /// Cookies marked as such must only be sent via an encrypted connection to - /// trusted servers (i.e. via SSL or TLS), and should not be delievered to any + /// trusted servers (i.e. via SSL or TLS), and should not be delivered to any /// javascript applications to prevent cross-site scripting vulnerabilities. open var isSecure: Bool { return _secure @@ -650,13 +769,24 @@ fileprivate extension String { func trim() -> String { return self.trimmingCharacters(in: .whitespacesAndNewlines) } +} - func maskCommas() -> String { - return self.replacingOccurrences(of: ",", with: "&comma") +fileprivate extension Character { + var isSpace: Bool { + return self == " " || self == "\t" || self == "\n" || self == "\r" } - func unmaskCommas() -> String { - return self.replacingOccurrences(of: "&comma", with: ",") + var isTokenCharacter: Bool { + guard let asciiValue = self.asciiValue else { + return false + } + + // CTL, 0-31 and DEL (127) + if asciiValue <= 31 || asciiValue >= 127 { + return false + } + + let nonTokenCharacters = "()<>@,;:\\\"/[]?={} \t" + return !nonTokenCharacters.contains(self) } } - diff --git a/TestFoundation/TestHTTPCookie.swift b/TestFoundation/TestHTTPCookie.swift index aee767f6fd..74cabda112 100644 --- a/TestFoundation/TestHTTPCookie.swift +++ b/TestFoundation/TestHTTPCookie.swift @@ -19,8 +19,18 @@ class TestHTTPCookie: XCTestCase { ("test_cookiesWithResponseHeaderNoDomain", test_cookiesWithResponseHeaderNoDomain), ("test_cookiesWithResponseHeaderNoPathNoDomain", test_cookiesWithResponseHeaderNoPathNoDomain), ("test_cookieExpiresDateFormats", test_cookieExpiresDateFormats), + ("test_pathEndingWithSemicolonMustNotCrash", test_pathEndingWithSemicolonMustNotCrash), ] } + + func test_pathEndingWithSemicolonMustNotCrash() throws { + let cookies = HTTPCookie.cookies(withResponseHeaderFields: ["Set-Cookie": "foo=bar;"], for: URL(string: "https://foo.bar")!) + XCTAssertEqual(cookies.count, 1) + let cookie = try cookies.first.unwrapped() + XCTAssertEqual(cookie.name, "foo") + XCTAssertEqual(cookie.value, "bar") + XCTAssertEqual(cookie.domain, "foo.bar") + } func test_BasicConstruction() { let invalidVersionZeroCookie = HTTPCookie(properties: [ @@ -187,7 +197,7 @@ class TestHTTPCookie: XCTestCase { XCTAssertEqual(cookies.count, 3) cookies.forEach { cookie in XCTAssertEqual(cookie.expiresDate, testDate) - XCTAssertEqual(cookie.domain, "swift.org") + XCTAssertEqual(cookie.domain, ".swift.org") XCTAssertEqual(cookie.path, "/") } } diff --git a/TestFoundation/TestURL.swift b/TestFoundation/TestURL.swift index c6396eb6e6..df04a46065 100644 --- a/TestFoundation/TestURL.swift +++ b/TestFoundation/TestURL.swift @@ -39,7 +39,7 @@ private func getTestData() -> [Any]? { XCTFail("Unable to deserialize property list data") return nil } - #if swift(>=5.0) + #if compiler(>=5.0) guard let parsingTests = testRoot[kURLTestParsingTestsKey] as? [Any] else { XCTFail("Unable to create the parsingTests dictionary") return nil