httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Garrett Rooney <roo...@electricjellyfish.net>
Subject Re: fcgi
Date Wed, 04 Jan 2006 15:38:46 GMT
On 1/4/06, Jim Jagielski <jim@jagunet.com> wrote:
>
> On Jan 4, 2006, at 4:32 AM, Ian Holsman wrote:
>
> > I'm not sure why we aren't just reading the plen at the same time
> > as the clen... but as is when the 2nd header is read, it is not in
> > sync (out by padding-len bytes)
> >
> > this patch makes it read at the same time, and it seems to make the
> > handler work for larger responses (as the following header is now
> > synced up properly)
> >
>
> +1

The reason it isn't reading it all at once is that it's difficult to
get all the cases correct.  This patch, for example, does not get it
right in all cases, if clen + plen > the amount read, but clen <
amount read, you aren't subtracting the remainder off of plen, so the
next read will get too much data and you'll be off again.  I had a
version of this code in one of my patches that Paul didn't commit,
because while it tried really hard to get all the cases right it
screwed up some of them.  See the list archives from last week for my
attempts if you want someplace to start.

I'd really like a better explenation of exactly what case we're
hitting here that's throwing off the parsing.  I suppose it could be
that we're hitting a case where clen == 0 at the end, in which case
you can correct it by moving the cleanup plen read down to the bottom
of the FCGI_STDOUT case in the switch, instead of having it in the
clen != 0 part of the if:

Index: modules/proxy/mod_proxy_fcgi.c
===================================================================
--- modules/proxy/mod_proxy_fcgi.c      (revision 365824)
+++ modules/proxy/mod_proxy_fcgi.c      (working copy)
@@ -574,15 +574,6 @@
                         clen -= readbuflen;
                         goto recv_again;
                     }
-
-                    if (plen) {
-                        readbuflen = plen;
-
-                        rv = apr_socket_recv(conn->sock, readbuf,
&readbuflen);-                        if (rv != APR_SUCCESS) {
-                            break;
-                        }
-                    }
                 } else {
                     b = apr_bucket_eos_create(c->bucket_alloc);

@@ -595,6 +586,15 @@

                     /* XXX Why don't we cleanup here?  (logic from AJP) */
                 }
+
+                if (plen) {
+                    readbuflen = plen;
+
+                    rv = apr_socket_recv(conn->sock, readbuf, &readbuflen);
+                    if (rv != APR_SUCCESS) {
+                        break;
+                    }
+                }
                 break;

             case FCGI_STDERR:

(totally untested, use at your own risk ;-)

-garrett

Mime
View raw message