Skip to content

Commit 7a80b69

Browse files
committed
Don't convert params if the request isn't HTML - fixes rails#5341
1 parent ffc861c commit 7a80b69

File tree

2 files changed

+58
-8
lines changed

2 files changed

+58
-8
lines changed

actionpack/lib/action_controller/test_case.rb

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -147,17 +147,23 @@ def assign_parameters(routes, controller_path, action, parameters = {})
147147
extra_keys = routes.extra_keys(parameters)
148148
non_path_parameters = get? ? query_parameters : request_parameters
149149
parameters.each do |key, value|
150-
if value.is_a? Fixnum
151-
value = value.to_s
152-
elsif value.is_a? Array
153-
value = Result.new(value.map { |v| v.is_a?(String) ? v.dup : v })
154-
elsif value.is_a? String
150+
if value.is_a?(Array) && (value.frozen? || value.any?(&:frozen?))
151+
value = value.map{ |v| v.duplicable? ? v.dup : v }
152+
elsif value.is_a?(Hash) && (value.frozen? || value.any?{ |k,v| v.frozen? })
153+
value = Hash[value.map{ |k,v| [k, v.duplicable? ? v.dup : v] }]
154+
elsif value.frozen? && value.duplicable?
155155
value = value.dup
156156
end
157157

158158
if extra_keys.include?(key.to_sym)
159159
non_path_parameters[key] = value
160160
else
161+
if value.is_a?(Array)
162+
value = Result.new(value.map(&:to_param))
163+
else
164+
value = value.to_param
165+
end
166+
161167
path_parameters[key.to_s] = value
162168
end
163169
end
@@ -453,7 +459,7 @@ def process(action, http_method = 'GET', *args)
453459

454460
# Ensure that numbers and symbols passed as params are converted to
455461
# proper params, as is the case when engaging rack.
456-
parameters = paramify_values(parameters)
462+
parameters = paramify_values(parameters) if html_format?(parameters)
457463

458464
@request.recycle!
459465
@response.recycle!
@@ -546,6 +552,12 @@ def build_request_uri(action, parameters)
546552
@request.env["QUERY_STRING"] = query_string || ""
547553
end
548554
end
555+
556+
def html_format?(parameters)
557+
return true unless parameters.is_a?(Hash)
558+
format = Mime[parameters[:format]]
559+
format.nil? || format.html?
560+
end
549561
end
550562

551563
# When the request.remote_addr remains the default for testing, which is 0.0.0.0, the exception is simply raised inline

actionpack/test/controller/test_case_test.rb

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ def test_uri
4545
render :text => request.fullpath
4646
end
4747

48+
def test_format
49+
render :text => request.format
50+
end
51+
4852
def test_query_string
4953
render :text => request.query_string
5054
end
@@ -579,14 +583,34 @@ def test_params_passing_with_fixnums
579583
)
580584
end
581585

586+
def test_params_passing_with_fixnums_when_not_html_request
587+
get :test_params, :format => 'json', :count => 999
588+
parsed_params = eval(@response.body)
589+
assert_equal(
590+
{'controller' => 'test_case_test/test', 'action' => 'test_params',
591+
'format' => 'json', 'count' => 999 },
592+
parsed_params
593+
)
594+
end
595+
596+
def test_params_passing_path_parameter_is_string_when_not_html_request
597+
get :test_params, :format => 'json', :id => 1
598+
parsed_params = eval(@response.body)
599+
assert_equal(
600+
{'controller' => 'test_case_test/test', 'action' => 'test_params',
601+
'format' => 'json', 'id' => '1' },
602+
parsed_params
603+
)
604+
end
605+
582606
def test_params_passing_with_frozen_values
583607
assert_nothing_raised do
584-
get :test_params, :frozen => 'icy'.freeze, :frozens => ['icy'.freeze].freeze
608+
get :test_params, :frozen => 'icy'.freeze, :frozens => ['icy'.freeze].freeze, :deepfreeze => { :frozen => 'icy'.freeze }.freeze
585609
end
586610
parsed_params = eval(@response.body)
587611
assert_equal(
588612
{'controller' => 'test_case_test/test', 'action' => 'test_params',
589-
'frozen' => 'icy', 'frozens' => ['icy']},
613+
'frozen' => 'icy', 'frozens' => ['icy'], 'deepfreeze' => { 'frozen' => 'icy' }},
590614
parsed_params
591615
)
592616
end
@@ -687,6 +711,20 @@ def test_request_protocol_is_reset_after_request
687711
assert_equal "http://", @response.body
688712
end
689713

714+
def test_request_format
715+
get :test_format, :format => 'html'
716+
assert_equal 'text/html', @response.body
717+
718+
get :test_format, :format => 'json'
719+
assert_equal 'application/json', @response.body
720+
721+
get :test_format, :format => 'xml'
722+
assert_equal 'application/xml', @response.body
723+
724+
get :test_format
725+
assert_equal 'text/html', @response.body
726+
end
727+
690728
def test_should_have_knowledge_of_client_side_cookie_state_even_if_they_are_not_set
691729
cookies['foo'] = 'bar'
692730
get :no_op

0 commit comments

Comments
 (0)