You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Bill Stoddard <bi...@wstoddard.com> on 2002/03/22 20:51:07 UTC
[PATCH] default_handler and no directory_walk
Here is a patch to error out if the default handler is entered w/o directory walk being
done. I believe strongly that we should not attempt to recover from this error (ie
"back-up and try directory walk"). This failure means that a module is defective and that
module should be fixed.
Bill
Index: include/http_core.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_core.h,v
retrieving revision 1.64
diff -u -r1.64 http_core.h
--- include/http_core.h 20 Mar 2002 02:05:42 -0000 1.64
+++ include/http_core.h 22 Mar 2002 17:52:09 -0000
@@ -342,6 +342,16 @@
* to add elements)
*/
void **notes;
+
+ /* This field is used exclusively by the core default_handler
+ * to determine if directory_walk was performed. The
+ * default_handler must never be allowed to run if
+ * directory_walk has not been run. This error can happen
+ * if a module author implements a map_to_storage
+ * hook that circumvents directory_walk then that modules
+ * handler DECLINES the request.
+ */
+ apr_size_t directory_walked;
} core_request_config;
Index: server/core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.166
diff -u -r1.166 core.c
--- server/core.c 20 Mar 2002 22:19:10 -0000 1.166
+++ server/core.c 22 Mar 2002 17:52:16 -0000
@@ -3070,6 +3070,8 @@
static int core_map_to_storage(request_rec *r)
{
int access_status;
+ core_request_config *req_cfg = (core_request_config *)
+ ap_get_module_config(r->request_config, &core_module);
if ((access_status = ap_directory_walk(r))) {
return access_status;
@@ -3079,6 +3081,8 @@
return access_status;
}
+ req_cfg->directory_walked = 1;
+
return OK;
}
@@ -3137,6 +3141,9 @@
*/
int bld_content_md5;
+ core_request_config *req_cfg = (core_request_config *)
+ ap_get_module_config(r->request_config, &core_module);
+
/*
* The old way of doing handlers meant that this handler would
* match literally anything - this way will require handler to
@@ -3193,6 +3200,13 @@
return HTTP_NOT_FOUND;
}
+ if (!req_cfg->directory_walked) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r,
+ "directory_walk was not performed for file: %s",
+ apr_pstrcat(r->pool, r->filename, r->path_info, NULL));
+ return HTTP_NOT_FOUND;
+ }
+
if ((status = apr_file_open(&fd, r->filename, APR_READ | APR_BINARY, 0,
r->pool)) != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,
@@ -3913,8 +3927,9 @@
core_request_config *req_cfg;
req_cfg = apr_pcalloc(r->pool, sizeof(core_request_config) +
- sizeof(void *) * num_request_notes);
+ sizeof(void *) * num_request_notes);
req_cfg->notes = (void **)((char *)req_cfg + sizeof(core_request_config));
+ req_cfg->directory_walked = 0;
if (r->main) {
core_request_config *main_req_cfg = (core_request_config *)
ap_get_module_config(r->main->request_config, &core_module);
Re: [PATCH] default_handler and no directory_walk
Posted by Bill Stoddard <bi...@wstoddard.com>.
>
> On Fri, 22 Mar 2002, Bill Stoddard wrote:
>
> > Here is a patch to error out if the default handler is entered w/o directory walk
being
> > done. I believe strongly that we should not attempt to recover from this error (ie
> > "back-up and try directory walk"). This failure means that a module is defective and
that
> > module should be fixed.
>
> > + if (!req_cfg->directory_walked) {
> > + ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r,
> > + "directory_walk was not performed for file: %s",
> > + apr_pstrcat(r->pool, r->filename, r->path_info, NULL));
> > + return HTTP_NOT_FOUND;
> > + }
> > +
>
> More comments from the peanut gallery: If this request is being refused
> because of a broken module, shouldn't that be 500 Internal Server Error
> rather than 404 File Not Found?
>
> Joshua.
>
Perhaps. I think I prefer 404 though. I've not thought this completely through but it is
not inconceivable that a module author could rely on this behaviour to protect against
race conditions between when the map_to_storage hook is entered and the handler being
entered. A modules map_to_storage hook may 'claim' the request then the handler DECLINES
because some state information in the module has changed between the time the
map_to_storage hook was run and the handler running. Easy enough to code around this
though... I am not even sure I want a patch like this going into the server...
Bill
Re: [PATCH] default_handler and no directory_walk
Posted by Joshua Slive <jo...@slive.ca>.
On Fri, 22 Mar 2002, Bill Stoddard wrote:
> Here is a patch to error out if the default handler is entered w/o directory walk being
> done. I believe strongly that we should not attempt to recover from this error (ie
> "back-up and try directory walk"). This failure means that a module is defective and that
> module should be fixed.
> + if (!req_cfg->directory_walked) {
> + ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r,
> + "directory_walk was not performed for file: %s",
> + apr_pstrcat(r->pool, r->filename, r->path_info, NULL));
> + return HTTP_NOT_FOUND;
> + }
> +
More comments from the peanut gallery: If this request is being refused
because of a broken module, shouldn't that be 500 Internal Server Error
rather than 404 File Not Found?
Joshua.