Failsafe response handler for dispatcher.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2841 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 68 insertions, 9 deletions

diff --git a/railties/CHANGELOG b/railties/CHANGELOG
index 48ff33608e..eed9a4e4b1 100644
--- a/railties/CHANGELOG
+++ b/railties/CHANGELOG
@@ -1,6 +1,6 @@
 *SVN*
 
-* Move Dispatcher.dispatch CGI.new out of default args and into rescuable block so the dispatcher catches its errors rather than the fcgi handler.  [Jeremy Kemper]
+* Catch CGI multipart parse errors.  Wrap dispatcher internals in a failsafe response handler.  [Jeremy Kemper]
 
 * The freeze_gems Rake task accepts the VERSION environment variable to decide which version of Rails to pull into vendor/rails.  [Chad Fowler, Jeremy Kemper]
 
diff --git a/railties/lib/dispatcher.rb b/railties/lib/dispatcher.rb
index 73bedf5ecf..a2fc6e8553 100644
--- a/railties/lib/dispatcher.rb
+++ b/railties/lib/dispatcher.rb
@@ -29,15 +29,20 @@ class Dispatcher
 
     # Dispatch the given CGI request, using the given session options, and
     # emitting the output via the given output.  If you dispatch with your
-    # own CGI object, be sure to handle the exceptions it raises.
+    # own CGI object be sure to handle the exceptions it raises on multipart
+    # requests (EOFError and ArgumentError).
     def dispatch(cgi = nil, session_options = ActionController::CgiRequest::DEFAULT_SESSION_OPTIONS, output = $stdout)
-      cgi ||= CGI.new
-      request, response = ActionController::CgiRequest.new(cgi, session_options), ActionController::CgiResponse.new(cgi)
-      prepare_application
-      ActionController::Routing::Routes.recognize!(request).process(request, response).out(output)
+      if cgi ||= new_cgi(output)
+        request, response = ActionController::CgiRequest.new(cgi, session_options), ActionController::CgiResponse.new(cgi)
+        prepare_application
+        ActionController::Routing::Routes.recognize!(request).process(request, response).out(output)
+      end
     rescue Object => exception
-      ActionController::Base.process_with_exception(request, response, exception).out(output)
+      failsafe_response(output, '500 Internal Server Error') do
+        ActionController::Base.process_with_exception(request, response, exception).out(output)
+      end
     ensure
+      # Do not give a failsafe response here.
       reset_after_dispatch
     end
 
@@ -51,8 +56,15 @@ class Dispatcher
       Dependencies.remove_subclasses_for(ActiveRecord::Base, ActiveRecord::Observer, ActionController::Base)
       Dependencies.remove_subclasses_for(ActionMailer::Base) if defined?(ActionMailer::Base)
     end
-    
+
     private
+      # CGI.new plus exception handling.  CGI#read_multipart raises EOFError
+      # if body.empty? or body.size != Content-Length and raises ArgumentError
+      # if Content-Length is non-integer.
+      def new_cgi(output)
+        failsafe_response(output, '400 Bad Request') { CGI.new }
+      end
+
       def prepare_application
         ActionController::Routing::Routes.reload if Dependencies.load?
         prepare_breakpoint
@@ -72,5 +84,15 @@ class Dispatcher
       rescue
         nil
       end
+
+      # If the block raises, send status code as a last-ditch response.
+      def failsafe_response(output, status)
+        yield
+      rescue Object
+        begin
+          output.write "Status: #{status}\r\n"
+        rescue Object
+        end
+      end
   end
 end
diff --git a/railties/test/dispatcher_test.rb b/railties/test/dispatcher_test.rb
index caf36fc9e9..bf5d235dfe 100644
--- a/railties/test/dispatcher_test.rb
+++ b/railties/test/dispatcher_test.rb
@@ -50,9 +50,46 @@ class DispatcherTest < Test::Unit::TestCase
     assert_equal 0, ActionMailer::Base.subclasses.length
   end
 
+
+  INVALID_MULTIPART = [
+    'POST /foo HTTP/1.0',
+    'Host: example.com',
+    'Content-Type: multipart/form-data;boundary=foo'
+  ]
+
+  EMPTY_CONTENT = (INVALID_MULTIPART + [
+    'Content-Length: 100',
+    nil, nil
+  ]).join("\r\n")
+
+  CONTENT_LENGTH_MISMATCH = (INVALID_MULTIPART + [
+    'Content-Length: 100',
+    nil, nil,
+    'foobar'
+  ]).join("\r\n")
+
+  NONINTEGER_CONTENT_LENGTH = (INVALID_MULTIPART + [
+    'Content-Length: abc',
+    nil, nil
+  ]).join("\r\n")
+
+  def test_bad_multipart_request
+    old_stdin = $stdin
+    [EMPTY_CONTENT, CONTENT_LENGTH_MISMATCH, NONINTEGER_CONTENT_LENGTH].each do |bad_request|
+      $stdin = StringIO.new(bad_request)
+      output = StringIO.new
+      assert_nothing_raised do
+        Dispatcher.dispatch(nil, ActionController::CgiRequest::DEFAULT_SESSION_OPTIONS, output)
+      end
+      assert_equal "Status: 400 Bad Request\r\n", output.string
+    end
+  ensure
+    $stdin = old_stdin
+  end
+
   private
     def dispatch
-      Dispatcher.dispatch(CGI.new, ActionController::CgiRequest::DEFAULT_SESSION_OPTIONS, @output)
+      Dispatcher.dispatch(nil, ActionController::CgiRequest::DEFAULT_SESSION_OPTIONS, @output)
     end
 
     def setup_minimal_environment