httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <>
Subject Re: AddOutputFilterByType oddness
Date Wed, 22 Sep 2004 16:22:32 GMT
--On Wednesday, September 22, 2004 5:01 PM +0100 Nick Kew <> 

> I've said it before and I'll say it again: AddOutputFilterByType is
> fundamentally unsatisfactory.  This confusion is an effect, not cause.

Suffice to say, I disagree.

> * Configuration is inconsistent with other filter directives.  The
>   relationship with [Set|Add|Remove]OutputFilter is utterly unintuitive
>   and, from a user POV, broken.

I think it's really clear from the user's perspective.  I think the problem 
comes in on the developer's side.

> * Tying it to ap_set_content_type is, to say the least, hairy.
>   IMO we shouldn't *require* modules to call this, and it's utterly
>   unreasonable to expect that it will never be called more than once
>   for a request, given the number of modules that might take an interest.
>   Especially when subrequests and internal redirects may be involved.

We have *always* mandated that ap_set_content_type() should be called rather 
than setting r->content_type.  (I wish we could remove content_type from 
request_rec instead.)

> * It's a complexity just waiting for modules to break on it.

Anything that depends upon content-type like this is going to be hairy because 
there may be several 'right' answers during the course of the request.

> I've made some more updates to mod_filter since I last posted on the
> subject, and I'm getting some very positive feedback from real users.
> For 2.2 I'd like to remove AddOutputFilterByType entirely, replacing
> it with mod_filter.

I've yet to see a clear and concise statement as to how mod_filter will solve 
this problem in a better and more efficient way.  (Especially from a user's 
perspective, but also from a developer's perspective.)

> mod_filter can also obsolete [Set|Add|Remove]OutputFilter, though I'm
> in no hurry to do that.  What I can also do is re-implement all the
> outputfilter directives within mod_filter and its updated framework.

I will also comment that I looked in the mod_filter code the other day and was 
disappointed that it doesn't follow our coding style at all or even have 
comments that help people understand what it is trying to do inside the .c 
file.  This all makes it very difficult to understand the code.  I'd greatly 
appreciate it if mod_filter (and the code that you inserted elsewhere - i.e. 
in util_filter.c) would conform to our style guidelines and had some comments 
inside of it that say what it does (or trying to do).

For example, some of the things it does just makes no sense at all. 
filter_bucket_type() is completely bogus and needs to be tossed.  The 
type->name field in the bucket should be used instead.

I'm getting annoyed by people doing massive code drops (i.e. mod_filter, 
mod_proxy, mod_auth_ldap, etc.) that don't conform to our code style and have 
no comments.  It makes it much harder to go and fix bugs in 'em.  -- justin

View raw message