From 13fd5586cef628a71e0e2900820010742a911099 Mon Sep 17 00:00:00 2001 From: Derek Prior Date: Tue, 15 Dec 2015 20:17:32 -0500 Subject: [PATCH] Add `redirect_back` for safer referrer redirects `redirect_to :back` is a somewhat common pattern in Rails apps, but it is not completely safe. There are a number of circumstances where HTTP referrer information is not available on the request. This happens often with bot traffic and occasionally to user traffic depending on browser security settings. When there is no referrer available on the request, `redirect_to :back` will raise `ActionController::RedirectBackError`, usually resulting in an application error. `redirect_back` takes a required `fallback_location` keyword argument that specifies the redirect when the referrer information is not available. This prevents 500 errors caused by `ActionController::RedirectBackError`. --- actionpack/CHANGELOG.md | 6 +++++ .../action_controller/metal/redirecting.rb | 23 +++++++++++++++++++ actionpack/test/controller/redirect_test.rb | 21 +++++++++++++++++ guides/source/action_controller_overview.md | 2 +- guides/source/layouts_and_rendering.md | 8 +++++++ 5 files changed, 59 insertions(+), 1 deletion(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 608512d846..bdc0fbe01a 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,9 @@ +* Add `redirect_back` method to `ActionController::Redirecting` to provide a + way to safely redirect to the `HTTP_REFERER` if it is present, falling back + to a provided redirect otherwise. + + *Derek Prior* + * `ActionController::TestCase` will be moved to it's own gem in Rails 5.1 With the speed improvements made to `ActionDispatch::IntegrationTest` we no diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 0febc905f1..72ceb9a5bc 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -75,6 +75,29 @@ module ActionController self.response_body = "You are being redirected." end + # Redirects the browser to the page that issued the request if possible, + # otherwise redirects to provided default fallback location. + # + # This avoids the ActionController::RedirectBackError that can + # occur if the request has no associated HTTP_REFERER. + # + # redirect_back fallback_location: { action: "show", id: 5 } + # redirect_back fallback_location: post + # redirect_back fallback_location: "http://www.rubyonrails.org" + # redirect_back fallback_location: "/images/screenshot.jpg" + # redirect_back fallback_location: articles_url + # redirect_back fallback_location: proc { edit_post_url(@post) } + # + # All options that can be passed to redirect_to are accepted as + # options and the behavior is indetical. + def redirect_back(fallback_location:, **args) + if referer = request.headers["Referer"] + redirect_to referer, **args + else + redirect_to fallback_location, **args + end + end + def _compute_redirect_to_location(request, options) #:nodoc: case options # The scheme name consist of a letter followed by any combination of diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 631ff7d02a..28ff1f36cf 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -42,6 +42,10 @@ class RedirectController < ActionController::Base redirect_to :back, :status => 307 end + def redirect_back_with_status + redirect_back(fallback_location: "/things/stuff", status: 307) + end + def host_redirect redirect_to :action => "other_host", :only_path => false, :host => 'other.test.host' end @@ -248,6 +252,23 @@ class RedirectTest < ActionController::TestCase } end + def test_redirect_back + referer = "http://www.example.com/coming/from" + @request.env["HTTP_REFERER"] = referer + + get :redirect_back_with_status + + assert_response 307 + assert_equal referer, redirect_to_url + end + + def test_redirect_back_with_no_referer + get :redirect_back_with_status + + assert_response 307 + assert_equal "http://test.host/things/stuff", redirect_to_url + end + def test_redirect_to_record with_routing do |set| set.draw do diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index 7e43ba375a..6c622a3643 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -1150,7 +1150,7 @@ class ApplicationController < ActionController::Base def user_not_authorized flash[:error] = "You don't have access to this section." - redirect_to :back + redirect_back(fallback_location: root_path) end end diff --git a/guides/source/layouts_and_rendering.md b/guides/source/layouts_and_rendering.md index 71cc030f6a..779ba6e5e5 100644 --- a/guides/source/layouts_and_rendering.md +++ b/guides/source/layouts_and_rendering.md @@ -628,6 +628,14 @@ You can use `redirect_to` with any arguments that you could use with `link_to` o redirect_to :back ``` +This will raise `ActionController::RedirectBackError` if the request had no +`HTTP_REFERER` information set. To guard against this case, you can provide a +fall back redirect URL by using `redirect_back`: + +```ruby +redirect_back(fallback_location: root_path) +``` + #### Getting a Different Redirect Status Code Rails uses HTTP status code 302, a temporary redirect, when you call `redirect_to`. If you'd like to use a different status code, perhaps 301, a permanent redirect, you can use the `:status` option: