Aiohttp: ClientSession cookies does not work with IP

[imbolc]

Long story short

I use 127.0.0.1 instead of localhost for development and wasted few hours before understand why my tests breaks :)

import asyncio
import aiohttp


async def main():
    urls = [
        'http://httpbin.org/cookies/set?test=ok',
        'http://54.175.219.8/cookies/set?test=ok',
    ]
    for url in urls:
        with aiohttp.ClientSession() as s:
            async with s.get(url) as r:
                print(await r.json())
                print(list(s.cookie_jar))


loop = asyncio.get_event_loop()
loop.run_until_complete(main())

Actual behaviour

{'cookies': {'test': 'ok'}}
[<Morsel: test=ok; Domain=httpbin.org; Path=/>]
{'cookies': {}}
[]

Your environment

Last stable release of aiohttp, linux

[asvetlov]

It's intended behavior.
Please use aiohttp.ClientSession(cookie_jar=aiohttp.CookieJar(unsafe=True)) for enabling cookie processing for IP addresses.

[imbolc]

Thanks, it works :)

I see mention of this in the docs in CookieJar section. Actually I was looking for answer in the docs before I asked here, but I didn't found. Maybe it deserves to be mention in the main section of docs, because working with IP looks obvious and because this behavior differs from last release.

[asvetlov]

unsafe option was introduced by 0.22.3 as workaround for new aiohttp.CookieJar from 0.22.0.

Please make a PR for docs update.
You know as aiohttp _code writer_ I'm not the best person for finding quirks and subtle places in the documentation.
Maybe unsafe flag should be mentioned not in _the main section_ but in testing chapter?

[imbolc]

Yes, I can up docs. Could you please explain me little more about safety. Working with cookies in requests by IP itself does not seem unsafe. Because browsers, requests and aiohttp < 0.22.3 are doing this. Why should not this behavior stay default?

[asvetlov]

RFC 6265 declares in section _5.1.3. Domain Matching_:
A string domain-matches a given domain string if at least one of the
following conditions hold:

• The domain string and the string are identical. (Note that both
  the domain string and the string will have been canonicalized to
  lower case at this point.)
• All of the following conditions hold:
  
  
  
  • The domain string is a suffix of the string.
  
  
  • The last character of the string that is not included in the
    
    domain string is a %x2E (".") character.
  
  
  • The string is a host name (i.e., not an IP address).
  
  

Thus by the last rule IP addresses are not considered as safe source.
Well, we could relax the rule but _right now_ I want to keep it for sake of security.
Most likely IP addresses are needed for testing use cases only.

P.S.
Technically we may add all these options like unsafe, verify_ssl etc. into ClientSession ctor but it brings much more mess into the library maintenance.

[imbolc]

I understood, thank you :)

Besides the tests, IPs may be used widely in micro-service based architecture, clusters etc, to avoid any dns queries. But I think most important case we need to consider is implementing of libs. Now any open-source libs which use aiohttp client should change its APIs to pass unsafe param. And developers who will implement new libs should be known about it, to include the param to their API.

I'm not sure where is the right way: to be more secure by default or to work as user expect intuitively. I think requests is the source of expectations now, and it works with IPs by default.

[asvetlov]

Most likely you overestimate the problem.
We may speculate a lot about how unsafe mode is important -- but the reality is: you have raised the first issue about IP addresses for more than two months of the feature existence.

requests has no own cookie jar, it uses standard python's implementation. Which in turn is very old, written in outmoded style, has bad test coverage and really not supported by core developers.
It is the perfect example of _death in stdlib_.
I prefer to follow RFC updates if possible. At least by default.

[wood-j]

It's intended behavior.
Please use aiohttp.ClientSession(cookie_jar=aiohttp.CookieJar(unsafe=True)) for enabling cookie processing for IP addresses.

thanks alooooooooooooooooooooot

[kohtala]

@asvetlov, I think the interpretation of RFC 6265 is not right.

IP addresses are safe according to the first rule: "The domain string and the string are identical."

That is sufficient. The other condition is about matching subdomains and there IP addresses are not safe. That condition fails, but the first sentence says "at least one of the ... conditions", so it does not matter.

I quote the section here to show the two levels of lists:

5.1.3. Domain Matching

A string domain-matches a given domain string if at least one of the following conditions hold:

• The domain string and the string are identical. (Note that both the domain string and the string will have been canonicalized to lower case at this point.)

• All of the following conditions hold:

  • The domain string is a suffix of the string.

  • The last character of the string that is not included in the domain string is a %x2E (".") character.

  • The string is a host name (i.e., not an IP address).

This would explain why everyone else does it differently.

Thanks for your work on aiohttp!

[asvetlov]

Domain string is not an IP address but the domain name, isn't it?

Anyway, the issue appears only if local server is tested IMHO. Passing unsafe for this case is pretty reasonable.

[kohtala]

The component processing defined in 5.1.3 is domain-matching with values as specified in 5.3 step 6 and 5.4 step 1.

The string is request-host as defined in 2.3 and domain-string is based on 4.1.1 syntax that says

domain-value =
; defined in [RFC1034], Section 3.5, as
; enhanced by [RFC1123], Section 2.1

where the RFC1123 section 2.1 reference allows IP addresses as domain strings.

... One aspect of host name syntax is hereby changed: the
restriction on the first character is relaxed to allow either a
letter or a digit.

aiohttp currently requires unsafe for RFC compliant behaviour. I fail to see what is unsafe about matching identical IP addresses. Rules in 5.3 and 5.4 make sure the cookie is only sent to origin server and nowhere else.