I had some offline feedback for my previous Location speeder-upper which I though had merit. That patch skips the directory walk when it detects a SetHandler directive inside of a Location block. The jist of the criticism was that some user might have a Location block with a URI that overlaps with something in the filesystem, despite what we say in the doc. I quote:

-----------------------------------------------------
When to use <Location>

Use <Location> to apply directives to content that lives outside the filesystem. For content that lives in the filesystem, use <Directory> and <Files>. An exception is <Location / >, which is an easy way to apply a configuration to the entire server.
-----------------------------------------------------


If there are sites out there that ignore the piece about the content living outside the filesystem, my previous patch which requires no new configuration might create problems. This version adds an OutsideFilesystem directive, only valid in <Location>, to eliminate the possibility for unpleasant surprises when upgrading httpd. It causes the directory walk to be skipped if set. btw, I'm thinking of this as a 2.1 thing which wouldn't be backported.

But then if I play devil's advocate, someone could see the new directive and turn it on when it's not appropriate and cause Bad Things to happen. Mainly I'm looking for comments on whether this should be configurable or not.

Thanks,
Greg
Index: include/http_core.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_core.h,v
retrieving revision 1.77
diff -u -d -b -r1.77 http_core.h
--- include/http_core.h 1 Jan 2004 13:26:16 -0000       1.77
+++ include/http_core.h 22 Jan 2004 21:31:51 -0000
@@ -367,6 +367,7 @@
      * won't actually be delivered as the response for the non-GET method.
      */
     int deliver_script;
+    int outside_filesystem; /* <Location > contains OutsideFilesystem */
 } core_request_config;
 
 /* Standard entries that are guaranteed to be accessible via
@@ -553,6 +554,8 @@
     unsigned int enable_sendfile : 2;  /* files in this dir can be mmap'ed */
     unsigned int allow_encoded_slashes : 1; /* URLs may contain %2f w/o being
                                              * pitched indiscriminately */
+    unsigned int outside_filesystem : 1; /* no point in doing dir walk
+                                          * if true */
 } core_dir_config;
 
 /* Per-server core configuration */
Index: server/core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.256
diff -u -d -b -r1.256 core.c
--- server/core.c       8 Jan 2004 18:56:20 -0000       1.256
+++ server/core.c       22 Jan 2004 21:31:51 -0000
@@ -181,6 +181,7 @@
     conf->enable_mmap = ENABLE_MMAP_UNSET;
     conf->enable_sendfile = ENABLE_SENDFILE_UNSET;
     conf->allow_encoded_slashes = 0;
+    conf->outside_filesystem = 0;
 
     return (void *)conf;
 }
@@ -448,6 +449,7 @@
     }
 
     conf->allow_encoded_slashes = new->allow_encoded_slashes;
+    conf->outside_filesystem = new->outside_filesystem;
     
     return (void*)conf;
 }
@@ -2336,6 +2338,23 @@
 }
 
 /*
+ * Declare that a <Location > section is outside of the filesystem
+ * so that directory walk and file walk may be safely bypassed
+ */
+static const char *set_outside_filesystem(cmd_parms *cmd, void *d_,
+                                          int arg)
+{
+    core_dir_config *d = (core_dir_config *)d_;
+    
+    if (!find_parent(cmd->directive, "<Location")) {
+        return apr_pstrcat(cmd->pool, cmd->cmd->name, 
+                           " is only valid within a <Location> section");
+    }
+    d->outside_filesystem = arg != 0;
+    return NULL;
+}
+
+/*
  * Handle a request to include the server's OS platform in the Server
  * response header field (the ServerTokens directive).  Unfortunately
  * this requires a new global in order to communicate the setting back to
@@ -3105,6 +3124,8 @@
   "The name of the default charset to add to any Content-Type without one or 'Off' to 
disable"),
 AP_INIT_TAKE1("AcceptPathInfo", set_accept_path_info, NULL, OR_FILEINFO,
   "Set to on or off for PATH_INFO to be accepted by handlers, or default for the 
per-handler preference"),
+AP_INIT_FLAG("OutsideFilesystem", set_outside_filesystem, NULL, OR_ALL,
+  "Set to On if <Location > container does not map to the filesystem"),
 
 /* Old resource config file commands */
 
@@ -3350,12 +3371,17 @@
 {
     int access_status;
 
+    core_request_config *rconf = ap_get_module_config(r->request_config,
+                                                      &core_module);
+        
+    if (!rconf->outside_filesystem) {
     if ((access_status = ap_directory_walk(r))) {
         return access_status;
     }
 
     if ((access_status = ap_file_walk(r))) {
         return access_status;
+        }
     }
 
     return OK;
Index: server/request.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/request.c,v
retrieving revision 1.132
diff -u -d -b -r1.132 request.c
--- server/request.c    1 Jan 2004 13:26:23 -0000       1.132
+++ server/request.c    22 Jan 2004 21:31:51 -0000
@@ -1330,9 +1330,22 @@
      * and note the end result to (potentially) skip this step next time.
      */
     if (now_merged) {
+        core_dir_config *dconf;
+        core_request_config *rconf;
+
         r->per_dir_config = ap_merge_per_dir_configs(r->pool,
                                                      r->per_dir_config,
                                                      now_merged);
+        /*
+         * Little known trivia:  a map_to_storage hook can't use
+         * anything set in a <Location> block because the per_dir_config
+         * is reset to the default before calling map_to_storage.
+         * 
+         * workaround: pass the stuff in the request_config
+         */
+        dconf = ap_get_module_config(r->per_dir_config, &core_module);
+        rconf = ap_get_module_config(r->request_config, &core_module);
+        rconf->outside_filesystem = dconf->outside_filesystem;        
     }
     cache->per_dir_result = r->per_dir_config;
 

Reply via email to