[PATCH v4 07/12] Static: add "index" option
Andrei Zeliankou
zelenkov at nginx.com
Fri Jan 28 12:27:25 UTC 2022
Hi!
> On 23 Dec 2021, at 19:25, Alejandro Colomar <alx.manpages at gmail.com> wrote:
>
> This supports a new option "index", that configures a custom index
> file name that will be served when a directory is requested. This
> initial support only allows a single fixed string. An example:
>
> {
> "share": "/www/data/static/$uri",
> "index": "lookatthis.htm"
> }
>
> When <example.com/foo/bar/> is requested,
> </www/data/static/foo/bar/lookatthis.html> will be served.
>
> If the option is missing, default to "index.html".
>
> ===
>
> nxt_conf_validator.c:
>
> Accept "index" as a member of "share", and make sure it is a
> string.
>
> ===
>
> test_static.py:
>
> Rename <index.html> to <index_.html>, to be able to test this new
> option, and set "index": "index_.html".
>
> Perhaps we should also add a test to check that if "index" is
> omitted, the default "index.html" is used.
>
> I also tried this feature in my own computer, where I tried the
> following:
>
> - Setting "index" to "lookatthis.htm", and check that the correct
> file is being served (check both a different name and a
> different extension).
> - Not setting "index", and check that <index.html> is being
> served.
> - Settind "index" to an array of strings, and check that the
> configuration fails:
>
> {
> "error": "Invalid configuration.",
> "detail": "The \"index\" value must be a string, but not an array."
> }
>
> Signed-off-by: Alejandro Colomar <alx.manpages at gmail.com>
> Cc: Nginx Unit <unit at nginx.org>
> Cc: "Valentin V. Bartenev" <vbart at nginx.com>
> Cc: Zhidao HONG <z.hong at f5.com>
> Cc: Igor Sysoev <igor at sysoev.ru>
> Cc: Oisin Canty <o.canty at f5.com>
> ---
> docs/changes.xml | 6 ++++++
> src/nxt_conf_validation.c | 3 +++
> src/nxt_http.h | 1 +
> src/nxt_http_route.c | 5 +++++
> src/nxt_http_static.c | 32 +++++++++++++++++++++++---------
> test/test_static.py | 13 +++++++------
> 6 files changed, 45 insertions(+), 15 deletions(-)
[..]
> diff --git a/test/test_static.py b/test/test_static.py
> index 76668a1..70ae676 100644
> --- a/test/test_static.py
> +++ b/test/test_static.py
> @@ -14,7 +14,7 @@ class TestStatic(TestApplicationProto):
>
> def setup_method(self):
> os.makedirs(option.temp_dir + '/assets/dir')
> - with open(option.temp_dir + '/assets/index.html', 'w') as index, open(
> + with open(option.temp_dir + '/assets/index_.html', 'w') as index, open(
> option.temp_dir + '/assets/README', 'w'
> ) as readme, open(
> option.temp_dir + '/assets/log.log', 'w'
> @@ -32,7 +32,8 @@ class TestStatic(TestApplicationProto):
> "routes": [
> {
> "action": {
> - "share": option.temp_dir + "/assets$uri"
> + "share": option.temp_dir + "/assets$uri",
> + "index": "index_.html"
> }
> }
> ],
> @@ -90,7 +91,7 @@ class TestStatic(TestApplicationProto):
> assert self.get(url='/')['body'] == '0123456789', 'before 1.26.0 2'
>
> def test_static_index(self):
> - assert self.get(url='/index.html')['body'] == '0123456789', 'index'
> + assert self.get(url='/index_.html')['body'] == '0123456789', 'index'
> assert self.get(url='/')['body'] == '0123456789', 'index 2'
> assert self.get(url='//')['body'] == '0123456789', 'index 3'
> assert self.get(url='/.')['body'] == '0123456789', 'index 4'
> @@ -99,7 +100,7 @@ class TestStatic(TestApplicationProto):
> assert self.get(url='/#blah')['body'] == '0123456789', 'index anchor'
> assert self.get(url='/dir/')['status'] == 404, 'index not found'
>
> - resp = self.get(url='/index.html/')
> + resp = self.get(url='/index_.html/')
> assert resp['status'] == 404, 'index not found 2 status'
> assert (
> resp['headers']['Content-Type'] == 'text/html'
> @@ -123,7 +124,7 @@ class TestStatic(TestApplicationProto):
> assert etag != etag_2, 'different ETag'
> assert etag == self.get(url='/')['headers']['ETag'], 'same ETag'
>
> - with open(temp_dir + '/assets/index.html', 'w') as f:
> + with open(temp_dir + '/assets/index_.html', 'w') as f:
> f.write('blah')
>
> assert etag != self.get(url='/')['headers']['ETag'], 'new ETag'
> @@ -262,7 +263,7 @@ class TestStatic(TestApplicationProto):
> == 'text/x-code/x-blah/x-blah'
> ), 'mime_types string case insensitive'
> assert (
> - self.get(url='/index.html')['headers']['Content-Type']
> + self.get(url='/index_.html')['headers']['Content-Type']
> == 'text/plain'
> ), 'mime_types html'
> assert (
> --
For this patch and all the others in the set: as far as patches are expanding
existing functionality, test patches should do the same thing (expand existing
test cases, not to replace them). So, I would suggest one following patch for
tests for the whole bundle:
# HG changeset patch
# User Andrei Zeliankou <zelenkov at nginx.com>
# Date 1643372490 0
# Fri Jan 28 12:21:30 2022 +0000
# Node ID afe306b9b8d4aed242456aeb0c9e5703e7da3edb
# Parent f05ba33bcd79ab13a838cf80242d784d4c385bbf
Tests: added tests for "index" option.
diff --git a/test/test_static.py b/test/test_static.py
--- a/test/test_static.py
+++ b/test/test_static.py
@@ -86,6 +86,53 @@ class TestStatic(TestApplicationProto):
assert self.get(url='/')['body'] == '0123456789', 'before 1.26.0 2'
def test_static_index(self):
+ def set_index(index):
+ assert 'success' in self.conf(
+ {"share": option.temp_dir + "/assets$uri", "index": index},
+ 'routes/0/action',
+ ), 'configure index'
+
+ set_index('README')
+ assert self.get()['body'] == 'readme', 'index'
+
+ self.conf_delete('routes/0/action/index')
+ assert self.get()['body'] == '0123456789', 'delete index'
+
+ set_index('$host')
+ assert (
+ self.get(headers={"Host": "README", "Connection": "close"})['body']
+ == 'readme'
+ ), 'index var'
+ self.get(headers={"Connection": "close"})[
+ 'status'
+ ] == 404, 'index var empty'
+
+ set_index([])
+ self.get()['status'] == 404, 'index array empty'
+
+ set_index(['blah'])
+ self.get()['status'] == 404, 'index array not found'
+
+ set_index(['$host', 'blah', 'index.html'])
+ assert self.get()['body'] == '0123456789', 'index array'
+ assert (
+ self.get(headers={"Host": "README", "Connection": "close"})['body']
+ == 'readme'
+ ), 'index array 2'
+
+ def test_static_index_invalid(self, skip_alert):
+ skip_alert(r'failed to apply new conf')
+
+ def check_index(index):
+ assert 'error' in self.conf(
+ {"share": option.temp_dir + "/assets$uri", "index": index},
+ 'routes/0/action',
+ )
+
+ check_index({})
+ check_index(['index.html', '$blah'])
+
+ def test_static_index_default(self):
assert self.get(url='/index.html')['body'] == '0123456789', 'index'
assert self.get(url='/')['body'] == '0123456789', 'index 2'
assert self.get(url='//')['body'] == '0123456789', 'index 3’
Also noticed problems:
1) can't compile patch set with specified `--debug` option:
src/nxt_http_static.c:264:23: error: no member named 'index' in 'nxt_http_static_conf_t'
nxt_var_raw(conf->index, &idx);
~~~~ ^
2) router crash when index is the empty array
3) router crash when variable in index value is empty
Kind regards,
Andrei
More information about the unit
mailing list