Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Params middleware errors when a JSON array is posted #218

Merged
merged 1 commit into from

2 participants

@radsaq

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

@igrigorik
Owner

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

@radsaq

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.)

@igrigorik
Owner

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?

@radsaq

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

@igrigorik
Owner

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

lgtm.

@igrigorik igrigorik merged commit 34ac587 into postrank-labs:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 28, 2012
  1. @radsaq
This page is out of date. Refresh to see the latest.
Showing with 16 additions and 1 deletion.
  1. +6 −1 lib/goliath/rack/params.rb
  2. +10 −0 spec/unit/rack/params_spec.rb
View
7 lib/goliath/rack/params.rb
@@ -31,7 +31,12 @@ def retrieve_params(env)
when URL_ENCODED then
::Rack::Utils.parse_nested_query(body)
when JSON_ENCODED then
- MultiJson.load(body)
+ json = MultiJson.load(body)
+ if json.is_a?(Hash)
+ json
+ else
+ {'_json' => json}
+ end
else
{}
end
View
10 spec/unit/rack/params_spec.rb
@@ -142,6 +142,16 @@
ret['foo'].should == 'bar'
end
+ it "parses json that does not evaluate to a hash" do
+ @env['CONTENT_TYPE'] = 'application/json'
+ @env['rack.input'] = StringIO.new
+ @env['rack.input'] << %|["foo","bar"]|
+ @env['rack.input'].rewind
+
+ ret = @params.retrieve_params(@env)
+ ret['_json'].should == ['foo', 'bar']
+ end
+
it "handles empty input gracefully on JSON" do
@env['CONTENT_TYPE'] = 'application/json'
@env['rack.input'] = StringIO.new
Something went wrong with that request. Please try again.