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
Chris Wanstrath <chris at 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
On Tue, Oct 6, 2009 at 2:22 PM, Eric Wong <normalperson at 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
Chris Wanstrath <chris at ozmm.org> wrote:> On Tue, Oct 6, 2009 at 2:22 PM, Eric Wong <normalperson at 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
On Tue, Oct 6, 2009 at 2:30 PM, Eric Wong <normalperson at 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
Chris Wanstrath <chris at ozmm.org> wrote:> On Tue, Oct 6, 2009 at 2:30 PM, Eric Wong <normalperson at 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
On Tue, Oct 6, 2009 at 2:50 PM, Eric Wong <normalperson at 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
Chris Wanstrath <chris at ozmm.org> wrote:> On Tue, Oct 6, 2009 at 2:50 PM, Eric Wong <normalperson at 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
Eric Wong <normalperson at yhbt.net> wrote:> Chris Wanstrath <chris at ozmm.org> wrote: > > On Tue, Oct 6, 2009 at 2:50 PM, Eric Wong <normalperson at 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
On Tue, Oct 6, 2009 at 3:52 PM, Eric Wong <normalperson at 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
Chris Wanstrath <chris at ozmm.org> wrote:> On Tue, Oct 6, 2009 at 3:52 PM, Eric Wong <normalperson at 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 2001From: Eric Wong <normalperson at 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