Adam,
I am leaving ap_proxy_string_read() in place for now. I think the 'right' thing to do
is
to use ap_getline() but I havent the time to figure out how to replace
ap_proxy_string_read() in the proxy_ftp code (no request_rec and ap_getline requires a
request_rec).
I am checking in your fixes with some small tweaks here and there as needed.
Bill
>
> 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]
>
