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

Jump to bottom

Fix #8832 - Parse '{"person":[]}' JSON/XML as {'person' => []}. #9111

Merged
merged 1 commit into from Jan 30, 2013
+11 −1
Merged

Fix #8832 - Parse '{"person":[]}' JSON/XML as {'person' => []}. #9111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion actionpack/lib/action_dispatch/http/request.rb
View file
Open in desktop
Expand Up @@ -263,9 +263,12 @@ def deep_munge(hash)
hash.each do |k, v|
case v
when Array
if v.size > 0 && v.all?(&:nil?)
hash[k] = nil
next
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like your code will convert this JSON:

{"person":[null]}

to this Ruby hash:

{"person": nil}

Is that right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right.

After thinking about it some more, I personally think [null] is a valid query, and the problem should be solved with something like .query_present?, that returns false for "", nil, [], [nil], [""], etc.

v.grep(Hash) { |x| deep_munge(x) }
v.compact!
hash[k] = nil if v.empty?
when Hash
deep_munge(v)
end
Expand Down
4 changes: 4 additions & 0 deletions actionpack/test/dispatch/request/json_params_parsing_test.rb
View file
Open in desktop
Expand Up @@ -31,6 +31,10 @@ def teardown
end

test "nils are stripped from collections" do
assert_parses(
{"person" => []},
"{\"person\":[]}", { 'CONTENT_TYPE' => 'application/json' }
)
assert_parses(
{"person" => nil},
"{\"person\":[null]}", { 'CONTENT_TYPE' => 'application/json' }
Expand Down
3 changes: 3 additions & 0 deletions actionpack/test/dispatch/request/xml_params_parsing_test.rb
View file
Open in desktop
Expand Up @@ -41,6 +41,9 @@ def assert_parses(expected, xml)
assert_parses(
{"hash" => { "person" => nil} },
"<hash><person type=\"array\"><person nil=\"true\"/></person></hash>")
assert_parses(
{"hash" => { "person" => []} },
"<hash><person type=\"array\"></person></hash>")
assert_parses(
{"hash" => { "person" => ['foo']} },
"<hash><person type=\"array\"><person>foo</person><person nil=\"true\"/></person>\n</hash>")
Expand Down