Hi,
I've encountered an issue with add_any_filter_handle(). Let me explain the
background.
My understanding of filter chains is that they form a linked list with three
entry points. The entry points correspond to resource filters, protocol
filters
and connection filters. Depending on the filters present, these entry points
can
overlap, but the order must be: resource filters <= protocol filters <=
connection filters.
I have a module that sets up a filter chain from scratch, with perhaps unusual
characteristics. I've noticed that if the chain has no resource filters and
you
try to add a protocol filter, then the protocol filter entry point moves to
accommodate new filter, but the resource filter entry point remains unchanged.
For example, before add_any_filter_handle() the chain looks like this:
filter1 (protocol) <- resource filters <- protocol filters
filter2 (connection) <- connection filters
After adding another protocol filter:
new filter(protocol) <- protocol filters
filter1 (protocol) <- resource filters
filter2 (connection) <- connection filters
I'd expect that traversing the chain starting at the resource filters entry
point would pass by every filter, hence the expected result would be:
new filter(protocol) <- resource filters <- protocol filters
filter1 (protocol)
filter2 (connection) <- connection filters
I've extracted the add_any_filter_handle code from httpd trunk and created a
unit test.
http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_filter.c?revision=965325&view=co
With my "patch" enabled, add_any_filter_handle() functions as I'd
expect. When the patch is disabled, via conditional compilation, the test2()
function fails.
I'd like to know if there's a valid reason for the current behaviour, or if the
fix I've posted is appropriate.
Thanks,
Paul
#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
typedef struct ap_filter_t ap_filter_t;
typedef int apr_pool_t;
typedef struct
{
ap_filter_t *input_filters;
ap_filter_t *proto_input_filters;
apr_pool_t *pool;
} request_rec;
typedef struct
{
ap_filter_t *input_filters;
apr_pool_t *pool;
} conn_rec;
typedef enum {
/** These filters are used to alter the content that is passed through
* them. Examples are SSI or PHP. */
AP_FTYPE_RESOURCE = 10,
/** These filters are used to alter the content as a whole, but after all
* AP_FTYPE_RESOURCE filters are executed. These filters should not
* change the content-type. An example is deflate. */
AP_FTYPE_CONTENT_SET = 20,
/** These filters are used to handle the protocol between server and
* client. Examples are HTTP and POP. */
AP_FTYPE_PROTOCOL = 30,
/** These filters implement transport encodings (e.g., chunking). */
AP_FTYPE_TRANSCODE = 40,
/** These filters will alter the content, but in ways that are
* more strongly associated with the connection. Examples are
* splitting an HTTP connection into multiple requests and
* buffering HTTP responses across multiple requests.
*
* It is important to note that these types of filters are not
* allowed in a sub-request. A sub-request's output can certainly
* be filtered by ::AP_FTYPE_RESOURCE filters, but all of the "final
* processing" is determined by the main request. */
AP_FTYPE_CONNECTION = 50,
/** These filters don't alter the content. They are responsible for
* sending/receiving data to/from the client. */
AP_FTYPE_NETWORK = 60
} ap_filter_type;
typedef struct
{
ap_filter_type ftype;
char *name;
} ap_filter_rec_t;
struct ap_filter_t
{
ap_filter_rec_t *frec;
struct ap_filter_t *next;
void *ctx;
request_rec *r;
conn_rec *c;
};
#define apr_palloc(a,b) malloc((b))
/* Should be a variadic function, but ap_log_error is only used twice
* and both times has only one ... argument
*/
#define ap_log_error(a,b,c,d,e,f) printf((e), (f))
#define APLOG_MARK __FILE__,__LINE__
#define APLOG_ERR 3
#define INSERT_BEFORE(f, before_this) ((before_this) == NULL \
|| (before_this)->frec->ftype > (f)->frec->ftype \
|| (before_this)->r != (f)->r)
static ap_filter_t *add_any_filter_handle(ap_filter_rec_t *frec, void *ctx,
request_rec *r, conn_rec *c,
ap_filter_t **r_filters,
ap_filter_t **p_filters,
ap_filter_t **c_filters)
{
apr_pool_t *p = frec->ftype < AP_FTYPE_CONNECTION && r ? r->pool : c->pool;
ap_filter_t *f = apr_palloc(p, sizeof(*f));
ap_filter_t **outf;
if (frec->ftype < AP_FTYPE_PROTOCOL) {
if (r) {
outf = r_filters;
}
else {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL,
"a content filter was added without a request: %s",
frec->name);
return NULL;
}
}
else if (frec->ftype < AP_FTYPE_CONNECTION) {
if (r) {
outf = p_filters;
}
else {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL,
"a protocol filter was added without a request: %s",
frec->name);
return NULL;
}
}
else {
outf = c_filters;
}
f->frec = frec;
f->ctx = ctx;
/* f->r must always be NULL for connection filters */
f->r = frec->ftype < AP_FTYPE_CONNECTION ? r : NULL;
f->c = c;
f->next = NULL;
if (INSERT_BEFORE(f, *outf)) {
f->next = *outf;
if (*outf) {
ap_filter_t *first = NULL;
if (r) {
/* If we are adding our first non-connection filter,
* Then don't try to find the right location, it is
* automatically first.
*/
if (*r_filters != *c_filters) {
first = *r_filters;
while (first && (first->next != (*outf))) {
first = first->next;
}
}
}
if (first && first != (*outf)) {
first->next = f;
}
}
#if 1 /* PATCH */
if (*r_filters == *p_filters && outf == p_filters) {
/* Adding a protocol filter when there are no
* resource filters. Therefore both r_filters
* and p_filters must be moved when inserting the
* new filter.
*/
*r_filters = f;
}
#endif
*outf = f;
}
else {
ap_filter_t *fscan = *outf;
while (!INSERT_BEFORE(f, fscan->next))
fscan = fscan->next;
f->next = fscan->next;
fscan->next = f;
}
if (frec->ftype < AP_FTYPE_CONNECTION && (*r_filters == *c_filters)) {
*r_filters = *p_filters;
}
return f;
}
void test1()
{
ap_filter_rec_t first = { AP_FTYPE_PROTOCOL, "first"};
ap_filter_rec_t second = { 35, "second"};
ap_filter_rec_t third = { AP_FTYPE_NETWORK, "third"};
ap_filter_rec_t add = { AP_FTYPE_PROTOCOL, "add" };
request_rec r;
conn_rec c;
ap_filter_t *chain;
int count = 0;
ap_filter_t chain3 = { &third, NULL, NULL, &r, &c};
ap_filter_t chain2 = { &second, &chain3, NULL, &r, &c};
ap_filter_t chain1 = { &first, &chain2, NULL, &r, &c};
printf("Test 1\n");
r.input_filters = &chain1;
r.proto_input_filters = &chain2;
c.input_filters = &chain3;
add_any_filter_handle(&add, NULL, &r, &c,
&r.input_filters, &r.proto_input_filters, &c.input_filters);
for (chain = r.input_filters;
chain;
chain = chain->next)
{
printf("Filter: %s %d\n", chain->frec->name, chain->frec->ftype);
count++;
}
assert(count==4);
}
void test2()
{
ap_filter_rec_t second = { 35, "second"};
ap_filter_rec_t third = { AP_FTYPE_NETWORK, "third"};
ap_filter_rec_t add = { AP_FTYPE_PROTOCOL, "add" };
request_rec r;
conn_rec c;
ap_filter_t *chain;
int count = 0;
ap_filter_t chain3 = { &third, NULL, NULL, &r, &c};
ap_filter_t chain2 = { &second, &chain3, NULL, &r, &c};
r.input_filters = &chain2;
r.proto_input_filters = &chain2;
c.input_filters = &chain3;
printf("Test 2\n");
add_any_filter_handle(&add, NULL, &r, &c,
&r.input_filters, &r.proto_input_filters, &c.input_filters);
for (chain = r.input_filters;
chain;
chain = chain->next)
{
printf("Filter: %s %d\n", chain->frec->name, chain->frec->ftype);
count++;
}
assert(count==3);
}
int main(int argc, char *argv[])
{
test1();
test2();
return 0;
}
