From 5e0a05b8cb236d285ebb45de006dd3600c69357d Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Tue, 2 Mar 2010 18:57:02 -0800 Subject: [PATCH] Tweak the semantic of various URL related methods of ActionDispatch::Request --- actionpack/lib/action_controller/test_case.rb | 22 +++-- actionpack/lib/action_dispatch/http/url.rb | 31 ++----- .../lib/action_view/helpers/url_helper.rb | 5 +- actionpack/lib/action_view/test_case.rb | 2 + .../http_digest_authentication_test.rb | 10 ++- .../test/controller/integration_test.rb | 4 +- actionpack/test/dispatch/request_test.rb | 86 ++----------------- actionpack/test/template/url_helper_test.rb | 3 - 8 files changed, 41 insertions(+), 122 deletions(-) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 64d9bdab2a8b3..7e0a833dfa6ef 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -322,6 +322,8 @@ def setup_controller_request_and_response @controller ||= klass.new rescue nil end + @request.env.delete('PATH_INFO') + if @controller @controller.request = @request @controller.params = {} @@ -333,15 +335,19 @@ def rescue_action_in_public! @request.remote_addr = '208.77.188.166' # example.com end - private - def build_request_uri(action, parameters) - unless @request.env['REQUEST_URI'] - options = @controller.__send__(:url_options).merge(parameters) - options.update(:only_path => true, :action => action) + private + def build_request_uri(action, parameters) + unless @request.env["PATH_INFO"] + options = @controller.__send__(:url_options).merge(parameters) + options.update(:only_path => true, :action => action, :relative_url_root => nil) + rewriter = ActionController::UrlRewriter.new(@request, parameters) - url = ActionController::UrlRewriter.new(@request, parameters) - @request.request_uri = url.rewrite(@router, options) - end + url, query_string = rewriter.rewrite(@router, options).split("?", 2) + + @request.env["SCRIPT_NAME"] = @controller.config.relative_url_root + @request.env["PATH_INFO"] = url + @request.env["QUERY_STRING"] = query_string || "" end + end end end diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 42ad19044d417..e67d970a2399f 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -85,42 +85,23 @@ def subdomain(tld_length = 1) subdomains(tld_length).join('.') end - # Returns the query string, accounting for server idiosyncrasies. + # The query string must always be present def query_string - @env['QUERY_STRING'].present? ? @env['QUERY_STRING'] : (@env['REQUEST_URI'].to_s.split('?', 2)[1] || '') + @env['QUERY_STRING'] end # Returns the request URI, accounting for server idiosyncrasies. # WEBrick includes the full URL. IIS leaves REQUEST_URI blank. def request_uri - if uri = @env['REQUEST_URI'] - # Remove domain, which webrick puts into the request_uri. - (%r{^\w+\://[^/]+(/.*|$)$} =~ uri) ? $1 : uri - else - # Construct IIS missing REQUEST_URI from SCRIPT_NAME and PATH_INFO. - uri = @env['PATH_INFO'].to_s - - if script_filename = @env['SCRIPT_NAME'].to_s.match(%r{[^/]+$}) - uri = uri.sub(/#{script_filename}\//, '') - end - - env_qs = @env['QUERY_STRING'].to_s - uri += "?#{env_qs}" unless env_qs.empty? - - if uri.blank? - @env.delete('REQUEST_URI') - else - @env['REQUEST_URI'] = uri - end - end + uri = "#{@env["SCRIPT_NAME"]}#{@env["PATH_INFO"]}" + uri << "?#{@env["QUERY_STRING"]}" if @env["QUERY_STRING"].present? + uri end # Returns the interpreted \path to requested resource after all the installation # directory of this application was taken into account. def path - path = request_uri.to_s[/\A[^\?]*/] - path.sub!(/\A#{ActionController::Base.relative_url_root}/, '') - path + @env['PATH_INFO'] end private diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index d9607c0095783..148f2868e9f6e 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -544,10 +544,11 @@ def current_page?(options) # submitted url doesn't have any either. This lets the function # work with things like ?order=asc if url_string.index("?") - request_uri = request.request_uri + request_uri = request.fullpath else - request_uri = request.request_uri.split('?').first + request_uri = request.path end + if url_string =~ /^\w+:\/\// url_string == "#{request.protocol}#{request.host_with_port}#{request_uri}" else diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index f639700eaf922..12142a372839c 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -34,6 +34,8 @@ def initialize @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new + @request.env.delete('PATH_INFO') + @params = {} end end diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index 22d2e23de100d..6f167fe627d7b 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -139,7 +139,7 @@ def authenticate_with_request test "authentication request with request-uri that doesn't match credentials digest-uri" do @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please') - @request.env['REQUEST_URI'] = "/http_digest_authentication_test/dummy_digest/altered/uri" + @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest/altered/uri" get :display assert_response :unauthorized @@ -148,7 +148,8 @@ def authenticate_with_request test "authentication request with absolute request uri (as in webrick)" do @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please') - @request.env['REQUEST_URI'] = "http://test.host/http_digest_authentication_test/dummy_digest" + @request.env["SERVER_NAME"] = "test.host" + @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest" get :display @@ -171,7 +172,8 @@ def authenticate_with_request test "authentication request with absolute uri in both request and credentials (as in Webrick with IE)" do @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:url => "http://test.host/http_digest_authentication_test/dummy_digest", :username => 'pretty', :password => 'please') - @request.env['REQUEST_URI'] = "http://test.host/http_digest_authentication_test/dummy_digest" + @request.env['SERVER_NAME'] = "test.host" + @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest" get :display @@ -226,7 +228,7 @@ def encode_credentials(options) credentials = decode_credentials(@response.headers['WWW-Authenticate']) credentials.merge!(options) - credentials.merge!(:uri => @request.env['REQUEST_URI'].to_s) + credentials.merge!(:uri => @request.env['PATH_INFO'].to_s) ActionController::HttpAuthentication::Digest.encode_credentials(method, credentials, password, options[:password_is_ha1]) end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index c38348fa68a07..814699978db70 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -346,8 +346,8 @@ def test_get_with_query_string def test_get_with_parameters with_test_route_set do get '/get_with_params', :foo => "bar" - assert_equal '/get_with_params', request.env["REQUEST_URI"] - assert_equal '/get_with_params', request.request_uri + assert_equal '/get_with_params', request.env["PATH_INFO"] + assert_equal '/get_with_params', request.path_info assert_equal 'foo=bar', request.env["QUERY_STRING"] assert_equal 'foo=bar', request.query_string assert_equal 'bar', request.parameters['foo'] diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 703f03fa96dbb..1dfcdaebfdbe9 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -144,103 +144,33 @@ class RequestTest < ActiveSupport::TestCase end test "request uri" do - request = stub_request 'REQUEST_URI' => "http://www.rubyonrails.org/path/of/some/uri?mapped=1" + request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/path/of/some/uri', 'QUERY_STRING' => 'mapped=1' assert_equal "/path/of/some/uri?mapped=1", request.request_uri assert_equal "/path/of/some/uri", request.path - request = stub_request 'REQUEST_URI' => "http://www.rubyonrails.org/path/of/some/uri" + request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/path/of/some/uri' assert_equal "/path/of/some/uri", request.request_uri assert_equal "/path/of/some/uri", request.path - request = stub_request 'REQUEST_URI' => "/path/of/some/uri" - assert_equal "/path/of/some/uri", request.request_uri - assert_equal "/path/of/some/uri", request.path - - request = stub_request 'REQUEST_URI' => "/" + request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/' assert_equal "/", request.request_uri assert_equal "/", request.path - request = stub_request 'REQUEST_URI' => "/?m=b" + request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/', 'QUERY_STRING' => 'm=b' assert_equal "/?m=b", request.request_uri assert_equal "/", request.path - request = stub_request 'REQUEST_URI' => "/", 'SCRIPT_NAME' => '/dispatch.cgi' - assert_equal "/", request.request_uri - assert_equal "/", request.path - - ActionController::Base.relative_url_root = "/hieraki" - request = stub_request 'REQUEST_URI' => "/hieraki/", 'SCRIPT_NAME' => "/hieraki/dispatch.cgi" + request = stub_request 'SCRIPT_NAME' => '/hieraki', 'PATH_INFO' => '/' assert_equal "/hieraki/", request.request_uri assert_equal "/", request.path - ActionController::Base.relative_url_root = nil - ActionController::Base.relative_url_root = "/collaboration/hieraki" - request = stub_request 'REQUEST_URI' => "/collaboration/hieraki/books/edit/2", - 'SCRIPT_NAME' => "/collaboration/hieraki/dispatch.cgi" + request = stub_request 'SCRIPT_NAME' => '/collaboration/hieraki', 'PATH_INFO' => '/books/edit/2' assert_equal "/collaboration/hieraki/books/edit/2", request.request_uri assert_equal "/books/edit/2", request.path - ActionController::Base.relative_url_root = nil - - # The following tests are for when REQUEST_URI is not supplied (as in IIS) - request = stub_request 'PATH_INFO' => "/path/of/some/uri?mapped=1", - 'SCRIPT_NAME' => nil, - 'REQUEST_URI' => nil - assert_equal "/path/of/some/uri?mapped=1", request.request_uri - assert_equal "/path/of/some/uri", request.path - ActionController::Base.relative_url_root = '/path' - request = stub_request 'PATH_INFO' => "/path/of/some/uri?mapped=1", - 'SCRIPT_NAME' => "/path/dispatch.rb", - 'REQUEST_URI' => nil + request = stub_request 'SCRIPT_NAME' => '/path', 'PATH_INFO' => '/of/some/uri', 'QUERY_STRING' => 'mapped=1' assert_equal "/path/of/some/uri?mapped=1", request.request_uri assert_equal "/of/some/uri", request.path - ActionController::Base.relative_url_root = nil - - request = stub_request 'PATH_INFO' => "/path/of/some/uri", - 'SCRIPT_NAME' => nil, - 'REQUEST_URI' => nil - assert_equal "/path/of/some/uri", request.request_uri - assert_equal "/path/of/some/uri", request.path - - request = stub_request 'PATH_INFO' => '/', 'REQUEST_URI' => nil - assert_equal "/", request.request_uri - assert_equal "/", request.path - - request = stub_request 'PATH_INFO' => '/?m=b', 'REQUEST_URI' => nil - assert_equal "/?m=b", request.request_uri - assert_equal "/", request.path - - request = stub_request 'PATH_INFO' => "/", - 'SCRIPT_NAME' => "/dispatch.cgi", - 'REQUEST_URI' => nil - assert_equal "/", request.request_uri - assert_equal "/", request.path - - ActionController::Base.relative_url_root = '/hieraki' - request = stub_request 'PATH_INFO' => "/hieraki/", - 'SCRIPT_NAME' => "/hieraki/dispatch.cgi", - 'REQUEST_URI' => nil - assert_equal "/hieraki/", request.request_uri - assert_equal "/", request.path - ActionController::Base.relative_url_root = nil - - request = stub_request 'REQUEST_URI' => '/hieraki/dispatch.cgi' - ActionController::Base.relative_url_root = '/hieraki' - assert_equal "/dispatch.cgi", request.path - ActionController::Base.relative_url_root = nil - - request = stub_request 'REQUEST_URI' => '/hieraki/dispatch.cgi' - ActionController::Base.relative_url_root = '/foo' - assert_equal "/hieraki/dispatch.cgi", request.path - ActionController::Base.relative_url_root = nil - - # This test ensures that Rails uses REQUEST_URI over PATH_INFO - ActionController::Base.relative_url_root = nil - request = stub_request 'REQUEST_URI' => "/some/path", - 'PATH_INFO' => "/another/path", - 'SCRIPT_NAME' => "/dispatch.cgi" - assert_equal "/some/path", request.request_uri - assert_equal "/some/path", request.path end @@ -498,7 +428,7 @@ class RequestTest < ActiveSupport::TestCase protected - def stub_request(env={}) + def stub_request(env = {}) ActionDispatch::Request.new(env) end diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index b047466aaf2e1..afec78ddce500 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -464,8 +464,6 @@ def render_default class LinkToUnlessCurrentWithControllerTest < ActionController::TestCase def setup super - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new @controller = TasksController.new end @@ -565,7 +563,6 @@ def rescue_action(e) raise e end class PolymorphicControllerTest < ActionController::TestCase def setup super - @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new end