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.



Reply via email to