Request cookies should not be persisted to session

After the fix for #1630, cookies sent with a request are now incorrectly persisted to the session.

Specifically, problem lies here: https://github.com/kennethreitz/requests/blob/1511dfa637643bae5b6111a20ecb80ec9ae26032/requests/sessions.py#L330

Removing that breaks the test case for #1630 though, still investigating a solution.


Wrote up a test case for this, investigating a proper solution with no other regressions.


Oh yes. I remember this problem. In the redirect handler, we need to add the cookies set by set-cookie responses to our requests, but we have lost access to the original cookiejar, as we only have the prepared request there. It seems as a fix to that, the request cookies were just added to the session cookie jar, so that they can just be regenerated on each loop in resolving redirects.

What we really need in the redirect handler is access to the cookiejar from the unprepared request. Then we can just keep merging the set-cookie responses into that and preparing the new cookie header.


Correction. Not the cookiejar of the request, the one we create to send to the `PreparedRequest.prepare` method. Are we against just storing that on the prepared request?


Now I am once again looking at stuff unrelated to this ticket, but the way auth is doing it: https://github.com/kennethreitz/requests/blob/master/requests/auth.py#L167

Is preparing the cookies based only off of the response cookies and not the original request cookies, which seems wrong and would probably just not send any cookies on the new request in many cases, but... Chat actually isn't even happening if there were cookies in the original request, because if there was, `prepare_cookies` doesn't actually do anything, and just leaves the cookie string in place.

The summary being: we really need the actual cookiejar for the prepared request there too.


Yeah, I was trying not to add to the PreparedRequest object purposefully. Seems we either should pass along the original request or add a cookie jar to the PreparedRequest. 


I added the cookiejar to the prepared request in #1729, what do we think of that?


Woah woah woah woah woah. Slow your horses a second. =D

There is a key design decision here that has been implicit that I think we should talk about, and I addressed it a little bit in #1630 without ever really coming to a conclusion. That question is: should cookies set on calls to `Session.request` via the `cookie` parameter be persisted to the `Session`?

It's not entirely obvious to me whether the answer is 'yes' or 'no' here. From our perspective the answer is an obvious 'no': if you wanted your cookies persisted then you should have set them on the `Session`. But for a newcomer that distinction isn't necessarily as clear. Can I get some input from you guys on where you fall in this discussion?


I think the answer is 'no' but like you said, from our perspective the answer is obvious. I'm also comfortable with that being the API as that is how the API should have behaved regardless of the changes I made.

I'm not entirely sold on the idea of having a cookie-jar on the `PreparedRequest` object though.


> There is a key design decision here that has been implicit that I think we should talk about, and I addressed it a little bit in #1630 without ever really coming to a conclusion. That question is: should cookies set on calls to Session.request via the cookie parameter be persisted to the Session?

I think this is a definitive no. Take this example:

``` python
session = Session()
session.get('http://somewhere', cookies={'my': 'secret'})
session.get('http://somewhereelse')  # It's pretty clear to me that we should not be sending the cookies from the last request here again
```

> I'm not entirely sold on the idea of having a cookie-jar on the PreparedRequest object though.

Yeah, I don't really like it either. It just happened to be the easiest fix I came up with. :/
I guess the issue is that we are preparing a new request, so really should have access to the original Request object.


@gazpachoking I agree that for that specific example the correct behaviour is clear. But what about this one?

``` python
s = Session()
s.get('http://myhost.com/page1', cookies={'from-my': 'browser'})
s.get('http://myhost.com/page2')  # Is it unreasonable to assume that the cookie would be persisted?
```

I continue to believe that from where we stand it seems obvious that the answer should be 'it shouldn't be persisted', but I'm convinced that people will get this wrong. I just want to make sure we're all aware of the various subtleties here before we make a call. =)


I'm not arguing that people won't get it wrong. I'm just arguing that persisting those cookies is very wrong for us to do implicitly, and we have a clear and explicit method to do it they will need to learn to use.


Suits me. =)


I'm seeing another regression in 2.1.0 that I think is related.  I'm POSTing a login form to, call it https://someurl/login, which returns a 301 redirect, along with a session cookie.  The catch is that the location being returned with the 301 points to an http url (ie. http://someurl/).  The redirect handler does a GET on that url (http://someurl/), but the site returns a 302 from that to redirect us to the https version of the url (ie. https://someurl/).

I'm trying to track down exactly why it's getting dropped, but the session cookie doesn't get sent along with the GET https://someurl/, so the login seems to fail.  The same calling code (basically just "session.post(url, login_form_as_dict)") succeeds in logging in with requests 2.0.1.

The history ends up looking like
POST https://someurl/login
<- 301 Location: http://someurl/, Cookie: somesession
GET http://someurl/, Cookie: somesesion
<- 302 Location: https://someurl/, Cookie: somesession
GET https://someurl/