refactor Request#cookies, fixes #225

2 files changed, 27 insertions, 16 deletions

diff --git a/lib/rack/request.rb b/lib/rack/request.rb
index 59987bf3..0df78a93 100644
--- a/lib/rack/request.rb
+++ b/lib/rack/request.rb
@@ -230,22 +230,20 @@ module Rack
     end
 
     def cookies
-      return {}  unless @env["HTTP_COOKIE"]
-
-      if @env["rack.request.cookie_string"] == @env["HTTP_COOKIE"]
-        @env["rack.request.cookie_hash"]
-      else
-        @env["rack.request.cookie_string"] = @env["HTTP_COOKIE"]
-        # According to RFC 2109:
-        #   If multiple cookies satisfy the criteria above, they are ordered in
-        #   the Cookie header such that those with more specific Path attributes
-        #   precede those with less specific.  Ordering with respect to other
-        #   attributes (e.g., Domain) is unspecified.
-        @env["rack.request.cookie_hash"] =
-          Hash[*Utils.parse_query(@env["rack.request.cookie_string"], ';,').map {|k,v|
-            [k, Array === v ? v.first : v]
-          }.flatten]
-      end
+      hash   = @env["rack.request.cookie_hash"] ||= {}
+      string = @env["HTTP_COOKIE"]
+
+      hash.clear unless string
+      return hash if string == @env["rack.request.cookie_string"]
+
+      # According to RFC 2109:
+      #   If multiple cookies satisfy the criteria above, they are ordered in
+      #   the Cookie header such that those with more specific Path attributes
+      #   precede those with less specific.  Ordering with respect to other
+      #   attributes (e.g., Domain) is unspecified.
+      Utils.parse_query(string, ';,').each { |k,v| hash[k] = Array(v).first }
+      @env["rack.request.cookie_string"] = string
+      hash
     end
 
     def xhr?
diff --git a/test/spec_request.rb b/test/spec_request.rb
index 5c089b5e..0196267f 100644
--- a/test/spec_request.rb
+++ b/test/spec_request.rb
@@ -351,6 +351,19 @@ describe Rack::Request do
     req.cookies.should.equal({})
   end
 
+  should "always return the same hash object" do
+    req = Rack::Request.new \
+      Rack::MockRequest.env_for("", "HTTP_COOKIE" => "foo=bar;quux=h&m")
+    hash = req.cookies
+    req.env.delete("HTTP_COOKIE")
+    req.cookies.should.equal(hash)
+  end
+
+  should "raise any errors on every request" do
+    req = Rack::Request.new Rack::MockRequest.env_for("", "HTTP_COOKIE" => "foo=%")
+    2.times { proc { req.cookies }.should.raise(ArgumentError) }
+  end
+
   should "parse cookies according to RFC 2109" do
     req = Rack::Request.new \
       Rack::MockRequest.env_for('', 'HTTP_COOKIE' => 'foo=bar;foo=car')