Allow engine root relative redirects using an empty string.

Example: # application routes.rb mount BlogEngine => '/blog' # engine routes.rb get '/welcome' => redirect('') This now redirects to the path `/blog`, whereas before it would redirect to the application root path. In the case of a path redirect or a custom redirect if the path returned contains a host then the path is treated as absolute. Similarly for option redirects, if the options hash returned contains a `:host` or `:domain` key then the path is treated as absolute. Fixes #7977
author: Andrew White <andyw@pixeltrix.co.uk> 2014-01-01 11:15:31 +0000
committer: Andrew White <andyw@pixeltrix.co.uk> 2014-01-01 11:19:14 +0000
commit: b64bac489c58ce1169941f77f4d633cb4eb87cd8 (patch)
tree: 2e90df47cd144978e039690d391bdae9faa72ccf /actionpack
parent: 0d4614ca21d147ae3fac915c7a637d7520ff56e3 (diff)
download: rails-b64bac489c58ce1169941f77f4d633cb4eb87cd8.tar.gz
rails-b64bac489c58ce1169941f77f4d633cb4eb87cd8.tar.bz2
rails-b64bac489c58ce1169941f77f4d633cb4eb87cd8.zip
3 files changed, 149 insertions, 49 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 24f36e2496..922fa949a9 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,23 @@
+*   Allow engine root relative redirects using an empty string.
+
+    Example:
+
+        # application routes.rb
+        mount BlogEngine => '/blog'
+
+        # engine routes.rb
+        get '/welcome' => redirect('')
+
+    This now redirects to the path `/blog`, whereas before it would redirect
+    to the application root path. In the case of a path redirect or a custom
+    redirect if the path returned contains a host then the path is treated as
+    absolute. Similarly for option redirects, if the options hash returned
+    contains a `:host` or `:domain` key then the path is treated as absolute.
+
+    Fixes #7977
+
+    *Andrew White*
+
 *   Fix `Encoding::CompatibilityError` when public path is UTF-8
 
     In #5337 we forced the path encoding to ASCII-8BIT to prevent static file handling
diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb
index cbf4c5aa8b..b08e62543b 100644
--- a/actionpack/lib/action_dispatch/routing/redirection.rb
+++ b/actionpack/lib/action_dispatch/routing/redirection.rb
@@ -26,14 +26,19 @@ module ActionDispatch
         end
 
         uri = URI.parse(path(req.symbolized_path_parameters, req))
+        
+        unless uri.host
+          if relative_path?(uri.path)
+            uri.path = "#{req.script_name}/#{uri.path}"
+          elsif uri.path.empty?
+            uri.path = req.script_name.empty? ? "/" : req.script_name
+          end
+        end
+          
         uri.scheme ||= req.scheme
         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 = {
@@ -112,11 +117,16 @@ 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
+        unless options[:host] || options[:domain]
+          if relative_path?(url_options[:path])
+            url_options[:path] = "/#{url_options[:path]}"
+            url_options[:script_name] = request.script_name
+          elsif url_options[:path].empty?
+            url_options[:path] = request.script_name.empty? ? "/" : ""
+            url_options[:script_name] = request.script_name
+          end
         end
-
+        
         ActionDispatch::Http::URL.url_for url_options
       end
 
diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb
index e519fff51e..08501d19c0 100644
--- a/actionpack/test/dispatch/prefix_generation_test.rb
+++ b/actionpack/test/dispatch/prefix_generation_test.rb
@@ -32,12 +32,18 @@ module TestGenerationPrefix
             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_path_root",       :to => redirect("")
+            get "/relative_path_redirect",   :to => redirect("foo")
+            get "/relative_option_root",     :to => redirect(:path => "")
             get "/relative_option_redirect", :to => redirect(:path => "foo")
+            get "/relative_custom_root",     :to => redirect { |params, request| "" }
             get "/relative_custom_redirect", :to => redirect { |params, request| "foo" }
 
-            get "/absolute_path_redirect", :to => redirect("/foo")
+            get "/absolute_path_root",       :to => redirect("/")
+            get "/absolute_path_redirect",   :to => redirect("/foo")
+            get "/absolute_option_root",     :to => redirect(:path => "/")
             get "/absolute_option_redirect", :to => redirect(:path => "/foo")
+            get "/absolute_custom_root",     :to => redirect { |params, request| "/" }
             get "/absolute_custom_redirect", :to => redirect { |params, request| "/foo" }
           end
 
@@ -190,46 +196,64 @@ module TestGenerationPrefix
       assert_equal "engine", last_response.body
     end
 
+    test "[ENGINE] relative path root uses SCRIPT_NAME from request" do
+      get "/awesome/blog/relative_path_root"
+      verify_redirect "http://example.org/awesome/blog"
+    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
+      verify_redirect "http://example.org/awesome/blog/foo"
+    end
+
+    test "[ENGINE] relative option root uses SCRIPT_NAME from request" do
+      get "/awesome/blog/relative_option_root"
+      verify_redirect "http://example.org/awesome/blog"
     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
+      verify_redirect "http://example.org/awesome/blog/foo"
+    end
+
+    test "[ENGINE] relative custom root uses SCRIPT_NAME from request" do
+      get "/awesome/blog/relative_custom_root"
+      verify_redirect "http://example.org/awesome/blog"
     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
+      verify_redirect "http://example.org/awesome/blog/foo"
+    end
+
+    test "[ENGINE] absolute path root doesn't use SCRIPT_NAME from request" do
+      get "/awesome/blog/absolute_path_root"
+      verify_redirect "http://example.org/"
     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
+      verify_redirect "http://example.org/foo"
+    end
+
+    test "[ENGINE] absolute option root doesn't use SCRIPT_NAME from request" do
+      get "/awesome/blog/absolute_option_root"
+      verify_redirect "http://example.org/"
     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
+      verify_redirect "http://example.org/foo"
+    end
+
+    test "[ENGINE] absolute custom root doesn't use SCRIPT_NAME from request" do
+      get "/awesome/blog/absolute_custom_root"
+      verify_redirect "http://example.org/"
     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
+      verify_redirect "http://example.org/foo"
     end
 
     # Inside Application
@@ -320,6 +344,17 @@ module TestGenerationPrefix
       path = engine_object.polymorphic_url(Post.new, :host => "www.example.com")
       assert_equal "http://www.example.com/awesome/blog/posts/1", path
     end
+    
+    private
+      def verify_redirect(url, status = 301)
+        assert_equal status, last_response.status
+        assert_equal url, last_response.headers["Location"]
+        assert_equal expected_redirect_body(url), last_response.body
+      end
+      
+      def expected_redirect_body(url)
+        %(<html><body>You are being <a href="#{url}">redirected</a>.</body></html>)
+      end
   end
 
   class EngineMountedAtRoot < ActionDispatch::IntegrationTest
@@ -332,12 +367,18 @@ module TestGenerationPrefix
           routes.draw do
             get "/posts/:id", :to => "posts#show", :as => :post
 
-            get "/relative_path_redirect", :to => redirect("foo")
+            get "/relative_path_root",       :to => redirect("")
+            get "/relative_path_redirect",   :to => redirect("foo")
+            get "/relative_option_root",     :to => redirect(:path => "")
             get "/relative_option_redirect", :to => redirect(:path => "foo")
+            get "/relative_custom_root",     :to => redirect { |params, request| "" }
             get "/relative_custom_redirect", :to => redirect { |params, request| "foo" }
 
-            get "/absolute_path_redirect", :to => redirect("/foo")
+            get "/absolute_path_root",       :to => redirect("/")
+            get "/absolute_path_redirect",   :to => redirect("/foo")
+            get "/absolute_option_root",     :to => redirect(:path => "/")
             get "/absolute_option_redirect", :to => redirect(:path => "/foo")
+            get "/absolute_custom_root",     :to => redirect { |params, request| "/" }
             get "/absolute_custom_redirect", :to => redirect { |params, request| "/foo" }
           end
 
@@ -390,46 +431,75 @@ module TestGenerationPrefix
       assert_equal "/posts/1", last_response.body
     end
 
+    test "[ENGINE] relative path root uses SCRIPT_NAME from request" do
+      get "/relative_path_root"
+      verify_redirect "http://example.org/"
+    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
+      verify_redirect "http://example.org/foo"
+    end
+
+    test "[ENGINE] relative option root uses SCRIPT_NAME from request" do
+      get "/relative_option_root"
+      verify_redirect "http://example.org/"
     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
+      verify_redirect "http://example.org/foo"
+    end
+
+    test "[ENGINE] relative custom root uses SCRIPT_NAME from request" do
+      get "/relative_custom_root"
+      verify_redirect "http://example.org/"
     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
+      verify_redirect "http://example.org/foo"
+    end
+
+    test "[ENGINE] absolute path root doesn't use SCRIPT_NAME from request" do
+      get "/absolute_path_root"
+      verify_redirect "http://example.org/"
     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
+      verify_redirect "http://example.org/foo"
+    end
+
+    test "[ENGINE] absolute option root doesn't use SCRIPT_NAME from request" do
+      get "/absolute_option_root"
+      verify_redirect "http://example.org/"
     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
+      verify_redirect "http://example.org/foo"
+    end
+
+    test "[ENGINE] absolute custom root doesn't use SCRIPT_NAME from request" do
+      get "/absolute_custom_root"
+      verify_redirect "http://example.org/"
     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
+      verify_redirect "http://example.org/foo"
+    end
+    
+    private
+      def verify_redirect(url, status = 301)
+        assert_equal status, last_response.status
+        assert_equal url, last_response.headers["Location"]
+        assert_equal expected_redirect_body(url), last_response.body
+      end
+      
+      def expected_redirect_body(url)
+        %(<html><body>You are being <a href="#{url}">redirected</a>.</body></html>)
+      end
   end
 end
author	Andrew White <andyw@pixeltrix.co.uk>	2014-01-01 11:15:31 +0000
committer	Andrew White <andyw@pixeltrix.co.uk>	2014-01-01 11:19:14 +0000
commit	b64bac489c58ce1169941f77f4d633cb4eb87cd8 (patch)
tree	2e90df47cd144978e039690d391bdae9faa72ccf /actionpack
parent	0d4614ca21d147ae3fac915c7a637d7520ff56e3 (diff)
download	rails-b64bac489c58ce1169941f77f4d633cb4eb87cd8.tar.gz rails-b64bac489c58ce1169941f77f4d633cb4eb87cd8.tar.bz2 rails-b64bac489c58ce1169941f77f4d633cb4eb87cd8.zip