about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2009-07-01 13:59:40 -0700
committerEric Wong <normalperson@yhbt.net>2009-07-01 14:12:07 -0700
commit06bf73975864b8e16ef1ee977f8424a0e5517fd6 (patch)
treeedb02fb04b9edf26dc7a8b9add0c5e5c6e43d8aa
parentec5d374768ced6aba3fed8a9481d2ac3c07cdb98 (diff)
downloadunicorn-06bf73975864b8e16ef1ee977f8424a0e5517fd6.tar.gz
This change gives applications full control to deny clients
from uploading unwanted message bodies.  This also paves the
way for doing things like upload progress notification within
applications in a Rack::Lint-compatible manner.

Since we don't support HTTP keepalive, so we have more freedom
here by being able to close TCP connections and deny clients the
ability to write to us (and thus wasting our bandwidth).

While I could've left this feature off by default indefinitely
for maximum backwards compatibility (for arguably broken
applications), Unicorn is not and has never been about
supporting the lowest common denominator.
-rw-r--r--TODO4
-rw-r--r--examples/echo.ru1
-rw-r--r--lib/unicorn/app/inetd.rb2
-rw-r--r--lib/unicorn/configurator.rb27
-rw-r--r--lib/unicorn/const.rb1
-rw-r--r--lib/unicorn/http_request.rb3
-rw-r--r--lib/unicorn/tee_input.rb18
-rw-r--r--test/unit/test_configurator.rb2
-rw-r--r--test/unit/test_upload.rb19
9 files changed, 18 insertions, 59 deletions
diff --git a/TODO b/TODO
index 7e36cc2..df0d06e 100644
--- a/TODO
+++ b/TODO
@@ -1,7 +1,3 @@
-* Support HTTP/1.1 keepalive if (and probably only if) pipelining.
-  We can do this by testing readability of socket immediately after
-  the response is written.
-
 * integration tests with nginx including bad client handling
 
 * manpages (why do so few Ruby executables come with proper manpages?)
diff --git a/examples/echo.ru b/examples/echo.ru
index e13721a..ebc4d9d 100644
--- a/examples/echo.ru
+++ b/examples/echo.ru
@@ -7,7 +7,6 @@
 #
 # Then type random stuff in your terminal to watch it get echoed back!
 
-Unicorn::HttpRequest::DEFAULTS["unicorn.stream_input"] = true
 class EchoBody
   def initialize(input)
     @input = input
diff --git a/lib/unicorn/app/inetd.rb b/lib/unicorn/app/inetd.rb
index c3b8bbc..e22b308 100644
--- a/lib/unicorn/app/inetd.rb
+++ b/lib/unicorn/app/inetd.rb
@@ -91,8 +91,6 @@ module Unicorn::App
     end
 
     def initialize(*cmd)
-      # enable streaming input mode in Unicorn
-      Unicorn::HttpRequest::DEFAULTS["unicorn.stream_input"] = true
       @cmd = cmd
     end
 
diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb
index 1f18515..bd0a198 100644
--- a/lib/unicorn/configurator.rb
+++ b/lib/unicorn/configurator.rb
@@ -44,7 +44,6 @@ module Unicorn
       :preload_app => false,
       :stderr_path => nil,
       :stdout_path => nil,
-      :stream_input => false,
     }
 
     attr_reader :config_file #:nodoc:
@@ -65,10 +64,6 @@ module Unicorn
 
     def commit!(server, options = {}) #:nodoc:
       skip = options[:skip] || []
-      stream_input = @set.delete(:stream_input)
-      unless stream_input.nil?
-        Unicorn::HttpRequest::DEFAULTS[Const::STREAM_INPUT] = stream_input
-      end
       @set.each do |key, value|
         value == :unset and next
         skip.include?(key) and next
@@ -276,28 +271,6 @@ module Unicorn
       end
     end
 
-    # Allow applications to stream input as it is being read from the
-    # network directly to the application.  Enabling this can allow
-    # real-time processing of request bodies as it is being sent by
-    # the client, useful for things like upload progress notification
-    # and tunneling arbitrary stream protocols via bidirectional chunked
-    # transfer encoding.
-    # This may not work with all applications because some broken
-    # applications assume env['rack.input'].read(size) always returns
-    # the requested amount of data.  This causes env['rack.input']#read
-    # to provide IO#readpartial semantics instead.  Some applications
-    # may also fully receive an input and never attempt to process it,
-    # causing clients confusion when they receive a response after
-    # only a partial request has been sent.
-    def stream_input(bool)
-      case bool
-      when TrueClass, FalseClass
-        @set[:stream_input] = bool
-      else
-        raise ArgumentError, "stream_input=#{bool.inspect} not a boolean"
-      end
-    end
-
     # Allow redirecting $stderr to a given path.  Unlike doing this from
     # the shell, this allows the unicorn process to know the path its
     # writing to and rotate the file if it is used for logging.  The
diff --git a/lib/unicorn/const.rb b/lib/unicorn/const.rb
index be69753..be23bb4 100644
--- a/lib/unicorn/const.rb
+++ b/lib/unicorn/const.rb
@@ -34,7 +34,6 @@ module Unicorn
     HTTP_EXPECT="HTTP_EXPECT".freeze
     HTTP_TRAILER="HTTP_TRAILER".freeze
     RACK_INPUT="rack.input".freeze
-    STREAM_INPUT="unicorn.stream_input".freeze
   end
 
 end
diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb
index ad1e23f..779cd32 100644
--- a/lib/unicorn/http_request.rb
+++ b/lib/unicorn/http_request.rb
@@ -98,8 +98,7 @@ module Unicorn
           end
         end
 
-        inp = TeeInput.new(socket, length, body)
-        DEFAULTS[Const::STREAM_INPUT] ? inp : inp.consume
+        TeeInput.new(socket, length, body)
       else
         NULL_IO.closed? ? NULL_IO.reopen(Z) : NULL_IO
       end
diff --git a/lib/unicorn/tee_input.rb b/lib/unicorn/tee_input.rb
index 8dd8954..06028a6 100644
--- a/lib/unicorn/tee_input.rb
+++ b/lib/unicorn/tee_input.rb
@@ -29,22 +29,20 @@ module Unicorn
       @size = size # nil if chunked
     end
 
-    def consume
-      @input or return
-      buf = Z.dup
-      while tee(Const::CHUNK_SIZE, buf)
-      end
-      @tmp.rewind
-      self
-    end
-
     # returns the size of the input.  This is what the Content-Length
     # header value should be, and how large our input is expected to be.
     # For TE:chunked, this requires consuming all of the input stream
     # before returning since there's no other way
     def size
       @size and return @size
-      @input and consume
+
+      if @input
+        buf = Z.dup
+        while tee(Const::CHUNK_SIZE, buf)
+        end
+        @tmp.rewind
+      end
+
       @size = @tmp.stat.size
     end
 
diff --git a/test/unit/test_configurator.rb b/test/unit/test_configurator.rb
index f836647..bcdd2f5 100644
--- a/test/unit/test_configurator.rb
+++ b/test/unit/test_configurator.rb
@@ -53,7 +53,6 @@ class TestConfigurator < Test::Unit::TestCase
     cfg = Unicorn::Configurator.new(:use_defaults => true)
     assert_nothing_raised { cfg.commit!(self) }
     Unicorn::Configurator::DEFAULTS.each do |key,value|
-      next if key == :stream_input
       assert_equal value, instance_variable_get("@#{key.to_s}")
     end
   end
@@ -65,7 +64,6 @@ class TestConfigurator < Test::Unit::TestCase
     @logger = nil
     Unicorn::Configurator::DEFAULTS.each do |key,value|
       next if skip.include?(key)
-      next if key == :stream_input
       assert_equal value, instance_variable_get("@#{key.to_s}")
     end
     assert_nil @logger
diff --git a/test/unit/test_upload.rb b/test/unit/test_upload.rb
index 37baa30..dad5825 100644
--- a/test/unit/test_upload.rb
+++ b/test/unit/test_upload.rb
@@ -142,20 +142,19 @@ class UploadTest < Test::Unit::TestCase
   end
 
   def test_put_excessive_overwrite_closed
-    start_server(lambda { |env| [ 200, @hdr, [] ] })
+    start_server(lambda { |env|
+      while env['rack.input'].read(65536); end
+      [ 200, @hdr, [] ]
+    })
     sock = TCPSocket.new(@addr, @port)
     buf = ' ' * @bs
     sock.syswrite("PUT / HTTP/1.0\r\nContent-Length: #{length}\r\n\r\n")
-    if Unicorn::HttpRequest::DEFAULTS['unicorn.stream_input']
-      assert_raise(Errno::ECONNRESET, Errno::EPIPE) do
-        @count.times { sock.syswrite(buf) }
-      end
-    else
-      @count.times { sock.syswrite(buf) }
-      assert_raise(Errno::ECONNRESET, Errno::EPIPE) do
-        ::Unicorn::Const::CHUNK_SIZE.times { sock.syswrite(buf) }
-      end
+
+    @count.times { sock.syswrite(buf) }
+    assert_raise(Errno::ECONNRESET, Errno::EPIPE) do
+      ::Unicorn::Const::CHUNK_SIZE.times { sock.syswrite(buf) }
     end
+    assert_equal "HTTP/1.1 200 OK\r\n", sock.gets
   end
 
   # Despite reading numerous articles and inspecting the 1.9.1-p0 C