From 0a34753dcdf7416c8abcc3b551eab039ae16bc75 Mon Sep 17 00:00:00 2001 From: Rich Cavanaugh Date: Wed, 7 May 2008 00:34:01 -0400 Subject: [PATCH] - CGI::Cookie should not split a String value on \n when dealing with a Hash in the name argument - No reason for CGI::Session::CookieStore to deal with escaping, this is handled later in the request cycle. This was double escaping the cookies resulting in interoperability issues with Merb and a minor bit of extra work. --- actionpack/lib/action_controller/cgi_ext/cookie.rb | 2 +- .../lib/action_controller/session/cookie_store.rb | 4 ++-- actionpack/test/controller/cookie_test.rb | 5 +++++ .../test/controller/session/cookie_store_test.rb | 13 +++++++++++-- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/actionpack/lib/action_controller/cgi_ext/cookie.rb b/actionpack/lib/action_controller/cgi_ext/cookie.rb index 3dd374f..e35bab5 100644 --- a/actionpack/lib/action_controller/cgi_ext/cookie.rb +++ b/actionpack/lib/action_controller/cgi_ext/cookie.rb @@ -37,7 +37,7 @@ class CGI #:nodoc: @path = nil else @name = name['name'] - @value = Array(name['value']) + @value = name['value'].kind_of?(String) ? [name['value']] : Array(name['value']) @domain = name['domain'] @expires = name['expires'] @secure = name['secure'] || false diff --git a/actionpack/lib/action_controller/session/cookie_store.rb b/actionpack/lib/action_controller/session/cookie_store.rb index 560491f..1e5e9cd 100644 --- a/actionpack/lib/action_controller/session/cookie_store.rb +++ b/actionpack/lib/action_controller/session/cookie_store.rb @@ -130,13 +130,13 @@ class CGI::Session::CookieStore # Marshal a session hash into safe cookie data. Include an integrity hash. def marshal(session) data = ActiveSupport::Base64.encode64(Marshal.dump(session)).chop - CGI.escape "#{data}--#{generate_digest(data)}" + "#{data}--#{generate_digest(data)}" end # Unmarshal cookie data to a hash and verify its integrity. def unmarshal(cookie) if cookie - data, digest = CGI.unescape(cookie).split('--') + data, digest = cookie.split('--') unless digest == generate_digest(data) delete raise TamperedWithCookie diff --git a/actionpack/test/controller/cookie_test.rb b/actionpack/test/controller/cookie_test.rb index 42f3bd2..b3c1a85 100644 --- a/actionpack/test/controller/cookie_test.rb +++ b/actionpack/test/controller/cookie_test.rb @@ -137,4 +137,9 @@ class CookieTest < Test::Unit::TestCase cookies = CGI::Cookie.parse('return_to=http://rubyonrails.org/search?term=api&scope=all&global=true') assert_equal({"return_to" => ["http://rubyonrails.org/search?term=api&scope=all&global=true"]}, cookies) end + + def test_cookies_should_not_be_split_on_values_with_newlines + cookies = CGI::Cookie.new("name" => "val", "value" => "this\nis\na\ntest") + assert cookies.size == 1 + end end diff --git a/actionpack/test/controller/session/cookie_store_test.rb b/actionpack/test/controller/session/cookie_store_test.rb index d308f2a..afbb736 100755 --- a/actionpack/test/controller/session/cookie_store_test.rb +++ b/actionpack/test/controller/session/cookie_store_test.rb @@ -43,13 +43,22 @@ class CookieStoreTest < Test::Unit::TestCase { :empty => ['BAgw--0686dcaccc01040f4bd4f35fe160afe9bc04c330', {}], :a_one => ['BAh7BiIGYWkG--5689059497d7f122a7119f171aef81dcfd807fec', { 'a' => 1 }], :typical => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7BiILbm90aWNlIgxIZXkgbm93--9d20154623b9eeea05c62ab819be0e2483238759', { 'user_id' => 123, 'flash' => { 'notice' => 'Hey now' }}], - :flashed => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA%3D%3D--bf9785a666d3c4ac09f7fe3353496b437546cfbf', { 'user_id' => 123, 'flash' => {} }] } + :flashed => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA==--bf9785a666d3c4ac09f7fe3353496b437546cfbf', { 'user_id' => 123, 'flash' => {} }] } end def setup ENV.delete('HTTP_COOKIE') end def test_raises_argument_error_if_missing_session_key [nil, ''].each do |blank| assert_raise(ArgumentError, blank.inspect) { new_session 'session_key' => blank } @@ -241,6 +250,6 @@ class CookieStoreWithMD5DigestTest < CookieStoreTest { :empty => ['BAgw--0415cc0be9579b14afc22ee2d341aa21', {}], :a_one => ['BAh7BiIGYWkG--5a0ed962089cc6600ff44168a5d59bc8', { 'a' => 1 }], :typical => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7BiILbm90aWNlIgxIZXkgbm93--f426763f6ef435b3738b493600db8d64', { 'user_id' => 123, 'flash' => { 'notice' => 'Hey now' }}], - :flashed => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA%3D%3D--0af9156650dab044a53a91a4ddec2c51', { 'user_id' => 123, 'flash' => {} }] } + :flashed => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA==--0af9156650dab044a53a91a4ddec2c51', { 'user_id' => 123, 'flash' => {} }] } end end -- 1.5.5.1