Skip to content

Commit

Permalink
Fix redirect_to host verification error with non-ASCII querystring …
Browse files Browse the repository at this point in the history
…characters
  • Loading branch information
ghiculescu committed Oct 29, 2024
1 parent 1ecc84a commit 7cd43b9
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 6 deletions.
12 changes: 6 additions & 6 deletions actionpack/lib/action_controller/metal/redirecting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,10 @@ def redirect_to(options = {}, response_options = {})

self.status = _extract_redirect_to_status(options, response_options)

redirect_to_location = _compute_redirect_to_location(request, options)
redirect_to_location = _compute_redirect_to_location(request, options, allow_other_host: allow_other_host)
_ensure_url_is_http_header_safe(redirect_to_location)

self.location = _enforce_open_redirect_protection(redirect_to_location, allow_other_host: allow_other_host)
self.location = redirect_to_location
self.response_body = ""
end

Expand Down Expand Up @@ -155,21 +155,21 @@ def redirect_back_or_to(fallback_location, allow_other_host: _allow_other_host,
end
end

def _compute_redirect_to_location(request, options) # :nodoc:
def _compute_redirect_to_location(request, options, allow_other_host: _allow_other_host) # :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
# https://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.to_str
_enforce_open_redirect_protection(options.to_str, allow_other_host:)
when String
request.protocol + request.host_with_port + options
when Proc
_compute_redirect_to_location request, instance_eval(&options)
_compute_redirect_to_location request, instance_eval(&options), allow_other_host:
else
url_for(options)
_enforce_open_redirect_protection(url_for(options), allow_other_host:)
end.delete("\0\r\n")
end
module_function :_compute_redirect_to_location
Expand Down
20 changes: 20 additions & 0 deletions actionpack/test/controller/redirect_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ def redirect_to_url_with_unescaped_query_string
redirect_to "http://example.com/query?status=new"
end

def redirect_to_url_with_non_ascii_query_string
redirect_to "/query?name=fernández", allow_other_host: false
end

def redirect_to_url_with_complex_scheme
redirect_to "x-test+scheme.complex:redirect"
end
Expand Down Expand Up @@ -191,6 +195,10 @@ def redirect_to_with_block_and_options
redirect_to proc { { action: "hello_world" } }
end

def redirect_to_with_block_and_path
redirect_to proc { "/things/stuff" }, allow_other_host: false
end

def redirect_to_out_of_scope_block
redirect_to Workshop::OUT_OF_SCOPE_BLOCK
end
Expand Down Expand Up @@ -323,6 +331,12 @@ def test_redirect_to_url_with_unescaped_query_string
assert_redirected_to "http://example.com/query?status=new"
end

def test_redirect_to_url_with_non_ascii_query_string
get :redirect_to_url_with_non_ascii_query_string
assert_response :redirect
assert_redirected_to "http://test.host/query?name=fernández"
end

def test_redirect_to_url_with_complex_scheme
get :redirect_to_url_with_complex_scheme
assert_response :redirect
Expand Down Expand Up @@ -483,6 +497,12 @@ def test_redirect_to_with_block
assert_redirected_to "http://www.rubyonrails.org/"
end

def test_redirect_to_with_block_and_path
get :redirect_to_with_block_and_path
assert_response :redirect
assert_redirected_to "/things/stuff"
end

def test_redirect_to_with_block_and_assigns
get :redirect_to_with_block_and_assigns
assert_response :redirect
Expand Down

0 comments on commit 7cd43b9

Please sign in to comment.