diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index 1315c7b37..38ab90fb2 100644 --- a/lib/rack/multipart/parser.rb +++ b/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 diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index a163c49a4..b8ea5a73b 100644 --- a/lib/rack/utils.rb +++ b/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 '&' diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb index bd0b07b68..84087a89a 100644 --- a/test/spec_multipart.rb +++ b/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 diff --git a/test/spec_request.rb b/test/spec_request.rb index 877b16b6c..5c177612d 100644 --- a/test/spec_request.rb +++ b/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 = <