Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix permit matcher for multiple instances of params #902

Merged
merged 1 commit into from
Nov 21, 2016
Merged

Fix permit matcher for multiple instances of params #902

merged 1 commit into from
Nov 21, 2016

Conversation

aripollak
Copy link

When the permit matcher is used without #on, the controller does not use params#require, and the params object gets duplicated, the matcher did not recognize the #permit call inside the controller. This happened because the matcher overwrote double registries with the same parameter hash whenever ActionController::Parameters was instantiated. The duplication happens unintentionally when passing the params to a model's method that ends up calling #assign_attributes, like #new.
This is related to #899.

Here's the part of the call stack that was doing the duplication in Rails:

/home/ari/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/activesupport-4.2.5.1/lib/active_support/hash_with_indifferent_access.rb:190:in `dup'
/home/ari/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/actionpack-4.2.5.1/lib/action_controller/metal/strong_parameters.rb:438:in `dup'
/home/ari/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/activesupport-4.2.5.1/lib/active_support/hash_with_indifferent_access.rb:232:in `stringify_keys'
/home/ari/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/protected_attributes-1.1.3/lib/active_record/mass_assignment_security/attribute_assignment.rb:51:in `assign_attributes'
/home/ari/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/protected_attributes-1.1.3/lib/active_record/mass_assignment_security/persistence.rb:64:in `block in update'

When the permit matcher is used without `#on`, the controller does
not use `params#require`, and the params object is duplicated, the
matcher did not recognize the `#permit` call inside the controller.
This happened because the matcher overwrote double registries with the
same parameter hash whenever ActionController::Parameters was
instantiated.
This is related to #899.
@mcmire
Copy link
Collaborator

mcmire commented Jun 13, 2016

The duplication happens unintentionally when passing the params to a model's method that ends up calling #assign_attributes, like #new.

I know it's been a little while but can you explain this further?

@aripollak
Copy link
Author

If I'm remembering this correctly, the matcher was failing when doing something like this in the controller:

@model = Model.new(params.fetch(:model, {}).permit(:foo))
@model.update(params.fetch(:model, {}).permit(:foo))

@mcmire
Copy link
Collaborator

mcmire commented Jun 13, 2016

Okay, thanks.

The other thought I had while reading your PR was that there was a reason why I was using a hash instead of an array to store doubles for ActionController::Parameters. I can't remember why I did that off-hand, though. I'll have to take a closer look at that.

@geoffharcourt
Copy link
Contributor

Merging this after testing.

@geoffharcourt geoffharcourt merged commit 44c0198 into thoughtbot:master Nov 21, 2016
@mcmire
Copy link
Collaborator

mcmire commented Nov 21, 2016

@geoffharcourt Woohoo, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants