On 12/29/05, Garrett Rooney <[EMAIL PROTECTED]> 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.
Index: modules/proxy/mod_proxy_fcgi.c
===================================================================
--- modules/proxy/mod_proxy_fcgi.c (revision 359901)
+++ modules/proxy/mod_proxy_fcgi.c (working copy)
@@ -351,11 +351,16 @@
return 0;
}
+typedef struct {
+ apr_pool_t *scratch_pool;
+} proxy_fcgi_baton_t;
+
static apr_status_t dispatch(proxy_conn_rec *conn, request_rec *r,
int request_id)
{
apr_bucket_brigade *ib, *ob;
int seen_end_of_headers = 0, done = 0;
+ proxy_fcgi_baton_t *pfb = conn->data;
apr_status_t rv = APR_SUCCESS;
conn_rec *c = r->connection;
struct iovec vec[2];
@@ -388,7 +393,7 @@
rv = ap_get_brigade(r->input_filters, ib,
AP_MODE_READBYTES, APR_BLOCK_READ,
- AP_IOBUFSIZE);
+ sizeof(writebuf));
if (rv != APR_SUCCESS) {
break;
}
@@ -496,33 +501,29 @@
/* Clear out the header so our buffer is zeroed out again */
memset(readbuf, 0, 8);
- /* XXX We need support for content length > buffer size, but for
- * now just punt. */
+recv_again:
if ((clen + plen) > sizeof(readbuf) - 1) {
- ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
- "proxy: FCGI: back end server send more data "
- "than fits in buffer");
- rv = APR_EINVAL;
- break;
+ readbuflen = sizeof(readbuf) - 1;
+ } else {
+ readbuflen = clen + plen;
}
/* Now get the actual data. Yes it sucks to do this in a second
* recv call, this will eventually change when we move to real
* nonblocking recv calls. */
- if ((clen + plen) != 0) {
- readbuflen = clen + plen;
-
+ if (readbuflen != 0) {
rv = apr_socket_recv(conn->sock, readbuf, &readbuflen);
if (rv != APR_SUCCESS) {
break;
}
+ readbuf[readbuflen] = 0;
}
switch (type) {
case FCGI_STDOUT:
if (clen != 0) {
b = apr_bucket_transient_create(readbuf,
- clen,
+ readbuflen,
c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(ob, b);
@@ -548,11 +549,31 @@
}
apr_brigade_cleanup(ob);
+
+ apr_pool_clear(pfb->scratch_pool);
} else {
/* We're still looking for the end of the headers,
* so this part of the data will need to persist. */
- apr_bucket_setaside(b, r->pool);
+ apr_bucket_setaside(b, pfb->scratch_pool);
}
+
+ /* If we didn't read all the data go back and get the
+ * rest of it. */
+ if ((clen + plen) > readbuflen) {
+ if (! plen) {
+ clen -= readbuflen;
+ }
+ else {
+ if (clen > readbuflen) {
+ clen -= readbuflen;
+ }
+ else {
+ plen -= (readbuflen - clen);
+ clen = 0;
+ }
+ }
+ goto recv_again;
+ }
} else {
b = apr_bucket_eos_create(c->bucket_alloc);
@@ -699,6 +720,14 @@
}
return status;
}
+
+ {
+ proxy_fcgi_baton_t *pfb = apr_pcalloc(r->pool, sizeof(*pfb));
+
+ apr_pool_create(&pfb->scratch_pool, r->pool);
+
+ backend->data = pfb;
+ }
}
backend->is_ssl = 0;
