mirror of mongrel-development@rubyforge.org (inactive)
 help / color / mirror / Atom feed
From: Eric Wong <normalperson-rMlxZR9MS24@public.gmane.org>
To: mongrel-development-GrnCvJ7WPxnNLxjTenLetw@public.gmane.org
Subject: Re: [RFC Mongrel2] simpler response API + updated HTTP parser
Date: Wed, 7 Oct 2009 18:35:43 -0700	[thread overview]
Message-ID: <20091008013542.GA12370@dcvr.yhbt.net> (raw)
In-Reply-To: <20090912235729.GA9370-yBiyF41qdooeIZ0/mPfg9Q@public.gmane.org>

Eric Wong <normalperson-rMlxZR9MS24@public.gmane.org> wrote:
> Hi all,
> 
> I've pushed out some changes based on fauna/master[1] to
> git://git.bogomips.org/ur-mongrel that includes a good chunk of the
> platform-independent stuff found in Unicorn.

We hit one bug/weird-interaction with Rails in Unicorn so here's a fix I
put it.  Unfortunately the test cases for TeeInput in Unicorn currently
rely on fork() + pipe() (it was just more natural for me to write), but
if there's interest I could be persuaded to write a non-*nix version.

This issue that could be arguably considered a bug in Rails:
  https://rails.lighthouseapp.com/projects/8994/tickets/3343

Just in case, I'm also asking for Rack to allow the readpartial method
into the "rack.input" spec here:
   http://groups.google.com/group/rack-devel/browse_thread/thread/3dfccb68172a6ed6

>>From 87254d37c519b63a1d39c938cd4a53b08e2a1065 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson-rMlxZR9MS24@public.gmane.org>
Date: Wed, 7 Oct 2009 18:24:27 -0700
Subject: [PATCH] more-compatible TeeInput#read for POSTs with Content-Length

There are existing applications and libraries that don't check
the return value of env['rack.input'].read(length) (like Rails
:x).  Those applications became broken under the IO#readpartial
semantics of TeeInput#read when handling larger request bodies.

We'll preserve the IO#readpartial semantics _only_ when handling
chunked requests (as long as Rack allows it, it's useful for
real-time processing of audio/video streaming uploads,
especially with Rainbows! and mobile clients) but use
read-in-full semantics for TeeInput#read on requests with a
known Content-Length.
---
 lib/mongrel/tee_input.rb |   43 +++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/lib/mongrel/tee_input.rb b/lib/mongrel/tee_input.rb
index 442c55a..3605e20 100644
--- a/lib/mongrel/tee_input.rb
+++ b/lib/mongrel/tee_input.rb
@@ -44,6 +44,26 @@ module Mongrel
       @size = tmp_size
     end
 
+    # call-seq:
+    #   ios = env['rack.input']
+    #   ios.read([length [, buffer ]]) => string, buffer, or nil
+    #
+    # Reads at most length bytes from the I/O stream, or to the end of
+    # file if length is omitted or is nil. length must be a non-negative
+    # integer or nil. If the optional buffer argument is present, it
+    # must reference a String, which will receive the data.
+    #
+    # At end of file, it returns nil or "" depend on length.
+    # ios.read() and ios.read(nil) returns "".
+    # ios.read(length [, buffer]) returns nil.
+    #
+    # If the Content-Length of the HTTP request is known (as is the common
+    # case for POST requests), then ios.read(length [, buffer]) will block
+    # until the specified length is read (or it is the last chunk).
+    # Otherwise, for uncommon "Transfer-Encoding: chunked" requests,
+    # ios.read(length [, buffer]) will return immediately if there is
+    # any data and only block when nothing is available (providing
+    # IO#readpartial semantics).
     def read(*args)
       socket or return @tmp.read(*args)
 
@@ -58,9 +78,9 @@ module Mongrel
         rv = args.shift || @buf2.dup
         diff = tmp_size - @tmp.pos
         if 0 == diff
-          tee(length, rv)
+          ensure_length(tee(length, rv), length)
         else
-          @tmp.read(diff > length ? length : diff, rv)
+          ensure_length(@tmp.read(diff > length ? length : diff, rv), length)
         end
       end
     end
@@ -140,5 +160,24 @@ module Mongrel
       tmp
     end
 
+    # tee()s into +buf+ until it is of +length+ bytes (or until
+    # we've reached the Content-Length of the request body).
+    # Returns +buf+ (the exact object, not a duplicate)
+    # To continue supporting applications that need near-real-time
+    # streaming input bodies, this is a no-op for
+    # "Transfer-Encoding: chunked" requests.
+    def ensure_length(buf, length)
+      # @size is nil for chunked bodies, so we can't ensure length for those
+      # since they could be streaming bidirectionally and we don't want to
+      # block the caller in that case.
+      return buf if buf.nil? || @size.nil?
+
+      while buf.size < length && @size != @tmp.pos
+        buf << tee(length - buf.size, @buf2)
+      end
+
+      buf
+    end
+
   end
 end
-- 
Eric Wong

  parent reply	other threads:[~2009-10-08  1:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-12 23:57 Eric Wong
     [not found] ` <20090912235729.GA9370-yBiyF41qdooeIZ0/mPfg9Q@public.gmane.org>
2009-10-08  1:35   ` Eric Wong [this message]
     [not found]     ` <20091008013542.GA12370-yBiyF41qdooeIZ0/mPfg9Q@public.gmane.org>
2009-10-27 21:59       ` Eric Wong
replies disabled, historical list

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).