Respect SCRIPT_NAME when using redirect with a relative path

Example: # application routes.rb mount BlogEngine => '/blog' # engine routes.rb get '/admin' => redirect('admin/dashboard') This now redirects to the path `/blog/admin/dashboard`, whereas before it would've generated an invalid url because there would be no slash between the host name and the path. It also allows redirects to work where the application is deployed to a subdirectory of a website. Fixes #7977
2022-11-09 12:12:34 -05:00 · 2013-10-10 13:01:03 +01:00 · 2013-10-10 13:01:03 +01:00 · 9dbd208562
commit 9dbd208562
parent 0061c5e1ef
3 changed files with 136 additions and 0 deletions
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@ -1,3 +1,21 @@
+*   Respect `SCRIPT_NAME` when using `redirect` with a relative path
+
+    Example:
+        # application routes.rb
+        mount BlogEngine => '/blog'
+
+        # engine routes.rb
+        get '/admin' => redirect('admin/dashboard')
+
+    This now redirects to the path `/blog/admin/dashboard`, whereas before it would've
+    generated an invalid url because there would be no slash between the host name and
+    the path. It also allows redirects to work where the application is deployed to a
+    subdirectory of a website.
+
+    Fixes #7977
+
+    *Andrew White*
+
 *   Fixing repond_with working directly on the options hash
    This fixes an issue where the respond_with worked directly with the given
    options hash, so that if a user relied on it after calling respond_with,
--- a/actionpack/lib/action_dispatch/routing/redirection.rb
+++ b/actionpack/lib/action_dispatch/routing/redirection.rb
@ -30,6 +30,10 @@ module ActionDispatch
        uri.host   ||= req.host
        uri.port   ||= req.port unless req.standard_port?

+        if relative_path?(uri.path)
+          uri.path = "#{req.script_name}/#{uri.path}"
+        end
+
        body = %(<html><body>You are being <a href="#{ERB::Util.h(uri.to_s)}">redirected</a>.</body></html>)

        headers = {
@ -48,6 +52,11 @@ module ActionDispatch
      def inspect
        "redirect(#{status})"
      end
+
+      private
+        def relative_path?(path)
+          path && !path.empty? && path[0] != '/'
+        end
    end

    class PathRedirect < Redirect
@ -81,6 +90,11 @@ module ActionDispatch
          url_options[:path] = (url_options[:path] % escape_path(params))
        end

+        if relative_path?(url_options[:path])
+          url_options[:path] = "/#{url_options[:path]}"
+          url_options[:script_name] = request.script_name
+        end
+
        ActionDispatch::Http::URL.url_for url_options
      end

@ -104,6 +118,10 @@ module ActionDispatch
      #
      #   get 'docs/:article', to: redirect('/wiki/%{article}')
      #
+      # Note that if you return a path without a leading slash then the url is prefixed with the
+      # current SCRIPT_NAME environment variable. This is typically '/' but may be different in
+      # a mounted engine or where the application is deployed to a subdirectory of a website.
+      #
      # Alternatively you can use one of the other syntaxes:
      #
      # The block version of redirect allows for the easy encapsulation of any logic associated with
--- a/actionpack/test/dispatch/prefix_generation_test.rb
+++ b/actionpack/test/dispatch/prefix_generation_test.rb
@ -31,6 +31,14 @@ module TestGenerationPrefix
            get "/polymorphic_path_for_engine", :to => "inside_engine_generating#polymorphic_path_for_engine"
            get "/conflicting_url", :to => "inside_engine_generating#conflicting"
            get "/foo", :to => "never#invoked", :as => :named_helper_that_should_be_invoked_only_in_respond_to_test
+
+            get "/relative_path_redirect", :to => redirect("foo")
+            get "/relative_option_redirect", :to => redirect(:path => "foo")
+            get "/relative_custom_redirect", :to => redirect { |params, request| "foo" }
+
+            get "/absolute_path_redirect", :to => redirect("/foo")
+            get "/absolute_option_redirect", :to => redirect(:path => "/foo")
+            get "/absolute_custom_redirect", :to => redirect { |params, request| "/foo" }
          end

          routes
@ -182,6 +190,48 @@ module TestGenerationPrefix
      assert_equal "engine", last_response.body
    end

+    test "[ENGINE] relative path redirect uses SCRIPT_NAME from request" do
+      get "/awesome/blog/relative_path_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/awesome/blog/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/awesome/blog/foo">redirected</a>.</body></html>), last_response.body
+    end
+
+    test "[ENGINE] relative option redirect uses SCRIPT_NAME from request" do
+      get "/awesome/blog/relative_option_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/awesome/blog/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/awesome/blog/foo">redirected</a>.</body></html>), last_response.body
+    end
+
+    test "[ENGINE] relative custom redirect uses SCRIPT_NAME from request" do
+      get "/awesome/blog/relative_custom_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/awesome/blog/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/awesome/blog/foo">redirected</a>.</body></html>), last_response.body
+    end
+
+    test "[ENGINE] absolute path redirect doesn't use SCRIPT_NAME from request" do
+      get "/awesome/blog/absolute_path_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+    end
+
+    test "[ENGINE] absolute option redirect doesn't use SCRIPT_NAME from request" do
+      get "/awesome/blog/absolute_option_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+    end
+
+    test "[ENGINE] absolute custom redirect doesn't use SCRIPT_NAME from request" do
+      get "/awesome/blog/absolute_custom_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+    end
+
    # Inside Application
    test "[APP] generating engine's route includes prefix" do
      get "/generate"
@ -281,6 +331,14 @@ module TestGenerationPrefix
          routes = ActionDispatch::Routing::RouteSet.new
          routes.draw do
            get "/posts/:id", :to => "posts#show", :as => :post
+
+            get "/relative_path_redirect", :to => redirect("foo")
+            get "/relative_option_redirect", :to => redirect(:path => "foo")
+            get "/relative_custom_redirect", :to => redirect { |params, request| "foo" }
+
+            get "/absolute_path_redirect", :to => redirect("/foo")
+            get "/absolute_option_redirect", :to => redirect(:path => "/foo")
+            get "/absolute_custom_redirect", :to => redirect { |params, request| "/foo" }
          end

          routes
@ -331,5 +389,47 @@ module TestGenerationPrefix
      get "/posts/1"
      assert_equal "/posts/1", last_response.body
    end
+
+    test "[ENGINE] relative path redirect uses SCRIPT_NAME from request" do
+      get "/relative_path_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+    end
+
+    test "[ENGINE] relative option redirect uses SCRIPT_NAME from request" do
+      get "/relative_option_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+    end
+
+    test "[ENGINE] relative custom redirect uses SCRIPT_NAME from request" do
+      get "/relative_custom_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+    end
+
+    test "[ENGINE] absolute path redirect doesn't use SCRIPT_NAME from request" do
+      get "/absolute_path_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+    end
+
+    test "[ENGINE] absolute option redirect doesn't use SCRIPT_NAME from request" do
+      get "/absolute_option_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+    end
+
+    test "[ENGINE] absolute custom redirect doesn't use SCRIPT_NAME from request" do
+      get "/absolute_custom_redirect"
+      assert_equal 301, last_response.status
+      assert_equal "http://example.org/foo", last_response.headers["Location"]
+      assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+    end
  end
 end