Skip to content

Commit 0821d25

Browse files
skipkayhilrafaelfranca
authored andcommitted
Add custom ArgumentError for invalid to: values
Previously, it was theoretically possible to define a route with a Symbol as a `to:` value (or at least, it would not raise a `NoMethodError`). However, passing a Symbol broke when `/#/.match?(to)` was [replaced][1] with `to&.include?("#")` with the assumption that `to` was always a String. Instead of restoring the previous error, this commit improves how the `to:` value is checked so that it raises an `ArgumentError` for any invalid values. The extra strictness will specifically improve the error when a Symbol or String that doesn't include a "#" are passed since they were effectively equivalent to passing a `nil` value, or not specifying `to:` at all. [1]: 5726b1d
1 parent cddf163 commit 0821d25

File tree

2 files changed

+23
-13
lines changed

2 files changed

+23
-13
lines changed

actionpack/lib/action_dispatch/routing/mapper.rb

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -216,14 +216,23 @@ def normalize_options!(options, path_params, modyoule)
216216

217217
if to.respond_to?(:action) || to.respond_to?(:call)
218218
options
219-
else
220-
to_endpoint = split_to to
221-
controller = to_endpoint[0] || default_controller
222-
action = to_endpoint[1] || default_action
219+
elsif to.nil?
220+
controller = default_controller
221+
action = default_action
222+
223+
controller = add_controller_module(controller, modyoule)
224+
225+
options.merge! check_controller_and_action(path_params, controller, action)
226+
elsif to.is_a?(String) && to.include?("#")
227+
to_endpoint = to.split("#").map!(&:-@)
228+
controller = to_endpoint[0]
229+
action = to_endpoint[1]
223230

224231
controller = add_controller_module(controller, modyoule)
225232

226233
options.merge! check_controller_and_action(path_params, controller, action)
234+
else
235+
raise ArgumentError, ":to must respond_to? :action or :call, or it must be a String that includes '#'"
227236
end
228237
end
229238

@@ -308,14 +317,6 @@ def check_part(name, part, path_params, hash)
308317
hash
309318
end
310319

311-
def split_to(to)
312-
if to&.include?("#")
313-
to.split("#").map!(&:-@)
314-
else
315-
[]
316-
end
317-
end
318-
319320
def add_controller_module(controller, modyoule)
320321
if modyoule && !controller.is_a?(Regexp)
321322
if controller&.start_with?("/")

actionpack/test/dispatch/routing_test.rb

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4054,7 +4054,16 @@ def test_missing_controller_with_to
40544054
get "/foo/bar", to: "foo"
40554055
end
40564056
}
4057-
assert_match(/Missing :controller/, ex.message)
4057+
assert_match(/:to must respond_to/, ex.message)
4058+
end
4059+
4060+
def test_to_is_a_symbol
4061+
ex = assert_raises(ArgumentError) {
4062+
draw do
4063+
get "/foo/bar", to: :foo
4064+
end
4065+
}
4066+
assert_match(/:to must respond_to/, ex.message)
40584067
end
40594068

40604069
def test_missing_action_on_hash

0 commit comments

Comments
 (0)