Better handling of multipart parsing when request is missing body.

- Introduce `module Rack::BadRequest` for unified exception handling.
- Make `env['rack.input']` optional.

SPEC.rdoc

@@ -121,7 +121,7 @@ If the string values for CGI keys contain non-ASCII characters,
 they should use ASCII-8BIT encoding.
 There are the following restrictions:
 * <tt>rack.url_scheme</tt> must either be +http+ or +https+.
-* There must be a valid input stream in <tt>rack.input</tt>.
+* There may be a valid input stream in <tt>rack.input</tt>.
 * There must be a valid error stream in <tt>rack.errors</tt>.
 * There may be a valid hijack callback in <tt>rack.hijack</tt>
 * The <tt>REQUEST_METHOD</tt> must be a valid token.

lib/rack/bad_request.rb

@@ -0,0 +1,4 @@
+module Rack
+	module BadRequest
+	end
+end

lib/rack/lint.rb

@@ -56,9 +56,6 @@ def response
         raise LintError, "No env given" unless @env
         check_environment(@env)
 
-        @env[RACK_INPUT] = InputWrapper.new(@env[RACK_INPUT])
-        @env[RACK_ERRORS] = ErrorWrapper.new(@env[RACK_ERRORS])
-
         ## and returns a non-frozen Array of exactly three values:
         @response = @app.call(@env)
         raise LintError, "response is not an Array, but #{@response.class}" unless @response.kind_of? Array
@@ -265,11 +262,9 @@ def check_environment(env)
         ## is reserved for use with the Rack core distribution and other
         ## accepted specifications and must not be used otherwise.
         ##
-
-        %w[REQUEST_METHOD SERVER_NAME QUERY_STRING SERVER_PROTOCOL
-           rack.input rack.errors].each { |header|
+        %w[REQUEST_METHOD SERVER_NAME QUERY_STRING SERVER_PROTOCOL rack.errors].each do |header|
           raise LintError, "env missing required key #{header}" unless env.include? header
-        }
+        end
 
         ## The <tt>SERVER_PORT</tt> must be an Integer if set.
         server_port = env["SERVER_PORT"]
@@ -328,10 +323,20 @@ def check_environment(env)
           raise LintError, "rack.url_scheme unknown: #{env[RACK_URL_SCHEME].inspect}"
         end
 
-        ## * There must be a valid input stream in <tt>rack.input</tt>.
-        check_input env[RACK_INPUT]
+        ## * There may be a valid input stream in <tt>rack.input</tt>.
+        if rack_input = env[RACK_INPUT]
+          check_input_stream(rack_input)
+          @env[RACK_INPUT] = InputWrapper.new(rack_input)
+        end
+
         ## * There must be a valid error stream in <tt>rack.errors</tt>.
-        check_error env[RACK_ERRORS]
+        if rack_errors = env[RACK_ERRORS]
+          check_error_stream(rack_errors)
+          @env[RACK_ERRORS] = ErrorWrapper.new(rack_errors)
+        else
+          raise LintError, "rack.errors is not set"
+        end
+
         ## * There may be a valid hijack callback in <tt>rack.hijack</tt>
         check_hijack env
 
@@ -384,7 +389,7 @@ def check_environment(env)
       ##
       ## The input stream is an IO-like object which contains the raw HTTP
       ## POST data.
-      def check_input(input)
+      def check_input_stream(input)
         ## When applicable, its external encoding must be "ASCII-8BIT" and it
         ## must be opened in binary mode, for Ruby 1.9 compatibility.
         if input.respond_to?(:external_encoding) && input.external_encoding != Encoding::ASCII_8BIT
@@ -488,7 +493,7 @@ def close(*args)
       ##
       ## === The Error Stream
       ##
-      def check_error(error)
+      def check_error_stream(error)
         ## The error stream must respond to +puts+, +write+ and +flush+.
         [:puts, :write, :flush].each { |method|
           unless error.respond_to? method

lib/rack/mock_request.rb

@@ -42,8 +42,7 @@ def string
     end
 
     DEFAULT_ENV = {
-      RACK_INPUT        => StringIO.new,
-      RACK_ERRORS       => StringIO.new,
+      RACK_ERRORS => StringIO.new,
     }.freeze
 
     def initialize(app)
@@ -144,20 +143,25 @@ def self.env_for(uri = "", opts = {})
         end
       end
 
-      opts[:input] ||= String.new
+      unless opts.key?(:input)
+        opts[:input] = String.new
+      end
+
       if String === opts[:input]
         rack_input = StringIO.new(opts[:input])
       else
         rack_input = opts[:input]
       end
 
-      rack_input.set_encoding(Encoding::BINARY)
-      env[RACK_INPUT] = rack_input
+      if rack_input
+        rack_input.set_encoding(Encoding::BINARY)
+        env[RACK_INPUT] = rack_input
 
-      env["CONTENT_LENGTH"] ||= env[RACK_INPUT].size.to_s if env[RACK_INPUT].respond_to?(:size)
+        env["CONTENT_LENGTH"] ||= env[RACK_INPUT].size.to_s if env[RACK_INPUT].respond_to?(:size)
+      end
 
       opts.each { |field, value|
-        env[field] = value  if String === field
+        env[field] = value if String === field
       }
 
       env

lib/rack/multipart.rb

@@ -6,16 +6,24 @@
 require_relative 'multipart/parser'
 require_relative 'multipart/generator'
 
+require_relative 'bad_request'
+
 module Rack
   # A multipart form data parser, adapted from IOWA.
   #
   # Usually, Rack::Request#POST takes care of calling this.
   module Multipart
     MULTIPART_BOUNDARY = "AaB03x"
 
+    class MissingInputError < StandardError
+      include BadRequest
+    end
+
     class << self
       def parse_multipart(env, params = Rack::Utils.default_query_parser)
-        io = env[RACK_INPUT]
+        unless io = env[RACK_INPUT]
+          raise MissingInputError, "Missing input stream!"
+        end
 
         if content_length = env['CONTENT_LENGTH']
           content_length = content_length.to_i

lib/rack/request.rb

@@ -497,7 +497,8 @@ def GET
       # multipart/form-data.
       def POST
         if get_header(RACK_INPUT).nil?
-          raise "Missing rack.input"
+          set_header RACK_REQUEST_FORM_INPUT, get_header(RACK_INPUT)
+          set_header(RACK_REQUEST_FORM_HASH, {})
         elsif get_header(RACK_REQUEST_FORM_INPUT) == get_header(RACK_INPUT)
           get_header(RACK_REQUEST_FORM_HASH)
         elsif form_data? || parseable_data?

test/spec_mock_request.rb

@@ -46,7 +46,7 @@
   end
 
   it "should handle a non-GET request with both :input and :params" do
-    env = Rack::MockRequest.env_for("/", method: :post, input: nil, params: {})
+    env = Rack::MockRequest.env_for("/", method: :post, input: "", params: {})
     env["PATH_INFO"].must_equal "/"
     env.must_be_kind_of Hash
     env['rack.input'].read.must_equal ''

test/spec_multipart.rb

@@ -42,6 +42,13 @@ def multipart_file(name)
     }.must_raise Rack::Multipart::Error
   end
 
+  it "raises a bad request exception if no body is given" do
+    env = Rack::MockRequest.env_for("/", "CONTENT_TYPE" => 'multipart/form-data; boundary=BurgerBurger', :input => nil)
+    lambda {
+      Rack::Multipart.parse_multipart(env)
+    }.must_raise Rack::BadRequest
+  end
+
   it "parse multipart content when content type present but disposition is not" do
     env = Rack::MockRequest.env_for("/", multipart_fixture(:content_type_and_no_disposition))
     params = Rack::Multipart.parse_multipart(env)

test/spec_request.rb

@@ -696,11 +696,6 @@ def initialize(*)
       message.must_equal "invalid %-encoding (a%)"
   end
 
-  it "raise if rack.input is missing" do
-    req = make_request({})
-    lambda { req.POST }.must_raise RuntimeError
-  end
-
   it "parse POST data when method is POST and no content-type given" do
     req = make_request \
       Rack::MockRequest.env_for("/?foo=quux",