Restore and adapt the implementation reverted at

https://github.com/rails/rails/commit/cc1c3c5be061e7572018f734e5239750ab449e3f

Now instead of raise, we log by default in development and test

5 files changed, 114 insertions, 37 deletions

diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb
index 317cf101b7..2c96e03f55 100644
--- a/actionpack/lib/action_controller/metal/strong_parameters.rb
+++ b/actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -23,11 +23,11 @@ module ActionController
   #
   #   params = ActionController::Parameters.new(a: "123", b: "456")
   #   params.permit(:c)
-  #   # => ActionController::UnexpectedParameter: found unexpected keys: a, b
-  class UnexpectedParameters < IndexError
-    attr_reader :params
+  #   # => ActionController::UnpermittedParameters: found unexpected keys: a, b
+  class UnpermittedParameters < IndexError
+    attr_reader :params # :nodoc:
 
-    def initialize(params)
+    def initialize(params) # :nodoc:
       @params = params
       super("found unpermitted parameters: #{params.join(", ")}")
     end
@@ -57,10 +57,15 @@ module ActionController
   #   Person.first.update!(permitted)
   #   # => #<Person id: 1, name: "Francesco", age: 22, role: "user">
   #
-  # It provides a +permit_all_parameters+ option that controls the top-level
-  # behavior of new instances. If it's +true+, all the parameters will be
-  # permitted by default. The default value for +permit_all_parameters+
-  # option is +false+.
+  # It provides two options that controls the top-level behavior of new instances:
+  #
+  # * +permit_all_parameters+ - If it's +true+, all the parameters will be
+  #   permitted by default. The default is +false+.
+  # * +action_on_unpermitted_parameters+ - Allow to control the behavior when parameters
+  #   that are not explicitly permitted are found. The values can be <tt>:log</tt> to
+  #   write a message on the logger or <tt>:raise</tt> to raise
+  #   ActionController::UnpermittedParameters exception. The default value is <tt>:log</tt>
+  #   in test and development environments, +false+ otherwise.
   #
   #   params = ActionController::Parameters.new
   #   params.permitted? # => false
@@ -70,6 +75,16 @@ module ActionController
   #   params = ActionController::Parameters.new
   #   params.permitted? # => true
   #
+  #   params = ActionController::Parameters.new(a: "123", b: "456")
+  #   params.permit(:c)
+  #   # => {}
+  #
+  #   ActionController::Parameters.action_on_unpermitted_parameters = :raise
+  #
+  #   params = ActionController::Parameters.new(a: "123", b: "456")
+  #   params.permit(:c)
+  #   # => ActionController::UnpermittedParameters: found unpermitted keys: a, b
+  #
   # <tt>ActionController::Parameters</tt> is inherited from
   # <tt>ActiveSupport::HashWithIndifferentAccess</tt>, this means
   # that you can fetch values using either <tt>:key</tt> or <tt>"key"</tt>.
@@ -79,7 +94,11 @@ module ActionController
   #   params["key"] # => "value"
   class Parameters < ActiveSupport::HashWithIndifferentAccess
     cattr_accessor :permit_all_parameters, instance_accessor: false
-    cattr_accessor :action_on_unpermitted, instance_accessor: false
+    cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false
+
+    # Never raise an UnpermittedParameters exception because of these params
+    # are present. They are added by Rails and it's of no concern.
+    NEVER_UNPERMITTED_PARAMS = %w( controller action )
 
     # Returns a new instance of <tt>ActionController::Parameters</tt>.
     # Also, sets the +permitted+ attribute to the default value of
@@ -237,16 +256,8 @@ module ActionController
         end
       end
 
-      unpermitted_keys = self.keys - params.keys
-      if unpermitted_keys.any?
-        case self.class.action_on_unpermitted
-        when :log
-          ActionController::Base.logger.debug "Unpermitted parameters: #{unpermitted_keys.join(", ")}"
-        when :raise
-          raise ActionController::UnexpectedParameters.new(unpermitted_keys)
-        end
-      end
-      
+      unpermitted_parameters!(params)
+
       params.permit!
     end
 
@@ -325,6 +336,22 @@ module ActionController
           yield object
         end
       end
+
+      def unpermitted_parameters!(params)
+        unpermitted_keys = unpermitted_keys(params)
+        if unpermitted_keys.any?
+          case self.class.action_on_unpermitted_parameters
+          when :log
+            ActionController::Base.logger.debug "Unpermitted parameters: #{unpermitted_keys.join(", ")}"
+          when :raise
+            raise ActionController::UnpermittedParameters.new(unpermitted_keys)
+          end
+        end
+      end
+
+      def unpermitted_keys(params)
+        self.keys - params.keys - NEVER_UNPERMITTED_PARAMS
+      end
   end
 
   # == Strong \Parameters
diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb
index bd012965c3..5379547c57 100644
--- a/actionpack/lib/action_controller/railtie.rb
+++ b/actionpack/lib/action_controller/railtie.rb
@@ -20,25 +20,27 @@ module ActionController
     end
 
     initializer "action_controller.parameters_config" do |app|
-      ActionController::Parameters.permit_all_parameters = app.config.action_controller.delete(:permit_all_parameters) { false }
-      ActionController::Parameters.action_on_unpermitted = app.config.action_controller.action_on_unpermitted_params
+      options = app.config.action_controller
+
+      ActionController::Parameters.permit_all_parameters = options.delete(:permit_all_parameters) { false }
+      ActionController::Parameters.action_on_unpermitted_parameters = options.delete(:action_on_unpermitted_parameters) do
+        (Rails.env.test? || Rails.env.development?) ? :log : false
+      end
     end
 
     initializer "action_controller.set_configs" do |app|
       paths   = app.config.paths
       options = app.config.action_controller
 
-      options.logger                        ||= Rails.logger
-      options.cache_store                   ||= Rails.cache
+      options.logger      ||= Rails.logger
+      options.cache_store ||= Rails.cache
 
-      options.javascripts_dir               ||= paths["public/javascripts"].first
-      options.stylesheets_dir               ||= paths["public/stylesheets"].first
+      options.javascripts_dir ||= paths["public/javascripts"].first
+      options.stylesheets_dir ||= paths["public/stylesheets"].first
 
       # Ensure readers methods get compiled
-      options.asset_host                    ||= app.config.asset_host
-      options.relative_url_root             ||= app.config.relative_url_root
-
-      options.action_on_unpermitted_params  ||= (Rails.env.test? || Rails.env.development?) ? :log : false
+      options.asset_host        ||= app.config.asset_host
+      options.relative_url_root ||= app.config.relative_url_root
 
       ActiveSupport.on_load(:action_controller) do
         include app.routes.mounted_helpers
diff --git a/actionpack/test/controller/parameters/log_on_unpermitted_params_test.rb b/actionpack/test/controller/parameters/log_on_unpermitted_params_test.rb
index fb50dabff9..22e603b881 100644
--- a/actionpack/test/controller/parameters/log_on_unpermitted_params_test.rb
+++ b/actionpack/test/controller/parameters/log_on_unpermitted_params_test.rb
@@ -3,11 +3,11 @@ require 'action_controller/metal/strong_parameters'
 
 class LogOnUnpermittedParamsTest < ActiveSupport::TestCase
   def setup
-    ActionController::Parameters.action_on_unpermitted = :log
+    ActionController::Parameters.action_on_unpermitted_parameters = :log
   end
 
   def teardown
-    ActionController::Parameters.action_on_unpermitted = false
+    ActionController::Parameters.action_on_unpermitted_parameters = false
   end
 
   test "logs on unexpected params" do
@@ -47,4 +47,4 @@ class LogOnUnpermittedParamsTest < ActiveSupport::TestCase
       ActionController::Base.logger = old_logger
     end
   end
-end
\ No newline at end of file
+end
diff --git a/actionpack/test/controller/parameters/raise_on_unpermitted_params_test.rb b/actionpack/test/controller/parameters/raise_on_unpermitted_params_test.rb
index e2933b86eb..f9cc9f96f1 100644
--- a/actionpack/test/controller/parameters/raise_on_unpermitted_params_test.rb
+++ b/actionpack/test/controller/parameters/raise_on_unpermitted_params_test.rb
@@ -3,11 +3,11 @@ require 'action_controller/metal/strong_parameters'
 
 class RaiseOnUnpermittedParamsTest < ActiveSupport::TestCase
   def setup
-    ActionController::Parameters.action_on_unpermitted = :raise
+    ActionController::Parameters.action_on_unpermitted_parameters = :raise
   end
 
   def teardown
-    ActionController::Parameters.action_on_unpermitted = false
+    ActionController::Parameters.action_on_unpermitted_parameters = false
   end
 
   test "raises on unexpected params" do
@@ -16,7 +16,7 @@ class RaiseOnUnpermittedParamsTest < ActiveSupport::TestCase
       fishing: "Turnips"
     })
 
-    assert_raises(ActionController::UnexpectedParameters) do
+    assert_raises(ActionController::UnpermittedParameters) do
       params.permit(book: [:pages])
     end
   end
@@ -26,8 +26,8 @@ class RaiseOnUnpermittedParamsTest < ActiveSupport::TestCase
       book: { pages: 65, title: "Green Cats and where to find then." }
     })
 
-    assert_raises(ActionController::UnexpectedParameters) do
+    assert_raises(ActionController::UnpermittedParameters) do
       params.permit(book: [:pages])
     end
   end
-end
\ No newline at end of file
+end
diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb
index f9108ed7b9..78aefa1437 100644
--- a/railties/test/application/configuration_test.rb
+++ b/railties/test/application/configuration_test.rb
@@ -577,6 +577,54 @@ module ApplicationTests
       assert_equal 'permitted', last_response.body
     end
 
+    test "config.action_controller.action_on_unpermitted_parameters = :raise" do
+      app_file 'app/controllers/posts_controller.rb', <<-RUBY
+      class PostsController < ActionController::Base
+        def create
+          render text: params.require(:post).permit(:name)
+        end
+      end
+      RUBY
+
+      add_to_config <<-RUBY
+        routes.prepend do
+          resources :posts
+        end
+        config.action_controller.action_on_unpermitted_parameters = :raise
+      RUBY
+
+      require "#{app_path}/config/environment"
+
+      assert_equal :raise, ActionController::Parameters.action_on_unpermitted_parameters
+
+      post "/posts", {post: {"title" =>"zomg"}}
+      assert_match "We're sorry, but something went wrong", last_response.body
+    end
+
+    test "config.action_controller.action_on_unpermitted_parameters is :log by default on development" do
+      ENV["RAILS_ENV"] = "development"
+
+      require "#{app_path}/config/environment"
+
+      assert_equal :log, ActionController::Parameters.action_on_unpermitted_parameters
+    end
+
+    test "config.action_controller.action_on_unpermitted_parameters is :log by defaul on test" do
+      ENV["RAILS_ENV"] = "test"
+
+      require "#{app_path}/config/environment"
+
+      assert_equal :log, ActionController::Parameters.action_on_unpermitted_parameters
+    end
+
+    test "config.action_controller.action_on_unpermitted_parameters is false by default on production" do
+      ENV["RAILS_ENV"] = "production"
+
+      require "#{app_path}/config/environment"
+
+      assert_equal false, ActionController::Parameters.action_on_unpermitted_parameters
+    end
+
     test "config.action_dispatch.ignore_accept_header" do
       make_basic_app do |app|
         app.config.action_dispatch.ignore_accept_header = true