POST Body Truncated

POST Body Truncated

[Chris Wanstrath]

Hello list,

I'm running into an issue where POST bodies are truncated by the time
they hit Rails. It occurs with both TCP and UDS (both behind nginx).

This problem does not happen when I'm running TCP Thin behind nginx.

I have Unicorn 0.93.1 installed. I've tried tweaking the rcvbuf
setting but it seems to do nothing.

Any ideas? Thanks!

-- 
Chris Wanstrath
http://github.com/defunkt

Re: POST Body Truncated

[Eric Wong]

Chris Wanstrath <chris@ozmm.org> wrote:
> Hello list,
> 
> I'm running into an issue where POST bodies are truncated by the time
> they hit Rails. It occurs with both TCP and UDS (both behind nginx).
> 
> This problem does not happen when I'm running TCP Thin behind nginx.
> 
> I have Unicorn 0.93.1 installed. I've tried tweaking the rcvbuf
> setting but it seems to do nothing.
> 
> Any ideas? Thanks!

This is not good :x

It could be a bug in the relatively new TeeInput class.  How large are
the POST bodies that are getting truncated?  Which version of Rails is
this and which middleware(s), if any, are you running?  I'd like to get
to the bottom of this ASAP, thanks!

-- 
Eric Wong

Re: POST Body Truncated

[Chris Wanstrath]

On Tue, Oct 6, 2009 at 2:22 PM, Eric Wong <normalperson@yhbt.net> wrote:

> This is not good :x
>
> It could be a bug in the relatively new TeeInput class.  How large are
> the POST bodies that are getting truncated?  Which version of Rails is
> this and which middleware(s), if any, are you running?  I'd like to get
> to the bottom of this ASAP, thanks!

My current test payload is 25740 bytes, consistently truncated to
12945 bytes when using Unicorn with TCP.

We're on Rails 2.2.2 - no middlewares, no Rack.

-- 
Chris Wanstrath
http://github.com/defunkt

Re: POST Body Truncated

[Eric Wong]

Chris Wanstrath <chris@ozmm.org> wrote:
> On Tue, Oct 6, 2009 at 2:22 PM, Eric Wong <normalperson@yhbt.net> wrote:
> 
> > This is not good :x
> >
> > It could be a bug in the relatively new TeeInput class.  How large are
> > the POST bodies that are getting truncated?  Which version of Rails is
> > this and which middleware(s), if any, are you running?  I'd like to get
> > to the bottom of this ASAP, thanks!
> 
> My current test payload is 25740 bytes, consistently truncated to
> 12945 bytes when using Unicorn with TCP.
> 
> We're on Rails 2.2.2 - no middlewares, no Rack.

Thanks.  I'll take a look.  Do you have any example controllers/actions
to reproduce this with so I can track this down easier?  (I completely
understand if you can't share secrets, too :).  Thanks.

-- 
Eric Wong

Re: POST Body Truncated

[Chris Wanstrath]

On Tue, Oct 6, 2009 at 2:30 PM, Eric Wong <normalperson@yhbt.net> wrote:

> Thanks.  I'll take a look.  Do you have any example controllers/actions
> to reproduce this with so I can track this down easier?  (I completely
> understand if you can't share secrets, too :).  Thanks.

It happens everywhere for us - big comments, big wikis, big file
edits, big gists.

I'm not sure what code I could share because it happens between the
browser sending a POST and Rails actually receiving the request. Let
me know if you have a suggestion for what you want and I'll whip it
up.

-- 
Chris Wanstrath
http://github.com/defunkt

Re: POST Body Truncated

[Eric Wong]

Chris Wanstrath <chris@ozmm.org> wrote:
> On Tue, Oct 6, 2009 at 2:30 PM, Eric Wong <normalperson@yhbt.net> wrote:
> 
> > Thanks.  I'll take a look.  Do you have any example controllers/actions
> > to reproduce this with so I can track this down easier?  (I completely
> > understand if you can't share secrets, too :).  Thanks.
> 
> It happens everywhere for us - big comments, big wikis, big file
> edits, big gists.
> 
> I'm not sure what code I could share because it happens between the
> browser sending a POST and Rails actually receiving the request. Let
> me know if you have a suggestion for what you want and I'll whip it
> up.

No worries, I'm working on a a reproducible test case.  Can you try
and see if you're using curl's -F to POST, you _don't_ hit it?

Thanks.

-- 
Eric Wong

Re: POST Body Truncated

[Chris Wanstrath]

On Tue, Oct 6, 2009 at 2:50 PM, Eric Wong <normalperson@yhbt.net> wrote:

> No worries, I'm working on a a reproducible test case.  Can you try
> and see if you're using curl's -F to POST, you _don't_ hit it?

`curl -F` does NOT trigger POST body truncation.

-- 
Chris Wanstrath
http://github.com/defunkt

Re: POST Body Truncated

[Eric Wong]

Chris Wanstrath <chris@ozmm.org> wrote:
> On Tue, Oct 6, 2009 at 2:50 PM, Eric Wong <normalperson@yhbt.net> wrote:
> 
> > No worries, I'm working on a a reproducible test case.  Can you try
> > and see if you're using curl's -F to POST, you _don't_ hit it?
> 
> `curl -F` does NOT trigger POST body truncation.

Still trying to reproduce this, I think you're hitting a buffer boundary
in nginx and triggering a bug in Unicorn::TeeInput somewhere.   You may
be able to increase some userspace buffer sizes in nginx to
workaround it until I (or somebody else) can fix it (hopefully very soon
now).

-- 
Eric Wong

Re: POST Body Truncated

[Eric Wong]

Eric Wong <normalperson@yhbt.net> wrote:
> Chris Wanstrath <chris@ozmm.org> wrote:
> > On Tue, Oct 6, 2009 at 2:50 PM, Eric Wong <normalperson@yhbt.net> wrote:
> > 
> > > No worries, I'm working on a a reproducible test case.  Can you try
> > > and see if you're using curl's -F to POST, you _don't_ hit it?
> > 
> > `curl -F` does NOT trigger POST body truncation.
> 
> Still trying to reproduce this, I think you're hitting a buffer boundary
> in nginx and triggering a bug in Unicorn::TeeInput somewhere.   You may
> be able to increase some userspace buffer sizes in nginx to
> workaround it until I (or somebody else) can fix it (hopefully very soon
> now).

OK, here's a workaround that should work for now.  I have to hit the
road in a few minutes but will be back on a computer in a few hours.

This only affects older Rails (and I'm supposed to still be supporting
1.2.x!) and its interaction with a wrapped CGI.stdinput somewhere is
going bad...

diff --git a/lib/unicorn/app/old_rails.rb b/lib/unicorn/app/old_rails.rb
index ba1260a..51bda52 100644
--- a/lib/unicorn/app/old_rails.rb
+++ b/lib/unicorn/app/old_rails.rb
@@ -14,6 +14,8 @@ module Unicorn; module App; end; end
 class Unicorn::App::OldRails
 
   def call(env)
+    env['rack.input'].each { |x| }
+    env['rack.input'].rewind
     cgi = Unicorn::CGIWrapper.new(env)
     begin
       Dispatcher.dispatch(cgi,
---

I'll have a better diagnosis+fix+test when I return.
Sorry for the bug :x

-- 
Eric Wong

Re: POST Body Truncated

[Chris Wanstrath]

On Tue, Oct 6, 2009 at 3:52 PM, Eric Wong <normalperson@yhbt.net> wrote:

> OK, here's a workaround that should work for now.  I have to hit the
> road in a few minutes but will be back on a computer in a few hours.
>
> This only affects older Rails (and I'm supposed to still be supporting
> 1.2.x!) and its interaction with a wrapped CGI.stdinput somewhere is
> going bad...

This works!

I suppose we should upgrade to a newer Rails :)

As usual, thanks a million.

-- 
Chris Wanstrath
http://github.com/defunkt

Re: POST Body Truncated

[Eric Wong]

Chris Wanstrath <chris@ozmm.org> wrote:
> On Tue, Oct 6, 2009 at 3:52 PM, Eric Wong <normalperson@yhbt.net> wrote:
> 
> > OK, here's a workaround that should work for now.  I have to hit the
> > road in a few minutes but will be back on a computer in a few hours.
> >
> > This only affects older Rails (and I'm supposed to still be supporting
> > 1.2.x!) and its interaction with a wrapped CGI.stdinput somewhere is
> > going bad...
> 
> This works!
> 
> I suppose we should upgrade to a newer Rails :)
> 
> As usual, thanks a million.

Here's a real patch with lots of documentation I just pushed out, still
working on automated test cases.  Can you let me know how it works?
Thanks.

>From 438c99aec2d74489fa89b3a6c60d1fb41bb2f7e6 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Tue, 6 Oct 2009 19:45:05 -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/unicorn/tee_input.rb |   43 +++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/lib/unicorn/tee_input.rb b/lib/unicorn/tee_input.rb
index 96a053a..188e2ea 100644
--- a/lib/unicorn/tee_input.rb
+++ b/lib/unicorn/tee_input.rb
@@ -41,6 +41,26 @@ module Unicorn
       @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)
 
@@ -55,9 +75,9 @@ module Unicorn
         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
@@ -130,5 +150,24 @@ module Unicorn
       StringIO === @tmp ? @tmp.size : @tmp.stat.size
     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