Merge branch 'master' into bug/filtered_parameters_class

13 files changed, 206 insertions, 37 deletions

diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb
index 3e067314d6..ae2b45c9f0 100644
--- a/actionpack/test/controller/parameters/parameters_permit_test.rb
+++ b/actionpack/test/controller/parameters/parameters_permit_test.rb
@@ -377,17 +377,17 @@ class ParametersPermitTest < ActiveSupport::TestCase
     assert_equal "32", @params[:person].permit([ :age ])[:age]
   end
 
-  test "to_h returns empty hash on unpermitted params" do
-    assert @params.to_h.is_a? ActiveSupport::HashWithIndifferentAccess
-    assert_not @params.to_h.is_a? ActionController::Parameters
-    assert @params.to_h.empty?
+  test "to_h raises UnfilteredParameters on unfiltered params" do
+    assert_raises(ActionController::UnfilteredParameters) do
+      @params.to_h
+    end
   end
 
   test "to_h returns converted hash on permitted params" do
     @params.permit!
 
-    assert @params.to_h.is_a? ActiveSupport::HashWithIndifferentAccess
-    assert_not @params.to_h.is_a? ActionController::Parameters
+    assert_instance_of ActiveSupport::HashWithIndifferentAccess, @params.to_h
+    assert_not_kind_of ActionController::Parameters, @params.to_h
   end
 
   test "to_h returns converted hash when .permit_all_parameters is set" do
@@ -395,39 +395,71 @@ class ParametersPermitTest < ActiveSupport::TestCase
       ActionController::Parameters.permit_all_parameters = true
       params = ActionController::Parameters.new(crab: "Senjougahara Hitagi")
 
-      assert params.to_h.is_a? ActiveSupport::HashWithIndifferentAccess
-      assert_not @params.to_h.is_a? ActionController::Parameters
+      assert_instance_of ActiveSupport::HashWithIndifferentAccess, params.to_h
+      assert_not_kind_of ActionController::Parameters, params.to_h
       assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_h)
     ensure
       ActionController::Parameters.permit_all_parameters = false
     end
   end
 
-  test "to_h returns always permitted parameter on unpermitted params" do
-    params = ActionController::Parameters.new(
-      controller: "users",
-      action: "create",
-      user: {
-        name: "Sengoku Nadeko"
-      }
-    )
+  test "to_hash raises UnfilteredParameters on unfiltered params" do
+    assert_raises(ActionController::UnfilteredParameters) do
+      @params.to_hash
+    end
+  end
+
+  test "to_hash returns converted hash on permitted params" do
+    @params.permit!
+
+    assert_instance_of Hash, @params.to_hash
+    assert_not_kind_of ActionController::Parameters, @params.to_hash
+  end
+
+  test "parameters can be implicit converted to Hash" do
+    params = ActionController::Parameters.new
+    params.permit!
+
+    assert_equal({ a: 1 }, { a: 1 }.merge!(params))
+  end
 
-    assert_equal({ "controller" => "users", "action" => "create" }, params.to_h)
+  test "to_hash returns converted hash when .permit_all_parameters is set" do
+    begin
+      ActionController::Parameters.permit_all_parameters = true
+      params = ActionController::Parameters.new(crab: "Senjougahara Hitagi")
+
+      assert_instance_of Hash, params.to_hash
+      assert_not_kind_of ActionController::Parameters, params.to_hash
+      assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_hash)
+      assert_equal({ "crab" => "Senjougahara Hitagi" }, params)
+    ensure
+      ActionController::Parameters.permit_all_parameters = false
+    end
   end
 
   test "to_unsafe_h returns unfiltered params" do
-    assert @params.to_unsafe_h.is_a? ActiveSupport::HashWithIndifferentAccess
-    assert_not @params.to_unsafe_h.is_a? ActionController::Parameters
+    assert_instance_of ActiveSupport::HashWithIndifferentAccess, @params.to_unsafe_h
+    assert_not_kind_of ActionController::Parameters, @params.to_unsafe_h
   end
 
   test "to_unsafe_h returns unfiltered params even after accessing few keys" do
     params = ActionController::Parameters.new("f" => { "language_facet" => ["Tibetan"] })
     expected = { "f" => { "language_facet" => ["Tibetan"] } }
 
-    assert params["f"].is_a? ActionController::Parameters
+    assert_instance_of ActionController::Parameters, params["f"]
     assert_equal expected, params.to_unsafe_h
   end
 
+  test "to_unsafe_h does not mutate the parameters" do
+    params = ActionController::Parameters.new("f" => { "language_facet" => ["Tibetan"] })
+    params[:f]
+
+    params.to_unsafe_h
+
+    assert_not_predicate params, :permitted?
+    assert_not_predicate params[:f], :permitted?
+  end
+
   test "to_h only deep dups Ruby collections" do
     company = Class.new do
       attr_reader :dupped
diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb
index f06a1f4d23..5b16af78c4 100644
--- a/actionpack/test/controller/redirect_test.rb
+++ b/actionpack/test/controller/redirect_test.rb
@@ -285,10 +285,10 @@ class RedirectTest < ActionController::TestCase
   end
 
   def test_redirect_to_params
-    error = assert_raise(ArgumentError) do
+    error = assert_raise(ActionController::UnfilteredParameters) do
       get :redirect_to_params
     end
-    assert_equal ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE, error.message
+    assert_equal "unable to convert unpermitted parameters to hash", error.message
   end
 
   def test_redirect_to_with_block
diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb
index d645ddfdbe..521d93f02e 100644
--- a/actionpack/test/controller/request_forgery_protection_test.rb
+++ b/actionpack/test/controller/request_forgery_protection_test.rb
@@ -35,6 +35,22 @@ module RequestForgeryProtectionActions
     render inline: "<%= form_for(:some_resource, :remote => true, :authenticity_token => 'external_token') {} %>"
   end
 
+  def form_with_remote
+    render inline: "<%= form_with(scope: :some_resource) {} %>"
+  end
+
+  def form_with_remote_with_token
+    render inline: "<%= form_with(scope: :some_resource, authenticity_token: true) {} %>"
+  end
+
+  def form_with_local_with_token
+    render inline: "<%= form_with(scope: :some_resource, local: true, authenticity_token: true) {} %>"
+  end
+
+  def form_with_remote_with_external_token
+    render inline: "<%= form_with(scope: :some_resource, authenticity_token: 'external_token') {} %>"
+  end
+
   def same_origin_js
     render js: "foo();"
   end
@@ -235,6 +251,80 @@ module RequestForgeryProtectionTests
     end
   end
 
+  def test_should_render_form_with_with_token_tag_if_remote
+    assert_not_blocked do
+      get :form_with_remote
+    end
+    assert_match(/authenticity_token/, response.body)
+  end
+
+  def test_should_render_form_with_without_token_tag_if_remote_and_embedding_token_is_off
+    original = ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms
+    begin
+      ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = false
+      assert_not_blocked do
+        get :form_with_remote
+      end
+      assert_no_match(/authenticity_token/, response.body)
+    ensure
+      ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = original
+    end
+  end
+
+  def test_should_render_form_with_with_token_tag_if_remote_and_external_authenticity_token_requested_and_embedding_is_on
+    original = ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms
+    begin
+      ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = true
+      assert_not_blocked do
+        get :form_with_remote_with_external_token
+      end
+      assert_select "form>input[name=?][value=?]", "custom_authenticity_token", "external_token"
+    ensure
+      ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = original
+    end
+  end
+
+  def test_should_render_form_with_with_token_tag_if_remote_and_external_authenticity_token_requested
+    assert_not_blocked do
+      get :form_with_remote_with_external_token
+    end
+    assert_select "form>input[name=?][value=?]", "custom_authenticity_token", "external_token"
+  end
+
+  def test_should_render_form_with_with_token_tag_if_remote_and_authenticity_token_requested
+    @controller.stub :form_authenticity_token, @token do
+      assert_not_blocked do
+        get :form_with_remote_with_token
+      end
+      assert_select "form>input[name=?][value=?]", "custom_authenticity_token", @token
+    end
+  end
+
+  def test_should_render_form_with_with_token_tag_with_authenticity_token_requested
+    @controller.stub :form_authenticity_token, @token do
+      assert_not_blocked do
+        get :form_with_local_with_token
+      end
+      assert_select "form>input[name=?][value=?]", "custom_authenticity_token", @token
+    end
+  end
+
+  def test_should_render_form_with_with_token_tag_if_remote_and_embedding_token_is_on
+    original = ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms
+    begin
+      ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = true
+
+      @controller.stub :form_authenticity_token, @token do
+        assert_not_blocked do
+          get :form_with_remote
+        end
+      end
+      assert_select "form>input[name=?][value=?]", "custom_authenticity_token", @token
+    ensure
+      ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = original
+    end
+  end
+
   def test_should_allow_get
     assert_not_blocked { get :index }
   end
@@ -347,6 +437,10 @@ module RequestForgeryProtectionTests
   end
 
   def test_should_block_post_with_origin_checking_and_wrong_origin
+    old_logger = ActionController::Base.logger
+    logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new
+    ActionController::Base.logger = logger
+
     forgery_protection_origin_check do
       session[:_csrf_token] = @token
       @controller.stub :form_authenticity_token, @token do
@@ -356,6 +450,13 @@ module RequestForgeryProtectionTests
         end
       end
     end
+
+    assert_match(
+      "HTTP Origin header (http://bad.host) didn't match request.base_url (http://test.host)",
+      logger.logged(:warn).last
+    )
+  ensure
+    ActionController::Base.logger = old_logger
   end
 
   def test_should_warn_on_missing_csrf_token
diff --git a/actionpack/test/controller/required_params_test.rb b/actionpack/test/controller/required_params_test.rb
index dd07c2486b..46bb374b3f 100644
--- a/actionpack/test/controller/required_params_test.rb
+++ b/actionpack/test/controller/required_params_test.rb
@@ -72,9 +72,27 @@ class ParametersRequireTest < ActiveSupport::TestCase
     assert params.value?("cinco")
   end
 
-  test "to_query is not supported" do
-    assert_raises(NoMethodError) do
-      ActionController::Parameters.new(foo: "bar").to_param
+  test "to_param works like in a Hash" do
+    params = ActionController::Parameters.new(nested: { key: "value" }).permit!
+    assert_equal({ nested: { key: "value" } }.to_param, params.to_param)
+
+    params = { root: ActionController::Parameters.new(nested: { key: "value" }).permit! }
+    assert_equal({ root: { nested: { key: "value" } } }.to_param, params.to_param)
+
+    assert_raise(ActionController::UnfilteredParameters) do
+      ActionController::Parameters.new(nested: { key: "value" }).to_param
+    end
+  end
+
+  test "to_query works like in a Hash" do
+    params = ActionController::Parameters.new(nested: { key: "value" }).permit!
+    assert_equal({ nested: { key: "value" } }.to_query, params.to_query)
+
+    params = { root: ActionController::Parameters.new(nested: { key: "value" }).permit! }
+    assert_equal({ root: { nested: { key: "value" } } }.to_query, params.to_query)
+
+    assert_raise(ActionController::UnfilteredParameters) do
+      ActionController::Parameters.new(nested: { key: "value" }).to_query
     end
   end
 end
diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb
index 862dcf01c3..2afe67ed91 100644
--- a/actionpack/test/controller/url_for_test.rb
+++ b/actionpack/test/controller/url_for_test.rb
@@ -386,7 +386,7 @@ module AbstractController
 
       def test_url_action_controller_parameters
         add_host!
-        assert_raise(ArgumentError) do
+        assert_raise(ActionController::UnfilteredParameters) do
           W.new.url_for(ActionController::Parameters.new(controller: "c", action: "a", protocol: "javascript", f: "%0Aeval(name)"))
         end
       end
diff --git a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
index cb5ca5888b..338992dda5 100644
--- a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
+++ b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
@@ -165,8 +165,8 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest
 
     assert_equal "/", params_path(@safe_params)
     assert_equal "/", Routes.url_helpers.params_path(@safe_params)
-    assert_raises(ArgumentError) { params_path(@unsafe_params) }
-    assert_raises(ArgumentError) { Routes.url_helpers.params_path(@unsafe_params) }
+    assert_raises(ActionController::UnfilteredParameters) { params_path(@unsafe_params) }
+    assert_raises(ActionController::UnfilteredParameters) { Routes.url_helpers.params_path(@unsafe_params) }
 
     assert_equal "/basket", symbol_path
     assert_equal "/basket", Routes.url_helpers.symbol_path
@@ -208,8 +208,8 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest
 
     assert_equal "http://www.example.com/", params_url(@safe_params)
     assert_equal "http://www.example.com/", Routes.url_helpers.params_url(@safe_params)
-    assert_raises(ArgumentError) { params_url(@unsafe_params) }
-    assert_raises(ArgumentError) { Routes.url_helpers.params_url(@unsafe_params) }
+    assert_raises(ActionController::UnfilteredParameters) { params_url(@unsafe_params) }
+    assert_raises(ActionController::UnfilteredParameters) { Routes.url_helpers.params_url(@unsafe_params) }
 
     assert_equal "http://www.example.com/basket", symbol_url
     assert_equal "http://www.example.com/basket", Routes.url_helpers.symbol_url
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 64818e6ca1..d64917e0d3 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -3633,7 +3633,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
     params = ActionController::Parameters.new(id: "1")
 
-    assert_raises ArgumentError do
+    assert_raises ActionController::UnfilteredParameters do
       root_path(params)
     end
   end
@@ -3706,6 +3706,24 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     assert_equal "/bar", bar_root_path
   end
 
+  def test_nested_routes_under_format_resource
+    draw do
+      resources :formats do
+        resources :items
+      end
+    end
+
+    get "/formats/1/items.json"
+    assert_equal 200, @response.status
+    assert_equal "items#index", @response.body
+    assert_equal "/formats/1/items.json", format_items_path(1, :json)
+
+    get "/formats/1/items/2.json"
+    assert_equal 200, @response.status
+    assert_equal "items#show", @response.body
+    assert_equal "/formats/1/items/2.json", format_item_path(1, 2, :json)
+  end
+
 private
 
   def draw(&block)
diff --git a/actionpack/test/fixtures/layouts/builder.builder b/actionpack/test/fixtures/layouts/builder.builder
index 7c7d4b2dd1..c55488edd0 100644
--- a/actionpack/test/fixtures/layouts/builder.builder
+++ b/actionpack/test/fixtures/layouts/builder.builder
@@ -1,3 +1,3 @@
 xml.wrapper do
   xml << yield
-end
\ No newline at end of file
+end
diff --git a/actionpack/test/fixtures/old_content_type/render_default_for_builder.builder b/actionpack/test/fixtures/old_content_type/render_default_for_builder.builder
index 598d62e2fc..15c8a7f5cf 100644
--- a/actionpack/test/fixtures/old_content_type/render_default_for_builder.builder
+++ b/actionpack/test/fixtures/old_content_type/render_default_for_builder.builder
@@ -1 +1 @@
-xml.p "Hello world!"
\ No newline at end of file
+xml.p "Hello world!"
diff --git a/actionpack/test/fixtures/respond_to/using_defaults.xml.builder b/actionpack/test/fixtures/respond_to/using_defaults.xml.builder
index 598d62e2fc..15c8a7f5cf 100644
--- a/actionpack/test/fixtures/respond_to/using_defaults.xml.builder
+++ b/actionpack/test/fixtures/respond_to/using_defaults.xml.builder
@@ -1 +1 @@
-xml.p "Hello world!"
\ No newline at end of file
+xml.p "Hello world!"
diff --git a/actionpack/test/fixtures/respond_to/using_defaults_with_type_list.xml.builder b/actionpack/test/fixtures/respond_to/using_defaults_with_type_list.xml.builder
index 598d62e2fc..15c8a7f5cf 100644
--- a/actionpack/test/fixtures/respond_to/using_defaults_with_type_list.xml.builder
+++ b/actionpack/test/fixtures/respond_to/using_defaults_with_type_list.xml.builder
@@ -1 +1 @@
-xml.p "Hello world!"
\ No newline at end of file
+xml.p "Hello world!"
diff --git a/actionpack/test/fixtures/test/formatted_xml_erb.builder b/actionpack/test/fixtures/test/formatted_xml_erb.builder
index 14fd3549fb..f98aaa34a5 100644
--- a/actionpack/test/fixtures/test/formatted_xml_erb.builder
+++ b/actionpack/test/fixtures/test/formatted_xml_erb.builder
@@ -1 +1 @@
-xml.test 'failed'
\ No newline at end of file
+xml.test "failed"
diff --git a/actionpack/test/fixtures/test/hello_xml_world.builder b/actionpack/test/fixtures/test/hello_xml_world.builder
index e7081b89fe..d16bb6b5cb 100644
--- a/actionpack/test/fixtures/test/hello_xml_world.builder
+++ b/actionpack/test/fixtures/test/hello_xml_world.builder
@@ -8,4 +8,4 @@ xml.html do
     xml.p "monks"
     xml.p "wiseguys"
   end
-end
\ No newline at end of file
+end