diff options
-rw-r--r-- | actionpack/CHANGELOG.md | 20 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/redirection.rb | 26 | ||||
-rw-r--r-- | actionpack/test/dispatch/prefix_generation_test.rb | 152 |
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 |