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: [email protected] ReportedBy: [EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
