Skip to content

Commit 9a89f7b

Browse files
committed
Allow rescuing ActionController::Redirecting::UnsafeRedirectError in controllers
Consider a controller that does this: ```ruby begin redirect_to "http://www.rubyonrails.org/", allow_other_host: false rescue ActionController::Redirecting::UnsafeRedirectError render plain: "caught error" end ``` The `redirect_to` will raise and the `rescue` will execute. But currently, the response status will still be changed (to 302). So even if you render something, we will return to the browser a 302 response code, with no response location. This is not a valid response. This PR fixes this, by only setting the status once the location has been verified. Note: I came across this issue while trying to work around rails#53464, but it's not dependent on that issue.
1 parent e5cd491 commit 9a89f7b

File tree

2 files changed

+15
-1
lines changed

2 files changed

+15
-1
lines changed

actionpack/lib/action_controller/metal/redirecting.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,14 @@ def redirect_to(options = {}, response_options = {})
106106

107107
allow_other_host = response_options.delete(:allow_other_host) { _allow_other_host }
108108

109-
self.status = _extract_redirect_to_status(options, response_options)
109+
proposed_status = _extract_redirect_to_status(options, response_options)
110110

111111
redirect_to_location = _compute_redirect_to_location(request, options)
112112
_ensure_url_is_http_header_safe(redirect_to_location)
113113

114114
self.location = _enforce_open_redirect_protection(redirect_to_location, allow_other_host: allow_other_host)
115115
self.response_body = ""
116+
self.status = proposed_status
116117
end
117118

118119
# Soft deprecated alias for #redirect_back_or_to where the `fallback_location`

actionpack/test/controller/redirect_test.rb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,14 @@ def redirect_with_null_bytes
203203
redirect_to "\000/lol\r\nwat"
204204
end
205205

206+
def redirect_to_external_with_rescue
207+
begin
208+
redirect_to "http://www.rubyonrails.org/", allow_other_host: false
209+
rescue ActionController::Redirecting::UnsafeRedirectError
210+
render plain: "caught error"
211+
end
212+
end
213+
206214
def rescue_errors(e) raise e end
207215

208216
private
@@ -617,6 +625,11 @@ def test_redirect_to_instrumentation
617625
assert_equal "http://test.host/redirect/hello_world", payload[:location]
618626
end
619627

628+
def test_redirect_to_external_with_rescue
629+
get :redirect_to_external_with_rescue
630+
assert_response :ok
631+
end
632+
620633
private
621634
def with_raise_on_open_redirects
622635
old_raise_on_open_redirects = ActionController::Base.raise_on_open_redirects

0 commit comments

Comments
 (0)