httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r692567 - /httpd/httpd/trunk/modules/filters/mod_charset_lite.c
Date Sat, 06 Sep 2008 08:38:43 GMT


On 09/06/2008 12:21 AM, gregames@apache.org wrote:
> Author: gregames
> Date: Fri Sep  5 15:21:36 2008
> New Revision: 692567
> 
> URL: http://svn.apache.org/viewvc?rev=692567&view=rev
> Log:
> PR 45687: Detect and pass along error buckets
> 
> Submitted by: Dan Poirier <poirier pobox.org> 
> Reviewed by:  trawick
> 
> Modified:
>     httpd/httpd/trunk/modules/filters/mod_charset_lite.c
> 
> Modified: httpd/httpd/trunk/modules/filters/mod_charset_lite.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_charset_lite.c?rev=692567&r1=692566&r2=692567&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/mod_charset_lite.c (original)
> +++ httpd/httpd/trunk/modules/filters/mod_charset_lite.c Fri Sep  5 15:21:36 2008
> @@ -414,6 +414,23 @@
>      return rv;
>  }
>  
> +static apr_status_t send_bucket_downstream(ap_filter_t *f, apr_bucket *b)
> +{
> +    request_rec *r = f->r;
> +    conn_rec *c = r->connection;
> +    apr_bucket_brigade *bb;
> +    charset_filter_ctx_t *ctx = f->ctx;
> +    apr_status_t rv;
> +
> +    bb = apr_brigade_create(r->pool, c->bucket_alloc);

This is bad. Please store the brigade in the filter context and reuse it,
by cleaning it. See also http://httpd.apache.org/docs/trunk/en/developer/output-filters.html#filtering
The same error is in send_downstream and IMHO they can share the same brigade.

> +    APR_BRIGADE_INSERT_TAIL(bb, b);
> +    rv = ap_pass_brigade(f->next, bb);
> +    if (rv != APR_SUCCESS) {
> +        ctx->ees = EES_DOWNSTREAM;
> +    }
> +    return rv;
> +}
> +
>  static apr_status_t set_aside_partial_char(charset_filter_ctx_t *ctx,
>                                             const char *partial,
>                                             apr_size_t partial_len)

> @@ -673,7 +690,7 @@
>              }
>              b = APR_BRIGADE_FIRST(bb);
>              if (b == APR_BRIGADE_SENTINEL(bb) ||
> -                APR_BUCKET_IS_EOS(b)) {
> +                APR_BUCKET_IS_METADATA(b)) {
>                  break;
>              }
>              rv = apr_bucket_read(b, &bucket, &bytes_in_bucket, APR_BLOCK_READ);
> @@ -892,6 +909,17 @@
>                  }
>                  break;
>              }
> +            if (APR_BUCKET_IS_METADATA(dptr)) {

Don't we need to handle flush buckets separately and flush our buffer downstream
as far as we can in this case?

> +                apr_bucket *metadata_bucket;
> +                metadata_bucket = dptr;
> +                dptr = APR_BUCKET_NEXT(dptr);
> +                APR_BUCKET_REMOVE(metadata_bucket);
> +                rv = send_bucket_downstream(f, metadata_bucket);
> +                if (rv != APR_SUCCESS) {
> +                    done = 1;
> +                }
> +                continue;
> +            }
>              rv = apr_bucket_read(dptr, &cur_str, &cur_len, APR_BLOCK_READ);
>              if (rv != APR_SUCCESS) {
>                  done = 1;

Regards

RĂ¼diger



Mime
View raw message