> -----Original Message-----
> From: "Plüm, Rüdiger, VF-Group"
> Sent: Mittwoch, 12. Januar 2011 16:05
> To: [email protected]
> Subject: RE: svn commit: r1051468 - in /httpd/httpd/trunk:
> CHANGES modules/ssl/ssl_engine_io.c
>
>
>
> > -----Original Message-----
> > From: Joe Orton
> > Sent: Mittwoch, 12. Januar 2011 15:51
> > To: [email protected]
> > Subject: Re: svn commit: r1051468 - in /httpd/httpd/trunk:
> > CHANGES modules/ssl/ssl_engine_io.c
> >
> > On Wed, Jan 12, 2011 at 03:29:45PM +0100, "Plüm, Rüdiger,
> > VF-Group" wrote:
> > >
> > > I guess the second call to char_buffer_write can happen in
> > the situation
> > > you describe above, but IMHO it can also happen if we have
> > a non blocking read
> > > and we just did not get enough data.
> >
> > How? If no LF is ever found the loop continues until the
> > buffer is full
> > with tmplen == 0. In the case of read failure/EAGAIN, the
> > code returns
> > from the function early and never falls out of the loop.
> >
> > (This is all non-obvious because of the structure of the
> > code; it might
> > read more clearly if the if (pos) bit was moved inside the
> > loop before
> > the break.)
>
> Ok. I guess you are correct. I thought about a situation similar
> to ap_get_brigade where a non blocking read can return APR_SUCCESS
> and an empty brigade but ssl_io_input_read returning APR_SUCCESS
> and tmplen == 0 would lead to an endless loop in the while loop.
> Besides this leading to another bug of a spinning httpd process
> my code would never be triggered in this case.
> So what is the way forward now?
> My second propsal to return just what we have?
Should I commit the patch below now to resolve the issue and address
your point?
Index: modules/ssl/ssl_engine_io.c
===================================================================
--- modules/ssl/ssl_engine_io.c (revision 1058133)
+++ modules/ssl/ssl_engine_io.c (working copy)
@@ -757,8 +757,7 @@
if (status != APR_SUCCESS) {
if (APR_STATUS_IS_EAGAIN(status) && (*len > 0)) {
- /* Save the part of the line we already got */
- char_buffer_write(&inctx->cbuf, buf, *len);
+ return APR_SUCCESS;
}
return status;
}
@@ -786,10 +785,6 @@
*len = bytes;
}
- else {
- /* Save the part of the line we already got */
- char_buffer_write(&inctx->cbuf, buf, *len);
- }
return APR_SUCCESS;
}
which would result in the following complete patch (r105148, r1058133, Patch
above):
Index: modules/ssl/ssl_engine_io.c
===================================================================
--- modules/ssl/ssl_engine_io.c (revision 1051467)
+++ modules/ssl/ssl_engine_io.c (working copy)
@@ -756,6 +756,9 @@
status = ssl_io_input_read(inctx, buf + offset, &tmplen);
if (status != APR_SUCCESS) {
+ if (APR_STATUS_IS_EAGAIN(status) && (*len > 0)) {
+ return APR_SUCCESS;
+ }
return status;
}
Regards
Rüdiger
