Skip to content

Commit

Permalink
RFC Compliant Cookie Parsing (#2540)
Browse files Browse the repository at this point in the history 
* Improve cookie parsing test

* Strictly conform to RFC2616

* Add invalid cookie test cases

* Remove unnecessary vars

* Fix comment typo

* Restore test case

Co-authored-by: Tim Condon <0xTim@users.noreply.github.com>
  • Loading branch information
@t-ae @0xTim
t-ae and 0xTim committed Mar 11, 2021
1 parent 43ec9b4 commit 062a408
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 36 deletions.
44 changes: 29 additions & 15 deletions Sources/Vapor/HTTP/Headers/HTTPHeaders+Directive.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ extension HTTPHeaders {
let character = self.current[index]
if character == .equals {
return index
} else if !character.isDirectiveKey {
} else if !character.isTokenCharacter {
return nil
}
}
Expand Down Expand Up @@ -230,27 +230,41 @@ private extension Character {
static var equals: Self {
.init("=")
}
static var dash: Self {
.init("-")
}
static var comma: Self {
.init(",")
}
static var underscore: Self {
.init("_")
}
static var period: Self {
.init(".")
}
static var space: Self {
.init(" ")
}
static var percent: Self {
.init("%")

/// The characters defined in RFC2616.
///
/// Description from [RFC2616](https://tools.ietf.org/html/rfc2616):
///
/// separators = "(" | ")" | "<" | ">" | "@"
/// | "," | ";" | ":" | "\" | <">
/// | "/" | "[" | "]" | "?" | "="
/// | "{" | "}" | SP | HT
static var separators: [Self] {
["(", ")", "<", ">", "@", ",", ":", ";", "\\", "\"", "/", "[", "]", "?", "=", "{", "}", " ", "\t"]
}

var isDirectiveKey: Bool {
self.isLetter || self.isNumber || self == .dash || self == .underscore || self == .period || self == .percent

/// Check if this is valid character for token.
///
/// Description from [RFC2616](]https://tools.ietf.org/html/rfc2616):
///
/// token = 1*<any CHAR except CTLs or separators>
/// CHAR = <any US-ASCII character (octets 0 - 127)>
/// CTL = <any US-ASCII control character
/// (octets 0 - 31) and DEL (127)>
var isTokenCharacter: Bool {
guard let asciiValue = self.asciiValue else {
return false
}
guard asciiValue > 31 && asciiValue != 127 else {
return false
}
return !Self.separators.contains(self)
}
}

Expand Down
32 changes: 11 additions & 21 deletions Tests/VaporTests/HTTPHeaderTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,19 @@ final class HTTPHeaderTests: XCTestCase {

func testCookie_parsing() throws {
let headers = HTTPHeaders([
("cookie", "vapor-session=0FuTYcHmGw7Bz1G4HiF+EA==; _ga=GA1.1.500315824.1585154561; _gid=GA1.1.500224287.1585154561")
(
"cookie",
"""
vapor-session=0FuTYcHmGw7Bz1G4HiF+EA==; _ga=GA1.1.500315824.1585154561; _gid=GA1.1.500224287.1585154561; !#$%&'*+-.^_`~=symbols
"""
)
])
print(headers.cookie!.all.keys)
XCTAssertEqual(headers.cookie?["vapor-session"]?.string, "0FuTYcHmGw7Bz1G4HiF+EA==")
XCTAssertEqual(headers.cookie?["vapor-session"]?.sameSite, .lax)
XCTAssertEqual(headers.cookie?["_ga"]?.string, "GA1.1.500315824.1585154561")
XCTAssertEqual(headers.cookie?["_gid"]?.string, "GA1.1.500224287.1585154561")
XCTAssertEqual(headers.cookie?["!#$%&'*+-.^_`~"]?.string, "symbols")
}

// https://github.com/vapor/vapor/issues/2316
Expand All @@ -176,35 +183,18 @@ final class HTTPHeaderTests: XCTestCase {
XCTAssertEqual(headers.cookie?["vapor-session"]?.string, "ZFPQ46p3frNX52i3dM+JFlWbTxQX5rtGuQ5r7Gb6JUs=")
XCTAssertEqual(headers.cookie?["oauth2_consent_csrf"]?.string, "MTU4NjkzNzgwMnxEdi1CQkFFQ180SUFBUkFCRUFBQVB2LUNBQUVHYzNSeWFXNW5EQVlBQkdOemNtWUdjM1J5YVc1bkRDSUFJR1ExWVRnM09USmhOamRsWXpSbU4yRmhOR1UwTW1KaU5tRXpPRGczTmpjMHweHbVecAf193ev3_1Tcf60iY9jSsq5-IQxGTyoztRTfg==")
}

func testCookie_dotParsing() throws {
let headers = HTTPHeaders([
("cookie", "cookie_one=1;cookie.two=2")
])

XCTAssertEqual(headers.cookie?["cookie_one"]?.string, "1")
XCTAssertEqual(headers.cookie?["cookie.two"]?.string, "2")
}

func testCookie_percentParsing() throws {
let headers = HTTPHeaders([
("cookie", "cookie_one=1;cookie%40cookieCom=2;cookie_3=three")
])

XCTAssertEqual(headers.cookie?["cookie_one"]?.string, "1")
XCTAssertEqual(headers.cookie?["cookie%40cookieCom"]?.string, "2")
XCTAssertEqual(headers.cookie?["cookie_3"]?.string, "three")
}

func testCookie_invalidCookie() throws {
let headers = HTTPHeaders([
("cookie", "cookie_one=1;cookie\ntwo=2;cookie_three=3")
("cookie", "cookie_one=1;cookie\ntwo=2;cookie_three=3;cookie_④=4;cookie_fivé=5")
])

XCTAssertEqual(headers.cookie?.all.count, 2)
XCTAssertEqual(headers.cookie?["cookie_one"]?.string, "1")
XCTAssertNil(headers.cookie?["cookie\ntwo"])
XCTAssertEqual(headers.cookie?["cookie_three"]?.string, "3")
XCTAssertNil(headers.cookie?["cookie_④"])
XCTAssertNil(headers.cookie?["cookie_fivé"])
}

func testMediaTypeMainTypeCaseInsensitive() throws {
Expand Down

0 comments on commit 062a408

Please sign in to comment.