Rails core - Jul 2013 - ParamsParser middleware behaviour with Mime::JSON

Hi guys!

My eye caught this part in ParamsParser middleware:
https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/params_parser.rb#L45
data = {:_json => data} unless data.is_a?(Hash)

If data is a hash, we''re leaving it as is (aka hash with pretty random 
keys), else we''re getting a new hash with key :_json on the top level.
As a
consequence on some occasions we''ll get :_json key, on some others - we
won''t. This kind of implicit transformation just feels inconsistent and
wrong to me. Honestly i''d like to change this behaviour (by submitting
a
patch of course) with setting :_json key anytime we''re dealing with 
Mime::JSON.
I''d like to hear some opinions/feedback on this matter. Will it be big 
backwards compability issue? In my own practice it won''t.

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to rubyonrails-core+unsubscribe@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/groups/opt_out.

On Jul 7, 2013, at 4:39 PM, Denis Redozubov wrote:
> Hi guys!
> 
> My eye caught this part in ParamsParser middleware:
>
https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/params_parser.rb#L45
> data = {:_json => data} unless data.is_a?(Hash)
> 
> If data is a hash, we''re leaving it as is (aka hash with pretty
random keys), else we''re getting a new hash with key :_json on the top
level. As a consequence on some occasions we''ll get :_json key, on some
others - we won''t. This kind of implicit transformation just feels
inconsistent and wrong to me. Honestly i''d like to change this
behaviour (by submitting a patch of course) with setting :_json key anytime
we''re dealing with Mime::JSON.
> I''d like to hear some opinions/feedback on this matter. Will it be
big backwards compability issue? In my own practice it won''t.
The challenge is that the hash that''s set up there is merged with
query_parameters, etc:

https://github.com/rails/rails/blob/bc8f8026232bdc55ebed24d8326a863036b816fd/actionpack/lib/action_dispatch/http/parameters.rb#L15

so if the request body parses to something that *isn''t* a Hash (if
it''s a JSON array, for instance) than pretty spectacular fireworks
ensue. Presumably that''s the intent behind the _json business.

As to backwards compatibility, inserting an extra :_json key (changing
params[:foo] into params[:_json][:foo]) in literally every params hash in every
controller action that accepts JSON does seem like a bit of an issue. :)


--Matt Jones

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to rubyonrails-core+unsubscribe@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/groups/opt_out.