rails / strong_parameters

Taint and required checking for Action Pack and enforcement in Active Model
MIT License
1.27k stars 164 forks source link

Parameter Missing Exception even though it is not missing #162

Open HoneyryderChuck opened 11 years ago

HoneyryderChuck commented 11 years ago

I'm implementing the strong_parameters gem on a rails 3.2 app (don't know the status of this in rails 4). I'm testing with rspec submission to a create action with invalid parameters. In this particular case I'm sending no parameters at all (an empty hash). Something like:

post :create, :user => {}

On the controller I'm requiring the user key using the strong parameters syntax:

params.require(:user).permit(:name, :password)

thing is, I'm getting the "ActionController::ParameterMissing: key not found: user" Exception message whenever I submit this empty hash. Granted, live the hash will always have something (csrf tag, utf8 val) but this seems kind of wrong. the user key is in fact there, so it should be checked whether the key exists and not if the value pointed to it is blank (nil or empty). Is it really like that?

kangguru commented 11 years ago

I was also stumbling about that. But actually this looks like intended behavior as one can see from:

https://github.com/rails/strong_parameters/blob/master/test/parameters_require_test.rb#L7

Questions is if a required parameter is present though it is just empty :)

just3ws commented 10 years ago

Came to submit an issue on this exact topic. My example was I have a parameter that's present but has an empty hash.

    [5] badgiy(#<ProtipsController>) »  ap params
    {
                "id" => "tkg0ca",
            "protip" => {},
        "controller" => "protips",
            "action" => "update"
    }
    => nil

    [6] badgiy(#<ProtipsController>) »  params[:protip]
    => {}

    [7] badgiy(#<ProtipsController>) »  params.require(:protip)
    ActionController::ParameterMissing: param is missing or the value is empty: protip
    from /Users/mike/.rvm/gems/ruby-1.9.3-p545@coderwall-web/gems/strong_parameters-0.2.3/lib/action_controller/parameters.rb:58:in `require'

It seems unintuitive for the key to present and not be nil but still have the validation fail with an exception.

flatrocks commented 10 years ago

Yes, my opinion too. Throwing an "ActionController::ParameterMissing" exception when a require()-ed parameter is present as an empty hash (common in testing) seemed wrong, but the docs do say require() "Ensures that a parameter is present" and {}.present? evaluates to false, so I suppose it's just an annoying design decision and not really a bug.

Since I don't really care if "strong parameters" protect against missing parameters anyway, this:

params.fetch(:csv_import, Hash.new).permit(:target_class_name, :csv_file, :delimiter, :encoding)

works well, using fetch to replace the annoying require() method. Perhaps examples and scaffolding should use this approach.

mjrk commented 9 years ago

+1 scaffolds should be changed. When submitting empty forms - it results in an exception. This is really counter-intuitive. Newly generated scaffolds should kind of work. The point with require() to be patched nil? and not present? is that it would not fix the issue for submitting empty forms.

IMHO the perfect solution: forms - even though all inputs are empty - should transmit the key with an empty hash and require should check for nil? then. But this would require too many changes. Too bad, at the end I cannot use strong parameters' require() anymore.

MrTheWalrus commented 9 years ago

+1 something here should be changed. An empty form/parameter hash may or may not be a valid submission, but it's definitely not the kind of thing that strong params is supposed to be catching - that's what validations are for.

hernansartorio commented 9 years ago

+1

askl56 commented 9 years ago

I agree. It demands needless complexity in your tests.

fredngo commented 9 years ago

Agreed. Ran into this most recently when a form had only radio inputs, and it is submitted without selecting an option. It turns out radio buttons don't create a parameter when they are empty.

I agree with @MrTheWalrus that validations should handle when an option isn't selected, not strong parameters.

fredngo commented 9 years ago

I found myself implementing a hack similar to that described by @flatrocks all over the place, so I've replaced them all with this monkey patch. I know monkey patches are not in style anymore, but in this case it allows me to keep my controller code idiomatic. So for anyone else who might need something similar, here it is:

module ActionController
  module ParametersExtensions

    # See https://github.com/rails/strong_parameters/issues/162
    def require key
      begin
        super key
      rescue ParameterMissing => e
        if self[key].nil?
          return Parameters.new
        else
          raise e
        end
      end
    end

  end

  class Parameters
    prepend ParametersExtensions
  end
end
ryannealmes commented 8 years ago

+1

adilsoncarvalho commented 8 years ago

I solved that by creating an instance of the parameters class and passed it to the method. Doing so it finds no issue with strong parameters params.require(:user).permit(:email, :password) blocks.

process :create, method: :post, params: { session: { email: 'a@a.com', password: 'pwd' } }

I hope that it helps someone.