You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2004/06/09 04:12:15 UTC
DO NOT REPLY [Bug 29450] New: -
Improved logging for mod_access
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=29450>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=29450
Improved logging for mod_access
Summary: Improved logging for mod_access
Product: Apache httpd-2.0
Version: 2.0.49
Platform: All
OS/Version: Other
Status: NEW
Severity: Enhancement
Priority: Other
Component: mod_access
AssignedTo: bugs@httpd.apache.org
ReportedBy: ricilake@speedy.com.pe
The following patch logs the location in the config file responsible for a mod_access denial. It appears
that it is very common for inexperienced Apache users to have difficulty untangling the various
<directory> <location> and .htaccess configurations, particularly in distributions which divide the
configuration system into multiple files.
Following some suggestions from the apache-dev mailing list, I modified the patch to produce two log
messages; the first at level "error" which is identical to the current behaviour, and the second at level
"info" with the debugging information. I don't regard this as ideal, but it is hard to find consensus. I was
going to put it at level "notice" but it turns out that level "notice" is logged regardless of the setting of
LogLevel, which is slightly odd behaviour.
The patch is dependent on the filename/linenumber information stored in the struct directive for the
mod_access directives. Since I couldn't convince myself that it was safe to store the pointer from that
structure, the patch copies the first 80 bytes of the filename into a static buffer. This is obviously not
ideal either, but it works and the storage impact is limited to 80*(number of mod_access directives).
I think this same strategy could be extended to other directives which frequently frustrate new users,
particularly the "AllowOverride" and "Options" directives. But it would be useful to have some better
understanding of the memory-pool strategy for struct directive. There is a note in the source code that
the filename/linenumber info should be removed from struct directive; I think this patch demonstrates
the utility of that information.
---- patch follows ----
--- httpd-2.0.49-orig/modules/aaa/mod_access.c Mon Feb 9 15:53:14 2004
+++ httpd-2.0.49/modules/aaa/mod_access.c Tue Jun 8 20:55:00 2004
@@ -39,6 +39,9 @@
#include <netinet/in.h>
#endif
+/* If their config pathnames are longer than this, they deserve what they get */
+#define AP_ACCESS_MAXPATH 80
+
enum allowdeny_type {
T_ENV,
T_ALL,
@@ -54,6 +57,7 @@
apr_ipsubnet_t *ip;
} x;
enum allowdeny_type type;
+ unsigned line_number;
} allowdeny;
/* things in the 'order' array */
@@ -65,6 +69,8 @@
int order[METHODS];
apr_array_header_t *allows;
apr_array_header_t *denys;
+ char filename[AP_ACCESS_MAXPATH];
+ unsigned line_number;
} access_dir_conf;
module AP_MODULE_DECLARE_DATA access_module;
@@ -80,10 +86,38 @@
}
conf->allows = apr_array_make(p, 1, sizeof(allowdeny));
conf->denys = apr_array_make(p, 1, sizeof(allowdeny));
-
+ conf->filename[0] = 0;
+ conf->line_number = 0;
return (void *)conf;
}
+/* it's like you see this cute directive and you have to...*/
+static void get_name_and_number(cmd_parms *cmd, char *name, unsigned *number)
+{
+ /* There is a comment saying directive->filename might go away. It is not at
+ * all clear to me how we are supposed to get this info if it does
+ * On the other hand, no other module seems to care
+ */
+ const char *fname = NULL;
+ if (cmd->directive && cmd->directive->filename) {
+ fname = cmd->directive->filename;
+ *number = cmd->directive->line_num;
+ }
+ /* You gotta love the consistent naming convention */
+ else if (cmd->config_file && cmd->config_file->name) {
+ fname = cmd->config_file->name;
+ *number = cmd->config_file->line_number;
+ }
+ if (fname) {
+ if (apr_cpystrn(name, fname, AP_ACCESS_MAXPATH) == &name[AP_ACCESS_MAXPATH-1])
+ name[AP_ACCESS_MAXPATH-2] = name[AP_ACCESS_MAXPATH-3] = name[AP_ACCESS_MAXPATH
-4] = '.';
+ } else {
+ name[0] = name[1] = name[2] = '?'; name[3] = '\0';
+ *number = 0;
+ }
+}
+
+
static const char *order(cmd_parms *cmd, void *dv, const char *arg)
{
access_dir_conf *d = (access_dir_conf *) dv;
@@ -102,6 +136,7 @@
if (cmd->limited & (AP_METHOD_BIT << i))
d->order[i] = o;
+ get_name_and_number(cmd, d->filename, &d->line_number);
return NULL;
}
@@ -122,6 +157,8 @@
a->x.from = where;
a->limited = cmd->limited;
+ get_name_and_number(cmd, d->filename, &a->line_number);
+
if (!strncasecmp(where, "env=", 4)) {
a->type = T_ENV;
a->x.from += 4;
@@ -193,7 +230,7 @@
return 0;
}
-static int find_allowdeny(request_rec *r, apr_array_header_t *a, int method)
+static allowdeny *find_allowdeny(request_rec *r, apr_array_header_t *a, int method)
{
allowdeny *ap = (allowdeny *) a->elts;
@@ -209,16 +246,16 @@
switch (ap[i].type) {
case T_ENV:
if (apr_table_get(r->subprocess_env, ap[i].x.from)) {
- return 1;
+ return &ap[i];
}
break;
case T_ALL:
- return 1;
+ return &ap[i];
case T_IP:
if (apr_ipsubnet_test(ap[i].x.ip, r->connection->remote_addr)) {
- return 1;
+ return &ap[i];
}
break;
@@ -236,7 +273,7 @@
}
if ((gothost == 2) && in_domain(ap[i].x.from, remotehost))
- return 1;
+ return &ap[i];
break;
case T_FAIL:
@@ -245,13 +282,14 @@
}
}
- return 0;
+ return NULL;
}
static int check_dir_access(request_rec *r)
{
int method = r->method_number;
int ret = OK;
+ allowdeny *which = NULL;
access_dir_conf *a = (access_dir_conf *)
ap_get_module_config(r->per_dir_config, &access_module);
@@ -259,18 +297,18 @@
ret = HTTP_FORBIDDEN;
if (find_allowdeny(r, a->allows, method))
ret = OK;
- if (find_allowdeny(r, a->denys, method))
+ if (which = find_allowdeny(r, a->denys, method))
ret = HTTP_FORBIDDEN;
}
else if (a->order[method] == DENY_THEN_ALLOW) {
- if (find_allowdeny(r, a->denys, method))
+ if (which = find_allowdeny(r, a->denys, method))
ret = HTTP_FORBIDDEN;
if (find_allowdeny(r, a->allows, method))
ret = OK;
}
else {
if (find_allowdeny(r, a->allows, method)
- && !find_allowdeny(r, a->denys, method))
+ && !(which = find_allowdeny(r, a->denys, method)))
ret = OK;
else
ret = HTTP_FORBIDDEN;
@@ -281,6 +319,14 @@
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"client denied by server configuration: %s",
r->filename);
+ if (which)
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ "client denied by deny directive at line %d of %s",
+ which->line_number, a->filename);
+ else
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ "client denied by order directive at line %d of %s",
+ a->line_number, a->filename);
}
return ret;
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org