Redo HTTP cookie parsing using strptime

karwa · karwa · commit d790ad4b42db · 2021-12-03T00:03:40.000+01:00
diff --git a/Package.swift b/Package.swift
@@ -29,9 +29,11 @@ let package = Package(
         .package(url: "https://github.com/apple/swift-log.git", from: "1.4.0"),
     ],
     targets: [
+        .target(name: "CShims"),
         .target(
             name: "AsyncHTTPClient",
             dependencies: [
+                .target(name: "CShims"),
                 .product(name: "NIO", package: "swift-nio"),
                 .product(name: "NIOCore", package: "swift-nio"),
                 .product(name: "NIOPosix", package: "swift-nio"),
diff --git a/Sources/AsyncHTTPClient/FoundationExtensions.swift b/Sources/AsyncHTTPClient/FoundationExtensions.swift
@@ -0,0 +1,49 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the AsyncHTTPClient open source project
+//
+// Copyright (c) 2018-2019 Apple Inc. and the AsyncHTTPClient project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+// Extensions which provide better ergonomics when using Foundation types,
+// or by using Foundation APIs.
+
+import Foundation
+
+extension HTTPClient.Cookie {
+    /// The cookie's expiration date.
+    public var expires: Date? {
+        expires_timestamp.map { Date(timeIntervalSince1970: TimeInterval($0)) }
+    }
+
+    /// Create HTTP cookie.
+    ///
+    /// - parameters:
+    ///     - name: The name of the cookie.
+    ///     - value: The cookie's string value.
+    ///     - path: The cookie's path.
+    ///     - domain: The domain of the cookie, defaults to nil.
+    ///     - expires: The cookie's expiration date, defaults to nil.
+    ///     - maxAge: The cookie's age in seconds, defaults to nil.
+    ///     - httpOnly: Whether this cookie should be used by HTTP servers only, defaults to false.
+    ///     - secure: Whether this cookie should only be sent using secure channels, defaults to false.
+    public init(name: String, value: String, path: String = "/", domain: String? = nil, expires: Date? = nil, maxAge: Int? = nil, httpOnly: Bool = false, secure: Bool = false) {
+        self.init(
+            name: name,
+            value: value,
+            path: path,
+            domain: domain,
+            expires_timestamp: expires.map { Int64($0.timeIntervalSince1970) },
+            maxAge: maxAge,
+            httpOnly: httpOnly,
+            secure: secure
+        )
+    }
+}
diff --git a/Sources/AsyncHTTPClient/HTTPClient+HTTPCookie.swift b/Sources/AsyncHTTPClient/HTTPClient+HTTPCookie.swift
@@ -12,7 +12,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-import Foundation
 import NIOHTTP1
 
 extension HTTPClient {
@@ -26,8 +25,8 @@ extension HTTPClient {
         public var path: String
         /// The domain of the cookie.
         public var domain: String?
-        /// The cookie's expiration date.
-        public var expires: Date?
+        /// The cookie's expiration date, as a number of seconds since the Unix epoch.
+        var expires_timestamp: Int64?
         /// The cookie's age in seconds.
         public var maxAge: Int?
         /// Whether the cookie should only be sent to HTTP servers.
@@ -42,38 +41,29 @@ extension HTTPClient {
         ///     - defaultDomain: Default domain to use if cookie was sent without one.
         /// - returns: nil if the header is invalid.
         public init?(header: String, defaultDomain: String) {
-            let components = header.components(separatedBy: ";").map {
-                $0.trimmingCharacters(in: .whitespaces)
-            }
+            let components = header.split(separator: ";", omittingEmptySubsequences: false)
 
-            if components.isEmpty {
+            guard let keyValuePair = components.first?.trimmingWhitespace() else {
                 return nil
             }
-
-            let nameAndValue = components[0].split(separator: "=", maxSplits: 1, omittingEmptySubsequences: false).map {
-                $0.trimmingCharacters(in: .whitespaces)
-            }
-
-            guard nameAndValue.count == 2 else {
+            guard let (trimmedName, trimmedValue) = keyValuePair.parseKeyValuePair() else {
                 return nil
             }
-
-            self.name = nameAndValue[0]
-            self.value = nameAndValue[1].omittingQuotes()
-
-            guard !self.name.isEmpty else {
+            guard !trimmedName.isEmpty else {
                 return nil
             }
 
+            self.name = String(trimmedName)
+            self.value = String(trimmedValue.omittingQuotes())
             self.path = "/"
             self.domain = defaultDomain
-            self.expires = nil
+            self.expires_timestamp = nil
             self.maxAge = nil
             self.httpOnly = false
             self.secure = false
 
             for component in components[1...] {
-                switch self.parseComponent(component) {
+                switch component.parseCookieComponent() {
                 case (nil, nil):
                     continue
                 case ("path", .some(let value)):
@@ -84,27 +74,7 @@ extension HTTPClient {
                     guard let value = value else {
                         continue
                     }
-
-                    let formatter = DateFormatter()
-                    formatter.locale = Locale(identifier: "en_US")
-                    formatter.timeZone = TimeZone(identifier: "GMT")
-
-                    formatter.dateFormat = "EEE, dd MMM yyyy HH:mm:ss z"
-                    if let date = formatter.date(from: value) {
-                        self.expires = date
-                        continue
-                    }
-
-                    formatter.dateFormat = "EEE, dd-MMM-yy HH:mm:ss z"
-                    if let date = formatter.date(from: value) {
-                        self.expires = date
-                        continue
-                    }
-
-                    formatter.dateFormat = "EEE MMM d hh:mm:s yyyy"
-                    if let date = formatter.date(from: value) {
-                        self.expires = date
-                    }
+                    self.expires_timestamp = parseCookieTime(value)
                 case ("max-age", let value):
                     self.maxAge = value.flatMap(Int.init)
                 case ("secure", nil):
@@ -124,51 +94,111 @@ extension HTTPClient {
         ///     - value: The cookie's string value.
         ///     - path: The cookie's path.
         ///     - domain: The domain of the cookie, defaults to nil.
-        ///     - expires: The cookie's expiration date, defaults to nil.
+        ///     - expires_timestamp: The cookie's expiration date, as a number of seconds since the Unix epoch. defaults to nil.
         ///     - maxAge: The cookie's age in seconds, defaults to nil.
         ///     - httpOnly: Whether this cookie should be used by HTTP servers only, defaults to false.
         ///     - secure: Whether this cookie should only be sent using secure channels, defaults to false.
-        public init(name: String, value: String, path: String = "/", domain: String? = nil, expires: Date? = nil, maxAge: Int? = nil, httpOnly: Bool = false, secure: Bool = false) {
+        internal init(name: String, value: String, path: String = "/", domain: String? = nil, expires_timestamp: Int64? = nil, maxAge: Int? = nil, httpOnly: Bool = false, secure: Bool = false) {
             self.name = name
             self.value = value
             self.path = path
             self.domain = domain
-            self.expires = expires
+            self.expires_timestamp = expires_timestamp
             self.maxAge = maxAge
             self.httpOnly = httpOnly
             self.secure = secure
         }
+    }
+}
 
-        func parseComponent(_ component: String) -> (String?, String?) {
-            let nameAndValue = component.split(separator: "=", maxSplits: 1).map {
-                $0.trimmingCharacters(in: .whitespaces)
-            }
-            if nameAndValue.count == 2 {
-                return (nameAndValue[0].lowercased(), nameAndValue[1])
-            } else if nameAndValue.count == 1 {
-                return (nameAndValue[0].lowercased(), nil)
-            }
-            return (nil, nil)
-        }
+extension HTTPClient.Response {
+    /// List of HTTP cookies returned by the server.
+    public var cookies: [HTTPClient.Cookie] {
+        return self.headers["set-cookie"].compactMap { HTTPClient.Cookie(header: $0, defaultDomain: self.host) }
     }
 }
 
-extension String {
-    fileprivate func omittingQuotes() -> String {
+extension StringProtocol {
+    fileprivate func omittingQuotes() -> SubSequence {
         let dquote = "\""
-        if !hasPrefix(dquote) || !hasSuffix(dquote) {
-            return self
+        guard hasPrefix(dquote), hasSuffix(dquote) else {
+            return self[startIndex..<endIndex]
         }
+        return self.dropFirst().dropLast()
+    }
 
-        let begin = index(after: startIndex)
-        let end = index(before: endIndex)
-        return String(self[begin..<end])
+    fileprivate func trimmingWhitespace() -> SubSequence {
+        guard let start = self.firstIndex(where: { !$0.isWhitespace }) else {
+            return self[self.endIndex..<self.endIndex]
+        }
+        let end = self.lastIndex(where: { !$0.isWhitespace })!
+        return self[start...end]
+    }
+
+    /// Splits this string at the first "=" sign, and trims whitespace from each side.
+    fileprivate func parseKeyValuePair() -> (key: SubSequence, value: SubSequence)? {
+        guard let keyValueSeparator = self.firstIndex(of: "=") else {
+            return nil
+        }
+        let trimmedName = self[..<keyValueSeparator].trimmingWhitespace()
+        let trimmedValue = self[self.index(after: keyValueSeparator)...].trimmingWhitespace()
+        return (trimmedName, trimmedValue)
+    }
+
+    /// If this string is a key-value pair, splits it and normalizes the key.
+    /// Otherwise, return this string, trimmed of whitespace and normalized to lowercase.
+    fileprivate func parseCookieComponent() -> (String?, String?) {
+        guard let (trimmedName, trimmedValue) = self.parseKeyValuePair() else {
+            let trimmedName = self.trimmingWhitespace()
+            return (trimmedName.isEmpty ? nil : trimmedName.lowercased(), nil)
+        }
+        guard !trimmedName.isEmpty else {
+            return (nil, nil)
+        }
+        return (trimmedName.lowercased(), trimmedValue.isEmpty ? nil : String(trimmedValue))
     }
 }
 
-extension HTTPClient.Response {
-    /// List of HTTP cookies returned by the server.
-    public var cookies: [HTTPClient.Cookie] {
-        return self.headers["set-cookie"].compactMap { HTTPClient.Cookie(header: $0, defaultDomain: self.host) }
+#if canImport(Darwin)
+import Darwin
+#elseif canImport(Glibc)
+import Glibc
+#endif
+import CShims
+
+private let posixLocale: locale_t = {
+    // All POSIX systems must provide a "POSIX" locale, and its date/time formats are US English.
+    // https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap07.html#tag_07_03_05
+    newlocale(LC_TIME_MASK | LC_NUMERIC_MASK, "POSIX", nil)!
+}()
+
+private func parseCookieTime(_ string: String) -> Int64? {
+    var timeComps = tm()
+    var hasExplicitTimeZone = false
+    parse: do {
+        if swiftahc_cshims_strptime_l(string, "%a, %d %b %Y %H:%M:%S %Z", &timeComps, .init(posixLocale)) != nil {
+            hasExplicitTimeZone = true
+            break parse
+        }
+        timeComps = tm()
+        if swiftahc_cshims_strptime_l(string, "%a, %d-%b-%y %H:%M:%S %Z", &timeComps, .init(posixLocale)) != nil {
+            hasExplicitTimeZone = true
+            break parse
+        }
+        timeComps = tm()
+        if swiftahc_cshims_strptime_l(string, "%a %b %d %H:%M:%S %Y", &timeComps, .init(posixLocale)) != nil {
+            break parse
+        }
+        return nil
+    }
+    #if canImport(Darwin)
+    // Darwin converts values with explicit time-zones to the local time-zone. Glibc appears not to.
+    // https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/strptime.3.html
+    if hasExplicitTimeZone {
+        let timestamp = Int64(mktime(&timeComps))
+        return timestamp == -1 && errno == EOVERFLOW ? nil : timestamp
     }
+    #endif
+    let timestamp = Int64(timegm(&timeComps))
+    return timestamp == -1 && errno == EOVERFLOW ? nil : timestamp
 }
diff --git a/Sources/CShims/include/module.modulemap b/Sources/CShims/include/module.modulemap
@@ -0,0 +1,4 @@
+module CShims {
+    header "shims.h"
+    export *
+}
diff --git a/Sources/CShims/include/shims.h b/Sources/CShims/include/shims.h
@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the AsyncHTTPClient open source project
+//
+// Copyright (c) 2018-2019 Apple Inc. and the AsyncHTTPClient project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef ASYNC_HTTP_CLIENT_CSHIMS_H
+#define ASYNC_HTTP_CLIENT_CSHIMS_H
+
+#include <time.h>
+
+char* _Nullable swiftahc_cshims_strptime(
+    const char * _Nonnull input,
+    const char * _Nonnull format,
+    struct tm * _Nonnull result
+);
+
+char* _Nullable swiftahc_cshims_strptime_l(
+    const char * _Nonnull input,
+    const char * _Nonnull format,
+    struct tm * _Nonnull result,
+    void * _Nullable locale
+);
+
+#endif // ASYNC_HTTP_CLIENT_CSHIMS_H
diff --git a/Sources/CShims/shims.c b/Sources/CShims/shims.c
@@ -0,0 +1,31 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the AsyncHTTPClient open source project
+//
+// Copyright (c) 2018-2019 Apple Inc. and the AsyncHTTPClient project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+#if __APPLE__
+    #include <xlocale.h>
+#elif __linux__
+    #define _GNU_SOURCE
+    #include <locale.h>
+#endif
+
+#include <time.h>
+
+char* swiftahc_cshims_strptime(const char * string, const char * format, struct tm * result) {
+    return strptime(string, format, result);
+}
+
+char* swiftahc_cshims_strptime_l(const char * string, const char * format, struct tm * result, void * locale) {
+    // The pointer cast is fine as long we make sure it really points to a locale_t.
+    return strptime_l(string, format, result, (locale_t)locale);
+}
diff --git a/Tests/AsyncHTTPClientTests/HTTPClientCookieTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientCookieTests+XCTest.swift
@@ -32,6 +32,7 @@ extension HTTPClientCookieTests {
             ("testMalformedCookies", testMalformedCookies),
             ("testCookieExpiresDateParsing", testCookieExpiresDateParsing),
             ("testQuotedCookies", testQuotedCookies),
+            ("testCookieExpiresDateParsingWithNonEnglishLocale", testCookieExpiresDateParsingWithNonEnglishLocale),
         ]
     }
 }
diff --git a/Tests/AsyncHTTPClientTests/HTTPClientCookieTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientCookieTests.swift
diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift b/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift

Original file line number	Diff line number	Diff line change
`@@ -32,6 +32,7 @@ extension HTTPClientCookieTests {`
`32`	`32`	`("testMalformedCookies", testMalformedCookies),`
`33`	`33`	`("testCookieExpiresDateParsing", testCookieExpiresDateParsing),`
`34`	`34`	`("testQuotedCookies", testQuotedCookies),`
	`35`	`+ ("testCookieExpiresDateParsingWithNonEnglishLocale", testCookieExpiresDateParsingWithNonEnglishLocale),`
`35`	`36`	`]`
`36`	`37`	`}`
`37`	`38`	`}`