Re: svn commit: r598299 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_filter.c

[Ruediger Pluem <rp...@apache.org>]

On 11/26/2007 03:56 PM, niq@apache.org wrote:
> Author: niq
> Date: Mon Nov 26 06:56:12 2007
> New Revision: 598299
> 
> URL: http://svn.apache.org/viewvc?rev=598299&view=rev
> Log:
> mod_filter: don't segfault on (unsupported) chained FilterProviders.
> PR 43956
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/filters/mod_filter.c
> 
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=598299&r1=598298&r2=598299&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Mon Nov 26 06:56:12 2007
> @@ -2,6 +2,9 @@
>  Changes with Apache 2.3.0
>  [ When backported to 2.2.x, remove entry from this file ]
>  
> +  *) mod_filter: Don't segfault on (unsupported) chained FilterProvider usage.
> +     PR 43956 [Nick Kew]
> +
>    *) mod_unique_id: Fix timestamp value in UNIQUE_ID.
>       PR 37064 [Kobayashi <kobayashi firstserver.co.jp>]
>  
> 
> Modified: httpd/httpd/trunk/modules/filters/mod_filter.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_filter.c?rev=598299&r1=598298&r2=598299&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/mod_filter.c (original)
> +++ httpd/httpd/trunk/modules/filters/mod_filter.c Mon Nov 26 06:56:12 2007
> @@ -137,7 +137,12 @@
>  
>      harness_ctx *fctx = apr_pcalloc(f->r->pool, sizeof(harness_ctx));
>      for (p = filter->providers; p; p = p->next) {
> -        if (p->frec->filter_init_func) {
> +        if (p->frec->filter_init_func == filter_init) {
> +            ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, f->c,
> +                          "Chaining of FilterProviders not supported");
> +            return HTTP_INTERNAL_SERVER_ERROR;
> +        }
> +        else if (p->frec->filter_init_func) {
>              f->ctx = NULL;
>              if ((err = p->frec->filter_init_func(f)) != OK) {
>                  ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, f->c,
> 

Wouldn't it make more sense to prevent this already at configuration time instead of
failing during request processing (the request processing check seems to be a good
sanity check that should stay)?
How about deleting the following code from filter_provider?

    if (!provider_frec) {
        provider_frec = apr_hash_get(cfg->live_filters, pname,
                                     APR_HASH_KEY_STRING);
    }



Regards

Rüdiger