In discussions about how to properly handle 304 (not modified), it
seems to me that we need a way for a filter to explicitly run some
code before the handler (and hence any data is generated) is
executed.
So, I propose adding another argument to the register functions -
typedef int (*ap_init_filter_func)(ap_filter_t *f);
ap_register_output_filter will look like:
AP_DECLARE(ap_filter_rec_t *) ap_register_output_filter(const char *name,
ap_out_filter_func filter_func,
ap_init_filter_func filter_init,
ap_filter_type ftype)
ap_register_input_filter will look like:
AP_DECLARE(ap_filter_rec_t *) ap_register_input_filter(const char *name,
ap_in_filter_func filter_func,
ap_init_filter_func filter_init,
ap_filter_type ftype);
For both functions, the filter_init may be NULL.
Then, in ap_invoke_handler(), we walk all of the filter chains and
iff the filter_rec has a init function, we call it. This way only
the filters that are involved in the request are called and they
have a chance to tweak the request_rec (or anything else) before the
actual data is generated.
Now, the question remains what do we do here in this init function.
My case is that we could in theory move the context-initialization
code for most filters to this new function. (The case where !f->ctx
is handled is either too complicated or incorrect in a lot of our
modules. This would simplify the core filter operations for a lot
of modules.)
However, this isn't required and current modules can just add NULL to
their parameter list, but it allows for modules like php and
mod_include to set r->no_local_copy so that the handler can call
ap_meets_conditions() before any data is generated and the 304 case
will be ignored properly. So, mod_include's register line could be:
ap_register_output_filter("INCLUDES", includes_filter, includes_setup,
AP_FTYPE_RESOURCE);
where the bare minimum includes_setup function is:
apr_status_t includes_setup(ap_filter_t *f) {
f->r->no_local_copy = 1;
}
(I would also move the context-initialization code, but it isn't
strictly required.)
This is the best and cleanest solution I can come up with. Does
anyone see any problems with this? I will most likely take a pass
at implementing this tonight and post a patch for review. -- justin
