Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Cookie value with spaces #601

Closed
ddosia opened this Issue · 11 comments

3 participants

@ddosia

Recently we upgraded our cowboy from "0.6.0" to "0.8.6" and notice some odd cookie handling behavior. Further investigation lead us to a cause: some external web page component set cookie on a web site with such value:

<<"cookiename=Thu Jul 11 2013 15:38:43 GMT+0400 (MSK)">>

As a result cookie parsing cowboy_http:cookie_list/1 start to return {error, badarg} and whole cookie list became an empty list.

I compare this behaviour with "0.6.0" and it is different. Although this cookie is still broken (it will be [{<<"cookiename">>,<<"Thu">>}]), but other cookies are safe.

Moreover I compared it with how php does this, and result is full variable value.

I read rfc2109, rfc2965 and rfc6265 and it is still not clear to me is it possible put spaces in cookie values.
As I see from comment https://github.com/extend/cowboy/blob/0.8.6/src/cowboy_http.erl#L98 cowboy keeps in mind rfc2109 as a rule of cookie parsing.
rfc2109 states that value is a token | quoted-string and token is

a sequence of non-special, non-white space characters

Sentence near:

White space is permitted between tokens

I don't know who is right and how this rfc should be read, but truth is, currently any external component (i.e. google analytics) could be a reason why all cookies looks like empty for cowboy.

@ddosia

I have a fix for that, basically I surround funs which extracts cookie name and cookie val with whitespace/2 fun, if this is acceptable I could send a PR

@ddosia ddosia referenced this issue from a commit in unisontech/cowboy
@ddosia ddosia Fix broken cookie handling (#601) fa5289e
@ronalfei

I have this problem too,
another way is remove " C =:= $\s;" at line 159,
https://github.com/extend/cowboy/blob/0.8.6/src/cowboy_http.erl#L159

@essen
Owner

Yeah it's the other side of the code that does it like PHP, this side doesn't yet.

@essen
Owner

Well even the latest spec doesn't seem to allow whitespaces in values. But I suppose we'll have to not follow the spec here too. PHP is incredibly permissive, it pretty much just cuts ; and then cuts = and that's it. It also urldecodes both the cookie name and value, I still don't understand why. I'll revisit the code tomorrow.

@ddosia

@essen because some web devs who knows how browsers do compose cookies header do javascript's encode(CookieVal), which basically does base64. But not everybody use it.

@essen
Owner

Urlencode and base64 is hardly the same, but I guess it's just a matter of allowing any value, yes.

@essen
Owner

Rails also urlencode apparently.

I'll leave that out of Cowboy though since there are situations where you don't want that to happen.

@essen
Owner

So I rewrote the cookie parsing function and made it more efficient in the process (I was meaning to do that anyway) and added support for old style cookies with spaces in them. I will push that either soon today or tomorrow.

@essen
Owner

Please try faf6452. It should now be fixed.

@ddosia

@essen seems that everything works fine. Agreed that keeping urldecode is better to leave for those, who know what they are doing. Thanks.

@essen
Owner

Awesome. Closing then, thanks!

@essen essen closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.