Skip to content
This repository
Browse code

Restore and adapt the implementation reverted at

cc1c3c5

Now instead of raise, we log by default in development and test
  • Loading branch information...
commit 57126ee5e392a4dc2eed22963d25020a48a93413 1 parent af5edef
Rafael Mendonça França authored
65  actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -23,11 +23,11 @@ def initialize(param) # :nodoc:
23 23
   #
24 24
   #   params = ActionController::Parameters.new(a: "123", b: "456")
25 25
   #   params.permit(:c)
26  
-  #   # => ActionController::UnexpectedParameter: found unexpected keys: a, b
27  
-  class UnexpectedParameters < IndexError
28  
-    attr_reader :params
  26
+  #   # => ActionController::UnpermittedParameters: found unexpected keys: a, b
  27
+  class UnpermittedParameters < IndexError
  28
+    attr_reader :params # :nodoc:
29 29
 
30  
-    def initialize(params)
  30
+    def initialize(params) # :nodoc:
31 31
       @params = params
32 32
       super("found unpermitted parameters: #{params.join(", ")}")
33 33
     end
@@ -57,10 +57,15 @@ def initialize(params)
57 57
   #   Person.first.update!(permitted)
58 58
   #   # => #<Person id: 1, name: "Francesco", age: 22, role: "user">
59 59
   #
60  
-  # It provides a +permit_all_parameters+ option that controls the top-level
61  
-  # behavior of new instances. If it's +true+, all the parameters will be
62  
-  # permitted by default. The default value for +permit_all_parameters+
63  
-  # option is +false+.
  60
+  # It provides two options that controls the top-level behavior of new instances:
  61
+  #
  62
+  # * +permit_all_parameters+ - If it's +true+, all the parameters will be
  63
+  #   permitted by default. The default is +false+.
  64
+  # * +action_on_unpermitted_parameters+ - Allow to control the behavior when parameters
  65
+  #   that are not explicitly permitted are found. The values can be <tt>:log</tt> to
  66
+  #   write a message on the logger or <tt>:raise</tt> to raise
  67
+  #   ActionController::UnpermittedParameters exception. The default value is <tt>:log</tt>
  68
+  #   in test and development environments, +false+ otherwise.
64 69
   #
65 70
   #   params = ActionController::Parameters.new
66 71
   #   params.permitted? # => false
@@ -70,6 +75,16 @@ def initialize(params)
70 75
   #   params = ActionController::Parameters.new
71 76
   #   params.permitted? # => true
72 77
   #
  78
+  #   params = ActionController::Parameters.new(a: "123", b: "456")
  79
+  #   params.permit(:c)
  80
+  #   # => {}
  81
+  #
  82
+  #   ActionController::Parameters.action_on_unpermitted_parameters = :raise
  83
+  #
  84
+  #   params = ActionController::Parameters.new(a: "123", b: "456")
  85
+  #   params.permit(:c)
  86
+  #   # => ActionController::UnpermittedParameters: found unpermitted keys: a, b
  87
+  #
73 88
   # <tt>ActionController::Parameters</tt> is inherited from
74 89
   # <tt>ActiveSupport::HashWithIndifferentAccess</tt>, this means
75 90
   # that you can fetch values using either <tt>:key</tt> or <tt>"key"</tt>.
@@ -79,7 +94,11 @@ def initialize(params)
79 94
   #   params["key"] # => "value"
80 95
   class Parameters < ActiveSupport::HashWithIndifferentAccess
81 96
     cattr_accessor :permit_all_parameters, instance_accessor: false
82  
-    cattr_accessor :action_on_unpermitted, instance_accessor: false
  97
+    cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false
  98
+
  99
+    # Never raise an UnpermittedParameters exception because of these params
  100
+    # are present. They are added by Rails and it's of no concern.
  101
+    NEVER_UNPERMITTED_PARAMS = %w( controller action )
83 102
 
84 103
     # Returns a new instance of <tt>ActionController::Parameters</tt>.
85 104
     # Also, sets the +permitted+ attribute to the default value of
@@ -237,16 +256,8 @@ def permit(*filters)
237 256
         end
238 257
       end
239 258
 
240  
-      unpermitted_keys = self.keys - params.keys
241  
-      if unpermitted_keys.any?
242  
-        case self.class.action_on_unpermitted
243  
-        when :log
244  
-          ActionController::Base.logger.debug "Unpermitted parameters: #{unpermitted_keys.join(", ")}"
245  
-        when :raise
246  
-          raise ActionController::UnexpectedParameters.new(unpermitted_keys)
247  
-        end
248  
-      end
249  
-      
  259
+      unpermitted_parameters!(params)
  260
+
250 261
       params.permit!
251 262
     end
252 263
 
@@ -325,6 +336,22 @@ def each_element(object)
325 336
           yield object
326 337
         end
327 338
       end
  339
+
  340
+      def unpermitted_parameters!(params)
  341
+        unpermitted_keys = unpermitted_keys(params)
  342
+        if unpermitted_keys.any?
  343
+          case self.class.action_on_unpermitted_parameters
  344
+          when :log
  345
+            ActionController::Base.logger.debug "Unpermitted parameters: #{unpermitted_keys.join(", ")}"
  346
+          when :raise
  347
+            raise ActionController::UnpermittedParameters.new(unpermitted_keys)
  348
+          end
  349
+        end
  350
+      end
  351
+
  352
+      def unpermitted_keys(params)
  353
+        self.keys - params.keys - NEVER_UNPERMITTED_PARAMS
  354
+      end
328 355
   end
329 356
 
330 357
   # == Strong \Parameters
22  actionpack/lib/action_controller/railtie.rb
@@ -20,25 +20,27 @@ class Railtie < Rails::Railtie #:nodoc:
20 20
     end
21 21
 
22 22
     initializer "action_controller.parameters_config" do |app|
23  
-      ActionController::Parameters.permit_all_parameters = app.config.action_controller.delete(:permit_all_parameters) { false }
24  
-      ActionController::Parameters.action_on_unpermitted = app.config.action_controller.action_on_unpermitted_params
  23
+      options = app.config.action_controller
  24
+
  25
+      ActionController::Parameters.permit_all_parameters = options.delete(:permit_all_parameters) { false }
  26
+      ActionController::Parameters.action_on_unpermitted_parameters = options.delete(:action_on_unpermitted_parameters) do
  27
+        (Rails.env.test? || Rails.env.development?) ? :log : false
  28
+      end
25 29
     end
26 30
 
27 31
     initializer "action_controller.set_configs" do |app|
28 32
       paths   = app.config.paths
29 33
       options = app.config.action_controller
30 34
 
31  
-      options.logger                        ||= Rails.logger
32  
-      options.cache_store                   ||= Rails.cache
  35
+      options.logger      ||= Rails.logger
  36
+      options.cache_store ||= Rails.cache
33 37
 
34  
-      options.javascripts_dir               ||= paths["public/javascripts"].first
35  
-      options.stylesheets_dir               ||= paths["public/stylesheets"].first
  38
+      options.javascripts_dir ||= paths["public/javascripts"].first
  39
+      options.stylesheets_dir ||= paths["public/stylesheets"].first
36 40
 
37 41
       # Ensure readers methods get compiled
38  
-      options.asset_host                    ||= app.config.asset_host
39  
-      options.relative_url_root             ||= app.config.relative_url_root
40  
-
41  
-      options.action_on_unpermitted_params  ||= (Rails.env.test? || Rails.env.development?) ? :log : false
  42
+      options.asset_host        ||= app.config.asset_host
  43
+      options.relative_url_root ||= app.config.relative_url_root
42 44
 
43 45
       ActiveSupport.on_load(:action_controller) do
44 46
         include app.routes.mounted_helpers
6  actionpack/test/controller/parameters/log_on_unpermitted_params_test.rb
@@ -3,11 +3,11 @@
3 3
 
4 4
 class LogOnUnpermittedParamsTest < ActiveSupport::TestCase
5 5
   def setup
6  
-    ActionController::Parameters.action_on_unpermitted = :log
  6
+    ActionController::Parameters.action_on_unpermitted_parameters = :log
7 7
   end
8 8
 
9 9
   def teardown
10  
-    ActionController::Parameters.action_on_unpermitted = false
  10
+    ActionController::Parameters.action_on_unpermitted_parameters = false
11 11
   end
12 12
 
13 13
   test "logs on unexpected params" do
@@ -47,4 +47,4 @@ def assert_logged(message)
47 47
       ActionController::Base.logger = old_logger
48 48
     end
49 49
   end
50  
-end
  50
+end
10  actionpack/test/controller/parameters/raise_on_unpermitted_params_test.rb
@@ -3,11 +3,11 @@
3 3
 
4 4
 class RaiseOnUnpermittedParamsTest < ActiveSupport::TestCase
5 5
   def setup
6  
-    ActionController::Parameters.action_on_unpermitted = :raise
  6
+    ActionController::Parameters.action_on_unpermitted_parameters = :raise
7 7
   end
8 8
 
9 9
   def teardown
10  
-    ActionController::Parameters.action_on_unpermitted = false
  10
+    ActionController::Parameters.action_on_unpermitted_parameters = false
11 11
   end
12 12
 
13 13
   test "raises on unexpected params" do
@@ -16,7 +16,7 @@ def teardown
16 16
       fishing: "Turnips"
17 17
     })
18 18
 
19  
-    assert_raises(ActionController::UnexpectedParameters) do
  19
+    assert_raises(ActionController::UnpermittedParameters) do
20 20
       params.permit(book: [:pages])
21 21
     end
22 22
   end
@@ -26,8 +26,8 @@ def teardown
26 26
       book: { pages: 65, title: "Green Cats and where to find then." }
27 27
     })
28 28
 
29  
-    assert_raises(ActionController::UnexpectedParameters) do
  29
+    assert_raises(ActionController::UnpermittedParameters) do
30 30
       params.permit(book: [:pages])
31 31
     end
32 32
   end
33  
-end
  33
+end
48  railties/test/application/configuration_test.rb
@@ -577,6 +577,54 @@ def create
577 577
       assert_equal 'permitted', last_response.body
578 578
     end
579 579
 
  580
+    test "config.action_controller.action_on_unpermitted_parameters = :raise" do
  581
+      app_file 'app/controllers/posts_controller.rb', <<-RUBY
  582
+      class PostsController < ActionController::Base
  583
+        def create
  584
+          render text: params.require(:post).permit(:name)
  585
+        end
  586
+      end
  587
+      RUBY
  588
+
  589
+      add_to_config <<-RUBY
  590
+        routes.prepend do
  591
+          resources :posts
  592
+        end
  593
+        config.action_controller.action_on_unpermitted_parameters = :raise
  594
+      RUBY
  595
+
  596
+      require "#{app_path}/config/environment"
  597
+
  598
+      assert_equal :raise, ActionController::Parameters.action_on_unpermitted_parameters
  599
+
  600
+      post "/posts", {post: {"title" =>"zomg"}}
  601
+      assert_match "We're sorry, but something went wrong", last_response.body
  602
+    end
  603
+
  604
+    test "config.action_controller.action_on_unpermitted_parameters is :log by default on development" do
  605
+      ENV["RAILS_ENV"] = "development"
  606
+
  607
+      require "#{app_path}/config/environment"
  608
+
  609
+      assert_equal :log, ActionController::Parameters.action_on_unpermitted_parameters
  610
+    end
  611
+
  612
+    test "config.action_controller.action_on_unpermitted_parameters is :log by defaul on test" do
  613
+      ENV["RAILS_ENV"] = "test"
  614
+
  615
+      require "#{app_path}/config/environment"
  616
+
  617
+      assert_equal :log, ActionController::Parameters.action_on_unpermitted_parameters
  618
+    end
  619
+
  620
+    test "config.action_controller.action_on_unpermitted_parameters is false by default on production" do
  621
+      ENV["RAILS_ENV"] = "production"
  622
+
  623
+      require "#{app_path}/config/environment"
  624
+
  625
+      assert_equal false, ActionController::Parameters.action_on_unpermitted_parameters
  626
+    end
  627
+
580 628
     test "config.action_dispatch.ignore_accept_header" do
581 629
       make_basic_app do |app|
582 630
         app.config.action_dispatch.ignore_accept_header = true

0 notes on commit 57126ee

Please sign in to comment.
Something went wrong with that request. Please try again.