Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
asvetlov opened this issue Jul 16, 2016 · 13 comments
Closed

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

asvetlov opened this issue Jul 16, 2016 · 13 comments

Comments

@asvetlov
Copy link
Member

Long story short

Working on aiohttp_session I've figured out that current behavior is not convenient for development and tests writing.

Expected behaviour

Don't ignore Set-Cookie from sites with IP address in URL

It's result of #799 PR

@panda73111 could you remind why you have added the filter?

As an option we can have StrictCookieJar class or strict=True CookieJar parameter with IP filter enabled but I believe default behavior should accept cookies from IP URLs.

@popravich what do you think?

@asvetlov
Copy link
Member Author

@kxepal @1st1 @fafhrd91 and others, I'm asking for your opinions

@fafhrd91
Copy link
Member

I don't have opinion on this topic. One note, would it be better to allow to use different implementation and provide cookies with filtering as an extra instead of adding more parameters?

@popravich
Copy link
Member

same as @fafhrd91

@kxepal
Copy link
Member

kxepal commented Jul 18, 2016

IP addresses in cookies are strongly discouraged by RFC. However, it's still allowed case per RFC-2109 and RFC-2965, but not for current one RFC-6265.

So it's ok to have that behaviour optional, but not by default.

@asvetlov
Copy link
Member Author

@kxepal I don't understand you clean: are you suggest allowing ip addresses by default or forbidding?

@kxepal
Copy link
Member

kxepal commented Jul 22, 2016

@asvetlov forbid as newest RFC defines and common guidelines defines. Enabling this is actually fallback to old RFCs where this is allowed, but discouraged, so I think that should be default state of things today.

@asvetlov
Copy link
Member Author

asvetlov commented Jul 22, 2016

Gotcha. Will publish bug fix release soon.

@asvetlov
Copy link
Member Author

BTW what is good name for the class?
Both RelaxedCookieJar and CookieJarWithoutIPFiltering looks ugly.

@kxepal
Copy link
Member

kxepal commented Jul 22, 2016

CookieJarDiscouraged

@asvetlov
Copy link
Member Author

@kxepal are you kidding me?

@kxepal
Copy link
Member

kxepal commented Jul 22, 2016

@asvetlov Sorry, but it was too seductive to not propose (:
Actually, why not to have a special option for CookieJar which controls that behaviour?

@asvetlov
Copy link
Member Author

Fixed by aiohttp 0.22.2 release.
CookieJar(unsafe=True) disables IP address check

@lock
Copy link

lock bot commented Oct 29, 2019

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.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants