hmm, so I tried out this patch and found that it does work correctly
for most cases and it does solve the original infinite loop problem.
However, it appears to have introduced a new infinite loop problem
as well as some truncation of proxy data.
Once status and header data have been read (or attempted to be read
in the case of HTTP/0.9), mod_proxy is busy waiting for body content.
This shows up as 100% cpu on my setup. The loop where this is happening
is based on a non-blocking call to ap_get_brigade() in proxy_http.c:856.
Can anyone tell me why this call should not block?
In the case of a HTTP/0.9 response, the line feed on the first line
(where status is tested) is eaten and never shows up in the output.
I suspect that is because of ap_rgetline().
Lastly, proxy_ftp also uses ap_proxy_string_read and will need to be
dealt with if we trash that function.
-adam
On Sat, Dec 29, 2001 at 08:02:32AM -0500, Bill Stoddard wrote:
> I spent a bit of time looking at this one and I am pretty sure this is not the right
> patch. The problem is that ap_proxy_string_read() is completely broken. Among other
> things, it completely chokes if the 'string' spans multiple brigades.
> ap_proxy_string_read should be trashed and something like this patch should be used
> instead (not tested):
>
> Index: proxy_http.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/proxy/proxy_http.c,v
> retrieving revision 1.114
> diff -u -r1.114 proxy_http.c
> --- proxy_http.c 19 Dec 2001 16:32:01 -0000 1.114
> +++ proxy_http.c 29 Dec 2001 12:57:09 -0000
> @@ -657,6 +657,22 @@
> while (received_continue) {
> apr_brigade_cleanup(bb);
>
> + while ((len = ap_getline(buffer, sizeof(buffer), rp, 0)) <= 0) {
> + if (len < 0) {
> + /* return status... what? timeout? connection dropped?
> + * for now, just use what was returned in the original broken code
> + * set rp->aborted?
> + */
> + apr_socket_close(p_conn->sock);
> + backend->connection = NULL;
> + ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
> + "proxy: error reading status line from remote "
> + "server %s", p_conn->name);
> + return ap_proxyerror(r, HTTP_BAD_GATEWAY,
> + "Error reading from remote server");
> + }
> + }
> +#if 0
> if (APR_SUCCESS != (rv = ap_proxy_string_read(origin, bb, buffer,
>sizeof(buffer),
> &eos))) {
> apr_socket_close(p_conn->sock);
> backend->connection = NULL;
> @@ -667,7 +683,7 @@
> "Error reading from remote server");
> }
> len = strlen(buffer);
> -
> +#endif
> /* Is it an HTTP/1 response?
> * This is buggy if we ever see an HTTP/1.10
> */
>
> ----- Original Message -----
> From: "Adam Sussman" <[EMAIL PROTECTED]>
> To: <[EMAIL PROTECTED]>
> Sent: Friday, December 28, 2001 8:24 PM
> Subject: [PATCH] mod_proxy infinite cpu eating loop
>
>
> >
> > ap_proxy_string_read currently goes into an infinite loop when the proxied server
> > closes the connection without sending any data. This patch fixes the problem
> > but I am not sure that this is the right way to do it.
> >
> > -adam
> >
> >
> > Index: modules/proxy/proxy_util.c
> > ===================================================================
> > RCS file: /home/cvspublic/httpd-2.0/modules/proxy/proxy_util.c,v
> > retrieving revision 1.73
> > diff -u -r1.73 proxy_util.c
> > --- modules/proxy/proxy_util.c 28 Nov 2001 21:07:32 -0000 1.73
> > +++ modules/proxy/proxy_util.c 29 Dec 2001 00:14:18 -0000
> > @@ -1039,6 +1039,7 @@
> > APR_BUCKET_REMOVE(e);
> > apr_bucket_destroy(e);
> > }
> > + if (APR_BRIGADE_EMPTY(bb)) break;
> > }
> >
> > return APR_SUCCESS;
> >
--
"I believe in Kadath in the cold waste, and Ultima Thule. But you
cannot prove to me that Harvard Law School actually exists."
- Theodora Goss
"I'm not like that, I have a cat, I don't need you.. My cat, and
about 18 lines of bourne shell code replace you in life."
- anonymous
Adam Sussman
Vidya Media Ventures
[EMAIL PROTECTED]
