httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jus...@erenkrantz.com>
Subject Re: Reviewing the Filtering API
Date Wed, 22 Sep 2004 18:09:46 GMT
--On Wednesday, September 22, 2004 5:13 PM +0100 Nick Kew 
<nick@webthing.com> wrote:

> *** A few issues with util_filter in 2.0:
>
> ap_filter_type
> ==============
>
> Making this an enum and then using values like AP_FTYPE_[anything] + 5
> (as is done in, for example, mod_ssl) makes no sense.  An int with
> a set of #defined values makes more sense.

Why?  A C enum is operationally equivalent to an int with a set of 
#defines.  So, I'm not seeing the benefit here.  With an enum, we can get 
some range of type safety.

> ap_filter_t
> ===========
>
> This inclues both request_rec and conn_rec fields, but the request_rec
> is invalid in content-level filters, while the conn_rec is of course
> available from the request_rec where valid.  So, shouldn't that be a
> union?

Um, request_rec and conn_rec is certainly valid in the content-level 
filters.  So, I'm not sure what you mean.  As an aside, unions lead to 
confusion (i.e. when is member 'a' valid?); so I wouldn't want us to use 
them here: especially for something that third-parties have to deal with 
directly.

> Documentation
> =============
>
> I recently fixed PR:19688, but there are other less critical issues
> outstanding, such as
>  * @param ftype The type of filter function, either ::AP_FTYPE_CONTENT or
>  *              ::AP_FTYPE_CONNECTION

I'm not sure what you mean.

> Yesterday I introduced two new API functions in util_filter:
>
> 	ap_register_output_filter_protocol
> 	ap_filter_protocol
>
> together with a set of associated #defines
>
> The first function is ap_register_output_filter with an additional
> argument proto_flags.  The second sets proto_flags during a request.
> The purpose of these is to enable filters to 'opt out' of concerning
> themselves with the lower-level details of supporting HTTP.

For the record, I sort of like this idea.  But, I'm concerned about how 
these 'flags' are enforced.

> Example: mod_include
>
> mod_include is a typical output content filter, in that it changes the
> data passing through, including changing the byte count.  It's almost
> certainly the most widely known and used such filter.
>
> As it stands, it correctly unsets content length, and it deals with
> cacheing/Last-Modified in its own way based on configuration (XBitHack).
> But it also has some bugs: for example:
> 	* if a Content-MD5 is set, it doesn't unset it.  Likewise an ETag.
> 	* it won't work correctly if served partial contents, but it
> 	  does nothing to prevent that happening (vide discussion on
> 	  handling ranges a couple of months ago).
> For mod_include to deal fully with these is a significant burden on the
> modules authors.
>
> The new API calls offers mod_include the opportunity to be simplified
> at the same time as fixing edge-case bugs such as those I've discussed.
> A simple way is to replace the existing
>
> ap_register_output_filter("INCLUDES", includes_filter, includes_setup,
>                               AP_FTYPE_RESOURCE);
>
> with the new variant
>
> ap_register_output_filter_protocol("INCLUDES",
> 		includes_filter, includes_setup, AP_FTYPE_RESOURCE,
> 		AP_FILTER_PROTO_CHANGE | AP_FILTER_PROTO_CHANGE_LENGTH
> 		| AP_FILTER_PROTO_NO_BYTERANGE );
>
> This causes mod_filter to unset all headers that are invalidated by
> the module's content transformation, and prevent it getting byteranges
> from the backend.  With this, mod_include still has to process XBitHack
> and cacheing headers itself - these are very specific to SSI and don't
> generalise to other filters - but mod_filter does everything else.
>
> As with any other filter, mod_include will run unchanged within the
> new framework by simply ignoring the additional API calls.

This sounds good.  -- justin

Mime
View raw message