From 5460591f0226a9d248b7b4f89186bd5553e7768f Mon Sep 17 00:00:00 2001 From: Andrew White Date: Sun, 20 Apr 2014 10:08:32 +0100 Subject: Make URL escaping more consistent 1. Escape '%' characters in URLs - only unescaped data should be passed to URL helpers 2. Add an `escape_segment` helper to `Router::Utils` that escapes '/' characters 3. Use `escape_segment` rather than `escape_fragment` in optimized URL generation 4. Use `escape_segment` rather than `escape_path` in URL generation For point 4 there are two exceptions. Firstly, when a route uses wildcard segments (e.g. *foo) then we use `escape_path` as the value may contain '/' characters. This means that wildcard routes can't be optimized. Secondly, if a `:controller` segment is used in the path then this uses `escape_path` as the controller may be namespaced. Fixes #14629, #14636 and #14070. --- actionpack/test/journey/router/utils_test.rb | 4 ++++ actionpack/test/journey/router_test.rb | 13 ++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) (limited to 'actionpack/test/journey') diff --git a/actionpack/test/journey/router/utils_test.rb b/actionpack/test/journey/router/utils_test.rb index 8b3a4e340a..584fd56a5c 100644 --- a/actionpack/test/journey/router/utils_test.rb +++ b/actionpack/test/journey/router/utils_test.rb @@ -8,6 +8,10 @@ module ActionDispatch assert_equal "a/b%20c+d%25", Utils.escape_path("a/b c+d%") end + def test_segment_escape + assert_equal "a%2Fb%20c+d%25", Utils.escape_segment("a/b c+d%") + end + def test_fragment_escape assert_equal "a/b%20c+d%25?e", Utils.escape_fragment("a/b c+d%?e") end diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index a286f77633..e54b64e0f3 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -367,7 +367,18 @@ module ActionDispatch nil, { :controller => "tasks", :action => "a/b c+d", }, {}) - assert_equal '/tasks/a/b%20c+d', path + assert_equal '/tasks/a%2Fb%20c+d', path + end + + def test_generate_escapes_with_namespaced_controller + path = Path::Pattern.new '/:controller(/:action)' + @router.routes.add_route @app, path, {}, {}, {} + + path, _ = @formatter.generate(:path_info, + nil, { :controller => "admin/tasks", + :action => "a/b c+d", + }, {}) + assert_equal '/admin/tasks/a%2Fb%20c+d', path end def test_generate_extra_params -- cgit v1.2.3