Skip to content

Implement URLSessionConfiguration.ephemeral #2125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 28 additions & 12 deletions Foundation/HTTPCookieStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ open class HTTPCookieStorage: NSObject {
private static let sharedSyncQ = DispatchQueue(label: "org.swift.HTTPCookieStorage.sharedSyncQ")

/* only modified in init */
private var cookieFilePath: String!
private var cookieFilePath: String?

/* synchronized on syncQ, please don't use _allCookies directly outside of init/deinit */
private var _allCookies: [String: HTTPCookie]
Expand All @@ -62,22 +62,27 @@ open class HTTPCookieStorage: NSObject {
}
private let syncQ = DispatchQueue(label: "org.swift.HTTPCookieStorage.syncQ")

private init(cookieStorageName: String) {
private let isEphemeral: Bool

private init(cookieStorageName: String, isEphemeral: Bool = false) {
_allCookies = [:]
cookieAcceptPolicy = .always
self.isEphemeral = isEphemeral
super.init()
let bundlePath = Bundle.main.bundlePath
var bundleName = bundlePath.components(separatedBy: "/").last!
if let range = bundleName.range(of: ".", options: .backwards, range: nil, locale: nil) {
bundleName = String(bundleName[..<range.lowerBound])
if !isEphemeral {
let bundlePath = Bundle.main.bundlePath
var bundleName = bundlePath.components(separatedBy: "/").last!
if let range = bundleName.range(of: ".", options: .backwards, range: nil, locale: nil) {
bundleName = String(bundleName[..<range.lowerBound])
}
let cookieFolderPath = _CFXDGCreateDataHomePath()._swiftObject + "/" + bundleName
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still in the learning phase, ignore this comment if it is wrong,

After looking the code from TestHTTPCookieStorage.test_cookieInXDGSpecPath(), it seems you are missing,

if let xdg_data_home = getenv("XDG_DATA_HOME") {
   // XDG Path
} else {
   // Non XDG Path
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@karthikkeyan For ephemeral storage, we don't persist the cookies in a file. The XDG spec standardises some common locations on the file system. It's not relevant if we don't want to store cookies to a file. I hope I got your question right.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late response, the statement that creates the cookie folder's path, is inside if !isEphemeral

if !isEphemeral {
    // ...

    let cookieFolderPath = _CFXDGCreateDataHomePath()._swiftObject + "/" + bundleName

    // ...
}

We can leave that discussion for different PR as the code is not relevant to the intention of this PR.

cookieFilePath = filePath(path: cookieFolderPath, fileName: "/.cookies." + cookieStorageName, bundleName: bundleName)
loadPersistedCookies()
}
let cookieFolderPath = _CFXDGCreateDataHomePath()._swiftObject + "/" + bundleName
cookieFilePath = filePath(path: cookieFolderPath, fileName: "/.cookies." + cookieStorageName, bundleName: bundleName)
loadPersistedCookies()
}

private func loadPersistedCookies() {
guard let cookiesData = try? Data(contentsOf: URL(fileURLWithPath: cookieFilePath)) else { return }
guard let cookieFilePath = self.cookieFilePath, let cookiesData = try? Data(contentsOf: URL(fileURLWithPath: cookieFilePath)) else { return }
guard let cookies = try? PropertyListSerialization.propertyList(from: cookiesData, format: nil) else { return }
var cookies0 = cookies as? [String: [String: Any]] ?? [:]
self.syncQ.sync {
Expand Down Expand Up @@ -109,6 +114,12 @@ open class HTTPCookieStorage: NSObject {
return FileManager.default.currentDirectoryPath + "/" + bundleName + fileName
}

// `URLSessionConfiguration.ephemeral` needs an ephemeral cookie storage.
// Ephemeral cookie storage is an in-memory store and does not load from, and store to, a persistent store.
internal class func ephemeralStorage() -> HTTPCookieStorage {
return HTTPCookieStorage(cookieStorageName: "Ephemeral", isEphemeral: true)
}

open var cookies: [HTTPCookie]? {
return Array(self.syncQ.sync { self.allCookies.values })
}
Expand All @@ -130,7 +141,7 @@ open class HTTPCookieStorage: NSObject {
}
}
}

/*!
@method sharedCookieStorageForGroupContainerIdentifier:
@abstract Get the cookie storage for the container associated with the specified application group identifier
Expand Down Expand Up @@ -183,7 +194,7 @@ open class HTTPCookieStorage: NSObject {
}

open override var description: String {
return "<NSHTTPCookieStorage cookies count:\(cookies?.count ?? 0)>"
return "\(self.isEphemeral ? "Ephemeral" : "")<NSHTTPCookieStorage cookies count:\(cookies?.count ?? 0)>"
}

private func createCookie(_ properties: [String: Any]) -> HTTPCookie? {
Expand All @@ -200,6 +211,11 @@ open class HTTPCookieStorage: NSObject {
}

private func updatePersistentStore() {
// No persistence if this is an ephemeral storage
if self.isEphemeral { return }

guard let cookieFilePath = self.cookieFilePath else { return }

if #available(macOS 10.12, iOS 10.0, tvOS 10.0, watchOS 3.0, *) {
dispatchPrecondition(condition: DispatchPredicate.onQueue(self.syncQ))
}
Expand Down
10 changes: 9 additions & 1 deletion Foundation/URLSession/URLSessionConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,15 @@ open class URLSessionConfiguration : NSObject, NSCopying {
open class var `default`: URLSessionConfiguration {
return URLSessionConfiguration()
}
open class var ephemeral: URLSessionConfiguration { NSUnimplemented() }

open class var ephemeral: URLSessionConfiguration {
// Return a new ephemeral URLSessionConfiguration every time this property is invoked
// TODO: urlCache and urlCredentialStorage should also be ephemeral/in-memory
// URLCache and URLCredentialStorage are still unimplemented
let ephemeralConfiguration = URLSessionConfiguration()
ephemeralConfiguration.httpCookieStorage = HTTPCookieStorage.ephemeralStorage()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just printed all the public property of ephemeral configuration, and noticed some minor discrepancies of values between the existing implementation and yours,

  • value for httpMaximumConnectionsPerHost is 4, but it is 6 in yours
  • protocolClass array contains [_NSURLHTTPProtocol, _NSURLDataProtocol, _NSURLFTPProtocol, _NSURLFileProtocol, NSAboutURLProtocol], but it is just [SwiftFoundation._HTTPURLProtocol] in yours.

Hope this helps.

Copy link
Member Author

@pushkarnk pushkarnk Apr 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right. The protocolClass array contains all the default native protocol implementations that come with URLSession. These are http, ftp, file and data. We currently have only http implemented and there is a pull request for ftp too. This array will be populated as and when the respective implementations are available. Thanks for noticing this, though!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@karthikkeyan I checked the value for httpMaximumConnectionsPerHost on Darwin with the latest dev snapshot. It is 6.

Welcome to Apple Swift version 5.0-dev (LLVM 082dec2e22, Swift 1e5637f841).
Type :help for assistance.
  1> import Foundation
  2> URLSessionConfiguration.ephemeral.httpMaximumConnectionsPerHost
$R0: Int = 6

return ephemeralConfiguration
}

open class func background(withIdentifier identifier: String) -> URLSessionConfiguration { NSUnimplemented() }

Expand Down
24 changes: 24 additions & 0 deletions TestFoundation/TestURLSession.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class TestURLSession : LoopbackServerTest {
("test_concurrentRequests", test_concurrentRequests),
("test_disableCookiesStorage", test_disableCookiesStorage),
("test_cookiesStorage", test_cookiesStorage),
("test_cookieStorageForEphmeralConfiguration", test_cookieStorageForEphmeralConfiguration),
("test_setCookies", test_setCookies),
("test_dontSetCookies", test_dontSetCookies),
("test_initURLSessionConfiguration", test_initURLSessionConfiguration),
Expand Down Expand Up @@ -707,6 +708,29 @@ class TestURLSession : LoopbackServerTest {
waitForExpectations(timeout: 30)
}

func test_cookieStorageForEphmeralConfiguration() {
let config = URLSessionConfiguration.ephemeral
config.timeoutIntervalForRequest = 5
let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/requestCookies"
let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil)
var expect = expectation(description: "POST \(urlString)")
var req = URLRequest(url: URL(string: urlString)!)
req.httpMethod = "POST"
var task = session.dataTask(with: req) { (data, _, error) -> Void in
defer { expect.fulfill() }
XCTAssertNotNil(data)
XCTAssertNil(error as? URLError, "error = \(error as! URLError)")
}
task.resume()
waitForExpectations(timeout: 30)
let cookies = config.httpCookieStorage?.cookies
XCTAssertEqual(cookies?.count, 1)

let config2 = URLSessionConfiguration.ephemeral
let cookies2 = config2.httpCookieStorage?.cookies
XCTAssertEqual(cookies2?.count, 0)
}

func test_dontSetCookies() {
let config = URLSessionConfiguration.default
config.timeoutIntervalForRequest = 5
Expand Down