ninenines / cowboy

Small, fast, modern HTTP server for Erlang/OTP.
https://ninenines.eu
ISC License
7.29k stars 1.17k forks source link

Cookie: Foo=Bar= chokes #340

Closed dvv closed 11 years ago

dvv commented 11 years ago

Hi!

cowboy_req:cookie(<<"foo">>, Req) chokes when request headers contain a cookie with value ending with =, say Cookie: Foo=Bar=, which is the common case when base64:encode/1 was used to make cookie a valid HTTP header.

Please, confirm/void. I couldn't find any pointers whether having = is invalid in cookie value, and I believe it's just valid. If so and cowboy doesn't per se imply this limitation somewhere, please consider fixing the parser.

TIA, --Vladimir

essen commented 11 years ago

The cookie value is a word which is token|quoted_string and a token is:

          token          = 1*<any CHAR except CTLs or tspecials>

          tspecials      = "(" | ")" | "<" | ">" | "@"
                         | "," | ";" | ":" | "\" | <">
                         | "/" | "[" | "]" | "?" | "="
                         | "{" | "}" | SP | HT

What's the code sending this cookie? Cookie: Foo="Bar=" would work, otherwise you should use base62 encoding which is guaranteed to work.

essen commented 11 years ago

Edited a typo.

dvv commented 11 years ago

Please try to test a simple cowboy server which echoes back the value of a Foo cookie with

curl -v -b "Foo=Bar=" http://localhost:8080/
essen commented 11 years ago

That's not a valid cookie value like I said. :)

dvv commented 11 years ago
curl -v -b 'Foo="Bar="' http://localhost:8080/

does deliver escaped cookie to cowboy.

But

cowboy_req:set_resp_cookie(<<"Foo">>, <<$", "Bar=", $">>, Opts, Req)

sets the following to the resp_headers

{<<"set-cookie">>, [<<"Foo">>,<<"=">>,<<"\\\"Bar=\\\"">>, ...

and

cowboy_req:set_resp_cookie(<<"Foo">>, <<"Bar=">>, Opts, Req)

sets the following to the resp_headers

{<<"set-cookie">>, [<<"Foo">>,<<"=">>,<<"Bar=">>, ...

and both effectively respond with wrong values (<<"Foo=\"Bar=\"">> or <<"Foo=Bar=">>, which results in peer receiving wrong cookie value.

So questions:

essen commented 11 years ago

I'm confused, why would Foo, Bar= give s, HZ=?

Also, the quoting in the cookie value from set_resp_cookie is wrong, I forgot something.

dvv commented 11 years ago

Don't be confused, I copy-pasted from real stacktrace. I'll update the post

essen commented 11 years ago

Please check 9821290d69cc24d623f8782b48254bcbd8f203e5.

dvv commented 11 years ago

Pulled afresh.

Below is testing handler which I presume should just relay the cookie:

handle(Req, State) ->
   SessionCookie = <<"s">>,
   SessionSecret = <<"FOO">>,
   SessionMaxAge = 1000,
   SessionDefault = {foo, bar},
   % get session cookie
   {Cookie, Req2} = cowboy_req:cookie(SessionCookie, Req),
   Body = case Cookie of undefined -> <<"NONE">>; _Else -> Cookie end,
   Cookie2 = <<$", "HZ=", $">>,
   Req3 = cowboy_req:set_resp_cookie(SessionCookie, Cookie2, [
       http_only,
       {max_age, 1000}
     ], Req2),
   {ok, Req4} = cowboy_req:reply(200, [], Body, Req3),
   {ok, Req4, State}.

Passing it "HZ=" value results in getting "\"HZ=\"" value. I believe it's still not right.

Can we instead of unconditional (right?) use of quote/2 rely on additional option to cowboy_req:set_resp_cookie/4, say, named escape, to enclose the passed cookie value verbatim with double quotes?

essen commented 11 years ago

Getting the value where?

I don't want a set_resp_cookie/4.

dvv commented 11 years ago

I meant getting back in Set-Cookie: If you run

curl -v -b 'S="HZ="' http://localhost:8080/

on the handler in question, you'll see set-cookie: s="\"HZ=\""; in output.

But you do have set_resp_cookie/4.

To sum: The best would be if it analysed the value passed on presence of tspecials and enclose value with double quotes @automatically@, as it's not user function to think of http protocol details. If such magic is undesirable, an option to do so is welcome.

essen commented 11 years ago

Yes that's the expected output for Set-Cookie for your code. :)

dvv commented 11 years ago

So, to clarify, there's no way to pipe a cookie intact, right? First its name is mangled, then aggressive escaping applied.

essen commented 11 years ago

The cookie value is the same as what you gave. You gave <<$", "HZ=", $">> and it returned \"HZ=\". Looks pretty identical to me. Don't confuse the set-cookie syntax with the actual value.

As for the name, it means the same regardless of the case.

dvv commented 11 years ago

I see. Lemme check afresh.

dvv commented 11 years ago

The result of the struggle here -- feedback is very welcome.

Providing #341 solved, this issue is closed.

I thank you. --Vladimir

essen commented 11 years ago

This one still doesn't work. I'll go through this part of cookies tomorrow and I'll publish a cookie example alongside it.

dvv commented 11 years ago

it's np since https://github.com/dvv/cookie_session/commit/6f00d59cda5c99d3bf5e60ac841263d55a8e6470 -- i doublequote cookie value manually since i know value can contain =s and can't contain "s.

To me, it's closed :)

essen commented 11 years ago

Yeah but I need to make sure that what we send can be received the same. Hence the example to use the JS sent cookies for testing. I'll use this ticket for notes.

essen commented 11 years ago

OK this changed again. You'll want to remove these double quotes and it should work fine.

Thanks for your patience! Reopen if there's still a problem.

dvv commented 11 years ago

confirm double quotes must be removed. thank you!