[RFC] Static: return 404 when "index" is a non-regular file

[Alejandro Colomar]

When index is a file, but not a regular file nor a directory, so
it may be for example a FIFO, Unit returns 404.

However, for directories, it was returning 301.  Nginx also
returns 301 (and is documented, as Valentin pointed out)[1], but
we prefer to be consistent (and intuitive) with other invalid
files in Unit and so return 404 for dirs too.

[1]: <http://nginx.org/en/docs/http/ngx_http_index_module.html>

I got the following pytest errors after this patch, but I'm not
sure of what those tests mean.

===================================== FAILURES =====================================
____________________ TestStaticChroot.test_static_chroot_empty _____________________

self = <test_static_chroot.TestStaticChroot object at 0x7ffb48a2cac0>
temp_dir = '/tmp/unit-test-19ucrwpo'

    def test_static_chroot_empty(self, temp_dir):
        assert 'success' in self.conf(
            {"share": temp_dir + "/assets$uri", "chroot": ""},
            'routes/0/action',
        ), 'configure chroot empty absolute'

        assert (
            self.get(url='/dir/file')['status'] == 200
        ), 'chroot empty absolute'

        assert 'success' in self.conf(
            {"share": ".$uri", "chroot": ""}, 'routes/0/action',
        ), 'configure chroot empty relative'

>       assert (
            self.get(url=self.test_path)['status'] == 200
        ), 'chroot empty relative'
E       AssertionError: chroot empty relative
E       assert 404 == 200
E         +404
E         -200

test_static_chroot.py:107: AssertionError
___________________ TestStaticChroot.test_static_chroot_relative ___________________

self = <test_static_chroot.TestStaticChroot object at 0x7ffb48a81880>, is_su = False
temp_dir = '/tmp/unit-test-19ucrwpo'

    def test_static_chroot_relative(self, is_su, temp_dir):
        if is_su:
            pytest.skip('does\'t work under root')

        assert 'success' in self.conf(
            {"share": temp_dir + "/assets$uri", "chroot": "."},
            'routes/0/action',
        ), 'configure relative chroot'

        assert self.get(url='/dir/file')['status'] == 403, 'relative chroot'

        assert 'success' in self.conf(
            {"share": ".$uri"}, 'routes/0/action',
        ), 'configure relative share'

>       assert self.get(url=self.test_path)['status'] == 200, 'relative share'
E       AssertionError: relative share
E       assert 404 == 200
E         +404
E         -200

test_static_chroot.py:126: AssertionError
============================= short test summary info ==============================
FAILED test_static_chroot.py::TestStaticChroot::test_static_chroot_empty - Assert...
FAILED test_static_chroot.py::TestStaticChroot::test_static_chroot_relative - Ass...
==================== 2 failed, 62 passed, 706 skipped in 2.77s =====================

Reported-by: Andrei Zeliankou <zelenkov at nginx.com>
Suggested-by: "Valentin V. Bartenev" <vbart at nginx.com>
Signed-off-by: Alejandro Colomar <alx.manpages at gmail.com>
Cc: Nginx Unit <unit at nginx.org>
Cc: Igor Sysoev <igor at sysoev.ru>
Cc: Zhidao HONG <z.hong at f5.com>
Cc: Oisin Canty <o.canty at f5.com>
Cc: Maxim Romanov <m.romanov at f5.com>
---

Hi Valentin,

I'm not entirely convinced about this patch.  I used the same
condition that triggers the use of the index, to consider that a
directory is invalid.  That is, we know that the path is a dir
_after_ index has been applied.

But since I don't know all the other conditions that can lead to
that code, I'm not entirely sure this is exactly what we want.


 src/nxt_http_static.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/nxt_http_static.c b/src/nxt_http_static.c
index ea97147..74245cd 100644
--- a/src/nxt_http_static.c
+++ b/src/nxt_http_static.c
@@ -623,7 +623,9 @@ nxt_http_static_send_ready(nxt_task_t *task, void *obj, void *data)
         /* Not a file. */
         nxt_file_close(task, f);
 
-        if (nxt_slow_path(!nxt_is_dir(&fi))) {
+        if (nxt_slow_path(!nxt_is_dir(&fi)
+                          || ctx->share.start[ctx->share.length - 1] == '/'))
+        {
             nxt_log(task, NXT_LOG_ERR, "\"%FN\" is not a regular file",
                     f->name);
 
-- 
2.34.1