From 9775c25824feb35a5c42f3838d21c7e5faba9ca0 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 13 Jan 2009 17:21:45 -0600 Subject: Update multipart tests to expose (another) bug in Rack's multipart parser --- actionpack/lib/action_controller.rb | 1 + actionpack/lib/action_controller/integration.rb | 11 ----------- .../lib/action_controller/middleware_stack.rb | 2 ++ actionpack/lib/action_controller/rack_ext.rb | 22 ++++++++++++++++++++++ .../lib/action_controller/rewindable_input.rb | 10 +++++++--- .../request/multipart_params_parsing_test.rb | 2 +- .../request/url_encoded_params_parsing_test.rb | 2 +- 7 files changed, 34 insertions(+), 16 deletions(-) create mode 100644 actionpack/lib/action_controller/rack_ext.rb diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 0d81b9c346..a9cd09e58d 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -33,6 +33,7 @@ end gem 'rack', '>= 0.9.0' require 'rack' +require 'action_controller/rack_ext' module ActionController # TODO: Review explicit to see if they will automatically be handled by diff --git a/actionpack/lib/action_controller/integration.rb b/actionpack/lib/action_controller/integration.rb index 5b08e30d49..163ba84a3e 100644 --- a/actionpack/lib/action_controller/integration.rb +++ b/actionpack/lib/action_controller/integration.rb @@ -2,17 +2,6 @@ require 'stringio' require 'uri' require 'active_support/test_case' -# Monkey patch Rack::Lint to support rewind -module Rack - class Lint - class InputWrapper - def rewind - @input.rewind - end - end - end -end - module ActionController module Integration #:nodoc: # An integration Session instance represents a set of requests and responses diff --git a/actionpack/lib/action_controller/middleware_stack.rb b/actionpack/lib/action_controller/middleware_stack.rb index 2bccba2ba1..b94bf6ec4a 100644 --- a/actionpack/lib/action_controller/middleware_stack.rb +++ b/actionpack/lib/action_controller/middleware_stack.rb @@ -32,6 +32,8 @@ module ActionController else @klass.to_s.constantize end + rescue NameError + @klass end def active? diff --git a/actionpack/lib/action_controller/rack_ext.rb b/actionpack/lib/action_controller/rack_ext.rb new file mode 100644 index 0000000000..3b142307e9 --- /dev/null +++ b/actionpack/lib/action_controller/rack_ext.rb @@ -0,0 +1,22 @@ +module Rack + module Utils + module Multipart + class << self + def parse_multipart_with_rewind(env) + result = parse_multipart_without_rewind(env) + + begin + env['rack.input'].rewind if env['rack.input'].respond_to?(:rewind) + rescue Errno::ESPIPE + # Handles exceptions raised by input streams that cannot be rewound + # such as when using plain CGI under Apache + end + + result + end + + alias_method_chain :parse_multipart, :rewind + end + end + end +end diff --git a/actionpack/lib/action_controller/rewindable_input.rb b/actionpack/lib/action_controller/rewindable_input.rb index 296d8aed22..058453ea68 100644 --- a/actionpack/lib/action_controller/rewindable_input.rb +++ b/actionpack/lib/action_controller/rewindable_input.rb @@ -15,15 +15,19 @@ module ActionController @io.rewind end + def string + @string + end + def method_missing(method, *args, &block) @io.send(method, *args, &block) end private def read_original_io - unless @str - @str = @io.read - @io = StringIO.new(@str) + unless @string + @string = @io.read + @io = StringIO.new(@string) end end end diff --git a/actionpack/test/controller/request/multipart_params_parsing_test.rb b/actionpack/test/controller/request/multipart_params_parsing_test.rb index d976ab8512..137fdbee54 100644 --- a/actionpack/test/controller/request/multipart_params_parsing_test.rb +++ b/actionpack/test/controller/request/multipart_params_parsing_test.rb @@ -200,7 +200,7 @@ class MultipartParamsParsingTest < ActionController::IntegrationTest def with_muck_middleware original_middleware = ActionController::Dispatcher.middleware middleware = original_middleware.dup - middleware.use MuckMiddleware + middleware.insert_after ActionController::RewindableInput, MuckMiddleware ActionController::Dispatcher.middleware = middleware yield ActionController::Dispatcher.middleware = original_middleware diff --git a/actionpack/test/controller/request/url_encoded_params_parsing_test.rb b/actionpack/test/controller/request/url_encoded_params_parsing_test.rb index b162796e5b..ee2a239d50 100644 --- a/actionpack/test/controller/request/url_encoded_params_parsing_test.rb +++ b/actionpack/test/controller/request/url_encoded_params_parsing_test.rb @@ -183,7 +183,7 @@ class UrlEncodedParamsParsingTest < ActionController::IntegrationTest def with_muck_middleware original_middleware = ActionController::Dispatcher.middleware middleware = original_middleware.dup - middleware.use MuckMiddleware + middleware.insert_after ActionController::RewindableInput, MuckMiddleware ActionController::Dispatcher.middleware = middleware yield ActionController::Dispatcher.middleware = original_middleware -- cgit v1.2.3 From b281a6a5b2137548501ef590379d7af5f6955d2d Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 13 Jan 2009 17:26:29 -0600 Subject: Use Rack's MethodOverride lib [#1699 state:resolved] --- actionpack/lib/action_controller.rb | 1 - actionpack/lib/action_controller/middlewares.rb | 2 +- .../lib/action_controller/verb_piggybacking.rb | 24 ---------------------- railties/lib/initializer.rb | 2 +- 4 files changed, 2 insertions(+), 27 deletions(-) delete mode 100644 actionpack/lib/action_controller/verb_piggybacking.rb diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index a9cd09e58d..f808bdd910 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -80,7 +80,6 @@ module ActionController autoload :UrlEncodedPairParser, 'action_controller/url_encoded_pair_parser' autoload :UrlRewriter, 'action_controller/url_rewriter' autoload :UrlWriter, 'action_controller/url_rewriter' - autoload :VerbPiggybacking, 'action_controller/verb_piggybacking' autoload :Verification, 'action_controller/verification' module Assertions diff --git a/actionpack/lib/action_controller/middlewares.rb b/actionpack/lib/action_controller/middlewares.rb index 914750bc0c..0f038b8856 100644 --- a/actionpack/lib/action_controller/middlewares.rb +++ b/actionpack/lib/action_controller/middlewares.rb @@ -19,4 +19,4 @@ use "ActiveRecord::QueryCache", :if => lambda { defined?(ActiveRecord) } end use ActionController::RewindableInput -use ActionController::VerbPiggybacking +use Rack::MethodOverride diff --git a/actionpack/lib/action_controller/verb_piggybacking.rb b/actionpack/lib/action_controller/verb_piggybacking.rb deleted file mode 100644 index 86cde304a0..0000000000 --- a/actionpack/lib/action_controller/verb_piggybacking.rb +++ /dev/null @@ -1,24 +0,0 @@ -module ActionController - # TODO: Use Rack::MethodOverride when it is released - class VerbPiggybacking - HTTP_METHODS = %w(GET HEAD PUT POST DELETE OPTIONS) - - def initialize(app) - @app = app - end - - def call(env) - if env["REQUEST_METHOD"] == "POST" - req = Request.new(env) - if method = (req.parameters[:_method] || env["HTTP_X_HTTP_METHOD_OVERRIDE"]) - method = method.to_s.upcase - if HTTP_METHODS.include?(method) - env["REQUEST_METHOD"] = method - end - end - end - - @app.call(env) - end - end -end diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 619701460d..824d1d6096 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -537,7 +537,7 @@ Run `rake gems:install` to install the missing gems. end def initialize_metal - configuration.middleware.insert_before(:"ActionController::VerbPiggybacking", Rails::Rack::Metal) + configuration.middleware.insert_before(:"ActionController::RewindableInput", Rails::Rack::Metal) end # Initializes framework-specific settings for each of the loaded frameworks -- cgit v1.2.3 From 9bcf01b23c25e640da7908ac8b1b49fbf7d2e51a Mon Sep 17 00:00:00 2001 From: "Hongli Lai (Phusion)" Date: Tue, 13 Jan 2009 16:01:44 +0100 Subject: Fix PostgreSQL unit test failures that only occur when using the old 'postgres' driver. [#1748 state:committed] Signed-off-by: Jeremy Kemper --- .../connection_adapters/abstract/database_statements.rb | 12 ++++++++---- .../active_record/connection_adapters/postgresql_adapter.rb | 10 ++++------ activerecord/test/cases/transactions_test.rb | 3 +-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 39118583bd..08601da00a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -89,7 +89,7 @@ module ActiveRecord # - The block will be run without doing anything. All database statements # that happen within the block are effectively appended to the already # open database transaction. - # - However, if +requires_new+ is set, the block will be wrapped in a + # - However, if +:requires_new+ is set, the block will be wrapped in a # database savepoint acting as a sub-transaction. # # === Caveats @@ -113,8 +113,12 @@ module ActiveRecord def transaction(options = {}) options.assert_valid_keys :requires_new, :joinable - last_transaction_joinable, @transaction_joinable = - @transaction_joinable, options[:joinable] || true + last_transaction_joinable = @transaction_joinable + if options.has_key?(:joinable) + @transaction_joinable = options[:joinable] + else + @transaction_joinable = true + end requires_new = options[:requires_new] || !last_transaction_joinable transaction_open = false @@ -141,7 +145,7 @@ module ActiveRecord rollback_to_savepoint end end - raise unless database_transaction_rollback.is_a? ActiveRecord::Rollback + raise unless database_transaction_rollback.is_a?(ActiveRecord::Rollback) end ensure @transaction_joinable = last_transaction_joinable diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 5a8d99924d..913bb521ca 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -533,13 +533,11 @@ module ActiveRecord execute "ROLLBACK" end - if PGconn.public_method_defined?(:transaction_status) - # ruby-pg defines Ruby constants for transaction status, - # ruby-postgres does not. - PQTRANS_IDLE = defined?(PGconn::PQTRANS_IDLE) ? PGconn::PQTRANS_IDLE : 0 - + if defined?(PGconn::PQTRANS_IDLE) + # The ruby-pg driver supports inspecting the transaction status, + # while the ruby-postgres driver does not. def outside_transaction? - @connection.transaction_status == PQTRANS_IDLE + @connection.transaction_status == PGconn::PQTRANS_IDLE end end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index f07fad1828..4a07a8bb1d 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -321,9 +321,8 @@ class TransactionTest < ActiveRecord::TestCase end end - if current_adapter?(:PostgreSQLAdapter) && PGconn.public_method_defined?(:transaction_status) + if current_adapter?(:PostgreSQLAdapter) && defined?(PGconn::PQTRANS_IDLE) def test_outside_transaction_works - Topic.logger.info("-------------") assert Topic.connection.outside_transaction? Topic.connection.begin_db_transaction assert !Topic.connection.outside_transaction? -- cgit v1.2.3