Cookie filter #799

panda73111 · 2016-02-22T19:56:03Z

Relevant issue: #792
This branch will add cookie management adhering to RFC 6265.
Right now, it adds a simple filter for cookies sent, regarding their "Domain" attribute.

Cookies passed to ClientSession without a "Domain" attribute (as SimpleCookie() or dict/tuple) are still shared across all requests, to keep backwards compatibility.
Leading dots are ignored (Domain=.example.com becomes Domain=example.com)
Cookies with matching domain are sent (Domain=example.com is sent to sub.example.com, but not the other way around)
Only shared cookies are sent to addresses that are not a domain, like an IP address

More filters (for the remaining attributes and for storing incoming cookies) will be added as soon as I have the time, until eventually ClientSession behaves like stated in RFC 6265.
CORS is completely new to me and low priority until further research.
This branch can be merged into master at any time, I'll add pull requests as needed and all tests pass.

EDIT 23.02.2016:

The Domain attribute of received cookies is set to the response hostname if empty, their host-only-flag is set to True
Cookies from IP addresses are rejected

EDIT 29.02.2016:

Cookies with secure-flag set are only sent to secure hosts (https://)

EDIT 18.03.2016:

Cookies with matching path are sent
If received cookies do not have the path-attribute defined, it is set to the response path

EDIT 20.03.2016:

Stored cookies are deleted when the time defined in the max-age- or expires-attribute is reached. A timer is set via loop.call_later()/call_at() when a cookie is added, that removes the cookie from the jar.

kxepal · 2016-02-22T20:04:11Z

aiohttp/client.py

+        """Returns this session's cookies filtered by their attributes"""
+        # TODO: filter by 'expires', 'path', ...
+        netloc = urllib.parse.urlsplit(url).netloc
+        is_ip = IPV4_PATTERN.match(netloc) is not None


ipaddress.IPAddress(netloc) to support both IPv4 and IPv6 in the right way without regexps (which accepts invalid IPv4 addresses).

…mpleCookie class modifies value before Python 3.4"

…Fixed handling starting dot in cookie domains

asvetlov · 2016-02-24T12:37:24Z

Looks interesting. I need reread RFC first for making careful review.
I hope to make it during weekend.

Anyway I have a strong feeling that we need to fix cookies problem in the next aiohttp release.

…ieStore, which ClientSession refers to

…ering methods inside it

coveralls · 2016-03-18T19:01:22Z

Coverage decreased (-0.2%) to 98.24% when pulling 4959234 on panda73111:cookie_filter into 496779e on KeepSafe:master.

coveralls · 2016-03-19T23:41:56Z

Coverage decreased (-0.3%) to 98.119% when pulling 1cfb2c6 on panda73111:cookie_filter into 496779e on KeepSafe:master.

panda73111 · 2016-03-20T15:23:38Z

I added the Expires- and Max-Age-attributes, by scheduling deletion of the cookie via call_later()/call_at(). But this gets problematic as soon as the user changes these attributes, the cookie would still be removed at the original timestamp. How should this be handled? For "Expires", an absolute date, the cookie could just remain in the cookie jar, and just not be sent if the time of sending the request is after the date defined in the "Expires" attribute. But "Max-Age" only consists of a time span in seconds, of which it is not possible to know when it has been set by the user.

panda73111 · 2016-04-06T18:19:31Z

...Or maybe any other comments? This is basically done, as it is.

(invalid values, host only cookie, domain matching, path matching, date parsing)

auvipy · 2016-04-24T03:08:42Z

you could fix the merge conflicts and failing builds

# Conflicts: # aiohttp/client.py

coveralls · 2016-04-24T12:45:11Z

Coverage decreased (-1.1%) to 97.068% when pulling f8c0e9a on panda73111:cookie_filter into 024f6e1 on KeepSafe:master.

coveralls · 2016-04-24T12:55:30Z

Coverage increased (+0.04%) to 98.26% when pulling f6a5da4 on panda73111:cookie_filter into 024f6e1 on KeepSafe:master.

panda73111 · 2016-04-24T13:26:02Z

Well, I wanted my code to get approved first, since I don't want to create a merge commit every time master becomes incompatible, until this gets merged or rejected eventually.

And what's up with these coverage comments? Shouldn't these be posted only after all tests pass? The next commit makes it up to 98.26% again only because flake8 is satisfied...
(Also, coveralls doesn't let me view 'changed' lines, because "The owner of this repo needs to re-authorize with github; their OAuth credentials are no longer valid so the file cannot be pulled from the github API.")

KostyaEsmukov · 2016-05-23T04:42:32Z

Any update on this?
@asvetlov @fafhrd91

fafhrd91 · 2016-05-23T18:06:24Z

aiohttp/helpers.py

+        return non_matching.startswith("/")
+
+    @classmethod
+    def _parse_date(cls, date_str):


can't we reuse some existing function?

fafhrd91 · 2016-05-23T18:08:18Z

i am +1 for this change, but we need opinion from @asvetlov before inclusion.

asvetlov · 2016-06-03T22:10:56Z

Squashed and merged as 0016999
@panda73111 thanks you very much for your hard work

lock · 2019-10-29T18:04:12Z

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

panda73111 added 2 commits February 22, 2016 20:16

Implemented simple sending cookie domain filter

e84757a

Added tests for simple sending cookie domain filter

67f278d

kxepal reviewed Feb 22, 2016
View reviewed changes

panda73111 added 9 commits February 22, 2016 21:09

Fixed flake8 errors

eb8ff5c

Using ipaddress module to determine if a netloc is an IP address

1def5b0

Added workaround somewhere else commented "use dict method because Si…

fbe82c4

…mpleCookie class modifies value before Python 3.4"

Fixed errors with ports in URLs by using hostname instead of netloc; …

4bd4a4e

…Fixed handling starting dot in cookie domains

Fixed not accepting cookies from IPs

87bad1e

Removed impossible condition

3088e09

Changed an obsolete test, where explicit cookie sharing is now required

c7a9efe

Added tests for filtering of received cookies

8720b8e

Implemented host-only-flag

f8812d5

asvetlov added this to the 0.22 milestone Feb 24, 2016

panda73111 added 11 commits February 24, 2016 19:04

Refactoring; Moved new code over to the new class helpers.SessionCook…

90bb61b

…ieStore, which ClientSession refers to

Split the tests into testing calls to SessionCookieStore and the filt…

7a19859

…ering methods inside it

Modified test_cookie_store_usage, now using mock.patch

5d5dd5e

Renamed SessionCookieStore to CookieJar

0d3c2be

Implemented filtering by secure-flag

1d28248

Added test for secure-flag

a648d14

Added rudimentary test for is_ip_address()

bb559cd

Implemented filtering by path-attribute

cdf703f

Added tests for path-attribute

1a409ea

Added test for path-attributes of received cookies

3be84c2

Using TestCase's assertEqual()

4959234

panda73111 added 3 commits March 20, 2016 00:21

Implemented expires- and max-age-attribute

2d21ff4

Added tests for expires- and max-age-attribute

2832ea4

Passing the ClientSession's loop through to its CookieJar

1cfb2c6

Added more tests

7917132

(invalid values, host only cookie, domain matching, path matching, date parsing)

Merge remote-tracking branch 'origin/master' into cookie_filter

f8c0e9a

# Conflicts: # aiohttp/client.py

Fixed flake8 "Missing whitespace around operator"

f6a5da4

fafhrd91 reviewed May 23, 2016
View reviewed changes

asvetlov closed this Jun 3, 2016

asvetlov mentioned this pull request Jun 3, 2016

ClientSession is leaking cookies across hostnames #792

Closed

asvetlov pushed a commit that referenced this pull request Jun 3, 2016

Implement cookie filter #799

0016999

asvetlov mentioned this pull request Jul 16, 2016

Enable cookie parsing for URLs which are IP-based, not DNS #968

Closed

lock bot added the outdated label Oct 29, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cookie filter #799

Cookie filter #799

panda73111 commented Feb 22, 2016

kxepal Feb 22, 2016

asvetlov commented Feb 24, 2016

coveralls commented Mar 18, 2016

coveralls commented Mar 19, 2016

panda73111 commented Mar 20, 2016

panda73111 commented Apr 6, 2016

auvipy commented Apr 24, 2016

coveralls commented Apr 24, 2016

coveralls commented Apr 24, 2016

panda73111 commented Apr 24, 2016

KostyaEsmukov commented May 23, 2016

fafhrd91 May 23, 2016

fafhrd91 commented May 23, 2016

asvetlov commented Jun 3, 2016

lock bot commented Oct 29, 2019

Cookie filter #799

Cookie filter #799

Conversation

panda73111 commented Feb 22, 2016

kxepal Feb 22, 2016

Choose a reason for hiding this comment

asvetlov commented Feb 24, 2016

coveralls commented Mar 18, 2016

coveralls commented Mar 19, 2016

panda73111 commented Mar 20, 2016

panda73111 commented Apr 6, 2016

auvipy commented Apr 24, 2016

coveralls commented Apr 24, 2016

coveralls commented Apr 24, 2016

panda73111 commented Apr 24, 2016

KostyaEsmukov commented May 23, 2016

fafhrd91 May 23, 2016

Choose a reason for hiding this comment

fafhrd91 commented May 23, 2016

asvetlov commented Jun 3, 2016

lock bot commented Oct 29, 2019