Explicitly fail when hitting the multipart limit

Signed-off-by: Santiago Pastorino <santiago@wyeworks.com>

Conflicts:
	lib/rack/utils.rb

lib/rack/multipart/parser.rb

@@ -2,6 +2,8 @@
 
 module Rack
   module Multipart
+    class MultipartLimitError < Errno::EMFILE; end
+
     class Parser
       BUFSIZE = 16384
 
@@ -14,7 +16,13 @@ def parse
 
         fast_forward_to_first_boundary
 
+        opened_files = 0
         loop do
+          if Utils.multipart_part_limit > 0
+            raise MultipartLimitError, 'Maximum file multiparts in content reached' if opened_files >= Utils.multipart_part_limit
+            opened_files += 1
+          end
+
           head, filename, content_type, name, body =
             get_current_head_and_filename_and_content_type_and_name_and_body
 

lib/rack/utils.rb

@@ -53,6 +53,7 @@ def unescape(s, encoding = nil)
     class << self
       attr_accessor :key_space_limit
       attr_accessor :param_depth_limit
+      attr_accessor :multipart_part_limit
     end
 
     # The default number of bytes to allow parameter keys to take up.
@@ -62,6 +63,11 @@ class << self
     # Default depth at which the parameter parser will raise an exception for
     # being too deep.  This helps prevent SystemStackErrors
     self.param_depth_limit = 100
+    #
+    # The maximum number of parts a request can contain. Accepting to many part
+    # can lead to the server running out of file handles.
+    # Set to `0` for no limit.
+    self.multipart_part_limit = (ENV['RACK_MULTIPART_PART_LIMIT'] || 128).to_i
 
     # Stolen from Mongrel, with some small modifications:
     # Parses a query string by breaking it up at the '&'

test/spec_multipart.rb

@@ -364,22 +364,29 @@ def rd.length
   end
 
   it "builds complete params with the chunk size of 16384 slicing exactly on boundary" do
-    data = File.open(multipart_file("fail_16384_nofile"), 'rb') { |f| f.read }.gsub(/\n/, "\r\n")
-    options = {
-      "CONTENT_TYPE" => "multipart/form-data; boundary=----WebKitFormBoundaryWsY0GnpbI5U7ztzo",
-      "CONTENT_LENGTH" => data.length.to_s,
-      :input => StringIO.new(data)
-    }
-    env = Rack::MockRequest.env_for("/", options)
-    params = Rack::Multipart.parse_multipart(env)
-
-    params.should.not.equal nil
-    params.keys.should.include "AAAAAAAAAAAAAAAAAAA"
-    params["AAAAAAAAAAAAAAAAAAA"].keys.should.include "PLAPLAPLA_MEMMEMMEMM_ATTRATTRER"
-    params["AAAAAAAAAAAAAAAAAAA"]["PLAPLAPLA_MEMMEMMEMM_ATTRATTRER"].keys.should.include "new"
-    params["AAAAAAAAAAAAAAAAAAA"]["PLAPLAPLA_MEMMEMMEMM_ATTRATTRER"]["new"].keys.should.include "-2"
-    params["AAAAAAAAAAAAAAAAAAA"]["PLAPLAPLA_MEMMEMMEMM_ATTRATTRER"]["new"]["-2"].keys.should.include "ba_unit_id"
-    params["AAAAAAAAAAAAAAAAAAA"]["PLAPLAPLA_MEMMEMMEMM_ATTRATTRER"]["new"]["-2"]["ba_unit_id"].should.equal "1017"
+    begin
+      previous_limit = Rack::Utils.multipart_part_limit
+      Rack::Utils.multipart_part_limit = 256
+
+      data = File.open(multipart_file("fail_16384_nofile"), 'rb') { |f| f.read }.gsub(/\n/, "\r\n")
+      options = {
+        "CONTENT_TYPE" => "multipart/form-data; boundary=----WebKitFormBoundaryWsY0GnpbI5U7ztzo",
+        "CONTENT_LENGTH" => data.length.to_s,
+        :input => StringIO.new(data)
+      }
+      env = Rack::MockRequest.env_for("/", options)
+      params = Rack::Multipart.parse_multipart(env)
+
+      params.should.not.equal nil
+      params.keys.should.include "AAAAAAAAAAAAAAAAAAA"
+      params["AAAAAAAAAAAAAAAAAAA"].keys.should.include "PLAPLAPLA_MEMMEMMEMM_ATTRATTRER"
+      params["AAAAAAAAAAAAAAAAAAA"]["PLAPLAPLA_MEMMEMMEMM_ATTRATTRER"].keys.should.include "new"
+      params["AAAAAAAAAAAAAAAAAAA"]["PLAPLAPLA_MEMMEMMEMM_ATTRATTRER"]["new"].keys.should.include "-2"
+      params["AAAAAAAAAAAAAAAAAAA"]["PLAPLAPLA_MEMMEMMEMM_ATTRATTRER"]["new"]["-2"].keys.should.include "ba_unit_id"
+      params["AAAAAAAAAAAAAAAAAAA"]["PLAPLAPLA_MEMMEMMEMM_ATTRATTRER"]["new"]["-2"]["ba_unit_id"].should.equal "1017"
+    ensure
+      Rack::Utils.multipart_part_limit = previous_limit
+    end
   end
 
   should "return nil if no UploadedFiles were used" do

test/spec_request.rb

@@ -2,6 +2,7 @@
 require 'cgi'
 require 'rack/request'
 require 'rack/mock'
+require 'securerandom'
 
 describe Rack::Request do
   should "wrap the rack variables" do
@@ -718,6 +719,22 @@
     f[:tempfile].size.should.equal 76
   end
 
+  should "MultipartLimitError when request has too many multipart parts if limit set" do
+    begin
+      data = 10000.times.map { "--AaB03x\r\nContent-Type: text/plain\r\nContent-Disposition: attachment; name=#{SecureRandom.hex(10)}; filename=#{SecureRandom.hex(10)}\r\n\r\ncontents\r\n" }.join("\r\n")
+      data += "--AaB03x--\r"
+
+      options = {
+        "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x",
+        "CONTENT_LENGTH" => data.length.to_s,
+        :input => StringIO.new(data)
+      }
+
+      request = Rack::Request.new Rack::MockRequest.env_for("/", options)
+      lambda { request.POST }.should.raise(Rack::Multipart::MultipartLimitError)
+    end
+  end
+
   should "parse big multipart form data" do
     input = <<EOF
 --AaB03x\r