mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Fix incorrect assert_redirected_to failure message
In some instances, `assert_redirected_to` assertion was returning an incorrect and misleading failure message when the assertion failed. This was due to a disconnect in how the assertion computes the redirect string for the failure message and how `redirect_to` computes the string that is actually used for redirection. I made the `_compute_redirect_to_loaction` method used by `redirect_to` public and call that from the method `assert_redirect_to` uses to calculate the URL. The reveals a new test failure due to the regex used by `_compute_redirect_to_location` allow `_` in the URL scheme.
This commit is contained in:
parent
fb2785ec3d
commit
1dacfbabf3
4 changed files with 48 additions and 34 deletions
|
@ -1,3 +1,8 @@
|
||||||
|
* 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.
|
* Fix an issue where router can't recognize downcased url encoding path.
|
||||||
|
|
||||||
Fixes #12269
|
Fixes #12269
|
||||||
|
|
|
@ -71,6 +71,26 @@ module ActionController
|
||||||
self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.h(location)}\">redirected</a>.</body></html>"
|
self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.h(location)}\">redirected</a>.</body></html>"
|
||||||
end
|
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 %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
|
||||||
|
|
||||||
private
|
private
|
||||||
def _extract_redirect_to_status(options, response_status)
|
def _extract_redirect_to_status(options, response_status)
|
||||||
if options.is_a?(Hash) && options.key?(:status)
|
if options.is_a?(Hash) && options.key?(:status)
|
||||||
|
@ -81,24 +101,5 @@ module ActionController
|
||||||
302
|
302
|
||||||
end
|
end
|
||||||
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
|
||||||
end
|
end
|
||||||
|
|
|
@ -67,21 +67,11 @@ module ActionDispatch
|
||||||
end
|
end
|
||||||
|
|
||||||
def normalize_argument_to_redirection(fragment)
|
def normalize_argument_to_redirection(fragment)
|
||||||
normalized = case fragment
|
if Regexp === fragment
|
||||||
when Regexp
|
fragment
|
||||||
fragment
|
else
|
||||||
when %r{^\w[A-Za-z\d+.-]*:.*}
|
@controller._compute_redirect_to_location(fragment)
|
||||||
fragment
|
end
|
||||||
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
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -39,6 +39,8 @@ class ActionPackAssertionsController < ActionController::Base
|
||||||
|
|
||||||
def redirect_external() redirect_to "http://www.rubyonrails.org"; end
|
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 response404() head '404 AWOL' end
|
||||||
|
|
||||||
def response500() head '500 Sorry' end
|
def response500() head '500 Sorry' end
|
||||||
|
@ -258,6 +260,19 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase
|
||||||
end
|
end
|
||||||
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
|
def test_template_objects_exist
|
||||||
process :assign_this
|
process :assign_this
|
||||||
assert !@controller.instance_variable_defined?(:"@hi")
|
assert !@controller.instance_variable_defined?(:"@hi")
|
||||||
|
@ -309,6 +324,9 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase
|
||||||
|
|
||||||
process :redirect_external
|
process :redirect_external
|
||||||
assert_equal 'http://www.rubyonrails.org', @response.redirect_url
|
assert_equal 'http://www.rubyonrails.org', @response.redirect_url
|
||||||
|
|
||||||
|
process :redirect_external_protocol_relative
|
||||||
|
assert_equal '//www.rubyonrails.org', @response.redirect_url
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_no_redirect_url
|
def test_no_redirect_url
|
||||||
|
|
Loading…
Reference in a new issue