From 0c5aded0922f80bd1a31c7d2a3974469a18160a8 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 6 Apr 2011 12:05:58 -0300 Subject: raise if someone tries to modify the cookies when it was already streamed back to the client or converted to HTTP headers --- actionpack/test/dispatch/cookies_test.rb | 51 ++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) (limited to 'actionpack/test/dispatch') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 39159fd629..0d374e1d8b 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -495,3 +495,54 @@ class CookiesTest < ActionController::TestCase end end end + +class CookiesIntegrationTest < ActionDispatch::IntegrationTest + class TestController < ActionController::Base + def dont_set_cookies + head :ok + end + + def set_cookies + cookies["that"] = "hello" + head :ok + end + end + + def test_setting_cookies_raises_after_stream_back_to_client + with_test_route_set do + env = {} + get '/set_cookies', nil, env + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar['alert'] = 'alert' + cookies['alert'] = 'alert' + } + end + end + + def test_setting_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash + with_test_route_set do + env = {} + get '/dont_set_cookies', nil, {} + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar['alert'] = 'alert' + } + end + end + + private + + def with_test_route_set + with_routing do |set| + set.draw do + match ':action', :to => CookiesIntegrationTest::TestController + end + + @app = self.class.build_app(set) do |middleware| + middleware.use ActionDispatch::Cookies + middleware.delete "ActionDispatch::ShowExceptions" + end + + yield + end + end +end -- cgit v1.2.3 From 0d455673620a1646b1149e1e4a185143d46bb7b8 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 6 Apr 2011 19:12:32 -0300 Subject: Add tests to verify that signed and permanent cookies raises if someone tries to modify the cookies when it was already streamed back to the client or converted to HTTP headers --- actionpack/test/dispatch/cookies_test.rb | 51 ++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) (limited to 'actionpack/test/dispatch') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 0d374e1d8b..2a32614ca1 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -497,6 +497,9 @@ class CookiesTest < ActionController::TestCase end class CookiesIntegrationTest < ActionDispatch::IntegrationTest + SessionKey = '_myapp_session' + SessionSecret = 'b3c631c314c0bbca50c1b2843150fe33' + class TestController < ActionController::Base def dont_set_cookies head :ok @@ -529,8 +532,56 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest end end + def test_setting_permanent_cookies_raises_after_stream_back_to_client + with_test_route_set do + env = {} + get '/set_cookies', nil, env + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar.permanent['alert'] = 'alert' + cookies['alert'] = 'alert' + } + end + end + + def test_setting_permanent_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash + with_test_route_set do + env = {} + get '/dont_set_cookies', nil, {} + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar.permanent['alert'] = 'alert' + } + end + end + + def test_setting_signed_cookies_raises_after_stream_back_to_client + with_test_route_set do + env = {} + get '/set_cookies', nil, env + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar.signed['alert'] = 'alert' + cookies['alert'] = 'alert' + } + end + end + + def test_setting_signed_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash + with_test_route_set do + env = {} + get '/dont_set_cookies', nil, {} + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar.signed['alert'] = 'alert' + } + end + end + private + # Overwrite get to send SessionSecret in env hash + def get(path, parameters = nil, env = {}) + env["action_dispatch.secret_token"] ||= SessionSecret + super + end + def with_test_route_set with_routing do |set| set.draw do -- cgit v1.2.3 From ed042436295f0f55dc57c582d0b94628e6376e97 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 6 Apr 2011 19:15:33 -0300 Subject: Delete useless env variable --- actionpack/test/dispatch/cookies_test.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'actionpack/test/dispatch') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 2a32614ca1..2eb477e472 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -513,8 +513,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_cookies_raises_after_stream_back_to_client with_test_route_set do - env = {} - get '/set_cookies', nil, env + get '/set_cookies', nil, {} assert_raise(ActionDispatch::ClosedError) { request.cookie_jar['alert'] = 'alert' cookies['alert'] = 'alert' @@ -524,7 +523,6 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash with_test_route_set do - env = {} get '/dont_set_cookies', nil, {} assert_raise(ActionDispatch::ClosedError) { request.cookie_jar['alert'] = 'alert' @@ -534,8 +532,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_permanent_cookies_raises_after_stream_back_to_client with_test_route_set do - env = {} - get '/set_cookies', nil, env + get '/set_cookies', nil, {} assert_raise(ActionDispatch::ClosedError) { request.cookie_jar.permanent['alert'] = 'alert' cookies['alert'] = 'alert' @@ -545,7 +542,6 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_permanent_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash with_test_route_set do - env = {} get '/dont_set_cookies', nil, {} assert_raise(ActionDispatch::ClosedError) { request.cookie_jar.permanent['alert'] = 'alert' @@ -555,8 +551,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_signed_cookies_raises_after_stream_back_to_client with_test_route_set do - env = {} - get '/set_cookies', nil, env + get '/set_cookies', nil, {} assert_raise(ActionDispatch::ClosedError) { request.cookie_jar.signed['alert'] = 'alert' cookies['alert'] = 'alert' @@ -566,7 +561,6 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_signed_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash with_test_route_set do - env = {} get '/dont_set_cookies', nil, {} assert_raise(ActionDispatch::ClosedError) { request.cookie_jar.signed['alert'] = 'alert' -- cgit v1.2.3 From 9f765f4e0990742519b91d759a6b4374d940ab98 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 6 Apr 2011 19:17:39 -0300 Subject: Delete useless arguments --- actionpack/test/dispatch/cookies_test.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'actionpack/test/dispatch') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 2eb477e472..1d06a755b8 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -513,7 +513,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_cookies_raises_after_stream_back_to_client with_test_route_set do - get '/set_cookies', nil, {} + get '/set_cookies' assert_raise(ActionDispatch::ClosedError) { request.cookie_jar['alert'] = 'alert' cookies['alert'] = 'alert' @@ -523,7 +523,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash with_test_route_set do - get '/dont_set_cookies', nil, {} + get '/dont_set_cookies' assert_raise(ActionDispatch::ClosedError) { request.cookie_jar['alert'] = 'alert' } @@ -532,7 +532,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_permanent_cookies_raises_after_stream_back_to_client with_test_route_set do - get '/set_cookies', nil, {} + get '/set_cookies' assert_raise(ActionDispatch::ClosedError) { request.cookie_jar.permanent['alert'] = 'alert' cookies['alert'] = 'alert' @@ -542,7 +542,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_permanent_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash with_test_route_set do - get '/dont_set_cookies', nil, {} + get '/dont_set_cookies' assert_raise(ActionDispatch::ClosedError) { request.cookie_jar.permanent['alert'] = 'alert' } @@ -551,7 +551,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_signed_cookies_raises_after_stream_back_to_client with_test_route_set do - get '/set_cookies', nil, {} + get '/set_cookies' assert_raise(ActionDispatch::ClosedError) { request.cookie_jar.signed['alert'] = 'alert' cookies['alert'] = 'alert' @@ -561,7 +561,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest def test_setting_signed_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash with_test_route_set do - get '/dont_set_cookies', nil, {} + get '/dont_set_cookies' assert_raise(ActionDispatch::ClosedError) { request.cookie_jar.signed['alert'] = 'alert' } -- cgit v1.2.3 From 29592a7f09dda2e7e1e0a915d9230fe6a9b5c0af Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 6 Apr 2011 20:53:48 -0300 Subject: Use freeze instead of close! --- actionpack/test/dispatch/cookies_test.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'actionpack/test/dispatch') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 1d06a755b8..cfa521b2d9 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -502,10 +502,16 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest class TestController < ActionController::Base def dont_set_cookies + # initialize lazy loaded objects + cookies.permanent + cookies.signed head :ok end def set_cookies + # initialize lazy loaded objects + cookies.permanent + cookies.signed cookies["that"] = "hello" head :ok end -- cgit v1.2.3 From 17205435f8b8c01cc0ef72d8b92faf771bae1b40 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 7 Apr 2011 09:20:35 -0300 Subject: cookies here --- actionpack/test/dispatch/cookies_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack/test/dispatch') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index cfa521b2d9..cd07230a57 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -527,7 +527,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest end end - def test_setting_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash + def test_setting_cookies_raises_after_stream_back_to_client_even_without_cookies with_test_route_set do get '/dont_set_cookies' assert_raise(ActionDispatch::ClosedError) { @@ -546,7 +546,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest end end - def test_setting_permanent_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash + def test_setting_permanent_cookies_raises_after_stream_back_to_client_even_without_cookies with_test_route_set do get '/dont_set_cookies' assert_raise(ActionDispatch::ClosedError) { @@ -565,7 +565,7 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest end end - def test_setting_signed_cookies_raises_after_stream_back_to_client_even_with_an_empty_flash + def test_setting_signed_cookies_raises_after_stream_back_to_client_even_without_cookies with_test_route_set do get '/dont_set_cookies' assert_raise(ActionDispatch::ClosedError) { -- cgit v1.2.3 From 03d561ad77085f17ba816ebec619a3d359b2164e Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 7 Apr 2011 09:26:04 -0300 Subject: Revert "Use freeze instead of close!" This reverts commit 29592a7f09dda2e7e1e0a915d9230fe6a9b5c0af. --- actionpack/test/dispatch/cookies_test.rb | 6 ------ 1 file changed, 6 deletions(-) (limited to 'actionpack/test/dispatch') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index cd07230a57..ebc16694db 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -502,16 +502,10 @@ class CookiesIntegrationTest < ActionDispatch::IntegrationTest class TestController < ActionController::Base def dont_set_cookies - # initialize lazy loaded objects - cookies.permanent - cookies.signed head :ok end def set_cookies - # initialize lazy loaded objects - cookies.permanent - cookies.signed cookies["that"] = "hello" head :ok end -- cgit v1.2.3 From a9f3c9da01d721963d05949604ead909aaabbf36 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Mon, 11 Apr 2011 00:52:42 +0800 Subject: Using Object#in? and Object#either? in various places There're a lot of places in Rails source code which make a lot of sense to switching to Object#in? or Object#either? instead of using [].include?. --- actionpack/test/dispatch/routing_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack/test/dispatch') diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 5e5758a60e..51355b3ac7 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -1,6 +1,7 @@ require 'erb' require 'abstract_unit' require 'controller/fake_controllers' +require 'active_support/core_ext/object/inclusion' class TestRoutingMapper < ActionDispatch::IntegrationTest SprocketsApp = lambda { |env| @@ -495,7 +496,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest resources :todos, :id => /\d+/ end - scope '/countries/:country', :constraints => lambda { |params, req| %[all France].include?(params[:country]) } do + scope '/countries/:country', :constraints => lambda { |params, req| params[:country].either?("all", "france") } do match '/', :to => 'countries#index' match '/cities', :to => 'countries#cities' end -- cgit v1.2.3 From d6edaeeaf8b6c0f0b741c0827ab6e091bdd4e197 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Mon, 11 Apr 2011 12:35:20 +0700 Subject: Fix failing test case on master It turned out that I overlook at some replacements .. --- actionpack/test/dispatch/routing_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/test/dispatch') diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 51355b3ac7..8d23d63ebb 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -496,7 +496,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest resources :todos, :id => /\d+/ end - scope '/countries/:country', :constraints => lambda { |params, req| params[:country].either?("all", "france") } do + scope '/countries/:country', :constraints => lambda { |params, req| params[:country].either?("all", "France") } do match '/', :to => 'countries#index' match '/cities', :to => 'countries#cities' end -- cgit v1.2.3 From d1575ae1b9658c91145d6a46ec2a144a5a089207 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 12 Apr 2011 00:23:07 +0200 Subject: Change Object#either? to Object#among? -- thanks to @jamesarosen for the suggestion! --- actionpack/test/dispatch/routing_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/test/dispatch') diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 8d23d63ebb..cf7a5aaa3b 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -496,7 +496,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest resources :todos, :id => /\d+/ end - scope '/countries/:country', :constraints => lambda { |params, req| params[:country].either?("all", "France") } do + scope '/countries/:country', :constraints => lambda { |params, req| params[:country].among?("all", "France") } do match '/', :to => 'countries#index' match '/cities', :to => 'countries#cities' end -- cgit v1.2.3