search

Kitura / Kitura-net

Kitura networking
Apache License 2.0
104 stars 79 forks source link

Support cookies #276

Closed NocturnalSolutions closed 6 years ago

NocturnalSolutions commented 6 years ago

Curl already has built-in support for maintaining cookies between HTTP requests, including doing things like expiring old cookies and other things that normal HTTP clients do. This change surfaces that functionality for use with Kitura-net.

Description

Adds a public private(set) var cookieJar: String? property to ClientRequest. If this is set to a non-nil String, that string is passed to Curl to use as a location for writing and reading cookies (Curl options CURLOPT_COOKIEFILE and CURLOPT_COOKIEJAR respectively). If nil, an empty string is passed to Curl for these options, which disables this functionality.

Motivation and Context

I'm writing tests for a site which has some pages which only logged-in users should be able to access. I want to test this by first using Midnight Test to fetch the pages and make sure I get 403 Forbidden responses for them; then POST credentials to the log in form's destination and re-access the pages to see if I get 200 OK responses. I could do this by managing cookies "manually" by reading and setting HTTP headers, but if Curl already supports it, why reinvent the wheel?

How Has This Been Tested?

This PR includes a new test method testing the new functionality (testCookieJar() in ClientE2ETests).

Checklist:

ianpartridge commented 6 years ago

Thanks for the PR. While in principle there's no problem with adding something like this, there are a few practical obstacles in the way...

SemVer ClientRequest.Options is a public enum, and adding new cases to a public enum is unfortunately a semver major change (because switches in Swift are exhaustive).

Kitura-NIO We now support 2 implementations of this API - the other is in Kitura-NIO and runs on top of SwiftNIO's ClientBootstrap. As SwiftNIO does not currently provide support for a cookie store like curl does, we would have to implement a cookie store in Kitura-NIO to keep the two implementations in sync.

Strategy Kitura is in the process of adopting SwiftNIO as its transport library, and we expect SwiftNIO to be our default in future. Hence the Kitura-net API, and its inclusion of an HTTP client, is going to become something of a historical artifact. Longer term, I expect a separate full-featured HTTP client library built on SwiftNIO to be developed (either by us, the SwiftNIO folks or the wider community, who knows...). Cookie support would be important in that. Such a new library would render the HTTP.get() API in Kitura-net obsolete.

This is a rather long-winded way of saying that we are not particularly keen on expanding this API at the moment 😞

NocturnalSolutions commented 6 years ago

Hmm, I didn't know the NIO branch was changing how outgoing connections are made in addition to incoming ones.

Guess I will have to hack it together myself, then.

ianpartridge commented 6 years ago

I'm sorry but I don't think we can add this support at the moment. I'm going to close this PR for now, if anything changes we can revisit this in future. Again, thanks for the PR and sorry we couldn't merge it.