Merge branch 'codekitchen-streaming-multipart'

Closes #639

5 files changed, 58 insertions, 8 deletions

diff --git a/SPEC b/SPEC
index 0deb57b5..5b665955 100644
--- a/SPEC
+++ b/SPEC
@@ -112,6 +112,8 @@ be implemented by the server.
                         warn(message, &block)
                         error(message, &block)
                         fatal(message, &block)
+<tt>rack.multipart.buffer_size</tt>:: An Integer hint to the multipart parser as to what chunk size to use for reads and writes.
+<tt>rack.multipart.tempfile_factory</tt>:: An object responding to #call with two arguments, the filename and content_type given for the multipart form field, and returning an IO-like object that responds to #<< and optionally #rewind. This factory will be used to instantiate the tempfile for each multipart form file upload field, rather than the default class of Tempfile.
 The server or the application can store their own data in the
 environment, too.  The keys must contain at least one dot,
 and should be prefixed uniquely.  The prefix <tt>rack.</tt>
diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb
index 5cdef3f8..0fcdc5c4 100644
--- a/lib/rack/lint.rb
+++ b/lib/rack/lint.rb
@@ -228,6 +228,23 @@ module Rack
         }
       end
 
+      ## <tt>rack.multipart.buffer_size</tt>:: An Integer hint to the multipart parser as to what chunk size to use for reads and writes.
+      if bufsize = env['rack.multipart.buffer_size']
+        assert("rack.multipart.buffer_size must be an Integer > 0 if specified") {
+          bufsize.is_a?(Integer) && bufsize > 0
+        }
+      end
+
+      ## <tt>rack.multipart.tempfile_factory</tt>:: An object responding to #call with two arguments, the filename and content_type given for the multipart form field, and returning an IO-like object that responds to #<< and optionally #rewind. This factory will be used to instantiate the tempfile for each multipart form file upload field, rather than the default class of Tempfile.
+      if tempfile_factory = env['rack.multipart.tempfile_factory']
+        assert("rack.multipart.tempfile_factory must respond to #call") { tempfile_factory.respond_to?(:call) }
+        env['rack.multipart.tempfile_factory'] = lambda do |filename, content_type|
+          io = tempfile_factory.call(filename, content_type)
+          assert("rack.multipart.tempfile_factory return value must respond to #<<") { io.respond_to?(:<<) }
+          io
+        end
+      end
+
       ## The server or the application can store their own data in the
       ## environment, too.  The keys must contain at least one dot,
       ## and should be prefixed uniquely.  The prefix <tt>rack.</tt>
diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb
index fa90ca96..faa98c7e 100644
--- a/lib/rack/multipart/parser.rb
+++ b/lib/rack/multipart/parser.rb
@@ -18,10 +18,13 @@ module Rack
         content_length = env['CONTENT_LENGTH']
         content_length = content_length.to_i if content_length
 
-        new($1, io, content_length, env)
+        tempfile = env['rack.multipart.tempfile_factory'] || lambda { |filename, content_type| Tempfile.new("RackMultipart") }
+        bufsize = env['rack.multipart.buffer_size'] || BUFSIZE
+
+        new($1, io, content_length, env, tempfile, bufsize)
       end
 
-      def initialize(boundary, io, content_length, env)
+      def initialize(boundary, io, content_length, env, tempfile, bufsize)
         @buf            = ""
 
         if @buf.respond_to? :force_encoding
@@ -34,6 +37,8 @@ module Rack
         @content_length = content_length
         @boundary_size  = Utils.bytesize(@boundary) + EOL.size
         @env = env
+        @tempfile       = tempfile
+        @bufsize        = bufsize
 
         if @content_length
           @content_length -= @boundary_size
@@ -86,7 +91,7 @@ module Rack
 
       def fast_forward_to_first_boundary
         loop do
-          content = @io.read(BUFSIZE)
+          content = @io.read(@bufsize)
           raise EOFError, "bad content body" unless content
           @buf << content
 
@@ -95,7 +100,7 @@ module Rack
             return if read_buffer == full_boundary
           end
 
-          raise EOFError, "bad content body" if Utils.bytesize(@buf) >= BUFSIZE
+          raise EOFError, "bad content body" if Utils.bytesize(@buf) >= @bufsize
         end
       end
 
@@ -125,8 +130,7 @@ module Rack
             end
 
             if filename
-              extname = ::File.extname(filename)
-              (@env['rack.tempfiles'] ||= []) << body = Tempfile.new(["RackMultipart", extname])
+              (@env['rack.tempfiles'] ||= []) << body = @tempfile.call(filename, content_type)
               body.binmode  if body.respond_to?(:binmode)
             end
 
@@ -138,7 +142,7 @@ module Rack
             body << @buf.slice!(0, @buf.size - (@boundary_size+4))
           end
 
-          content = @io.read(@content_length && BUFSIZE >= @content_length ? @content_length : BUFSIZE)
+          content = @io.read(@content_length && @bufsize >= @content_length ? @content_length : @bufsize)
           raise EOFError, "bad content body"  if content.nil? || content.empty?
 
           @buf << content
@@ -223,7 +227,7 @@ module Rack
           # filename is blank which means no file has been selected
           return
         elsif filename
-          body.rewind
+          body.rewind if body.respond_to?(:rewind)
 
           # Take the basename of the upload's original filename.
           # This handles the full Windows paths given by Internet Explorer
diff --git a/test/spec_lint.rb b/test/spec_lint.rb
index fb60b7ef..4686a509 100644
--- a/test/spec_lint.rb
+++ b/test/spec_lint.rb
@@ -1,4 +1,5 @@
 require 'stringio'
+require 'tempfile'
 require 'rack/lint'
 require 'rack/mock'
 
@@ -75,6 +76,23 @@ describe Rack::Lint do
       message.should.equal("logger [] must respond to info")
 
     lambda {
+      Rack::Lint.new(nil).call(env("rack.multipart.buffer_size" => 0))
+    }.should.raise(Rack::Lint::LintError).
+      message.should.equal("rack.multipart.buffer_size must be an Integer > 0 if specified")
+
+    lambda {
+      Rack::Lint.new(nil).call(env("rack.multipart.tempfile_factory" => Tempfile))
+    }.should.raise(Rack::Lint::LintError).
+      message.should.equal("rack.multipart.tempfile_factory must respond to #call")
+
+    lambda {
+      Rack::Lint.new(lambda { |env|
+        env['rack.multipart.tempfile_factory'].call("testfile", "text/plain")
+      }).call(env("rack.multipart.tempfile_factory" => lambda { |filename, content_type| Object.new }))
+    }.should.raise(Rack::Lint::LintError).
+      message.should.equal("rack.multipart.tempfile_factory return value must respond to #<<")
+
+    lambda {
       Rack::Lint.new(nil).call(env("REQUEST_METHOD" => "FUCKUP?"))
     }.should.raise(Rack::Lint::LintError).
       message.should.match(/REQUEST_METHOD/)
diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb
index ca733dae..327c6a2a 100644
--- a/test/spec_multipart.rb
+++ b/test/spec_multipart.rb
@@ -171,6 +171,15 @@ describe Rack::Multipart do
     params["file1.txt"][:tempfile].read.should.equal "contents"
   end
 
+  should "parse multipart upload file using custom tempfile class" do
+    env = Rack::MockRequest.env_for("/", multipart_fixture(:text))
+    my_tempfile = ""
+    env['rack.multipart.tempfile_factory'] = lambda { |filename, content_type| my_tempfile }
+    params = Rack::Multipart.parse_multipart(env)
+    params["files"][:tempfile].object_id.should.equal my_tempfile.object_id
+    my_tempfile.should.equal "contents"
+  end
+
   should "parse multipart upload with nested parameters" do
     env = Rack::MockRequest.env_for("/", multipart_fixture(:nested))
     params = Rack::Multipart.parse_multipart(env)