You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Garrett Rooney <ro...@electricjellyfish.net> on 2005/12/30 00:08:58 UTC

[PATCH] mod_proxy_fcgi - handle content lengths larger than AP_IOBUFSIZE

Here's a very lightly tested patch to allow mod_proxy_fcgi to deal
with fastcgi records with content length greater than AP_IOBUFSIZE. 
If someone could double check the math to make sure it's correct in
all cases I'd appreciate it, I tested it by reducing the buffers to
very small sizes, and it seems to work fine.

Log follows, patch attached, comments welcome as always.

-garrett

Handle reading fastcgi records with content length larger than AP_IOBUFSIZE.

* modules/proxy/mod_proxy_fcgi.c
  (proxy_fcgi_baton_t): New struct, holds per-connection data.
  (dispatch): Set buckets aside into the scratch pool in the baton, clearing
   it when we pass the brigade on.  Deal with the case where the content
   length is larger than AP_IOBUFSIZE.  Consistently use sizeof when referring
   to the length of buffers.
  (proxy_fcgi_handler): Create our baton and stash it in the connection's
   data member.

Re: [PATCH] mod_proxy_fcgi - handle content lengths larger than AP_IOBUFSIZE

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 12/30/05, Garrett Rooney <ro...@electricjellyfish.net> wrote:
> On 12/29/05, Garrett Rooney <ro...@electricjellyfish.net> wrote:
> > Here's a very lightly tested patch to allow mod_proxy_fcgi to deal
> > with fastcgi records with content length greater than AP_IOBUFSIZE.
> > If someone could double check the math to make sure it's correct in
> > all cases I'd appreciate it, I tested it by reducing the buffers to
> > very small sizes, and it seems to work fine.
>
> Ok, since Paul apparently saw some trouble with the django tutorial
> app in the first version of this patch, here's an updated version.  It
> specifically null terminates the read buffer before checking for the
> end of headers, and switches to using readbuflen explicitly in the
> calculations for figuring out how much we have left to read.
>
> I tested this with a large variety of different buffer sizes, and the
> only known problem is that with certain cases we fail to find the end
> of the headers (when the \r\n\r\n falls on the edge between two
> reads), but that problem was already known.  So if there's a problem
> other than that I'd love to get a small test case for it.

There still appears to be something funky in the logic for reading the
content and the padding in a single recv call...  For now, here's a
third version that just puts off reading the padding till after the
content is done.  We can revisit this and reduce the number of reads
later on when it becomes an issue, especially since not all fcgi
libraries even use padding bytes.

-garrett

Handle reading fastcgi records with content length larger than AP_IOBUFSIZE.

* modules/proxy/mod_proxy_fcgi.c
  (proxy_fcgi_baton_t): New struct, holds per-connection data.
  (dispatch): Set buckets aside into the scratch pool in the baton,
   clearing it when we pass the baton on.  Deal with the case where
   the content length is larger than AP_IOBUFSIZE.  Consistently use
   sizeof when referring to the length of buffers.  Explicitly null
   terminate the read buffer after reading.  Read the padding bytes
   in a second pass to simplify logic.
  (proxy_fcgi_handler): Create our baton and stash it in the connection's
   data member.

Re: [PATCH] mod_proxy_fcgi - handle content lengths larger than AP_IOBUFSIZE

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 12/29/05, Garrett Rooney <ro...@electricjellyfish.net> wrote:
> Here's a very lightly tested patch to allow mod_proxy_fcgi to deal
> with fastcgi records with content length greater than AP_IOBUFSIZE.
> If someone could double check the math to make sure it's correct in
> all cases I'd appreciate it, I tested it by reducing the buffers to
> very small sizes, and it seems to work fine.

Ok, since Paul apparently saw some trouble with the django tutorial
app in the first version of this patch, here's an updated version.  It
specifically null terminates the read buffer before checking for the
end of headers, and switches to using readbuflen explicitly in the
calculations for figuring out how much we have left to read.

I tested this with a large variety of different buffer sizes, and the
only known problem is that with certain cases we fail to find the end
of the headers (when the \r\n\r\n falls on the edge between two
reads), but that problem was already known.  So if there's a problem
other than that I'd love to get a small test case for it.

-garrett

Handle reading fastcgi records with content length larger than AP_IOBUFSIZE.

* modules/proxy/mod_proxy_fcgi.c
  (proxy_fcgi_baton_t): New struct, holds per-connection data.
  (dispatch): Set buckets aside into the scratch pool in the baton,
   clearing it when we pass the brigade on.  Deal with the case where
   the content length is larger than AP_IOBUFSIZE.  Consistently use
   sizeof when referring to the length of buffers.  Explicitly null
   terminate the read buffer after reading.
  (proxy_fcgi_handler): Create our baton and stash it in the connection's
   data member.