This repository has been archived by the owner. It is now read-only.

ActionController::Parameters#fetch has side effects #56

Open
phene opened this Issue Nov 2, 2012 · 3 comments

Comments

Projects
None yet
3 participants

phene commented Nov 2, 2012

It appears that calling fetch will modify the receiver via convert_hashes_to_parameters. The 2nd argument to fetch should not assign the default value to the provided key.

params = ActionController::Parameters.new :controller => 'foo', :action => 'bar' #=> {"controller"=>"foo", "action"=>"bar"} 
params.fetch(:message, {})[:id] #=> nil
params #=> {"controller"=>"foo", "action"=>"bar", "message"=>{}} 

hsh = {:controller => 'foo', :action => 'bar'}  #=> {"controller"=>"foo", "action"=>"bar"} 
hsh.fetch(:message, {})[:id] #=> nil
hsh #=> {"controller"=>"foo", "action"=>"bar"} 

@phene convert_hashes_to_parameters is available in ActionController::Parameters so this can work if hash is a object of class ActionController::Parameters. in your second example ruby#hash default fetch method is invoking so its not updating the receiver.

/cc @rafaelfranca

phene commented Nov 15, 2012

@vatrai but convert_hashes_to_parameters modifies the receiver, and fetch calls it. This is a pretty bad break of the Hash#fetch contract.

@bemurphy bemurphy added a commit to bemurphy/strong_parameters that referenced this issue Feb 19, 2013

@bemurphy bemurphy Stop fetch from mutating when default is a hash
See gh-56 for an issue on this
4dd0a1d
Contributor

bemurphy commented Feb 19, 2013

I've noticed btw this only happens when the default value provided is a hash. If it's a string, fixnum, etc, won't happen. I think the fix is for fetch to handle the hash wrapping and not rely on convert_hashes_to_parameters so its not surprising.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.