Skip to content

Loading…

Fix cookie deletion with path specified #208

Merged
merged 1 commit into from

2 participants

@joevandyk

The annoying thing about this bug is that if you:

  • create two cookies, one with a path of "/" and the other with a path of "/path"
  • delete one cookie with path=/path set
  • modify the cookie with path=/

The modified cookie with path=/ won't be set to the client. :(

@joevandyk

I think there's still bugs in how Rack deletes cookies -- if you specify both a domain and path to delete, I think the current code will pick the first cookie that matches the name and domain, instead of the first cookie to match the first name, path, and domain.

But this fixes at least one major bug.

@raggi raggi merged commit 1ce8d39 into rack:master
@raggi
Official Rack repositories member

Yeah, we definitely need to improve the rest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 13, 2011
  1. @joevandyk
Showing with 14 additions and 0 deletions.
  1. +2 −0 lib/rack/utils.rb
  2. +12 −0 test/spec_response.rb
View
2 lib/rack/utils.rb
@@ -228,6 +228,8 @@ def delete_cookie_header!(header, key, value = {})
cookies.reject! { |cookie|
if value[:domain]
cookie =~ /\A#{escape(key)}=.*domain=#{value[:domain]}/
+ elsif value[:path]
+ cookie =~ /\A#{escape(key)}=.*path=#{value[:path]}/
else
cookie =~ /\A#{escape(key)}=/
end
View
12 test/spec_response.rb
@@ -109,6 +109,18 @@
"foo=; domain=sample.example.com; expires=Thu, 01-Jan-1970 00:00:00 GMT"].join("\n")
end
+ it "can delete cookies with the same name with different paths" do
+ response = Rack::Response.new
+ response.set_cookie "foo", {:value => "bar", :path => "/"}
+ response.set_cookie "foo", {:value => "bar", :path => "/path"}
+ response["Set-Cookie"].should.equal ["foo=bar; path=/",
+ "foo=bar; path=/path"].join("\n")
+
+ response.delete_cookie "foo", :path => "/path"
+ response["Set-Cookie"].should.equal ["foo=bar; path=/",
+ "foo=; path=/path; expires=Thu, 01-Jan-1970 00:00:00 GMT"].join("\n")
+ end
+
it "can do redirects" do
response = Rack::Response.new
response.redirect "/foo"
Something went wrong with that request. Please try again.