httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Kew <n...@webthing.com>
Subject Re: AddOutputFilterByType oddness
Date Thu, 23 Sep 2004 11:43:19 GMT
On Wed, 22 Sep 2004, Justin Erenkrantz wrote:

> --On Wednesday, September 22, 2004 6:17 PM +0100 Nick Kew
> <nick@webthing.com> wrote:
>
> > It seems to me heavily counterintuitive that mixing ByType directives
> > with anything else means that the ByType filters *always* come last.
> > And that Remove won't affect them, but will affect others.
>
> I think we could get Remove*Filter to also delete the content-type filters.
>
> > Indeed.  mod_filter addresses this by configuring at the last moment,
> > so any earlier set_content_type()s are irrelevant.  I don't suppose it's
> > a panacaea for everything, but I do think it's a significant improvement
> > on what we have.
>
> I'm concerned about the overhead of mod_filter having to check all of its
> rules each time a filter is invoked.  This is why I started to look through
> the code last night to see how it worked and how invasive it is.

It's improving with time (except when I introduce bugs...).  Merging in
the structs with util_filter saves on having to do superfluous lookups.

Basically it does the lookup/dispatch once per filter in the filterchain
per request.  It checks that filter's providers until it finds a match.
So for anything you could do with an [Add|Set]OutputFilter[ByType]
that's one lookup per request.

> How would you handle the situation when filter #1 sets C-T to be
> "text/plain" and then filter #2 sets C-T to be "text/html"?

mod_filter takes the content-type as it is at that point in the chain.

Isn't the real nightmare where a filter calls ap_set_content_type and
some AddOutputFilterByTypes are in effect?  I guess what *really* bothers
me is the idea of adding filters *as a side-effect*.

>	  And, then
> mod_deflate needs to be conditionally added (sub-case #1: it needs to be
> added for 'text/plain'; sub-case #2: it needs to be added for 'text/html').
> How and where is it added?  Are you inserting dummy filters?

I'm not sure I follow.  It will dispatch to deflate based on the
content-type (or other dispatch criterion) as it is at that point
in the chain.

So if the handler sets application/xml but that goes through an XSLT
filter which sets it to text/html, then mod_filter sees application/xml
if it's before the XSLT filter in the chain, or text/html after it.

How can AddOutputFilterByType expect to cope with that?

>
> > From the user's perspective, it's simply more powerful and flexible.
> > Works with any request or response headers (not just content-type) or
> > environment variables.  Gets rid of constraints on ordering, like
> > AddOutputFilterbyType filter always coming after other filters
> > regardless of ordering in httpd.conf.
> >
> > Example: I have a user who wants to insert mod_deflate in a reverse
> > proxy, but only for selected content-types AND not if the content
> > length is below a threshold.  How would he do that with the old filter
> > framework?
>
> I guess I'm not clear what the syntax is (I guess I should go read the
> docs).

That particular scenario is complex, and requires mod_filter to be
used as its own provider.  The point is, we *can* now support complex
setups (or will be - that chaining is still broken in CVS).

But FWIW I have that working locally with

FilterDeclare   filter1 Content-Type    CONTENT_SET
FilterDeclare   filter2 Content-Length  CONTENT_SET

FilterProvider  filter1 filter2 $text
FilterProvider  filter2 DEFLATE >4000

FilterChain     filter1

to deflate all "text/*" documents of 4k or greater.


>	  I definitely don't want to see the filters be configured like
> mod_rewrite.  It needs to be fairly straightforward, but still fairly
> simplistic.  I don't want to have users have to read a complicated manual
> or docs to set up filters.  KISS.

Indeed.  Do you think the examples in the manual page are too complex?

Bear in mind that the third example is no more complex than the first two,
yet suddenly enables a frequently-requested capability that simply isn't
possible with the old filtering.

> Well, the point by you committing it into our tree is that the rest of us
> are now responsible for it.  That's why I brought up the code style issue:

OK, OKOK!   I promise to look harder at the code style guidelines!
And I _did_ ask on the list a couple of weeks before introducing to CVS.

> I looked yesterday afternoon (and haven't seen any commits since then).  I

That'll be the latest version.  Which FWIW was introduced prematurely
because it introduced a new feature demanded by a user.  Only that turned
out to be broken, which is why I'm re-hacking that now.

-- 
Nick Kew

Mime
View raw message