"index off;" directive?
Fabiano Furtado
fabianofurtado at gmail.com
Mon May 4 01:22:16 UTC 2026
Hi!
On Thu, Apr 30, 2026 at 12:34 AM Maxim Dounin wrote:
>
> 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 multiline 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.
Thank you for your time and help!!
As you mentioned, the "log_forbidden" change is completely orthogonal,
so let's set it aside for now.
While searching for a code pattern in the freenginx codebase to guide
my implementation, I accidentally discovered a bug in
ngx_http_log_module (ngx_http_log_open_file_cache) when testing the
"off" parameter in different positions. For example, the configuration
"open_log_file_cache off max=1;" should fail, but it does not.
I hope this patch works well! Thank you, again!
Fabiano Furtado
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ngx_http_index_module.c.patch
Type: text/x-patch
Size: 3735 bytes
Desc: not available
URL: <http://freenginx.org/pipermail/nginx-devel/attachments/20260503/e18d1369/attachment.bin>
More information about the nginx-devel
mailing list