"index off;" directive?
Maxim Dounin
mdounin at mdounin.ru
Thu Apr 30 03:34:29 UTC 2026
Hello!
On Tue, Apr 28, 2026 at 08:32:32PM -0300, Fabiano Furtado wrote:
> On Mon, Apr 27, 2026 at 9:04 PM Maxim Dounin wrote:
> >
> > Hello!
> >
> > On Mon, Apr 27, 2026 at 09:22:59 AM -0300, Fabiano Furtado wrote:
> >
> > > On Sun, Apr 26, 2026 at 7:53 PM Maxim Dounin wrote:
> > > > ...
> > > Anyway, I'm not sure if it's possible, but I'd like to try developing
> > > this patch. Would it be okay if I tried to create this new patch?
> >
> > Yep, feel free to.
> >
> > Just in case, some hints can be found here:
> > https://freenginx.org/en/docs/contributing_changes.html
>
> This patch introduces the "index off;" directive for freenginx, but I
> also had to modify other parts of the code to ensure this new
> directive behaves correctly.
Please make sure to read the above link. In particular, following
the coding style and using Mercurial is highly recommended, as it
makes it much easier to review the code.
> When "off" is applied in the config and an HTTP access occurs, a
> message appears in the error_log: "directory index of ".../" is
> forbidden". In this context, I believe this message is not
> particularly relevant for the error_log. So, I added a flag to the
> core called "log_forbidden", which works similarly to "log_not_found".
I believe this change is completely orthogonal and has nothing to
do with the "index off;". The message is perfectly identical with
both "index off;" and the "index .non_such_file;" workaround, so
"index off;" introduces nothing new here. Also, the message can
be suppressed with "location ~ /$ { return 403; }" if no indexing
is indeed desired.
That is, if at all, this should be a separate patch, explaining
the change and reasons for it.
> As a result, instead of modifying only "ngx_http_index_module.c", I
> ended up changing four files:
> * src/http/ngx_http_core_module.c
> * src/http/ngx_http_core_module.h
> * src/http/modules/ngx_http_access_module.c
> * src/http/modules/ngx_http_index_module.c
>
> I would appreciate it if the code could be reviewed. Any feedback or
> suggestions are welcome.
>
> Thank you in advance for the opportunity to submit this patch.
> Fabiano Furtado
> # diff -Nau freenginx-1.30.0/src/http/ngx_http_core_module.c freenginx-1.30.0_patched/src/http/ngx_http_core_module.c >ngx_http_core_module.c.patch
> --- freenginx-1.30.0/src/http/ngx_http_core_module.c 2026-04-14 05:23:27.000000000 -0300
> +++ freenginx-1.30.0_patched/src/http/ngx_http_core_module.c 2026-04-28 19:56:18.478687257 -0300
> @@ -628,7 +628,14 @@
> offsetof(ngx_http_core_loc_conf_t, msie_refresh),
> NULL },
>
> - { ngx_string("log_not_found"),
> + { ngx_string("log_forbidden"),
> + 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_forbidden),
> + NULL },
> +
> + { ngx_string("log_not_found"),
Note that "log_not_found" in diff suggests there is a white space
damage. And, indeed, the line lost one of the leading spaces.
Also, in many cases it is a good idea to define new directives
after the one you want to mimic, and not before it.
> 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,
> @@ -1177,7 +1184,8 @@
> ngx_http_core_post_access_phase(ngx_http_request_t *r,
> ngx_http_phase_handler_t *ph)
> {
> - ngx_int_t access_code;
> + ngx_int_t access_code;
> + ngx_http_core_loc_conf_t *clcf;
>
> ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
> "post access phase: %ui", r->phase_handler);
> @@ -1187,7 +1195,9 @@
> if (access_code) {
> r->access_code = 0;
>
> - if (access_code == NGX_HTTP_FORBIDDEN) {
> + clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> +
> + if ((clcf->log_forbidden) && (access_code == NGX_HTTP_FORBIDDEN)) {
> ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> "access forbidden by rule");
> }
Requests rejected by the access phase handlers are generally
expected to be logged. Also, there are various other cases when
access phase handlers log errors, such as in the
ngx_http_auth_basic_module on user/password mismatch.
While changing this is possible and I can't say I strongly object
it, I would like to ensure that this is done in some consistent
manner, affecting all the standard access modules, and not just
the errors typically returned by the access module.
Note well that there are various other places where
NGX_HTTP_FORBIDDEN can be returned, and these log errors,
including with your patch - autoindex, random index, flv, mp4, and
static modules all log errors and return NGX_HTTP_FORBIDDEN when
the file/directory cannot be opened due to OS-level access
restrictions, and your patch only changes relevant code in the
index module. It is not immediately clear how these are different
if the "log_forbidden" directive is expected to behave like
"log_not_found". And, at the same time, it might be seen as a
configuration error when a file which cannot be opened by
[free]nginx is accessible under the document root, so not logging
the error might be the wrong way to go.
Also, it might be a better idea to follow the log limiting
approach as used by the limit_req and limit_conn modules, that is,
introduce some directives similar to limit_req_log_level.
Not well that there is a couple of style issues in the code
suggested: it should be faster to check "access_code ==
NGX_HTTP_FORBIDDEN" first, similarly to how clcf->log_not_found is
checked in the static module, and there is no need to place extra
parentheses where execution order is well defined and immediately
obvious.
> @@ -1273,9 +1283,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;
> @@ -1306,7 +1317,11 @@
>
> if (r->uri.data[r->uri.len - 1] == '/') {
>
> - if (ngx_http_map_uri_to_path(r, &path, &root, 0) != NULL) {
> + clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> +
> + if ((clcf->log_forbidden)
> + && (ngx_http_map_uri_to_path(r, &path, &root, 0) != NULL)) {
> +
Style issues here: unneeded parentheses, wrong indentation of the
second line, "{" after multi-line conditions should be on its own
line.
> ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> "directory index of \"%s\" is forbidden", path.data);
> }
> @@ -3649,6 +3664,7 @@
> clcf->port_in_redirect = NGX_CONF_UNSET;
> clcf->msie_padding = NGX_CONF_UNSET;
> clcf->msie_refresh = NGX_CONF_UNSET;
> + clcf->log_forbidden = NGX_CONF_UNSET;
> clcf->log_not_found = NGX_CONF_UNSET;
> clcf->log_subrequest = NGX_CONF_UNSET;
> clcf->recursive_error_pages = NGX_CONF_UNSET;
> @@ -3923,6 +3939,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_forbidden, prev->log_forbidden, 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 -Nau freenginx-1.30.0/src/http/modules/ngx_http_index_module.c freenginx-1.30.0_patched/src/http/modules/ngx_http_index_module.c >ngx_http_index_module.c.patch
> --- freenginx-1.30.0/src/http/modules/ngx_http_index_module.c 2026-04-14 05:23:27.000000000 -0300
> +++ freenginx-1.30.0_patched/src/http/modules/ngx_http_index_module.c 2026-04-28 20:07:00.825547974 -0300
> @@ -20,6 +20,7 @@
> typedef struct {
> ngx_array_t *indices; /* array of ngx_http_index_t */
> size_t max_index_len;
> + ngx_flag_t off;
> } ngx_http_index_loc_conf_t;
>
>
> @@ -109,6 +110,12 @@
> ngx_http_index_loc_conf_t *ilcf;
> ngx_http_script_len_code_pt lcode;
>
> + ilcf = ngx_http_get_module_loc_conf(r, ngx_http_index_module);
> +
> + if (ilcf->off) {
> + return NGX_DECLINED;
> + }
> +
> if (r->uri.data[r->uri.len - 1] != '/') {
> return NGX_DECLINED;
> }
I believe checking r->uri before and method before even trying to
access the configuration is optimal from performance point of
view, and this is what index module does now, and other modules do
as well (see autoindex or random index for some examples). There
is no need to change this order.
> @@ -117,7 +124,6 @@
> return NGX_DECLINED;
> }
>
> - ilcf = ngx_http_get_module_loc_conf(r, ngx_http_index_module);
> clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
>
> allocated = 0;
> @@ -369,8 +375,10 @@
> u_char *file, ngx_err_t err)
> {
> if (err == NGX_EACCES) {
> - ngx_log_error(NGX_LOG_ERR, r->connection->log, err,
> - "\"%s\" is forbidden", file);
> + if (clcf->log_forbidden) {
> + ngx_log_error(NGX_LOG_ERR, r->connection->log, err,
> + "\"%s\" is forbidden", file);
> + }
>
> return NGX_HTTP_FORBIDDEN;
> }
Note that this change should be in a separate patch, and see above
for more comments about this.
> @@ -396,6 +404,7 @@
>
> conf->indices = NULL;
> conf->max_index_len = 0;
> + conf->off = NGX_CONF_UNSET;
>
> return conf;
> }
> @@ -409,6 +418,12 @@
>
> ngx_http_index_t *index;
>
> + ngx_conf_merge_value(conf->off, prev->off, 0);
> +
> + if (conf->off) {
> + return NGX_CONF_OK;
> + }
> +
> if (conf->indices == NULL) {
> conf->indices = prev->indices;
> conf->max_index_len = prev->max_index_len;
Consider the following configuration:
server {
index off;
location /public/ {
index index.html;
}
}
With the merge logic you've implemented this will result in
"conf->off" being set in "location /public/", leading to no index
files being used there, which is obviously not the requested
behaviour.
There are multiple possible solutions here:
- write a custom merge logic, checking both conf->off and
conf->indices and inheriting conf->off and conf->indices only if
neither of the two was set at the current level,
- change the code to instead rely on conf->indices special values
(for example, conf->indices == NGX_CONF_UNSET_PTR for not set,
and NULL for off; or indices->nelts == 0 for off),
- change "off" to an additional flag, similarly to how it is done
with "max_index_len", and inherit it along with conf->indices.
> @@ -470,6 +485,22 @@
> ngx_http_index_t *index;
> ngx_http_script_compile_t sc;
>
> + value = cf->args->elts;
> +
> + if (ngx_strcmp(value[1].data, "off") == 0) {
> + if (cf->args->nelts == 2) {
> +
> + ilcf->off = 1;
> +
> + return NGX_CONF_OK;
> + } else {
> +
> + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
> + "\"index off\" must be the only parameter");
> + return NGX_CONF_ERROR;
> + }
> + }
> +
Consider the following configuration:
index index.html;
index off;
It is expected to be equivalent to "index index.html off;" (if
supported; currently it is), yet these behave very differently
with your code: multiple directives will switch off indexing and
will silently ignore "index.html", yet "index index.html off;"
will try to access the "off" index file.
Similarly,
index default.html;
index off index.html;
is not equivalent to "index default.html off index.html;" and will
be rejected by your code.
(Also, there are multiple style issues here: wrong indentation,
inconsistent empty lines.)
> if (ilcf->indices == NULL) {
> ilcf->indices = ngx_array_create(cf->pool, 2, sizeof(ngx_http_index_t));
> if (ilcf->indices == NULL) {
> @@ -477,8 +508,6 @@
> }
> }
>
> - value = cf->args->elts;
> -
> for (i = 1; i < cf->args->nelts; i++) {
>
> if (value[i].data[0] == '/' && i != cf->args->nelts - 1) {
> # diff -Nau freenginx-1.30.0/src/http/ngx_http_core_module.h freenginx-1.30.0_patched/src/http/ngx_http_core_module.h >ngx_http_core_module.h.patch
> --- freenginx-1.30.0/src/http/ngx_http_core_module.h 2026-04-14 05:23:27.000000000 -0300
> +++ freenginx-1.30.0_patched/src/http/ngx_http_core_module.h 2026-04-28 19:56:25.382446864 -0300
> @@ -406,6 +406,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_forbidden; /* log_forbidden */
> 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 */
> # diff -Nau freenginx-1.30.0/src/http/modules/ngx_http_access_module.c freenginx-1.30.0_patched/src/http/modules/ngx_http_access_module.c >ngx_http_access_module.c.patch
> --- freenginx-1.30.0/src/http/modules/ngx_http_access_module.c 2026-04-14 05:23:27.000000000 -0300
> +++ freenginx-1.30.0_patched/src/http/modules/ngx_http_access_module.c 2026-04-28 19:56:52.838228281 -0300
> @@ -280,7 +280,7 @@
> if (deny) {
> clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
>
> - if (clcf->satisfy == NGX_HTTP_SATISFY_ALL) {
> + if ((clcf->log_forbidden) && (clcf->satisfy == NGX_HTTP_SATISFY_ALL)) {
> ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> "access forbidden by rule");
> }
Hope this helps.
--
Maxim Dounin
http://mdounin.ru/
More information about the nginx-devel
mailing list