Merge commit 'FooBarWidget/master'

lib/rack/handler/fastcgi.rb

@@ -1,6 +1,7 @@
 require 'fcgi'
 require 'socket'
 require 'rack/content_length'
+require 'rack/rewindable_input'
 
 module Rack
   module Handler
@@ -13,38 +14,18 @@ def self.run(app, options={})
         }
       end
 
-      module ProperStream       # :nodoc:
-        def each                # This is missing by default.
-          while line = gets
-            yield line
-          end
-        end
-
-        def read(*args)
-          if args.empty?
-            super || ""           # Empty string on EOF.
-          else
-            super
-          end
-        end
-
-        def rewind
-          0                     # Ignore
-        end
-      end
-
       def self.serve(request, app)
         app = Rack::ContentLength.new(app)
 
         env = request.env
         env.delete "HTTP_CONTENT_LENGTH"
 
-        request.in.extend ProperStream
-
         env["SCRIPT_NAME"] = ""  if env["SCRIPT_NAME"] == "/"
+
+        rack_input = RewindableInput.new(request.in)
 
         env.update({"rack.version" => [0,1],
-                     "rack.input" => request.in,
+                     "rack.input" => rack_input,
                      "rack.errors" => request.err,
 
                      "rack.multithread" => false,
@@ -61,12 +42,16 @@ def self.serve(request, app)
         env.delete "CONTENT_TYPE"  if env["CONTENT_TYPE"] == ""
         env.delete "CONTENT_LENGTH"  if env["CONTENT_LENGTH"] == ""
 
-        status, headers, body = app.call(env)
         begin
-          send_headers request.out, status, headers
-          send_body request.out, body
+          status, headers, body = app.call(env)
+          begin
+            send_headers request.out, status, headers
+            send_body request.out, body
+          ensure
+            body.close  if body.respond_to? :close
+          end
         ensure
-          body.close  if body.respond_to? :close
+          rack_input.close
           request.finish
         end
       end

lib/rack/lint.rb

@@ -231,9 +231,12 @@ def check_env(env)
     end
 
     ## === The Input Stream
+    ##
+    ## The input stream is an IO-like object which contains the raw HTTP
+    ## POST data. If it is a file then it must be opened in binary mode.
     def check_input(input)
-      ## The input stream must respond to +gets+, +each+ and +read+.
-      [:gets, :each, :read].each { |method|
+      ## The input stream must respond to +gets+, +each+, +read+ and +rewind+.
+      [:gets, :each, :read, :rewind].each { |method|
         assert("rack.input #{input} does not respond to ##{method}") {
           input.respond_to? method
         }
@@ -251,10 +254,6 @@ def size
         @input.size
       end
 
-      def rewind
-        @input.rewind
-      end
-
       ## * +gets+ must be called without arguments and return a string,
       ##   or +nil+ on EOF.
       def gets(*args)
@@ -266,21 +265,44 @@ def gets(*args)
         v
       end
 
-      ## * +read+ must be called without or with one integer argument
-      ##   and return a string, or +nil+ on EOF.
+      ## * +read+ behaves like IO#read. Its signature is <tt>read([length, [buffer]])</tt>.
+      ##   If given, +length+ must be an non-negative Integer (>= 0) or +nil+, and +buffer+ must
+      ##   be a String and may not be nil. If +length+ is given and not nil, then this method
+      ##   reads at most +length+ bytes from the input stream. If +length+ is not given or nil,
+      ##   then this method reads all data until EOF.
+      ##   When EOF is reached, this method returns nil if +length+ is given and not nil, or ""
+      ##   if +length+ is not given or is nil.
+      ##   If +buffer+ is given, then the read data will be placed into +buffer+ instead of a
+      ##   newly created String object.
       def read(*args)
         assert("rack.input#read called with too many arguments") {
-          args.size <= 1
+          args.size <= 2
         }
-        if args.size == 1
-          assert("rack.input#read called with non-integer argument") {
-            args.first.kind_of? Integer
+        if args.size >= 1
+          assert("rack.input#read called with non-integer and non-nil length") {
+            args.first.kind_of?(Integer) || args.first.nil?
+          }
+          assert("rack.input#read called with a negative length") {
+            args.first.nil? || args.first >= 0
+          }
+        end
+        if args.size >= 2
+          assert("rack.input#read called with non-String buffer") {
+            args[1].kind_of?(String)
           }
         end
+
         v = @input.read(*args)
-        assert("rack.input#read didn't return a String") {
+
+        assert("rack.input#read didn't return nil or a String") {
           v.nil? or v.instance_of? String
         }
+        if args[0].nil?
+          assert("rack.input#read(nil) returned nil on EOF") {
+            !v.nil?
+          }
+        end
+
         v
       end
 
@@ -294,6 +316,23 @@ def each(*args)
           yield line
         }
       end
+
+      ## * +rewind+ must be called without arguments. It rewinds the input
+      ##   stream back to the beginning. It must not raise Errno::ESPIPE:
+      ##   that is, it may not be a pipe or a socket. Therefore, handler
+      ##   developers must buffer the input data into some rewindable object
+      ##   if the underlying input stream is not rewindable.
+      def rewind(*args)
+        assert("rack.input#rewind called with arguments") { args.size == 0 }
+        assert("rack.input#rewind raised Errno::ESPIPE") {
+          begin
+            @input.rewind
+            true
+          rescue Errno::ESPIPE
+            false
+          end
+        }
+      end
 
       ## * +close+ must never be called on the input stream.
       def close(*args)
@@ -345,13 +384,14 @@ def close(*args)
 
     ## === The Status
     def check_status(status)
-      ## The status, if parsed as integer (+to_i+), must be greater than or equal to 100.
+      ## This is an HTTP status. When parsed as integer (+to_i+), it must be
+      ## greater than or equal to 100.
       assert("Status must be >=100 seen as integer") { status.to_i >= 100 }
     end
 
     ## === The Headers
     def check_headers(header)
-      ## The header must respond to each, and yield values of key and value.
+      ## The header must respond to +each+, and yield values of key and value.
       assert("headers object should respond to #each, but doesn't (got #{header.class} as headers)") {
          header.respond_to? :each
       }
@@ -373,7 +413,8 @@ def check_headers(header)
         ## The values of the header must be Strings,
         assert("a header value must be a String, but the value of " +
           "'#{key}' is a #{value.class}") { value.kind_of? String }
-        ## consisting of lines (for multiple header values) seperated by "\n".
+        ## consisting of lines (for multiple header values, e.g. multiple
+        ## <tt>Set-Cookie</tt> values) seperated by "\n".
         value.split("\n").each { |item|
           ## The lines must not contain characters below 037.
           assert("invalid header value #{key}: #{item.inspect}") {
@@ -445,7 +486,7 @@ def check_content_length(status, headers, env)
     ## === The Body
     def each
       @closed = false
-      ## The Body must respond to #each
+      ## The Body must respond to +each+
       @body.each { |part|
         ## and must only yield String values.
         assert("Body yielded non-string value #{part.inspect}") {
@@ -454,17 +495,17 @@ def each
         yield part
       }
       ##
-      ## The Body should not be an instance of String, as this will
+      ## The Body itself should not be an instance of String, as this will
       ## break in Ruby 1.9.
       ##
-      ## If the Body responds to #close, it will be called after iteration.
+      ## If the Body responds to +close+, it will be called after iteration.
       # XXX howto: assert("Body has not been closed") { @closed }
 
 
       ##
-      ## If the Body responds to #to_path, it must return a String
+      ## If the Body responds to +to_path+, it must return a String
       ## identifying the location of a file whose contents are identical
-      ## to that produced by calling #each.
+      ## to that produced by calling +each+.
 
       if @body.respond_to?(:to_path)
         assert("The file identified by body.to_path does not exist") {

lib/rack/request.rb

@@ -147,12 +147,7 @@ def POST
           @env["rack.request.form_vars"] = form_vars
           @env["rack.request.form_hash"] = Utils.parse_nested_query(form_vars)
 
-          begin
-            @env["rack.input"].rewind if @env["rack.input"].respond_to?(:rewind)
-          rescue Errno::ESPIPE
-            # Handles exceptions raised by input streams that cannot be rewound
-            # such as when using plain CGI under Apache
-          end
+          @env["rack.input"].rewind
         end
         @env["rack.request.form_hash"]
       else

lib/rack/rewindable_input.rb

@@ -0,0 +1,98 @@
+require 'tempfile'
+
+module Rack
+  # Class which can make any IO object rewindable, including non-rewindable ones. It does
+  # this by buffering the data into a tempfile, which is rewindable.
+  #
+  # rack.input is required to be rewindable, so if your input stream IO is non-rewindable
+  # by nature (e.g. a pipe or a socket) then you can wrap it in an object of this class
+  # to easily make it rewindable.
+  #
+  # Don't forget to call #close when you're done. This frees up temporary resources that
+  # RewindableInput uses, though it does *not* close the original IO object.
+  class RewindableInput
+    def initialize(io)
+      @io = io
+      @rewindable_io = nil
+      @unlinked = false
+    end
+
+    def gets
+      make_rewindable unless @rewindable_io
+      @rewindable_io.gets
+    end
+
+    def read(*args)
+      make_rewindable unless @rewindable_io
+      @rewindable_io.read(*args)
+    end
+
+    def each(&block)
+      make_rewindable unless @rewindable_io
+      @rewindable_io.each(&block)
+    end
+
+    def rewind
+      make_rewindable unless @rewindable_io
+      @rewindable_io.rewind
+    end
+
+    # Closes this RewindableInput object without closing the originally
+    # wrapped IO oject. Cleans up any temporary resources that this RewindableInput
+    # has created.
+    #
+    # This method may be called multiple times. It does nothing on subsequent calls.
+    def close
+      if @rewindable_io
+        if @unlinked
+          @rewindable_io.close
+        else
+          @rewindable_io.close!
+        end
+        @rewindable_io = nil
+      end
+    end
+
+    private
+
+    # Ruby's Tempfile class has a bug. Subclass it and fix it.
+    class Tempfile < ::Tempfile
+      def _close
+        @tmpfile.close if @tmpfile
+        @data[1] = nil if @data
+        @tmpfile = nil
+      end
+    end
+
+    def make_rewindable
+      # Buffer all data into a tempfile. Since this tempfile is private to this
+      # RewindableInput object, we chmod it so that nobody else can read or write
+      # it. On POSIX filesystems we also unlink the file so that it doesn't
+      # even have a file entry on the filesystem anymore, though we can still
+      # access it because we have the file handle open.
+      @rewindable_io = Tempfile.new('RackRewindableInput')
+      @rewindable_io.chmod(0000)
+      if filesystem_has_posix_semantics?
+        @rewindable_io.unlink
+        @unlinked = true
+      end
+
+      buffer = ""
+      while @io.read(1024 * 4, buffer)
+        entire_buffer_written_out = false
+        while !entire_buffer_written_out
+          written = @rewindable_io.write(buffer)
+          entire_buffer_written_out = written == buffer.size
+          if !entire_buffer_written_out
+            buffer.slice!(0 .. written - 1)
+          end
+        end
+      end
+      @rewindable_io.rewind
+    end
+
+    def filesystem_has_posix_semantics?
+      RUBY_PLATFORM !~ /(mswin|mingw|cygwin)/
+    end
+  end
+end

lib/rack/utils.rb

@@ -302,13 +302,16 @@ def self.parse_multipart(env)
           buf = ""
           content_length = env['CONTENT_LENGTH'].to_i
           input = env['rack.input']
+          input.rewind
 
           boundary_size = boundary.size + EOL.size
           bufsize = 16384
 
           content_length -= boundary_size
 
-          status = input.read(boundary_size)
+          read_buffer = ''
+
+          status = input.read(boundary_size, read_buffer)
           raise EOFError, "bad content body"  unless status == boundary + EOL
 
           rx = /(?:#{EOL})?#{Regexp.quote boundary}(#{EOL}|--)/n
@@ -340,7 +343,7 @@ def self.parse_multipart(env)
                 body << buf.slice!(0, buf.size - (boundary_size+4))
               end
 
-              c = input.read(bufsize < content_length ? bufsize : content_length)
+              c = input.read(bufsize < content_length ? bufsize : content_length, read_buffer)
               raise EOFError, "bad content body"  if c.nil? || c.empty?
               buf << c
               content_length -= c.size
@@ -384,12 +387,7 @@ def self.parse_multipart(env)
             break  if buf.empty? || content_length == -1
           }
 
-          begin
-            input.rewind if input.respond_to?(:rewind)
-          rescue Errno::ESPIPE
-            # Handles exceptions raised by input streams that cannot be rewound
-            # such as when using plain CGI under Apache
-          end
+          input.rewind
 
           params
         end