From 224a534400fd622dda57058d1eed349b8375e5e3 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 9 Mar 2009 22:45:38 -0500 Subject: reset_session should force a new session id to be generated [#2173] --- actionpack/lib/action_controller/request.rb | 1 + .../test/activerecord/active_record_store_test.rb | 25 ++++++++++++++ .../controller/session/mem_cache_store_test.rb | 40 +++++++++++++--------- 3 files changed, 50 insertions(+), 16 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 2cabab9ec8..ef223f157c 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -442,6 +442,7 @@ EOM end def reset_session + @env['rack.session.options'].delete(:id) @env['rack.session'] = {} end diff --git a/actionpack/test/activerecord/active_record_store_test.rb b/actionpack/test/activerecord/active_record_store_test.rb index 7998f9c22f..c98892edc1 100644 --- a/actionpack/test/activerecord/active_record_store_test.rb +++ b/actionpack/test/activerecord/active_record_store_test.rb @@ -21,8 +21,15 @@ class ActiveRecordStoreTest < ActionController::IntegrationTest render :text => "foo: #{session[:foo].inspect}" end + def get_session_id + session[:foo] + render :text => "#{request.session_options[:id]}" + end + def call_reset_session + session[:bar] reset_session + session[:bar] = "baz" head :ok end @@ -71,6 +78,7 @@ class ActiveRecordStoreTest < ActionController::IntegrationTest get '/set_session_value' assert_response :success assert cookies['_session_id'] + session_id = cookies['_session_id'] get '/call_reset_session' assert_response :success @@ -79,6 +87,23 @@ class ActiveRecordStoreTest < ActionController::IntegrationTest get '/get_session_value' assert_response :success assert_equal 'foo: nil', response.body + + get '/get_session_id' + assert_response :success + assert_not_equal session_id, response.body + end + end + + def test_getting_session_id + with_test_route_set do + get '/set_session_value' + assert_response :success + assert cookies['_session_id'] + session_id = cookies['_session_id'] + + get '/get_session_id' + assert_response :success + assert_equal session_id, response.body end end diff --git a/actionpack/test/controller/session/mem_cache_store_test.rb b/actionpack/test/controller/session/mem_cache_store_test.rb index c3a6c8ce45..2f80a3c7c2 100644 --- a/actionpack/test/controller/session/mem_cache_store_test.rb +++ b/actionpack/test/controller/session/mem_cache_store_test.rb @@ -17,11 +17,14 @@ class MemCacheStoreTest < ActionController::IntegrationTest end def get_session_id - render :text => "foo: #{session[:foo].inspect}; id: #{request.session_options[:id]}" + session[:foo] + render :text => "#{request.session_options[:id]}" end def call_reset_session + session[:bar] reset_session + session[:bar] = "baz" head :ok end @@ -58,47 +61,52 @@ class MemCacheStoreTest < ActionController::IntegrationTest end end - def test_getting_session_id + def test_setting_session_value_after_session_reset with_test_route_set do get '/set_session_value' assert_response :success assert cookies['_session_id'] session_id = cookies['_session_id'] - get '/get_session_id' + get '/call_reset_session' assert_response :success - assert_equal "foo: \"bar\"; id: #{session_id}", response.body - end - end + assert_not_equal [], headers['Set-Cookie'] - def test_prevents_session_fixation - with_test_route_set do get '/get_session_value' assert_response :success assert_equal 'foo: nil', response.body - session_id = cookies['_session_id'] - - reset! - get '/set_session_value', :_session_id => session_id + get '/get_session_id' assert_response :success - assert_equal nil, cookies['_session_id'] + assert_not_equal session_id, response.body end end - def test_setting_session_value_after_session_reset + def test_getting_session_id with_test_route_set do get '/set_session_value' assert_response :success assert cookies['_session_id'] + session_id = cookies['_session_id'] - get '/call_reset_session' + get '/get_session_id' assert_response :success - assert_not_equal [], headers['Set-Cookie'] + assert_equal session_id, response.body + end + end + def test_prevents_session_fixation + with_test_route_set do get '/get_session_value' assert_response :success assert_equal 'foo: nil', response.body + session_id = cookies['_session_id'] + + reset! + + get '/set_session_value', :_session_id => session_id + assert_response :success + assert_equal nil, cookies['_session_id'] end end rescue LoadError, RuntimeError -- cgit v1.2.3 From bdfa733d04c5843eadc181f2fd1723db614fc3d0 Mon Sep 17 00:00:00 2001 From: Eugene Pimenov Date: Tue, 10 Mar 2009 16:52:21 +0000 Subject: Ensure auto_link doesnt linkify URLs in the middle of a tag [#1523 state:resolved] Signed-off-by: Pratik Naik --- actionpack/lib/action_view/helpers/text_helper.rb | 5 +++-- actionpack/test/template/text_helper_test.rb | 6 ++++++ 2 files changed, 9 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index 63fe0c1c57..48bf4717ad 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -536,8 +536,9 @@ module ActionView text.gsub(AUTO_LINK_RE) do href = $& punctuation = '' - # detect already linked URLs - if $` =~ /]*href="$/ + left, right = $`, $' + # detect already linked URLs and URLs in the middle of a tag + if left =~ /<[^>]+$/ && right =~ /^[^>]*>/ # do not change string; URL is alreay linked href else diff --git a/actionpack/test/template/text_helper_test.rb b/actionpack/test/template/text_helper_test.rb index 564845779f..a370f1458f 100644 --- a/actionpack/test/template/text_helper_test.rb +++ b/actionpack/test/template/text_helper_test.rb @@ -375,6 +375,12 @@ class TextHelperTest < ActionView::TestCase assert_equal "{link: #{link3_result}}", auto_link("{link: #{link3_raw}}") end + def test_auto_link_in_tags + link_raw = 'http://www.rubyonrails.org/images/rails.png' + link_result = %Q() + assert_equal link_result, auto_link(link_result) + end + def test_auto_link_at_eol url1 = "http://api.rubyonrails.com/Foo.html" url2 = "http://www.ruby-doc.org/core/Bar.html" -- cgit v1.2.3 From 0464254430f1e594cb2f54dc1e37510fc052c63a Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Tue, 10 Mar 2009 16:52:40 +0000 Subject: Add a missing CHANGELOG entry --- actionpack/CHANGELOG | 2 ++ 1 file changed, 2 insertions(+) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index c4e4211c42..90232d8c2d 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -6,6 +6,8 @@ * Fixed that passing a custom form builder would be forwarded to nested fields_for calls #2023 [Eloy Duran/Nate Wiger] +* Form option helpers now support disabled option tags and the use of lambdas for selecting/disabling option tags from collections #837 [Tekin] + * Added partial scoping to TranslationHelper#translate, so if you call translate(".foo") from the people/index.html.erb template, you'll actually be calling I18n.translate("people.index.foo") [DHH] * Fix a syntax error in current_page?() that was prevent matches against URL's with multiple query parameters #1385, #1868 [chris finne/Andrew White] -- cgit v1.2.3 From 8970f8ace59cb9d07e913566d8d6e87b828759e7 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 10 Mar 2009 15:02:58 -0500 Subject: remove rack gem dependency --- actionpack/Rakefile | 1 - 1 file changed, 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/Rakefile b/actionpack/Rakefile index 0d673c617d..2c0c28b755 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -81,7 +81,6 @@ spec = Gem::Specification.new do |s| s.requirements << 'none' s.add_dependency('activesupport', '= 2.3.1' + PKG_BUILD) - s.add_dependency('rack', '>= 0.9.0') s.require_path = 'lib' s.autorequire = 'action_controller' -- cgit v1.2.3 From 572e0aac802334d2029e67eb1e87356d890f4255 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 10 Mar 2009 15:05:38 -0500 Subject: update bundled version of rack before 2.3 final --- .../lib/action_controller/vendor/rack-1.0/rack.rb | 1 + .../vendor/rack-1.0/rack/auth/abstract/handler.rb | 4 ++-- .../vendor/rack-1.0/rack/auth/digest/md5.rb | 2 +- .../vendor/rack-1.0/rack/auth/digest/request.rb | 2 +- .../vendor/rack-1.0/rack/builder.rb | 6 +----- .../vendor/rack-1.0/rack/content_type.rb | 23 ++++++++++++++++++++++ .../vendor/rack-1.0/rack/directory.rb | 2 ++ .../vendor/rack-1.0/rack/handler/webrick.rb | 7 ++++++- .../action_controller/vendor/rack-1.0/rack/lint.rb | 4 +++- .../vendor/rack-1.0/rack/response.rb | 2 ++ 10 files changed, 42 insertions(+), 11 deletions(-) create mode 100644 actionpack/lib/action_controller/vendor/rack-1.0/rack/content_type.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack.rb index 6c03b55552..e3e20ed2a3 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack.rb @@ -31,6 +31,7 @@ module Rack autoload :CommonLogger, "rack/commonlogger" autoload :ConditionalGet, "rack/conditionalget" autoload :ContentLength, "rack/content_length" + autoload :ContentType, "rack/content_type" autoload :File, "rack/file" autoload :Deflater, "rack/deflater" autoload :Directory, "rack/directory" diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/abstract/handler.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/abstract/handler.rb index 8489c9b9c4..214df6299e 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/abstract/handler.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/abstract/handler.rb @@ -8,8 +8,8 @@ module Rack attr_accessor :realm - def initialize(app, &authenticator) - @app, @authenticator = app, authenticator + def initialize(app, realm=nil, &authenticator) + @app, @realm, @authenticator = app, realm, authenticator end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/md5.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/md5.rb index 6d2bd29c2e..e579dc9632 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/md5.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/md5.rb @@ -21,7 +21,7 @@ module Rack attr_writer :passwords_hashed - def initialize(app) + def initialize(*args) super @passwords_hashed = nil end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/request.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/request.rb index a40f57b7f1..a8aa3bf996 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/request.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/request.rb @@ -8,7 +8,7 @@ module Rack class Request < Auth::AbstractRequest def method - @env['REQUEST_METHOD'] + @env['rack.methodoverride.original_method'] || @env['REQUEST_METHOD'] end def digest? diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/builder.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/builder.rb index 25994d5a44..295235e56a 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/builder.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/builder.rb @@ -34,11 +34,7 @@ module Rack end def use(middleware, *args, &block) - @ins << if block_given? - lambda { |app| middleware.new(app, *args, &block) } - else - lambda { |app| middleware.new(app, *args) } - end + @ins << lambda { |app| middleware.new(app, *args, &block) } end def run(app) diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_type.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_type.rb new file mode 100644 index 0000000000..0c1e1ca3e1 --- /dev/null +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_type.rb @@ -0,0 +1,23 @@ +require 'rack/utils' + +module Rack + + # Sets the Content-Type header on responses which don't have one. + # + # Builder Usage: + # use Rack::ContentType, "text/plain" + # + # When no content type argument is provided, "text/html" is assumed. + class ContentType + def initialize(app, content_type = "text/html") + @app, @content_type = app, content_type + end + + def call(env) + status, headers, body = @app.call(env) + headers = Utils::HeaderHash.new(headers) + headers['Content-Type'] ||= @content_type + [status, headers.to_hash, body] + end + end +end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb index 56ee5e7b60..0f37df3a86 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb @@ -89,6 +89,8 @@ table { width:100%%; } type = stat.directory? ? 'directory' : Mime.mime_type(ext) size = stat.directory? ? '-' : filesize_format(size) mtime = stat.mtime.httpdate + url << '/' if stat.directory? + basename << '/' if stat.directory? @files << [ url, basename, size, type, mtime ] end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/webrick.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/webrick.rb index 40be79de13..138aae0ee9 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/webrick.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/webrick.rb @@ -35,7 +35,12 @@ module Rack env["HTTP_VERSION"] ||= env["SERVER_PROTOCOL"] env["QUERY_STRING"] ||= "" env["REQUEST_PATH"] ||= "/" - env.delete "PATH_INFO" if env["PATH_INFO"] == "" + if env["PATH_INFO"] == "" + env.delete "PATH_INFO" + else + path, n = req.request_uri.path, env["SCRIPT_NAME"].length + env["PATH_INFO"] = path[n, path.length-n] + end status, headers, body = @app.call(env) begin diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb index 7eb05437f0..66d252b8b0 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb @@ -88,7 +88,9 @@ module Rack ## within the application. This may be an ## empty string, if the request URL targets ## the application root and does not have a - ## trailing slash. + ## trailing slash. This information should be + ## decoded by the server if it comes from a + ## URL. ## QUERY_STRING:: The portion of the request URL that ## follows the ?, if any. May be diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/response.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/response.rb index a593110139..caf60d5b19 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/response.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/response.rb @@ -16,6 +16,8 @@ module Rack # Your application's +call+ should end returning Response#finish. class Response + attr_accessor :length + def initialize(body=[], status=200, header={}, &block) @status = status @header = Utils::HeaderHash.new({"Content-Type" => "text/html"}. -- cgit v1.2.3 From 0a887e2386a827f554c685dccf91701bb38422b5 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 10 Mar 2009 15:09:44 -0500 Subject: check for rack 1.0 gem before falling back to bundled version --- actionpack/lib/action_controller.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index ca826e7bfc..d03f4cb231 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -31,7 +31,12 @@ rescue LoadError end end -require 'action_controller/vendor/rack-1.0/rack' +begin + gem 'rack', '~> 1.0.0' + require 'rack' +rescue Gem::LoadError + require 'action_controller/vendor/rack-1.0/rack' +end module ActionController # TODO: Review explicit to see if they will automatically be handled by -- cgit v1.2.3 From fa45540cdb30cee44983c9121e3ebfc317d21668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 11 Mar 2009 00:02:08 +0100 Subject: Ensure correct content type is declared after cache hits on actions with string cache keys [#1585 state:resolved] Signed-off-by: Pratik Naik --- .../lib/action_controller/caching/actions.rb | 27 ++++++++-------------- actionpack/test/controller/caching_test.rb | 14 +++++++++++ 2 files changed, 24 insertions(+), 17 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index 34e1c3527f..87b5029e57 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -129,24 +129,23 @@ module ActionController #:nodoc: attr_reader :path, :extension class << self - def path_for(controller, options, infer_extension=true) + def path_for(controller, options, infer_extension = true) new(controller, options, infer_extension).path end end # When true, infer_extension will look up the cache path extension from the request's path & format. - # This is desirable when reading and writing the cache, but not when expiring the cache - expire_action should expire the same files regardless of the request format. - def initialize(controller, options = {}, infer_extension=true) - if infer_extension and options.is_a? Hash - request_extension = extract_extension(controller.request) - options = options.reverse_merge(:format => request_extension) + # This is desirable when reading and writing the cache, but not when expiring the cache - + # expire_action should expire the same files regardless of the request format. + def initialize(controller, options = {}, infer_extension = true) + if infer_extension + extract_extension(controller.request) + options = options.reverse_merge(:format => @extension) if options.is_a?(Hash) end + path = controller.url_for(options).split('://').last normalize!(path) - if infer_extension - @extension = request_extension - add_extension!(path, @extension) - end + add_extension!(path, @extension) @path = URI.unescape(path) end @@ -162,13 +161,7 @@ module ActionController #:nodoc: def extract_extension(request) # Don't want just what comes after the last '.' to accommodate multi part extensions # such as tar.gz. - extension = request.path[/^[^.]+\.(.+)$/, 1] - - # If there's no extension in the path, check request.format - if extension.nil? - extension = request.cache_format - end - extension + @extension = request.path[/^[^.]+\.(.+)$/, 1] || request.cache_format end end end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 9af1ccc740..86dafd9221 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -428,6 +428,20 @@ class ActionCacheTest < ActionController::TestCase assert_equal 'application/xml', @response.content_type end + def test_correct_content_type_is_returned_for_cache_hit_on_action_with_string_key + # run it twice to cache it the first time + get :show, :format => 'xml' + get :show, :format => 'xml' + assert_equal 'application/xml', @response.content_type + end + + def test_correct_content_type_is_returned_for_cache_hit_on_action_with_string_key_from_proc + # run it twice to cache it the first time + get :edit, :id => 1, :format => 'xml' + get :edit, :id => 1, :format => 'xml' + assert_equal 'application/xml', @response.content_type + end + def test_empty_path_is_normalized @mock_controller.mock_url_for = 'http://example.org/' @mock_controller.mock_path = '/' -- cgit v1.2.3 From f2c7508befb085ffe19ec7fb9ca2e6919cc919c9 Mon Sep 17 00:00:00 2001 From: Russ Smith Date: Wed, 11 Mar 2009 12:50:24 -0500 Subject: Update bundled Rack to fix Litespeed compatibility [#2198 state:resolved] Signed-off-by: Joshua Peek --- .../vendor/rack-1.0/rack/content_length.rb | 8 +++++--- .../lib/action_controller/vendor/rack-1.0/rack/deflater.rb | 4 ++-- .../lib/action_controller/vendor/rack-1.0/rack/directory.rb | 4 ++-- .../action_controller/vendor/rack-1.0/rack/handler/lsws.rb | 2 +- .../lib/action_controller/vendor/rack-1.0/rack/lint.rb | 2 +- .../action_controller/vendor/rack-1.0/rack/showstatus.rb | 2 +- .../lib/action_controller/vendor/rack-1.0/rack/utils.rb | 13 +++++++++++++ 7 files changed, 25 insertions(+), 10 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_length.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_length.rb index bce22a32c5..1e56d43853 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_length.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_length.rb @@ -3,21 +3,23 @@ require 'rack/utils' module Rack # Sets the Content-Length header on responses with fixed-length bodies. class ContentLength + include Rack::Utils + def initialize(app) @app = app end def call(env) status, headers, body = @app.call(env) - headers = Utils::HeaderHash.new(headers) + headers = HeaderHash.new(headers) - if !Utils::STATUS_WITH_NO_ENTITY_BODY.include?(status) && + if !STATUS_WITH_NO_ENTITY_BODY.include?(status) && !headers['Content-Length'] && !headers['Transfer-Encoding'] && (body.respond_to?(:to_ary) || body.respond_to?(:to_str)) body = [body] if body.respond_to?(:to_str) # rack 0.4 compat - length = body.to_ary.inject(0) { |len, part| len + part.length } + length = body.to_ary.inject(0) { |len, part| len + bytesize(part) } headers['Content-Length'] = length.to_s end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/deflater.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/deflater.rb index 3e66680092..a42b7477ae 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/deflater.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/deflater.rb @@ -36,12 +36,12 @@ module Rack mtime = headers.key?("Last-Modified") ? Time.httpdate(headers["Last-Modified"]) : Time.now body = self.class.gzip(body, mtime) - size = body.respond_to?(:bytesize) ? body.bytesize : body.size + size = Rack::Utils.bytesize(body) headers = headers.merge("Content-Encoding" => "gzip", "Content-Length" => size.to_s) [status, headers, [body]] when "deflate" body = self.class.deflate(body) - size = body.respond_to?(:bytesize) ? body.bytesize : body.size + size = Rack::Utils.bytesize(body) headers = headers.merge("Content-Encoding" => "deflate", "Content-Length" => size.to_s) [status, headers, [body]] when "identity" diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb index 0f37df3a86..acdd3029d3 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb @@ -70,7 +70,7 @@ table { width:100%%; } return unless @path_info.include? ".." body = "Forbidden\n" - size = body.respond_to?(:bytesize) ? body.bytesize : body.size + size = Rack::Utils.bytesize(body) return [403, {"Content-Type" => "text/plain","Content-Length" => size.to_s}, [body]] end @@ -122,7 +122,7 @@ table { width:100%%; } def entity_not_found body = "Entity not found: #{@path_info}\n" - size = body.respond_to?(:bytesize) ? body.bytesize : body.size + size = Rack::Utils.bytesize(body) return [404, {"Content-Type" => "text/plain", "Content-Length" => size.to_s}, [body]] end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/lsws.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/lsws.rb index 1f850fc77b..dfc79c204b 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/lsws.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/lsws.rb @@ -13,7 +13,7 @@ module Rack env.delete "HTTP_CONTENT_LENGTH" env["SCRIPT_NAME"] = "" if env["SCRIPT_NAME"] == "/" env.update({"rack.version" => [0,1], - "rack.input" => $stdin, + "rack.input" => StringIO.new($stdin.read.to_s), "rack.errors" => $stderr, "rack.multithread" => false, "rack.multiprocess" => true, diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb index 66d252b8b0..ec4dac96f8 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb @@ -403,7 +403,7 @@ module Rack break end - bytes += (part.respond_to?(:bytesize) ? part.bytesize : part.size) + bytes += Rack::Utils.bytesize(part) } if env["REQUEST_METHOD"] == "HEAD" diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/showstatus.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/showstatus.rb index 5f13404dce..28258c7c89 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/showstatus.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/showstatus.rb @@ -27,7 +27,7 @@ module Rack message = Rack::Utils::HTTP_STATUS_CODES[status.to_i] || status.to_s detail = env["rack.showstatus.detail"] || message body = @template.result(binding) - size = body.respond_to?(:bytesize) ? body.bytesize : body.size + size = Rack::Utils.bytesize(body) [status, headers.merge("Content-Type" => "text/html", "Content-Length" => size.to_s), [body]] else [status, headers, body] diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/utils.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/utils.rb index f352cb6783..e86d4ccdcd 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/utils.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/utils.rb @@ -144,6 +144,19 @@ module Rack end module_function :select_best_encoding + # Return the bytesize of String; uses String#length under Ruby 1.8 and + # String#bytesize under 1.9. + if ''.respond_to?(:bytesize) + def bytesize(string) + string.bytesize + end + else + def bytesize(string) + string.size + end + end + module_function :bytesize + # Context allows the use of a compatible middleware at different points # in a request handling stack. A compatible middleware must define # #context which should take the arguments env and app. The first of which -- cgit v1.2.3 From be7b64b35aac1c9e9063d1d8317f8b1be2a3411c Mon Sep 17 00:00:00 2001 From: Donald Parish Date: Thu, 12 Mar 2009 13:24:54 +0000 Subject: Support MD5 passwords for Digest auth and use session_options[:secret] in nonce [#2209 state:resolved] Signed-off-by: Pratik Naik --- .../lib/action_controller/http_authentication.rb | 72 +++++++++++++++------- .../controller/http_digest_authentication_test.rb | 53 +++++++++++++--- 2 files changed, 97 insertions(+), 28 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/http_authentication.rb b/actionpack/lib/action_controller/http_authentication.rb index 2ccbc22420..b6b5267c66 100644 --- a/actionpack/lib/action_controller/http_authentication.rb +++ b/actionpack/lib/action_controller/http_authentication.rb @@ -68,8 +68,11 @@ module ActionController # # Simple Digest example: # + # require 'digest/md5' # class PostsController < ApplicationController - # USERS = {"dhh" => "secret"} + # REALM = "SuperSecret" + # USERS = {"dhh" => "secret", #plain text password + # "dap" => Digest:MD5::hexdigest(["dap",REALM,"secret"].join(":")) #ha1 digest password # # before_filter :authenticate, :except => [:index] # @@ -83,14 +86,18 @@ module ActionController # # private # def authenticate - # authenticate_or_request_with_http_digest(realm) do |username| + # authenticate_or_request_with_http_digest(REALM) do |username| # USERS[username] # end # end # end # - # NOTE: The +authenticate_or_request_with_http_digest+ block must return the user's password so the framework can appropriately - # hash it to check the user's credentials. Returning +nil+ will cause authentication to fail. + # NOTE: The +authenticate_or_request_with_http_digest+ block must return the user's password or the ha1 digest hash so the framework can appropriately + # hash to check the user's credentials. Returning +nil+ will cause authentication to fail. + # Storing the ha1 hash: MD5(username:realm:password), is better than storing a plain password. If + # the password file or database is compromised, the attacker would be able to use the ha1 hash to + # authenticate as the user at this +realm+, but would not have the user's password to try using at + # other sites. # # On shared hosts, Apache sometimes doesn't pass authentication headers to # FCGI instances. If your environment matches this description and you cannot @@ -177,26 +184,37 @@ module ActionController end # Raises error unless the request credentials response value matches the expected value. + # First try the password as a ha1 digest password. If this fails, then try it as a plain + # text password. def validate_digest_response(request, realm, &password_procedure) credentials = decode_credentials_header(request) valid_nonce = validate_nonce(request, credentials[:nonce]) - if valid_nonce && realm == credentials[:realm] && opaque(request.session.session_id) == credentials[:opaque] + if valid_nonce && realm == credentials[:realm] && opaque == credentials[:opaque] password = password_procedure.call(credentials[:username]) - expected = expected_response(request.env['REQUEST_METHOD'], credentials[:uri], credentials, password) - expected == credentials[:response] + + [true, false].any? do |password_is_ha1| + expected = expected_response(request.env['REQUEST_METHOD'], request.env['REQUEST_URI'], credentials, password, password_is_ha1) + expected == credentials[:response] + end end end # Returns the expected response for a request of +http_method+ to +uri+ with the decoded +credentials+ and the expected +password+ - def expected_response(http_method, uri, credentials, password) - ha1 = ::Digest::MD5.hexdigest([credentials[:username], credentials[:realm], password].join(':')) + # Optional parameter +password_is_ha1+ is set to +true+ by default, since best practice is to store ha1 digest instead + # of a plain-text password. + def expected_response(http_method, uri, credentials, password, password_is_ha1=true) + ha1 = password_is_ha1 ? password : ha1(credentials, password) ha2 = ::Digest::MD5.hexdigest([http_method.to_s.upcase, uri].join(':')) ::Digest::MD5.hexdigest([ha1, credentials[:nonce], credentials[:nc], credentials[:cnonce], credentials[:qop], ha2].join(':')) end - def encode_credentials(http_method, credentials, password) - credentials[:response] = expected_response(http_method, credentials[:uri], credentials, password) + def ha1(credentials, password) + ::Digest::MD5.hexdigest([credentials[:username], credentials[:realm], password].join(':')) + end + + def encode_credentials(http_method, credentials, password, password_is_ha1) + credentials[:response] = expected_response(http_method, credentials[:uri], credentials, password, password_is_ha1) "Digest " + credentials.sort_by {|x| x[0].to_s }.inject([]) {|a, v| a << "#{v[0]}='#{v[1]}'" }.join(', ') end @@ -213,8 +231,7 @@ module ActionController end def authentication_header(controller, realm) - session_id = controller.request.session.session_id - controller.headers["WWW-Authenticate"] = %(Digest realm="#{realm}", qop="auth", algorithm=MD5, nonce="#{nonce(session_id)}", opaque="#{opaque(session_id)}") + controller.headers["WWW-Authenticate"] = %(Digest realm="#{realm}", qop="auth", algorithm=MD5, nonce="#{nonce}", opaque="#{opaque}") end def authentication_request(controller, realm, message = nil) @@ -252,23 +269,36 @@ module ActionController # POST or PUT requests and a time-stamp for GET requests. For more details on the issues involved see Section 4 # of this document. # - # The nonce is opaque to the client. - def nonce(session_id, time = Time.now) + # The nonce is opaque to the client. Composed of Time, and hash of Time with secret + # key from the Rails session secret generated upon creation of project. Ensures + # the time cannot be modifed by client. + def nonce(time = Time.now) t = time.to_i - hashed = [t, session_id] + hashed = [t, secret_key] digest = ::Digest::MD5.hexdigest(hashed.join(":")) Base64.encode64("#{t}:#{digest}").gsub("\n", '') end - def validate_nonce(request, value) + # Might want a shorter timeout depending on whether the request + # is a PUT or POST, and if client is browser or web service. + # Can be much shorter if the Stale directive is implemented. This would + # allow a user to use new nonce without prompting user again for their + # username and password. + def validate_nonce(request, value, seconds_to_timeout=5*60) t = Base64.decode64(value).split(":").first.to_i - nonce(request.session.session_id, t) == value && (t - Time.now.to_i).abs <= 10 * 60 + nonce(t) == value && (t - Time.now.to_i).abs <= seconds_to_timeout end - # Opaque based on digest of session_id - def opaque(session_id) - Base64.encode64(::Digest::MD5::hexdigest(session_id)).gsub("\n", '') + # Opaque based on random generation - but changing each request? + def opaque() + ::Digest::MD5.hexdigest(secret_key) end + + # Set in /initializers/session_store.rb, and loaded even if sessions are not in use. + def secret_key + ActionController::Base.session_options[:secret] + end + end end end diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index 4913e7633b..00789eea38 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -5,7 +5,8 @@ class HttpDigestAuthenticationTest < ActionController::TestCase before_filter :authenticate, :only => :index before_filter :authenticate_with_request, :only => :display - USERS = { 'lifo' => 'world', 'pretty' => 'please' } + USERS = { 'lifo' => 'world', 'pretty' => 'please', + 'dhh' => ::Digest::MD5::hexdigest(["dhh","SuperSecret","secret"].join(":"))} def index render :text => "Hello Secret" @@ -107,8 +108,42 @@ class HttpDigestAuthenticationTest < ActionController::TestCase assert_equal 'Definitely Maybe', @response.body end - test "authentication request with relative URI" do - @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:uri => "/", :username => 'pretty', :password => 'please') + test "authentication request with valid credential and nil session" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please') + + # session_id = "" in functional test, but is +nil+ in real life + @request.session.session_id = nil + get :display + + assert_response :success + assert assigns(:logged_in) + assert_equal 'Definitely Maybe', @response.body + end + + 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" + get :display + + assert_response :unauthorized + assert_equal "Authentication Failed", @response.body + end + + test "authentication request with absolute uri" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:uri => "http://test.host/http_digest_authentication_test/dummy_digest/display", + :username => 'pretty', :password => 'please') + @request.env['REQUEST_URI'] = "http://test.host/http_digest_authentication_test/dummy_digest/display" + get :display + + assert_response :success + assert assigns(:logged_in) + assert_equal 'Definitely Maybe', @response.body + end + + test "authentication request with password stored as ha1 digest hash" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'dhh', + :password => ::Digest::MD5::hexdigest(["dhh","SuperSecret","secret"].join(":")), + :password_is_ha1 => true) get :display assert_response :success @@ -119,18 +154,22 @@ class HttpDigestAuthenticationTest < ActionController::TestCase private def encode_credentials(options) - options.reverse_merge!(:nc => "00000001", :cnonce => "0a4f113b") + options.reverse_merge!(:nc => "00000001", :cnonce => "0a4f113b", :password_is_ha1 => false) password = options.delete(:password) - # Perform unautheticated get to retrieve digest parameters to use on subsequent request + # Set in /initializers/session_store.rb. Used as secret in generating nonce + # to prevent tampering of timestamp + ActionController::Base.session_options[:secret] = "session_options_secret" + + # Perform unauthenticated GET to retrieve digest parameters to use on subsequent request get :index assert_response :unauthorized credentials = decode_credentials(@response.headers['WWW-Authenticate']) credentials.merge!(options) - credentials.reverse_merge!(:uri => "http://#{@request.host}#{@request.env['REQUEST_URI']}") - ActionController::HttpAuthentication::Digest.encode_credentials("GET", credentials, password) + credentials.reverse_merge!(:uri => "#{@request.env['REQUEST_URI']}") + ActionController::HttpAuthentication::Digest.encode_credentials("GET", credentials, password, options[:password_is_ha1]) end def decode_credentials(header) -- cgit v1.2.3