Reqwest: cookie jar implementation

[pfernie]

About a year ago I wrote a user_agent crate (not published to crates.io) for an internal project, which provides the concept of a Session wrapping a Client and does cookie handling per RFC6265. At the time, the only client I targeted was hyper::Client. I'd be interested in updating/adapting the code to reqwest, but wanted to touch base on any existing plans/work here.

First off, the library suffers some warts:

• Perhaps not the cleanest rust, one of the first libraries I developed when first learning rust.
• It implements its own Cookie class, which wraps/extends the existing cookie crate (providing a From implementation to handle conversion). This was to introduce some more domain objects (CookiePath, CookieDomain, etc.) to enforce/aid the implementation of some RFC6265 guidelines.
• At the time, hyper was on a different (0.1?) version of cookie-rs, whereas I was developing against a more recent version (0.2+?), meaning the library goes through some gyrations to "convert" between the two versions by stringifying-then-parsing cookies.
• While I wrote a fair set of tests, usage and testing was primarly geared towards my particular use case at the time, so implementation re: RFC6265 may not be complete or necessarily fully correct.

Some of these issues can be cleaned up over time, but in terms of bigger picture I have some questions/concerns:

• The re-implementation/extension of Cookie seems un-necessary; submitting PRs against cookie-rs to bring them in line would make sense...
• cookie-rs already provides a CookieJar, implementing hierarchical jars and jar signing. In my crate, user_agent::CookieStore is more concerned with storing/expiring/updating cookies per RFC6265, and handles storing cookies and setting outgoing cookie headers based on URLs visited during the Session, as well as serializing the current session (to .json currently, but really to anything serde will serialize to) so they can be saved/restored. So, they are in some sense orthogonal currently, but feels as though they could be merged as well.
• As mentioned, the crate currently tries to generalize the concept of Session, although the only concrete impl is HyperSession. This was done with the vain hope of making a library for "all the webs" (curl clients, etc.), but the realities of needing to get my project done means this was (IMO) sub-optimally implemented. I'm not sure if it makes more sense to ditch the currently not very ergonomic abstraction and focus on implementing specifically towards reqwest, or to fix/maintain the abstraction and publish the crate separately as user_agent and just implement a ReqwestSession here. At a high level, the Session implementation is mainly concerned with defining how to get/set cookies into/out of Request/Response objects.

Obviously some questions would be more easily answered with some code to review; I just wanted to get a feel for the lay of the land before updating code to be suitable for a PR.

[seanmonstar]

Thanks for writing this up, it's awesome!

I have actually wanted to write a new cookie crate for hyper for a while now, since I imagine some of my opinions at this point would be too divisive to get into cookie-rs. For example, I would like to make the Cookie struct not contain so many Strings, perhaps only owning a single String and then holding indices, similar to how rust-url does Url. I've also felt the secure feature of cookie-rs doesn't belong, and would want to move signing of cookies completely out of crate. Another crate could provide that, there are so many different ways someone might want to sign and/or encrypt cookies. I registered the cookies crate to do this, and then found I had way too many other things to do before that.

Does doing that plus the Cookie Jar / Session stuff you've done seem like a good idea?

[Michael-F-Bryan]

Say you were to make a new cookie manager from scratch, how difficult would it be?

I recently came across your blog post and thought I'd try it out. So far I'm really liking the user friendly API you've built up, but the thing that's preventing me from writing any non-trivial web client stuff (as opposed to, say, Python's Requests) is the lack of cookie support.

I'd be interested in sending a pull request or two, but I'm not sure how you'd want to implement cookies and integrate it with the rest of the project. What have you done towards cookies so far?

[pfernie]

I was hoping to push on this a bit this weekend; I'm hoping my existing code (which implements the linked rfc) can quickly be adapted for something at least functional that can be worked on for efficiency later w/o breaking back compat.

[Michael-F-Bryan]

After looking at how hyper has done cookies, @seanmonstar's idea of storing a single string and then getting slices of that using some accessor function when you retrieve a particular field sounds like the way to go. It's not really adding that much complexity and makes things a lot nicer.

How did your cookie jar work? Isn't it essentially just a trie/hashmap under the hood?

[pfernie]

Apologies for radio silence on this; I only recently had some time to revisit. It's certainly a WIP, but in the interest of getting something rolling I've made user_agent available (but not on crates.io), updated to use reqwest as a client. Revisiting the code, I think it makes sense for this to exist as its own crate user_agent, and in the "batteries included" spirit, reqwest may introduce a dependency and implement Session as seen here.

I'd be interested in feedback in terms of API, ergonomics, and as mentioned much in here may not be particularly rust-y as it was originally written a while ago. In particular, the set of traits exposed for clients to be able to be used in user_agent (Session, CarriesCookies, HasSetCookies) were tailored to my specific use case (& hyper), and may not be generally useful... Error handling is a little scatter-brained; nowadays I tend to use error-chain in my projects, although this doesn't seem to be the practice in either hyper or reqwest. I've recently started using slog for logging, although perhaps in this case sticking to the vanilla log makes more sense?

In terms of writing a new/reqwest/hyper-specific Cookie:
The current "cookie-version-mismatch" issue shows up here. There are aspects of the current user_agent implementation that really make more sense as part of a Cookie implementation (CookieDomain, CookiePath, etc.), and paring down user_agent to be concerned with storage, serialization, etc. (e.g. basically a hashmap as @Michael-F-Bryan mentions) and providing the Session and related traits for decorating clients. I'd be interested in implementing that; I started out implementing the "single serialization" a-la-url style here, although after doing this basic work I had some design questions/thoughts. Rather than pollute this thread, I've opened this issue to outline some of them.

[seanmonstar]

I do agree that this could be a separate crate, I have no problem with reqwest depending on it.

Looking at user_agent: I like a lot of what I see, and I see that you've thought through a lot of the concepts of storing and sending cookies, fantastic! I'd probably handle the Session and related traits differently, myself. Here's how I'd imagine such a crate working, and how I'd expect to use such a crate in reqwest:

• I would likely make the reqwest::Client contain a CookieStore/CookieJar/BunchOfCookes, and let each Client essentially be its own session. The store probably wouldn't be exposed publicly, but perhaps Client could take some configuration if needed.
• I wouldn't expect the Session to do the actual requests/responses, or really even know about them. I'd probably want to ask the store for a list of Cookies to send in a header, according to a given URL. Something along these lines:

if let Some(cookies) = self.jar.get(&url) {
    headers.set(Cookie(cookies));
}

• I'd afterwards want to store the cookies back in the jar when I get a response:

let res = do_request(args)?;
if let Some(cookies) = res.headers.get::<SetCookie>() {
    self.jar.put(res.url, cookies);
}

• I'd expect the jar to check for expiration, sub domains, paths, secure, etc, when getting and putting cookies.
• I don't necessarily expect the jar to know how to convert to/from hyper's Cookie/SetCookie headers, and would probably do the conversion myself in reqwest.
• I have only ever glanced over slog, and while it seems nice, I'd probably make libraries that interact with the more common logging library (which I assume is log).
• I'd probably make most of the fields of your types private, and use getter/setters, ot reduce the need to bump major versions when you decide to structure something differently.

I'll hop over to the cooky issue to talk about that specifically.

Exciting!

[pfernie]

Thanks for the comments. I agree that the current Session scheme feels backwards/unnecessary; it was a product of my use case at the time. I'd like to update the current implementation to take these comments into account (remove the Session, CarriesCookies, HasSetCookies traits, etc.), and publish a 0.1 to crates.io to make it available as a dependency for reqwest.

In regards to how you'd like to see functionality implemented within reqwest: You mention not exposing the store publicly, and that effectively each Client instantiation is its own session. But for the use case of wanting to be able to maintain sessions over time, I was thinking it makes more sense to have a reqwest::Client have a field cookie_jar: Option<CookieStore>. and a setter method set_jar(..) and a get_jar() to retrieve it once done. This would allow users to handle serialization and the like as they see fit and pass in the actual CookieStore instance. Calls to get(url) and similar would then utilize the jar if present. Alternatively, to make usage more explicit, do not introduce a new field to Client, but rather extend the API for Client to include session_get(url, &mut jar) and similar. Having a separate/parallel API for jar usage doesn't really feel as ergonomic to me, and I'm not sure what the added value would be (wanting to use a jar for some queries and not others...?), so I'd be inclined to the former.

Either way, it makes sense to move the conversion logic around hyper's headers out of the CookieStore and into reqwest, as you say; that goes along naturally with getting rid of the various CarriesCookies traits and whatnot.

[pfernie]

@seanmonstar not sure if you follow the cookie-rs crate directly, but there is an interesting PR open which may make that crate more attractive for use as hyper/reqwest Cookie impl as opposed to rolling some independent cooky crate. Most of the ideas I tossed around in cooky are implemented here (minimizing allocations, builder pattern, etc.).

[SergioBenitez]

@pfernie Saw your post from the PR. Is there something I missed in the PR that you proposed as an idea earlier (I couldn't find where you shared your ideas)? Would love to hear it if so!

[pfernie]

@SergioBenitez No, I actually think your PR is pretty in line with most of the ideas I had been entertaining! I haven't had a chance to dig in deeply, though. I have an un-published (on crates.io) implementation of a cookie jar which handles path/domain matching, etc. here. That crate does introduce some more specific types (CookieDomain, CookiePath, etc.) to handle/enforce that logic, but I am not sure that is appropriate for pushing into the "lighter" Cookie implementation in cookie-rs. The validation, etc. involved is really only of interest to jar implementations, so is overhead that probably doesn't make sense for just Cookie.

As you can tell from the age of this issue, I've been letting it get a little stagnant, but have been hoping to get back to it soon to implement ideas discussed here, add support for public suffix checking, etc. When I do so, I'll have a better idea if there are any enhancements that would make sense to push into the cookie-rs Cookie implementation directly.

[seanmonstar]

@pfernie it looks really promising! I'd also want 1 other change, which is to rip out the signing secure feature, so that someone enabling it cannot cause a conflict with the included native-tls crate in reqwest.

[SergioBenitez]

@seanmonstar I'll be looking into just that in the near future.

[SergioBenitez]

@seanmonstar I've opened a PR that overhauls CookieJar: https://github.com/alexcrichton/cookie-rs/pull/76. In particular, it removes the openssl dependency in favor of ring. The secure feature is disabled by default.

[erickt]

To update this ticket, @pfernie and I have been getting https://github.com/pfernie/user_agent back to compiling on the latest cookie and reqwest. This should help make it easier to get it feature complete, or merge with another project.

[margolisj]

Howdy @pfernie and @erickt still working on this at all?

[pfernie]

@margolisj unfortunately I haven't had time to actively work on this. Things are at least in a building state, but it really needs some thought on where to go forward, especially in terms of how it should work w/ reqwest. In particular, I saw #155 recently; event hooks might be a sensible way for something like user_agent to hook into the pipeline to handle setting and acquiring cookies. PRs, ideas, or even just issues are welcome!

[margolisj]

@seanmonstar Do you see this as something that can be pretty closely ported from the python requests library? I guess if it was possible, I'd really appreciate a short synthesis of how you see this fitting / if anything has changed form your previous opinions posted here.

[seanmonstar]

@margolisj I haven't really read through that part of requests, so I couldn't comment there. I don't imagine the exposed API would be the same (I wouldn't want someone to have to use reqwest::Session instead of reqwest::Client).

My thoughts otherwise have stayed the same. If you'd like, I can find the cookie spec and list out the key points about safely storing and sending cookies to the correct urls.

[margolisj]

I don't imagine the exposed API would be the same (I wouldn't want someone to have to use reqwest::Session instead of reqwest::Client).

Ah very important. requests does provide another object that wraps around a Cleitn Any preference for triggering "session" mode then? Just add it as a param to the builder?

The cookie spec and/or work seems to be pretty far along in their user_agent and I'd mostly like to piggy back off that.

[seanmonstar]

Yes, I'd just have a cookie_jar or whatever option on the ClientBuilder.

[pfernie]

The existing user_agent does do the Session-wrapping-Client thing, but purely because I baked it up for an internal project and that made sense at the time. Just saying I'm totally open to aggressive PRs that change the API to be client owns the cookie_jar and mediates the process. Or if simply lifting out the tests and logic relevant to the cookies RFC to build something new is more expedient, I would have no issue with that either. I do think it would be Nice™ if it were reusable with other crates, but I do not have a requirement that it be so (I use this solely w/ hyper/reqwest for my original use case).

It is alluded to in this issue, but as a NB: I threw in public suffix support via the publicsuffix crate per the spec, but this has _not been tested for correctness_.

[margolisj]

@seanmonstar on second thought what I was was short sighted; for sure if you'd like to look through the cookie spec and list any key points I'm sure that would help towards getting it merged to your liking.

@pfernie you meant the cookie jar part as something talk could be easily used outside of reqwest, correct?

[pfernie]

@margolisj correct; I mean if user_agent changed so it could be used for cookie management with other http crates. A design where user_agent needn't concern itself with the particulars of reqwest::{Request,Response} and the like, which off the cuff doesn't seem like a particularly high hurdle... in fact it seems more natural: if the client (reqwest or some other crates') owns the jar, it just hands cookies from whatever responses to store and urls to retrieve cookies to include in whatever requests, so the user_agent API only deals in structs/enums from common crates.

[shssoichiro]

I'd like to add my two cents in favor of this, with a scenario I ran into the other night. I was making a post request to a website using reqwest, and the response contained a SetCookie header that I needed in order to authenticate future requests. However, the response then redirected, and the next response in the chain did not have the SetCookie header I needed. My workaround, once I discovered the issue, was to set this request to not redirect automatically, and then grab the cookie from the header, and manually make the next request, but this issue was very hard to pinpoint the root cause. If reqwest had built-in cookie jar handling, it could have automatically put the cookies from the first response into the jar, and everything would have been happy.

My opinion on the API I'd like to see is that a Client should automatically instantiate a CookieJar (which can be customized during the ClientBuilder), and that CookieJar can be accessed via a method on the Client. Then during each request, the Client will automatically build a Cookie header from the CookieJar, and during each response, the Client will automatically put any headers from the SetCookie header into the CookieJar. But of course there may be factors I have not taken into account that may make this a suboptimal approach.

[botev]

@pfernie @seanmonstar Any progress on this? It might be worthwhile maybe @seanmonstar to actually post bullet points of an exact skeleton of the ideas so that someone interested could implement this unless there is a need for more discussion.

[felix-d]

Just hit the same issue. I was setting cookies manually using cookie-rs but I'm losing cookies on redirections.

Would be awesome if the client could take an optional cookie jar so we can still benefit from auto redirects.

For those who need to use a manual approach in the meantime, this is what I came up with using cookie-rs. It also needs to be paired with a no redirect policy. Also note that this code might not be optimal as I'm not a cookies expert.

let mut req = reqwest::Request::new(reqwest::Method::Get, uri);

// We set cookies from the jar where self.cookie_jar is a cookie::CookieJar (from cookie-rs crate).
// We need to match on domains.
let mut cookie = reqwest::header::Cookie::new();
{
  let domain = req.url().domain().unwrap();
  self.cookie_jar.iter().for_each(|cookie_from_jar| {
    if let Some(cookie_domain) = cookie_from_jar.domain() {
      if domain_match(domain, cookie_domain) {
        cookie.set(cookie_from_jar.name().to_string(), cookie_from_jar.value().to_string());
      }
    }
  });
}

req.headers_mut().set(cookie);

let response = self.client.execute(req)?;

// Then we add cookies in the jar given the response
if let Some(raw_cookies) = response.headers().get::<reqwest::header::SetCookie>() {
    raw_cookies.iter().for_each(|raw_cookie| {
        let cookie =  cookie::Cookie::parse(raw_cookie).unwrap();
        self.cookie_jar.add(cookie)
    })
}

Where domain_match is something like

fn domain_match(string: &str, domain: &str) -> bool {
    // RFC https://tools.ietf.org/html/rfc6265#section-5.1.3
    let ip_regex = regex::Regex::new(r"\b\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\b").unwrap();
    string == domain || (
        domain.is_suffix_of(string) &&
        string[0..(string.len() - domain.len())].ends_with(".") &&
        !ip_regex.is_match(string)
    )
}

[mgoszcz2]

@seanmonstar I'd love for reqwest to support this. Is the only thing missing right now domain matching (and any canonicalization it might involve)?

With alexcrichton/cookie-rs#76 it seems the native-tls conflict is no longer an issue. Would it make sense to open an issue/PR on alexcrichton/cookie-rs to implement domain matching? If that's too much for cookie-rs, it might be easiest to implement it directly into reqwest, leaving to cookie-rs to specialise in a funny subset of cookie magics.

[seanmonstar]

@mgoszcz2 I don't remember perfectly, it's been a little while. Domain matching, Secure, paths, and expiration are all needed.

[mgoszcz2]

@seanmonstar Mhmm.. I see hyper 0.12 will remove any cookie parsing code in favour of using the 'http' crate types. Right now reqwest uses hyper for all its cookie needs. Is there a master plan on how cookies will be implemented in reqwest once this functionality is removed from hyper?

I'd love to take a shot writing YACC (Yet another cookie crate)[1] but I'm a bit hesitant given the major changes reqwest might experience upgrading to hyper 0.12.

[1] Might need to change that acronym

[xurtis]

I'm working on another cookie jar implementation rather slowly, but would appreciate feedback.

If anyone wants to take a look:
https://github.com/xurtis/cookie-jar

[dorfsmay]

Any update?
What are the blockers?
I'd rather help with this rather than re-implement sessions on each project.

[xurtis]

This cookie jar is intended to have a relatively simple interface. Individual cookies can be added to the jar as well as a series of cookies (or cookie strings) for a request associated with a given URL. Similarly, an iterator of cookies can be produced that would match a query at a given URL. As it is, the interface for this needs a bit of tidying up which I plan to do over the next day or so before finishing up implementation of the basic necessities.

If you wanted to build a session around that, I could work with you to finalise an interface that would make that convenient and you could get started. My understanding of what you mean by a session is some wrapper around an object that uses a cookie-jar to handle cookie and session state and I don't think I'm too far off from having enough of an interface to fulfill this.

If you want, I can ping you in a few days when I have what I think to be roughly the final interface and get your feedback on it so that you can ge started on sessions.

The current status of this project is a fully compliant cookie parser and partial implementation of a cookie builder (so that the cookies in the package can be used to build cookies on the server side and parse/store on the client side). Basic plans and structure are also in place for the cookie jar structure itself. The ability to import/export the entire store in any way to disk would come after a complete in-memory implementation was finished.

[dorfsmay]

Once it's established what cookie jar implementation is acceptable for reqwest, then we can start working on a session.

Have criteria been established on what will make a cookie jar acceptable?

[SergioBenitez]

@xurtis Why another crate that appears to do exactly what cookie does? How does it differ? And, why not suggest those changes to cookie?

[xurtis]

When I wrote this, I looked at cookie and or had some critical issues,
particularly with regards understanding of security concepts. It also
appeared to be entirely server side (even the 'cookie jars').

As far as I can tell, that crate makes absolutely no sense for either
server or client side and greatly confuses the protocol.

[dorfsmay]

@SergioBenitez @xurtis is there any way to merge both repos?

Maybe create a co-owned repo an pull the best code from both repos, then deprecate the original repos to force people to move to this one?

An http client without a concept of Session is very limited.... I'm seriously considering forking this repo and use whichever cookie jar repo works at this point.

[seanmonstar]

@dorfsmay the problem is that none currently do, for a client. The jar in the cookie crate collects all cookies, but does not have a concept of querying for active cookies that match a domain, path, secure. If you just attached all cookies from the jar to every request, you'd leak cookie secrets if you request different domains, or hit redirects.

If none of that matters to you, you can right now just grab the Set-Cookie headers from a response, and just add them to every request you make. But that's not an acceptable general solution :)

[dorfsmay]

@seanmonstar I agree with you regarding the need to query per domain (including the right behaviour for subdomains and "naked domain"), but I haven't looked at either crate.
What about the cookie-jar crate then?
If not, should we start create unit tests for what is expected.

@SergioBenitez @xurtis is this a direction you're comfortable taking your respective crate to?
Do you understand the requirements here (basically need to check the right RFCs)?

[xurtis]

The end goal for cookie-jar is a fully RFC compliant implementation of client-side cookie handling.

I've read through all of the relevant RFCs and it doesn't seem too difficult. I've already got parsing of cookies into useful structures so just arranging them into a tree and setting up correct matching is the next big step. You should have all you need to implement a session once the crate is complete.

[SergioBenitez]

@xurtis

When I wrote this, I looked at cookie and or had some critical issues,
particularly with regards understanding of security concepts. It also
appeared to be entirely server side (even the 'cookie jars').

As far as I can tell, that crate makes absolutely no sense for either
server or client side and greatly confuses the protocol.

Can you expand? What "critical issues" are you referring to? How does it misunderstand security concepts? The crate is being used both client-side (in servo) _and_ server-side (in rocket, actix-web, iron, and others), so, at least in practice, it makes _some_ sense, and dismissing it so readily isn't constructive.

I've read through all of the relevant RFCs and it doesn't seem too difficult. I've already got parsing of cookies into useful structures...

For instance, this is something that the cookie crate takes great care to do correctly and efficiently, and I see no use in replicating effort there.

@seanmonstar

The jar in the cookie crate collects all cookies, but does not have a concept of querying for active cookies that match a domain, path, secure. If you just attached all cookies from the jar to every request, you'd leak cookie secrets if you request different domains, or hit redirects.

This doesn't seem difficult to add at all. Do we really need an entirely new crate for this one feature?

[seanmonstar]

I'm not too worried about where this logic exists. It just needs to exist somewhere in order to provide this functionality in reqwest's client.

(I do still personally wish the signing stuff was in a different crate from cookie, but that's really minor, and wouldn't stop me from accepting cookie as a private dependency.)

[xurtis]

As of https://github.com/xurtis/cookie-jar/commit/8ee9eda3bc7556be1e8cb13ccef226bddd851d69, most of the necessary mechanics needed to build a session for Reqwest are implemented (although much of the matching specification is still WIP).

I have rendered the documentation at https://xurtis.pw/cookie-jar/doc/cookie_jar/index.html if you want to look at the interface. The aim is to implement the RFCs without using an excessively _clever_ interface. I still need to implement expiry and matching for http/secure but the current implementation should at least match the correct host/domain as well as HostOnly cookies correctly.

You can create a new jar with Jar::default and use Jar::add_cookie to add cookies generated with Cookie::parse. You can then get an iterator over all of the matched name=value pairs with Jar::url_matches, to match against the Url of a query.

[xurtis]

@SergioBenitez the existence of a PrivateJar and SignedJar greatly concern me as they suggest that either storing cookies on the server side is useful or that trusting a client to authenticate themselves is in anyway a level of useful security. There is nothing that these two types provide beyond memory safety, in which case these are the wrong tools to use for providing that.

Any signing performed with cookies should be done on the server side on the values stored in the cookies, not on the cookies themselves and certainly not within the cookie jar _on the client_.

Secondly, as it stands, the cookies in the cookie crate do not encode a concept of origin, making them useless to any client library unless they are extended.

As far as I can tell, there is much of the specification of cookies as far as an HTTP client is concerned missing from the cookie crate that makes it barely useful to an HTTP client. Only its Cookie and CookieBuilder types seem useful and only then do they appear useful to a server.

Overall, the design of the cookie crate suggests a large misunderstanding of the protocol as well as the security implications related to it.

[SergioBenitez]

@SergioBenitez the existence of a PrivateJar and SignedJar greatly concern me as they suggest that either storing cookies on the server side is useful or that trusting a client to authenticate themselves is in anyway a level of useful security. There is nothing that these two types provide beyond memory safety, in which case these are the wrong tools to use for providing that.

It sounds like you've either gravely misunderstood what those types do or are pitting them against a scenario where their utility is dubious. The idea is that, on the server-side, you hold a secret key and can use it to sign/encrypt cookies; the resulting ciphertext is sent to the client. The client sends these cookies back at some point in the future. You can then cryptographically assert the confidentiality, integrity, and/or authenticity of the cookie's values. These properties are significantly stronger than memory safety.

Perhaps you believe that cookie somehow advocates for the use of these types client-side? This is stated nowhere in the documentation, and I can't imagine how that would make sense, so it's odd to me that you would jump to this conclusion and or base your opinions on this non-fact.

As far as I can tell, there is much of the specification of cookies as far as an HTTP client is concerned missing from the cookie crate that makes it barely useful to an HTTP client.

Great! The cookie crate was certainly developed with more server-side oriented applications in mind. Let's improve the crate to work well with client-side uses as well!

Only its Cookie and CookieBuilder types seem useful and only then do they appear useful to a server.

I'm not sure why they wouldn't be useful for building/reading/parsing cookies on the client-side? What's more, I've used the CookieJar type as the storage component of a client-side application, and its delta functionality was very handy.

Overall, the design of the cookie crate suggests a large misunderstanding of the protocol as well as the security implications related to it.

This feels baseless. Nothing that you've said demonstrates this. I'm also not sure which protocol you're referring to; cookies aren't a protocol.

[pfernie]

Glad to see this issue getting some attention; I regret that I never had the time to push on this myself. @SergioBenitez , I believe by "protocol" other users in this thread are referring to the behavior of cookie handling as described by RFC6265, which my user_agent code followed to provide an API. As such, I think the "security implications" mentioned was referring to the fact that without this RFC implementation, there is a temptation in client code to collect and send all cookies on all requests for a session; doing anything more sophisticated is effectively implementing the RFC...

@xurtis, I have not had a chance to review your crate, but I will say a reason I never pushed on this was I was unclear if the additional RFC functionality was appropriate for the cookie-rs project, or should be something external. Having a single definitive crate seems desirable, but IIRC some aspects of the work @SergioBenitez did in cookie-rs was focused on performance and avoiding allocations. I believe supporting RFC6265 fully involves some bookkeeping/allocations etc. such as for domain canonicalization, which some use cases may not want baked (pun intended...) into their CookieJar.

I never dug deep, but I considered that perhaps adding functionality to cookie-rs via implementing some new container (CookieRouter or CookieSession?) that owned a CookieJar and implemented the RFC6265 might be a direction to consider; this would be to allow lightweight usage for cookies in scenarios where RFC6265 is not useful (server-side), and clients that needed this functionality could opt-in. A similar approach might also make sense to separate out the concept of signing as @seanmonstar sort of suggests, and allow users to layer the concepts as needed.

All that being said, I will re-link the repo with my old implementation. The RFC is not that complicated, so perhaps it is simpler to reimplement, but it does contain implementations of most (all?) of the RFC, as well as a set of tests. And being both code from when I was first learning rust, and some niggles of the ecosystem back then (mismatched cookie-rs versions, etc.) any retained code would definitely benefit from some polish. But it may at least be useful as a reference.

Obviously, I have not had time to make any useful progress on developing my old code into something suitable as a crate, but I continue to keep an eye on this issue, and would be interested to contribute to whatever effort is born of the discussion.

[gzsombor]

@pfernie : I see you worked on your user_agent crate, but it's not on crates.io, so I guess, you don't consider ready for public usage, however there are some reqwest integration already there. Based on a cursory code reading, it seems pretty complete. Do you plan to finis and publish your work? It would be nice to have a working, cookie store solution for client side.

[pfernie]

@gzsombor user_agent is now published on crates.io. I tidied up some of my bigger objections with the crate; I split out the actual RFC6265 implementation into a separate cookie_store crate.

I did not publish as 1.0 since I think there is still design work to be done to make this fit more nicely in the ecosystem. Specifically:

• reqwest exposes its own Request(/RequestBuilder) and Response types, is there a longer term plan to migrate to http::{Request,Response}?
• cookie_store still implements its own notion of Cookie (which wraps cookie-rs), as the path and domain values maintained in that crate do not conform to the specs of RFC6265.
• I had intended to split out a separate user_agent_reqwest crate with the specific reqwest implementation there, but hit ye olde orphan rules and didn't want to jump through newtype hoops.

[seanmonstar]

@pfernie that's fantastic!

I'd probably as a next step try to use cookie_store inside reqwest, as part of the async client, and probably even expose wrappers around the Cookie types, to protect against any breaking changes (we can upgrade internally without users needing to worry about breakage).

[pfernie]

Was going to open a fresh issue, but I see e.g. #396 refers back to here. Just wanted to link related issue raised on user_agent.

[theduke]

Since #480 has been merged, a little update:

A cookie store is now supported (in master) and can be enabled with ClientBuilder::cookie_store(true).

For now, the cookie store is private though and can not be accessed for either populating it manually or storing/saving it somewhere.

This is of course desired and should be added, it just requires fleshing out good method signatures that won't need a breaking change.

[eutampieri]

@theduke what does

The rudimentary cookie support means that the cookies need to be manually configured for every single request. In other words, there's no cookie jar support as of now. The tracking issue for this feature is available on GitHub.

mean in the 0.9.x documentation? Is it outdated? Shouldn't be enough to just do the thing below to have the cookie stored inside the client?

let client = reqwest::Client::builder()
    .cookie_store(true)
    .build()
    .unwrap();
client.get(url.as_str()).send()

[theduke]

Yes that is outdated, where in the code is it?

[eutampieri]

In the Readme.MD, 0.9.x branch.

Il giorno 16 ott 2019, alle ore 16:08, theduke [e-mail] ha scritto:

Yes that is outdated, where in the code is it?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/seanmonstar/reqwest/issues/14?email_source=notifications&email_token=ACOOZ3C3MRAYDVQAYAUY63DQO4N65A5CNFSM4CWND4N2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBMT32Y#issuecomment-542719467, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACOOZ3H7HWB4TXISKBXV3YDQO4N65ANCNFSM4CWND4NQ.

[gnzlbg]

What else is required to close this issue ?

[crusty-dave]

I just stumbled across this. I would like to see an explicit reqwest::Session that works like Python requests.Session(). The Python code handles TLS correctly in that it doesn't create a new TLS session for each request, while reqwest is creating a separate TLS session per request. I haven't found anyway to avoid that. I am running 0.10.4 because our mirror hasn't been updated in a while, so if there is capability in 0.10.6, I cannot currently test it due to mismatched dependencies.

I should add that in this case, my client is running on Windows and I am not specifying any certificates, so it is presumably using native-tls.

I just tested on Ubuntu using openssl and a root CA and it has the same issue - not surprisingly.