On 02.12.2007 02:10, Graham Leggett wrote:
Nick Kew wrote:
You created a new kept_body input filter, which seems exactly right
to me. But you're storing the kept body on the request_rec itself,
which looks like extra complexity.
Why not make the new filter into its own module, and keep the kept_body
brigade on the filter's own config? That way the complexity of dealing
with top-level vs other requests is kept in one place. It's not clear
to me from the changes in http_filter.h that this patch deals with
that distinction (or is kept_body supposed to be able to originate
in non-origin requests)?
The kept body stuff is formed of two parts, the bit that sets aside the
body (in ap_discard_request_body()), and the filter that puts it back
again. It didn't make sense to separate the two functions, which is why
they are both in the same source file, the http module http_filters.c.
On principle, we should be reducing the request_rec, not adding to it!
If we can find a way to avoid it, certainly.
The kept body filter is only added on subrequests, and only where
r->kept_body is not-NULL, and subrequests as I recall have their own
filter stack separate from the main filter stack. So as I understand it,
linking it to the subrequest filter stack may not work, as the
subrequest filter stack doesn't exist when ap_discard_request_body() is
called.
First of all sorry for commenting on this that late. I should have paid more
attention back in December.
While it is true that the filter stack branches under certain conditions for
output filters, the same is not true for input filters. Given Joe's recent true
comment that ap_discard_request_body pulls all *through* the input filter chain
the correct (or more correct) solution for the problem seems to be to
1. Move the saving of the request body from ap_discard_request_body to a
separate input filter that gets implemented by a module.
2. Use the insert filter_hook to insert this filter whenever configured.
3. I guess the filter can decide on its own what to do: If it is a subrequest
pulling from it, then deliver from the buffered brigade otherwise buffer it
and pass it on. Of course it has to paid attention in this case for
incomplete request bodies and how to handle this.
IMHO this would make it possible to remove this stuff from the core and from
the request_rec where it does not really belong.
Regards
Rüdiger
