]> granicus.if.org Git - apache/commitdiff
Eliminate proxy: (and all other 'special') processing from the
authorWilliam A. Rowe Jr <wrowe@apache.org>
Sun, 26 Aug 2001 05:10:17 +0000 (05:10 +0000)
committerWilliam A. Rowe Jr <wrowe@apache.org>
Sun, 26 Aug 2001 05:10:17 +0000 (05:10 +0000)
  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 <Directory > 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 <Directory > block errors.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@90684 13f79535-47bb-0310-9956-ffa450edef68

CHANGES
include/http_core.h
modules/mappers/mod_negotiation.c
server/core.c
server/request.c

diff --git a/CHANGES b/CHANGES
index de0d1f28d6f7261be3bd707305840a5a883b6214..54717adddfff5ef8773b3a516b0bdba01b22be07 100644 (file)
--- 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 <Directory > 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
index af7697d53ad9c5026cac906324b8a0c4df850575..1193fb37e0b4b4d9bf2ca633da288313be772a8f 100644 (file)
@@ -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?
      */
index 46bed0326e9b73c3b96512d141a23260dcec82c3..35f68a29e39c43a559945fa10333c71af8194689 100644 (file)
@@ -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;
     }
index 6598e6b79995052b55e0dc94e832dedc88c3bfaf..9b3d25bde46cdf050e6c4f5d97d7baf3430cf1cb 100644 (file)
@@ -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) { /* <DirectoryMatch> */
-       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 "<Directory ~ > 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) { /* <DirectoryMatch> */
+       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, "<Directory \"", cmd->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, "<Directory \"", cmd->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;
     }
index ef7041456f81c052b22b6c19486626d9f6c8be95..5fe486f0296f49d1f108166f7624bea6b8b7a26b 100644 (file)
@@ -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 : "<NULL>", 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 : "<NULL>", 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;