Set-Cookie - violating rfc6265

Hey,

I found that requests (version 2.6.0) violates one paragraph of rfc6265 - http://tools.ietf.org/html/rfc6265#section-4.1.2.3

"If the server omits the Domain attribute, the user agent will return the cookie only to the origin server."

It means that if Set-Cookie is set without domain at test.com, the cookie shouldn't be visible at subdomain.test.com. 

However, the RFC warns that some user agents behave that way. Have this been discussed before? I haven't found anything. I encountered that issue when I was requesting endpoint which sent me 301 to subdomain of the initial endpoint. The service didn't handle that properly.

Below the code that shows the issue.

``` python
"""
Use twoliner to add hosts to /etc/hosts

echo "127.0.0.1       test.com
127.0.0.1       subdomain.test.com" |  sudo tee -a /etc/hosts
"""

import BaseHTTPServer
import requests
import threading


# http server
class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
    def do_GET(self):
        l = self.headers.get('Host').lower()
        print "Server: processing {}".format(l)

        if l == 'test.com:8000':
            self.send_response(301)
            self.send_header('Set-Cookie', 'Test=True')
            self.send_header('Location', 'http://subdomain.test.com:8000')
        elif l == 'subdomain.test.com:8000':
            print "---> Cookie: {} <---".format(self.headers.get('Cookie'))
            self.send_response(200)
        else:
            assert False, "Not supported"


httpd = BaseHTTPServer.HTTPServer(('', 8000), RequestHandler)


def server_callback():
    while True:
        httpd.handle_request()

server_thread = threading.Thread(target=server_callback)
server_thread.daemon = True
server_thread.start()

# http client
requests.get('http://test.com:8000')
```

The code outputs 

```
Server: processing test.com:8000
127.0.0.1 - - [28/Apr/2015 23:47:55] "GET / HTTP/1.1" 301 -
Server: processing subdomain.test.com:8000
---> Cookie: Test=True <---
127.0.0.1 - - [28/Apr/2015 23:47:55] "GET / HTTP/1.1" 200 -
```


So the code that handles cookies for us in the standard library. We rely on [the `add_cookie_header` method on a CookieJar](https://github.com/kennethreitz/requests/blob/master/requests/cookies.py#L135). We create a mock request object but as far as I can tell we're doing everything we should. If we look at that, we'll see that [the `add_cookie_header` method](https://hg.python.org/cpython/file/293d9964cf6e/Lib/http/cookiejar.py#l1331) calls [`_cookies_for_domain`](https://hg.python.org/cpython/file/293d9964cf6e/Lib/http/cookiejar.py#l1247) which calls [`domain_return_ok` on `DefaultCookiePolicy`](https://hg.python.org/cpython/file/293d9964cf6e/Lib/http/cookiejar.py#l1166). The check explicitly says that it is a liberal check. 

With that information, I think I'm comfortable saying that we should start providing a way for users to enable a stricter policy. I don't think we can start doing that by default, though, until 3.0 since it will be a breaking change. Does that sound reasonable?


Also, I wasn't previously aware of this so I'd like to sincerely thank you for bringing this to our attention.


@sigmavirus24 Can users not provide a stricter policy by replacing the cookiejar on the `Session` object?


@lukasa they can. I'm proposing that we provide a stricter policy by default in the near future to make requests comply better with 6265


> With that information, I think I'm comfortable saying that we should start providing a way for users to enable a stricter policy. I don't think we can start doing that by default, though, until 3.0 since it will be a breaking change. Does that sound reasonable?

I very agree. I am not sure how many people this change could break and whether the behavior should be configurable. I'm also wondering how this is handled in requests analogues.


Yeah, I want to know what browsers do and what curl does. I'll test this at some stage.


I'm pretty sure we can use the script in @henadzit's original report to test browsers and curl both. It's well done.


I updated the test script to test curl and wget. I also tried to use Ruby httpclient but it failed on my server answers. Probably, it requires some headers or something like that. I will look into it later. Anyway, the script is at https://github.com/henadzit/henadzit-various/blob/master/requests-show-rfc6265.py

Its output

```
Client: curl
Server: processing test.com:8000
127.0.0.1 - - [30/Apr/2015 01:02:01] "GET / HTTP/1.1" 301 -
Server: processing subdomain.test.com:8000
---> Cookie: None <---
127.0.0.1 - - [30/Apr/2015 01:02:01] "GET / HTTP/1.0" 200 -

Client: requests
Server: processing test.com:8000
127.0.0.1 - - [30/Apr/2015 01:02:01] "GET / HTTP/1.1" 301 -
Server: processing subdomain.test.com:8000
---> Cookie: Test=True <---
127.0.0.1 - - [30/Apr/2015 01:02:01] "GET / HTTP/1.1" 200 -

Client: wget
Server: processing test.com:8000
127.0.0.1 - - [30/Apr/2015 01:02:01] "GET / HTTP/1.1" 301 -
Server: processing subdomain.test.com:8000
---> Cookie: None <---
127.0.0.1 - - [30/Apr/2015 01:02:01] "GET / HTTP/1.1" 200 -
```

As you can see, both wget and curl follow the RFC and don't store the cookie.

I still want to test browser behavior.


So I've had this bug nagging at me for no particular reason. I started digging into it tonight and I found that the problem is that the standard library's cookie handling library actually [always adds a domain](https://hg.python.org/cpython/file/293d9964cf6e/Lib/http/cookiejar.py#l1461) if it's absent to a cookie. It also, helpfully (and I literally just realized this while writing this comment) adds `domain_specified` which is a bool. If `domain` isn't in the parsed cookie then that attribute will be false. This makes making our own policy really easy.


In an effort to clean up the issue tracker to only have issues that are still relevant to the project we've done a quick pass and decided this issue may no longer be relevant for a variety of potential reasons, including:

* Applies to a much older version, unclear whether the issue still applies.
* Change requires a backwards incompatible release and it's unclear if the benefits are worth the migration effort from the community.
* There isn't a clear demand from the community on the change landing in Requests.

If you think the issue should remain open, please comment so below or open a new issue and link back to the original issue. Again, thank you for opening the issue and for the discussion, it's much appreciated.