Params middleware errors when a JSON array is posted #218

Merged
merged 1 commit into from Nov 30, 2012

Conversation

Projects
None yet
2 participants
Contributor

radsaq commented Nov 28, 2012

The middleware should not assume that the JSON is always going to evaluate to a hash.

Owner

igrigorik commented Nov 28, 2012

Huh.. why? Not sure I follow. What's wrong with an array as an input?

Contributor

radsaq commented Nov 28, 2012

The code later calls Hash#merge! using the result from the case statement. Naturally, this errors if an array was passed in as the post body.

(For more context, I'm also still using 0.9.4 where this code is used inside of builder.rb, which causes the app to silently and mysteriously fail if you pass a JSON array and an content type of application/json, even without explicitly using the middleware: https://github.com/postrank-labs/goliath/blob/d6db2d3d8f76fadaafef4e2a583363371e6e8b30/lib/goliath/rack/builder.rb ... But regardless, there's no reason for this code to fail in this case.)

Owner

igrigorik commented Nov 29, 2012

Seems like modifying the input payload is the wrong way to go about it.. Perhaps if its an array, we should simply skip the merge? How does Rails handle this case?

Contributor

radsaq commented Nov 29, 2012

My patch makes the handling be essentially the same as what rails does: https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/params_parser.rb#L54

Owner

igrigorik commented Nov 30, 2012

Hmm, that's just confusing.. The magic of Rails conventions. :)

lgtm.

igrigorik added a commit that referenced this pull request Nov 30, 2012

Merge pull request #218 from radsaq/fix_json_params
Params middleware errors when a JSON array is posted

@igrigorik igrigorik merged commit 34ac587 into postrank-labs:master Nov 30, 2012

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment