Merge pull request #22828 from ma2gedev/should-escape-cookie

A cookie value is incorrect if value contains an escapable string in Rails 5 ActionController::TestCase

2 files changed, 21 insertions, 1 deletions

diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb
index 3477aa8b29..f2f3150b56 100644
--- a/actionpack/lib/action_dispatch/middleware/cookies.rb
+++ b/actionpack/lib/action_dispatch/middleware/cookies.rb
@@ -2,6 +2,7 @@ require 'active_support/core_ext/hash/keys'
 require 'active_support/key_generator'
 require 'active_support/message_verifier'
 require 'active_support/json'
+require 'rack/utils'
 
 module ActionDispatch
   class Request
@@ -337,7 +338,7 @@ module ActionDispatch
       end
 
       def to_header
-        @cookies.map { |k,v| "#{k}=#{v}" }.join ';'
+        @cookies.map { |k,v| "#{escape(k)}=#{escape(v)}" }.join '; '
       end
 
       def handle_options(options) #:nodoc:
@@ -419,6 +420,10 @@ module ActionDispatch
 
       private
 
+      def escape(string)
+        ::Rack::Utils.escape(string)
+      end
+
       def make_set_cookie_header(header)
         header = @set_cookies.inject(header) { |m, (k, v)|
           if write_cookie?(v)
diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb
index b9caddcdb7..aeea974212 100644
--- a/actionpack/test/controller/test_case_test.rb
+++ b/actionpack/test/controller/test_case_test.rb
@@ -137,6 +137,15 @@ XML
       head :created, location: 'created resource'
     end
 
+    def read_cookie
+      cookies["foo"]
+      render plain: 'ok'
+    end
+
+    def render_cookie
+      render plain: cookies["foo"]
+    end
+
     def delete_cookie
       cookies.delete("foo")
       render plain: 'ok'
@@ -829,6 +838,12 @@ XML
     assert_equal 'bar', cookies['foo']
   end
 
+  def test_cookies_should_be_escaped_properly
+    cookies['foo'] = '+'
+    get :render_cookie
+    assert_equal '+', @response.body
+  end
+
   def test_should_detect_if_cookie_is_deleted
     cookies['foo'] = 'bar'
     get :delete_cookie