Skip to content

Commit

Permalink
Allow engine root relative redirects using an empty string.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
pixeltrix committed Jan 1, 2014
1 parent 0d4614c commit b64bac4
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 49 deletions.
20 changes: 20 additions & 0 deletions 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
Expand Down
26 changes: 18 additions & 8 deletions actionpack/lib/action_dispatch/routing/redirection.rb
Expand Up @@ -26,14 +26,19 @@ def call(env)
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 = {
Expand Down Expand Up @@ -112,11 +117,16 @@ def path(params, request)
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

Expand Down
152 changes: 111 additions & 41 deletions actionpack/test/dispatch/prefix_generation_test.rb
Expand Up @@ -32,12 +32,18 @@ def self.routes
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

Expand Down Expand Up @@ -190,46 +196,64 @@ def setup
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
Expand Down Expand Up @@ -320,6 +344,17 @@ def setup
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
Expand All @@ -332,12 +367,18 @@ def self.routes
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

Expand Down Expand Up @@ -390,46 +431,75 @@ def app
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

0 comments on commit b64bac4

Please sign in to comment.