dgaudet 98/03/15 13:39:56
Modified: htdocs/manual/mod mod_setenvif.html src CHANGES src/modules/standard mod_setenvif.c Log: - The "merging" optimization in mod_setenvif did not deal with SetEnvIfNoCase properly; and it used strcmp() to compare header names when it should use strcasecmp(). - Change the merging optimization so that it only considers the most recent setenvif for merging. This means that mod_setenvif will consider all directives in the order they appear in the config file. - Document that mod_setenvif considers directives in the order they appear, and give an example use. - Perform more comparisons at compile-time in order to speed up things at compile-time. Revision Changes Path 1.3 +10 -0 apache-1.3/htdocs/manual/mod/mod_setenvif.html Index: mod_setenvif.html =================================================================== RCS file: /export/home/cvs/apache-1.3/htdocs/manual/mod/mod_setenvif.html,v retrieving revision 1.2 retrieving revision 1.3 diff -u -r1.2 -r1.3 --- mod_setenvif.html 1998/01/28 19:11:57 1.2 +++ mod_setenvif.html 1998/03/15 21:39:50 1.3 @@ -26,6 +26,16 @@ regular expressions you specify. These envariables can be used by other parts of the server to make decisions about actions to be taken. </P> + <p>The directives are considered in the order they appear in the + configuration files. So more complex sequences can be used, such + as this example, which sets <code>netscape</code> if the browser + is mozilla but not MSIE. + <blockquote><pre> + BrowserMatch ^Mozilla netscape + BrowserMatch MSIE !netscape + </pre></blockquote> + </p> + <H2>Directives</H2> <UL> <LI><A HREF="#BrowserMatch">BrowserMatch</A> 1.712 +13 -1 apache-1.3/src/CHANGES Index: CHANGES =================================================================== RCS file: /export/home/cvs/apache-1.3/src/CHANGES,v retrieving revision 1.711 retrieving revision 1.712 diff -u -r1.711 -r1.712 --- CHANGES 1998/03/14 16:25:44 1.711 +++ CHANGES 1998/03/15 21:39:52 1.712 @@ -1,5 +1,17 @@ Changes with Apache 1.3b6 - + + *) Clean up some undocumented behaviour of mod_setenvif related to + "merging" two SetEnvIf directives when they match the same header + and regex. Document that mod_setenvif will perform comparisons in + the order they appear in the config file. Optimize mod_setenvif by + doing more work at config time rather than at runtime. + [Dean Gaudet] + + *) mod_setenvif would incorrectly merge a SetEnvIf and SetEnvIfNoCase (or + BrowserMatch and BrowserMatchNoCase) when they matched the same header + and regex. Fix this; but also fix it so that this merging optimization + only happens + *) src/include/ap_config.h now wraps it's #define's with #ifndef/#endif's to allow for modules to overrule them and to reduce redefinition warnings [Jim Jagielski] 1.19 +96 -37 apache-1.3/src/modules/standard/mod_setenvif.c Index: mod_setenvif.c =================================================================== RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_setenvif.c,v retrieving revision 1.18 retrieving revision 1.19 diff -u -r1.18 -r1.19 --- mod_setenvif.c 1998/03/13 19:20:43 1.18 +++ mod_setenvif.c 1998/03/15 21:39:55 1.19 @@ -115,11 +115,21 @@ #include "http_core.h" #include "http_log.h" +enum special { + SPECIAL_NOT, + SPECIAL_REMOTE_ADDR, + SPECIAL_REMOTE_HOST, + SPECIAL_REMOTE_USER, + SPECIAL_REQUEST_URI, + SPECIAL_REQUEST_METHOD +}; typedef struct { char *name; /* header name */ char *regex; /* regex to match against */ regex_t *preg; /* compiled regex */ table *features; /* env vars to set (or unset) */ + enum special special_type : 4; /* is it a "special" header ? */ + unsigned icase : 1; /* ignoring case? */ } sei_entry; typedef struct { @@ -161,6 +171,7 @@ char *var; int i; int beenhere = 0; + unsigned icase; /* get regex */ regex = getword_conf(cmd->pool, &args); @@ -170,33 +181,68 @@ } /* - * First, try to merge into an existing entry + * If we've already got a sei_entry with the same name we want to + * just copy the name pointer... so that later on we can compare + * two header names just by comparing the pointers. */ for (i = 0; i < sconf->conditionals->nelts; ++i) { new = &entries[i]; - if (!strcmp(new->name, fname) && !strcmp(new->regex, regex)) - goto gotit; + if (!strcasecmp(new->name, fname)) { + fname = new->name; + break; + } } - /* - * If none was found, create a new entry + /* if the last entry has an idential headername and regex then + * merge with it */ - - new = push_array(sconf->conditionals); - new->name = fname; - new->regex = regex; - new->preg = pregcomp(cmd->pool, regex, - (REG_EXTENDED | REG_NOSUB - | (cmd->info == ICASE_MAGIC ? REG_ICASE : 0))); - if (new->preg == NULL) { - return pstrcat(cmd->pool, cmd->cmd->name, - " regex could not be compiled.", NULL); + i = sconf->conditionals->nelts - 1; + icase = cmd->info == ICASE_MAGIC; + if (i < 0 + || entries[i].name != fname + || entries[i].icase != icase + || strcmp(entries[i].regex, regex)) { + + /* no match, create a new entry */ + + new = push_array(sconf->conditionals); + new->name = fname; + new->regex = regex; + new->icase = icase; + new->preg = pregcomp(cmd->pool, regex, + (REG_EXTENDED | REG_NOSUB + | (icase ? REG_ICASE : 0))); + if (new->preg == NULL) { + return pstrcat(cmd->pool, cmd->cmd->name, + " regex could not be compiled.", NULL); + } + new->features = make_table(cmd->pool, 2); + + if (!strcasecmp(fname, "remote_addr")) { + new->special_type = SPECIAL_REMOTE_ADDR; + } + else if (!strcasecmp(fname, "remote_host")) { + new->special_type = SPECIAL_REMOTE_HOST; + } + else if (!strcasecmp(fname, "remote_user")) { + new->special_type = SPECIAL_REMOTE_USER; + } + else if (!strcasecmp(fname, "request_uri")) { + new->special_type = SPECIAL_REQUEST_URI; + } + else if (!strcasecmp(fname, "request_method")) { + new->special_type = SPECIAL_REQUEST_METHOD; + } + else { + new->special_type = SPECIAL_NOT; + } + } + else { + new = &entries[i]; } - new->features = make_table(cmd->pool, 5); -gotit: - for( ; ; ) { + for (;;) { feature = getword_conf(cmd->pool, &args); if(!*feature) break; @@ -265,30 +311,43 @@ &setenvif_module); sei_entry *entries = (sei_entry *) sconf->conditionals->elts; table_entry *elts; - char *val; + const char *val; int i, j; + char *last_name; + last_name = NULL; + val = NULL; for (i = 0; i < sconf->conditionals->nelts; ++i) { sei_entry *b = &entries[i]; - if (!strcasecmp(b->name, "remote_addr")) { - val = r->connection->remote_ip; - } - else if (!strcasecmp(b->name, "remote_host")) { - val = (char *) get_remote_host(r->connection, r->per_dir_config, - REMOTE_NAME); - } - else if (!strcasecmp(b->name, "remote_user")) { - val = r->connection->user; - } - else if (!strcasecmp(b->name, "request_uri")) { - val = r->uri; - } - else if (!strcasecmp(b->name, "request_method")) { - val = r->method; - } - else { - val = table_get(r->headers_in, b->name); + /* Optimize the case where a bunch of directives in a row use the + * same header. Remember we don't need to strcmp the two header + * names because we made sure the pointers were equal during + * configuration. + */ + if (b->name != last_name) { + last_name = b->name; + switch (b->special_type) { + case SPECIAL_REMOTE_ADDR: + val = r->connection->remote_ip; + break; + case SPECIAL_REMOTE_HOST: + val = get_remote_host(r->connection, r->per_dir_config, + REMOTE_NAME); + break; + case SPECIAL_REMOTE_USER: + val = r->connection->user; + break; + case SPECIAL_REQUEST_URI: + val = r->uri; + break; + case SPECIAL_REQUEST_METHOD: + val = r->method; + break; + case SPECIAL_NOT: + val = table_get(r->headers_in, b->name); + break; + } } if (!val) {