Merge branch 'master' into bug/filtered_parameters_class

22 files changed, 316 insertions, 93 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index c5b679207a..51cec82801 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1 +1,13 @@
+*  Add `action_controller_api` and `action_controller_base` load hooks to be called in `ActiveSupport.on_load`
+
+    `ActionController::Base` and `ActionController::API` have differing implementations. This means that
+    the one umbrella hook `action_controller` is not able to address certain situations where a method
+    may not exist in a certain implementation.
+
+    This is fixed by adding two new hooks so you can target `ActionController::Base` vs `ActionController::API`
+
+    Fixes #27013.
+
+    *Julian Nadeau*
+
 Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/actionpack/CHANGELOG.md) for previous changes.
diff --git a/actionpack/lib/action_controller/api.rb b/actionpack/lib/action_controller/api.rb
index 0d1af0d0bd..94698df730 100644
--- a/actionpack/lib/action_controller/api.rb
+++ b/actionpack/lib/action_controller/api.rb
@@ -141,6 +141,7 @@ module ActionController
       include mod
     end
 
+    ActiveSupport.run_load_hooks(:action_controller_api, self)
     ActiveSupport.run_load_hooks(:action_controller, self)
   end
 end
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index b420e00c78..8c2b111f89 100644
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -267,6 +267,7 @@ module ActionController
       end
     end
 
+    ActiveSupport.run_load_hooks(:action_controller_base, self)
     ActiveSupport.run_load_hooks(:action_controller, self)
   end
 end
diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
index d9a8b9c12d..5051c02a62 100644
--- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb
+++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
@@ -213,7 +213,11 @@ module ActionController #:nodoc:
 
         if !verified_request?
           if logger && log_warning_on_csrf_failure
-            logger.warn "Can't verify CSRF token authenticity."
+            if valid_request_origin?
+              logger.warn "Can't verify CSRF token authenticity."
+            else
+              logger.warn "HTTP Origin header (#{request.origin}) didn't match request.base_url (#{request.base_url})"
+            end
           end
           handle_unverified_request
         end
diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb
index baf15570d5..7864f9decd 100644
--- a/actionpack/lib/action_controller/metal/strong_parameters.rb
+++ b/actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -43,6 +43,18 @@ module ActionController
     end
   end
 
+  # Raised when a Parameters instance is not marked as permitted and
+  # an operation to transform it to hash is called.
+  #
+  #   params = ActionController::Parameters.new(a: "123", b: "456")
+  #   params.to_h
+  #   # => ActionController::UnfilteredParameters: unable to convert unpermitted parameters to hash
+  class UnfilteredParameters < ArgumentError
+    def initialize # :nodoc:
+      super("unable to convert unpermitted parameters to hash")
+    end
+  end
+
   # == Action Controller \Parameters
   #
   # Allows you to choose which attributes should be whitelisted for mass updating
@@ -53,9 +65,9 @@ module ActionController
   #
   #   params = ActionController::Parameters.new({
   #     person: {
-  #       name: 'Francesco',
+  #       name: "Francesco",
   #       age:  22,
-  #       role: 'admin'
+  #       role: "admin"
   #     }
   #   })
   #
@@ -103,7 +115,7 @@ module ActionController
   # You can fetch values of <tt>ActionController::Parameters</tt> using either
   # <tt>:key</tt> or <tt>"key"</tt>.
   #
-  #   params = ActionController::Parameters.new(key: 'value')
+  #   params = ActionController::Parameters.new(key: "value")
   #   params[:key]  # => "value"
   #   params["key"] # => "value"
   class Parameters
@@ -203,13 +215,13 @@ module ActionController
     #   class Person < ActiveRecord::Base
     #   end
     #
-    #   params = ActionController::Parameters.new(name: 'Francesco')
+    #   params = ActionController::Parameters.new(name: "Francesco")
     #   params.permitted?  # => false
     #   Person.new(params) # => ActiveModel::ForbiddenAttributesError
     #
     #   ActionController::Parameters.permit_all_parameters = true
     #
-    #   params = ActionController::Parameters.new(name: 'Francesco')
+    #   params = ActionController::Parameters.new(name: "Francesco")
     #   params.permitted?  # => true
     #   Person.new(params) # => #<Person id: nil, name: "Francesco">
     def initialize(parameters = {})
@@ -228,13 +240,14 @@ module ActionController
     end
 
     # Returns a safe <tt>ActiveSupport::HashWithIndifferentAccess</tt>
-    # representation of this parameter with all unpermitted keys removed.
+    # representation of the parameters with all unpermitted keys removed.
     #
     #   params = ActionController::Parameters.new({
-    #     name: 'Senjougahara Hitagi',
-    #     oddity: 'Heavy stone crab'
+    #     name: "Senjougahara Hitagi",
+    #     oddity: "Heavy stone crab"
     #   })
-    #   params.to_h # => {}
+    #   params.to_h
+    #   # => ActionController::UnfilteredParameters: unable to convert unfiltered parameters to hash
     #
     #   safe_params = params.permit(:name)
     #   safe_params.to_h # => {"name"=>"Senjougahara Hitagi"}
@@ -242,17 +255,61 @@ module ActionController
       if permitted?
         convert_parameters_to_hashes(@parameters, :to_h)
       else
-        slice(*self.class.always_permitted_parameters).permit!.to_h
+        raise UnfilteredParameters
       end
     end
 
+    # Returns a safe <tt>Hash</tt> representation of the parameters
+    # with all unpermitted keys removed.
+    #
+    #   params = ActionController::Parameters.new({
+    #     name: "Senjougahara Hitagi",
+    #     oddity: "Heavy stone crab"
+    #   })
+    #   params.to_hash
+    #   # => ActionController::UnfilteredParameters: unable to convert unfiltered parameters to hash
+    #
+    #   safe_params = params.permit(:name)
+    #   safe_params.to_hash # => {"name"=>"Senjougahara Hitagi"}
+    def to_hash
+      to_h.to_hash
+    end
+
+    # Returns a string representation of the receiver suitable for use as a URL
+    # query string:
+    #
+    #   params = ActionController::Parameters.new({
+    #     name: "David",
+    #     nationality: "Danish"
+    #   })
+    #   params.to_query
+    #   # => "name=David&nationality=Danish"
+    #
+    # An optional namespace can be passed to enclose key names:
+    #
+    #   params = ActionController::Parameters.new({
+    #     name: "David",
+    #     nationality: "Danish"
+    #   })
+    #   params.to_query("user")
+    #   # => "user%5Bname%5D=David&user%5Bnationality%5D=Danish"
+    #
+    # The string pairs "key=value" that conform the query string
+    # are sorted lexicographically in ascending order.
+    #
+    # This method is also aliased as +to_param+.
+    def to_query(*args)
+      to_h.to_query(*args)
+    end
+    alias_method :to_param, :to_query
+
     # Returns an unsafe, unfiltered
-    # <tt>ActiveSupport::HashWithIndifferentAccess</tt> representation of this
-    # parameter.
+    # <tt>ActiveSupport::HashWithIndifferentAccess</tt> representation of the
+    # parameters.
     #
     #   params = ActionController::Parameters.new({
-    #     name: 'Senjougahara Hitagi',
-    #     oddity: 'Heavy stone crab'
+    #     name: "Senjougahara Hitagi",
+    #     oddity: "Heavy stone crab"
     #   })
     #   params.to_unsafe_h
     #   # => {"name"=>"Senjougahara Hitagi", "oddity" => "Heavy stone crab"}
@@ -297,7 +354,7 @@ module ActionController
     #   class Person < ActiveRecord::Base
     #   end
     #
-    #   params = ActionController::Parameters.new(name: 'Francesco')
+    #   params = ActionController::Parameters.new(name: "Francesco")
     #   params.permitted?  # => false
     #   Person.new(params) # => ActiveModel::ForbiddenAttributesError
     #   params.permit!
@@ -319,7 +376,7 @@ module ActionController
     # When passed a single key, if it exists and its associated value is
     # either present or the singleton +false+, returns said value:
     #
-    #   ActionController::Parameters.new(person: { name: 'Francesco' }).require(:person)
+    #   ActionController::Parameters.new(person: { name: "Francesco" }).require(:person)
     #   # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false>
     #
     # Otherwise raises <tt>ActionController::ParameterMissing</tt>:
@@ -352,7 +409,7 @@ module ActionController
     # Technically this method can be used to fetch terminal values:
     #
     #   # CAREFUL
-    #   params = ActionController::Parameters.new(person: { name: 'Finn' })
+    #   params = ActionController::Parameters.new(person: { name: "Finn" })
     #   name = params.require(:person).require(:name) # CAREFUL
     #
     # but take into account that at some point those ones have to be permitted:
@@ -382,7 +439,7 @@ module ActionController
     # for the object to +true+. This is useful for limiting which attributes
     # should be allowed for mass updating.
     #
-    #   params = ActionController::Parameters.new(user: { name: 'Francesco', age: 22, role: 'admin' })
+    #   params = ActionController::Parameters.new(user: { name: "Francesco", age: 22, role: "admin" })
     #   permitted = params.require(:user).permit(:name, :age)
     #   permitted.permitted?      # => true
     #   permitted.has_key?(:name) # => true
@@ -402,7 +459,7 @@ module ActionController
     # You may declare that the parameter should be an array of permitted scalars
     # by mapping it to an empty array:
     #
-    #   params = ActionController::Parameters.new(tags: ['rails', 'parameters'])
+    #   params = ActionController::Parameters.new(tags: ["rails", "parameters"])
     #   params.permit(tags: [])
     #
     # Sometimes it is not possible or convenient to declare the valid keys of
@@ -418,11 +475,11 @@ module ActionController
     #
     #   params = ActionController::Parameters.new({
     #     person: {
-    #       name: 'Francesco',
+    #       name: "Francesco",
     #       age:  22,
     #       pets: [{
-    #         name: 'Purplish',
-    #         category: 'dogs'
+    #         name: "Purplish",
+    #         category: "dogs"
     #       }]
     #     }
     #   })
@@ -441,8 +498,8 @@ module ActionController
     #   params = ActionController::Parameters.new({
     #     person: {
     #       contact: {
-    #         email: 'none@test.com',
-    #         phone: '555-1234'
+    #         email: "none@test.com",
+    #         phone: "555-1234"
     #       }
     #     }
     #   })
@@ -475,7 +532,7 @@ module ActionController
     # Returns a parameter for the given +key+. If not found,
     # returns +nil+.
     #
-    #   params = ActionController::Parameters.new(person: { name: 'Francesco' })
+    #   params = ActionController::Parameters.new(person: { name: "Francesco" })
     #   params[:person] # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false>
     #   params[:none]   # => nil
     def [](key)
@@ -494,11 +551,11 @@ module ActionController
     # if more arguments are given, then that will be returned; if a block
     # is given, then that will be run and its result returned.
     #
-    #   params = ActionController::Parameters.new(person: { name: 'Francesco' })
+    #   params = ActionController::Parameters.new(person: { name: "Francesco" })
     #   params.fetch(:person)               # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false>
     #   params.fetch(:none)                 # => ActionController::ParameterMissing: param is missing or the value is empty: none
-    #   params.fetch(:none, 'Francesco')    # => "Francesco"
-    #   params.fetch(:none) { 'Francesco' } # => "Francesco"
+    #   params.fetch(:none, "Francesco")    # => "Francesco"
+    #   params.fetch(:none) { "Francesco" } # => "Francesco"
     def fetch(key, *args)
       convert_value_to_parameters(
         @parameters.fetch(key) {
@@ -715,8 +772,6 @@ module ActionController
       end
     end
 
-    undef_method :to_param
-
     # Returns duplicate of object including all parameters.
     def deep_dup
       self.class.new(@parameters.deep_dup).tap do |duplicate|
diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb
index 60d4789a63..87dd1eba38 100644
--- a/actionpack/lib/action_dispatch/routing.rb
+++ b/actionpack/lib/action_dispatch/routing.rb
@@ -254,14 +254,5 @@ module ActionDispatch
 
     SEPARATORS = %w( / . ? ) #:nodoc:
     HTTP_METHODS = [:get, :head, :post, :patch, :put, :delete, :options] #:nodoc:
-
-    #:stopdoc:
-    INSECURE_URL_PARAMETERS_MESSAGE = <<-MSG.squish
-      Attempting to generate a URL from non-sanitized request parameters!
-
-      An attacker can inject malicious data into the generated URL, such as
-      changing the host. Whitelist and sanitize passed parameters to be secure.
-    MSG
-    #:startdoc:
   end
 end
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 8ad17504ae..74904e3d45 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -54,6 +54,7 @@ module ActionDispatch
 
       class Mapping #:nodoc:
         ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z}
+        OPTIONAL_FORMAT_REGEX = %r{(?:\(\.:format\)+|\.:format|/)\Z}
 
         attr_reader :requirements, :defaults
         attr_reader :to, :default_controller, :default_action
@@ -93,7 +94,7 @@ module ActionDispatch
         end
 
         def self.optional_format?(path, format)
-          format != false && !path.include?(":format") && !path.end_with?("/")
+          format != false && path !~ OPTIONAL_FORMAT_REGEX
         end
 
         def initialize(set, ast, defaults, controller, default_action, modyoule, to, formatted, scope_constraints, blocks, via, options_constraints, anchor, options)
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 129e90037e..e1f9fc9ecc 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -318,11 +318,7 @@ module ActionDispatch
                   when Hash
                     args.pop
                   when ActionController::Parameters
-                    if last.permitted?
-                      args.pop.to_h
-                    else
-                      raise ArgumentError, ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE
-                    end
+                    args.pop.to_h
                   end
                 helper.call self, args, options
               end
diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb
index 008216cc80..a9bdefa775 100644
--- a/actionpack/lib/action_dispatch/routing/url_for.rb
+++ b/actionpack/lib/action_dispatch/routing/url_for.rb
@@ -171,17 +171,10 @@ module ActionDispatch
         case options
         when nil
           _routes.url_for(url_options.symbolize_keys)
-        when Hash
+        when Hash, ActionController::Parameters
           route_name = options.delete :use_route
-          _routes.url_for(options.symbolize_keys.reverse_merge!(url_options),
-                         route_name)
-        when ActionController::Parameters
-          unless options.permitted?
-            raise ArgumentError.new(ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE)
-          end
-          route_name = options.delete :use_route
-          _routes.url_for(options.to_h.symbolize_keys.
-                          reverse_merge!(url_options), route_name)
+          merged_url_options = options.to_h.symbolize_keys.reverse_merge!(url_options)
+          _routes.url_for(merged_url_options, route_name)
         when String
           options
         when Symbol
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