Is it safe to stop reading a bucket brigade in an input filter before the end?

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Is it safe to stop reading a bucket brigade in an input filter before the end?

Paul Callahan
I have an apache body filter that copies off data from the incoming
request.   But I terminate the reading of the data if the size of the
accumulated data is over some limit.

This is my function, it just breaks  out of the loop when the max is read.
 It seems to work ok.   Do I need to do any additional cleanup or anything
because I did not go to the end?

Thank you

// returns 0 after desired length is read
int my_append_data(const char *data, apr_size_t len, void *request_ctx);
void my_body_read_done(void *request_ctx);

apr_status_t my_body_input_filter(ap_filter_t *f, apr_bucket_brigade *out_bb,
                                     ap_input_mode_t mode,
apr_read_type_e block, apr_off_t nbytes,
                                    void *request_ctx) {
    request_rec *r = f->r;
    conn_rec *c = r->connection;

    apr_bucket_brigade *tmp_bb;
    int ret;

    tmp_bb = apr_brigade_create(r->pool, c->bucket_alloc);
    if (APR_BRIGADE_EMPTY(tmp_bb)) {
        ret = ap_get_brigade(f->next, tmp_bb, mode, block, nbytes);

        if (mode == AP_MODE_EATCRLF || ret != APR_SUCCESS)
            return ret;
    }

    while (!APR_BRIGADE_EMPTY(tmp_bb)) {
        apr_bucket *in_bucket = APR_BRIGADE_FIRST(tmp_bb);
        apr_bucket *out_bucket;
        const char *data;
        apr_size_t len;

        if (APR_BUCKET_IS_EOS(in_bucket)) {
            APR_BUCKET_REMOVE(in_bucket);
            APR_BRIGADE_INSERT_TAIL(out_bb, in_bucket);
            my_body_read_done(request_ctx);
            break;
        }

        ret = apr_bucket_read(in_bucket, &data, &len, block);
        if (ret != APR_SUCCESS) {
            return ret;
        }


        // copy read data up to a limit of 1mb, then stop.
        if (!my_append_data(data, len, request_ctx)) {
            apr_bucket_delete(in_bucket);
            break;
        }

        out_bucket = apr_bucket_heap_create(data, len, 0, c->bucket_alloc);
        APR_BRIGADE_INSERT_TAIL(out_bb, out_bucket);
        apr_bucket_delete(in_bucket);
    }
    return APR_SUCCESS;
}
Reply | Threaded
Open this post in threaded view
|

Re: Is it safe to stop reading a bucket brigade in an input filter before the end?

Sorin Manolache
On 16/05/2019 00.26, Paul Callahan wrote:
> I have an apache body filter that copies off data from the incoming
> request.   But I terminate the reading of the data if the size of the
> accumulated data is over some limit.
>
> This is my function, it just breaks  out of the loop when the max is read.
>   It seems to work ok.   Do I need to do any additional cleanup or anything
> because I did not go to the end?

Hello,

I suppose your code is ok.

Keep in mind that apache must clear the request body in order to be able
to parse and serve a new request that arrives on same, reused TCP
connection.

So towards the end of the processing of the current request, apache
calls a function, I think it's called ap_discard_request_body. This
function will read data from the network socket and pass it to the
filter chain. So your input filter will be called again (unless you
called ap_remove_input_filter somewhere), after you've already sent the
response to the client.

Best regards,
Sorin


>
> Thank you
>
> // returns 0 after desired length is read
> int my_append_data(const char *data, apr_size_t len, void *request_ctx);
> void my_body_read_done(void *request_ctx);
>
> apr_status_t my_body_input_filter(ap_filter_t *f, apr_bucket_brigade *out_bb,
>                                       ap_input_mode_t mode,
> apr_read_type_e block, apr_off_t nbytes,
>                                      void *request_ctx) {
>      request_rec *r = f->r;
>      conn_rec *c = r->connection;
>
>      apr_bucket_brigade *tmp_bb;
>      int ret;
>
>      tmp_bb = apr_brigade_create(r->pool, c->bucket_alloc);
>      if (APR_BRIGADE_EMPTY(tmp_bb)) {
>          ret = ap_get_brigade(f->next, tmp_bb, mode, block, nbytes);
>
>          if (mode == AP_MODE_EATCRLF || ret != APR_SUCCESS)
>              return ret;
>      }
>
>      while (!APR_BRIGADE_EMPTY(tmp_bb)) {
>          apr_bucket *in_bucket = APR_BRIGADE_FIRST(tmp_bb);
>          apr_bucket *out_bucket;
>          const char *data;
>          apr_size_t len;
>
>          if (APR_BUCKET_IS_EOS(in_bucket)) {
>              APR_BUCKET_REMOVE(in_bucket);
>              APR_BRIGADE_INSERT_TAIL(out_bb, in_bucket);
>              my_body_read_done(request_ctx);
>              break;
>          }
>
>          ret = apr_bucket_read(in_bucket, &data, &len, block);
>          if (ret != APR_SUCCESS) {
>              return ret;
>          }
>
>
>          // copy read data up to a limit of 1mb, then stop.
>          if (!my_append_data(data, len, request_ctx)) {
>              apr_bucket_delete(in_bucket);
>              break;
>          }
>
>          out_bucket = apr_bucket_heap_create(data, len, 0, c->bucket_alloc);
>          APR_BRIGADE_INSERT_TAIL(out_bb, out_bucket);
>          apr_bucket_delete(in_bucket);
>      }
>      return APR_SUCCESS;
> }
>

Reply | Threaded
Open this post in threaded view
|

Re: Is it safe to stop reading a bucket brigade in an input filter before the end?

Stefan Eissing


> Am 19.05.2019 um 13:11 schrieb Sorin Manolache <[hidden email]>:
>
> On 16/05/2019 00.26, Paul Callahan wrote:
>> I have an apache body filter that copies off data from the incoming
>> request.   But I terminate the reading of the data if the size of the
>> accumulated data is over some limit.
>> This is my function, it just breaks  out of the loop when the max is read.
>>  It seems to work ok.   Do I need to do any additional cleanup or anything
>> because I did not go to the end?
>
> Hello,
>
> I suppose your code is ok.
>
> Keep in mind that apache must clear the request body in order to be able to parse and serve a new request that arrives on same, reused TCP connection.
>
> So towards the end of the processing of the current request, apache calls a function, I think it's called ap_discard_request_body. This function will read data from the network socket and pass it to the filter chain. So your input filter will be called again (unless you called ap_remove_input_filter somewhere), after you've already sent the response to the client.

Good advice. When your input filter aborts (it needs to return an error code), the connection is in an undefined state for HTTP/1.1. You could make sure that the connection->keepalive flag is set to AP_CONN_CLOSE. Otherwise, as Sorin said, the dicard_request_body() might attempt to read the rest of the input that hangs before the next request.

Cheers, Stefan


> Best regards,
> Sorin
>
>
>> Thank you
>> // returns 0 after desired length is read
>> int my_append_data(const char *data, apr_size_t len, void *request_ctx);
>> void my_body_read_done(void *request_ctx);
>> apr_status_t my_body_input_filter(ap_filter_t *f, apr_bucket_brigade *out_bb,
>>                                      ap_input_mode_t mode,
>> apr_read_type_e block, apr_off_t nbytes,
>>                                     void *request_ctx) {
>>     request_rec *r = f->r;
>>     conn_rec *c = r->connection;
>>     apr_bucket_brigade *tmp_bb;
>>     int ret;
>>     tmp_bb = apr_brigade_create(r->pool, c->bucket_alloc);
>>     if (APR_BRIGADE_EMPTY(tmp_bb)) {
>>         ret = ap_get_brigade(f->next, tmp_bb, mode, block, nbytes);
>>         if (mode == AP_MODE_EATCRLF || ret != APR_SUCCESS)
>>             return ret;
>>     }
>>     while (!APR_BRIGADE_EMPTY(tmp_bb)) {
>>         apr_bucket *in_bucket = APR_BRIGADE_FIRST(tmp_bb);
>>         apr_bucket *out_bucket;
>>         const char *data;
>>         apr_size_t len;
>>         if (APR_BUCKET_IS_EOS(in_bucket)) {
>>             APR_BUCKET_REMOVE(in_bucket);
>>             APR_BRIGADE_INSERT_TAIL(out_bb, in_bucket);
>>             my_body_read_done(request_ctx);
>>             break;
>>         }
>>         ret = apr_bucket_read(in_bucket, &data, &len, block);
>>         if (ret != APR_SUCCESS) {
>>             return ret;
>>         }
>>         // copy read data up to a limit of 1mb, then stop.
>>         if (!my_append_data(data, len, request_ctx)) {
>>             apr_bucket_delete(in_bucket);
>>             break;
>>         }
>>         out_bucket = apr_bucket_heap_create(data, len, 0, c->bucket_alloc);
>>         APR_BRIGADE_INSERT_TAIL(out_bb, out_bucket);
>>         apr_bucket_delete(in_bucket);
>>     }
>>     return APR_SUCCESS;
>> }
>