Skip to content

Commit

Permalink
Modify Hash#to_query to not sort lexicographically for nested struc…
Browse files Browse the repository at this point in the history
…ture:

- As discussed in rails#33341 (comment)
  • Loading branch information
Edouard-chin committed Jul 11, 2018
1 parent 7fae8e3 commit 11fc857
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 5 deletions.
2 changes: 1 addition & 1 deletion actionpack/lib/action_controller/test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def assign_parameters(routes, controller_path, action, parameters, generated_pat
when :xml
data = non_path_parameters.to_xml
when :url_encoded_form
data = Rack::Utils.build_nested_query(non_path_parameters)
data = non_path_parameters.to_query
else
@custom_param_parsers[content_mime_type.symbol] = ->(_) { non_path_parameters }
data = non_path_parameters.to_query
Expand Down
17 changes: 15 additions & 2 deletions actionpack/test/controller/test_case_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ def test_raw_post_handling
params = Hash[:page, { name: "page name" }, "some key", 123]
post :render_raw_post, params: params.dup

assert_equal Rack::Utils.build_nested_query(params), @response.body
assert_equal params.to_query, @response.body
end

def test_params_round_trip
Expand All @@ -231,12 +231,25 @@ def test_params_round_trip
assert_equal params.merge(controller_info), JSON.parse(@response.body)
end

def test_handle_to_params
klass = Class.new do
def to_param
"bar"
end
end

post :test_params, params: { foo: klass.new }

assert_equal JSON.parse(@response.body)["foo"], "bar"
end


def test_body_stream
params = Hash[:page, { name: "page name" }, "some key", 123]

post :render_body, params: params.dup

assert_equal Rack::Utils.build_nested_query(params), @response.body
assert_equal params.to_query, @response.body
end

def test_document_body_and_params_with_post
Expand Down
7 changes: 5 additions & 2 deletions activesupport/lib/active_support/core_ext/object/to_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,14 @@ class Hash
#
# This method is also aliased as +to_param+.
def to_query(namespace = nil)
collect do |key, value|
query = collect do |key, value|
unless (value.is_a?(Hash) || value.is_a?(Array)) && value.empty?
value.to_query(namespace ? "#{namespace}[#{key}]" : key)
end
end.compact.sort! * "&"
end.compact

query.sort! unless namespace =~ /\[\w+\]\[\]/
query.join("&")
end

alias_method :to_param, :to_query
Expand Down
14 changes: 14 additions & 0 deletions activesupport/test/core_ext/object/to_query_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,20 @@ def test_hash_sorted_lexicographically
assert_equal "name=Nakshay&type=human", hash.to_query
end

def test_hash_not_sorted_lexicographically_for_nested_structure
params = {
"foo" => {
"contents" => [
{ "name" => "gorby", "id" => "123" },
{ "name" => "puff", "d" => "true" }
]
}
}
expected = "foo[contents][][name]=gorby&foo[contents][][id]=123&foo[contents][][name]=puff&foo[contents][][d]=true"

assert_equal expected, URI.decode(params.to_query)
end

private
def assert_query_equal(expected, actual)
assert_equal expected.split("&"), actual.to_query.split("&")
Expand Down

0 comments on commit 11fc857

Please sign in to comment.