diff options
| author | Andrew White <andyw@pixeltrix.co.uk> | 2013-09-19 06:33:15 -0700 |
|---|---|---|
| committer | Andrew White <andyw@pixeltrix.co.uk> | 2013-09-19 06:33:15 -0700 |
| commit | acd83a7e20f9d477dd738f515866c46edf01b542 (patch) | |
| tree | 2b69e77ffe8091859cb81d4448f7f22d86fcd7df | |
| parent | fb2785ec3d97f0e3578e181012f18eda920dca10 (diff) | |
| parent | a78c10d3c787c56106353eb025ebb93ffcdb7bac (diff) | |
| download | rails-acd83a7e20f9d477dd738f515866c46edf01b542.tar.gz rails-acd83a7e20f9d477dd738f515866c46edf01b542.tar.bz2 rails-acd83a7e20f9d477dd738f515866c46edf01b542.zip | |
Merge pull request #10362 from derekprior/dp-fix-assert-redirect-to
Fix incorrect assert_redirected_to failure message
| -rw-r--r-- | actionpack/CHANGELOG.md | 10 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/redirecting.rb | 39 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/testing/assertions/response.rb | 20 | ||||
| -rw-r--r-- | actionpack/test/controller/action_pack_assertions_test.rb | 18 |
4 files changed, 53 insertions, 34 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 63a208dbcf..b0b75f6909 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,13 @@ +* Fix regex used to detect URI schemes in `redirect_to` to be consistent with + RFC 3986. + + *Derek Prior* + +* Fix incorrect `assert_redirected_to` failure message for protocol-relative + URLs. + + *Derek Prior* + * Fix an issue where router can't recognize downcased url encoding path. Fixes #12269 diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index e9031f3fac..ab14a61b97 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -71,6 +71,26 @@ module ActionController self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.h(location)}\">redirected</a>.</body></html>" end + def _compute_redirect_to_location(options) #:nodoc: + case options + # The scheme name consist of a letter followed by any combination of + # letters, digits, and the plus ("+"), period ("."), or hyphen ("-") + # characters; and is terminated by a colon (":"). + # See http://tools.ietf.org/html/rfc3986#section-3.1 + # The protocol relative scheme starts with a double slash "//". + when /\A([a-z][a-z\d\-+\.]*:|\/\/).*/i + options + when String + request.protocol + request.host_with_port + options + when :back + request.headers["Referer"] or raise RedirectBackError + when Proc + _compute_redirect_to_location options.call + else + url_for(options) + end.delete("\0\r\n") + end + private def _extract_redirect_to_status(options, response_status) if options.is_a?(Hash) && options.key?(:status) @@ -81,24 +101,5 @@ module ActionController 302 end end - - def _compute_redirect_to_location(options) - case options - # The scheme name consist of a letter followed by any combination of - # letters, digits, and the plus ("+"), period ("."), or hyphen ("-") - # characters; and is terminated by a colon (":"). - # The protocol relative scheme starts with a double slash "//" - when %r{\A(\w[\w+.-]*:|//).*} - options - when String - request.protocol + request.host_with_port + options - when :back - request.headers["Referer"] or raise RedirectBackError - when Proc - _compute_redirect_to_location options.call - else - url_for(options) - end.delete("\0\r\n") - end end end diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 44ed0ac1f3..93f9fab9c2 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -67,21 +67,11 @@ module ActionDispatch end def normalize_argument_to_redirection(fragment) - normalized = case fragment - when Regexp - fragment - when %r{^\w[A-Za-z\d+.-]*:.*} - fragment - when String - @request.protocol + @request.host_with_port + fragment - when :back - raise RedirectBackError unless refer = @request.headers["Referer"] - refer - else - @controller.url_for(fragment) - end - - normalized.respond_to?(:delete) ? normalized.delete("\0\r\n") : normalized + if Regexp === fragment + fragment + else + @controller._compute_redirect_to_location(fragment) + end end end end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 22a410db94..ba4cffcd3e 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -39,6 +39,8 @@ class ActionPackAssertionsController < ActionController::Base def redirect_external() redirect_to "http://www.rubyonrails.org"; end + def redirect_external_protocol_relative() redirect_to "//www.rubyonrails.org"; end + def response404() head '404 AWOL' end def response500() head '500 Sorry' end @@ -258,6 +260,19 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase end end + def test_assert_redirect_failure_message_with_protocol_relative_url + begin + process :redirect_external_protocol_relative + assert_redirected_to "/foo" + rescue ActiveSupport::TestCase::Assertion => ex + assert_no_match( + /#{request.protocol}#{request.host}\/\/www.rubyonrails.org/, + ex.message, + 'protocol relative url was incorrectly normalized' + ) + end + end + def test_template_objects_exist process :assign_this assert !@controller.instance_variable_defined?(:"@hi") @@ -309,6 +324,9 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase process :redirect_external assert_equal 'http://www.rubyonrails.org', @response.redirect_url + + process :redirect_external_protocol_relative + assert_equal '//www.rubyonrails.org', @response.redirect_url end def test_no_redirect_url |