[PATCH] Add 'log_index_denied' directive
Maxim Dounin
mdounin at mdounin.ru
Thu Nov 9 16:10:16 UTC 2017
Hello!
On Mon, Oct 30, 2017 at 09:17:59PM +0000, Ben Brown wrote:
> # HG changeset patch
> # User Ben Brown <ben at isitdoneyet.co.uk>
> # Date 1509396532 0
> # Mon Oct 30 20:48:52 2017 +0000
> # Node ID 0c415222a6959147151422463261443275e69373
> # Parent 9ef704d8563af4aff6817ab1c694fb40591f20b3
> Add 'log_index_denied' directive
>
> This is similar to the 'log_not_found' directive but instead of
> suppressing 404 errors this can be used to suppress the 'index of
> directory...' error messages.
>
> It defaults to 'on', which is the current behaviour. It is valid in the
> same contexts as the 'log_not_found' directive.
>
> This was suggested by IRC user MacroMan to aid debugging where the logs
> contained a lot of these messages.
This seems very similar to quite a few other errors, including
"access forbidden by rule" message as logged in
ngx_http_core_post_access_phase(), and "password mismatch" as
logged in ngx_http_auth_basic_crypt_handler(). Trying to control
these errors on-by-one does not look like a good idea to me.
Also, such errors can be easily avoided by using a site-wide index
file, for example:
index index.html /403;
location = /403 {
return 403;
}
As such, I don't think we need to introduce a special directive to
control "directory index of ... forbidden" messages.
Se below for some additional code-related comments, though
probably they aren't relevant.
>
> diff -r 9ef704d8563a -r 0c415222a695 contrib/vim/syntax/nginx.vim
> --- a/contrib/vim/syntax/nginx.vim Tue Oct 17 19:52:16 2017 +0300
> +++ b/contrib/vim/syntax/nginx.vim Mon Oct 30 20:48:52 2017 +0000
> @@ -313,6 +313,7 @@
> syn keyword ngxDirective contained load_module
> syn keyword ngxDirective contained lock_file
> syn keyword ngxDirective contained log_format
> +syn keyword ngxDirective contained log_index_denied
> syn keyword ngxDirective contained log_not_found
> syn keyword ngxDirective contained log_subrequest
> syn keyword ngxDirective contained map_hash_bucket_size
> diff -r 9ef704d8563a -r 0c415222a695 src/http/ngx_http_core_module.c
> --- a/src/http/ngx_http_core_module.c Tue Oct 17 19:52:16 2017 +0300
> +++ b/src/http/ngx_http_core_module.c Mon Oct 30 20:48:52 2017 +0000
> @@ -583,6 +583,13 @@
> offsetof(ngx_http_core_loc_conf_t, msie_refresh),
> NULL },
>
> + { ngx_string("log_index_denied"),
> + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
> + ngx_conf_set_flag_slot,
> + NGX_HTTP_LOC_CONF_OFFSET,
> + offsetof(ngx_http_core_loc_conf_t, log_index_denied),
> + NULL },
> +
> { ngx_string("log_not_found"),
> NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
> ngx_conf_set_flag_slot,
The order here isn't alphabetical, so I would recommend adding the
new directive after log_not_found (if at all).
> @@ -1153,9 +1160,10 @@
> ngx_http_core_content_phase(ngx_http_request_t *r,
> ngx_http_phase_handler_t *ph)
> {
> - size_t root;
> - ngx_int_t rc;
> - ngx_str_t path;
> + size_t root;
> + ngx_int_t rc;
> + ngx_str_t path;
> + ngx_http_core_loc_conf_t *clcf;
>
> if (r->content_handler) {
> r->write_event_handler = ngx_http_request_empty_handler;
> @@ -1187,8 +1195,12 @@
> if (r->uri.data[r->uri.len - 1] == '/') {
>
> if (ngx_http_map_uri_to_path(r, &path, &root, 0) != NULL) {
> - ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> - "directory index of \"%s\" is forbidden", path.data);
> + clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> + if (clcf->log_index_denied) {
Are there any reasons to call ngx_http_map_uri_to_path() if we are
not going to use the result?
> + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> + "directory index of \"%s\" is forbidden",
> + path.data);
> + }
> }
>
> ngx_http_finalize_request(r, NGX_HTTP_FORBIDDEN);
> @@ -3384,6 +3396,7 @@
> clcf->port_in_redirect = NGX_CONF_UNSET;
> clcf->msie_padding = NGX_CONF_UNSET;
> clcf->msie_refresh = NGX_CONF_UNSET;
> + clcf->log_index_denied = NGX_CONF_UNSET;
> clcf->log_not_found = NGX_CONF_UNSET;
> clcf->log_subrequest = NGX_CONF_UNSET;
> clcf->recursive_error_pages = NGX_CONF_UNSET;
> @@ -3649,6 +3662,7 @@
> ngx_conf_merge_value(conf->port_in_redirect, prev->port_in_redirect, 1);
> ngx_conf_merge_value(conf->msie_padding, prev->msie_padding, 1);
> ngx_conf_merge_value(conf->msie_refresh, prev->msie_refresh, 0);
> + ngx_conf_merge_value(conf->log_index_denied, prev->log_index_denied, 1);
> ngx_conf_merge_value(conf->log_not_found, prev->log_not_found, 1);
> ngx_conf_merge_value(conf->log_subrequest, prev->log_subrequest, 0);
> ngx_conf_merge_value(conf->recursive_error_pages,
> diff -r 9ef704d8563a -r 0c415222a695 src/http/ngx_http_core_module.h
> --- a/src/http/ngx_http_core_module.h Tue Oct 17 19:52:16 2017 +0300
> +++ b/src/http/ngx_http_core_module.h Mon Oct 30 20:48:52 2017 +0000
> @@ -385,6 +385,7 @@
> ngx_flag_t port_in_redirect; /* port_in_redirect */
> ngx_flag_t msie_padding; /* msie_padding */
> ngx_flag_t msie_refresh; /* msie_refresh */
> + ngx_flag_t log_index_denied; /* log_index_denied */
> ngx_flag_t log_not_found; /* log_not_found */
> ngx_flag_t log_subrequest; /* log_subrequest */
> ngx_flag_t recursive_error_pages; /* recursive_error_pages */
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
--
Maxim Dounin
http://mdounin.ru/
More information about the nginx-devel
mailing list