Moving on to input filters :)

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

The docstring in util_filter.h for ap_get_brigade says:

  * Get the current bucket brigade from the next filter on the filter
  * stack.  The filter returns an apr_status_t value.  If the bottom-most
  * filter doesn't read from the network, then ::AP_NOBODY_READ is 
returned.
  * The bucket brigade will be empty when there is nothing left to get.

The best reference for the behaviour of ap_get_brigade would appear to 
be core_input_filter. Its behaviour differs from this docstring; in 
particular, end of input is (sometimes) signalled with an eos bucket 
being returned in the brigade.

I propose clarifying the last line of that docstring:

* If block is APR_NONBLOCK_READ and no data is currently available,
* the brigade will be empty on return, and the function will return
* APR_SUCCESS.
*
* If no more data is available, the brigade will contain an eos bucket
* on return, and the function will return APR_SUCCESS. A subsequent
* call to ap_get_brigade will return APR_EOF without altering the
* brigade.
*
* In no case will APR_EAGAIN be returned. Other error conditions may
* be returned, with or without data in the brigade.
*
* Input filters are expected to conform to the above interface.

The above is not quite the behaviour of core_input_filter, but I think
it could be with a few minor tweaks, none of which will cost cycles.
As I read the code, it behaves as follows;

In MODE_READBYTES:

With APR_BLOCK_READ, on EOF the filter will first return
   a brigade containing an EOS bucket, with status APR_SUCCESS.
   A subsequent call to the filter will return an empty
   brigade with status APR_EOF. It should not otherwise
   return an empty brigade.

With APR_NONBLOCK_READ, an empty brigade will be returned with
   status APR_SUCCESS if the the socket has no data ready OR
   if an EOF was encountered. There appears to be no way to
   distinguish between these cases other than by doing a blocking
   read. (Note 1)

In MODE_EXHAUSTIVE:
   regardless of read blocking, the socket bucket is simply
   appended to the returned brigade, followed by an EOS bucket.
   APR_SUCCESS is always returned.

In MODE_GETLINE and MODE_SPECULATIVE:

With APR_BLOCK_READ, on EOF the filter will return an empty
   brigade with status APR_SUCCESS. A subsequent call to the filter
   will return an empty brigade with status APR_EOF. (Notes 2 and 3)

With APR_NONBLOCK_READ, the filter will behave as in MODE_READBYTES.


Note 1:
   I believe this behaviour to be incorrect. Line 285 of
   server/core_filters.c is:

         else if (block == APR_BLOCK_READ && len == 0) {

   At this point, it has already checked for APR_EAGAIN and
   error status returns. Consequently, a zero-length read in
   either blocking or non-blocking mode ought to be an EOF.
   I think this should be changed to:

         else if (len == 0) {

   which would insert the EOS bucket in the case of a non-blocking
   get_brigade. However, I don't know if it would confuse consumers
   who were not expecting it.

   It would actually probably be simpler to insert the eos bucket
   in the ctx->b brigade when it is built, rather than inserting
   it in response to various eof-like conditions later on in the
   code.

Note 2:
   In MODE_GETLINE, no attempt is made to insert an eos bucket in
   either blocking or non-blocking mode; in the case of EOF, I
   believe that the socket bucket will remove itself, leading to
   an eventual return of APR_EOF. This seems inconsistent with
   MODE_READBYTES.

Note 3:
   In MODE_SPECULATIVE, the eos bucket is deliberately not created.
   A comment says that the next will return an EOS bucket, but
   as far as I can see, once ctx->b has been emptied, as it will
   have been by the call to apr_bucket_delete at line 294, the
   next call to the filter will return an empty bucket with
   status APR_EOF.

Finally:
   I think there is a possibility that the socket bucket's read
   function will lose data, if the underlying call to apr_socket_recv
   returns a non-zero-length buffer along with a non-APR_SUCCESS
   status. However, this may not actually ever happen. It would
   only really be serious in the case of an APR_EAGAIN return.