diff options
-rw-r--r-- | actionpack/lib/action_controller/metal/http_authentication.rb | 13 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/http/request.rb | 8 | ||||
-rw-r--r-- | actionpack/test/controller/http_digest_authentication_test.rb | 45 | ||||
-rw-r--r-- | actionpack/test/dispatch/request_test.rb | 24 | ||||
-rw-r--r-- | railties/lib/rails/application.rb | 17 | ||||
-rw-r--r-- | railties/test/application/build_original_fullpath_test.rb | 27 | ||||
-rw-r--r-- | railties/test/application/middleware_test.rb | 11 |
7 files changed, 137 insertions, 8 deletions
diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 264806cd36..56335a22cb 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -192,12 +192,15 @@ module ActionController return false unless password method = request.env['rack.methodoverride.original_method'] || request.env['REQUEST_METHOD'] - uri = credentials[:uri][0,1] == '/' ? request.fullpath : request.url + uri = credentials[:uri][0,1] == '/' ? request.original_fullpath : request.original_url - [true, false].any? do |password_is_ha1| - expected = expected_response(method, uri, credentials, password, password_is_ha1) - expected == credentials[:response] - end + [true, false].any? do |trailing_question_mark| + [true, false].any? do |password_is_ha1| + _uri = trailing_question_mark ? uri + "?" : uri + expected = expected_response(method, _uri, credentials, password, password_is_ha1) + expected == credentials[:response] + end + end end end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index c5c48ec489..820921252d 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -122,10 +122,18 @@ module ActionDispatch Http::Headers.new(@env) end + def original_fullpath + @original_fullpath ||= (env["ORIGINAL_FULLPATH"] || fullpath) + end + def fullpath @fullpath ||= super end + def original_url + base_url + original_fullpath + end + def media_type content_mime_type.to_s end diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index b011536717..a91e3cafa5 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -139,7 +139,7 @@ class HttpDigestAuthenticationTest < ActionController::TestCase 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['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest/altered/uri" + @request.env['ORIGINAL_FULLPATH'] = "/http_digest_authentication_test/dummy_digest/altered/uri" get :display assert_response :unauthorized @@ -208,6 +208,44 @@ class HttpDigestAuthenticationTest < ActionController::TestCase assert !ActionController::HttpAuthentication::Digest.validate_digest_response(@request, "SuperSecret"){nil} end + test "authentication request with request-uri ending in '/'" do + @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest/" + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please') + + # simulate normalizing PATH_INFO + @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest" + get :display + + assert_response :success + assert_equal 'Definitely Maybe', @response.body + end + + test "authentication request with request-uri ending in '?'" do + @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest/?" + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please') + + # simulate normalizing PATH_INFO + @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest" + get :display + + assert_response :success + assert_equal 'Definitely Maybe', @response.body + end + + test "authentication request with absolute uri in credentials (as in IE) ending with /" do + @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest/" + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:uri => "http://test.host/http_digest_authentication_test/dummy_digest/", + :username => 'pretty', :password => 'please') + + # simulate normalizing PATH_INFO + @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest" + get :display + + assert_response :success + assert assigns(:logged_in) + assert_equal 'Definitely Maybe', @response.body + end + private def encode_credentials(options) @@ -228,7 +266,10 @@ class HttpDigestAuthenticationTest < ActionController::TestCase credentials = decode_credentials(@response.headers['WWW-Authenticate']) credentials.merge!(options) - credentials.merge!(:uri => @request.env['PATH_INFO'].to_s) + path_info = @request.env['PATH_INFO'].to_s + uri = options[:uri] || path_info + credentials.merge!(:uri => uri) + @request.env["ORIGINAL_FULLPATH"] = path_info ActionController::HttpAuthentication::Digest.encode_credentials(method, credentials, password, options[:password_is_ha1]) end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 4d805464c2..5b3d38c48c 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -618,6 +618,30 @@ class RequestTest < ActiveSupport::TestCase assert_equal "/authenticate?secret", path end + test "original_fullpath returns ORIGINAL_FULLPATH" do + request = stub_request('ORIGINAL_FULLPATH' => "/foo?bar") + + path = request.original_fullpath + assert_equal "/foo?bar", path + end + + test "original_url returns url built using ORIGINAL_FULLPATH" do + request = stub_request('ORIGINAL_FULLPATH' => "/foo?bar", + 'HTTP_HOST' => "example.org", + 'rack.url_scheme' => "http") + + url = request.original_url + assert_equal "http://example.org/foo?bar", url + end + + test "original_fullpath returns fullpath if ORIGINAL_FULLPATH is not present" do + request = stub_request('PATH_INFO' => "/foo", + 'QUERY_STRING' => "bar") + + path = request.original_fullpath + assert_equal "/foo?bar", path + end + protected def stub_request(env = {}) diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index b20634c5a9..19e8426e60 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -215,6 +215,11 @@ module Rails config.helpers_paths end + def call(env) + env["ORIGINAL_FULLPATH"] = build_original_fullpath(env) + super(env) + end + protected alias :build_middleware_stack :app @@ -291,5 +296,17 @@ module Rails require "rails/console/app" require "rails/console/helpers" end + + def build_original_fullpath(env) + path_info = env["PATH_INFO"] + query_string = env["QUERY_STRING"] + script_name = env["SCRIPT_NAME"] + + if query_string.present? + "#{script_name}#{path_info}?#{query_string}" + else + "#{script_name}#{path_info}" + end + end end end diff --git a/railties/test/application/build_original_fullpath_test.rb b/railties/test/application/build_original_fullpath_test.rb new file mode 100644 index 0000000000..7a679ea04e --- /dev/null +++ b/railties/test/application/build_original_fullpath_test.rb @@ -0,0 +1,27 @@ +require "abstract_unit" + +module ApplicationTests + class BuildOriginalPathTest < Test::Unit::TestCase + def test_include_original_PATH_info_in_ORIGINAL_FULLPATH + env = { 'PATH_INFO' => '/foo/' } + assert_equal "/foo/", Rails.application.send(:build_original_fullpath, env) + end + + def test_include_SCRIPT_NAME + env = { + 'SCRIPT_NAME' => '/foo', + 'PATH_INFO' => '/bar' + } + + assert_equal "/foo/bar", Rails.application.send(:build_original_fullpath, env) + end + + def test_include_QUERY_STRING + env = { + 'PATH_INFO' => '/foo', + 'QUERY_STRING' => 'bar', + } + assert_equal "/foo?bar", Rails.application.send(:build_original_fullpath, env) + end + end +end diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 578370cfca..9e02ef9c66 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -1,5 +1,6 @@ require 'isolation/abstract_unit' require 'stringio' +require 'rack/test' module ApplicationTests class MiddlewareTest < Test::Unit::TestCase @@ -75,7 +76,7 @@ module ApplicationTests add_to_config "config.force_ssl = true" add_to_config "config.ssl_options = { :host => 'example.com' }" boot! - + assert_equal AppTemplate::Application.middleware.first.args, [{:host => 'example.com'}] end @@ -193,6 +194,14 @@ module ApplicationTests assert_equal nil, last_response.headers["Etag"] end + test "ORIGINAL_FULLPATH is passed to env" do + boot! + env = ::Rack::MockRequest.env_for("/foo/?something") + Rails.application.call(env) + + assert_equal "/foo/?something", env["ORIGINAL_FULLPATH"] + end + private def boot! |