Skip to content

Commit 11d3338

Browse files
drodriguezdrodriguez
committed
Improve cookie parsing algorithm.
While the previous parser dealt with commas, some cases were not correctly handled, like when a comma was in the last attribute before a second cookie. Also trim the names of the cookies to allow spaces before the equal.
1 parent 000c62b commit 11d3338

File tree

2 files changed

+118
-37
lines changed

2 files changed

+118
-37
lines changed

‎Foundation/HTTPCookie.swift

Lines changed: 87 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -412,30 +412,74 @@ open class HTTPCookie : NSObject {
412412
/// - Returns: An array of HTTPCookie objects
413413
open class func cookies(withResponseHeaderFields headerFields: [String : String], for URL: URL) -> [HTTPCookie] {
414414

415-
//HTTP Cookie parsing based on RFC 6265: https://tools.ietf.org/html/rfc6265
416-
//Though RFC6265 suggests that multiple cookies cannot be folded into a single Set-Cookie field, this is
417-
//pretty common. It also suggests that commas and semicolons among other characters, cannot be a part of
415+
// HTTP Cookie parsing based on RFC 6265: https://tools.ietf.org/html/rfc6265
416+
// Though RFC6265 suggests that multiple cookies cannot be folded into a single Set-Cookie field, this is
417+
// pretty common. It also suggests that commas and semicolons among other characters, cannot be a part of
418418
// names and values. This implementation takes care of multiple cookies in the same field, however it doesn't
419-
//support commas and semicolons in names and values(except for dates)
419+
// support commas and semicolons in names and values(except for dates)
420420

421421
guard let cookies: String = headerFields["Set-Cookie"] else { return [] }
422422

423-
let nameValuePairs = cookies.components(separatedBy: ";") //split the name/value and attribute/value pairs
424-
.map({$0.trim()}) //trim whitespaces
425-
.map({removeCommaFromDate($0)}) //get rid of commas in dates
426-
.flatMap({$0.components(separatedBy: ",")}) //cookie boundaries are marked by commas
427-
.map({$0.trim()}) //trim again
428-
.filter({$0.caseInsensitiveCompare("HTTPOnly") != .orderedSame}) //we don't use HTTPOnly, do we?
429-
.flatMap({createNameValuePair(pair: $0)}) //create Name and Value properties
423+
var httpCookies: [HTTPCookie] = []
430424

431-
//mark cookie boundaries in the name-value array
432-
var cookieIndices = (0..<nameValuePairs.count).filter({nameValuePairs[$0].hasPrefix("Name")})
433-
cookieIndices.append(nameValuePairs.count)
425+
// Let's do old school parsing, which should allow us to handle the
426+
// embedded commas correctly.
427+
var idx: String.Index = cookies.startIndex
428+
let end: String.Index = cookies.endIndex
429+
while idx < end {
430+
// Skip leading spaces.
431+
while idx < end && cookies[idx].isSpace {
432+
idx = cookies.index(after: idx)
433+
}
434+
let cookieStartIdx: String.Index = idx
435+
var cookieEndIdx: String.Index = idx
436+
437+
while idx < end {
438+
// Scan to the next comma, but check that the comma is not a
439+
// legal comma in a value, by looking ahead for the token,
440+
// which indicates the comma was separating cookies.
441+
let cookiesRest = cookies[idx..<end]
442+
if let commaIdx = cookiesRest.firstIndex(of: ",") {
443+
// We are looking for WSP* TOKEN_CHAR+ WSP* '='
444+
var lookaheadIdx = cookies.index(after: commaIdx)
445+
// Skip whitespace
446+
while lookaheadIdx < end && cookies[lookaheadIdx].isSpace {
447+
lookaheadIdx = cookies.index(after: lookaheadIdx)
448+
}
449+
// Skip over the token characters
450+
var tokenLength = 0
451+
while lookaheadIdx < end && cookies[lookaheadIdx].isTokenCharacter {
452+
lookaheadIdx = cookies.index(after: lookaheadIdx)
453+
tokenLength += 1
454+
}
455+
// Skip whitespace
456+
while lookaheadIdx < end && cookies[lookaheadIdx].isSpace {
457+
lookaheadIdx = cookies.index(after: lookaheadIdx)
458+
}
459+
// Check there was a token, and there's an equals.
460+
if lookaheadIdx < end && cookies[lookaheadIdx] == "=" && tokenLength > 0 {
461+
// We found a token after the comma, this is a cookie
462+
// separator, and not an embedded comma.
463+
idx = cookies.index(after: commaIdx)
464+
cookieEndIdx = commaIdx
465+
break
466+
}
467+
// Otherwise, keep scanning from the comma.
468+
idx = cookies.index(after: commaIdx)
469+
cookieEndIdx = idx
470+
} else {
471+
// No more commas, skip to the end.
472+
idx = end
473+
cookieEndIdx = end
474+
break
475+
}
476+
}
434477

435-
//bake the cookies
436-
var httpCookies: [HTTPCookie] = []
437-
for i in 0..<cookieIndices.count-1 {
438-
if let aCookie = createHttpCookie(url: URL, pairs: nameValuePairs[cookieIndices[i]..<cookieIndices[i+1]]) {
478+
if cookieEndIdx <= cookieStartIdx {
479+
continue
480+
}
481+
482+
if let aCookie = createHttpCookie(url: URL, cookie: cookies[cookieStartIdx..<cookieEndIdx]) {
439483
httpCookies.append(aCookie)
440484
}
441485
}
@@ -444,14 +488,16 @@ open class HTTPCookie : NSObject {
444488
}
445489

446490
//Bake a cookie
447-
private class func createHttpCookie(url: URL, pairs: ArraySlice<String>) -> HTTPCookie? {
448-
var properties: [HTTPCookiePropertyKey : Any] = [:]
491+
private class func createHttpCookie(url: URL, cookie: Substring) -> HTTPCookie? {
492+
let pairs = cookie.components(separatedBy: ";") // split the name/value and attribute/value pairs
493+
.map { $0.trim() } // trim whitespaces
494+
.filter { $0.caseInsensitiveCompare("HTTPOnly") != .orderedSame } // we don't use HTTPOnly, do we?
495+
.flatMap(HTTPCookie.createNameValuePair) // create Name and Value properties
496+
497+
var properties: [HTTPCookiePropertyKey : String] = [:]
449498
for pair in pairs {
450499
let name = pair.components(separatedBy: "=")[0]
451-
var value = pair.components(separatedBy: "\(name)=")[1] //a value can have an "="
452-
if canonicalize(name) == .expires {
453-
value = value.unmaskCommas() //re-insert the comma
454-
}
500+
let value = pair.components(separatedBy: "\(name)=")[1] //a value can have an "="
455501
properties[canonicalize(name)] = value
456502
}
457503

@@ -468,14 +514,6 @@ open class HTTPCookie : NSObject {
468514
return HTTPCookie(properties: properties)
469515
}
470516

471-
//we pass this to a map()
472-
private class func removeCommaFromDate(_ value: String) -> String {
473-
if value.hasPrefix("Expires") || value.hasPrefix("expires") {
474-
return value.maskCommas()
475-
}
476-
return value
477-
}
478-
479517
//These cookie attributes are defined in RFC 6265 and 2965(which is obsolete)
480518
//HTTPCookie supports these
481519
private class func isCookieAttribute(_ string: String) -> Bool {
@@ -497,7 +535,7 @@ open class HTTPCookie : NSObject {
497535
let name = pair.components(separatedBy: "=")[0]
498536
let value = pair.components(separatedBy: "\(name)=")[1]
499537
if !isCookieAttribute(name) {
500-
return ["Name=\(name)", "Value=\(value)"]
538+
return ["Name=\(name.trim())", "Value=\(value)"]
501539
}
502540
return [pair]
503541
}
@@ -654,12 +692,24 @@ fileprivate extension String {
654692
func trim() -> String {
655693
return self.trimmingCharacters(in: .whitespacesAndNewlines)
656694
}
695+
}
657696

658-
func maskCommas() -> String {
659-
return self.replacingOccurrences(of: ",", with: "&comma")
697+
fileprivate extension Character {
698+
var isSpace: Bool {
699+
return self == " " || self == "\t" || self == "\n" || self == "\r"
660700
}
661701

662-
func unmaskCommas() -> String {
663-
return self.replacingOccurrences(of: "&comma", with: ",")
702+
var isTokenCharacter: Bool {
703+
guard let asciiValue = self.asciiValue else {
704+
return false
705+
}
706+
707+
// CTL, 0-31 and DEL (127)
708+
if asciiValue <= 31 || asciiValue >= 127 {
709+
return false
710+
}
711+
712+
let nonTokenCharacters = "()<>@,;:\\\"/[]?={} \t"
713+
return !nonTokenCharacters.contains(self)
664714
}
665715
}

‎TestFoundation/TestHTTPCookie.swift

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ class TestHTTPCookie: XCTestCase {
1919
("test_cookiesWithResponseHeaderNoDomain", test_cookiesWithResponseHeaderNoDomain),
2020
("test_cookiesWithResponseHeaderNoPathNoDomain", test_cookiesWithResponseHeaderNoPathNoDomain),
2121
("test_cookieExpiresDateFormats", test_cookieExpiresDateFormats),
22+
("test_cookiesWithExpiresAsLastAttribute", test_cookiesWithExpiresAsLastAttribute),
23+
("test_cookiesWithResponseHeaderTrimsNames", test_cookiesWithResponseHeaderTrimsNames),
2224
("test_httpCookieWithSubstring", test_httpCookieWithSubstring),
2325
]
2426
}
@@ -171,6 +173,35 @@ class TestHTTPCookie: XCTestCase {
171173
XCTAssertEqual(cookies[0].path, "/")
172174
}
173175

176+
func test_cookiesWithExpiresAsLastAttribute() throws {
177+
let cookies = HTTPCookie.cookies(withResponseHeaderFields: [
178+
"Set-Cookie": "AAA=111; path=/; domain=.example.com; expires=Sun, 16-Aug-2025 22:39:54 GMT, BBB=222; path=/; domain=.example.com; HttpOnly; expires=Sat, 15-Feb-2014 22:39:54 GMT"
179+
], for: try URL(string: "http://www.example.com/").unwrapped())
180+
XCTAssertEqual(cookies.count, 2)
181+
XCTAssertEqual(cookies[0].name, "AAA")
182+
XCTAssertEqual(cookies[1].name, "BBB")
183+
}
184+
185+
func test_cookiesWithResponseHeaderTrimsNames() throws {
186+
do {
187+
let cookies = HTTPCookie.cookies(withResponseHeaderFields: [
188+
"Set-Cookie": "AAA =1; path=/; domain=.example.com; expires=Sun, 16-Aug-2025 22:39:54 GMT, BBB=2; path=/; domain=.example.com; HttpOnly; expires=Sat, 15-Feb-2014 22:39:54 GMT"
189+
], for: try URL(string: "http://www.example.com/").unwrapped())
190+
XCTAssertEqual(cookies.count, 2)
191+
XCTAssertEqual(cookies[0].name, "AAA")
192+
XCTAssertEqual(cookies[1].name, "BBB")
193+
}
194+
195+
do {
196+
let cookies = HTTPCookie.cookies(withResponseHeaderFields: [
197+
"Set-Cookie": " AAA=1; path=/; domain=.example.com; expires=Sun, 16-Aug-2025 22:39:54 GMT, BBB=2; path=/; domain=.example.com; HttpOnly; expires=Sat, 15-Feb-2014 22:39:54 GMT"
198+
], for: try URL(string: "http://www.example.com/").unwrapped())
199+
XCTAssertEqual(cookies.count, 2)
200+
XCTAssertEqual(cookies[0].name, "AAA")
201+
XCTAssertEqual(cookies[1].name, "BBB")
202+
}
203+
}
204+
174205
func test_cookieExpiresDateFormats() {
175206
let testDate = Date(timeIntervalSince1970: 1577881800)
176207
let cookieString =

0 commit comments

Comments
 (0)