From: William A. Rowe Jr Date: Sun, 26 Aug 2001 05:10:17 +0000 (+0000) Subject: Eliminate proxy: (and all other 'special') processing from the X-Git-Tag: 2.0.25~49 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7552c9cb16a58a708b327ae8e8f8e7a85ef07bab;p=apache Eliminate proxy: (and all other 'special') processing from the ap_directory_walk() phase. Modules that want to use special walk logic should refer to the mod_proxy map_to_location example, with it's proxy_walk and proxysection implementation. This makes either directory_walk flavor much more legible, since that phase only runs against real blocks. On a technical note, this patch also forces the Directory to be canonical (unless it is "/" or a regex.) It also allows us to be more explicit when declaring block errors. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@90684 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index de0d1f28d6..54717adddf 100644 --- a/CHANGES +++ b/CHANGES @@ -1,8 +1,12 @@ Changes with Apache 2.0.25-dev - *) Fix a security problem in mod_include which would allow - an SSI document to be passed to the client unparsed. - [Cliff Woolley, Brian Pane, William Rowe] + *) Eliminate proxy: (and all other 'special') processing from the + ap_directory_walk() phase. Modules that want to use special + walk logic should refer to the mod_proxy map_to_location example, + with it's proxy_walk and proxysection implementation. This makes + either directory_walk flavor much more legible, since that phase + only runs against real blocks. + [William Rowe] *) Introduce the map_to_storage hook, which allows modules to bypass the directory_walk and file_walk for non-file requests. TRACE diff --git a/include/http_core.h b/include/http_core.h index af7697d53a..1193fb37e0 100644 --- a/include/http_core.h +++ b/include/http_core.h @@ -426,12 +426,6 @@ typedef struct { */ unsigned d_is_fnmatch : 1; - /* since is_absolute(conf->d) was being called so frequently in - * directory_walk() and its relatives, this field was created and - * is set to the result of that call. - */ - unsigned d_is_absolute : 1; - /* should we force a charset on any outgoing parameterless content-type? * if so, which charset? */ diff --git a/modules/mappers/mod_negotiation.c b/modules/mappers/mod_negotiation.c index 46bed0326e..35f68a29e3 100644 --- a/modules/mappers/mod_negotiation.c +++ b/modules/mappers/mod_negotiation.c @@ -1027,6 +1027,7 @@ static int read_types_multi(negotiation_state *neg) return DECLINED; /* Weird... */ } + /* XXX this should be more general, and quit using 'specials' */ if (strncmp(r->filename, "proxy:", 6) == 0) { return DECLINED; } diff --git a/server/core.c b/server/core.c index 6598e6b799..9b3d25bde4 100644 --- a/server/core.c +++ b/server/core.c @@ -123,29 +123,8 @@ static void *create_core_dir_config(apr_pool_t *a, char *dir) core_dir_config *conf; conf = (core_dir_config *)apr_pcalloc(a, sizeof(core_dir_config)); - if (!dir || dir[strlen(dir) - 1] == '/') { - conf->d = dir; - } - else if (strncmp(dir, "proxy:", 6) == 0) { - conf->d = apr_pstrdup(a, dir); - } - else { - conf->d = apr_pstrcat(a, dir, "/", NULL); - } - conf->d_is_fnmatch = conf->d ? (apr_is_fnmatch(conf->d) != 0) : 0; - - /* On all platforms, "/" is (at minimum) a faux root */ - conf->d_is_absolute = conf->d ? (ap_os_is_path_absolute(a, conf->d) - || (strcmp(conf->d, "/") == 0)) : 0; - - /* Make this explicit - the "/" root has 0 elements, that is, we - * will always merge it, and it will always sort and merge first. - * All others are sorted and tested by the number of slashes. - */ - if (!conf->d || strcmp(conf->d, "/") == 0) - conf->d_components = 0; - else - conf->d_components = ap_count_dirs(conf->d); + + /* conf->r and conf->d[_*] are initialized in */ conf->opts = dir ? OPT_UNSET : OPT_UNSET|OPT_ALL; conf->opts_add = conf->opts_remove = OPT_NONE; @@ -205,7 +184,6 @@ static void *merge_core_dir_configs(apr_pool_t *a, void *basev, void *newv) conf->d = new->d; conf->d_is_fnmatch = new->d_is_fnmatch; - conf->d_is_absolute = new->d_is_absolute; conf->d_components = new->d_components; conf->r = new->r; @@ -397,37 +375,10 @@ AP_CORE_DECLARE(void) ap_add_file_conf(core_dir_config *conf, void *url_config) *new_space = url_config; } -/* core_reorder_directories reorders the directory sections such that the - * 1-component sections come first, then the 2-component, and so on, finally - * followed by the "special" sections. A section is "special" if it's a regex, - * or if it doesn't start with / -- consider proxy: matching. All movements - * are in-order to preserve the ordering of the sections from the config files. - * See directory_walk(). - */ - -#if defined(HAVE_DRIVE_LETTERS) -#define IS_SPECIAL(entry_core) \ - ((entry_core)->r != NULL \ - || ((entry_core)->d[0] != '/' && (entry_core)->d[1] != ':')) -#elif defined(NETWARE) -/* XXX: Fairly certain this is correct... '/' must prefix the path - * or else in the case xyz:/ or abc/xyz:/, '/' must follow the ':'. - * If there is no leading '/' or embedded ':/', then we are special. - */ -#define IS_SPECIAL(entry_core) \ - ((entry_core)->r != NULL \ - || ((entry_core)->d[0] != '/' \ - && strchr((entry_core)->d, ':') \ - && *(strchr((entry_core)->d, ':') + 1) != '/')) -#else -#define IS_SPECIAL(entry_core) \ - ((entry_core)->r != NULL || (entry_core)->d[0] != '/') -#endif - /* We need to do a stable sort, qsort isn't stable. So to make it stable * we'll be maintaining the original index into the list, and using it * as the minor key during sorting. The major key is the number of - * components (where a "special" section has infinite components). + * components (where the root component is zero). */ struct reorder_sort_rec { ap_conf_vector_t *elt; @@ -443,26 +394,22 @@ static int reorder_sorter(const void *va, const void *vb) core_a = ap_get_module_config(a->elt, &core_module); core_b = ap_get_module_config(b->elt, &core_module); - if (IS_SPECIAL(core_a)) { - if (!IS_SPECIAL(core_b)) { - return 1; - } + + if (core_a->r < core_b->r) { + return -1; } - else if (IS_SPECIAL(core_b)) { - return -1; + else if (core_a->r > core_b->r) { + return 1; } - else { - /* we know they're both not special */ - if (core_a->d_components < core_b->d_components) { - return -1; - } - else if (core_a->d_components > core_b->d_components) { - return 1; - } + if (core_a->d_components < core_b->d_components) { + return -1; + } + else if (core_a->d_components > core_b->d_components) { + return 1; } - /* Either they're both special, or they're both not special and have the - * same number of components. In any event, we now have to compare - * the minor key. */ + /* They have the same number of components, we now have to compare + * the minor key to maintain the original order. + */ return a->orig_index - b->orig_index; } @@ -1571,30 +1518,29 @@ static const char *dirsection(cmd_parms *cmd, void *mconfig, const char *arg) cmd->path = ap_getword_conf(cmd->pool, &arg); cmd->override = OR_ALL|ACCESS_CONF; - if (thiscmd->cmd_data) { /* */ - r = ap_pregcomp(cmd->pool, cmd->path, REG_EXTENDED|USE_ICASE); - } - else if (!strcmp(cmd->path, "~")) { + if (!strcmp(cmd->path, "~")) { cmd->path = ap_getword_conf(cmd->pool, &arg); if (!cmd->path) return " block must specify a path"; - r = ap_pregcomp(cmd->pool, cmd->path, REG_EXTENDED|USE_ICASE); } - else if (strcmp(cmd->path, "/") == 0) { - /* Treat 'default' path "/" as the inalienable root */ - cmd->path = apr_pstrdup(cmd->pool, cmd->path); + else if (thiscmd->cmd_data) { /* */ + r = ap_pregcomp(cmd->pool, cmd->path, REG_EXTENDED|USE_ICASE); } - else { - char *newpath; - /* Ensure that the pathname is canonical */ - if (apr_filepath_merge(&newpath, NULL, cmd->path, - APR_FILEPATH_TRUENAME, cmd->pool) != APR_SUCCESS) { - return apr_pstrcat(cmd->pool, "path, - "\"> path is invalid.", NULL); + else if (cmd->path[strlen(cmd->path) - 1] != '/') { + cmd->path = apr_pstrcat(cmd->pool, cmd->path, "/"); + + if (!strcmp(cmd->path, "/") == 0) + { + char *newpath; + /* Ensure that the pathname is canonical */ + if (apr_filepath_merge(&newpath, NULL, cmd->path, + APR_FILEPATH_TRUENAME, cmd->pool) != APR_SUCCESS) { + return apr_pstrcat(cmd->pool, "path, + "\"> path is invalid.", NULL); + } + cmd->path = newpath; } - cmd->path = newpath; } - /* initialize our config and fetch it */ conf = ap_set_config_vectors(cmd->server, new_dir_conf, cmd->path, &core_module, cmd->pool); @@ -1604,6 +1550,17 @@ static const char *dirsection(cmd_parms *cmd, void *mconfig, const char *arg) return errmsg; conf->r = r; + conf->d = cmd->path; + conf->d_is_fnmatch = (apr_is_fnmatch(conf->d) != 0); + + /* Make this explicit - the "/" root has 0 elements, that is, we + * will always merge it, and it will always sort and merge first. + * All others are sorted and tested by the number of slashes. + */ + if (strcmp(conf->d, "/") == 0) + conf->d_components = 0; + else + conf->d_components = ap_count_dirs(conf->d); ap_add_per_dir_conf(cmd->server, new_dir_conf); @@ -1659,9 +1616,8 @@ static const char *urlsection(cmd_parms *cmd, void *mconfig, const char *arg) if (errmsg != NULL) return errmsg; - conf->d = apr_pstrdup(cmd->pool, cmd->path); /* No mangling, please */ + conf->d = apr_pstrdup(cmd->pool, cmd->path); /* No mangling, please */ conf->d_is_fnmatch = apr_is_fnmatch(conf->d) != 0; - conf->d_is_absolute = (conf->d && (*conf->d == '/')); conf->r = r; ap_add_per_url_conf(cmd->server, new_url_conf); @@ -1734,7 +1690,6 @@ static const char *filesection(cmd_parms *cmd, void *mconfig, const char *arg) conf->d = cmd->path; conf->d_is_fnmatch = apr_is_fnmatch(conf->d) != 0; - conf->d_is_absolute = 0; conf->r = r; ap_add_file_conf(c, new_file_conf); @@ -2910,6 +2865,9 @@ AP_DECLARE_NONSTD(int) ap_core_translate(request_rec *r) void *sconf = r->server->module_config; core_server_config *conf = ap_get_module_config(sconf, &core_module); + /* XXX this seems too specific, this should probably become + * some general-case test + */ if (r->proxyreq) { return HTTP_FORBIDDEN; } diff --git a/server/request.c b/server/request.c index ef7041456f..5fe486f029 100644 --- a/server/request.c +++ b/server/request.c @@ -400,18 +400,16 @@ AP_DECLARE(int) ap_directory_walk(request_rec *r) core_dir_config *entry_core; /* - * Are we dealing with a file? If not, we can (hopefuly) safely assume we - * have a handler that doesn't require one, but for safety's sake, and so - * we have something find_types() can get something out of, fake one. But - * don't run through the directory entries. + * Are we dealing with a file? If not, the handler needed to register + * a hook to escape from our walking the file. Since they haven't, we + * are going to assume the worst and refuse to proceed. */ - - if (r->filename == NULL) { - r->filename = apr_pstrdup(r->pool, r->uri); - r->finfo.filetype = APR_NOFILE; - r->per_dir_config = per_dir_defaults; - - return OK; + if (r->filename == NULL || !ap_os_is_path_absolute(r->pool, r->filename)) { + ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r, + "Module bug? Request filename path %s is missing or " + "or not absolute for uri %s", + r->filename ? r->filename : "", r->uri); + return HTTP_INTERNAL_SERVER_ERROR; } /* @@ -421,44 +419,6 @@ AP_DECLARE(int) ap_directory_walk(request_rec *r) * around, see why, and adjust the lookup_rec accordingly --- this might * save us a call to get_path_info (with the attendant stat()s); however, * for the moment, that's not worth the trouble. - * - * Fake filenames (i.e. proxy:) only match Directory sections. - */ - - if (!ap_os_is_path_absolute(r->pool, r->filename)) - { - const char *entry_dir; - - for (j = 0; j < num_sec; ++j) { - - entry_config = sec_dir[j]; - entry_core = ap_get_module_config(entry_config, &core_module); - entry_dir = entry_core->d; - - this_conf = NULL; - if (entry_core->r) { - if (!ap_regexec(entry_core->r, r->filename, 0, NULL, 0)) - this_conf = entry_config; - } - else if (entry_core->d_is_fnmatch) { - if (!apr_fnmatch(entry_dir, r->filename, 0)) - this_conf = entry_config; - } - else if (!strncmp(r->filename, entry_dir, strlen(entry_dir))) - this_conf = entry_config; - - if (this_conf) - per_dir_defaults = ap_merge_per_dir_configs(r->pool, - per_dir_defaults, - this_conf); - } - - r->per_dir_config = per_dir_defaults; - - return OK; - } - - /* The replacement code [below] for directory_walk eliminates this issue. */ res = get_path_info(r); if (res != OK) { @@ -532,9 +492,7 @@ AP_DECLARE(int) ap_directory_walk(request_rec *r) */ test_dirname = apr_palloc(r->pool, test_filename_len + 2); - /* XXX These exception cases go away if apr_stat() returns the - * APR_PATHINCOMPLETE status, so we skip hard filesystem testing - * of the initial 'pseudo' elements: + /* XXX The garbage below disappears in the new directory_walk; */ #if defined(HAVE_UNC_PATHS) @@ -595,8 +553,7 @@ AP_DECLARE(int) ap_directory_walk(request_rec *r) entry_core = ap_get_module_config(entry_config, &core_module); entry_dir = entry_core->d; - if (entry_core->r || !entry_core->d_is_absolute - || entry_core->d_components > i) + if (entry_core->r || entry_core->d_components > i) break; this_conf = NULL; @@ -647,21 +604,20 @@ AP_DECLARE(int) ap_directory_walk(request_rec *r) } /* - * There's two types of IS_SPECIAL sections (see http_core.c), and we've - * already handled the proxy:-style stuff. Now we'll deal with the - * regexes. + * Now we'll deal with the regexes. */ for (; j < num_sec; ++j) { entry_config = sec_dir[j]; entry_core = ap_get_module_config(entry_config, &core_module); - if (entry_core->r) { - if (!ap_regexec(entry_core->r, test_dirname, 0, NULL, REG_NOTEOL)) { - per_dir_defaults = ap_merge_per_dir_configs(r->pool, - per_dir_defaults, - entry_config); - } + if (!entry_core->r) { + continue; + } + if (!ap_regexec(entry_core->r, test_dirname, 0, NULL, REG_NOTEOL)) { + per_dir_defaults = ap_merge_per_dir_configs(r->pool, + per_dir_defaults, + entry_config); } } r->per_dir_config = per_dir_defaults; @@ -721,18 +677,16 @@ AP_DECLARE(int) ap_directory_walk(request_rec *r) char *delim; /* - * Are we dealing with a file? If not, we simply assume we have a - * handler that doesn't require one, but for safety's sake, and so - * we have something find_types() can get something out of, fake - * one. But don't run through the directory entries. + * Are we dealing with a file? If not, the handler needed to register + * a hook to escape from our walking the file. Since they haven't, we + * are going to assume the worst and refuse to proceed. */ - - if (r->filename == NULL) { - r->filename = apr_pstrdup(r->pool, r->uri); - r->finfo.filetype = APR_NOFILE; - r->per_dir_config = per_dir_defaults; - - return OK; + if (r->filename == NULL || !ap_os_is_path_absolute(r->filename)) { + ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r, + "Module bug? Request filename path %s is missing or " + "or not absolute for uri %s", + r->filename ? r->filename : "", r->uri); + return HTTP_INTERNAL_SERVER_ERROR; } /* @@ -760,47 +714,14 @@ AP_DECLARE(int) ap_directory_walk(request_rec *r) r->filename = buf; r->finfo.valid = APR_FINFO_TYPE; r->finfo.filetype = APR_DIR; /* It's the root, of course it's a dir */ - } - else { - const char *entry_dir; - - /* Fake filenames (i.e. proxy:) only match Directory sections. - */ - if (rv != APR_EBADPATH) - return HTTP_FORBIDDEN; - } - - for (sec_idx = 0; sec_idx < num_sec; ++sec_idx) { - - entry_config = sec_ent[sec_idx]; - entry_core = ap_get_module_config(entry_config, &core_module); - entry_dir = entry_core->d; - - this_conf = NULL; - if (entry_core->r) { - if (!ap_regexec(entry_core->r, r->filename, 0, NULL, 0)) - this_conf = entry_config; - } - else if (entry_core->d_is_fnmatch) { - /* XXX: Gut instinct tells me this could be very, very nasty, - * have we thought through what 'faux' resource might be - * case senstitive or not? - */ - if (!apr_fnmatch(entry_dir, r->filename, 0)) - this_conf = entry_config; - } - else if (!strncmp(r->filename, entry_dir, strlen(entry_dir))) - this_conf = entry_config; - - if (this_conf) - per_dir_defaults = ap_merge_per_dir_configs(r->pool, - per_dir_defaults, - this_conf); + } else { + if (r->filename == NULL || !ap_os_is_path_absolute(r->filename)) { + ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r, + "Config bug? Request filename path %s is invalid or " + "or not absolute for uri %s", + r->filename, r->uri); + return HTTP_INTERNAL_SERVER_ERROR; } - - r->per_dir_config = per_dir_defaults; - - return OK; } /* @@ -833,8 +754,7 @@ AP_DECLARE(int) ap_directory_walk(request_rec *r) /* No more possible matches for this many segments? * We are done when we find relative/regex/longer components. */ - if (entry_core->r || !entry_core->d_is_absolute - || entry_core->d_components > seg) + if (entry_core->r || entry_core->d_components > seg) break; /* We will never skip '0' element components, e.g. plain old @@ -993,21 +913,20 @@ AP_DECLARE(int) ap_directory_walk(request_rec *r) } while (r->finfo.filetype == APR_DIR); /* - * There's two types of IS_SPECIAL sections (see http_core.c), and we've - * already handled the proxy:-style stuff. Now we'll deal with the - * regexes. + * Now we'll deal with the regexes. */ for (; sec_idx < num_sec; ++sec_idx) { entry_config = sec_ent[sec_idx]; entry_core = ap_get_module_config(entry_config, &core_module); - if (entry_core->r) { - if (!ap_regexec(entry_core->r, r->filename, 0, NULL, REG_NOTEOL)) { - per_dir_defaults = ap_merge_per_dir_configs(r->pool, - per_dir_defaults, - entry_config); - } + if (!entry_core->r) { + continue; + } + if (!ap_regexec(entry_core->r, r->filename, 0, NULL, REG_NOTEOL)) { + per_dir_defaults = ap_merge_per_dir_configs(r->pool, + per_dir_defaults, + entry_config); } } r->per_dir_config = per_dir_defaults;