unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: John Smart <smartj@gmail.com>
Cc: unicorn-public@bogomips.org
Subject: Re: Rack::Request#params EOFError
Date: Tue, 21 Mar 2017 19:06:39 +0000	[thread overview]
Message-ID: <20170321190639.GA13849@whir> (raw)
In-Reply-To: <CALPE1+VypRNKgC3_RitSkZPrRGFHfjY+-33BD3AkFy53Y-rQ=g@mail.gmail.com>

John Smart <smartj@gmail.com> wrote:
> I found an interesting bug today.  When sending a form multipart >
> 112KB, I noticed that Rack::Request#params was empty. This only occurs
> when using Unicorn, and does not occur when using Thin.
> 
> I dug into Rack source and noticed that any EOFError raised while
> reading the body input stream will cause the post body input stream to
> be ignored:
> 
> https://github.com/rack/rack/blob/cabaa58fe6ac355623746e287475af88c9395d66/lib/rack/request.rb#L357

EOFError should not happen with normal operations supported by
env['rack.input'] (read, gets, each, rewind).

So I'm not sure why rack does not let EOFError propagate up the
stack...

The equivalent Ruby methods (IO#{read,gets,each,rewind})
do not raise EOFError; read and gets should return nil on EOF...

> When multipart > 112KB, I noticed that Unicorn tees the input stream
> to a temporary file.  I was wondering: might Unicorn::TeeInput raise
> an EOFError as part of normal operation when reaching the end of the
> input stream?  If so, this would break the Rack spec.  I only tested
> this on Darwin, still working on a self-contained repro.

No, it should not raise EOFError unless a client sent less than
the Content-Length it declared in the header, or if it sent a
short chunk with "Transfer-Encoding: chunked".

EOFError should be raised to break out of the application
processing entirely if and only if the client decides to disconnect
prematurely.  This is needed to allow unicorn to move onto other
clients.

What unicorn could (and maybe should) do is raise a different
error which is not a subclass of EOFError; to prevent the error
from being caught by Rack (or any other middlewares).

What client are you using?

Is it sending "Transfer-Encoding: chunked" or a Content-Length?

Is nginx in front of unicorn?  If not, does it happen when nginx
is in front of unicorn?

> I ended up raising client_body_buffer_size to 1MB as a work-around.
> I'm wondering if this is a bug?

Not sure, yet.  unicorn isn't meant to handle unreliable
clients; it's designed to talk to nginx.

Thanks.

  reply	other threads:[~2017-03-21 19:06 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-21 18:44 Rack::Request#params EOFError John Smart
2017-03-21 19:06 ` Eric Wong [this message]
2017-03-26 20:52   ` Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://yhbt.net/unicorn/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170321190639.GA13849@whir \
    --to=e@80x24.org \
    --cc=smartj@gmail.com \
    --cc=unicorn-public@bogomips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://yhbt.net/unicorn.git/

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).