apr_buckets_socket.c possible buglet

[Rici Lake <ri...@ricilake.net>]

The documentation for apr_socket_recv says:

"It is possible for both bytes to be received and an APR_EOF or other 
error to be returned."

However, socket_bucket_read (in apr_buckets_socket.c) contains the 
following:

32    *str = NULL;
33    *len = APR_BUCKET_BUFF_SIZE;
34    buf = apr_bucket_alloc(*len, a->list); /* XXX: check for failure? 
*/
35
36    rv = apr_socket_recv(p, buf, len);
37
38    if (block == APR_NONBLOCK_READ) {
39        apr_socket_timeout_set(p, timeout);
40    }
41
42    if (rv != APR_SUCCESS && rv != APR_EOF) {
43        apr_bucket_free(buf);
44        return rv;
45    }

At line 32, the caller's buffer is set to NULL. At line 36, the 
caller's len is set to the length return from apr_socket_recv.

At line 44, if an error indication other than APR_EOF has been received 
from apr_socket_recv, then the bucket_read returns, handing the caller 
<NULL, len>. If len is not 0 at this point, as the documentation for 
apr_socket_recv says is possible, the caller may proceed to attempt to 
read the data from NULL and segfault.

A simple fix would be to delete lines 42-45 and replace line 75 with 
"return rv":

A better fix might be to hold onto the non-success indication until the 
subsequent call to socket_bucket_read.

Re: apr_buckets_socket.c possible buglet

[Rici Lake <ri...@ricilake.net>]

On 4-May-05, at 9:54 AM, Joe Orton wrote:

> On Tue, Apr 26, 2005 at 12:45:31PM -0500, Rici Lake wrote:
>> The documentation for apr_socket_recv says:
>>
>> "It is possible for both bytes to be received and an APR_EOF or other
>> error to be returned."
>
> Is that actually true?  It suprised me because apr_file_read()
> guarantees the exact opposite and it would be a rather awkward 
> interface
> to use.  Looking at both the Unix and Win32 implementations it does 
> seem
> that all times where an error is returned, *len is set to 0?

As far as I can see, it's not true. At least, after writing that
message, I looked through all the socket implementations I could
find and as far as I can see, none of them will return an error
accompanied by a non-zero length.

So perhaps the only buglet is the documentation.

My guess is that whoever wrote that documentation wanted to leave
the option open. But as you say, it would be an awkward interface
to use, so perhaps the option should be closed. The danger of
leaving the docstring as is is that someone might be misled by
it into actually implementing a version of apr_socket_recv which
exhibits that behaviour, and then breaking all the software which
is based on current behaviour rather than documented APIs.

Re: apr_buckets_socket.c possible buglet

[Joe Orton <jo...@redhat.com>]

On Tue, Apr 26, 2005 at 12:45:31PM -0500, Rici Lake wrote:
> The documentation for apr_socket_recv says:
> 
> "It is possible for both bytes to be received and an APR_EOF or other 
> error to be returned."

Is that actually true?  It suprised me because apr_file_read()
guarantees the exact opposite and it would be a rather awkward interface
to use.  Looking at both the Unix and Win32 implementations it does seem
that all times where an error is returned, *len is set to 0?

joe