You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Bill Stoddard <bi...@wstoddard.com> on 2001/05/18 14:07:58 UTC

Allowing modules to add input filters is broken

To the best of my knowledge, there is no way for a module to add input
filters -- in time for them to be used during input processing --.   Sure a
module can call add_input_filter, but the ap_run_insert_filter hook is run
too late for input filters to have any effect.

There are several different ways to fix this....

1.  Call ap_run_insert_filter in time to be considered by the inbound
processing.  (right after get_mime_headers() for instance). Using a single,
non parameterized function to add input and output filters to the stack with
the same call is trying to do too much with this one function. I cannot
point to a speficic problem this will cause but I am sure they exist.

2. Pass an additional parameter on ap_run_insert_filter designating whether
modules should insert their input filters or output filters. Call
ap_run_insert_filter(INSERT_INPUT_FILTERS) right after the call to
get_mime_headers(). Call ap_run_insert_filter(INSERT_OUTPUT_FILTERS) where
it is called today out of process_request_internal(). Modules would all have
to grok this parameter.

3. Another way to solve the this is to create a two new hooks to replace the
insert_filter hook:  ap_run_insert_input_filter() &
ap_run_insert_output_filter() and call each hook at the appropriate point in
the cycle.

2 or 3 work for me. Leaning toward #3 right now.

Ryan and Greg, opinions?

Bill



Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
Bill Stoddard wrote:

> I am getting a bit more insite into this problem.  If you want to add input
> filters, you need to do it with the pre_connection hook, not the
> insert_filters hook. The insert_filters hook is really only useful for
> output filters. This sould be a nice bit of info do document somewhere...

How about renaming insert_filters to insert_output_filters - then it
will be obvious.

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by Bill Stoddard <bi...@wstoddard.com>.
> I am getting a bit more insite into this problem.  If you want to add
input
> filters, you need to do it with the pre_connection hook, not the
> insert_filters hook. The insert_filters hook is really only useful for
> output filters.

and input filters that operate on a request body (but not request headers).

> This sould be a nice bit of info do document somewhere...
>
> Bill
>



Re: Allowing modules to add input filters is broken

Posted by Bill Stoddard <bi...@wstoddard.com>.
I am getting a bit more insite into this problem.  If you want to add input
filters, you need to do it with the pre_connection hook, not the
insert_filters hook. The insert_filters hook is really only useful for
output filters. This sould be a nice bit of info do document somewhere...

Bill

----- Original Message -----
From: "Bill Stoddard" <bi...@wstoddard.com>
To: <ne...@apache.org>
Sent: Friday, May 18, 2001 8:07 AM
Subject: Allowing modules to add input filters is broken


> To the best of my knowledge, there is no way for a module to add input
> filters -- in time for them to be used during input processing --.   Sure
a
> module can call add_input_filter, but the ap_run_insert_filter hook is run
> too late for input filters to have any effect.
>
> There are several different ways to fix this....
>
> 1.  Call ap_run_insert_filter in time to be considered by the inbound
> processing.  (right after get_mime_headers() for instance). Using a
single,
> non parameterized function to add input and output filters to the stack
with
> the same call is trying to do too much with this one function. I cannot
> point to a speficic problem this will cause but I am sure they exist.
>
> 2. Pass an additional parameter on ap_run_insert_filter designating
whether
> modules should insert their input filters or output filters. Call
> ap_run_insert_filter(INSERT_INPUT_FILTERS) right after the call to
> get_mime_headers(). Call ap_run_insert_filter(INSERT_OUTPUT_FILTERS) where
> it is called today out of process_request_internal(). Modules would all
have
> to grok this parameter.
>
> 3. Another way to solve the this is to create a two new hooks to replace
the
> insert_filter hook:  ap_run_insert_input_filter() &
> ap_run_insert_output_filter() and call each hook at the appropriate point
in
> the cycle.
>
> 2 or 3 work for me. Leaning toward #3 right now.
>
> Ryan and Greg, opinions?
>
> Bill
>
>


Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
Graham Leggett wrote:

> > The output filters should *always* run, if at least to send an EOS bucket.
> > That EOS is actually what triggers the delivery of the headers to the
> > network (or at least to the CORE filter for output buffering into the next
> > request).
> >
> > So if you're not seeing output filters run, then a bug exists.

Hmmm - looking at this some more something weird is going on.

When a normal 200 response is served, the output filter is run
correctly, the header values are modified, and the modified header
reaches the browser.

When a 304 is served however, the output filter is run correctly, the
header values are modified, but the modified header never reaches the
browser.

It seems the headers are handled differently when the response is a
header only, as opposed to when there are headers and a body.

Does this make sense to anyone?

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
Greg Stein wrote:

> The output filters should *always* run, if at least to send an EOS bucket.
> That EOS is actually what triggers the delivery of the headers to the
> network (or at least to the CORE filter for output buffering into the next
> request).
> 
> So if you're not seeing output filters run, then a bug exists.
> 
> (IOW: theory says you're doing the right thing)

Hmmm - it looks like the bug is with proxy... if there is a 304
response, proxy simply exits the content handler without passing
anything up the stack. As a result, none of the filters run - will fix.

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by Greg Stein <gs...@lyra.org>.
On Sat, May 19, 2001 at 01:09:09AM +0200, Graham Leggett wrote:
>...
> If the result of the request returns a body, then HeaderOut works great.
> If however the result is 304 Not Modified, there is no body, and the
> output filter never runs and we're back to the same problem we had with
> the input filters.
> 
> So the question is - which hook should be called to fiddle with the
> headers after the content generator runs, but before the headers are
> sent to the network, whether a body exists or not...?

The output filters should *always* run, if at least to send an EOS bucket.
That EOS is actually what triggers the delivery of the headers to the
network (or at least to the CORE filter for output buffering into the next
request).

So if you're not seeing output filters run, then a bug exists.

(IOW: theory says you're doing the right thing)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
rbb@covalent.net wrote:

> > So the question is - which hook should be called to fiddle with the
> > headers after the content generator runs, but before the headers are
> > sent to the network, whether a body exists or not...?
> 
> You are using the wrong kind of output filter.  Take a look at how
> http_header_filter is added, and just copy that model.

Looking at this it would mean I would need to use an
AP_FTYPE_HTTP_HEADER filter - but how would I guarantee that this filter
ran before the http_header_filter?

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
Greg Stein wrote:

> > I posted a patch to fix this.
> 
> Moving the insertion of those filters to the insert_filters hook is the
> right thing to do! Don't get me wrong.

Cool - I'll commit the change later tonight when I have some time...

> I was trying to point out that we've got a lot of flexibility in how the
> filters are inserted, and how to get them ordered.

Ok.

> Using AP_FTYPE_HTTP_HEADER-1 will put your filter "before" *all*
> AP_FTYPE_HTTP_HEADER filters. No matter *when* you end up calling "add
> filter".

The trouble is that mod_header wants to run as late as possible,
allowing the user to override as much as possible (within the limit of
what is a good idea). It really wants to run second last, before
HTTP_HEADERS but after everything else. 

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by Greg Stein <gs...@lyra.org>.
On Mon, May 21, 2001 at 11:52:14AM +0200, Graham Leggett wrote:
> Greg Stein wrote:
> 
> > If filter types are equal, then order is dependent upon insertion, which
> > falls back to the insert_filter ordering.
> > 
> > So... you can correct your filtering ordering by adjusting its type and
> > inserting it whenever, or you can have the same type as the core filters and
> > just make yourself run before the core filters.
> 
> But as Ryan pointed out, and as the code shows the HTTP_HEADER filter is
> added before the insert_filter hook, so it's now impossible to add
> AP_FTYPE_HTTP_HEADER filters that do anything.
> 
> I posted a patch to fix this.

Moving the insertion of those filters to the insert_filters hook is the
right thing to do! Don't get me wrong.

I was trying to point out that we've got a lot of flexibility in how the
filters are inserted, and how to get them ordered.

> > I'd say make your insert_filter hook APR_HOOK_MIDDLE and use
> > AP_FTYPE_HTTP_HEADER - 1 for the type. Then it will run just before the
> > HTTP_HEADER filters.
> 
> So adding a -1 will place the filter before the last filter?

Using AP_FTYPE_HTTP_HEADER-1 will put your filter "before" *all*
AP_FTYPE_HTTP_HEADER filters. No matter *when* you end up calling "add
filter".

In other words, it is *much* safer to use the -1 on the filter type. You
don't have to worry about whether your add_filter comes before/after the
core's add filter. And when you think about it: you shouldn't *have* to know
when the core does it. Really: the core is adding filters, but why should
you care?

> What if
> someone else tried to add an AP_FTYPE_HTTP_HEADER filter in another
> module without a -1? Now it would no longer work,

Huh? Ah. You mean that you could add some headers, a filter would run after
you (doing what?), and then the regular HTTP_HEADER filters would run.

I'm not sure that I see a problem with that. And recognize: it will *always*
be the case that somebody could insert a filter between yours and the core's
filters. You shouldn't have to care/worry about that.

Consider: if you use HTTP_HEADER and happen to beat the core's insertion, to
get yours "in front"... there could be somebody that adds an HTTP_HEADER
after you, but before the core gets its chance to insert filters.

So if you can't truly prevent a filter from coming between you and the core
filters, then don't even bother trying. And to isolate yourself from
"timing" changes in filter insertion, it is best to use the -1 on the filter
type to simply place yours before the core filters.

> and it would not be
> clear as to why. In addition, AP_FTYPE_HTTP_HEADER filter would have to
> work differently to the other filter types.

I'm not clear on "why" for that last statement. We can't/shouldn't special
case any of the types. They are only there to assist with ordering.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: Allowing modules to add input filters is broken

Posted by Greg Stein <gs...@lyra.org>.
On Mon, May 21, 2001 at 08:20:04PM +0100, Ben Laurie wrote:
>...
> Seems to me that filters need the same kind of predecessor/successor
> logic that hooks have.

Might be nice, but I'm generally against it... The filters shouldn't really
be concerned with what other filters may be in the stack. Knowing that kind
of breaks the abstraction of "accept some data; modify it; send it". It
reaches "out of the box" if you have to know about other filters.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: Allowing modules to add input filters is broken

Posted by Ben Laurie <be...@algroup.co.uk>.
Graham Leggett wrote:
> 
> Greg Stein wrote:
> 
> > If filter types are equal, then order is dependent upon insertion, which
> > falls back to the insert_filter ordering.
> >
> > So... you can correct your filtering ordering by adjusting its type and
> > inserting it whenever, or you can have the same type as the core filters and
> > just make yourself run before the core filters.
> 
> But as Ryan pointed out, and as the code shows the HTTP_HEADER filter is
> added before the insert_filter hook, so it's now impossible to add
> AP_FTYPE_HTTP_HEADER filters that do anything.
> 
> I posted a patch to fix this.
> 
> > I'd say make your insert_filter hook APR_HOOK_MIDDLE and use
> > AP_FTYPE_HTTP_HEADER - 1 for the type. Then it will run just before the
> > HTTP_HEADER filters.
> 
> So adding a -1 will place the filter before the last filter? What if
> someone else tried to add an AP_FTYPE_HTTP_HEADER filter in another
> module without a -1? Now it would no longer work, and it would not be
> clear as to why. In addition, AP_FTYPE_HTTP_HEADER filter would have to
> work differently to the other filter types.

Seems to me that filters need the same kind of predecessor/successor
logic that hooks have.

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
Greg Stein wrote:

> If filter types are equal, then order is dependent upon insertion, which
> falls back to the insert_filter ordering.
> 
> So... you can correct your filtering ordering by adjusting its type and
> inserting it whenever, or you can have the same type as the core filters and
> just make yourself run before the core filters.

But as Ryan pointed out, and as the code shows the HTTP_HEADER filter is
added before the insert_filter hook, so it's now impossible to add
AP_FTYPE_HTTP_HEADER filters that do anything.

I posted a patch to fix this.

> I'd say make your insert_filter hook APR_HOOK_MIDDLE and use
> AP_FTYPE_HTTP_HEADER - 1 for the type. Then it will run just before the
> HTTP_HEADER filters.

So adding a -1 will place the filter before the last filter? What if
someone else tried to add an AP_FTYPE_HTTP_HEADER filter in another
module without a -1? Now it would no longer work, and it would not be
clear as to why. In addition, AP_FTYPE_HTTP_HEADER filter would have to
work differently to the other filter types.

Regards,
Graham 
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by Greg Stein <gs...@lyra.org>.
On Sun, May 20, 2001 at 11:44:29AM -0700, rbb@covalent.net wrote:
> On Sun, 20 May 2001, Graham Leggett wrote:
>...
> > How about a new hook for adding AP_FTYPE_HTTP_HEADER filters?

We've already got flexibility out the wazoo... another hook won't help.

> IMO, this should be fixed by adding the three filters above in a
> core_insert_filter phase.  Then, mod_headers can easily be run before the
> core's insert_fitler phase.  Problem solved.

Let's not forget that we have *two* mechanisms for sorting.

1) the ordering of the insert_filter hook
2) the ordering based on filter type

If filter types are equal, then order is dependent upon insertion, which
falls back to the insert_filter ordering.

So... you can correct your filtering ordering by adjusting its type and
inserting it whenever, or you can have the same type as the core filters and
just make yourself run before the core filters.

I'd say make your insert_filter hook APR_HOOK_MIDDLE and use
AP_FTYPE_HTTP_HEADER - 1 for the type. Then it will run just before the
HTTP_HEADER filters.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

[PATCH] fix for AP_FTYPE_HTTP_HEADER filters

Posted by Graham Leggett <mi...@sharp.fm>.
rbb@covalent.net wrote:

> > How about a new hook for adding AP_FTYPE_HTTP_HEADER filters?
> 
> IMO, this should be fixed by adding the three filters above in a
> core_insert_filter phase.  Then, mod_headers can easily be run before the
> core's insert_fitler phase.  Problem solved.

The attached patch does what you suggested - mod_headers now works
correctly as an AP_FTYPE_HTTP_HEADER output filter. 

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
rbb@covalent.net wrote:

> > How about a new hook for adding AP_FTYPE_HTTP_HEADER filters?
> 
> IMO, this should be fixed by adding the three filters above in a
> core_insert_filter phase.  Then, mod_headers can easily be run before the
> core's insert_fitler phase.  Problem solved.

Trouble is - there is one last problem. mod_headers works great, but not
on 304 Not Modified responses.

The reason why is this bit of code in ap_http_header_filter():

    if (r->status == HTTP_NOT_MODIFIED) {
        apr_table_do((int (*)(void *, const char *, const char *))
form_header_field,
                    (void *) &h, r->headers_out,
                    "Connection",
                    "Keep-Alive",
                    "ETag",
                    "Content-Location",
                    "Expires",
                    "Cache-Control",
                    "Vary",
                    "Warning",
                    "WWW-Authenticate",
                    "Proxy-Authenticate",
                    NULL);
    }
    else {
        apr_table_do((int (*) (void *, const char *, const char *))
form_header_field,
                 (void *) &h, r->headers_out, NULL);
    }

It seems that only certain headers are sent on these responses, all
others are stripped. Is this a requirement of HTTP? Can anyone explain
why this is like this?

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

[PATCH] fix for AP_FTYPE_HTTP_HEADER filters

Posted by Graham Leggett <mi...@sharp.fm>.
rbb@covalent.net wrote:

> > How about a new hook for adding AP_FTYPE_HTTP_HEADER filters?
> 
> IMO, this should be fixed by adding the three filters above in a
> core_insert_filter phase.  Then, mod_headers can easily be run before the
> core's insert_fitler phase.  Problem solved.

The attached patch does what you suggested - mod_headers now works
correctly as an AP_FTYPE_HTTP_HEADER output filter. 

(Let's try actually add the patch this time)

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by rb...@covalent.net.
On Sun, 20 May 2001, Graham Leggett wrote:

> rbb@covalent.net wrote:
>
> > > So the question is - which hook should be called to fiddle with the
> > > headers after the content generator runs, but before the headers are
> > > sent to the network, whether a body exists or not...?
> >
> > You are using the wrong kind of output filter.  Take a look at how
> > http_header_filter is added, and just copy that model.
>
> I changed the filter type to AP_FTYPE_HTTP_HEADER, but now the filter
> runs too late and no headers are changed.
>
> Looking closer at the core code, in modules/http/http_request.c and
> server/protocol.c:
>
>     r->output_filters  = conn->output_filters;
>     r->input_filters   = conn->input_filters;
>
>     ap_add_output_filter("BYTERANGE", NULL, r, r->connection);
>     ap_add_output_filter("CONTENT_LENGTH", NULL, r, r->connection);
>     ap_add_output_filter("HTTP_HEADER", NULL, r, r->connection);
>
> All three of the above filters are AP_FTYPE_HTTP_HEADER filters - this
> means it's impossible to add any more AP_FTYPE_HTTP_HEADER filters,
> because they would always be added after the HTTP_HEADER filter ran,
> thus they cannot do anything.
>
> Adding the filter in the pre_connection phase could get round this, but
> would this then break TLS, etc?
>
> How about a new hook for adding AP_FTYPE_HTTP_HEADER filters?

IMO, this should be fixed by adding the three filters above in a
core_insert_filter phase.  Then, mod_headers can easily be run before the
core's insert_fitler phase.  Problem solved.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
rbb@covalent.net wrote:

> > So the question is - which hook should be called to fiddle with the
> > headers after the content generator runs, but before the headers are
> > sent to the network, whether a body exists or not...?
> 
> You are using the wrong kind of output filter.  Take a look at how
> http_header_filter is added, and just copy that model.

I changed the filter type to AP_FTYPE_HTTP_HEADER, but now the filter
runs too late and no headers are changed.

Looking closer at the core code, in modules/http/http_request.c and
server/protocol.c:

    r->output_filters  = conn->output_filters;
    r->input_filters   = conn->input_filters;

    ap_add_output_filter("BYTERANGE", NULL, r, r->connection);
    ap_add_output_filter("CONTENT_LENGTH", NULL, r, r->connection);
    ap_add_output_filter("HTTP_HEADER", NULL, r, r->connection);

All three of the above filters are AP_FTYPE_HTTP_HEADER filters - this
means it's impossible to add any more AP_FTYPE_HTTP_HEADER filters,
because they would always be added after the HTTP_HEADER filter ran,
thus they cannot do anything. 

Adding the filter in the pre_connection phase could get round this, but
would this then break TLS, etc?

How about a new hook for adding AP_FTYPE_HTTP_HEADER filters? 

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by rb...@covalent.net.
On Sat, 19 May 2001, Graham Leggett wrote:

> rbb@covalent.net wrote:
>
> > So this filter doesn't need to do anything if there is not body data?  If
> > there is no body data, then the filters will never be called.
> ...
> > > The input filter needs to run after the headers are read in and put in
> > > r->headers, but before the content generator runs.
> >
> > Use fixups, that is what it is there for.
>
> Ok - fixups have sorted out the HeadersIn issue, but this problem has
> bitten the HeadersOut directive.
>
> If the result of the request returns a body, then HeaderOut works great.
> If however the result is 304 Not Modified, there is no body, and the
> output filter never runs and we're back to the same problem we had with
> the input filters.
>
> So the question is - which hook should be called to fiddle with the
> headers after the content generator runs, but before the headers are
> sent to the network, whether a body exists or not...?

You are using the wrong kind of output filter.  Take a look at how
http_header_filter is added, and just copy that model.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
rbb@covalent.net wrote:

> So this filter doesn't need to do anything if there is not body data?  If
> there is no body data, then the filters will never be called.
...
> > The input filter needs to run after the headers are read in and put in
> > r->headers, but before the content generator runs.
> 
> Use fixups, that is what it is there for.

Ok - fixups have sorted out the HeadersIn issue, but this problem has
bitten the HeadersOut directive.

If the result of the request returns a body, then HeaderOut works great.
If however the result is 304 Not Modified, there is no body, and the
output filter never runs and we're back to the same problem we had with
the input filters.

So the question is - which hook should be called to fiddle with the
headers after the content generator runs, but before the headers are
sent to the network, whether a body exists or not...?

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by rb...@covalent.net.
On Fri, 18 May 2001, Graham Leggett wrote:

> Bill Stoddard wrote:
>
> > > This doesn't work - the filter is inserted, but it never runs.
> >
> > Read Ryan's response carefully (and my earlier responses this AM).  The
> > input filter can filter the request body (not headers).
>
> When I say "filter the headers" I mean "fiddle with the contents of
> r->headers_in" after the headers have been read in.
>
> The filter is actually a body filter, but the body is passed through
> untouched.

So this filter doesn't need to do anything if there is not body data?  If
there is no body data, then the filters will never be called.

> > As I (and now Ryan) have pointed out, the only way to filter headers is to
> > hook the pre_connection hook.  But this is a really bad idea for reasons
> > mentioned in both of our previous posts.  I think the right thing to do with
> > mod_headers is to handle the HeaderIn processing in the request fixup hook,
> > not as a filter.
>
> The input filter needs to run after the headers are read in and put in
> r->headers, but before the content generator runs.

Use fixups, that is what it is there for.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
Bill Stoddard wrote:

> > This doesn't work - the filter is inserted, but it never runs.
> 
> Read Ryan's response carefully (and my earlier responses this AM).  The
> input filter can filter the request body (not headers).

When I say "filter the headers" I mean "fiddle with the contents of
r->headers_in" after the headers have been read in.

The filter is actually a body filter, but the body is passed through
untouched.

> As I (and now Ryan) have pointed out, the only way to filter headers is to
> hook the pre_connection hook.  But this is a really bad idea for reasons
> mentioned in both of our previous posts.  I think the right thing to do with
> mod_headers is to handle the HeaderIn processing in the request fixup hook,
> not as a filter.

The input filter needs to run after the headers are read in and put in
r->headers, but before the content generator runs.

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by Ben Laurie <be...@algroup.co.uk>.
Bill Stoddard wrote:
> 
> > rbb@covalent.net wrote:
> > >
> > > On Sun, 20 May 2001, Ben Laurie wrote:
> > >
> > > > Bill Stoddard wrote:
> > > > > As I (and now Ryan) have pointed out, the only way to filter headers
> is to
> > > > > hook the pre_connection hook.  But this is a really bad idea for
> reasons
> > > > > mentioned in both of our previous posts.
> > > >
> > > > What about mod_tls?
> > >
> > > Mod_tls should check the port in the pre_connection phase, and add the
> tls
> > > filter if the port is an SSL enabled virtual host.
> >
> > I know. That was my point!
> >
> 
> What was your point? Guess I didn't get it. One question though... How do
> you know if "the port is an SSL enabled virtual host" (your words not mine)?

Actually, they were Ryan's.

> Seems the best you can do is to know that the port  is an SSL port (via a
> server wide config) You cannot know in the pre_config hook which VH the
> request belongs to.

My point was that mod_tls has to do its thing in the pre_connection
phase, which isn't compatible with "but this is a really bad idea for
reasons mentioned in both of our previous posts".

And you can know which VH to the level SSL can care about, since it is
purely IP/port based (if we ignore the upgrade header, which is a
different thing that no browser implements [which is not to say we
shouldn't, of course]).

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

How to unsubscribe from this list?

Posted by Luis <lv...@yahoo.com>.
Hi How do i unsubscribe from this list?


_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com


Re: Allowing modules to add input filters is broken

Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> I still don't understand what name based virtual hosting has to do with SSL.  You can know at global
> server scope whether a port/ip pair is SSL or not and you install mod_tls appropriately.  Maybe I am
> being dense or you are really meaning ip-based host rather than name based (i.e., the Host: header)
> virtual hosting.

I think Graham is referring to the use of the Upgrade header field to
change a connection from HTTP to HTTP-over-SSL, which is described in
a recent RFC.  That is the only way to use SSL with name-based virtual
hosts (as opposed to separate IP address virtual hosts).

....Roy


Re: Allowing modules to add input filters is broken

Posted by rb...@covalent.net.
On Mon, 21 May 2001, Bill Stoddard wrote:

> > > What was your point? Guess I didn't get it. One question though... How do
> > > you know if "the port is an SSL enabled virtual host" (your words not mine)?
> > > Seems the best you can do is to know that the port  is an SSL port (via a
> > > server wide config) You cannot know in the pre_config hook which VH the
> > > request belongs to.
> >
> > SSL is only a namebased virtual host feature.  Based on which port the
> > connection came in on, you can use that in the pre-connection phase.
> >
>
> I still don't understand what name based virtual hosting has to do with SSL.  You can know at global
> server scope whether a port/ip pair is SSL or not and you install mod_tls appropriately.  Maybe I am
> being dense or you are really meaning ip-based host rather than name based (i.e., the Host: header)
> virtual hosting.

You and I are saying the same thing.  Obviously, I messed up, and meant
ip-based, but essentially we agree completely on this.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: Allowing modules to add input filters is broken

Posted by Bill Stoddard <bi...@wstoddard.com>.
> > What was your point? Guess I didn't get it. One question though... How do
> > you know if "the port is an SSL enabled virtual host" (your words not mine)?
> > Seems the best you can do is to know that the port  is an SSL port (via a
> > server wide config) You cannot know in the pre_config hook which VH the
> > request belongs to.
>
> SSL is only a namebased virtual host feature.  Based on which port the
> connection came in on, you can use that in the pre-connection phase.
>

I still don't understand what name based virtual hosting has to do with SSL.  You can know at global
server scope whether a port/ip pair is SSL or not and you install mod_tls appropriately.  Maybe I am
being dense or you are really meaning ip-based host rather than name based (i.e., the Host: header)
virtual hosting.

Bill


Re: Allowing modules to add input filters is broken

Posted by rb...@covalent.net.
> > > > What about mod_tls?
> > >
> > > Mod_tls should check the port in the pre_connection phase, and add the
> tls
> > > filter if the port is an SSL enabled virtual host.
> >
> > I know. That was my point!
> >
>
> What was your point? Guess I didn't get it. One question though... How do
> you know if "the port is an SSL enabled virtual host" (your words not mine)?
> Seems the best you can do is to know that the port  is an SSL port (via a
> server wide config) You cannot know in the pre_config hook which VH the
> request belongs to.

SSL is only a namebased virtual host feature.  Based on which port the
connection came in on, you can use that in the pre-connection phase.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: Allowing modules to add input filters is broken

Posted by Bill Stoddard <bi...@wstoddard.com>.
> rbb@covalent.net wrote:
> >
> > On Sun, 20 May 2001, Ben Laurie wrote:
> >
> > > Bill Stoddard wrote:
> > > > As I (and now Ryan) have pointed out, the only way to filter headers
is to
> > > > hook the pre_connection hook.  But this is a really bad idea for
reasons
> > > > mentioned in both of our previous posts.
> > >
> > > What about mod_tls?
> >
> > Mod_tls should check the port in the pre_connection phase, and add the
tls
> > filter if the port is an SSL enabled virtual host.
>
> I know. That was my point!
>

What was your point? Guess I didn't get it. One question though... How do
you know if "the port is an SSL enabled virtual host" (your words not mine)?
Seems the best you can do is to know that the port  is an SSL port (via a
server wide config) You cannot know in the pre_config hook which VH the
request belongs to.

Bill


Re: Allowing modules to add input filters is broken

Posted by Ben Laurie <be...@algroup.co.uk>.
rbb@covalent.net wrote:
> 
> On Sun, 20 May 2001, Ben Laurie wrote:
> 
> > Bill Stoddard wrote:
> > > As I (and now Ryan) have pointed out, the only way to filter headers is to
> > > hook the pre_connection hook.  But this is a really bad idea for reasons
> > > mentioned in both of our previous posts.
> >
> > What about mod_tls?
> 
> Mod_tls should check the port in the pre_connection phase, and add the tls
> filter if the port is an SSL enabled virtual host.

I know. That was my point!

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

Re: Allowing modules to add input filters is broken

Posted by rb...@covalent.net.
On Sun, 20 May 2001, Ben Laurie wrote:

> Bill Stoddard wrote:
> > As I (and now Ryan) have pointed out, the only way to filter headers is to
> > hook the pre_connection hook.  But this is a really bad idea for reasons
> > mentioned in both of our previous posts.
>
> What about mod_tls?

Mod_tls should check the port in the pre_connection phase, and add the tls
filter if the port is an SSL enabled virtual host.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: Allowing modules to add input filters is broken

Posted by Ben Laurie <be...@algroup.co.uk>.
Bill Stoddard wrote:
> As I (and now Ryan) have pointed out, the only way to filter headers is to
> hook the pre_connection hook.  But this is a really bad idea for reasons
> mentioned in both of our previous posts.

What about mod_tls?

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

Re: Allowing modules to add input filters is broken

Posted by Bill Stoddard <bi...@wstoddard.com>.

> Bill Stoddard wrote:
>
> > > This doesn't work - the filter is inserted, but it never runs.
> >
> > Read Ryan's response carefully (and my earlier responses this AM).  The
> > input filter can filter the request body (not headers).
>
> When I say "filter the headers" I mean "fiddle with the contents of
> r->headers_in" after the headers have been read in.
>
> The filter is actually a body filter, but the body is passed through
> untouched.

yes, the mod_header input filter is a body filter. It is not inserted (by
ap_run_insert_filter) until -AFTER- the request headers have been read in.
The filter is being inserted after the request headers have been read in but
before any body data is read in. GET requests don't contain a body so the
input filter is not getting called.

>
> > As I (and now Ryan) have pointed out, the only way to filter headers is
to
> > hook the pre_connection hook.  But this is a really bad idea for reasons
> > mentioned in both of our previous posts.  I think the right thing to do
with
> > mod_headers is to handle the HeaderIn processing in the request fixup
hook,
> > not as a filter.
>
> The input filter needs to run after the headers are read in and put in
> r->headers, but before the content generator runs.
>

A good place to do this is in the fixup stage.

Bill



Re: Allowing modules to add input filters is broken

Posted by Bill Stoddard <bi...@wstoddard.com>.
> > > If you use ap_add_input_filter() inside the ap_hook_insert_filter()
hook
> > > the filter doesn't run ever.
> > >
> > > The HeaderIn directive allows you to add or alter incoming headers
> > > before they are sent to modules like mod_proxy or mod_cgi.
> >
> > As I (and now Ryan) have pointed out, the only way to filter headers is
to
> > hook the pre_connection hook.  But this is a really bad idea for reasons
> > mentioned in both of our previous posts.  I think the right thing to do
with
> > mod_headers is to handle the HeaderIn processing in the request fixup
hook,
> > not as a filter.
>
> Why is this a bad idea?  The filter should be able to determine if it is
> required very quickly, and if it isn't, it just removes itself.  If it is,
> then it just does it's job.
>

As mentioned earlier, HeadersIn would be limited to server scope (and not
allowed in any containers).  Well, you could get complicated and walk the
conf tree but that seems to be much too complex a solution to what I believe
the problem to be.  Hooking fixup is all mod_headers really requires to
manipulate inbound headers. I could be wrong about that though :-)

Bill


Re: Allowing modules to add input filters is broken

Posted by rb...@covalent.net.
> > > Installing a filter that deals strictly with body data can be done
> during
> > > the insert_filter phase.  This is where output filters are commonly
> > > inserted into the chain.  Since this is always called before the handler
> > > phase, we are alright.
> >
> > This doesn't work - the filter is inserted, but it never runs.
>
> Read Ryan's response carefully (and my earlier responses this AM).  The
> input filter can filter the request body (not headers).
>
> >
> > > This logically makes sense, because modules that
> > > handle request body data do so by calling ap_get_client_block in the
> > > handler phase.  So, as long as the input filter is inserted before the
> > > handler is called everything is okay.
> > >
> > > Exactly what are you trying to do, and how is it not working?
> >
> > If you use ap_add_input_filter() inside the ap_hook_insert_filter() hook
> > the filter doesn't run ever.
> >
> > The HeaderIn directive allows you to add or alter incoming headers
> > before they are sent to modules like mod_proxy or mod_cgi.
>
> As I (and now Ryan) have pointed out, the only way to filter headers is to
> hook the pre_connection hook.  But this is a really bad idea for reasons
> mentioned in both of our previous posts.  I think the right thing to do with
> mod_headers is to handle the HeaderIn processing in the request fixup hook,
> not as a filter.

Why is this a bad idea?  The filter should be able to determine if it is
required very quickly, and if it isn't, it just removes itself.  If it is,
then it just does it's job.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: Allowing modules to add input filters is broken

Posted by Bill Stoddard <bi...@wstoddard.com>.
> rbb@covalent.net wrote:
>
> > Installing a filter that deals strictly with body data can be done
during
> > the insert_filter phase.  This is where output filters are commonly
> > inserted into the chain.  Since this is always called before the handler
> > phase, we are alright.
>
> This doesn't work - the filter is inserted, but it never runs.

Read Ryan's response carefully (and my earlier responses this AM).  The
input filter can filter the request body (not headers).

>
> > This logically makes sense, because modules that
> > handle request body data do so by calling ap_get_client_block in the
> > handler phase.  So, as long as the input filter is inserted before the
> > handler is called everything is okay.
> >
> > Exactly what are you trying to do, and how is it not working?
>
> If you use ap_add_input_filter() inside the ap_hook_insert_filter() hook
> the filter doesn't run ever.
>
> The HeaderIn directive allows you to add or alter incoming headers
> before they are sent to modules like mod_proxy or mod_cgi.
>

As I (and now Ryan) have pointed out, the only way to filter headers is to
hook the pre_connection hook.  But this is a really bad idea for reasons
mentioned in both of our previous posts.  I think the right thing to do with
mod_headers is to handle the HeaderIn processing in the request fixup hook,
not as a filter.

Bill



Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
rbb@covalent.net wrote:

> If it isn't running, then try changing the filter type.  The general
> problem is how we order filters.  I am 99.9% sure that the way we link
> request_filters with connection_filters is just plain wrong, but I haven't
> had time to really look at it yet.  I would bet that the problem you are
> having, is that you are installing the filter, and it is just getting
> skipped because of how we link the filters together.
> 
> What kind of filter are you inserting?  can you send me the
> ap_add_input_filter line?  I'll take a look and fix the problem.

It's like this:

ap_register_input_filter("FIXUP_HEADERS_IN", ap_headers_input_filter,
AP_FTYPE_CONTENT);
ap_add_input_filter("FIXUP_HEADERS_IN", NULL, r, r->connection);

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by rb...@covalent.net.
On Fri, 18 May 2001, Graham Leggett wrote:

> rbb@covalent.net wrote:
>
> > Installing a filter that deals strictly with body data can be done during
> > the insert_filter phase.  This is where output filters are commonly
> > inserted into the chain.  Since this is always called before the handler
> > phase, we are alright.
>
> This doesn't work - the filter is inserted, but it never runs.
>
> > This logically makes sense, because modules that
> > handle request body data do so by calling ap_get_client_block in the
> > handler phase.  So, as long as the input filter is inserted before the
> > handler is called everything is okay.
> >
> > Exactly what are you trying to do, and how is it not working?
>
> If you use ap_add_input_filter() inside the ap_hook_insert_filter() hook
> the filter doesn't run ever.

If it isn't running, then try changing the filter type.  The general
problem is how we order filters.  I am 99.9% sure that the way we link
request_filters with connection_filters is just plain wrong, but I haven't
had time to really look at it yet.  I would bet that the problem you are
having, is that you are installing the filter, and it is just getting
skipped because of how we link the filters together.

What kind of filter are you inserting?  can you send me the
ap_add_input_filter line?  I'll take a look and fix the problem.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: Allowing modules to add input filters is broken

Posted by Graham Leggett <mi...@sharp.fm>.
rbb@covalent.net wrote:

> Installing a filter that deals strictly with body data can be done during
> the insert_filter phase.  This is where output filters are commonly
> inserted into the chain.  Since this is always called before the handler
> phase, we are alright.

This doesn't work - the filter is inserted, but it never runs.

> This logically makes sense, because modules that
> handle request body data do so by calling ap_get_client_block in the
> handler phase.  So, as long as the input filter is inserted before the
> handler is called everything is okay.
> 
> Exactly what are you trying to do, and how is it not working?

If you use ap_add_input_filter() inside the ap_hook_insert_filter() hook
the filter doesn't run ever.

The HeaderIn directive allows you to add or alter incoming headers
before they are sent to modules like mod_proxy or mod_cgi.

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: Allowing modules to add input filters is broken

Posted by rb...@covalent.net.
Modules have no trouble at all inserting input filters that are used.  I
have modules that do so.  The thing is, there are two types of input
filters, ones that process headers and ones that process body data.  The
ones that process headers can process body data too.

In order for a module to insert a filter that works on headers, it must be
added for all requests, and thus it can be added in the pre_connection
phase.  This is because conditionally installing filters would logically
be based on some element of the request, but you don't know what the
request looks like until you have the headers read.  Chicken and egg.

Installing a filter that deals strictly with body data can be done during
the insert_filter phase.  This is where output filters are commonly
inserted into the chain.  Since this is always called before the handler
phase, we are alright.  This logically makes sense, because modules that
handle request body data do so by calling ap_get_client_block in the
handler phase.  So, as long as the input filter is inserted before the
handler is called everything is okay.

Exactly what are you trying to do, and how is it not working?

Ryan

On Fri, 18 May 2001, Bill Stoddard wrote:

> To the best of my knowledge, there is no way for a module to add input
> filters -- in time for them to be used during input processing --.   Sure a
> module can call add_input_filter, but the ap_run_insert_filter hook is run
> too late for input filters to have any effect.
>
> There are several different ways to fix this....
>
> 1.  Call ap_run_insert_filter in time to be considered by the inbound
> processing.  (right after get_mime_headers() for instance). Using a single,
> non parameterized function to add input and output filters to the stack with
> the same call is trying to do too much with this one function. I cannot
> point to a speficic problem this will cause but I am sure they exist.
>
> 2. Pass an additional parameter on ap_run_insert_filter designating whether
> modules should insert their input filters or output filters. Call
> ap_run_insert_filter(INSERT_INPUT_FILTERS) right after the call to
> get_mime_headers(). Call ap_run_insert_filter(INSERT_OUTPUT_FILTERS) where
> it is called today out of process_request_internal(). Modules would all have
> to grok this parameter.
>
> 3. Another way to solve the this is to create a two new hooks to replace the
> insert_filter hook:  ap_run_insert_input_filter() &
> ap_run_insert_output_filter() and call each hook at the appropriate point in
> the cycle.
>
> 2 or 3 work for me. Leaning toward #3 right now.
>
> Ryan and Greg, opinions?
>
> Bill
>
>
>


_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------