From cc1c3c5be061e7572018f734e5239750ab449e3f Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 8 Jan 2013 16:10:34 +0100 Subject: Revert "unpermitted params" exception -- it's just not going to work. See the discussion on https://github.com/rails/strong_parameters/pull/75. --- .../action_controller/metal/strong_parameters.rb | 53 ++-------------------- actionpack/lib/action_controller/railtie.rb | 17 +++---- .../raise_on_unpermitted_parameters_test.rb | 43 ------------------ railties/test/application/configuration_test.rb | 48 -------------------- 4 files changed, 11 insertions(+), 150 deletions(-) delete mode 100644 actionpack/test/controller/parameters/raise_on_unpermitted_parameters_test.rb diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 3691dc699f..1d286524f4 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -19,20 +19,6 @@ module ActionController end end - # Raised when a supplied parameter is not permitted. - # - # params = ActionController::Parameters.new(a: "123", b: "456") - # params.permit(:c) - # # => ActionController::UnpermittedParameters: found unpermitted keys: a, b - class UnpermittedParameters < IndexError - attr_reader :params # :nodoc: - - def initialize(params) # :nodoc: - @params = params - super("found unpermitted keys: #{params.join(", ")}") - end - end - # == Action Controller \Parameters # # Allows to choose which attributes should be whitelisted for mass updating @@ -57,14 +43,10 @@ module ActionController # Person.first.update!(permitted) # # => # # - # 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+. - # * +raise_on_unpermitted_parameters+ - If it's +true+, it will raise an - # ActionController::UnpermittedParameters exception if parameters that are not - # explicitly permitted are found. The default value is +true+ in test and - # development environments, +false+ otherwise. + # It provides a +permit_all_parameters+ option that controls the top-level + # behaviour 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+. # # params = ActionController::Parameters.new # params.permitted? # => false @@ -74,16 +56,6 @@ module ActionController # params = ActionController::Parameters.new # params.permitted? # => true # - # params = ActionController::Parameters.new(a: "123", b: "456") - # params.permit(:c) - # # => {} - # - # ActionController::Parameters.raise_on_unpermitted_parameters = true - # - # params = ActionController::Parameters.new(a: "123", b: "456") - # params.permit(:c) - # # => ActionController::UnpermittedParameters: found unpermitted keys: a, b - # # ActionController::Parameters is inherited from # ActiveSupport::HashWithIndifferentAccess, this means # that you can fetch values using either :key or "key". @@ -93,11 +65,6 @@ module ActionController # params["key"] # => "value" class Parameters < ActiveSupport::HashWithIndifferentAccess cattr_accessor :permit_all_parameters, instance_accessor: false - cattr_accessor :raise_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 ActionController::Parameters. # Also, sets the +permitted+ attribute to the default value of @@ -255,8 +222,6 @@ module ActionController end end - raise_on_unpermitted_parameters!(params) - params.permit! end @@ -335,16 +300,6 @@ module ActionController yield object end end - - def raise_on_unpermitted_parameters!(params) - if self.class.raise_on_unpermitted_parameters && unpermitted_keys(params).any? - raise ActionController::UnpermittedParameters.new(unpermitted_keys(params)) - 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 731d66b0cf..3e44155f73 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -20,25 +20,22 @@ module ActionController end initializer "action_controller.parameters_config" do |app| - options = app.config.action_controller - - ActionController::Parameters.permit_all_parameters = options.delete(:permit_all_parameters) { false } - ActionController::Parameters.raise_on_unpermitted_parameters = options.delete(:raise_on_unpermitted_parameters) { Rails.env.test? || Rails.env.development? } + ActionController::Parameters.permit_all_parameters = app.config.action_controller.delete(:permit_all_parameters) { false } 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.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/raise_on_unpermitted_parameters_test.rb b/actionpack/test/controller/parameters/raise_on_unpermitted_parameters_test.rb deleted file mode 100644 index 3cedc16730..0000000000 --- a/actionpack/test/controller/parameters/raise_on_unpermitted_parameters_test.rb +++ /dev/null @@ -1,43 +0,0 @@ -require 'abstract_unit' -require 'action_controller/metal/strong_parameters' - -class RaiseOnUnpermittedParametersTest < ActiveSupport::TestCase - def setup - ActionController::Parameters.raise_on_unpermitted_parameters = true - end - - def teardown - ActionController::Parameters.raise_on_unpermitted_parameters = false - end - - test "raises on unexpected params" do - params = ActionController::Parameters.new({ - book: { pages: 65 }, - fishing: "Turnips" - }) - - assert_raises(ActionController::UnpermittedParameters) do - params.permit(book: [:pages]) - end - end - - test "raises on unexpected nested params" do - params = ActionController::Parameters.new({ - book: { pages: 65, title: "Green Cats and where to find then." } - }) - - assert_raises(ActionController::UnpermittedParameters) do - params.permit(book: [:pages]) - end - end - - test "action and controller keys are safe to ignore" do - params = ActionController::Parameters.new({ - action: 'index', controller: 'stuff', book: { pages: 65 } - }) - - assert_nothing_raised do - params.permit(book: [:pages]) - end - end -end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 654a44e648..f9108ed7b9 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -577,54 +577,6 @@ module ApplicationTests assert_equal 'permitted', last_response.body end - test "config.action_controller.raise_on_unpermitted_parameters = true" 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.raise_on_unpermitted_parameters = true - RUBY - - require "#{app_path}/config/environment" - - assert_equal true, ActionController::Parameters.raise_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.raise_on_unpermitted_parameters is true by default on development" do - ENV["RAILS_ENV"] = "development" - - require "#{app_path}/config/environment" - - assert_equal true, ActionController::Parameters.raise_on_unpermitted_parameters - end - - test "config.action_controller.raise_on_unpermitted_parameters is true by defaul on test" do - ENV["RAILS_ENV"] = "test" - - require "#{app_path}/config/environment" - - assert_equal true, ActionController::Parameters.raise_on_unpermitted_parameters - end - - test "config.action_controller.raise_on_unpermitted_parameters is false by default on production" do - ENV["RAILS_ENV"] = "production" - - require "#{app_path}/config/environment" - - assert_equal false, ActionController::Parameters.raise_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 -- cgit v1.2.3