Bugfix for Cookies with multiple paths #197
Conversation
There was a problem hiding this comment.
Thanks for the effort @kylewelsby, much appreciated! 👍 A few things that I think should be addressed, but should be mergable afterwards.
spec/rack/test/cookie_spec.rb
Outdated
| @@ -45,7 +70,7 @@ | |||
| it 'allow symbol access' do | |||
| jar = Rack::Test::CookieJar.new | |||
| jar['value'] = 'foo;abc' | |||
| jar[:value].should == 'foo;abc' | |||
| expect(jar[:value]).to eql('foo;abc') | |||
There was a problem hiding this comment.
Prefer eq to eql, since that's what's being used elsewhere in the code. (unless it actually causes a different result, but I don't think it should in this case)
spec/rack/test/cookie_spec.rb
Outdated
| @@ -28,6 +28,31 @@ | |||
| expect(last_request.cookies).to eq({}) | |||
| end | |||
|
|
|||
| it 'uses the first path when many paths are defiend' do | |||
There was a problem hiding this comment.
good spot. did write this a few years ago.
spec/rack/test/cookie_spec.rb
Outdated
| ].join('; ') | ||
| cookie = Rack::Test::Cookie.new(cookie_string) | ||
| expect(cookie.path).to eql '/' | ||
| end |
There was a problem hiding this comment.
Typo in spec name, and also a misleading name (it only provides a single path 😄), so you probably intended to write something else?
Also, prefer eq to eql in the expect calls, in both new tests.
|
Hey @perlun I have pushed the requested changes. 😇 |
| ].join('; ') | ||
| cookie = Rack::Test::Cookie.new(cookie_string) | ||
| expect(cookie.path).to eq('/') | ||
| end |
There was a problem hiding this comment.
This one still has the typo + misleading name. it 'properly uses a single path being defined' or something is fine.
There was a problem hiding this comment.
I've updated this. Thanks for pointing it out.
|
Thanks @kylewelsby, will merge this now. |
* Bugfix for Cookies with multiple paths closes rack#16 * Fixed spec names
closes #16 "Rack::Test::CookieJar cannot handle multiple 'path' values in the Cookie."
However this is not a valid cookie string, I've found some web servers that ignore this and still deliver multiple keys.
http://www.w3.org/Protocols/rfc2109/rfc2109
4.2.2 Set-Cookie Syntax
rebased after notification from #92