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

Set-cookie parsing #65

Closed
rafirafi opened this issue Aug 13, 2014 · 2 comments
Closed

Set-cookie parsing #65

rafirafi opened this issue Aug 13, 2014 · 2 comments

Comments

@rafirafi
Copy link

Hi,

I'm talking about:

bool parser::parse_cookie_header(ihash_multimap& dict, const char *ptr, const size_t len, bool set_cookie_header)
in src/http_parser.cpp

From what I understand this function was for breaking an individual cookie in its components and to put them in a dictionnary.

Well it seems this function is called for every cookie, that all individual components are mixed together and added as cookies themselves.

P. ex. , if I download one page, this is what a call to pion::http::response_ptr::write give me :

Set-Cookie: PREF=ID=b255244245f28df7:FF=0:TM=1407897815:LM=1407897815:S=p0ASItwZjDttyxBq; expires=Fri, 12-Aug-2016 02:43:35 GMT; path=/; domain=.google.fr
Set-Cookie: path="/"; Version=1; Path=/
Set-Cookie: path="/"; Version=1; Path=/
Set-Cookie: HttpOnly=""; Version=1; Path=/
Set-Cookie: domain=".google.fr"; Version=1; Path=/
Set-Cookie: domain=".google.fr"; Version=1; Path=/
Set-Cookie: 12-Aug-201602:43:35GMT=""; Version=1; Path=/
Set-Cookie: expires="Thu"; Version=1; Path=/
Set-Cookie: expires="Fri"; Version=1; Path=/
Set-Cookie: NID="67=CBnGymFTcCnhhkVxY0pfFRIY_C8cWJx5j0ATXCifbiv7Plcl60Gq6OrznUOoi9ng24WV0U06zHhPkeg4WcWS7UjlFP-tRoLQknBWhZd7ADe479UoUNMNkZE38g5qSZfu"; Version=1; Path=/
Set-Cookie: 12-Feb-201502:43:35GMT=""; Version=1; Path=/
Set-Cookie: PREF="ID=b255244245f28df7:FF=0:TM=1407897815:LM=1407897815:S=p0ASItwZjDttyxBq"; Version=1; Path=/
Set-Cookie: NID=67=CBnGymFTcCnhhkVxY0pfFRIY_C8cWJx5j0ATXCifbiv7Plcl60Gq6OrznUOoi9ng24WV0U06zHhPkeg4WcWS7UjlFP-tRoLQknBWhZd7ADe479UoUNMNkZE38g5qSZfu; expires=Thu, 12-Feb-2015 02:43:35 GMT; path=/; domain=.google.fr; HttpOnly

And as control this is what I get from wireshark:

Set-Cookie: PREF=ID=b255244245f28df7:FF=0:TM=1407897815:LM=1407897815:S=p0ASItwZjDttyxBq; expires=Fri, 12-Aug-2016 02:43:35 GMT; path=/; domain=.google.fr
Set-Cookie: NID=67=CBnGymFTcCnhhkVxY0pfFRIY_C8cWJx5j0ATXCifbiv7Plcl60Gq6OrznUOoi9ng24WV0U06zHhPkeg4WcWS7UjlFP-tRoLQknBWhZd7ADe479UoUNMNkZE38g5qSZfu; expires=Thu, 12-Feb-2015 02:43:35 GMT; path=/; domain=.google.fr; HttpOnly

@MignonBelongie
Copy link

I studied this and found a bug in our parser, namely that cookie attribute names in Set-Cookie headers were not being matched case-insensitively, which I fixed in PION-1324.
Unfortunately, this doesn't entirely solve your problem, because you will still get some spurious cookies,
because for backwards compatibility we have to support the possibility of comma separated cookies in Set-Cookie headers, even though such usage is obsolete. Please see the comments in the new code for more information.

@rafirafi
Copy link
Author

Ok, I run my example again, and it looks much better :

Set-Cookie: NID=67=fQUaCFOC5HYBfQbH_sPIllgkZDOUq1focANHSckQP9MZxV9oulaBg104XnoOn_KR_e1W03CIuU7aprYUSncwGgrrcMqfzul97WZAvvbd0lXN-2xjoHVZb4aTKid7GX8z; expires=Fri, 27-Feb-2015 19:58:19 GMT; path=/; domain=.google.fr; HttpOnly
Set-Cookie: PREF="ID=ef83f38acec36e29:FF=0:TM=1409255899:LM=1409255899:S=wQNIzayl5DTMkMJr"; Version=1; Path=/
Set-Cookie: NID="67=fQUaCFOC5HYBfQbH_sPIllgkZDOUq1focANHSckQP9MZxV9oulaBg104XnoOn_KR_e1W03CIuU7aprYUSncwGgrrcMqfzul97WZAvvbd0lXN-2xjoHVZb4aTKid7GX8z"; Version=1; Path=/
Set-Cookie: 27-Feb-201519:58:19GMT=""; Version=1; Path=/
Set-Cookie: 27-Aug-201619:58:19GMT=""; Version=1; Path=/
Set-Cookie: PREF=ID=ef83f38acec36e29:FF=0:TM=1409255899:LM=1409255899:S=wQNIzayl5DTMkMJr; expires=Sat, 27-Aug-2016 19:58:19 GMT; path=/; domain=.google.fr

I dug a little and the comma in the "expires" field is a legacy from netscape's time and is actually a part of the format, but was defined before any spec exists.

What I have pasted before is all the Set-Cookies from a single call to:

boost::system::error_code ec;
pion::http::response_ptr response;
// get the response
response->write(std::cout, ec, true);

It displays the Set-Cookie from the response along with the result of the parsing. And if I undestand correctly the comment in the test case this should no longer be the case.

Anyway thanks for your reactivity.

firedaemon-cto pushed a commit to FireDaemon/pion that referenced this issue Aug 27, 2020
trac-12101: fix is_pos_infinity example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants