From baf92303987582d226fb90175f51c4cf8457cb96 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Thu, 12 Mar 2020 17:14:16 +0000 Subject: Tests: skip "close failed" alert in test_proxy_parallel test. --- test/test_proxy.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test_proxy.py b/test/test_proxy.py index 74bd0873..5568550a 100644 --- a/test/test_proxy.py +++ b/test/test_proxy.py @@ -196,6 +196,8 @@ Content-Length: 10 self.assertEqual(resp['body'], payload, 'body') def test_proxy_parallel(self): + self.skip_alerts.append(r'close\(\d+\) failed') + payload = 'X' * 4096 * 257 buff_size = 4096 * 258 -- cgit From 7181a661c5db59a40ba6b3ad0a3d8a571ad10877 Mon Sep 17 00:00:00 2001 From: Konstantin Pavlov Date: Fri, 13 Mar 2020 17:35:47 +0300 Subject: Added a target to export docker images as tarballs --- pkg/docker/Makefile | 27 ++++++++++++++++++++++----- pkg/shasum.mak | 9 +++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 pkg/shasum.mak diff --git a/pkg/docker/Makefile b/pkg/docker/Makefile index d80b8763..4b7b3807 100644 --- a/pkg/docker/Makefile +++ b/pkg/docker/Makefile @@ -1,6 +1,7 @@ #!/usr/bin/make include ../../version +include ../shasum.mak DEFAULT_RELEASE := 1 @@ -29,12 +30,16 @@ MODULE_full="unit=$${UNIT_VERSION} unit-php=$${UNIT_VERSION} unit-python2.7=$${U MODULE_minimal="unit=$${UNIT_VERSION}" +EXPORT_DIR := $(VERSION) + default: - @echo "valid targets: all build dockerfiles push clean" + @echo "valid targets: all build dockerfiles push tag export clean" dockerfiles: $(addprefix Dockerfile., $(MODULES)) -build: dockerfiles $(addprefix build-,$(MODULES)) -push: build $(addprefix push-,$(MODULES)) latest +build: $(addprefix build-,$(MODULES)) +tag: $(addprefix tag-,$(MODULES)) +push: $(addprefix push-,$(MODULES)) latest +export: $(addsuffix .tar.gz,$(addprefix $(EXPORT_DIR)/nginx-unit-$(VERSION)-,$(MODULES))) $(addsuffix .tar.gz.sha512, $(addprefix $(EXPORT_DIR)/nginx-unit-$(VERSION)-,$(MODULES))) Dockerfile.%: ../../version @echo "===> Building $@" @@ -46,17 +51,29 @@ Dockerfile.%: ../../version build-%: Dockerfile.% docker build -t unit:$(VERSION)-$* -f Dockerfile.$* . -push-%: +tag-%: build-% docker tag unit:$(VERSION)-$* nginx/unit:$(VERSION)-$* + +push-%: tag-% docker push nginx/unit:$(VERSION)-$* latest: docker tag nginx/unit:$(VERSION)-full nginx/unit:latest docker push nginx/unit:latest +$(EXPORT_DIR): + mkdir -p $@ + +$(EXPORT_DIR)/nginx-unit-$(VERSION)-%.tar.gz: $(EXPORT_DIR) tag-% + docker save nginx/unit:$(VERSION)-$* | gzip > $@ + +$(EXPORT_DIR)/nginx-unit-$(VERSION)-%.tar.gz.sha512: $(EXPORT_DIR)/nginx-unit-$(VERSION)-%.tar.gz + $(SHA512SUM) $< > $@ + all: $(addprefix Dockerfile., $(MODULES)) clean: rm -f $(addprefix Dockerfile., $(MODULES)) + rm -rf $(EXPORT_DIR) -.PHONY: default all build dockerfiles latest push clean +.PHONY: default all build dockerfiles latest push tag export clean diff --git a/pkg/shasum.mak b/pkg/shasum.mak new file mode 100644 index 00000000..39ec09e6 --- /dev/null +++ b/pkg/shasum.mak @@ -0,0 +1,9 @@ +ifeq ($(shell sha512sum --version >/dev/null 2>&1 || echo FAIL),) +SHA512SUM = sha512sum +else ifeq ($(shell shasum --version >/dev/null 2>&1 || echo FAIL),) +SHA512SUM = shasum -a 512 +else ifeq ($(shell openssl version >/dev/null 2>&1 || echo FAIL),) +SHA512SUM = openssl sha512 +else +SHA512SUM = $(error no SHA-512 tool found!) +endif -- cgit From 3b94102f20290da2d272de651a616794000f9c81 Mon Sep 17 00:00:00 2001 From: Konstantin Pavlov Date: Fri, 13 Mar 2020 17:42:08 +0300 Subject: Added checksum generation to make dist target. While at it, clean up dist artifacts on make clean. --- pkg/Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/Makefile b/pkg/Makefile index 7926606d..15ff075d 100644 --- a/pkg/Makefile +++ b/pkg/Makefile @@ -1,6 +1,7 @@ #!/usr/bin/make include ../version +include shasum.mak VERSION ?= $(NXT_VERSION) RELEASE ?= 1 @@ -14,6 +15,7 @@ dist: -r $(VERSION) \ -p unit-$(VERSION) \ -X "../.hg*" -X "../pkg/" -X "../docs/" + $(SHA512SUM) unit-$(VERSION).tar.gz > unit-$(VERSION).tar.gz.sha512 rpm: @cd rpm && VERSION=$(VERSION) RELEASE=$(RELEASE) make all @@ -32,5 +34,7 @@ clean: @cd deb && make clean @cd docker && make clean @cd npm && make clean + rm -f unit-$(VERSION).tar.gz + rm -f unit-$(VERSION).tar.gz.sha512 .PHONY: default rpm deb docker npm clean -- cgit From b7f0e09de00e3a477f7708223c3db4ed05ab97cd Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Mon, 16 Mar 2020 13:27:10 +0300 Subject: Version bump. --- version | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version b/version index a0f9ae94..b5283235 100644 --- a/version +++ b/version @@ -1,5 +1,5 @@ # Copyright (C) NGINX, Inc. -NXT_VERSION=1.16.0 -NXT_VERNUM=11600 +NXT_VERSION=1.17.0 +NXT_VERNUM=11700 -- cgit From efbcd517fc9ec1ef5a6dfe85cb79bf0a57b954c5 Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Tue, 17 Mar 2020 14:44:06 +0300 Subject: Checking sendfile() availability in configure. Removing SF_NODISKIO flag for FreeBSD sendfile() check because it is not used yet and to support DragonFlyBSD. This closes #414 issue on GitHub. --- auto/sendfile | 56 ++++++++++++++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/auto/sendfile b/auto/sendfile index e5bf3b79..a065f7b6 100644 --- a/auto/sendfile +++ b/auto/sendfile @@ -46,7 +46,7 @@ if [ $nxt_found = no ]; then int main() { off_t sent; - sendfile(-1, -1, 0, 0, NULL, &sent, SF_NODISKIO); + sendfile(-1, -1, 0, 0, NULL, &sent, 0); return 0; }" . auto/feature @@ -57,55 +57,63 @@ if [ $nxt_found = no ]; then fi -NXT_LIBSENDFILE= - if [ $nxt_found = no ]; then - # Solaris 8 sendfilev(). + # MacOSX sendfile(). - nxt_feature="Solaris sendfilev()" - nxt_feature_name=NXT_HAVE_SOLARIS_SENDFILEV - nxt_feature_libs="-lsendfile" - nxt_feature_test="#include + nxt_feature="MacOSX sendfile()" + nxt_feature_name=NXT_HAVE_MACOSX_SENDFILE + nxt_feature_libs= + nxt_feature_test="#include + #include + #include + #include int main() { - size_t sent; - struct sendfilevec vec; + off_t sent; - sendfilev(-1, &vec, 0, &sent); + sendfile(-1, -1, 0, &sent, NULL, 0); return 0; }" . auto/feature if [ $nxt_found = yes ]; then - NXT_HAVE_SOLARIS_SENDFILEV=YES - NXT_LIBSENDFILE=$nxt_feature_libs + NXT_HAVE_MACOSX_SENDFILE=YES fi fi if [ $nxt_found = no ]; then + $echo + $echo "$0: error: no supported sendfile() found." + $echo + exit 1; +fi - # MacOSX sendfile(). - nxt_feature="MacOSX sendfile()" - nxt_feature_name=NXT_HAVE_MACOSX_SENDFILE - nxt_feature_libs= - nxt_feature_test="#include - #include - #include - #include +NXT_LIBSENDFILE= + +if [ $nxt_found = no ]; then + + # Solaris 8 sendfilev(). + + nxt_feature="Solaris sendfilev()" + nxt_feature_name=NXT_HAVE_SOLARIS_SENDFILEV + nxt_feature_libs="-lsendfile" + nxt_feature_test="#include int main() { - off_t sent; + size_t sent; + struct sendfilevec vec; - sendfile(-1, -1, 0, &sent, NULL, 0); + sendfilev(-1, &vec, 0, &sent); return 0; }" . auto/feature if [ $nxt_found = yes ]; then - NXT_HAVE_MACOSX_SENDFILE=YES + NXT_HAVE_SOLARIS_SENDFILEV=YES + NXT_LIBSENDFILE=$nxt_feature_libs fi fi -- cgit From c6f9ca79e6a8517544a0995414de8421a9983687 Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Tue, 17 Mar 2020 14:44:11 +0300 Subject: Fixing body fd access racing condition. To avoid closing the body fd prematurely, the fd value is moved from the request struct to the app link. The body fd should not be closed immediately after the request is sent to the application due to possible request rescheduling. --- src/nxt_router.c | 47 +++++++++++++++++++++++++++++++---------------- src/nxt_router_request.h | 1 + test/test_proxy.py | 2 -- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/nxt_router.c b/src/nxt_router.c index a913284c..d4f25d7e 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -462,6 +462,7 @@ nxt_inline void nxt_request_app_link_init(nxt_task_t *task, nxt_request_app_link_t *req_app_link, nxt_request_rpc_data_t *req_rpc_data) { + nxt_buf_t *body; nxt_event_engine_t *engine; engine = task->thread->engine; @@ -480,6 +481,17 @@ nxt_request_app_link_init(nxt_task_t *task, req_app_link->work.task = &engine->task; req_app_link->work.obj = req_app_link; req_app_link->work.data = engine; + + body = req_rpc_data->request->body; + + if (body != NULL && nxt_buf_is_file(body)) { + req_app_link->body_fd = body->file->fd; + + body->file->fd = -1; + + } else { + req_app_link->body_fd = -1; + } } @@ -513,6 +525,10 @@ nxt_request_app_link_alloc(nxt_task_t *task, nxt_request_app_link_init(task, req_app_link, req_rpc_data); + if (ra_src != NULL) { + req_app_link->body_fd = ra_src->body_fd; + } + req_app_link->mem_pool = mp; return req_app_link; @@ -654,6 +670,12 @@ nxt_request_app_link_release(nxt_task_t *task, req_app_link->app_port = NULL; } + if (req_app_link->body_fd != -1) { + nxt_fd_close(req_app_link->body_fd); + + req_app_link->body_fd = -1; + } + nxt_router_msg_cancel(task, &req_app_link->msg_info, req_app_link->stream); mp = req_app_link->mem_pool; @@ -4774,8 +4796,7 @@ static void nxt_router_app_prepare_request(nxt_task_t *task, nxt_request_app_link_t *req_app_link) { - nxt_fd_t fd; - nxt_buf_t *buf, *body; + nxt_buf_t *buf; nxt_int_t res; nxt_port_t *port, *c_port, *reply_port; nxt_apr_action_t apr_action; @@ -4834,13 +4855,15 @@ nxt_router_app_prepare_request(nxt_task_t *task, goto release_port; } - body = req_app_link->request->body; - fd = (body != NULL && nxt_buf_is_file(body)) ? body->file->fd : -1; + if (req_app_link->body_fd != -1) { + nxt_debug(task, "stream #%uD: send body fd %d", req_app_link->stream, + req_app_link->body_fd); - res = nxt_port_socket_twrite(task, port, - NXT_PORT_MSG_REQ_HEADERS - | NXT_PORT_MSG_CLOSE_FD, - fd, + lseek(req_app_link->body_fd, 0, SEEK_SET); + } + + res = nxt_port_socket_twrite(task, port, NXT_PORT_MSG_REQ_HEADERS, + req_app_link->body_fd, req_app_link->stream, reply_port->id, buf, &req_app_link->msg_info.tracking); @@ -4850,10 +4873,6 @@ nxt_router_app_prepare_request(nxt_task_t *task, goto release_port; } - if (fd != -1) { - body->file->fd = -1; - } - release_port: nxt_router_app_port_release(task, port, apr_action); @@ -5178,10 +5197,6 @@ nxt_router_prepare_msg(nxt_task_t *task, nxt_http_request_t *r, } } - if (r->body != NULL && nxt_buf_is_file(r->body)) { - lseek(r->body->file->fd, 0, SEEK_SET); - } - return out; } diff --git a/src/nxt_router_request.h b/src/nxt_router_request.h index c3d5767e..a38980ee 100644 --- a/src/nxt_router_request.h +++ b/src/nxt_router_request.h @@ -50,6 +50,7 @@ struct nxt_request_app_link_s { nxt_http_request_t *request; nxt_msg_info_t msg_info; nxt_request_rpc_data_t *req_rpc_data; + nxt_fd_t body_fd; nxt_nsec_t res_time; diff --git a/test/test_proxy.py b/test/test_proxy.py index 5568550a..74bd0873 100644 --- a/test/test_proxy.py +++ b/test/test_proxy.py @@ -196,8 +196,6 @@ Content-Length: 10 self.assertEqual(resp['body'], payload, 'body') def test_proxy_parallel(self): - self.skip_alerts.append(r'close\(\d+\) failed') - payload = 'X' * 4096 * 257 buff_size = 4096 * 258 -- cgit From 06c790ac1eea241224d184d48927f5f501df1309 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Thu, 19 Mar 2020 03:15:50 +0000 Subject: Tests: fixed prerequisite in test_share_fallback.py. --- test/test_share_fallback.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_share_fallback.py b/test/test_share_fallback.py index 8c45793e..1a5d4e4b 100644 --- a/test/test_share_fallback.py +++ b/test/test_share_fallback.py @@ -1,10 +1,10 @@ import os import unittest -from unit.applications.proto import TestApplicationProto +from unit.applications.lang.python import TestApplicationPython -class TestStatic(TestApplicationProto): - prerequisites = {} +class TestStatic(TestApplicationPython): + prerequisites = {'modules': ['python']} def setUp(self): super().setUp() -- cgit From 93207d4a8c462525cf2160d2099e44b86aa68b27 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Thu, 19 Mar 2020 03:17:00 +0000 Subject: Tests: test_python_procman.py refactored. --- test/test_python_procman.py | 173 +++++++++++++++----------------------------- 1 file changed, 58 insertions(+), 115 deletions(-) diff --git a/test/test_python_procman.py b/test/test_python_procman.py index 52d8cacb..8fb499f7 100644 --- a/test/test_python_procman.py +++ b/test/test_python_procman.py @@ -8,6 +8,13 @@ from unit.applications.lang.python import TestApplicationPython class TestPythonProcman(TestApplicationPython): prerequisites = {'modules': ['python']} + def setUp(self): + super().setUp() + + self.app_name = "app-" + self.testdir.split('/')[-1] + self.app_proc = 'applications/' + self.app_name + '/processes' + self.load('empty', self.app_name) + def pids_for_process(self): time.sleep(0.2) @@ -19,103 +26,20 @@ class TestPythonProcman(TestApplicationPython): return pids - def setUp(self): - super().setUp() - - self.app_name = "app-" + self.testdir.split('/')[-1] - self.load('empty', self.app_name) - - def test_python_processes_access(self): - self.conf('1', 'applications/' + self.app_name + '/processes') - - self.assertIn( - 'error', - self.conf_get('/applications/' + self.app_name + '/processes/max'), - 'max no access', - ) - self.assertIn( - 'error', - self.conf_get( - '/applications/' + self.app_name + '/processes/spare' - ), - 'spare no access', - ) - self.assertIn( - 'error', - self.conf_get( - '/applications/' + self.app_name + '/processes/idle_timeout' - ), - 'idle_timeout no access', - ) - - def test_python_processes_spare_negative(self): - self.assertIn( - 'error', - self.conf( - {"spare": -1}, 'applications/' + self.app_name + '/processes' - ), - 'negative spare', - ) - - def test_python_processes_max_negative(self): - self.assertIn( - 'error', - self.conf( - {"max": -1}, 'applications/' + self.app_name + '/processes' - ), - 'negative max', - ) - - def test_python_processes_idle_timeout_negative(self): - self.assertIn( - 'error', - self.conf( - {"idle_timeout": -1}, - 'applications/' + self.app_name + '/processes', - ), - 'negative idle_timeout', - ) - - def test_python_processes_spare_gt_max_default(self): - self.assertIn( - 'error', - self.conf( - {"spare": 2}, 'applications/' + self.app_name + '/processes' - ), - 'spare greater than max default', - ) - - def test_python_processes_spare_gt_max(self): - self.assertIn( - 'error', - self.conf( - {"spare": 2, "max": 1, "idle_timeout": 1}, - '/applications/' + self.app_name + '/processes', - ), - 'spare greater than max', - ) + def conf_proc(self, conf, path=None): + if path is None: + path = self.app_proc - def test_python_processes_max_zero(self): - self.assertIn( - 'error', - self.conf( - {"spare": 0, "max": 0, "idle_timeout": 1}, - 'applications/' + self.app_name + '/processes', - ), - 'max 0', - ) + self.assertIn('success', self.conf(conf, path), 'configure processes') def test_python_processes_idle_timeout_zero(self): - self.conf( - {"spare": 0, "max": 2, "idle_timeout": 0}, - 'applications/' + self.app_name + '/processes', - ) + self.conf_proc({"spare": 0, "max": 2, "idle_timeout": 0}) self.get() self.assertEqual(len(self.pids_for_process()), 0, 'idle timeout 0') def test_python_prefork(self): - self.conf('2', 'applications/' + self.app_name + '/processes') + self.conf_proc('2') pids = self.pids_for_process() self.assertEqual(len(pids), 2, 'prefork 2') @@ -123,7 +47,7 @@ class TestPythonProcman(TestApplicationPython): self.get() self.assertSetEqual(self.pids_for_process(), pids, 'prefork still 2') - self.conf('4', 'applications/' + self.app_name + '/processes') + self.conf_proc('4') pids = self.pids_for_process() self.assertEqual(len(pids), 4, 'prefork 4') @@ -135,21 +59,16 @@ class TestPythonProcman(TestApplicationPython): @unittest.skip('not yet') def test_python_prefork_same_processes(self): - self.conf('2', 'applications/' + self.app_name + '/processes') - + self.conf_proc('2') pids = self.pids_for_process() - self.conf('4', 'applications/' + self.app_name + '/processes') - + self.conf_proc('4') pids_new = self.pids_for_process() self.assertTrue(pids.issubset(pids_new), 'prefork same processes') def test_python_ondemand(self): - self.conf( - {"spare": 0, "max": 8, "idle_timeout": 1}, - 'applications/' + self.app_name + '/processes', - ) + self.conf_proc({"spare": 0, "max": 8, "idle_timeout": 1}) self.assertEqual(len(self.pids_for_process()), 0, 'on-demand 0') @@ -169,10 +88,7 @@ class TestPythonProcman(TestApplicationPython): self.stop_all() def test_python_scale_updown(self): - self.conf( - {"spare": 2, "max": 8, "idle_timeout": 1}, - 'applications/' + self.app_name + '/processes', - ) + self.conf_proc({"spare": 2, "max": 8, "idle_timeout": 1}) pids = self.pids_for_process() self.assertEqual(len(pids), 2, 'updown 2') @@ -200,10 +116,7 @@ class TestPythonProcman(TestApplicationPython): self.stop_all() def test_python_reconfigure(self): - self.conf( - {"spare": 2, "max": 6, "idle_timeout": 1}, - 'applications/' + self.app_name + '/processes', - ) + self.conf_proc({"spare": 2, "max": 6, "idle_timeout": 1}) pids = self.pids_for_process() self.assertEqual(len(pids), 2, 'reconf 2') @@ -213,7 +126,7 @@ class TestPythonProcman(TestApplicationPython): self.assertEqual(len(pids_new), 3, 'reconf 3') self.assertTrue(pids.issubset(pids_new), 'reconf 3 only 1 new') - self.conf('6', 'applications/' + self.app_name + '/processes/spare') + self.conf_proc('6', self.app_proc + '/spare') pids = self.pids_for_process() self.assertEqual(len(pids), 6, 'reconf 6') @@ -224,10 +137,7 @@ class TestPythonProcman(TestApplicationPython): self.stop_all() def test_python_idle_timeout(self): - self.conf( - {"spare": 0, "max": 6, "idle_timeout": 2}, - 'applications/' + self.app_name + '/processes', - ) + self.conf_proc({"spare": 0, "max": 6, "idle_timeout": 2}) self.get() pids = self.pids_for_process() @@ -250,10 +160,7 @@ class TestPythonProcman(TestApplicationPython): self.assertEqual(len(self.pids_for_process()), 0, 'idle timed out') def test_python_processes_connection_keepalive(self): - self.conf( - {"spare": 0, "max": 6, "idle_timeout": 2}, - 'applications/' + self.app_name + '/processes', - ) + self.conf_proc({"spare": 0, "max": 6, "idle_timeout": 2}) (resp, sock) = self.get( headers={'Host': 'localhost', 'Connection': 'keep-alive'}, @@ -272,6 +179,42 @@ class TestPythonProcman(TestApplicationPython): sock.close() + def test_python_processes_access(self): + self.conf_proc('1') + + path = '/' + self.app_proc + self.assertIn('error', self.conf_get(path + '/max')) + self.assertIn('error', self.conf_get(path + '/spare')) + self.assertIn('error', self.conf_get(path + '/idle_timeout')) + + def test_python_processes_invalid(self): + self.assertIn( + 'error', self.conf({"spare": -1}, self.app_proc), 'negative spare', + ) + self.assertIn( + 'error', self.conf({"max": -1}, self.app_proc), 'negative max', + ) + self.assertIn( + 'error', + self.conf({"idle_timeout": -1}, self.app_proc,), + 'negative idle_timeout', + ) + self.assertIn( + 'error', + self.conf({"spare": 2}, self.app_proc), + 'spare gt max default', + ) + self.assertIn( + 'error', + self.conf({"spare": 2, "max": 1}, self.app_proc,), + 'spare gt max', + ) + self.assertIn( + 'error', + self.conf({"spare": 0, "max": 0}, self.app_proc,), + 'max zero', + ) + def stop_all(self): self.conf({"listeners": {}, "applications": {}}) -- cgit From c26fbbe53a1ce656e05d3e1e86d019c6173715ab Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Thu, 19 Mar 2020 20:43:35 +0300 Subject: Completing request header buffers to avoid memory leak. Before this fix, only persistent connection request buffers were completed. This issue was introduced in dc403927ab0b. --- src/nxt_h1proto.c | 46 +++++++++++++++++++++++++-------------------- src/nxt_h1proto_websocket.c | 2 +- src/nxt_http.h | 3 ++- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/nxt_h1proto.c b/src/nxt_h1proto.c index 35918bd8..19b84108 100644 --- a/src/nxt_h1proto.c +++ b/src/nxt_h1proto.c @@ -1364,17 +1364,19 @@ nxt_h1p_request_header_send(nxt_task_t *task, nxt_http_request_t *r, void -nxt_h1p_complete_buffers(nxt_task_t *task, nxt_h1proto_t *h1p) +nxt_h1p_complete_buffers(nxt_task_t *task, nxt_h1proto_t *h1p, nxt_bool_t all) { - size_t size; - nxt_buf_t *b, *in, *next; - nxt_conn_t *c; + size_t size; + nxt_buf_t *b, *in, *next; + nxt_conn_t *c; + nxt_work_queue_t *wq; nxt_debug(task, "h1p complete buffers"); b = h1p->buffers; c = h1p->conn; in = c->read; + wq = &task->thread->engine->fast_work_queue; if (b != NULL) { if (in == NULL) { @@ -1390,8 +1392,7 @@ nxt_h1p_complete_buffers(nxt_task_t *task, nxt_h1proto_t *h1p) next = b->next; b->next = NULL; - nxt_work_queue_add(&task->thread->engine->fast_work_queue, - b->completion_handler, task, b, b->parent); + nxt_work_queue_add(wq, b->completion_handler, task, b, b->parent); b = next; } @@ -1403,9 +1404,9 @@ nxt_h1p_complete_buffers(nxt_task_t *task, nxt_h1proto_t *h1p) if (in != NULL) { size = nxt_buf_mem_used_size(&in->mem); - if (size == 0) { - nxt_work_queue_add(&task->thread->engine->fast_work_queue, - in->completion_handler, task, in, in->parent); + if (size == 0 || all) { + nxt_work_queue_add(wq, in->completion_handler, task, in, + in->parent); c->read = NULL; } @@ -1754,7 +1755,7 @@ nxt_h1p_keepalive(nxt_task_t *task, nxt_h1proto_t *h1p, nxt_conn_t *c) nxt_conn_tcp_nodelay_on(task, c); } - nxt_h1p_complete_buffers(task, h1p); + nxt_h1p_complete_buffers(task, h1p, 0); in = c->read; @@ -1952,20 +1953,25 @@ nxt_h1p_shutdown(nxt_task_t *task, nxt_conn_t *c) h1p = c->socket.data; - if (nxt_slow_path(h1p != NULL && h1p->websocket_timer != NULL)) { - timer = &h1p->websocket_timer->timer; + if (h1p != NULL) { + nxt_h1p_complete_buffers(task, h1p, 1); - if (timer->handler != nxt_h1p_conn_ws_shutdown) { - timer->handler = nxt_h1p_conn_ws_shutdown; - nxt_timer_add(task->thread->engine, timer, 0); + if (nxt_slow_path(h1p->websocket_timer != NULL)) { + timer = &h1p->websocket_timer->timer; - } else { - nxt_debug(task, "h1p already scheduled ws shutdown"); - } + if (timer->handler != nxt_h1p_conn_ws_shutdown) { + timer->handler = nxt_h1p_conn_ws_shutdown; + nxt_timer_add(task->thread->engine, timer, 0); - } else { - nxt_h1p_closing(task, c); + } else { + nxt_debug(task, "h1p already scheduled ws shutdown"); + } + + return; + } } + + nxt_h1p_closing(task, c); } diff --git a/src/nxt_h1proto_websocket.c b/src/nxt_h1proto_websocket.c index c9ff899c..42a50a34 100644 --- a/src/nxt_h1proto_websocket.c +++ b/src/nxt_h1proto_websocket.c @@ -135,7 +135,7 @@ nxt_h1p_websocket_frame_start(nxt_task_t *task, nxt_http_request_t *r, c = h1p->conn; c->read = ws_frame; - nxt_h1p_complete_buffers(task, h1p); + nxt_h1p_complete_buffers(task, h1p, 0); in = c->read; c->read_state = &nxt_h1p_read_ws_frame_header_state; diff --git a/src/nxt_http.h b/src/nxt_http.h index 0e0694e5..36ce74c6 100644 --- a/src/nxt_http.h +++ b/src/nxt_http.h @@ -320,7 +320,8 @@ void nxt_h1p_websocket_first_frame_start(nxt_task_t *task, nxt_http_request_t *r, nxt_buf_t *ws_frame); void nxt_h1p_websocket_frame_start(nxt_task_t *task, nxt_http_request_t *r, nxt_buf_t *ws_frame); -void nxt_h1p_complete_buffers(nxt_task_t *task, nxt_h1proto_t *h1p); +void nxt_h1p_complete_buffers(nxt_task_t *task, nxt_h1proto_t *h1p, + nxt_bool_t all); nxt_msec_t nxt_h1p_conn_request_timer_value(nxt_conn_t *c, uintptr_t data); extern const nxt_conn_state_t nxt_h1p_idle_close_state; -- cgit From 59e06e49101ef0eba3c4c9e351c23fa56e2137d8 Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Thu, 19 Mar 2020 22:04:43 +0300 Subject: Completing buffers immediately This fixes crash introduced in 039b00e32e3d. --- src/nxt_h1proto.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/nxt_h1proto.c b/src/nxt_h1proto.c index 19b84108..abc92dd4 100644 --- a/src/nxt_h1proto.c +++ b/src/nxt_h1proto.c @@ -1369,14 +1369,12 @@ nxt_h1p_complete_buffers(nxt_task_t *task, nxt_h1proto_t *h1p, nxt_bool_t all) size_t size; nxt_buf_t *b, *in, *next; nxt_conn_t *c; - nxt_work_queue_t *wq; nxt_debug(task, "h1p complete buffers"); b = h1p->buffers; c = h1p->conn; in = c->read; - wq = &task->thread->engine->fast_work_queue; if (b != NULL) { if (in == NULL) { @@ -1392,7 +1390,7 @@ nxt_h1p_complete_buffers(nxt_task_t *task, nxt_h1proto_t *h1p, nxt_bool_t all) next = b->next; b->next = NULL; - nxt_work_queue_add(wq, b->completion_handler, task, b, b->parent); + b->completion_handler(task, b, b->parent); b = next; } @@ -1405,8 +1403,7 @@ nxt_h1p_complete_buffers(nxt_task_t *task, nxt_h1proto_t *h1p, nxt_bool_t all) size = nxt_buf_mem_used_size(&in->mem); if (size == 0 || all) { - nxt_work_queue_add(wq, in->completion_handler, task, in, - in->parent); + in->completion_handler(task, in, in->parent); c->read = NULL; } -- cgit From c7cc247baa2f9b6cd2499416644adf0bd13ff070 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Mon, 23 Mar 2020 02:12:44 +0000 Subject: Tests: terminate unitd process on exit(). --- test/unit/main.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/test/unit/main.py b/test/unit/main.py index 69234dcc..a1182e5c 100644 --- a/test/unit/main.py +++ b/test/unit/main.py @@ -4,6 +4,7 @@ import sys import stat import time import fcntl +import atexit import shutil import signal import argparse @@ -188,10 +189,9 @@ class TestUnit(unittest.TestCase): self._print_log() def stop(self): - if self._started: - self._stop() - + self._stop() self.stop_processes() + atexit.unregister(self.stop) def _run(self): build_dir = self.pardir + '/build' @@ -224,11 +224,11 @@ class TestUnit(unittest.TestCase): stderr=log, ) + atexit.register(self.stop) + if not self.waitforfiles(self.testdir + '/control.unit.sock'): exit("Could not start unit") - self._started = True - self.skip_alerts = [ r'read signalfd\(4\) failed', r'last message send failed', @@ -238,6 +238,9 @@ class TestUnit(unittest.TestCase): self.skip_sanitizer = False def _stop(self): + if self._p.poll() is not None: + return + with self._p as p: p.send_signal(signal.SIGQUIT) @@ -248,10 +251,8 @@ class TestUnit(unittest.TestCase): "Child process terminated with code " + str(retcode) ) except: - self.fail("Could not terminate unit") p.kill() - - self._started = False + self.fail("Could not terminate unit") def _check_alerts(self, log): found = False @@ -295,11 +296,12 @@ class TestUnit(unittest.TestCase): return for process in self._processes: - process.terminate() - process.join(timeout=5) - if process.is_alive(): - self.fail('Fail to stop process') + process.terminate() + process.join(timeout=15) + + if process.is_alive(): + self.fail('Fail to stop process') def waitforfiles(self, *files): for i in range(50): -- cgit From 3fd4b4cfab9541437d07048f3973368e568fe98f Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Mon, 23 Mar 2020 02:13:46 +0000 Subject: Tests: rearranging functions in main.py. --- test/unit/main.py | 138 +++++++++++++++++++++++++++--------------------------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/test/unit/main.py b/test/unit/main.py index a1182e5c..cdab486f 100644 --- a/test/unit/main.py +++ b/test/unit/main.py @@ -152,47 +152,6 @@ class TestUnit(unittest.TestCase): def setUp(self): self._run() - def tearDown(self): - self.stop() - - # detect errors and failures for current test - - def list2reason(exc_list): - if exc_list and exc_list[-1][0] is self: - return exc_list[-1][1] - - if hasattr(self, '_outcome'): - result = self.defaultTestResult() - self._feedErrorsToResult(result, self._outcome.errors) - else: - result = getattr( - self, '_outcomeForDoCleanups', self._resultForDoCleanups - ) - - success = not list2reason(result.errors) and not list2reason( - result.failures - ) - - # check unit.log for alerts - - unit_log = self.testdir + '/unit.log' - - with open(unit_log, 'r', encoding='utf-8', errors='ignore') as f: - self._check_alerts(f.read()) - - # remove unit.log - - if not TestUnit.save_log and success: - shutil.rmtree(self.testdir) - - else: - self._print_log() - - def stop(self): - self._stop() - self.stop_processes() - atexit.unregister(self.stop) - def _run(self): build_dir = self.pardir + '/build' self.unitd = build_dir + '/unitd' @@ -237,6 +196,47 @@ class TestUnit(unittest.TestCase): ] self.skip_sanitizer = False + def tearDown(self): + self.stop() + + # detect errors and failures for current test + + def list2reason(exc_list): + if exc_list and exc_list[-1][0] is self: + return exc_list[-1][1] + + if hasattr(self, '_outcome'): + result = self.defaultTestResult() + self._feedErrorsToResult(result, self._outcome.errors) + else: + result = getattr( + self, '_outcomeForDoCleanups', self._resultForDoCleanups + ) + + success = not list2reason(result.errors) and not list2reason( + result.failures + ) + + # check unit.log for alerts + + unit_log = self.testdir + '/unit.log' + + with open(unit_log, 'r', encoding='utf-8', errors='ignore') as f: + self._check_alerts(f.read()) + + # remove unit.log + + if not TestUnit.save_log and success: + shutil.rmtree(self.testdir) + + else: + self._print_log() + + def stop(self): + self._stop() + self.stop_processes() + atexit.unregister(self.stop) + def _stop(self): if self._p.poll() is not None: return @@ -254,34 +254,6 @@ class TestUnit(unittest.TestCase): p.kill() self.fail("Could not terminate unit") - def _check_alerts(self, log): - found = False - - alerts = re.findall('.+\[alert\].+', log) - - if alerts: - print('All alerts/sanitizer errors found in log:') - [print(alert) for alert in alerts] - found = True - - if self.skip_alerts: - for skip in self.skip_alerts: - alerts = [al for al in alerts if re.search(skip, al) is None] - - if alerts: - self._print_log(log) - self.assertFalse(alerts, 'alert(s)') - - if not self.skip_sanitizer: - sanitizer_errors = re.findall('.+Sanitizer.+', log) - - if sanitizer_errors: - self._print_log(log) - self.assertFalse(sanitizer_errors, 'sanitizer error(s)') - - if found: - print('skipped.') - def run_process(self, target, *args): if not hasattr(self, '_processes'): self._processes = [] @@ -331,6 +303,34 @@ class TestUnit(unittest.TestCase): for f in files: os.chmod(os.path.join(root, f), 0o777) + def _check_alerts(self, log): + found = False + + alerts = re.findall('.+\[alert\].+', log) + + if alerts: + print('All alerts/sanitizer errors found in log:') + [print(alert) for alert in alerts] + found = True + + if self.skip_alerts: + for skip in self.skip_alerts: + alerts = [al for al in alerts if re.search(skip, al) is None] + + if alerts: + self._print_log(log) + self.assertFalse(alerts, 'alert(s)') + + if not self.skip_sanitizer: + sanitizer_errors = re.findall('.+Sanitizer.+', log) + + if sanitizer_errors: + self._print_log(log) + self.assertFalse(sanitizer_errors, 'sanitizer error(s)') + + if found: + print('skipped.') + @staticmethod def _parse_args(): parser = argparse.ArgumentParser(add_help=False) -- cgit From bac93637edc22e1953ad3bf9361a8a0526b3cdae Mon Sep 17 00:00:00 2001 From: Konstantin Pavlov Date: Thu, 19 Mar 2020 13:16:37 +0300 Subject: Fixed filepath in the image checksum file. --- pkg/docker/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/docker/Makefile b/pkg/docker/Makefile index 4b7b3807..7647e51b 100644 --- a/pkg/docker/Makefile +++ b/pkg/docker/Makefile @@ -68,7 +68,7 @@ $(EXPORT_DIR)/nginx-unit-$(VERSION)-%.tar.gz: $(EXPORT_DIR) tag-% docker save nginx/unit:$(VERSION)-$* | gzip > $@ $(EXPORT_DIR)/nginx-unit-$(VERSION)-%.tar.gz.sha512: $(EXPORT_DIR)/nginx-unit-$(VERSION)-%.tar.gz - $(SHA512SUM) $< > $@ + $(SHA512SUM) $< | sed 's,$(EXPORT_DIR)/,,' > $@ all: $(addprefix Dockerfile., $(MODULES)) -- cgit From b0161df42e9a20c69c912f67176c226ae9172b21 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Mon, 23 Mar 2020 19:09:29 +0000 Subject: Tests: wait for unit.pid file before running tests. Waiting for control.unit.sock was replaced by unit.pid due to current problem with race between connect() and listen() calls for control.unit.sock. This change should be reverted after fix. --- test/unit/main.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/unit/main.py b/test/unit/main.py index cdab486f..3d95a5b1 100644 --- a/test/unit/main.py +++ b/test/unit/main.py @@ -185,7 +185,10 @@ class TestUnit(unittest.TestCase): atexit.register(self.stop) - if not self.waitforfiles(self.testdir + '/control.unit.sock'): + # Due to race between connect() and listen() after the socket binding + # tests waits for unit.pid file which is created after listen(). + + if not self.waitforfiles(self.testdir + '/unit.pid'): exit("Could not start unit") self.skip_alerts = [ -- cgit From ac9ca6d75cf8987b6650923aa126c0d3ab06f41e Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Mon, 23 Mar 2020 19:12:22 +0000 Subject: Tests: added notification on unsuccessful connect(). --- test/unit/http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/http.py b/test/unit/http.py index 47fb48f1..281b27d6 100644 --- a/test/unit/http.py +++ b/test/unit/http.py @@ -60,7 +60,7 @@ class TestHTTP(TestUnit): sock.connect(connect_args) except ConnectionRefusedError: sock.close() - return None + self.fail('Client can\'t connect to the server.') else: sock = kwargs['sock'] -- cgit From 48ad88ee7209fd949f3ed1075f62ca78c2220224 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Mon, 23 Mar 2020 19:18:26 +0000 Subject: Tests: increase default "read_timeout" value to 60s. This change is necessary to avoid errors on slow hosts. Also slightly reworked argument passing to the recvall() function. --- test/unit/applications/websockets.py | 4 ++-- test/unit/http.py | 27 ++++++++++++++------------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/test/unit/applications/websockets.py b/test/unit/applications/websockets.py index ef16f433..eaed2ee7 100644 --- a/test/unit/applications/websockets.py +++ b/test/unit/applications/websockets.py @@ -52,7 +52,7 @@ class TestApplicationWebsocket(TestApplicationProto): ) resp = '' - while select.select([sock], [], [], 30)[0]: + while select.select([sock], [], [], 60)[0]: resp += sock.recv(4096).decode() if ( @@ -70,7 +70,7 @@ class TestApplicationWebsocket(TestApplicationProto): def serialize_close(self, code=1000, reason=''): return struct.pack('!H', code) + reason.encode('utf-8') - def frame_read(self, sock, read_timeout=30): + def frame_read(self, sock, read_timeout=60): def recv_bytes(sock, bytes): data = b'' while select.select([sock], [], [], read_timeout)[0]: diff --git a/test/unit/http.py b/test/unit/http.py index 281b27d6..8c71825a 100644 --- a/test/unit/http.py +++ b/test/unit/http.py @@ -17,11 +17,6 @@ class TestHTTP(TestUnit): port = 7080 if 'port' not in kwargs else kwargs['port'] url = '/' if 'url' not in kwargs else kwargs['url'] http = 'HTTP/1.0' if 'http_10' in kwargs else 'HTTP/1.1' - read_buffer_size = ( - 4096 - if 'read_buffer_size' not in kwargs - else kwargs['read_buffer_size'] - ) headers = ( {'Host': 'localhost', 'Connection': 'close'} @@ -101,12 +96,15 @@ class TestHTTP(TestUnit): resp = '' if 'no_recv' not in kwargs: - read_timeout = ( - 30 if 'read_timeout' not in kwargs else kwargs['read_timeout'] - ) - resp = self.recvall( - sock, read_timeout=read_timeout, buff_size=read_buffer_size - ).decode(encoding) + recvall_kwargs = {} + + if 'read_timeout' in kwargs: + recvall_kwargs['read_timeout'] = kwargs['read_timeout'] + + if 'read_buffer_size' in kwargs: + recvall_kwargs['buff_size'] = kwargs['read_buffer_size'] + + resp = self.recvall(sock, **recvall_kwargs).decode(encoding) self.log_in(resp) @@ -174,9 +172,12 @@ class TestHTTP(TestUnit): def put(self, **kwargs): return self.http('PUT', **kwargs) - def recvall(self, sock, read_timeout=30, buff_size=4096): + def recvall(self, sock, **kwargs): + timeout = 60 if 'read_timeout' not in kwargs else kwargs['read_timeout'] + buff_size = 4096 if 'buff_size' not in kwargs else kwargs['buff_size'] + data = b'' - while select.select([sock], [], [], read_timeout)[0]: + while select.select([sock], [], [], timeout)[0]: try: part = sock.recv(buff_size) except: -- cgit From fd8e524b823629b700ae550faced472757df3fbb Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Wed, 25 Mar 2020 19:14:15 +0300 Subject: Configuration: fixed comments parsing. Unclosed multi-line comments and "/" at the end of JSON shouldn't be allowed. --- src/nxt_conf.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/nxt_conf.c b/src/nxt_conf.c index 2a952c35..3e1130be 100644 --- a/src/nxt_conf.c +++ b/src/nxt_conf.c @@ -1269,6 +1269,7 @@ nxt_conf_json_skip_space(u_char *start, u_char *end) case '\r': continue; case '/': + start = p; state = sw_after_slash; continue; } @@ -1285,7 +1286,6 @@ nxt_conf_json_skip_space(u_char *start, u_char *end) continue; } - p--; break; case sw_single_comment: @@ -1318,6 +1318,10 @@ nxt_conf_json_skip_space(u_char *start, u_char *end) break; } + if (nxt_slow_path(state != sw_normal)) { + return start; + } + return p; } -- cgit From 2e4ad9fbc07a2ed408c017df93e5116d97a221e9 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Wed, 25 Mar 2020 19:31:42 +0000 Subject: Tests: UTF-8 BOM test. --- test/test_configuration.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/test_configuration.py b/test/test_configuration.py index 186e037d..2acb7ccd 100644 --- a/test/test_configuration.py +++ b/test/test_configuration.py @@ -83,6 +83,25 @@ class TestConfiguration(TestControl): 'unicode number', ) + def test_json_utf8_bom(self): + self.assertIn( + 'success', + self.conf( + b"""\xEF\xBB\xBF + { + "app": { + "type": "python", + "processes": {"spare": 0}, + "path": "/app", + "module": "wsgi" + } + } + """, + 'applications', + ), + 'UTF-8 BOM', + ) + def test_applications_open_brace(self): self.assertIn('error', self.conf('{', 'applications'), 'open brace') -- cgit From 8532cf6ae6e9ed7d03dd7e06e36eee63fd9d6ceb Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Wed, 25 Mar 2020 19:40:08 +0000 Subject: Tests: added tests for comments in JSON. --- test/test_configuration.py | 61 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/test/test_configuration.py b/test/test_configuration.py index 2acb7ccd..daba874b 100644 --- a/test/test_configuration.py +++ b/test/test_configuration.py @@ -102,6 +102,67 @@ class TestConfiguration(TestControl): 'UTF-8 BOM', ) + def test_json_comment_single_line(self): + self.assertIn( + 'success', + self.conf( + b""" + // this is bridge + { + "//app": { + "type": "python", // end line + "processes": {"spare": 0}, + // inside of block + "path": "/app", + "module": "wsgi" + } + // double // + } + // end of json \xEF\t + """, + 'applications', + ), + 'single line comments', + ) + + def test_json_comment_multi_line(self): + self.assertIn( + 'success', + self.conf( + b""" + /* this is bridge */ + { + "/*app": { + /** + * multiple lines + **/ + "type": "python", + "processes": /* inline */ {"spare": 0}, + "path": "/app", + "module": "wsgi" + /* + // end of block */ + } + /* blah * / blah /* blah */ + } + /* end of json \xEF\t\b */ + """, + 'applications', + ), + 'multi line comments', + ) + + def test_json_comment_invalid(self): + self.assertIn('error', self.conf(b'/{}', 'applications'), 'slash') + self.assertIn('error', self.conf(b'//{}', 'applications'), 'comment') + self.assertIn('error', self.conf(b'{} /', 'applications'), 'slash end') + self.assertIn( + 'error', self.conf(b'/*{}', 'applications'), 'slash star' + ) + self.assertIn( + 'error', self.conf(b'{} /*', 'applications'), 'slash star end' + ) + def test_applications_open_brace(self): self.assertIn('error', self.conf('{', 'applications'), 'open brace') -- cgit From 5f9c4754cbb1dfec0156b4473d1b31a4da8a3e3d Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Fri, 27 Mar 2020 17:22:52 +0300 Subject: Initialization of the action object made more consistent. --- src/nxt_http_route.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nxt_http_route.c b/src/nxt_http_route.c index d7f20bcb..ffa5f6e7 100644 --- a/src/nxt_http_route.c +++ b/src/nxt_http_route.c @@ -432,8 +432,6 @@ nxt_http_route_match_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, return NULL; } - match->action.u.route = NULL; - match->action.handler = NULL; match->items = n; action_conf = nxt_conf_get_path(cv, &action_path); @@ -613,6 +611,8 @@ nxt_http_route_action_create(nxt_router_temp_conf_t *tmcf, nxt_conf_value_t *cv, return ret; } + nxt_memzero(action, sizeof(nxt_http_action_t)); + conf = accf.pass; if (accf.share != NULL) { @@ -633,7 +633,7 @@ nxt_http_route_action_create(nxt_router_temp_conf_t *tmcf, nxt_conf_value_t *cv, } if (accf.fallback != NULL) { - action->u.fallback = nxt_mp_zalloc(mp, sizeof(nxt_http_action_t)); + action->u.fallback = nxt_mp_alloc(mp, sizeof(nxt_http_action_t)); if (nxt_slow_path(action->u.fallback == NULL)) { return NXT_ERROR; } -- cgit From 8d727774e3a2b2eaf194781c382fb953ed61f755 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Fri, 27 Mar 2020 17:22:52 +0300 Subject: Implemented "return" action. The "return" action can be used to immediately generate a simple HTTP response with an arbitrary status: { "action": { "return": 404 } } This is especially useful for denying access to specific resources. --- auto/sources | 1 + src/nxt_conf_validation.c | 38 ++++++++++++++++++++++++++++++++++---- src/nxt_http.h | 7 +++++++ src/nxt_http_return.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/nxt_http_route.c | 12 ++++++++++++ 5 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 src/nxt_http_return.c diff --git a/auto/sources b/auto/sources index 2283e543..c6b34bbc 100644 --- a/auto/sources +++ b/auto/sources @@ -87,6 +87,7 @@ NXT_LIB_SRCS=" \ src/nxt_http_error.c \ src/nxt_http_route.c \ src/nxt_http_route_addr.c \ + src/nxt_http_return.c \ src/nxt_http_static.c \ src/nxt_http_proxy.c \ src/nxt_application.c \ diff --git a/src/nxt_conf_validation.c b/src/nxt_conf_validation.c index 3a3654bd..ad921a7e 100644 --- a/src/nxt_conf_validation.c +++ b/src/nxt_conf_validation.c @@ -64,6 +64,8 @@ static nxt_int_t nxt_conf_vldt_action(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, void *data); static nxt_int_t nxt_conf_vldt_pass(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, void *data); +static nxt_int_t nxt_conf_vldt_return(nxt_conf_validation_t *vldt, + nxt_conf_value_t *value, void *data); static nxt_int_t nxt_conf_vldt_proxy(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, void *data); static nxt_int_t nxt_conf_vldt_routes(nxt_conf_validation_t *vldt, @@ -354,6 +356,16 @@ static nxt_conf_vldt_object_t nxt_conf_vldt_pass_action_members[] = { }; +static nxt_conf_vldt_object_t nxt_conf_vldt_return_action_members[] = { + { nxt_string("return"), + NXT_CONF_VLDT_INTEGER, + &nxt_conf_vldt_return, + NULL }, + + NXT_CONF_VLDT_END +}; + + static nxt_conf_vldt_object_t nxt_conf_vldt_share_action_members[] = { { nxt_string("share"), NXT_CONF_VLDT_STRING, @@ -978,6 +990,7 @@ nxt_conf_vldt_action(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, } actions[] = { { nxt_string("pass"), nxt_conf_vldt_pass_action_members }, + { nxt_string("return"), nxt_conf_vldt_return_action_members }, { nxt_string("share"), nxt_conf_vldt_share_action_members }, { nxt_string("proxy"), nxt_conf_vldt_proxy_action_members }, }; @@ -993,8 +1006,8 @@ nxt_conf_vldt_action(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, if (members != NULL) { return nxt_conf_vldt_error(vldt, "The \"action\" object must have " - "just one of \"pass\", \"share\" or " - "\"proxy\" options set."); + "just one of \"pass\", \"return\", " + "\"share\", or \"proxy\" options set."); } members = actions[i].members; @@ -1002,8 +1015,8 @@ nxt_conf_vldt_action(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, if (members == NULL) { return nxt_conf_vldt_error(vldt, "The \"action\" object must have " - "either \"pass\", \"share\", or " - "\"proxy\" option set."); + "either \"pass\", \"return\", \"share\", " + "or \"proxy\" option set."); } return nxt_conf_vldt_object(vldt, value, members); @@ -1114,6 +1127,23 @@ error: } +static nxt_int_t +nxt_conf_vldt_return(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, + void *data) +{ + int64_t status; + + status = nxt_conf_get_integer(value); + + if (status < NXT_HTTP_INVALID || status > NXT_HTTP_STATUS_MAX) { + return nxt_conf_vldt_error(vldt, "The \"return\" value is out of " + "allowed HTTP status code range 0-999."); + } + + return NXT_OK; +} + + static nxt_int_t nxt_conf_vldt_proxy(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, void *data) diff --git a/src/nxt_http.h b/src/nxt_http.h index 36ce74c6..a86b77f9 100644 --- a/src/nxt_http.h +++ b/src/nxt_http.h @@ -43,6 +43,9 @@ typedef enum { NXT_HTTP_SERVICE_UNAVAILABLE = 503, NXT_HTTP_GATEWAY_TIMEOUT = 504, NXT_HTTP_VERSION_NOT_SUPPORTED = 505, + NXT_HTTP_SERVER_ERROR_MAX = 599, + + NXT_HTTP_STATUS_MAX = 999, } nxt_http_status_t; @@ -192,6 +195,7 @@ struct nxt_http_action_s { nxt_http_action_t *fallback; nxt_upstream_t *upstream; uint32_t upstream_number; + nxt_http_status_t return_code; } u; nxt_str_t name; @@ -282,6 +286,9 @@ nxt_int_t nxt_upstreams_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, nxt_int_t nxt_upstreams_joint_create(nxt_router_temp_conf_t *tmcf, nxt_upstream_t ***upstream_joint); +nxt_http_action_t *nxt_http_return_handler(nxt_task_t *task, + nxt_http_request_t *r, nxt_http_action_t *action); + nxt_http_action_t *nxt_http_static_handler(nxt_task_t *task, nxt_http_request_t *r, nxt_http_action_t *action); nxt_int_t nxt_http_static_mtypes_init(nxt_mp_t *mp, nxt_lvlhsh_t *hash); diff --git a/src/nxt_http_return.c b/src/nxt_http_return.c new file mode 100644 index 00000000..770f5289 --- /dev/null +++ b/src/nxt_http_return.c @@ -0,0 +1,42 @@ + +/* + * Copyright (C) NGINX, Inc. + */ + +#include +#include + + +static const nxt_http_request_state_t nxt_http_return_send_state; + + +nxt_http_action_t * +nxt_http_return_handler(nxt_task_t *task, nxt_http_request_t *r, + nxt_http_action_t *action) +{ + nxt_http_status_t status; + + status = action->u.return_code; + + if (status >= NXT_HTTP_BAD_REQUEST + && status <= NXT_HTTP_SERVER_ERROR_MAX) + { + nxt_http_request_error(task, r, status); + return NULL; + } + + r->status = status; + r->resp.content_length_n = 0; + r->state = &nxt_http_return_send_state; + + nxt_http_request_header_send(task, r, NULL, NULL); + + return NULL; +} + + +static const nxt_http_request_state_t nxt_http_return_send_state + nxt_aligned(64) = +{ + .error_handler = nxt_http_request_error_handler, +}; diff --git a/src/nxt_http_route.c b/src/nxt_http_route.c index ffa5f6e7..6403a005 100644 --- a/src/nxt_http_route.c +++ b/src/nxt_http_route.c @@ -41,6 +41,7 @@ typedef enum { typedef struct { nxt_conf_value_t *pass; + nxt_conf_value_t *ret; nxt_conf_value_t *share; nxt_conf_value_t *proxy; nxt_conf_value_t *fallback; @@ -575,6 +576,11 @@ static nxt_conf_map_t nxt_http_route_action_conf[] = { NXT_CONF_MAP_PTR, offsetof(nxt_http_route_action_conf_t, pass) }, + { + nxt_string("return"), + NXT_CONF_MAP_PTR, + offsetof(nxt_http_route_action_conf_t, ret) + }, { nxt_string("share"), NXT_CONF_MAP_PTR, @@ -613,6 +619,12 @@ nxt_http_route_action_create(nxt_router_temp_conf_t *tmcf, nxt_conf_value_t *cv, nxt_memzero(action, sizeof(nxt_http_action_t)); + if (accf.ret != NULL) { + action->handler = nxt_http_return_handler; + action->u.return_code = nxt_conf_get_integer(accf.ret); + return NXT_OK; + } + conf = accf.pass; if (accf.share != NULL) { -- cgit From d4b4cb0438d753e7694f8f76c41207bbe01fe790 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Fri, 27 Mar 2020 17:22:52 +0300 Subject: Updated URI escaping table for better conformity with RFC 3986. Now '>', '<', '"', '^', '\', '}', '|', '{', and '`' are also escaped. --- src/nxt_string.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/nxt_string.c b/src/nxt_string.c index d567883f..dfaea6bc 100644 --- a/src/nxt_string.c +++ b/src/nxt_string.c @@ -521,19 +521,17 @@ nxt_encode_uri(u_char *dst, u_char *src, size_t length) static const u_char hex[16] = "0123456789ABCDEF"; - /* " ", "#", "%", "?", %00-%1F, %7F-%FF */ - static const uint32_t escape[] = { 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ /* ?>=< ;:98 7654 3210 /.-, +*)( '&%$ #"! */ - 0x80000029, /* 1000 0000 0000 0000 0000 0000 0010 1001 */ + 0xd000002d, /* 1101 0000 0000 0000 0000 0000 0010 1101 */ /* _^]\ [ZYX WVUT SRQP ONML KJIH GFED CBA@ */ - 0x00000000, /* 0000 0000 0000 0000 0000 0000 0000 0000 */ + 0x50000000, /* 0101 0000 0000 0000 0000 0000 0000 0000 */ - /* ~}| {zyx wvut srqp onml kjih gfed cba` */ - 0x80000000, /* 1000 0000 0000 0000 0000 0000 0000 0000 */ + /* ~}| {zyx wvut srqp onml kjih gfed cba` */ + 0xb8000001, /* 1011 1000 0000 0000 0000 0000 0000 0001 */ 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ -- cgit From 35d6f84426cfaa27587456a8ebb81b13f60e697a Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Fri, 27 Mar 2020 17:22:52 +0300 Subject: Added nxt_is_complex_uri_encoded()/nxt_encode_complex_uri(). --- src/nxt_string.c | 199 +++++++++++++++++++++++++++++++++++++++++++------------ src/nxt_string.h | 3 + 2 files changed, 158 insertions(+), 44 deletions(-) diff --git a/src/nxt_string.c b/src/nxt_string.c index dfaea6bc..667146d6 100644 --- a/src/nxt_string.c +++ b/src/nxt_string.c @@ -457,34 +457,54 @@ nxt_strvers_match(u_char *version, u_char *prefix, size_t length) } +static const uint8_t nxt_hex2int[256] + nxt_aligned(32) = +{ + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 16, 16, 16, 16, 16, 16, + 16, 10, 11, 12, 13, 14, 15, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 10, 11, 12, 13, 14, 15, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, + 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, +}; + + +static const uint32_t nxt_uri_escape[] = { + 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ + + /* ?>=< ;:98 7654 3210 /.-, +*)( '&%$ #"! */ + 0xd000002d, /* 1101 0000 0000 0000 0000 0000 0010 1101 */ + + /* _^]\ [ZYX WVUT SRQP ONML KJIH GFED CBA@ */ + 0x50000000, /* 0101 0000 0000 0000 0000 0000 0000 0000 */ + + /* ~}| {zyx wvut srqp onml kjih gfed cba` */ + 0xb8000001, /* 1011 1000 0000 0000 0000 0000 0000 0001 */ + + 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ + 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ + 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ + 0xffffffff /* 1111 1111 1111 1111 1111 1111 1111 1111 */ +}; + + u_char * nxt_decode_uri(u_char *dst, u_char *src, size_t length) { u_char *end, ch; uint8_t d0, d1; - static const uint8_t hex[256] - nxt_aligned(32) = - { - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 16, 16, 16, 16, 16, 16, - 16, 10, 11, 12, 13, 14, 15, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 10, 11, 12, 13, 14, 15, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, - }; - - nxt_prefetch(&hex['0']); + nxt_prefetch(&nxt_hex2int['0']); end = src + length; @@ -496,8 +516,8 @@ nxt_decode_uri(u_char *dst, u_char *src, size_t length) return NULL; } - d0 = hex[*src++]; - d1 = hex[*src++]; + d0 = nxt_hex2int[*src++]; + d1 = nxt_hex2int[*src++]; if (nxt_slow_path((d0 | d1) >= 16)) { return NULL; @@ -521,24 +541,6 @@ nxt_encode_uri(u_char *dst, u_char *src, size_t length) static const u_char hex[16] = "0123456789ABCDEF"; - static const uint32_t escape[] = { - 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ - - /* ?>=< ;:98 7654 3210 /.-, +*)( '&%$ #"! */ - 0xd000002d, /* 1101 0000 0000 0000 0000 0000 0010 1101 */ - - /* _^]\ [ZYX WVUT SRQP ONML KJIH GFED CBA@ */ - 0x50000000, /* 0101 0000 0000 0000 0000 0000 0000 0000 */ - - /* ~}| {zyx wvut srqp onml kjih gfed cba` */ - 0xb8000001, /* 1011 1000 0000 0000 0000 0000 0000 0001 */ - - 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ - 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ - 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ - 0xffffffff /* 1111 1111 1111 1111 1111 1111 1111 1111 */ - }; - end = src + length; if (dst == NULL) { @@ -549,7 +551,7 @@ nxt_encode_uri(u_char *dst, u_char *src, size_t length) while (src < end) { - if (escape[*src >> 5] & (1U << (*src & 0x1f))) { + if (nxt_uri_escape[*src >> 5] & (1U << (*src & 0x1f))) { n++; } @@ -561,7 +563,7 @@ nxt_encode_uri(u_char *dst, u_char *src, size_t length) while (src < end) { - if (escape[*src >> 5] & (1U << (*src & 0x1f))) { + if (nxt_uri_escape[*src >> 5] & (1U << (*src & 0x1f))) { *dst++ = '%'; *dst++ = hex[*src >> 4]; *dst++ = hex[*src & 0xf]; @@ -575,3 +577,112 @@ nxt_encode_uri(u_char *dst, u_char *src, size_t length) return (uintptr_t) dst; } + + +uintptr_t +nxt_encode_complex_uri(u_char *dst, u_char *src, size_t length) +{ + u_char *reserved, *end, ch; + nxt_uint_t n; + + static const u_char hex[16] = "0123456789ABCDEF"; + + reserved = (u_char *) "?#\0"; + + end = src + length; + + if (dst == NULL) { + + /* Find the number of the characters to be escaped. */ + + n = 0; + + while (src < end) { + ch = *src++; + + if (nxt_uri_escape[ch >> 5] & (1U << (ch & 0x1f))) { + if (ch == reserved[0]) { + reserved++; + continue; + } + + if (ch == reserved[1]) { + reserved += 2; + continue; + } + + n++; + } + } + + return (uintptr_t) n; + } + + while (src < end) { + ch = *src++; + + if (nxt_uri_escape[ch >> 5] & (1U << (ch & 0x1f))) { + if (ch == reserved[0]) { + reserved++; + + } else if (ch == reserved[1]) { + reserved += 2; + + } else { + *dst++ = '%'; + *dst++ = hex[ch >> 4]; + *dst++ = hex[ch & 0xf]; + continue; + } + } + + *dst++ = ch; + } + + return (uintptr_t) dst; +} + + +nxt_bool_t +nxt_is_complex_uri_encoded(u_char *src, size_t length) +{ + u_char *reserved, *end, ch; + uint8_t d0, d1; + + reserved = (u_char *) "?#\0"; + + for (end = src + length; src < end; src++) { + ch = *src; + + if (nxt_uri_escape[ch >> 5] & (1U << (ch & 0x1f))) { + if (ch == '%') { + if (end - src < 2) { + return 0; + } + + d0 = nxt_hex2int[*++src]; + d1 = nxt_hex2int[*++src]; + + if ((d0 | d1) >= 16) { + return 0; + } + + continue; + } + + if (ch == reserved[0]) { + reserved++; + continue; + } + + if (ch == reserved[1]) { + reserved += 2; + continue; + } + + return 0; + } + } + + return 1; +} diff --git a/src/nxt_string.h b/src/nxt_string.h index de498048..d10658f7 100644 --- a/src/nxt_string.h +++ b/src/nxt_string.h @@ -170,6 +170,9 @@ NXT_EXPORT nxt_bool_t nxt_strvers_match(u_char *version, u_char *prefix, NXT_EXPORT u_char *nxt_decode_uri(u_char *dst, u_char *src, size_t length); NXT_EXPORT uintptr_t nxt_encode_uri(u_char *dst, u_char *src, size_t length); +NXT_EXPORT uintptr_t nxt_encode_complex_uri(u_char *dst, u_char *src, + size_t length); +NXT_EXPORT nxt_bool_t nxt_is_complex_uri_encoded(u_char *s, size_t length); #endif /* _NXT_STRING_H_INCLUDED_ */ -- cgit From c63b498f9416d26c1288a86ae4fc0b6007a16142 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Sat, 21 Mar 2020 01:39:00 +0300 Subject: Implemented "location" option for "return" action. This allows to specify redirects: { "action": { "return": 301, "location": "https://www.example.com/" } } --- src/nxt_conf_validation.c | 5 +++++ src/nxt_h1proto.c | 2 ++ src/nxt_http.h | 2 ++ src/nxt_http_return.c | 15 +++++++++++++++ src/nxt_http_route.c | 38 ++++++++++++++++++++++++++++++++++++-- 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/nxt_conf_validation.c b/src/nxt_conf_validation.c index ad921a7e..3227a7e9 100644 --- a/src/nxt_conf_validation.c +++ b/src/nxt_conf_validation.c @@ -362,6 +362,11 @@ static nxt_conf_vldt_object_t nxt_conf_vldt_return_action_members[] = { &nxt_conf_vldt_return, NULL }, + { nxt_string("location"), + NXT_CONF_VLDT_STRING, + NULL, + NULL }, + NXT_CONF_VLDT_END }; diff --git a/src/nxt_h1proto.c b/src/nxt_h1proto.c index abc92dd4..5e3b2f82 100644 --- a/src/nxt_h1proto.c +++ b/src/nxt_h1proto.c @@ -1103,6 +1103,8 @@ static const nxt_str_t nxt_http_redirection[] = { nxt_string("HTTP/1.1 302 Found\r\n"), nxt_string("HTTP/1.1 303 See Other\r\n"), nxt_string("HTTP/1.1 304 Not Modified\r\n"), + nxt_string("HTTP/1.1 307 Temporary Redirect\r\n"), + nxt_string("HTTP/1.1 308 Permanent Redirect\r\n"), }; diff --git a/src/nxt_http.h b/src/nxt_http.h index a86b77f9..638affc8 100644 --- a/src/nxt_http.h +++ b/src/nxt_http.h @@ -23,6 +23,8 @@ typedef enum { NXT_HTTP_FOUND = 302, NXT_HTTP_SEE_OTHER = 303, NXT_HTTP_NOT_MODIFIED = 304, + NXT_HTTP_TEMPORARY_REDIRECT = 307, + NXT_HTTP_PERMANENT_REDIRECT = 308, NXT_HTTP_BAD_REQUEST = 400, NXT_HTTP_FORBIDDEN = 403, diff --git a/src/nxt_http_return.c b/src/nxt_http_return.c index 770f5289..c466cc25 100644 --- a/src/nxt_http_return.c +++ b/src/nxt_http_return.c @@ -14,6 +14,7 @@ nxt_http_action_t * nxt_http_return_handler(nxt_task_t *task, nxt_http_request_t *r, nxt_http_action_t *action) { + nxt_http_field_t *field; nxt_http_status_t status; status = action->u.return_code; @@ -27,6 +28,20 @@ nxt_http_return_handler(nxt_task_t *task, nxt_http_request_t *r, r->status = status; r->resp.content_length_n = 0; + + if (action->name.length > 0) { + field = nxt_list_zero_add(r->resp.fields); + if (nxt_slow_path(field == NULL)) { + nxt_http_request_error(task, r, NXT_HTTP_INTERNAL_SERVER_ERROR); + return NULL; + } + + nxt_http_field_name_set(field, "Location"); + + field->value = action->name.start; + field->value_length = action->name.length; + } + r->state = &nxt_http_return_send_state; nxt_http_request_header_send(task, r, NULL, NULL); diff --git a/src/nxt_http_route.c b/src/nxt_http_route.c index 6403a005..ee22f48d 100644 --- a/src/nxt_http_route.c +++ b/src/nxt_http_route.c @@ -42,6 +42,7 @@ typedef enum { typedef struct { nxt_conf_value_t *pass; nxt_conf_value_t *ret; + nxt_str_t location; nxt_conf_value_t *share; nxt_conf_value_t *proxy; nxt_conf_value_t *fallback; @@ -581,6 +582,11 @@ static nxt_conf_map_t nxt_http_route_action_conf[] = { NXT_CONF_MAP_PTR, offsetof(nxt_http_route_action_conf_t, ret) }, + { + nxt_string("location"), + NXT_CONF_MAP_STR, + offsetof(nxt_http_route_action_conf_t, location) + }, { nxt_string("share"), NXT_CONF_MAP_PTR, @@ -606,6 +612,7 @@ nxt_http_route_action_create(nxt_router_temp_conf_t *tmcf, nxt_conf_value_t *cv, nxt_mp_t *mp; nxt_int_t ret; nxt_str_t name, *string; + nxt_uint_t encode; nxt_conf_value_t *conf; nxt_http_route_action_conf_t accf; @@ -619,9 +626,38 @@ nxt_http_route_action_create(nxt_router_temp_conf_t *tmcf, nxt_conf_value_t *cv, nxt_memzero(action, sizeof(nxt_http_action_t)); + mp = tmcf->router_conf->mem_pool; + if (accf.ret != NULL) { action->handler = nxt_http_return_handler; action->u.return_code = nxt_conf_get_integer(accf.ret); + + if (accf.location.length > 0) { + if (nxt_is_complex_uri_encoded(accf.location.start, + accf.location.length)) + { + string = nxt_str_dup(mp, &action->name, &accf.location); + if (nxt_slow_path(string == NULL)) { + return NXT_ERROR; + } + + } else { + string = &action->name; + + encode = nxt_encode_complex_uri(NULL, accf.location.start, + accf.location.length); + string->length = accf.location.length + encode * 2; + + string->start = nxt_mp_nget(mp, string->length); + if (nxt_slow_path(string->start == NULL)) { + return NXT_ERROR; + } + + nxt_encode_complex_uri(string->start, accf.location.start, + accf.location.length); + } + } + return NXT_OK; } @@ -637,8 +673,6 @@ nxt_http_route_action_create(nxt_router_temp_conf_t *tmcf, nxt_conf_value_t *cv, nxt_conf_get_string(conf, &name); - mp = tmcf->router_conf->mem_pool; - string = nxt_str_dup(mp, &action->name, &name); if (nxt_slow_path(string == NULL)) { return NXT_ERROR; -- cgit From 5f2d07019c44855deabfc3f0a83ab7506f0d0183 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Fri, 27 Mar 2020 15:48:39 +0000 Subject: Tests: increase default "read_timeout" to 60s in message_read(). --- test/unit/applications/websockets.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/applications/websockets.py b/test/unit/applications/websockets.py index eaed2ee7..309d37a3 100644 --- a/test/unit/applications/websockets.py +++ b/test/unit/applications/websockets.py @@ -221,7 +221,7 @@ class TestApplicationWebsocket(TestApplicationProto): op_code = self.OP_CONT pos = end - def message_read(self, sock, read_timeout=10): + def message_read(self, sock, read_timeout=60): frame = self.frame_read(sock, read_timeout=read_timeout) while not frame['fin']: -- cgit From 6e5b5d2a0b97f6e981fef749a267103e21898a72 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Fri, 27 Mar 2020 15:50:09 +0000 Subject: Tests: added tests for "return" action. --- test/test_return.py | 103 ++++++++++++++++++++++++++ test/test_routing.py | 161 +++++++---------------------------------- test/test_routing_tls.py | 26 ++----- test/test_share_fallback.py | 171 +++++++++++++++----------------------------- 4 files changed, 191 insertions(+), 270 deletions(-) create mode 100644 test/test_return.py diff --git a/test/test_return.py b/test/test_return.py new file mode 100644 index 00000000..f973f05b --- /dev/null +++ b/test/test_return.py @@ -0,0 +1,103 @@ +import re +import unittest +from unit.applications.proto import TestApplicationProto + + +class TestReturn(TestApplicationProto): + prerequisites = {} + + def setUp(self): + super().setUp() + + self._load_conf( + { + "listeners": {"*:7080": {"pass": "routes"}}, + "routes": [{"action": {"return": 200}}], + "applications": {}, + } + ) + + def get_resps_sc(self, req=10): + to_send = b"""GET / HTTP/1.1 +Host: localhost + +""" * ( + req - 1 + ) + + to_send += b"""GET / HTTP/1.1 +Host: localhost +Connection: close + +""" + + return self.http(to_send, raw_resp=True, raw=True) + + def test_return(self): + resp = self.get() + self.assertEqual(resp['status'], 200) + self.assertIn('Server', resp['headers']) + self.assertIn('Date', resp['headers']) + self.assertEqual(resp['headers']['Content-Length'], '0') + self.assertEqual(resp['headers']['Connection'], 'close') + self.assertEqual(resp['body'], '', 'body') + + resp = self.post(body='blah') + self.assertEqual(resp['status'], 200) + self.assertEqual(resp['body'], '', 'body') + + resp = self.get_resps_sc() + self.assertEqual(len(re.findall('200 OK', resp)), 10) + self.assertEqual(len(re.findall('Connection:', resp)), 1) + self.assertEqual(len(re.findall('Connection: close', resp)), 1) + + resp = self.get(http_10=True) + self.assertEqual(resp['status'], 200) + self.assertIn('Server', resp['headers']) + self.assertIn('Date', resp['headers']) + self.assertEqual(resp['headers']['Content-Length'], '0') + self.assertNotIn('Connection', resp['headers']) + self.assertEqual(resp['body'], '', 'body') + + def test_return_update(self): + self.assertIn('success', self.conf('0', 'routes/0/action/return')) + + resp = self.get() + self.assertEqual(resp['status'], 0) + self.assertEqual(resp['body'], '') + + self.assertIn('success', self.conf('404', 'routes/0/action/return')) + + resp = self.get() + self.assertEqual(resp['status'], 404) + self.assertNotEqual(resp['body'], '') + + self.assertIn('success', self.conf('598', 'routes/0/action/return')) + + resp = self.get() + self.assertEqual(resp['status'], 598) + self.assertNotEqual(resp['body'], '') + + self.assertIn('success', self.conf('999', 'routes/0/action/return')) + + resp = self.get() + self.assertEqual(resp['status'], 999) + self.assertEqual(resp['body'], '') + + def test_return_invalid(self): + def check_error(conf): + self.assertIn('error', self.conf(conf, 'routes/0/action')) + + check_error({"return": "200"}) + check_error({"return": []}) + check_error({"return": 80.}) + check_error({"return": 1000}) + check_error({"return": 200, "share": "/blah"}) + + self.assertIn( + 'error', self.conf('001', 'routes/0/action/return'), 'leading zero' + ) + + +if __name__ == '__main__': + TestReturn.main() diff --git a/test/test_routing.py b/test/test_routing.py index 950923d6..bf741706 100644 --- a/test/test_routing.py +++ b/test/test_routing.py @@ -16,27 +16,10 @@ class TestRouting(TestApplicationProto): "routes": [ { "match": {"method": "GET"}, - "action": {"pass": "applications/empty"}, + "action": {"return": 200}, } ], - "applications": { - "empty": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + '/python/empty', - "working_directory": self.current_dir - + '/python/empty', - "module": "wsgi", - }, - "mirror": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + '/python/mirror', - "working_directory": self.current_dir - + '/python/mirror', - "module": "wsgi", - }, - }, + "applications": {}, } ), 'routing configure', @@ -48,18 +31,14 @@ class TestRouting(TestApplicationProto): def route_match(self, match): self.assertIn( 'success', - self.route( - {"match": match, "action": {"pass": "applications/empty"}} - ), + self.route({"match": match, "action": {"return": 200}}), 'route match configure', ) def route_match_invalid(self, match): self.assertIn( 'error', - self.route( - {"match": match, "action": {"pass": "applications/empty"}} - ), + self.route({"match": match, "action": {"return": 200}}), 'route match configure invalid', ) @@ -233,24 +212,7 @@ class TestRouting(TestApplicationProto): { "listeners": {"*:7080": {"pass": "routes/main"}}, "routes": {"main": []}, - "applications": { - "empty": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + '/python/empty', - "working_directory": self.current_dir - + '/python/empty', - "module": "wsgi", - }, - "mirror": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + '/python/mirror', - "working_directory": self.current_dir - + '/python/mirror', - "module": "wsgi", - }, - }, + "applications": {}, } ), 'route empty configure', @@ -272,7 +234,7 @@ class TestRouting(TestApplicationProto): def test_routes_route_match_absent(self): self.assertIn( 'success', - self.conf([{"action": {"pass": "applications/empty"}}], 'routes'), + self.conf([{"action": {"return": 200}}], 'routes'), 'route match absent configure', ) @@ -349,14 +311,8 @@ class TestRouting(TestApplicationProto): 'success', self.conf( [ - { - "match": {"method": "GET"}, - "action": {"pass": "applications/empty"}, - }, - { - "match": {"method": "POST"}, - "action": {"pass": "applications/mirror"}, - }, + {"match": {"method": "GET"}, "action": {"return": 200}}, + {"match": {"method": "POST"}, "action": {"return": 201}}, ], 'routes', ), @@ -364,18 +320,7 @@ class TestRouting(TestApplicationProto): ) self.assertEqual(self.get()['status'], 200, 'rules two match first') - self.assertEqual( - self.post( - headers={ - 'Host': 'localhost', - 'Content-Type': 'text/html', - 'Connection': 'close', - }, - body='X', - )['status'], - 200, - 'rules two match second', - ) + self.assertEqual(self.post()['status'], 201, 'rules two match second') def test_routes_two(self): self.assertIn( @@ -393,20 +338,11 @@ class TestRouting(TestApplicationProto): "second": [ { "match": {"host": "localhost"}, - "action": {"pass": "applications/empty"}, + "action": {"return": 200}, } ], }, - "applications": { - "empty": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + '/python/empty', - "working_directory": self.current_dir - + '/python/empty', - "module": "wsgi", - } - }, + "applications": {}, } ), 'routes two configure', @@ -556,7 +492,7 @@ class TestRouting(TestApplicationProto): self.assertIn( 'success', - self.conf([{"action": {"pass": "applications/empty"}}], 'routes'), + self.conf([{"action": {"return": 200}}], 'routes'), 'redefine 2', ) self.assertEqual(self.get()['status'], 200, 'redefine request 2') @@ -569,19 +505,8 @@ class TestRouting(TestApplicationProto): self.conf( { "listeners": {"*:7080": {"pass": "routes/main"}}, - "routes": { - "main": [{"action": {"pass": "applications/empty"}}] - }, - "applications": { - "empty": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + '/python/empty', - "working_directory": self.current_dir - + '/python/empty', - "module": "wsgi", - } - }, + "routes": {"main": [{"action": {"return": 200}}]}, + "applications": {}, } ), 'redefine 4', @@ -595,25 +520,19 @@ class TestRouting(TestApplicationProto): self.assertIn( 'success', - self.conf_post( - {"action": {"pass": "applications/empty"}}, 'routes/main' - ), + self.conf_post({"action": {"return": 200}}, 'routes/main'), 'redefine 6', ) self.assertEqual(self.get()['status'], 200, 'redefine request 6') self.assertIn( 'error', - self.conf( - {"action": {"pass": "applications/empty"}}, 'routes/main/2' - ), + self.conf({"action": {"return": 200}}, 'routes/main/2'), 'redefine 7', ) self.assertIn( 'success', - self.conf( - {"action": {"pass": "applications/empty"}}, 'routes/main/1' - ), + self.conf({"action": {"return": 201}}, 'routes/main/1'), 'redefine 8', ) @@ -631,10 +550,7 @@ class TestRouting(TestApplicationProto): self.assertIn( 'success', self.conf_post( - { - "match": {"method": "POST"}, - "action": {"pass": "applications/empty"}, - }, + {"match": {"method": "POST"}, "action": {"return": 200}}, 'routes', ), 'routes edit configure 2', @@ -654,9 +570,7 @@ class TestRouting(TestApplicationProto): self.assertEqual(self.post()['status'], 200, 'routes edit POST 2') self.assertIn( - 'success', - self.conf_delete('routes/0'), - 'routes edit configure 3', + 'success', self.conf_delete('routes/0'), 'routes edit configure 3', ) self.assertEqual(self.get()['status'], 404, 'routes edit GET 3') @@ -682,9 +596,7 @@ class TestRouting(TestApplicationProto): self.assertEqual(self.post()['status'], 200, 'routes edit POST 4') self.assertIn( - 'success', - self.conf_delete('routes/0'), - 'routes edit configure 5', + 'success', self.conf_delete('routes/0'), 'routes edit configure 5', ) self.assertEqual(self.get()['status'], 404, 'routes edit GET 5') @@ -693,10 +605,7 @@ class TestRouting(TestApplicationProto): self.assertIn( 'success', self.conf_post( - { - "match": {"method": "POST"}, - "action": {"pass": "applications/empty"}, - }, + {"match": {"method": "POST"}, "action": {"return": 200},}, 'routes', ), 'routes edit configure 6', @@ -710,19 +619,8 @@ class TestRouting(TestApplicationProto): self.conf( { "listeners": {"*:7080": {"pass": "routes/main"}}, - "routes": { - "main": [{"action": {"pass": "applications/empty"}}] - }, - "applications": { - "empty": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + '/python/empty', - "working_directory": self.current_dir - + '/python/empty', - "module": "wsgi", - } - }, + "routes": {"main": [{"action": {"return": 200}}]}, + "applications": {}, } ), 'route edit configure 7', @@ -1838,20 +1736,11 @@ class TestRouting(TestApplicationProto): "second": [ { "match": {"destination": ["127.0.0.1:7081"]}, - "action": {"pass": "applications/empty"}, + "action": {"return": 200}, } ], }, - "applications": { - "empty": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + "/python/empty", - "working_directory": self.current_dir - + "/python/empty", - "module": "wsgi", - } - }, + "applications": {}, } ), 'proxy configure', diff --git a/test/test_routing_tls.py b/test/test_routing_tls.py index c6648095..36bd9057 100644 --- a/test/test_routing_tls.py +++ b/test/test_routing_tls.py @@ -2,9 +2,9 @@ from unit.applications.tls import TestApplicationTLS class TestRoutingTLS(TestApplicationTLS): - prerequisites = {'modules': ['python', 'openssl']} + prerequisites = {'modules': ['openssl']} - def test_routes_match_scheme(self): + def test_routes_match_scheme_tls(self): self.certificate() self.assertIn( @@ -21,35 +21,21 @@ class TestRoutingTLS(TestApplicationTLS): "routes": [ { "match": {"scheme": "http"}, - "action": {"pass": "applications/empty"}, + "action": {"return": 200}, }, { "match": {"scheme": "https"}, - "action": {"pass": "applications/204_no_content"}, + "action": {"return": 201}, }, ], - "applications": { - "empty": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + "/python/empty", - "module": "wsgi", - }, - "204_no_content": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir - + "/python/204_no_content", - "module": "wsgi", - }, - }, + "applications": {}, } ), 'scheme configure', ) self.assertEqual(self.get()['status'], 200, 'http') - self.assertEqual(self.get_ssl(port=7081)['status'], 204, 'https') + self.assertEqual(self.get_ssl(port=7081)['status'], 201, 'https') if __name__ == '__main__': diff --git a/test/test_share_fallback.py b/test/test_share_fallback.py index 1a5d4e4b..c51e43ee 100644 --- a/test/test_share_fallback.py +++ b/test/test_share_fallback.py @@ -1,10 +1,10 @@ import os import unittest -from unit.applications.lang.python import TestApplicationPython +from unit.applications.proto import TestApplicationProto -class TestStatic(TestApplicationPython): - prerequisites = {'modules': ['python']} +class TestStatic(TestApplicationProto): + prerequisites = {} def setUp(self): super().setUp() @@ -20,19 +20,10 @@ class TestStatic(TestApplicationPython): { "listeners": { "*:7080": {"pass": "routes"}, - "*:7081": {"pass": "applications/empty"}, + "*:7081": {"pass": "routes"}, }, "routes": [{"action": {"share": self.testdir + "/assets"}}], - "applications": { - "empty": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + "/python/empty", - "working_directory": self.current_dir - + "/python/empty", - "module": "wsgi", - } - }, + "applications": {}, } ) @@ -41,37 +32,22 @@ class TestStatic(TestApplicationPython): super().tearDown() + def action_update(self, conf): + self.assertIn('success', self.conf(conf, 'routes/0/action')) + def test_fallback(self): - self.assertIn( - 'success', - self.conf({"share": "/blah"}, 'routes/0/action'), - 'configure bad path no fallback', - ) + self.action_update({"share": "/blah"}) self.assertEqual(self.get()['status'], 404, 'bad path no fallback') - self.assertIn( - 'success', - self.conf( - {"share": "/blah", "fallback": {"pass": "applications/empty"}}, - 'routes/0/action', - ), - 'configure bad path fallback', - ) + self.action_update({"share": "/blah", "fallback": {"return": 200}}) + resp = self.get() self.assertEqual(resp['status'], 200, 'bad path fallback status') self.assertEqual(resp['body'], '', 'bad path fallback') def test_fallback_valid_path(self): - self.assertIn( - 'success', - self.conf( - { - "share": self.testdir + "/assets", - "fallback": {"pass": "applications/empty"}, - }, - 'routes/0/action', - ), - 'configure fallback', + self.action_update( + {"share": self.testdir + "/assets", "fallback": {"return": 200}} ) resp = self.get() self.assertEqual(resp['status'], 200, 'fallback status') @@ -90,36 +66,28 @@ class TestStatic(TestApplicationPython): ) def test_fallback_nested(self): - self.assertIn( - 'success', - self.conf( - { - "share": "/blah", - "fallback": { - "share": "/blah/blah", - "fallback": {"pass": "applications/empty"}, - }, + self.action_update( + { + "share": "/blah", + "fallback": { + "share": "/blah/blah", + "fallback": {"return": 200}, }, - 'routes/0/action', - ), - 'configure fallback nested', + } ) + resp = self.get() self.assertEqual(resp['status'], 200, 'fallback nested status') self.assertEqual(resp['body'], '', 'fallback nested') def test_fallback_share(self): - self.assertIn( - 'success', - self.conf( - { - "share": "/blah", - "fallback": {"share": self.testdir + "/assets"}, - }, - 'routes/0/action', - ), - 'configure fallback share', + self.action_update( + { + "share": "/blah", + "fallback": {"share": self.testdir + "/assets"}, + } ) + resp = self.get() self.assertEqual(resp['status'], 200, 'fallback share status') self.assertEqual(resp['body'], '0123456789', 'fallback share') @@ -136,76 +104,51 @@ class TestStatic(TestApplicationPython): self.assertIn( 'success', self.conf( - { - "share": "/blah", - "fallback": {"proxy": "http://127.0.0.1:7081"}, - }, - 'routes/0/action', + [ + { + "match": {"destination": "*:7081"}, + "action": {"return": 200}, + }, + { + "action": { + "share": "/blah", + "fallback": {"proxy": "http://127.0.0.1:7081"}, + } + }, + ], + 'routes', ), - 'configure fallback proxy', + 'configure fallback proxy route', ) + resp = self.get() self.assertEqual(resp['status'], 200, 'fallback proxy status') self.assertEqual(resp['body'], '', 'fallback proxy') @unittest.skip('not yet') def test_fallback_proxy_cycle(self): - self.assertIn( - 'success', - self.conf( - { - "share": "/blah", - "fallback": {"proxy": "http://127.0.0.1:7080"}, - }, - 'routes/0/action', - ), - 'configure fallback cycle', + self.action_update( + { + "share": "/blah", + "fallback": {"proxy": "http://127.0.0.1:7080"}, + } ) self.assertNotEqual(self.get()['status'], 200, 'fallback cycle') - self.assertIn( - 'success', self.conf_delete('listeners/*:7081'), 'delete listener' - ) + self.assertIn('success', self.conf_delete('listeners/*:7081')) self.assertNotEqual(self.get()['status'], 200, 'fallback cycle 2') def test_fallback_invalid(self): - self.assertIn( - 'error', - self.conf({"share": "/blah", "fallback": {}}, 'routes/0/action'), - 'configure fallback empty', - ) - self.assertIn( - 'error', - self.conf({"share": "/blah", "fallback": ""}, 'routes/0/action'), - 'configure fallback not object', - ) - self.assertIn( - 'error', - self.conf( - { - "proxy": "http://127.0.0.1:7081", - "fallback": {"share": "/blah"}, - }, - 'routes/0/action', - ), - 'configure fallback proxy invalid', - ) - self.assertIn( - 'error', - self.conf( - { - "pass": "applications/empty", - "fallback": {"share": "/blah"}, - }, - 'routes/0/action', - ), - 'configure fallback pass invalid', - ) - self.assertIn( - 'error', - self.conf({"fallback": {"share": "/blah"}}, 'routes/0/action'), - 'configure fallback only', + def check_error(conf): + self.assertIn('error', self.conf(conf, 'routes/0/action')) + + check_error({"share": "/blah", "fallback": {}}) + check_error({"share": "/blah", "fallback": ""}) + check_error({"return": 200, "fallback": {"share": "/blah"}}) + check_error( + {"proxy": "http://127.0.0.1:7081", "fallback": {"share": "/blah"}} ) + check_error({"fallback": {"share": "/blah"}}) if __name__ == '__main__': -- cgit From f94e31b294d69df40b59970ef4c721324cd3596e Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Fri, 27 Mar 2020 17:29:45 +0000 Subject: Tests: added tests for "location" option. --- test/test_return.py | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/test/test_return.py b/test/test_return.py index f973f05b..e3edb700 100644 --- a/test/test_return.py +++ b/test/test_return.py @@ -84,6 +84,97 @@ Connection: close self.assertEqual(resp['status'], 999) self.assertEqual(resp['body'], '') + def test_return_location(self): + reserved = ":/?#[]@!$&'()*+,;=" + unreserved = ("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" + "0123456789-._~") + unsafe = " \"%<>\\^`{|}" + unsafe_enc = "%20%22%25%3C%3E%5C%5E%60%7B%7C%7D" + + def check_location(location, expect=None): + if expect is None: + expect = location + + self.assertIn( + 'success', + self.conf( + {"return": 301, "location": location}, 'routes/0/action' + ), + 'configure location' + ) + + self.assertEqual(self.get()['headers']['Location'], expect) + + # FAIL: can't specify empty header value. + # check_location("") + + check_location(reserved) + + # After first "?" all other "?" encoded. + check_location("/?" + reserved, "/?:/%3F#[]@!$&'()*+,;=") + check_location("???", "?%3F%3F") + + # After first "#" all other "?" or "#" encoded. + check_location("/#" + reserved, "/#:/%3F%23[]@!$&'()*+,;=") + check_location("##?#?", "#%23%3F%23%3F") + + # After first "?" next "#" not encoded. + check_location("/?#" + reserved, "/?#:/%3F%23[]@!$&'()*+,;=") + check_location("??##", "?%3F#%23") + check_location("/?##?", "/?#%23%3F") + + # Unreserved never encoded. + check_location(unreserved) + check_location("/" + unreserved + "?" + unreserved + "#" + unreserved) + + # Unsafe always encoded. + check_location(unsafe, unsafe_enc) + check_location("?" + unsafe, "?" + unsafe_enc) + check_location("#" + unsafe, "#" + unsafe_enc) + + # %00-%20 and %7F-%FF always encoded. + check_location(u"\u0000\u0018\u001F\u0020\u0021", "%00%18%1F%20!") + check_location(u"\u007F\u0080н\u20BD", "%7F%C2%80%D0%BD%E2%82%BD") + + # Encoded string detection. If at least one char need to be encoded + # then whole string will be encoded. + check_location("%20") + check_location("/%20?%20#%20") + check_location(" %20", "%20%2520") + check_location("%20 ", "%2520%20") + check_location("/%20?%20#%20 ", "/%2520?%2520#%2520%20") + + def test_return_location_edit(self): + self.assertIn( + 'success', + self.conf( + {"return": 302, "location": "blah"}, 'routes/0/action' + ), + 'configure init location' + ) + self.assertEqual(self.get()['headers']['Location'], 'blah') + + self.assertIn( + 'success', + self.conf_delete('routes/0/action/location'), + 'location delete' + ) + self.assertNotIn('Location', self.get()['headers']) + + self.assertIn( + 'success', + self.conf('"blah"', 'routes/0/action/location'), + 'location restore' + ) + self.assertEqual(self.get()['headers']['Location'], 'blah') + + self.assertIn( + 'error', + self.conf_post('"blah"', 'routes/0/action/location'), + 'location method not allowed' + ) + self.assertEqual(self.get()['headers']['Location'], 'blah') + def test_return_invalid(self): def check_error(conf): self.assertIn('error', self.conf(conf, 'routes/0/action')) @@ -98,6 +189,9 @@ Connection: close 'error', self.conf('001', 'routes/0/action/return'), 'leading zero' ) + check_error({"return": 301, "location": 0}) + check_error({"return": 301, "location": []}) + if __name__ == '__main__': TestReturn.main() -- cgit From 82b899b1365431951afc1da9b2b30065ac98fc94 Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Mon, 30 Mar 2020 14:08:20 +0300 Subject: Attributing libunit logging function for arguments validation. --- src/java/nxt_jni_InputStream.c | 4 ++-- src/nxt_unit.c | 12 +++++++----- src/nxt_unit.h | 23 +++++++++++++++++++++-- src/perl/nxt_perl_psgi.c | 2 +- 4 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/java/nxt_jni_InputStream.c b/src/java/nxt_jni_InputStream.c index 3b74b0c1..fabff685 100644 --- a/src/java/nxt_jni_InputStream.c +++ b/src/java/nxt_jni_InputStream.c @@ -104,7 +104,7 @@ nxt_java_InputStream_readLine(JNIEnv *env, jclass cls, res = nxt_unit_request_read(req, data + off, res); } - nxt_unit_req_debug(req, "readLine '%.*s'", res, (char *) data + off); + nxt_unit_req_debug(req, "readLine '%.*s'", (int) res, (char *) data + off); (*env)->ReleasePrimitiveArrayCritical(env, out, data, 0); @@ -152,7 +152,7 @@ nxt_java_InputStream_read(JNIEnv *env, jclass cls, jlong req_info_ptr, res = nxt_unit_request_read(req, data + off, len); - nxt_unit_req_debug(req, "read '%.*s'", res, (char *) data + off); + nxt_unit_req_debug(req, "read '%.*s'", (int) res, (char *) data + off); (*env)->ReleasePrimitiveArrayCritical(env, b, data, 0); diff --git a/src/nxt_unit.c b/src/nxt_unit.c index 7a4124fb..77e36771 100644 --- a/src/nxt_unit.c +++ b/src/nxt_unit.c @@ -971,8 +971,10 @@ nxt_unit_process_req_headers(nxt_unit_ctx_t *ctx, nxt_unit_recv_msg_t *recv_msg) req_impl->websocket = 0; nxt_unit_debug(ctx, "#%"PRIu32": %.*s %.*s (%d)", recv_msg->stream, - (int) r->method_length, nxt_unit_sptr_get(&r->method), - (int) r->target_length, nxt_unit_sptr_get(&r->target), + (int) r->method_length, + (char *) nxt_unit_sptr_get(&r->method), + (int) r->target_length, + (char *) nxt_unit_sptr_get(&r->target), (int) r->content_length); lib = nxt_container_of(ctx->unit, nxt_unit_impl_t, unit); @@ -2084,7 +2086,7 @@ nxt_unit_mmap_buf_send(nxt_unit_ctx_t *ctx, uint32_t stream, nxt_unit_debug(ctx, "process %d allocated_chunks %d", mmap_buf->process->pid, - mmap_buf->process->outgoing.allocated_chunks); + (int) mmap_buf->process->outgoing.allocated_chunks); } else { if (nxt_slow_path(mmap_buf->plain_ptr == NULL @@ -2972,7 +2974,7 @@ unlock: nxt_unit_debug(ctx, "process %d allocated_chunks %d", process->pid, - process->outgoing.allocated_chunks); + (int) process->outgoing.allocated_chunks); pthread_mutex_unlock(&process->outgoing.mutex); @@ -3691,7 +3693,7 @@ nxt_unit_mmap_release(nxt_unit_ctx_t *ctx, nxt_unit_debug(ctx, "process %d allocated_chunks %d", process->pid, - process->outgoing.allocated_chunks); + (int) process->outgoing.allocated_chunks); } if (hdr->dst_pid == lib->pid diff --git a/src/nxt_unit.h b/src/nxt_unit.h index 900f3ac2..596dd8b6 100644 --- a/src/nxt_unit.h +++ b/src/nxt_unit.h @@ -356,10 +356,29 @@ int nxt_unit_websocket_retain(nxt_unit_websocket_frame_t *ws); void nxt_unit_websocket_done(nxt_unit_websocket_frame_t *ws); -void nxt_unit_log(nxt_unit_ctx_t *ctx, int level, const char* fmt, ...); +#if defined __has_attribute + +#if __has_attribute(format) + +#define NXT_ATTR_FORMAT __attribute__((format(printf, 3, 4))) + +#endif + +#endif + + +#if !defined(NXT_ATTR_FORMAT) + +#define NXT_ATTR_FORMAT + +#endif + + +void nxt_unit_log(nxt_unit_ctx_t *ctx, int level, const char* fmt, ...) + NXT_ATTR_FORMAT; void nxt_unit_req_log(nxt_unit_request_info_t *req, int level, - const char* fmt, ...); + const char* fmt, ...) NXT_ATTR_FORMAT; #if (NXT_DEBUG) diff --git a/src/perl/nxt_perl_psgi.c b/src/perl/nxt_perl_psgi.c index 16159b5b..548e6daa 100644 --- a/src/perl/nxt_perl_psgi.c +++ b/src/perl/nxt_perl_psgi.c @@ -166,7 +166,7 @@ nxt_perl_psgi_io_error_write(PerlInterpreter *my_perl, nxt_perl_psgi_input_t *input; input = (nxt_perl_psgi_input_t *) arg->ctx; - nxt_unit_req_error(input->req, "Perl: %s", vbuf); + nxt_unit_req_error(input->req, "Perl: %s", (const char*) vbuf); return (long) length; } -- cgit From ab7b42a072e741b226749c416440f89fcaff3d2c Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Mon, 30 Mar 2020 14:18:41 +0300 Subject: Handling change file message in libunit. This is required for proper log file rotation action. --- src/nxt_unit.c | 10 ++++++++++ test/python/log_body/wsgi.py | 2 +- test/test_usr1.py | 1 - 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/nxt_unit.c b/src/nxt_unit.c index 77e36771..55926431 100644 --- a/src/nxt_unit.c +++ b/src/nxt_unit.c @@ -767,6 +767,16 @@ nxt_unit_process_msg(nxt_unit_ctx_t *ctx, nxt_unit_port_id_t *port_id, case _NXT_PORT_MSG_CHANGE_FILE: nxt_unit_debug(ctx, "#%"PRIu32": change_file: fd %d", port_msg->stream, recv_msg.fd); + + if (dup2(recv_msg.fd, lib->log_fd) == -1) { + nxt_unit_alert(ctx, "#%"PRIu32": dup2(%d, %d) failed: %s (%d)", + port_msg->stream, recv_msg.fd, lib->log_fd, + strerror(errno), errno); + + goto fail; + } + + rc = NXT_UNIT_OK; break; case _NXT_PORT_MSG_MMAP: diff --git a/test/python/log_body/wsgi.py b/test/python/log_body/wsgi.py index 9dcb1b0c..0ec07a68 100644 --- a/test/python/log_body/wsgi.py +++ b/test/python/log_body/wsgi.py @@ -2,7 +2,7 @@ def application(environ, start_response): content_length = int(environ.get('CONTENT_LENGTH', 0)) body = bytes(environ['wsgi.input'].read(content_length)) - environ['wsgi.errors'].write(body) + environ['wsgi.errors'].write(body.decode()) environ['wsgi.errors'].flush() start_response('200', [('Content-Length', '0')]) diff --git a/test/test_usr1.py b/test/test_usr1.py index 2b4f394b..204e2e0c 100644 --- a/test/test_usr1.py +++ b/test/test_usr1.py @@ -51,7 +51,6 @@ class TestUSR1(TestApplicationPython): self.search_in_log(r'/usr1', log_new), 'rename new 2' ) - @unittest.skip('not yet') def test_usr1_unit_log(self): self.load('log_body') -- cgit From 0935630cba069d6619e967404bb6c7c2a93fbe7e Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Mon, 30 Mar 2020 14:18:51 +0300 Subject: Fixing application process infinite loop. Main process exiting before app process init may have caused hanging. --- go/port.go | 4 ++++ src/nxt_unit.c | 49 +++++++++++++++++++++++++------------------- test/test_node_websockets.py | 2 +- test/unit/main.py | 1 - 4 files changed, 33 insertions(+), 23 deletions(-) diff --git a/go/port.go b/go/port.go index a68cae74..72d33d31 100644 --- a/go/port.go +++ b/go/port.go @@ -138,6 +138,8 @@ func nxt_go_port_send(pid C.int, id C.int, buf unsafe.Pointer, buf_size C.int, if err != nil { nxt_go_warn("write result %d (%d), %s", n, oobn, err) + + n = -1 } return C.ssize_t(n) @@ -164,6 +166,8 @@ func nxt_go_port_recv(pid C.int, id C.int, buf unsafe.Pointer, buf_size C.int, if err != nil { nxt_go_warn("read result %d (%d), %s", n, oobn, err) + + n = -1 } return C.ssize_t(n) diff --git a/src/nxt_unit.c b/src/nxt_unit.c index 55926431..160b849a 100644 --- a/src/nxt_unit.c +++ b/src/nxt_unit.c @@ -2635,7 +2635,6 @@ nxt_unit_buf_read(nxt_unit_buf_t **b, uint64_t *len, void *dst, size_t size) void nxt_unit_request_done(nxt_unit_request_info_t *req, int rc) { - ssize_t res; uint32_t size; nxt_port_msg_t msg; nxt_unit_impl_t *lib; @@ -2690,12 +2689,8 @@ skip_response_send: msg.mf = 0; msg.tracking = 0; - res = lib->callbacks.port_send(req->ctx, &req->response_port, - &msg, sizeof(msg), NULL, 0); - if (nxt_slow_path(res != sizeof(msg))) { - nxt_unit_req_alert(req, "last message send failed: %s (%d)", - strerror(errno), errno); - } + (void) lib->callbacks.port_send(req->ctx, &req->response_port, + &msg, sizeof(msg), NULL, 0); nxt_unit_request_info_release(req); } @@ -3013,9 +3008,6 @@ nxt_unit_send_oosm(nxt_unit_ctx_t *ctx, nxt_unit_port_id_t *port_id) res = lib->callbacks.port_send(ctx, port_id, &msg, sizeof(msg), NULL, 0); if (nxt_slow_path(res != sizeof(msg))) { - nxt_unit_warn(ctx, "failed to send oosm to %d: %s (%d)", - (int) port_id->pid, strerror(errno), errno); - return NXT_UNIT_ERROR; } @@ -3039,6 +3031,7 @@ nxt_unit_wait_shm_ack(nxt_unit_ctx_t *ctx) } nxt_unit_read_buf(ctx, rbuf); + if (nxt_slow_path(rbuf->size < (ssize_t) sizeof(nxt_port_msg_t))) { nxt_unit_read_buf_release(ctx, rbuf); @@ -3294,9 +3287,6 @@ nxt_unit_send_mmap(nxt_unit_ctx_t *ctx, nxt_unit_port_id_t *port_id, int fd) res = lib->callbacks.port_send(ctx, port_id, &msg, sizeof(msg), &cmsg, sizeof(cmsg)); if (nxt_slow_path(res != sizeof(msg))) { - nxt_unit_warn(ctx, "failed to send shm to %d: %s (%d)", - (int) port_id->pid, strerror(errno), errno); - return NXT_UNIT_ERROR; } @@ -3739,9 +3729,6 @@ nxt_unit_send_shm_ack(nxt_unit_ctx_t *ctx, pid_t pid) res = lib->callbacks.port_send(ctx, &port_id, &msg, sizeof(msg), NULL, 0); if (nxt_slow_path(res != sizeof(msg))) { - nxt_unit_warn(ctx, "failed to send ack to %d: %s (%d)", - (int) port_id.pid, strerror(errno), errno); - return NXT_UNIT_ERROR; } @@ -3894,6 +3881,10 @@ nxt_unit_run(nxt_unit_ctx_t *ctx) while (nxt_fast_path(lib->online)) { rc = nxt_unit_run_once(ctx); + + if (nxt_slow_path(rc != NXT_UNIT_OK)) { + break; + } } return rc; @@ -4552,14 +4543,24 @@ nxt_unit_port_send(nxt_unit_ctx_t *ctx, int fd, msg.msg_control = (void *) oob; msg.msg_controllen = oob_size; +retry: + res = sendmsg(fd, &msg, 0); if (nxt_slow_path(res == -1)) { - nxt_unit_warn(ctx, "port_send(%d, %d) failed: %s (%d)", + if (errno == EINTR) { + goto retry; + } + + /* + * FIXME: This should be "alert" after router graceful shutdown + * implementation. + */ + nxt_unit_warn(ctx, "sendmsg(%d, %d) failed: %s (%d)", fd, (int) buf_size, strerror(errno), errno); } else { - nxt_unit_debug(ctx, "port_send(%d, %d): %d", fd, (int) buf_size, + nxt_unit_debug(ctx, "sendmsg(%d, %d): %d", fd, (int) buf_size, (int) res); } @@ -4629,14 +4630,20 @@ nxt_unit_port_recv(nxt_unit_ctx_t *ctx, int fd, void *buf, size_t buf_size, msg.msg_control = oob; msg.msg_controllen = oob_size; +retry: + res = recvmsg(fd, &msg, 0); if (nxt_slow_path(res == -1)) { - nxt_unit_warn(ctx, "port_recv(%d) failed: %s (%d)", - fd, strerror(errno), errno); + if (errno == EINTR) { + goto retry; + } + + nxt_unit_alert(ctx, "recvmsg(%d) failed: %s (%d)", + fd, strerror(errno), errno); } else { - nxt_unit_debug(ctx, "port_recv(%d): %d", fd, (int) res); + nxt_unit_debug(ctx, "recvmsg(%d): %d", fd, (int) res); } return res; diff --git a/test/test_node_websockets.py b/test/test_node_websockets.py index bb189552..cb6bf137 100644 --- a/test/test_node_websockets.py +++ b/test/test_node_websockets.py @@ -22,7 +22,7 @@ class TestNodeWebsockets(TestApplicationNode): ) self.skip_alerts.extend( - [r'last message send failed', r'socket close\(\d+\) failed'] + [r'socket close\(\d+\) failed'] ) def close_connection(self, sock): diff --git a/test/unit/main.py b/test/unit/main.py index 3d95a5b1..49c1eed3 100644 --- a/test/unit/main.py +++ b/test/unit/main.py @@ -193,7 +193,6 @@ class TestUnit(unittest.TestCase): self.skip_alerts = [ r'read signalfd\(4\) failed', - r'last message send failed', r'sendmsg.+failed', r'recvmsg.+failed', ] -- cgit From 68c6b67ffc840c78eddd27a65e9bf1370aaf5631 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Mon, 30 Mar 2020 19:37:58 +0300 Subject: Configuration: support for rational numbers. --- src/nxt_conf.c | 257 ++++++++++++++++++---------------------------- src/nxt_conf.h | 2 +- src/nxt_conf_validation.c | 14 +-- src/nxt_errno.h | 1 + src/nxt_http_route.c | 2 +- src/nxt_string.c | 18 ++++ src/nxt_string.h | 5 + src/test/nxt_clone_test.c | 6 +- 8 files changed, 135 insertions(+), 170 deletions(-) diff --git a/src/nxt_conf.c b/src/nxt_conf.c index 3e1130be..7f09dac9 100644 --- a/src/nxt_conf.c +++ b/src/nxt_conf.c @@ -7,13 +7,13 @@ #include #include -#if 0 -#include + #include -#endif +#include #define NXT_CONF_MAX_SHORT_STRING 14 +#define NXT_CONF_MAX_NUMBER_LEN 14 #define NXT_CONF_MAX_STRING NXT_INT32_T_MAX #define NXT_CONF_MAX_TOKEN_LEN 256 @@ -46,8 +46,7 @@ typedef struct nxt_conf_object_s nxt_conf_object_t; struct nxt_conf_value_s { union { uint8_t boolean; /* 1 bit. */ - int64_t integer; - double number; + u_char number[NXT_CONF_MAX_NUMBER_LEN + 1];; struct { u_char start[NXT_CONF_MAX_SHORT_STRING]; @@ -130,8 +129,6 @@ static nxt_int_t nxt_conf_copy_array(nxt_mp_t *mp, nxt_conf_op_t *op, static nxt_int_t nxt_conf_copy_object(nxt_mp_t *mp, nxt_conf_op_t *op, nxt_conf_value_t *dst, nxt_conf_value_t *src); -static size_t nxt_conf_json_integer_length(nxt_conf_value_t *value); -static u_char *nxt_conf_json_print_integer(u_char *p, nxt_conf_value_t *value); static size_t nxt_conf_json_string_length(nxt_conf_value_t *value); static u_char *nxt_conf_json_print_string(u_char *p, nxt_conf_value_t *value); static size_t nxt_conf_json_array_length(nxt_conf_value_t *value, @@ -221,10 +218,10 @@ nxt_conf_set_string_dup(nxt_conf_value_t *value, nxt_mp_t *mp, nxt_str_t *str) } -int64_t -nxt_conf_get_integer(nxt_conf_value_t *value) +double +nxt_conf_get_number(nxt_conf_value_t *value) { - return value->u.integer; + return nxt_strtod(value->u.number, NULL); } @@ -312,13 +309,19 @@ void nxt_conf_set_member_integer(nxt_conf_value_t *object, nxt_str_t *name, int64_t value, uint32_t index) { + u_char *p, *end; nxt_conf_object_member_t *member; member = &object->u.object->members[index]; nxt_conf_set_string(&member->name, name); - member->value.u.integer = value; + p = member->value.u.number; + end = p + NXT_CONF_MAX_NUMBER_LEN; + + end = nxt_sprintf(p, end, "%L", value); + *end = '\0'; + member->value.type = NXT_CONF_VALUE_INTEGER; } @@ -551,6 +554,7 @@ nxt_int_t nxt_conf_map_object(nxt_mp_t *mp, nxt_conf_value_t *value, nxt_conf_map_t *map, nxt_uint_t n, void *data) { + double num; nxt_str_t str, *s; nxt_uint_t i; nxt_conf_value_t *v; @@ -600,30 +604,32 @@ nxt_conf_map_object(nxt_mp_t *mp, nxt_conf_value_t *value, nxt_conf_map_t *map, break; } + num = nxt_strtod(v->u.number, NULL); + switch (map[i].type) { case NXT_CONF_MAP_INT32: - ptr->i32 = v->u.integer; + ptr->i32 = num; break; case NXT_CONF_MAP_INT64: - ptr->i64 = v->u.integer; + ptr->i64 = num; break; case NXT_CONF_MAP_INT: - ptr->i = v->u.integer; + ptr->i = num; break; case NXT_CONF_MAP_SIZE: - ptr->size = v->u.integer; + ptr->size = num; break; case NXT_CONF_MAP_OFF: - ptr->off = v->u.integer; + ptr->off = num; break; case NXT_CONF_MAP_MSEC: - ptr->msec = v->u.integer * 1000; + ptr->msec = (nxt_msec_t) num * 1000; break; default: @@ -635,11 +641,7 @@ nxt_conf_map_object(nxt_mp_t *mp, nxt_conf_value_t *value, nxt_conf_map_t *map, case NXT_CONF_MAP_DOUBLE: if (v->type == NXT_CONF_VALUE_NUMBER) { - ptr->dbl = v->u.number; - - } else if (v->type == NXT_CONF_VALUE_INTEGER) { - ptr->dbl = v->u.integer; - + ptr->dbl = nxt_strtod(v->u.number, NULL); } break; @@ -2036,56 +2038,51 @@ static u_char * nxt_conf_json_parse_number(nxt_mp_t *mp, nxt_conf_value_t *value, u_char *start, u_char *end, nxt_conf_json_error_t *error) { - u_char *p, ch; - uint64_t integer; - nxt_int_t sign; -#if 0 - uint64_t frac, power - nxt_int_t e, negative; -#endif - - static const uint64_t cutoff = NXT_INT64_T_MAX / 10; - static const uint64_t cutlim = NXT_INT64_T_MAX % 10; + u_char *p, *s, ch, c, *dot_pos; + size_t size; + double num; - ch = *start; + s = start; + ch = *s; if (ch == '-') { - sign = -1; - start++; - - } else { - sign = 1; + s++; } - integer = 0; + dot_pos = NULL; - for (p = start; nxt_fast_path(p != end); p++) { + for (p = s; nxt_fast_path(p != end); p++) { ch = *p; /* Values below '0' become >= 208. */ - ch = ch - '0'; + c = ch - '0'; + + if (c > 9) { + if (ch == '.' && nxt_fast_path(dot_pos == NULL)) { + dot_pos = p; + continue; + } - if (ch > 9) { break; } + } - if (nxt_slow_path(integer >= cutoff - && (integer > cutoff || ch > cutlim))) - { - nxt_conf_json_parse_error(error, start, - "The integer is too large. Such a large JSON integer value " - "is not supported." + if (dot_pos != NULL) { + if (nxt_slow_path(p - dot_pos <= 1)) { + nxt_conf_json_parse_error(error, s, + "The number is invalid. A fraction part in JSON numbers " + "must contain at least one digit." ); return NULL; } - integer = integer * 10 + ch; + } else { + dot_pos = p; } - if (nxt_slow_path(p - start > 1 && *start == '0')) { - - nxt_conf_json_parse_error(error, start, + if (nxt_slow_path(dot_pos - s > 1 && *start == '0')) { + nxt_conf_json_parse_error(error, s, "The number is invalid. Leading zeros are not allowed in JSON " "numbers." ); @@ -2093,101 +2090,77 @@ nxt_conf_json_parse_number(nxt_mp_t *mp, nxt_conf_value_t *value, u_char *start, return NULL; } - if (ch != '.') { - value->type = NXT_CONF_VALUE_INTEGER; - value->u.integer = sign * integer; - return p; - } + if (ch == 'e' || ch == 'E') { + p++; + s = p; -#if 0 - start = p + 1; + if (nxt_fast_path(s != end)) { + ch = *s; - frac = 0; - power = 1; + if (ch == '-' || ch == '+') { + s++; + } - for (p = start; nxt_fast_path(p != end); p++) { - ch = *p; + for (p = s; nxt_fast_path(p != end); p++) { + ch = *p; - /* Values below '0' become >= 208. */ - ch = ch - '0'; + /* Values below '0' become >= 208. */ + c = ch - '0'; - if (ch > 9) { - break; + if (c > 9) { + break; + } + } } - if (nxt_slow_path((frac >= cutoff && (frac > cutoff || ch > cutlim)) - || power > cutoff)) - { + if (nxt_slow_path(p == s)) { + nxt_conf_json_parse_error(error, start, + "The number is invalid. An exponent part in JSON numbers " + "must contain at least one digit." + ); + return NULL; } - - frac = frac * 10 + ch; - power *= 10; } - if (nxt_slow_path(p == start)) { - return NULL; - } - - value->type = NXT_CONF_VALUE_NUMBER; - value->u.number = integer + (double) frac / power; - - value->u.number = copysign(value->u.number, sign); - - if (ch == 'e' || ch == 'E') { - start = p + 1; + size = p - start; - ch = *start; - - if (ch == '-' || ch == '+') { - start++; - } - - negative = (ch == '-') ? 1 : 0; - e = 0; + if (size > NXT_CONF_MAX_NUMBER_LEN) { + nxt_conf_json_parse_error(error, start, + "The number is too long. Such a long JSON number value " + "is not supported." + ); - for (p = start; nxt_fast_path(p != end); p++) { - ch = *p; + return NULL; + } - /* Values below '0' become >= 208. */ - ch = ch - '0'; + nxt_memcpy(value->u.number, start, size); + value->u.number[size] = '\0'; - if (ch > 9) { - break; - } + nxt_errno = 0; + end = NULL; - e = e * 10 + ch; + num = nxt_strtod(value->u.number, &end); - if (nxt_slow_path(e > DBL_MAX_10_EXP)) { - return NULL; - } - } - - if (nxt_slow_path(p == start)) { - return NULL; - } - - if (negative) { - value->u.number /= exp10(e); + if (nxt_slow_path(nxt_errno == NXT_ERANGE || fabs(num) > NXT_INT64_T_MAX)) { + nxt_conf_json_parse_error(error, start, + "The number is out of representable range. Such JSON number " + "value is not supported." + ); - } else { - value->u.number *= exp10(e); - } + return NULL; } - if (nxt_fast_path(isfinite(value->u.number))) { - return p; + if (nxt_slow_path(end == NULL || *end != '\0')) { + nxt_thread_log_alert("strtod(\"%s\", %s) failed %E", value->u.number, + end == NULL ? (u_char *) "NULL" : end, nxt_errno); + return NULL; } -#else - nxt_conf_json_parse_error(error, start, - "The number is not an integer. JSON numbers with decimals and " - "exponents are not supported." - ); - -#endif + value->type = (num == trunc(num)) ? NXT_CONF_VALUE_INTEGER + : NXT_CONF_VALUE_NUMBER; - return NULL; + return p; } @@ -2216,11 +2189,8 @@ nxt_conf_json_length(nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty) return value->u.boolean ? nxt_length("true") : nxt_length("false"); case NXT_CONF_VALUE_INTEGER: - return nxt_conf_json_integer_length(value); - case NXT_CONF_VALUE_NUMBER: - /* TODO */ - return 0; + return nxt_strlen(value->u.number); case NXT_CONF_VALUE_SHORT_STRING: case NXT_CONF_VALUE_STRING: @@ -2253,11 +2223,8 @@ nxt_conf_json_print(u_char *p, nxt_conf_value_t *value, : nxt_cpymem(p, "false", 5); case NXT_CONF_VALUE_INTEGER: - return nxt_conf_json_print_integer(p, value); - case NXT_CONF_VALUE_NUMBER: - /* TODO */ - return p; + return nxt_cpystr(p, value->u.number); case NXT_CONF_VALUE_SHORT_STRING: case NXT_CONF_VALUE_STRING: @@ -2276,32 +2243,6 @@ nxt_conf_json_print(u_char *p, nxt_conf_value_t *value, } -static size_t -nxt_conf_json_integer_length(nxt_conf_value_t *value) -{ - int64_t num; - - num = llabs(value->u.integer); - - if (num <= 9999) { - return nxt_length("-9999"); - } - - if (num <= 99999999999LL) { - return nxt_length("-99999999999"); - } - - return NXT_INT64_T_LEN; -} - - -static u_char * -nxt_conf_json_print_integer(u_char *p, nxt_conf_value_t *value) -{ - return nxt_sprintf(p, p + NXT_INT64_T_LEN, "%L", value->u.integer); -} - - static size_t nxt_conf_json_string_length(nxt_conf_value_t *value) { diff --git a/src/nxt_conf.h b/src/nxt_conf.h index 66201fee..201a3a14 100644 --- a/src/nxt_conf.h +++ b/src/nxt_conf.h @@ -114,7 +114,7 @@ NXT_EXPORT void nxt_conf_get_string(nxt_conf_value_t *value, nxt_str_t *str); NXT_EXPORT void nxt_conf_set_string(nxt_conf_value_t *value, nxt_str_t *str); NXT_EXPORT nxt_int_t nxt_conf_set_string_dup(nxt_conf_value_t *value, nxt_mp_t *mp, nxt_str_t *str); -NXT_EXPORT int64_t nxt_conf_get_integer(nxt_conf_value_t *value); +NXT_EXPORT double nxt_conf_get_number(nxt_conf_value_t *value); NXT_EXPORT uint8_t nxt_conf_get_boolean(nxt_conf_value_t *value); // FIXME reimplement and reorder functions below diff --git a/src/nxt_conf_validation.c b/src/nxt_conf_validation.c index 3227a7e9..aa48845a 100644 --- a/src/nxt_conf_validation.c +++ b/src/nxt_conf_validation.c @@ -17,7 +17,7 @@ typedef enum { NXT_CONF_VLDT_NULL = 1 << NXT_CONF_NULL, NXT_CONF_VLDT_BOOLEAN = 1 << NXT_CONF_BOOLEAN, NXT_CONF_VLDT_INTEGER = 1 << NXT_CONF_INTEGER, - NXT_CONF_VLDT_NUMBER = 1 << NXT_CONF_NUMBER, + NXT_CONF_VLDT_NUMBER = (1 << NXT_CONF_NUMBER) | NXT_CONF_VLDT_INTEGER, NXT_CONF_VLDT_STRING = 1 << NXT_CONF_STRING, NXT_CONF_VLDT_ARRAY = 1 << NXT_CONF_ARRAY, NXT_CONF_VLDT_OBJECT = 1 << NXT_CONF_OBJECT, @@ -773,8 +773,8 @@ nxt_conf_vldt_type(nxt_conf_validation_t *vldt, nxt_str_t *name, static nxt_str_t type_name[] = { nxt_string("a null"), nxt_string("a boolean"), - nxt_string("an integer"), - nxt_string("a number"), + nxt_string("an integer number"), + nxt_string("a fractional number"), nxt_string("a string"), nxt_string("an array"), nxt_string("an object"), @@ -1138,7 +1138,7 @@ nxt_conf_vldt_return(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, { int64_t status; - status = nxt_conf_get_integer(value); + status = nxt_conf_get_number(value); if (status < NXT_HTTP_INVALID || status > NXT_HTTP_STATUS_MAX) { return nxt_conf_vldt_error(vldt, "The \"return\" value is out of " @@ -1626,8 +1626,8 @@ nxt_conf_vldt_processes(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, nxt_int_t ret; nxt_conf_vldt_processes_conf_t proc; - if (nxt_conf_type(value) == NXT_CONF_INTEGER) { - int_value = nxt_conf_get_integer(value); + if (nxt_conf_type(value) == NXT_CONF_NUMBER) { + int_value = nxt_conf_get_number(value); if (int_value < 1) { return nxt_conf_vldt_error(vldt, "The \"processes\" number must be " @@ -2062,7 +2062,7 @@ nxt_conf_vldt_server_weight(nxt_conf_validation_t *vldt, { int64_t int_value; - int_value = nxt_conf_get_integer(value); + int_value = nxt_conf_get_number(value); if (int_value <= 0) { return nxt_conf_vldt_error(vldt, "The \"weight\" number must be " diff --git a/src/nxt_errno.h b/src/nxt_errno.h index 1b29ef2f..40bcfa3f 100644 --- a/src/nxt_errno.h +++ b/src/nxt_errno.h @@ -47,6 +47,7 @@ typedef int nxt_err_t; #define NXT_ETIME ETIME #define NXT_ENOMOREFILES 0 #define NXT_ENOBUFS ENOBUFS +#define NXT_ERANGE ERANGE #if (NXT_HPUX) /* HP-UX uses EWOULDBLOCK instead of EAGAIN. */ diff --git a/src/nxt_http_route.c b/src/nxt_http_route.c index ee22f48d..ca43c060 100644 --- a/src/nxt_http_route.c +++ b/src/nxt_http_route.c @@ -630,7 +630,7 @@ nxt_http_route_action_create(nxt_router_temp_conf_t *tmcf, nxt_conf_value_t *cv, if (accf.ret != NULL) { action->handler = nxt_http_return_handler; - action->u.return_code = nxt_conf_get_integer(accf.ret); + action->u.return_code = nxt_conf_get_number(accf.ret); if (accf.location.length > 0) { if (nxt_is_complex_uri_encoded(accf.location.start, diff --git a/src/nxt_string.c b/src/nxt_string.c index 667146d6..54f96abc 100644 --- a/src/nxt_string.c +++ b/src/nxt_string.c @@ -109,6 +109,24 @@ nxt_memcpy_upcase(u_char *dst, const u_char *src, size_t length) } +u_char * +nxt_cpystr(u_char *dst, const u_char *src) +{ + for ( ;; ) { + *dst = *src; + + if (*dst == '\0') { + break; + } + + dst++; + src++; + } + + return dst; +} + + u_char * nxt_cpystrn(u_char *dst, const u_char *src, size_t length) { diff --git a/src/nxt_string.h b/src/nxt_string.h index d10658f7..7863c60e 100644 --- a/src/nxt_string.h +++ b/src/nxt_string.h @@ -20,6 +20,10 @@ nxt_upcase(c) \ nxt_isdigit(c) \ ((u_char) ((c) - '0') <= 9) +#define \ +nxt_strtod(s, endptr) \ + strtod((char *) s, (char **) endptr) + #define \ nxt_strlen(s) \ @@ -83,6 +87,7 @@ nxt_strncmp(s1, s2, length) \ strncmp((char *) s1, (char *) s2, length) +NXT_EXPORT u_char *nxt_cpystr(u_char *dst, const u_char *src); NXT_EXPORT u_char *nxt_cpystrn(u_char *dst, const u_char *src, size_t length); NXT_EXPORT nxt_int_t nxt_strcasecmp(const u_char *s1, const u_char *s2); NXT_EXPORT nxt_int_t nxt_strncasecmp(const u_char *s1, const u_char *s2, diff --git a/src/test/nxt_clone_test.c b/src/test/nxt_clone_test.c index 15d36557..64b9ddea 100644 --- a/src/test/nxt_clone_test.c +++ b/src/test/nxt_clone_test.c @@ -588,13 +588,13 @@ nxt_clone_test_parse_map(nxt_task_t *task, nxt_str_t *map_str, obj = nxt_conf_get_array_element(array, i); value = nxt_conf_get_object_member(obj, &host_name, NULL); - map->map[i].host = nxt_conf_get_integer(value); + map->map[i].host = nxt_conf_get_number(value); value = nxt_conf_get_object_member(obj, &cont_name, NULL); - map->map[i].container = nxt_conf_get_integer(value); + map->map[i].container = nxt_conf_get_number(value); value = nxt_conf_get_object_member(obj, &size_name, NULL); - map->map[i].size = nxt_conf_get_integer(value); + map->map[i].size = nxt_conf_get_number(value); } return NXT_OK; -- cgit From 01e957ef64b63403ac2e9107e2a84578d68a09b3 Mon Sep 17 00:00:00 2001 From: Igor Sysoev Date: Mon, 30 Mar 2020 19:47:01 +0300 Subject: Rational number support in upstream server weight. --- src/nxt_conf_validation.c | 14 ++++++------ src/nxt_upstream_round_robin.c | 48 +++++++++++++++++++++++++++--------------- 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/nxt_conf_validation.c b/src/nxt_conf_validation.c index aa48845a..bc03bdfb 100644 --- a/src/nxt_conf_validation.c +++ b/src/nxt_conf_validation.c @@ -732,7 +732,7 @@ static nxt_conf_vldt_object_t nxt_conf_vldt_upstream_members[] = { static nxt_conf_vldt_object_t nxt_conf_vldt_upstream_server_members[] = { { nxt_string("weight"), - NXT_CONF_VLDT_INTEGER, + NXT_CONF_VLDT_NUMBER, &nxt_conf_vldt_server_weight, NULL }, @@ -2060,18 +2060,18 @@ static nxt_int_t nxt_conf_vldt_server_weight(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, void *data) { - int64_t int_value; + double num_value; - int_value = nxt_conf_get_number(value); + num_value = nxt_conf_get_number(value); - if (int_value <= 0) { + if (num_value < 0) { return nxt_conf_vldt_error(vldt, "The \"weight\" number must be " - "greater than 0."); + "positive."); } - if (int_value > NXT_INT32_T_MAX) { + if (num_value > 1000000) { return nxt_conf_vldt_error(vldt, "The \"weight\" number must " - "not exceed %d.", NXT_INT32_T_MAX); + "not exceed 1,000,000"); } return NXT_OK; diff --git a/src/nxt_upstream_round_robin.c b/src/nxt_upstream_round_robin.c index fd76ecb5..31e2f48a 100644 --- a/src/nxt_upstream_round_robin.c +++ b/src/nxt_upstream_round_robin.c @@ -4,6 +4,7 @@ * Copyright (C) NGINX, Inc. */ +#include #include #include #include @@ -38,34 +39,47 @@ static const nxt_upstream_server_proto_t nxt_upstream_round_robin_proto = { }; -static nxt_conf_map_t nxt_upstream_round_robin_server_conf[] = { - { - nxt_string("weight"), - NXT_CONF_MAP_INT32, - offsetof(nxt_upstream_round_robin_server_t, weight), - }, -}; - - nxt_int_t nxt_upstream_round_robin_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, nxt_conf_value_t *upstream_conf, nxt_upstream_t *upstream) { + double total, k, w; size_t size; - uint32_t i, n, next; + uint32_t i, n, next, wt; nxt_mp_t *mp; nxt_str_t name; nxt_sockaddr_t *sa; - nxt_conf_value_t *servers_conf, *srvcf; + nxt_conf_value_t *servers_conf, *srvcf, *wtcf; nxt_upstream_round_robin_t *urr; static nxt_str_t servers = nxt_string("servers"); + static nxt_str_t weight = nxt_string("weight"); mp = tmcf->router_conf->mem_pool; servers_conf = nxt_conf_get_object_member(upstream_conf, &servers, NULL); n = nxt_conf_object_members_count(servers_conf); + total = 0.0; + next = 0; + + for (i = 0; i < n; i++) { + srvcf = nxt_conf_next_object_member(servers_conf, &name, &next); + wtcf = nxt_conf_get_object_member(srvcf, &weight, NULL); + w = (wtcf != NULL) ? nxt_conf_get_number(wtcf) : 1; + total += w; + } + + /* + * This prevents overflow of int32_t + * in nxt_upstream_round_robin_server_get(). + */ + k = (total == 0) ? 0 : (NXT_INT32_T_MAX / 2) / total; + + if (isinf(k)) { + k = 1; + } + size = sizeof(nxt_upstream_round_robin_t) + n * sizeof(nxt_upstream_round_robin_server_t); @@ -88,14 +102,14 @@ nxt_upstream_round_robin_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, sa->type = SOCK_STREAM; urr->server[i].sockaddr = sa; - urr->server[i].weight = 1; urr->server[i].protocol = NXT_HTTP_PROTO_H1; - nxt_conf_map_object(mp, srvcf, nxt_upstream_round_robin_server_conf, - nxt_nitems(nxt_upstream_round_robin_server_conf), - &urr->server[i]); + wtcf = nxt_conf_get_object_member(srvcf, &weight, NULL); + w = (wtcf != NULL) ? k * nxt_conf_get_number(wtcf) : k; + wt = (w > 1 || w == 0) ? round(w) : 1; - urr->server[i].effective_weight = urr->server[i].weight; + urr->server[i].weight = wt; + urr->server[i].effective_weight = wt; } upstream->proto = &nxt_upstream_round_robin_proto; @@ -177,7 +191,7 @@ nxt_upstream_round_robin_server_get(nxt_task_t *task, nxt_upstream_server_t *us) } } - if (best == NULL) { + if (best == NULL || total == 0) { us->state->error(task, us); return; } -- cgit From 5954839773a5c2ab5391ea2d99062de23581eee6 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Mon, 30 Mar 2020 18:44:50 +0100 Subject: Tests: added tests for rational numbers in upstream server weight. --- test/test_return.py | 2 +- test/test_upstreams_rr.py | 108 +++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 98 insertions(+), 12 deletions(-) diff --git a/test/test_return.py b/test/test_return.py index e3edb700..bd65b9bb 100644 --- a/test/test_return.py +++ b/test/test_return.py @@ -181,7 +181,7 @@ Connection: close check_error({"return": "200"}) check_error({"return": []}) - check_error({"return": 80.}) + check_error({"return": 80.1}) check_error({"return": 1000}) check_error({"return": 200, "share": "/blah"}) diff --git a/test/test_upstreams_rr.py b/test/test_upstreams_rr.py index 2bc2d90a..8dc83487 100644 --- a/test/test_upstreams_rr.py +++ b/test/test_upstreams_rr.py @@ -193,6 +193,67 @@ Connection: close self.assertEqual(resps[0], 60, 'weight 2 0') self.assertEqual(resps[2], 40, 'weight 2 1') + def test_upstreams_rr_weight_rational(self): + def set_weights(w1, w2): + self.assertIn( + 'success', + self.conf( + { + "127.0.0.1:7081": {"weight": w1}, + "127.0.0.1:7082": {"weight": w2}, + }, + 'upstreams/one/servers', + ), + 'configure weights', + ) + + def check_reqs(w1, w2, reqs=10): + resps = self.get_resps_sc(req=reqs) + self.assertEqual(resps[0], reqs * w1 / (w1 + w2), 'weight 1') + self.assertEqual(resps[1], reqs * w2 / (w1 + w2), 'weight 2') + + def check_weights(w1, w2): + set_weights(w1, w2) + check_reqs(w1, w2) + + check_weights(0, 1) + check_weights(0, 999999.0123456) + check_weights(1, 9) + check_weights(100000, 900000) + check_weights(1, .25) + check_weights(1, 0.25) + check_weights(0.2, .8) + check_weights(1, 1.5) + check_weights(1e-3, 1E-3) + check_weights(1e-20, 1e-20) + check_weights(1e4, 1e4) + check_weights(1000000, 1000000) + + set_weights(0.25, 0.25) + self.assertIn( + 'success', + self.conf_delete('upstreams/one/servers/127.0.0.1:7081/weight'), + 'delete weight', + ) + check_reqs(1, 0.25) + + self.assertIn( + 'success', + self.conf( + { + "127.0.0.1:7081": {"weight": 0.1}, + "127.0.0.1:7082": {"weight": 1}, + "127.0.0.1:7083": {"weight": 0.9}, + }, + 'upstreams/one/servers', + ), + 'configure weights', + ) + resps = self.get_resps_sc(req=20) + self.assertEqual(resps[0], 1, 'weight 3 1') + self.assertEqual(resps[1], 10, 'weight 3 2') + self.assertEqual(resps[2], 9, 'weight 3 3') + def test_upstreams_rr_independent(self): def sum_resps(*args): sum = [0] * len(args[0]) @@ -429,9 +490,29 @@ Connection: close self.conf({}, 'upstreams/one/servers'), 'configure servers empty', ) - self.assertEqual(self.get()['status'], 502, 'servers empty') + self.assertIn( + 'success', + self.conf( + {"127.0.0.1:7081": {"weight": 0}}, 'upstreams/one/servers' + ), + 'configure servers empty one', + ) + self.assertEqual(self.get()['status'], 502, 'servers empty one') + self.assertIn( + 'success', + self.conf( + { + "127.0.0.1:7081": {"weight": 0}, + "127.0.0.1:7082": {"weight": 0}, + }, + 'upstreams/one/servers', + ), + 'configure servers empty two', + ) + self.assertEqual(self.get()['status'], 502, 'servers empty two') + def test_upstreams_rr_invalid(self): self.assertIn( 'error', self.conf({}, 'upstreams'), 'upstreams empty', @@ -449,16 +530,21 @@ Connection: close self.conf({}, 'upstreams/one/servers/127.0.0.1:7081/blah'), 'invalid server option', ) - self.assertIn( - 'error', - self.conf({}, 'upstreams/one/servers/127.0.0.1:7081/weight'), - 'invalid weight option', - ) - self.assertIn( - 'error', - self.conf('-1', 'upstreams/one/servers/127.0.0.1:7081/weight'), - 'invalid negative weight', - ) + + def check_weight(w): + self.assertIn( + 'error', + self.conf(w, 'upstreams/one/servers/127.0.0.1:7081/weight'), + 'invalid weight option', + ) + check_weight({}) + check_weight('-1') + check_weight('1.') + check_weight('1.1.') + check_weight('.') + check_weight('.01234567890123') + check_weight('1000001') + check_weight('2e6') if __name__ == '__main__': -- cgit From be943c9fd48b3e8d7f3e5be5b2fd251f958c63f7 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Wed, 1 Apr 2020 18:33:48 +0300 Subject: Fixed build with Clang 10, broken by 32578e837322. This silences the -Wimplicit-int-float-conversion warning. --- src/nxt_conf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nxt_conf.c b/src/nxt_conf.c index 7f09dac9..1aca0a7e 100644 --- a/src/nxt_conf.c +++ b/src/nxt_conf.c @@ -2142,7 +2142,9 @@ nxt_conf_json_parse_number(nxt_mp_t *mp, nxt_conf_value_t *value, u_char *start, num = nxt_strtod(value->u.number, &end); - if (nxt_slow_path(nxt_errno == NXT_ERANGE || fabs(num) > NXT_INT64_T_MAX)) { + if (nxt_slow_path(nxt_errno == NXT_ERANGE + || fabs(num) > (double) NXT_INT64_T_MAX)) + { nxt_conf_json_parse_error(error, start, "The number is out of representable range. Such JSON number " "value is not supported." -- cgit From 2bb8b3d88a191d96c6693007ad79ae808f872941 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Fri, 3 Apr 2020 01:03:26 +0100 Subject: Tests: minor fixes. --- test/test_java_websockets.py | 2 +- test/test_return.py | 1 + test/unit/main.py | 26 ++++++++++++++++++-------- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/test/test_java_websockets.py b/test/test_java_websockets.py index d75ee3a6..e0dbf539 100644 --- a/test/test_java_websockets.py +++ b/test/test_java_websockets.py @@ -22,7 +22,7 @@ class TestJavaWebsockets(TestApplicationJava): ) self.skip_alerts.extend( - [r'last message send failed', r'socket close\(\d+\) failed'] + [r'socket close\(\d+\) failed'] ) def close_connection(self, sock): diff --git a/test/test_return.py b/test/test_return.py index bd65b9bb..fcb51745 100644 --- a/test/test_return.py +++ b/test/test_return.py @@ -183,6 +183,7 @@ Connection: close check_error({"return": []}) check_error({"return": 80.1}) check_error({"return": 1000}) + check_error({"return": -1}) check_error({"return": 200, "share": "/blah"}) self.assertIn( diff --git a/test/unit/main.py b/test/unit/main.py index 49c1eed3..060a03a5 100644 --- a/test/unit/main.py +++ b/test/unit/main.py @@ -199,7 +199,7 @@ class TestUnit(unittest.TestCase): self.skip_sanitizer = False def tearDown(self): - self.stop() + stop_errs = self.stop() # detect errors and failures for current test @@ -234,11 +234,19 @@ class TestUnit(unittest.TestCase): else: self._print_log() + self.assertListEqual(stop_errs, [None, None], 'stop errors') + def stop(self): - self._stop() - self.stop_processes() + errors = [] + + errors.append(self._stop()) + + errors.append(self.stop_processes()) + atexit.unregister(self.stop) + return errors + def _stop(self): if self._p.poll() is not None: return @@ -249,12 +257,10 @@ class TestUnit(unittest.TestCase): try: retcode = p.wait(15) if retcode: - self.fail( - "Child process terminated with code " + str(retcode) - ) + return 'Child process terminated with code ' + str(retcode) except: p.kill() - self.fail("Could not terminate unit") + return 'Could not terminate unit' def run_process(self, target, *args): if not hasattr(self, '_processes'): @@ -269,13 +275,17 @@ class TestUnit(unittest.TestCase): if not hasattr(self, '_processes'): return + fail = False for process in self._processes: if process.is_alive(): process.terminate() process.join(timeout=15) if process.is_alive(): - self.fail('Fail to stop process') + fail = True + + if fail: + return 'Fail to stop process' def waitforfiles(self, *files): for i in range(50): -- cgit From d7aa514d6a586115f0b05d5d6465787da1fa9b6c Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Fri, 3 Apr 2020 01:46:59 +0100 Subject: Tests: added notification on "read_timeout" expiration. --- test/unit/applications/websockets.py | 16 ++++++++++++++-- test/unit/http.py | 18 ++++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/test/unit/applications/websockets.py b/test/unit/applications/websockets.py index 309d37a3..fc15e8e4 100644 --- a/test/unit/applications/websockets.py +++ b/test/unit/applications/websockets.py @@ -52,7 +52,11 @@ class TestApplicationWebsocket(TestApplicationProto): ) resp = '' - while select.select([sock], [], [], 60)[0]: + while True: + rlist = select.select([sock], [], [], 60)[0] + if not rlist: + self.fail('Can\'t read response from server.') + resp += sock.recv(4096).decode() if ( @@ -73,7 +77,15 @@ class TestApplicationWebsocket(TestApplicationProto): def frame_read(self, sock, read_timeout=60): def recv_bytes(sock, bytes): data = b'' - while select.select([sock], [], [], read_timeout)[0]: + while True: + rlist = select.select([sock], [], [], read_timeout)[0] + if not rlist: + # For all current cases if the "read_timeout" was changed + # than test do not expect to get a response from server. + if read_timeout == 60: + self.fail('Can\'t read response from server.') + break + data += sock.recv(bytes - len(data)) if len(data) == bytes: diff --git a/test/unit/http.py b/test/unit/http.py index 8c71825a..8aacf18b 100644 --- a/test/unit/http.py +++ b/test/unit/http.py @@ -173,11 +173,25 @@ class TestHTTP(TestUnit): return self.http('PUT', **kwargs) def recvall(self, sock, **kwargs): - timeout = 60 if 'read_timeout' not in kwargs else kwargs['read_timeout'] + timeout_default = 60 + + timeout = ( + timeout_default + if 'read_timeout' not in kwargs + else kwargs['read_timeout'] + ) buff_size = 4096 if 'buff_size' not in kwargs else kwargs['buff_size'] data = b'' - while select.select([sock], [], [], timeout)[0]: + while True: + rlist = select.select([sock], [], [], timeout)[0] + if not rlist: + # For all current cases if the "read_timeout" was changed + # than test do not expect to get a response from server. + if timeout == timeout_default: + self.fail('Can\'t read response from server.') + break + try: part = sock.recv(buff_size) except: -- cgit From a49023229ec0a404665a711fbf35f6b3bf715825 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Fri, 3 Apr 2020 01:49:18 +0100 Subject: Tests: use "return" action in upstream tests. --- test/python/delayed/wsgi.py | 1 + test/python/upstreams/0/wsgi.py | 8 -- test/python/upstreams/1/wsgi.py | 8 -- test/python/upstreams/2/wsgi.py | 8 -- test/test_upstreams_rr.py | 174 +++++++++++++++++++++++----------------- 5 files changed, 101 insertions(+), 98 deletions(-) delete mode 100644 test/python/upstreams/0/wsgi.py delete mode 100644 test/python/upstreams/1/wsgi.py delete mode 100644 test/python/upstreams/2/wsgi.py diff --git a/test/python/delayed/wsgi.py b/test/python/delayed/wsgi.py index d25e2765..3eb5a6f8 100644 --- a/test/python/delayed/wsgi.py +++ b/test/python/delayed/wsgi.py @@ -11,6 +11,7 @@ def application(environ, start_response): write = start_response('200', [('Content-Length', str(len(body)))]) if not body: + time.sleep(delay) return [] step = int(len(body) / parts) diff --git a/test/python/upstreams/0/wsgi.py b/test/python/upstreams/0/wsgi.py deleted file mode 100644 index 2c88979b..00000000 --- a/test/python/upstreams/0/wsgi.py +++ /dev/null @@ -1,8 +0,0 @@ -import time - -def application(env, start_response): - delay = int(env.get('HTTP_X_DELAY', 0)) - - start_response('200', [('Content-Length', '0'), ('X-Upstream', '0')]) - time.sleep(delay) - return [] diff --git a/test/python/upstreams/1/wsgi.py b/test/python/upstreams/1/wsgi.py deleted file mode 100644 index 5077bdb1..00000000 --- a/test/python/upstreams/1/wsgi.py +++ /dev/null @@ -1,8 +0,0 @@ -import time - -def application(env, start_response): - delay = int(env.get('HTTP_X_DELAY', 0)) - - start_response('200', [('Content-Length', '0'), ('X-Upstream', '1')]) - time.sleep(delay) - return [] diff --git a/test/python/upstreams/2/wsgi.py b/test/python/upstreams/2/wsgi.py deleted file mode 100644 index bb0ce797..00000000 --- a/test/python/upstreams/2/wsgi.py +++ /dev/null @@ -1,8 +0,0 @@ -import time - -def application(env, start_response): - delay = int(env.get('HTTP_X_DELAY', 0)) - - start_response('200', [('Content-Length', '0'), ('X-Upstream', '2')]) - time.sleep(delay) - return [] diff --git a/test/test_upstreams_rr.py b/test/test_upstreams_rr.py index 8dc83487..7045318a 100644 --- a/test/test_upstreams_rr.py +++ b/test/test_upstreams_rr.py @@ -16,10 +16,10 @@ class TestUpstreamsRR(TestApplicationPython): { "listeners": { "*:7080": {"pass": "upstreams/one"}, - "*:7081": {"pass": "applications/ups_0"}, - "*:7082": {"pass": "applications/ups_1"}, - "*:7083": {"pass": "applications/ups_2"}, "*:7090": {"pass": "upstreams/two"}, + "*:7081": {"pass": "routes/one"}, + "*:7082": {"pass": "routes/two"}, + "*:7083": {"pass": "routes/three"}, }, "upstreams": { "one": { @@ -35,32 +35,12 @@ class TestUpstreamsRR(TestApplicationPython): }, }, }, - "applications": { - "ups_0": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + "/python/upstreams/0", - "working_directory": self.current_dir - + "/python/upstreams/0", - "module": "wsgi", - }, - "ups_1": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + "/python/upstreams/1", - "working_directory": self.current_dir - + "/python/upstreams/1", - "module": "wsgi", - }, - "ups_2": { - "type": "python", - "processes": {"spare": 0}, - "path": self.current_dir + "/python/upstreams/2", - "working_directory": self.current_dir - + "/python/upstreams/2", - "module": "wsgi", - }, + "routes": { + "one": [{"action": {"return": 200}}], + "two": [{"action": {"return": 201}}], + "three": [{"action": {"return": 202}}], }, + "applications": {}, }, ), 'upstreams initial configuration', @@ -70,15 +50,17 @@ class TestUpstreamsRR(TestApplicationPython): def get_resps(self, req=100, port=7080): resps = [0] + for _ in range(req): - headers = self.get(port=port)['headers'] - if 'X-Upstream' in headers: - ups = int(headers['X-Upstream']) + status = self.get(port=port)['status'] + if 200 > status or status > 209: + continue - if ups > len(resps) - 1: - resps.extend([0] * (ups - len(resps) + 1)) + ups = status % 10 + if ups > len(resps) - 1: + resps.extend([0] * (ups - len(resps) + 1)) - resps[ups] += 1 + resps[ups] += 1 return resps @@ -97,16 +79,19 @@ Connection: close """ resp = self.http(to_send, raw_resp=True, raw=True, port=port) - ups = re.findall('X-Upstream: (\d+)', resp) - resps = [0] * (int(max(ups)) + 1) + status = re.findall(r'HTTP\/\d\.\d\s(\d\d\d)', resp) + status = list(filter(lambda x: x[:2] == '20', status)) + ups = list(map(lambda x: int(x[-1]), status)) + resps = [0] * (max(ups) + 1) for i in range(len(ups)): - resps[int(ups[i])] += 1 + resps[ups[i]] += 1 return resps def test_upstreams_rr_no_weight(self): resps = self.get_resps() + self.assertEqual(sum(resps), 100, 'no weight sum') self.assertLessEqual( abs(resps[0] - resps[1]), self.cpu_count, 'no weight' ) @@ -127,6 +112,7 @@ Connection: close ) resps = self.get_resps() + self.assertEqual(sum(resps), 100, 'no weight 3 sum') self.assertLessEqual( abs(resps[0] - resps[1]), self.cpu_count, 'no weight 3' ) @@ -138,6 +124,7 @@ Connection: close ) resps = self.get_resps() + self.assertEqual(sum(resps), 100, 'no weight 4 sum') self.assertLessEqual( max(resps) - min(resps), self.cpu_count, 'no weight 4' ) @@ -295,33 +282,71 @@ Connection: close r_one = sum_resps(r_one, self.get_resps(req=10)) r_two = sum_resps(r_two, self.get_resps(req=10, port=7090)) + + self.assertEqual(sum(r_one), 100, 'dep one mix sum') self.assertLessEqual( abs(r_one[0] - r_one[1]), self.cpu_count, 'dep one mix' ) + self.assertEqual(sum(r_two), 100, 'dep two mix sum') self.assertLessEqual( abs(r_two[0] - r_two[1]), self.cpu_count, 'dep two mix' ) def test_upstreams_rr_delay(self): - headers_delay_1 = { - 'Connection': 'close', - 'Host': 'localhost', - 'Content-Length': '0', - 'X-Delay': '1', - } - headers_no_delay = { - 'Connection': 'close', - 'Host': 'localhost', - 'Content-Length': '0', - } + self.assertIn( + 'success', + self.conf( + { + "listeners": { + "*:7080": {"pass": "upstreams/one"}, + "*:7081": {"pass": "routes"}, + "*:7082": {"pass": "routes"}, + }, + "upstreams": { + "one": { + "servers": { + "127.0.0.1:7081": {}, + "127.0.0.1:7082": {}, + }, + }, + }, + "routes": [ + { + "match": {"destination": "*:7081"}, + "action": {"pass": "applications/delayed"}, + }, + { + "match": {"destination": "*:7082"}, + "action": {"return": 201}, + }, + ], + "applications": { + "delayed": { + "type": "python", + "processes": {"spare": 0}, + "path": self.current_dir + "/python/delayed", + "working_directory": self.current_dir + + "/python/delayed", + "module": "wsgi", + } + }, + }, + ), + 'upstreams initial configuration', + ) req = 50 socks = [] for i in range(req): - headers = headers_delay_1 if i % 5 == 0 else headers_no_delay + delay = 1 if i % 5 == 0 else 0 _, sock = self.get( - headers=headers, + headers={ + 'Host': 'localhost', + 'Content-Length': '0', + 'X-Delay': str(delay), + 'Connection': 'close', + }, start=True, no_recv=True, ) @@ -332,12 +357,12 @@ Connection: close resp = self.recvall(socks[i]).decode() socks[i].close() - m = re.search('X-Upstream: (\d+)', resp) + m = re.search('HTTP/1.1 20(\d)', resp) + self.assertIsNotNone(m, 'status') resps[int(m.group(1))] += 1 - self.assertLessEqual( - abs(resps[0] - resps[1]), self.cpu_count, 'dep two mix' - ) + self.assertEqual(sum(resps), req, 'delay sum') + self.assertLessEqual(abs(resps[0] - resps[1]), self.cpu_count, 'delay') def test_upstreams_rr_active_req(self): conns = 5 @@ -364,7 +389,7 @@ Connection: close # Send one more request and read response to make sure that previous # requests had enough time to reach server. - self.assertEqual(self.get()['status'], 200) + self.assertEqual(self.get()['body'], '') self.assertIn( 'success', @@ -388,13 +413,17 @@ Connection: close ) for i in range(conns): - resp = self.recvall(socks[i]).decode() - socks[i].close() - - self.assertRegex(resp, r'X-Upstream', 'active req GET') + self.assertEqual( + self.http(b'', sock=socks[i], raw=True)['body'], + '', + 'active req GET', + ) - resp = self.http(b"""0123456789""", sock=socks2[i], raw=True) - self.assertEqual(resp['status'], 200, 'active req POST') + self.assertEqual( + self.http(b"""0123456789""", sock=socks2[i], raw=True)['body'], + '', + 'active req POST', + ) def test_upstreams_rr_bad_server(self): self.assertIn( @@ -417,14 +446,11 @@ Connection: close def test_upstreams_rr_post(self): resps = [0, 0] for _ in range(50): - resps[ - int(self.post(body='0123456789')['headers']['X-Upstream']) - ] += 1 - resps[int(self.get()['headers']['X-Upstream'])] += 1 + resps[self.get()['status'] % 10] += 1 + resps[self.post(body='0123456789')['status'] % 10] += 1 - self.assertLessEqual( - abs(resps[0] - resps[1]), self.cpu_count, 'post' - ) + self.assertEqual(sum(resps), 100, 'post sum') + self.assertLessEqual(abs(resps[0] - resps[1]), self.cpu_count, 'post') def test_upstreams_rr_unix(self): addr_0 = self.testdir + '/sock_0' @@ -435,8 +461,8 @@ Connection: close self.conf( { "*:7080": {"pass": "upstreams/one"}, - "unix:" + addr_0: {"pass": "applications/ups_0"}, - "unix:" + addr_1: {"pass": "applications/ups_1"}, + "unix:" + addr_0: {"pass": "routes/one"}, + "unix:" + addr_1: {"pass": "routes/two"}, }, 'listeners', ), @@ -446,7 +472,7 @@ Connection: close self.assertIn( 'success', self.conf( - {"unix:" + addr_0: {}, "unix:" + addr_1: {},}, + {"unix:" + addr_0: {}, "unix:" + addr_1: {}}, 'upstreams/one/servers', ), 'configure servers unix', @@ -463,8 +489,8 @@ Connection: close self.conf( { "*:7080": {"pass": "upstreams/one"}, - "[::1]:7081": {"pass": "applications/ups_0"}, - "[::1]:7082": {"pass": "applications/ups_1"}, + "[::1]:7081": {"pass": "routes/one"}, + "[::1]:7082": {"pass": "routes/two"}, }, 'listeners', ), @@ -474,7 +500,7 @@ Connection: close self.assertIn( 'success', self.conf( - {"[::1]:7081": {}, "[::1]:7082": {},}, 'upstreams/one/servers' + {"[::1]:7081": {}, "[::1]:7082": {}}, 'upstreams/one/servers' ), 'configure servers ipv6', ) -- cgit From 792ef9d3c71c6843dbbde450a2d6d1ade538f1f3 Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Mon, 6 Apr 2020 16:52:11 +0300 Subject: Fixing 'find & add' racing condition in connected ports hash. Missing error log messages added. --- src/nxt_process.c | 20 ++++++-------------- src/nxt_process.h | 6 ++---- src/nxt_router.c | 32 ++++++++++++++++++-------------- src/nxt_unit.c | 3 +++ 4 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/nxt_process.c b/src/nxt_process.c index 035f747f..4179844b 100644 --- a/src/nxt_process.c +++ b/src/nxt_process.c @@ -590,17 +590,6 @@ nxt_process_close_ports(nxt_task_t *task, nxt_process_t *process) } -void -nxt_process_connected_port_add(nxt_process_t *process, nxt_port_t *port) -{ - nxt_thread_mutex_lock(&process->cp_mutex); - - nxt_port_hash_add(&process->connected_ports, port); - - nxt_thread_mutex_unlock(&process->cp_mutex); -} - - void nxt_process_connected_port_remove(nxt_process_t *process, nxt_port_t *port) { @@ -613,14 +602,17 @@ nxt_process_connected_port_remove(nxt_process_t *process, nxt_port_t *port) nxt_port_t * -nxt_process_connected_port_find(nxt_process_t *process, nxt_pid_t pid, - nxt_port_id_t port_id) +nxt_process_connected_port_find_add(nxt_process_t *process, nxt_port_t *port) { nxt_port_t *res; nxt_thread_mutex_lock(&process->cp_mutex); - res = nxt_port_hash_find(&process->connected_ports, pid, port_id); + res = nxt_port_hash_find(&process->connected_ports, port->pid, port->id); + + if (nxt_slow_path(res == NULL)) { + nxt_port_hash_add(&process->connected_ports, port); + } nxt_thread_mutex_unlock(&process->cp_mutex); diff --git a/src/nxt_process.h b/src/nxt_process.h index 343fffb8..0c51adfb 100644 --- a/src/nxt_process.h +++ b/src/nxt_process.h @@ -105,13 +105,11 @@ void nxt_process_use(nxt_task_t *task, nxt_process_t *process, int i); void nxt_process_close_ports(nxt_task_t *task, nxt_process_t *process); -void nxt_process_connected_port_add(nxt_process_t *process, nxt_port_t *port); - void nxt_process_connected_port_remove(nxt_process_t *process, nxt_port_t *port); -nxt_port_t *nxt_process_connected_port_find(nxt_process_t *process, - nxt_pid_t pid, nxt_port_id_t port_id); +nxt_port_t *nxt_process_connected_port_find_add(nxt_process_t *process, + nxt_port_t *port); void nxt_worker_process_quit_handler(nxt_task_t *task, nxt_port_recv_msg_t *msg); diff --git a/src/nxt_router.c b/src/nxt_router.c index d4f25d7e..a70b03d1 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -735,12 +735,15 @@ nxt_request_app_link_use(nxt_task_t *task, nxt_request_app_link_t *req_app_link, nxt_inline void -nxt_request_app_link_error(nxt_request_app_link_t *req_app_link, int code, - const char *str) +nxt_request_app_link_error(nxt_task_t *task, nxt_app_t *app, + nxt_request_app_link_t *req_app_link, const char *str) { req_app_link->app_port = NULL; - req_app_link->err_code = code; + req_app_link->err_code = 500; req_app_link->err_str = str; + + nxt_alert(task, "app \"%V\" internal error: %s on #%uD", + &app->name, str, req_app_link->stream); } @@ -3909,7 +3912,7 @@ nxt_router_app_port_error(nxt_task_t *task, nxt_port_recv_msg_t *msg, nxt_debug(task, "app '%V' %p abort next stream #%uD", &app->name, app, req_app_link->stream); - nxt_request_app_link_error(req_app_link, 500, + nxt_request_app_link_error(task, app, req_app_link, "Failed to start application process"); nxt_request_app_link_use(task, req_app_link, -1); } @@ -4665,7 +4668,7 @@ nxt_router_port_post_select(nxt_task_t *task, nxt_port_select_state_t *state) nxt_port_use(task, state->port, -1); } - nxt_request_app_link_error(state->req_app_link, 500, + nxt_request_app_link_error(task, app, state->req_app_link, "Failed to allocate shared req<->app link"); return NXT_ERROR; @@ -4693,7 +4696,7 @@ nxt_router_port_post_select(nxt_task_t *task, nxt_port_select_state_t *state) res = nxt_router_start_app_process(task, app); if (nxt_slow_path(res != NXT_OK)) { - nxt_request_app_link_error(req_app_link, 500, + nxt_request_app_link_error(task, app, req_app_link, "Failed to start app process"); return NXT_ERROR; @@ -4808,25 +4811,26 @@ nxt_router_app_prepare_request(nxt_task_t *task, apr_action = NXT_APR_REQUEST_FAILED; - c_port = nxt_process_connected_port_find(port->process, reply_port->pid, - reply_port->id); + c_port = nxt_process_connected_port_find_add(port->process, reply_port); + if (nxt_slow_path(c_port != reply_port)) { res = nxt_port_send_port(task, port, reply_port, 0); if (nxt_slow_path(res != NXT_OK)) { - nxt_request_app_link_error(req_app_link, 500, + nxt_request_app_link_error(task, port->app, req_app_link, "Failed to send reply port to application"); + + nxt_process_connected_port_remove(port->process, reply_port); + goto release_port; } - - nxt_process_connected_port_add(port->process, reply_port); } buf = nxt_router_prepare_msg(task, req_app_link->request, port, nxt_app_msg_prefix[port->app->type]); if (nxt_slow_path(buf == NULL)) { - nxt_request_app_link_error(req_app_link, 500, + nxt_request_app_link_error(task, port->app, req_app_link, "Failed to prepare message for application"); goto release_port; } @@ -4850,7 +4854,7 @@ nxt_router_app_prepare_request(nxt_task_t *task, &req_app_link->msg_info.tracking, req_app_link->stream); if (nxt_slow_path(res != NXT_OK)) { - nxt_request_app_link_error(req_app_link, 500, + nxt_request_app_link_error(task, port->app, req_app_link, "Failed to get tracking area"); goto release_port; } @@ -4868,7 +4872,7 @@ nxt_router_app_prepare_request(nxt_task_t *task, &req_app_link->msg_info.tracking); if (nxt_slow_path(res != NXT_OK)) { - nxt_request_app_link_error(req_app_link, 500, + nxt_request_app_link_error(task, port->app, req_app_link, "Failed to send message to application"); goto release_port; } diff --git a/src/nxt_unit.c b/src/nxt_unit.c index 160b849a..c2e7f198 100644 --- a/src/nxt_unit.c +++ b/src/nxt_unit.c @@ -4312,6 +4312,9 @@ nxt_unit_add_port(nxt_unit_ctx_t *ctx, nxt_unit_port_t *port) rc = nxt_unit_port_hash_add(&lib->ports, &new_port->port); if (nxt_slow_path(rc != NXT_UNIT_OK)) { + nxt_unit_alert(ctx, "add_port: %d,%d hash_add failed", + port->id.pid, port->id.id); + goto unlock; } -- cgit From ce53d6bdb1a61de0f81dad39a978dec92e286071 Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Wed, 8 Apr 2020 14:44:53 +0300 Subject: Node.js: fixing Server.listen() method. This is required for Express framework compatibility. This closes #418 issue on GitHub. --- src/nodejs/unit-http/http_server.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/nodejs/unit-http/http_server.js b/src/nodejs/unit-http/http_server.js index 2f324329..d378e410 100644 --- a/src/nodejs/unit-http/http_server.js +++ b/src/nodejs/unit-http/http_server.js @@ -451,8 +451,18 @@ Server.prototype.setTimeout = function setTimeout(msecs, callback) { return this; }; -Server.prototype.listen = function () { +Server.prototype.listen = function (...args) { this.unit.listen(); + + const cb = args.pop(); + + if (typeof cb === 'function') { + this.once('listening', cb); + } + + this.emit('listening'); + + return this; }; Server.prototype.emit_request = function (req, res) { -- cgit From 27c1e268563da002e57f34032499efd7543b8b9d Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Wed, 8 Apr 2020 15:15:24 +0300 Subject: Controller: eliminated extra control socket's sockaddr copying. --- src/nxt_controller.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/nxt_controller.c b/src/nxt_controller.c index cc1ed534..ad292421 100644 --- a/src/nxt_controller.c +++ b/src/nxt_controller.c @@ -402,24 +402,14 @@ nxt_controller_conf_send(nxt_task_t *task, nxt_conf_value_t *conf, nxt_int_t nxt_runtime_controller_socket(nxt_task_t *task, nxt_runtime_t *rt) { - nxt_sockaddr_t *sa; nxt_listen_socket_t *ls; - sa = rt->controller_listen; - ls = nxt_mp_alloc(rt->mem_pool, sizeof(nxt_listen_socket_t)); if (ls == NULL) { return NXT_ERROR; } - ls->sockaddr = nxt_sockaddr_create(rt->mem_pool, &sa->u.sockaddr, - sa->socklen, sa->length); - if (ls->sockaddr == NULL) { - return NXT_ERROR; - } - - ls->sockaddr->type = sa->type; - nxt_sockaddr_text(ls->sockaddr); + ls->sockaddr = rt->controller_listen; nxt_listen_socket_remote_size(ls); -- cgit From 555d595f38801685f95f140f85b20f5dcfaa49cd Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Wed, 8 Apr 2020 15:15:24 +0300 Subject: Removed unused code related to testing of address binding. --- src/nxt_conn_connect.c | 2 +- src/nxt_controller.c | 2 +- src/nxt_listen_socket.c | 17 ++--------------- src/nxt_listen_socket.h | 2 +- src/nxt_runtime.c | 2 +- src/nxt_socket.c | 13 ++----------- src/nxt_socket.h | 2 +- 7 files changed, 9 insertions(+), 31 deletions(-) diff --git a/src/nxt_conn_connect.c b/src/nxt_conn_connect.c index d045853f..220fb5f9 100644 --- a/src/nxt_conn_connect.c +++ b/src/nxt_conn_connect.c @@ -108,7 +108,7 @@ nxt_conn_socket(nxt_task_t *task, nxt_conn_t *c) c->write_timer.task = task; if (c->local != NULL) { - if (nxt_slow_path(nxt_socket_bind(task, s, c->local, 0) != NXT_OK)) { + if (nxt_slow_path(nxt_socket_bind(task, s, c->local) != NXT_OK)) { nxt_socket_close(task, s); return NXT_ERROR; } diff --git a/src/nxt_controller.c b/src/nxt_controller.c index ad292421..26f1d53a 100644 --- a/src/nxt_controller.c +++ b/src/nxt_controller.c @@ -431,7 +431,7 @@ nxt_runtime_controller_socket(nxt_task_t *task, nxt_runtime_t *rt) #endif ls->handler = nxt_controller_conn_init; - if (nxt_listen_socket_create(task, ls, 0) != NXT_OK) { + if (nxt_listen_socket_create(task, ls) != NXT_OK) { return NXT_ERROR; } diff --git a/src/nxt_listen_socket.c b/src/nxt_listen_socket.c index 9eeca690..63ab3de3 100644 --- a/src/nxt_listen_socket.c +++ b/src/nxt_listen_socket.c @@ -27,8 +27,7 @@ nxt_listen_socket(nxt_task_t *task, nxt_socket_t s, int backlog) nxt_int_t -nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls, - nxt_bool_t bind_test) +nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls) { nxt_log_t log, *old; nxt_uint_t family; @@ -81,16 +80,8 @@ nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls, nxt_socket_defer_accept(task, s, sa); } - switch (nxt_socket_bind(task, s, sa, bind_test)) { - - case NXT_OK: - break; - - case NXT_ERROR: + if (nxt_socket_bind(task, s, sa) != NXT_OK) { goto fail; - - default: /* NXT_DECLINED: EADDRINUSE on bind() test */ - return NXT_OK; } #if (NXT_HAVE_UNIX_DOMAIN) @@ -106,10 +97,6 @@ nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls, if (nxt_file_set_access(name, access) != NXT_OK) { goto fail; } - - if (bind_test && nxt_file_delete(name) != NXT_OK) { - goto fail; - } } #endif diff --git a/src/nxt_listen_socket.h b/src/nxt_listen_socket.h index 80b95425..fac640de 100644 --- a/src/nxt_listen_socket.h +++ b/src/nxt_listen_socket.h @@ -55,7 +55,7 @@ NXT_EXPORT nxt_int_t nxt_listen_socket(nxt_task_t *task, nxt_socket_t s, int backlog); NXT_EXPORT nxt_int_t nxt_listen_socket_create(nxt_task_t *task, - nxt_listen_socket_t *ls, nxt_bool_t bind_test); + nxt_listen_socket_t *ls); NXT_EXPORT nxt_int_t nxt_listen_socket_update(nxt_task_t *task, nxt_listen_socket_t *ls, nxt_listen_socket_t *prev); NXT_EXPORT void nxt_listen_socket_remote_size(nxt_listen_socket_t *ls); diff --git a/src/nxt_runtime.c b/src/nxt_runtime.c index f6d80ccb..09fad1de 100644 --- a/src/nxt_runtime.c +++ b/src/nxt_runtime.c @@ -1205,7 +1205,7 @@ nxt_runtime_listen_sockets_create(nxt_task_t *task, nxt_runtime_t *rt) } } - if (nxt_listen_socket_create(task, &curr[c], 0) != NXT_OK) { + if (nxt_listen_socket_create(task, &curr[c]) != NXT_OK) { return NXT_ERROR; } diff --git a/src/nxt_socket.c b/src/nxt_socket.c index 2a809184..cc3d7378 100644 --- a/src/nxt_socket.c +++ b/src/nxt_socket.c @@ -184,11 +184,8 @@ nxt_socket_sockopt_name(nxt_uint_t level, nxt_uint_t sockopt) nxt_int_t -nxt_socket_bind(nxt_task_t *task, nxt_socket_t s, nxt_sockaddr_t *sa, - nxt_bool_t test) +nxt_socket_bind(nxt_task_t *task, nxt_socket_t s, nxt_sockaddr_t *sa) { - nxt_err_t err; - nxt_debug(task, "bind(%d, %*s)", s, (size_t) sa->length, nxt_sockaddr_start(sa)); @@ -196,14 +193,8 @@ nxt_socket_bind(nxt_task_t *task, nxt_socket_t s, nxt_sockaddr_t *sa, return NXT_OK; } - err = nxt_socket_errno; - - if (err == NXT_EADDRINUSE && test) { - return NXT_DECLINED; - } - nxt_alert(task, "bind(%d, %*s) failed %E", - s, (size_t) sa->length, nxt_sockaddr_start(sa), err); + s, (size_t) sa->length, nxt_sockaddr_start(sa), nxt_socket_errno); return NXT_ERROR; } diff --git a/src/nxt_socket.h b/src/nxt_socket.h index 6a450f83..e39d8e4d 100644 --- a/src/nxt_socket.h +++ b/src/nxt_socket.h @@ -101,7 +101,7 @@ NXT_EXPORT nxt_int_t nxt_socket_getsockopt(nxt_task_t *task, nxt_socket_t s, NXT_EXPORT nxt_int_t nxt_socket_setsockopt(nxt_task_t *task, nxt_socket_t s, nxt_uint_t level, nxt_uint_t sockopt, int val); NXT_EXPORT nxt_int_t nxt_socket_bind(nxt_task_t *task, nxt_socket_t s, - nxt_sockaddr_t *sa, nxt_bool_t test); + nxt_sockaddr_t *sa); NXT_EXPORT nxt_int_t nxt_socket_connect(nxt_task_t *task, nxt_socket_t s, nxt_sockaddr_t *sa); NXT_EXPORT void nxt_socket_shutdown(nxt_task_t *task, nxt_socket_t s, -- cgit From a6d9efcee1546f67a1a2b926744f7052f3536b03 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Wed, 8 Apr 2020 15:15:24 +0300 Subject: Controller: fixed cleaning up of control socket file in some cases. Previously, the unix domain control socket file might have been left in the file system after a failed nxt_listen_socket_create() call. --- src/nxt_listen_socket.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/nxt_listen_socket.c b/src/nxt_listen_socket.c index 63ab3de3..f433cf2b 100644 --- a/src/nxt_listen_socket.c +++ b/src/nxt_listen_socket.c @@ -48,7 +48,7 @@ nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls) s = nxt_socket_create(task, family, sa->type, 0, ls->flags); if (s == -1) { - goto socket_fail; + goto fail; } if (nxt_socket_setsockopt(task, s, SOL_SOCKET, SO_REUSEADDR, 1) != NXT_OK) { @@ -95,7 +95,7 @@ nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls) access = (S_IRUSR | S_IWUSR); if (nxt_file_set_access(name, access) != NXT_OK) { - goto fail; + goto listen_fail; } } @@ -106,7 +106,7 @@ nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls) if (listen(s, ls->backlog) != 0) { nxt_alert(task, "listen(%d, %d) failed %E", s, ls->backlog, nxt_socket_errno); - goto fail; + goto listen_fail; } ls->socket = s; @@ -114,11 +114,25 @@ nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls) return NXT_OK; -fail: +listen_fail: + +#if (NXT_HAVE_UNIX_DOMAIN) + + if (family == AF_UNIX) { + nxt_file_name_t *name; - nxt_socket_close(task, s); + name = (nxt_file_name_t *) sa->u.sockaddr_un.sun_path; -socket_fail: + (void) nxt_file_delete(name); + } + +#endif + +fail: + + if (s != -1) { + nxt_socket_close(task, s); + } thr->log = old; -- cgit From c7f5c1c6641838006088524c2122eae8f9c30431 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Wed, 8 Apr 2020 15:15:24 +0300 Subject: Controller: improved handling of unix domain control socket. One of the ways to detect Unit's startup and subsequent readiness to accept commands relies on waiting for the control socket file to be created. Earlier, it was unreliable due to a race condition between the client's connect() and the daemon's listen() calls after the socket's bind() call. Now, unix domain listening sockets are created with a nxt_listen_socket_create() call as follows: s = socket(); unlink("path/to/socket.tmp") bind(s, "path/to/socket.tmp"); listen(s); rename("path/to/socket.tmp", "path/to/socket"); This eliminates a time-lapse when the socket file is already created but nobody is listening on it yet, which therefore prevents the condition described above. Also, it allows reliably detecting whether the socket is being used or simply wasn't cleaned after the daemon stopped abruptly. A successful connection to the socket file means the daemon has been started; otherwise, the file can be overwritten. --- src/nxt_controller.c | 2 +- src/nxt_listen_socket.c | 97 +++++++++++++++++++++++++++++++++++++++++++------ src/nxt_listen_socket.h | 2 +- src/nxt_runtime.c | 2 +- test/unit/main.py | 5 +-- 5 files changed, 90 insertions(+), 18 deletions(-) diff --git a/src/nxt_controller.c b/src/nxt_controller.c index 26f1d53a..f4c3a00d 100644 --- a/src/nxt_controller.c +++ b/src/nxt_controller.c @@ -431,7 +431,7 @@ nxt_runtime_controller_socket(nxt_task_t *task, nxt_runtime_t *rt) #endif ls->handler = nxt_controller_conn_init; - if (nxt_listen_socket_create(task, ls) != NXT_OK) { + if (nxt_listen_socket_create(task, rt->mem_pool, ls) != NXT_OK) { return NXT_ERROR; } diff --git a/src/nxt_listen_socket.c b/src/nxt_listen_socket.c index f433cf2b..f10abdef 100644 --- a/src/nxt_listen_socket.c +++ b/src/nxt_listen_socket.c @@ -27,13 +27,23 @@ nxt_listen_socket(nxt_task_t *task, nxt_socket_t s, int backlog) nxt_int_t -nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls) +nxt_listen_socket_create(nxt_task_t *task, nxt_mp_t *mp, + nxt_listen_socket_t *ls) { - nxt_log_t log, *old; - nxt_uint_t family; - nxt_socket_t s; - nxt_thread_t *thr; - nxt_sockaddr_t *sa; + nxt_log_t log, *old; + nxt_uint_t family; + nxt_socket_t s; + nxt_thread_t *thr; + nxt_sockaddr_t *sa; +#if (NXT_HAVE_UNIX_DOMAIN) + int ret; + u_char *p; + nxt_err_t err; + nxt_socket_t ts; + nxt_sockaddr_t *orig_sa; + nxt_file_name_t *name, *tmp; + nxt_file_access_t access; +#endif sa = ls->sockaddr; @@ -80,6 +90,36 @@ nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls) nxt_socket_defer_accept(task, s, sa); } +#if (NXT_HAVE_UNIX_DOMAIN) + + if (family == AF_UNIX + && sa->type == SOCK_STREAM + && sa->u.sockaddr_un.sun_path[0] != '\0') + { + orig_sa = sa; + + sa = nxt_sockaddr_alloc(mp, sa->socklen + 4, sa->length + 4); + if (sa == NULL) { + goto fail; + } + + sa->type = SOCK_STREAM; + sa->u.sockaddr_un.sun_family = AF_UNIX; + + p = nxt_cpystr((u_char *) sa->u.sockaddr_un.sun_path, + (u_char *) orig_sa->u.sockaddr_un.sun_path); + nxt_memcpy(p, ".tmp", 4); + + nxt_sockaddr_text(sa); + + (void) unlink(sa->u.sockaddr_un.sun_path); + + } else { + orig_sa = NULL; + } + +#endif + if (nxt_socket_bind(task, s, sa) != NXT_OK) { goto fail; } @@ -87,9 +127,6 @@ nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls) #if (NXT_HAVE_UNIX_DOMAIN) if (family == AF_UNIX) { - nxt_file_name_t *name; - nxt_file_access_t access; - name = (nxt_file_name_t *) sa->u.sockaddr_un.sun_path; access = (S_IRUSR | S_IWUSR); @@ -109,6 +146,46 @@ nxt_listen_socket_create(nxt_task_t *task, nxt_listen_socket_t *ls) goto listen_fail; } +#if (NXT_HAVE_UNIX_DOMAIN) + + if (orig_sa != NULL) { + ts = nxt_socket_create(task, AF_UNIX, SOCK_STREAM, 0, 0); + if (ts == -1) { + goto listen_fail; + } + + ret = connect(ts, &orig_sa->u.sockaddr, orig_sa->socklen); + + err = nxt_socket_errno; + + nxt_socket_close(task, ts); + + if (ret == 0) { + nxt_alert(task, "connect(%d, %*s) succeed, address already in use", + ts, (size_t) orig_sa->length, + nxt_sockaddr_start(orig_sa)); + + goto listen_fail; + } + + if (err != NXT_ENOENT && err != NXT_ECONNREFUSED) { + nxt_alert(task, "connect(%d, %*s) failed %E", + ts, (size_t) orig_sa->length, + nxt_sockaddr_start(orig_sa), err); + + goto listen_fail; + } + + tmp = (nxt_file_name_t *) sa->u.sockaddr_un.sun_path; + name = (nxt_file_name_t *) orig_sa->u.sockaddr_un.sun_path; + + if (nxt_file_rename(tmp, name) != NXT_OK) { + goto listen_fail; + } + } + +#endif + ls->socket = s; thr->log = old; @@ -119,8 +196,6 @@ listen_fail: #if (NXT_HAVE_UNIX_DOMAIN) if (family == AF_UNIX) { - nxt_file_name_t *name; - name = (nxt_file_name_t *) sa->u.sockaddr_un.sun_path; (void) nxt_file_delete(name); diff --git a/src/nxt_listen_socket.h b/src/nxt_listen_socket.h index fac640de..e2435b76 100644 --- a/src/nxt_listen_socket.h +++ b/src/nxt_listen_socket.h @@ -54,7 +54,7 @@ typedef struct { NXT_EXPORT nxt_int_t nxt_listen_socket(nxt_task_t *task, nxt_socket_t s, int backlog); -NXT_EXPORT nxt_int_t nxt_listen_socket_create(nxt_task_t *task, +NXT_EXPORT nxt_int_t nxt_listen_socket_create(nxt_task_t *task, nxt_mp_t *mp, nxt_listen_socket_t *ls); NXT_EXPORT nxt_int_t nxt_listen_socket_update(nxt_task_t *task, nxt_listen_socket_t *ls, nxt_listen_socket_t *prev); diff --git a/src/nxt_runtime.c b/src/nxt_runtime.c index 09fad1de..bcd156ee 100644 --- a/src/nxt_runtime.c +++ b/src/nxt_runtime.c @@ -1205,7 +1205,7 @@ nxt_runtime_listen_sockets_create(nxt_task_t *task, nxt_runtime_t *rt) } } - if (nxt_listen_socket_create(task, &curr[c]) != NXT_OK) { + if (nxt_listen_socket_create(task, rt->mem_pool, &curr[c]) != NXT_OK) { return NXT_ERROR; } diff --git a/test/unit/main.py b/test/unit/main.py index 060a03a5..4507f71a 100644 --- a/test/unit/main.py +++ b/test/unit/main.py @@ -185,10 +185,7 @@ class TestUnit(unittest.TestCase): atexit.register(self.stop) - # Due to race between connect() and listen() after the socket binding - # tests waits for unit.pid file which is created after listen(). - - if not self.waitforfiles(self.testdir + '/unit.pid'): + if not self.waitforfiles(self.testdir + '/control.unit.sock'): exit("Could not start unit") self.skip_alerts = [ -- cgit From 58cc13ab291cac5b13462006e3feb780178ef5f3 Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Fri, 10 Apr 2020 16:21:58 +0300 Subject: Resolving a racing condition while adding ports on the app's side. An earlier attempt (ad6265786871) to resolve this condition on the router's side added a new issue: the app could get a request before acquiring a port. --- go/port.go | 6 +++++- src/nxt_process.c | 17 ++++++++++++----- src/nxt_process.h | 4 +++- src/nxt_router.c | 6 +++--- src/nxt_unit.c | 28 +++++++++++++++++++++++++--- 5 files changed, 48 insertions(+), 13 deletions(-) diff --git a/go/port.go b/go/port.go index 72d33d31..59a13f8b 100644 --- a/go/port.go +++ b/go/port.go @@ -50,7 +50,11 @@ func add_port(p *port) { port_registry_.m = make(map[port_key]*port) } - port_registry_.m[p.key] = p + old := port_registry_.m[p.key] + + if old == nil { + port_registry_.m[p.key] = p + } port_registry_.Unlock() } diff --git a/src/nxt_process.c b/src/nxt_process.c index 4179844b..f5959edf 100644 --- a/src/nxt_process.c +++ b/src/nxt_process.c @@ -590,6 +590,17 @@ nxt_process_close_ports(nxt_task_t *task, nxt_process_t *process) } +void +nxt_process_connected_port_add(nxt_process_t *process, nxt_port_t *port) +{ + nxt_thread_mutex_lock(&process->cp_mutex); + + nxt_port_hash_add(&process->connected_ports, port); + + nxt_thread_mutex_unlock(&process->cp_mutex); +} + + void nxt_process_connected_port_remove(nxt_process_t *process, nxt_port_t *port) { @@ -602,7 +613,7 @@ nxt_process_connected_port_remove(nxt_process_t *process, nxt_port_t *port) nxt_port_t * -nxt_process_connected_port_find_add(nxt_process_t *process, nxt_port_t *port) +nxt_process_connected_port_find(nxt_process_t *process, nxt_port_t *port) { nxt_port_t *res; @@ -610,10 +621,6 @@ nxt_process_connected_port_find_add(nxt_process_t *process, nxt_port_t *port) res = nxt_port_hash_find(&process->connected_ports, port->pid, port->id); - if (nxt_slow_path(res == NULL)) { - nxt_port_hash_add(&process->connected_ports, port); - } - nxt_thread_mutex_unlock(&process->cp_mutex); return res; diff --git a/src/nxt_process.h b/src/nxt_process.h index 0c51adfb..3f7155c8 100644 --- a/src/nxt_process.h +++ b/src/nxt_process.h @@ -105,10 +105,12 @@ void nxt_process_use(nxt_task_t *task, nxt_process_t *process, int i); void nxt_process_close_ports(nxt_task_t *task, nxt_process_t *process); +void nxt_process_connected_port_add(nxt_process_t *process, nxt_port_t *port); + void nxt_process_connected_port_remove(nxt_process_t *process, nxt_port_t *port); -nxt_port_t *nxt_process_connected_port_find_add(nxt_process_t *process, +nxt_port_t *nxt_process_connected_port_find(nxt_process_t *process, nxt_port_t *port); void nxt_worker_process_quit_handler(nxt_task_t *task, diff --git a/src/nxt_router.c b/src/nxt_router.c index a70b03d1..2f4ea698 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -4811,7 +4811,7 @@ nxt_router_app_prepare_request(nxt_task_t *task, apr_action = NXT_APR_REQUEST_FAILED; - c_port = nxt_process_connected_port_find_add(port->process, reply_port); + c_port = nxt_process_connected_port_find(port->process, reply_port); if (nxt_slow_path(c_port != reply_port)) { res = nxt_port_send_port(task, port, reply_port, 0); @@ -4820,10 +4820,10 @@ nxt_router_app_prepare_request(nxt_task_t *task, nxt_request_app_link_error(task, port->app, req_app_link, "Failed to send reply port to application"); - nxt_process_connected_port_remove(port->process, reply_port); - goto release_port; } + + nxt_process_connected_port_add(port->process, reply_port); } buf = nxt_router_prepare_msg(task, req_app_link->request, port, diff --git a/src/nxt_unit.c b/src/nxt_unit.c index c2e7f198..67244420 100644 --- a/src/nxt_unit.c +++ b/src/nxt_unit.c @@ -4282,16 +4282,38 @@ nxt_unit_add_port(nxt_unit_ctx_t *ctx, nxt_unit_port_t *port) int rc; nxt_unit_impl_t *lib; nxt_unit_process_t *process; - nxt_unit_port_impl_t *new_port; + nxt_unit_port_impl_t *new_port, *old_port; lib = nxt_container_of(ctx->unit, nxt_unit_impl_t, unit); + pthread_mutex_lock(&lib->mutex); + + old_port = nxt_unit_port_hash_find(&lib->ports, &port->id, 0); + + if (nxt_slow_path(old_port != NULL)) { + nxt_unit_debug(ctx, "add_port: duplicate %d,%d in_fd %d out_fd %d", + port->id.pid, port->id.id, + port->in_fd, port->out_fd); + + if (port->in_fd != -1) { + close(port->in_fd); + port->in_fd = -1; + } + + if (port->out_fd != -1) { + close(port->out_fd); + port->out_fd = -1; + } + + pthread_mutex_unlock(&lib->mutex); + + return NXT_UNIT_OK; + } + nxt_unit_debug(ctx, "add_port: %d,%d in_fd %d out_fd %d", port->id.pid, port->id.id, port->in_fd, port->out_fd); - pthread_mutex_lock(&lib->mutex); - process = nxt_unit_process_get(ctx, port->id.pid); if (nxt_slow_path(process == NULL)) { rc = NXT_UNIT_ERROR; -- cgit From 0bfa09dfa0ec6a1474ba30d0e1f8aea832fbc1fc Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Tue, 14 Apr 2020 02:35:04 +0100 Subject: Tests: minor fixes and style. --- test/test_go_application.py | 10 +++--- test/test_java_application.py | 10 +++--- test/test_java_websockets.py | 26 ++++++++-------- test/test_node_application.py | 8 +++-- test/test_node_websockets.py | 26 ++++++++-------- test/test_perl_application.py | 10 +++--- test/test_php_application.py | 10 +++--- test/test_php_basic.py | 56 +++++---------------------------- test/test_python_application.py | 16 ++++++---- test/test_python_basic.py | 69 ++++++----------------------------------- test/test_python_procman.py | 6 ++-- test/test_routing.py | 2 +- test/test_ruby_application.py | 10 +++--- test/test_tls.py | 5 +-- test/test_usr1.py | 2 +- test/unit/http.py | 8 ++--- 16 files changed, 99 insertions(+), 175 deletions(-) diff --git a/test/test_go_application.py b/test/test_go_application.py index 42429be7..c9d4ba77 100644 --- a/test/test_go_application.py +++ b/test/test_go_application.py @@ -89,6 +89,7 @@ class TestGoApplication(TestApplicationGo): self.assertEqual(self.get()['status'], 200, 'init') + body = '0123456789' * 500 (resp, sock) = self.post( headers={ 'Host': 'localhost', @@ -96,12 +97,13 @@ class TestGoApplication(TestApplicationGo): 'Content-Type': 'text/html', }, start=True, - body='0123456789' * 500, + body=body, read_timeout=1, ) - self.assertEqual(resp['body'], '0123456789' * 500, 'keep-alive 1') + self.assertEqual(resp['body'], body, 'keep-alive 1') + body = '0123456789' resp = self.post( headers={ 'Host': 'localhost', @@ -109,10 +111,10 @@ class TestGoApplication(TestApplicationGo): 'Connection': 'close', }, sock=sock, - body='0123456789', + body=body, ) - self.assertEqual(resp['body'], '0123456789', 'keep-alive 2') + self.assertEqual(resp['body'], body, 'keep-alive 2') def test_go_application_cookies(self): self.load('cookies') diff --git a/test/test_java_application.py b/test/test_java_application.py index 9d873d6b..7bd351a4 100644 --- a/test/test_java_application.py +++ b/test/test_java_application.py @@ -1085,6 +1085,7 @@ class TestJavaApplication(TestApplicationJava): self.assertEqual(self.post()['status'], 200, 'init') + body = '0123456789' * 500 (resp, sock) = self.post( headers={ 'Connection': 'keep-alive', @@ -1092,12 +1093,13 @@ class TestJavaApplication(TestApplicationJava): 'Host': 'localhost', }, start=True, - body='0123456789' * 500, + body=body, read_timeout=1, ) - self.assertEqual(resp['body'], '0123456789' * 500, 'keep-alive 1') + self.assertEqual(resp['body'], body, 'keep-alive 1') + body = '0123456789' resp = self.post( headers={ 'Connection': 'close', @@ -1105,10 +1107,10 @@ class TestJavaApplication(TestApplicationJava): 'Host': 'localhost', }, sock=sock, - body='0123456789', + body=body, ) - self.assertEqual(resp['body'], '0123456789', 'keep-alive 2') + self.assertEqual(resp['body'], body, 'keep-alive 2') def test_java_application_http_10(self): self.load('empty') diff --git a/test/test_java_websockets.py b/test/test_java_websockets.py index e0dbf539..7ea04620 100644 --- a/test/test_java_websockets.py +++ b/test/test_java_websockets.py @@ -26,7 +26,7 @@ class TestJavaWebsockets(TestApplicationJava): ) def close_connection(self, sock): - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty sock') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty soc') self.ws.frame_write(sock, self.ws.OP_CLOSE, self.ws.serialize_close()) @@ -441,12 +441,12 @@ class TestJavaWebsockets(TestApplicationJava): _, sock, _ = self.ws.upgrade() self.ws.frame_write(sock, self.ws.OP_PONG, '') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', '2_7') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', '2_7') # 2_8 self.ws.frame_write(sock, self.ws.OP_PONG, 'unsolicited pong payload') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', '2_8') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', '2_8') # 2_9 @@ -512,7 +512,7 @@ class TestJavaWebsockets(TestApplicationJava): self.check_close(sock, 1002, no_close=True) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty 3_2') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty 3_2') sock.close() # 3_3 @@ -530,7 +530,7 @@ class TestJavaWebsockets(TestApplicationJava): self.check_close(sock, 1002, no_close=True) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty 3_3') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty 3_3') sock.close() # 3_4 @@ -548,7 +548,7 @@ class TestJavaWebsockets(TestApplicationJava): self.check_close(sock, 1002, no_close=True) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty 3_4') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty 3_4') sock.close() # 3_5 @@ -734,7 +734,7 @@ class TestJavaWebsockets(TestApplicationJava): # 5_4 self.ws.frame_write(sock, self.ws.OP_TEXT, 'fragment1', fin=False) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', '5_4') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', '5_4') self.ws.frame_write(sock, self.ws.OP_CONT, 'fragment2', fin=True) frame = self.ws.frame_read(sock) @@ -771,7 +771,7 @@ class TestJavaWebsockets(TestApplicationJava): ping_payload = 'ping payload' self.ws.frame_write(sock, self.ws.OP_TEXT, 'fragment1', fin=False) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', '5_7') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', '5_7') self.ws.frame_write(sock, self.ws.OP_PING, ping_payload) @@ -955,7 +955,7 @@ class TestJavaWebsockets(TestApplicationJava): frame = self.ws.frame_read(sock) self.check_frame(frame, True, self.ws.OP_PONG, 'pongme 2!') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', '5_20') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', '5_20') self.ws.frame_write(sock, self.ws.OP_CONT, 'fragment5') self.check_frame( @@ -1088,7 +1088,7 @@ class TestJavaWebsockets(TestApplicationJava): self.check_close(sock, no_close=True) self.ws.frame_write(sock, self.ws.OP_PING, '') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty sock') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty soc') sock.close() @@ -1100,7 +1100,7 @@ class TestJavaWebsockets(TestApplicationJava): self.check_close(sock, no_close=True) self.ws.frame_write(sock, self.ws.OP_TEXT, payload) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty sock') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty soc') sock.close() @@ -1113,7 +1113,7 @@ class TestJavaWebsockets(TestApplicationJava): self.check_close(sock, no_close=True) self.ws.frame_write(sock, self.ws.OP_CONT, 'fragment2') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty sock') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty soc') sock.close() @@ -1128,7 +1128,7 @@ class TestJavaWebsockets(TestApplicationJava): self.recvall(sock, read_timeout=1) self.ws.frame_write(sock, self.ws.OP_PING, '') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty sock') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty soc') sock.close() diff --git a/test/test_node_application.py b/test/test_node_application.py index b80d17d3..174af15d 100644 --- a/test/test_node_application.py +++ b/test/test_node_application.py @@ -112,6 +112,7 @@ class TestNodeApplication(TestApplicationNode): self.assertEqual(self.get()['status'], 200, 'init') + body = '0123456789' * 500 (resp, sock) = self.post( headers={ 'Host': 'localhost', @@ -119,12 +120,13 @@ class TestNodeApplication(TestApplicationNode): 'Content-Type': 'text/html', }, start=True, - body='0123456789' * 500, + body=body, read_timeout=1, ) self.assertEqual(resp['body'], '0123456789' * 500, 'keep-alive 1') + body = '0123456789' resp = self.post( headers={ 'Host': 'localhost', @@ -132,10 +134,10 @@ class TestNodeApplication(TestApplicationNode): 'Content-Type': 'text/html', }, sock=sock, - body='0123456789', + body=body, ) - self.assertEqual(resp['body'], '0123456789', 'keep-alive 2') + self.assertEqual(resp['body'], body, 'keep-alive 2') def test_node_application_write_buffer(self): self.load('write_buffer') diff --git a/test/test_node_websockets.py b/test/test_node_websockets.py index cb6bf137..4ce727db 100644 --- a/test/test_node_websockets.py +++ b/test/test_node_websockets.py @@ -26,7 +26,7 @@ class TestNodeWebsockets(TestApplicationNode): ) def close_connection(self, sock): - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty sock') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty soc') self.ws.frame_write(sock, self.ws.OP_CLOSE, self.ws.serialize_close()) @@ -460,12 +460,12 @@ class TestNodeWebsockets(TestApplicationNode): _, sock, _ = self.ws.upgrade() self.ws.frame_write(sock, self.ws.OP_PONG, '') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', '2_7') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', '2_7') # 2_8 self.ws.frame_write(sock, self.ws.OP_PONG, 'unsolicited pong payload') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', '2_8') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', '2_8') # 2_9 @@ -531,7 +531,7 @@ class TestNodeWebsockets(TestApplicationNode): self.check_close(sock, 1002, no_close=True) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty 3_2') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty 3_2') sock.close() # 3_3 @@ -549,7 +549,7 @@ class TestNodeWebsockets(TestApplicationNode): self.check_close(sock, 1002, no_close=True) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty 3_3') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty 3_3') sock.close() # 3_4 @@ -567,7 +567,7 @@ class TestNodeWebsockets(TestApplicationNode): self.check_close(sock, 1002, no_close=True) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty 3_4') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty 3_4') sock.close() # 3_5 @@ -753,7 +753,7 @@ class TestNodeWebsockets(TestApplicationNode): # 5_4 self.ws.frame_write(sock, self.ws.OP_TEXT, 'fragment1', fin=False) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', '5_4') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', '5_4') self.ws.frame_write(sock, self.ws.OP_CONT, 'fragment2', fin=True) frame = self.ws.frame_read(sock) @@ -790,7 +790,7 @@ class TestNodeWebsockets(TestApplicationNode): ping_payload = 'ping payload' self.ws.frame_write(sock, self.ws.OP_TEXT, 'fragment1', fin=False) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', '5_7') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', '5_7') self.ws.frame_write(sock, self.ws.OP_PING, ping_payload) @@ -974,7 +974,7 @@ class TestNodeWebsockets(TestApplicationNode): frame = self.ws.frame_read(sock) self.check_frame(frame, True, self.ws.OP_PONG, 'pongme 2!') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', '5_20') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', '5_20') self.ws.frame_write(sock, self.ws.OP_CONT, 'fragment5') self.check_frame( @@ -1107,7 +1107,7 @@ class TestNodeWebsockets(TestApplicationNode): self.check_close(sock, no_close=True) self.ws.frame_write(sock, self.ws.OP_PING, '') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty sock') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty soc') sock.close() @@ -1119,7 +1119,7 @@ class TestNodeWebsockets(TestApplicationNode): self.check_close(sock, no_close=True) self.ws.frame_write(sock, self.ws.OP_TEXT, payload) - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty sock') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty soc') sock.close() @@ -1132,7 +1132,7 @@ class TestNodeWebsockets(TestApplicationNode): self.check_close(sock, no_close=True) self.ws.frame_write(sock, self.ws.OP_CONT, 'fragment2') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty sock') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty soc') sock.close() @@ -1147,7 +1147,7 @@ class TestNodeWebsockets(TestApplicationNode): self.recvall(sock, read_timeout=1) self.ws.frame_write(sock, self.ws.OP_PING, '') - self.assertEqual(self.recvall(sock, read_timeout=1), b'', 'empty sock') + self.assertEqual(self.recvall(sock, read_timeout=0.1), b'', 'empty soc') sock.close() diff --git a/test/test_perl_application.py b/test/test_perl_application.py index a4bac623..cc4eb915 100644 --- a/test/test_perl_application.py +++ b/test/test_perl_application.py @@ -197,6 +197,7 @@ class TestPerlApplication(TestApplicationPerl): self.assertEqual(self.get()['status'], 200, 'init') + body = '0123456789' * 500 (resp, sock) = self.post( headers={ 'Host': 'localhost', @@ -204,12 +205,13 @@ class TestPerlApplication(TestApplicationPerl): 'Content-Type': 'text/html', }, start=True, - body='0123456789' * 500, + body=body, read_timeout=1, ) - self.assertEqual(resp['body'], '0123456789' * 500, 'keep-alive 1') + self.assertEqual(resp['body'], body, 'keep-alive 1') + body = '0123456789' resp = self.post( headers={ 'Host': 'localhost', @@ -217,10 +219,10 @@ class TestPerlApplication(TestApplicationPerl): 'Content-Type': 'text/html', }, sock=sock, - body='0123456789', + body=body, ) - self.assertEqual(resp['body'], '0123456789', 'keep-alive 2') + self.assertEqual(resp['body'], body, 'keep-alive 2') def test_perl_body_io_fake(self): self.load('body_io_fake') diff --git a/test/test_php_application.py b/test/test_php_application.py index c3645a99..48e1e815 100644 --- a/test/test_php_application.py +++ b/test/test_php_application.py @@ -183,6 +183,7 @@ class TestPHPApplication(TestApplicationPHP): self.assertEqual(self.get()['status'], 200, 'init') + body = '0123456789' * 500 (resp, sock) = self.post( headers={ 'Host': 'localhost', @@ -190,12 +191,13 @@ class TestPHPApplication(TestApplicationPHP): 'Content-Type': 'text/html', }, start=True, - body='0123456789' * 500, + body=body, read_timeout=1, ) - self.assertEqual(resp['body'], '0123456789' * 500, 'keep-alive 1') + self.assertEqual(resp['body'], body, 'keep-alive 1') + body = '0123456789' resp = self.post( headers={ 'Host': 'localhost', @@ -203,10 +205,10 @@ class TestPHPApplication(TestApplicationPHP): 'Content-Type': 'text/html', }, sock=sock, - body='0123456789', + body=body, ) - self.assertEqual(resp['body'], '0123456789', 'keep-alive 2') + self.assertEqual(resp['body'], body, 'keep-alive 2') def test_php_application_conditional(self): self.load('conditional') diff --git a/test/test_php_basic.py b/test/test_php_basic.py index 7ecff1b2..5fde3e00 100644 --- a/test/test_php_basic.py +++ b/test/test_php_basic.py @@ -37,9 +37,6 @@ class TestPHPBasic(TestControl): 'applications', ) - def test_php_get_applications_prefix(self): - self.conf(self.conf_app, 'applications') - self.assertEqual( self.conf_get('applications'), { @@ -53,9 +50,6 @@ class TestPHPBasic(TestControl): 'applications prefix', ) - def test_php_get_applications_prefix_2(self): - self.conf(self.conf_app, 'applications') - self.assertEqual( self.conf_get('applications/app'), { @@ -67,9 +61,6 @@ class TestPHPBasic(TestControl): 'applications prefix 2', ) - def test_php_get_applications_prefix_3(self): - self.conf(self.conf_app, 'applications') - self.assertEqual(self.conf_get('applications/app/type'), 'php', 'type') self.assertEqual( self.conf_get('applications/app/processes/spare'), @@ -86,18 +77,12 @@ class TestPHPBasic(TestControl): 'listeners', ) - def test_php_get_listeners_prefix(self): - self.conf(self.conf_basic) - self.assertEqual( self.conf_get('listeners'), {"*:7080": {"pass": "applications/app"}}, 'listeners prefix', ) - def test_php_get_listeners_prefix_2(self): - self.conf(self.conf_basic) - self.assertEqual( self.conf_get('listeners/*:7080'), {"pass": "applications/app"}, @@ -147,49 +132,24 @@ class TestPHPBasic(TestControl): def test_php_delete(self): self.conf(self.conf_basic) - self.assertIn( - 'error', - self.conf_delete('applications/app'), - 'delete app before listener', - ) - self.assertIn( - 'success', self.conf_delete('listeners/*:7080'), 'delete listener' - ) - self.assertIn( - 'success', - self.conf_delete('applications/app'), - 'delete app after listener', - ) - self.assertIn( - 'error', self.conf_delete('applications/app'), 'delete app again' - ) + self.assertIn('error', self.conf_delete('applications/app')) + self.assertIn('success', self.conf_delete('listeners/*:7080')) + self.assertIn('success', self.conf_delete('applications/app')) + self.assertIn('error', self.conf_delete('applications/app')) def test_php_delete_blocks(self): self.conf(self.conf_basic) - self.assertIn( - 'success', - self.conf_delete('listeners'), - 'listeners delete', - ) - - self.assertIn( - 'success', - self.conf_delete('applications'), - 'applications delete', - ) - - self.assertIn( - 'success', - self.conf(self.conf_app, 'applications'), - 'listeners restore', - ) + self.assertIn('success', self.conf_delete('listeners')) + self.assertIn('success', self.conf_delete('applications')) + self.assertIn('success', self.conf(self.conf_app, 'applications')) self.assertIn( 'success', self.conf({"*:7081": {"pass": "applications/app"}}, 'listeners'), 'applications restore', ) + if __name__ == '__main__': TestPHPBasic.main() diff --git a/test/test_python_application.py b/test/test_python_application.py index 460cc804..5741d2d8 100644 --- a/test/test_python_application.py +++ b/test/test_python_application.py @@ -187,6 +187,7 @@ class TestPythonApplication(TestApplicationPython): self.assertEqual(self.get()['status'], 200, 'init') + body = '0123456789' * 500 (resp, sock) = self.post( headers={ 'Host': 'localhost', @@ -194,12 +195,13 @@ class TestPythonApplication(TestApplicationPython): 'Content-Type': 'text/html', }, start=True, - body='0123456789' * 500, + body=body, read_timeout=1, ) - self.assertEqual(resp['body'], '0123456789' * 500, 'keep-alive 1') + self.assertEqual(resp['body'], body, 'keep-alive 1') + body = '0123456789' resp = self.post( headers={ 'Host': 'localhost', @@ -207,10 +209,10 @@ class TestPythonApplication(TestApplicationPython): 'Content-Type': 'text/html', }, sock=sock, - body='0123456789', + body=body, ) - self.assertEqual(resp['body'], '0123456789', 'keep-alive 2') + self.assertEqual(resp['body'], body, 'keep-alive 2') def test_python_keepalive_reconfigure(self): self.skip_alerts.extend( @@ -340,14 +342,16 @@ class TestPythonApplication(TestApplicationPython): self.assertEqual(self.get()['status'], 200, 'init') - (resp, sock) = self.http( + (_, sock) = self.http( b"""GET / HTTP/1.1 """, start=True, raw=True, - read_timeout=5, + no_recv=True, ) + self.assertEqual(self.get()['status'], 200) + self.assertIn( 'success', self.conf({"listeners": {}, "applications": {}}), diff --git a/test/test_python_basic.py b/test/test_python_basic.py index 67a5f548..3233fca2 100644 --- a/test/test_python_basic.py +++ b/test/test_python_basic.py @@ -19,17 +19,9 @@ class TestPythonBasic(TestControl): } def test_python_get_empty(self): - self.assertEqual( - self.conf_get(), {'listeners': {}, 'applications': {}}, 'empty' - ) - - def test_python_get_prefix_listeners(self): - self.assertEqual(self.conf_get('listeners'), {}, 'listeners prefix') - - def test_python_get_prefix_applications(self): - self.assertEqual( - self.conf_get('applications'), {}, 'applications prefix' - ) + self.assertEqual(self.conf_get(), {'listeners': {}, 'applications': {}}) + self.assertEqual(self.conf_get('listeners'), {}) + self.assertEqual(self.conf_get('applications'), {}) def test_python_get_applications(self): self.conf(self.conf_app, 'applications') @@ -50,9 +42,6 @@ class TestPythonBasic(TestControl): 'applications', ) - def test_python_get_applications_prefix(self): - self.conf(self.conf_app, 'applications') - self.assertEqual( self.conf_get('applications'), { @@ -66,9 +55,6 @@ class TestPythonBasic(TestControl): 'applications prefix', ) - def test_python_get_applications_prefix_2(self): - self.conf(self.conf_app, 'applications') - self.assertEqual( self.conf_get('applications/app'), { @@ -80,9 +66,6 @@ class TestPythonBasic(TestControl): 'applications prefix 2', ) - def test_python_get_applications_prefix_3(self): - self.conf(self.conf_app, 'applications') - self.assertEqual( self.conf_get('applications/app/type'), 'python', 'type' ) @@ -99,18 +82,12 @@ class TestPythonBasic(TestControl): 'listeners', ) - def test_python_get_listeners_prefix(self): - self.conf(self.conf_basic) - self.assertEqual( self.conf_get('listeners'), {"*:7080": {"pass": "applications/app"}}, 'listeners prefix', ) - def test_python_get_listeners_prefix_2(self): - self.conf(self.conf_basic) - self.assertEqual( self.conf_get('listeners/*:7080'), {"pass": "applications/app"}, @@ -160,44 +137,18 @@ class TestPythonBasic(TestControl): def test_python_delete(self): self.conf(self.conf_basic) - self.assertIn( - 'error', - self.conf_delete('applications/app'), - 'delete app before listener', - ) - self.assertIn( - 'success', self.conf_delete('listeners/*:7080'), 'delete listener' - ) - self.assertIn( - 'success', - self.conf_delete('applications/app'), - 'delete app after listener', - ) - self.assertIn( - 'error', self.conf_delete('applications/app'), 'delete app again' - ) + self.assertIn('error', self.conf_delete('applications/app')) + self.assertIn('success', self.conf_delete('listeners/*:7080')) + self.assertIn('success', self.conf_delete('applications/app')) + self.assertIn('error', self.conf_delete('applications/app')) def test_python_delete_blocks(self): self.conf(self.conf_basic) - self.assertIn( - 'success', - self.conf_delete('listeners'), - 'listeners delete', - ) - - self.assertIn( - 'success', - self.conf_delete('applications'), - 'applications delete', - ) - - self.assertIn( - 'success', - self.conf(self.conf_app, 'applications'), - 'listeners restore', - ) + self.assertIn('success', self.conf_delete('listeners')) + self.assertIn('success', self.conf_delete('applications')) + self.assertIn('success', self.conf(self.conf_app, 'applications')) self.assertIn( 'success', self.conf({"*:7081": {"pass": "applications/app"}}, 'listeners'), diff --git a/test/test_python_procman.py b/test/test_python_procman.py index 8fb499f7..a2e6126c 100644 --- a/test/test_python_procman.py +++ b/test/test_python_procman.py @@ -196,7 +196,7 @@ class TestPythonProcman(TestApplicationPython): ) self.assertIn( 'error', - self.conf({"idle_timeout": -1}, self.app_proc,), + self.conf({"idle_timeout": -1}, self.app_proc), 'negative idle_timeout', ) self.assertIn( @@ -206,12 +206,12 @@ class TestPythonProcman(TestApplicationPython): ) self.assertIn( 'error', - self.conf({"spare": 2, "max": 1}, self.app_proc,), + self.conf({"spare": 2, "max": 1}, self.app_proc), 'spare gt max', ) self.assertIn( 'error', - self.conf({"spare": 0, "max": 0}, self.app_proc,), + self.conf({"spare": 0, "max": 0}, self.app_proc), 'max zero', ) diff --git a/test/test_routing.py b/test/test_routing.py index bf741706..ad793662 100644 --- a/test/test_routing.py +++ b/test/test_routing.py @@ -605,7 +605,7 @@ class TestRouting(TestApplicationProto): self.assertIn( 'success', self.conf_post( - {"match": {"method": "POST"}, "action": {"return": 200},}, + {"match": {"method": "POST"}, "action": {"return": 200}}, 'routes', ), 'routes edit configure 6', diff --git a/test/test_ruby_application.py b/test/test_ruby_application.py index 83a71f96..bdaabe51 100644 --- a/test/test_ruby_application.py +++ b/test/test_ruby_application.py @@ -322,6 +322,7 @@ class TestRubyApplication(TestApplicationRuby): self.assertEqual(self.get()['status'], 200, 'init') + body = '0123456789' * 500 (resp, sock) = self.post( headers={ 'Host': 'localhost', @@ -329,12 +330,13 @@ class TestRubyApplication(TestApplicationRuby): 'Content-Type': 'text/html', }, start=True, - body='0123456789' * 500, + body=body, read_timeout=1, ) - self.assertEqual(resp['body'], '0123456789' * 500, 'keep-alive 1') + self.assertEqual(resp['body'], body, 'keep-alive 1') + body = '0123456789' resp = self.post( headers={ 'Host': 'localhost', @@ -342,10 +344,10 @@ class TestRubyApplication(TestApplicationRuby): 'Content-Type': 'text/html', }, sock=sock, - body='0123456789', + body=body, ) - self.assertEqual(resp['body'], '0123456789', 'keep-alive 2') + self.assertEqual(resp['body'], body, 'keep-alive 2') def test_ruby_application_constants(self): self.load('constants') diff --git a/test/test_tls.py b/test/test_tls.py index 475e9919..d9dcf237 100644 --- a/test/test_tls.py +++ b/test/test_tls.py @@ -521,7 +521,6 @@ basicConstraints = critical,CA:TRUE""" ) def test_tls_application_respawn(self): - self.skip_alerts.append(r'process \d+ exited on signal 9') self.load('mirror') self.certificate() @@ -530,7 +529,7 @@ basicConstraints = critical,CA:TRUE""" self.add_tls(application='mirror') - (resp, sock) = self.post_ssl( + (_, sock) = self.post_ssl( headers={ 'Host': 'localhost', 'Connection': 'keep-alive', @@ -545,6 +544,8 @@ basicConstraints = critical,CA:TRUE""" subprocess.call(['kill', '-9', app_id]) + self.skip_alerts.append(r'process %s exited on signal 9' % app_id) + self.wait_for_record( re.compile( ' (?!' + app_id + '#)(\d+)#\d+ "mirror" application started' diff --git a/test/test_usr1.py b/test/test_usr1.py index 204e2e0c..155303ea 100644 --- a/test/test_usr1.py +++ b/test/test_usr1.py @@ -55,7 +55,7 @@ class TestUSR1(TestApplicationPython): self.load('log_body') log_new = 'new.log' - log_path = self.testdir + '/' + 'unit.log' + log_path = self.testdir + '/unit.log' log_path_new = self.testdir + '/' + log_new os.rename(log_path, log_path_new) diff --git a/test/unit/http.py b/test/unit/http.py index 8aacf18b..13384dc8 100644 --- a/test/unit/http.py +++ b/test/unit/http.py @@ -279,12 +279,8 @@ class TestHTTP(TestUnit): def _parse_json(self, resp): headers = resp['headers'] - self.assertIn('Content-Type', headers, 'Content-Type header set') - self.assertEqual( - headers['Content-Type'], - 'application/json', - 'Content-Type header is application/json', - ) + self.assertIn('Content-Type', headers) + self.assertEqual(headers['Content-Type'], 'application/json') resp['body'] = json.loads(resp['body']) -- cgit From 3c58a4bfc11d2d73fc72d1dbaeda4494d00508b8 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Tue, 14 Apr 2020 03:02:16 +0100 Subject: Tests: added test with rescheduling requests. --- test/test_python_application.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/test_python_application.py b/test/test_python_application.py index 5741d2d8..8d435b48 100644 --- a/test/test_python_application.py +++ b/test/test_python_application.py @@ -382,6 +382,38 @@ Connection: close self.wait_for_record(r'At exit called\.'), 'atexit' ) + def test_python_process_switch(self): + self.load('delayed') + + self.assertIn( + 'success', + self.conf('2', 'applications/delayed/processes'), + 'configure 2 processes', + ) + + self.get(headers={ + 'Host': 'localhost', + 'Content-Length': '0', + 'X-Delay': '5', + 'Connection': 'close', + }, no_recv=True) + + headers_delay_1 = { + 'Connection': 'close', + 'Host': 'localhost', + 'Content-Length': '0', + 'X-Delay': '1', + } + + self.get(headers=headers_delay_1, no_recv=True) + + time.sleep(0.5) + + for _ in range(10): + self.get(headers=headers_delay_1, no_recv=True) + + self.get(headers=headers_delay_1) + @unittest.skip('not yet') def test_python_application_start_response_exit(self): self.load('start_response_exit') -- cgit From 9a422b8984a3ed462a2c35ba97fa8234f3a45591 Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Tue, 14 Apr 2020 16:11:13 +0300 Subject: Completing chained shared memory buffers. After 41331471eee7 completion handlers should complete next buffer in chain. Otherwise buffer memory may leak. Thanks to Peter Tkatchenko for reporing the issue and testing fixes. --- src/nxt_port_memory.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/nxt_port_memory.c b/src/nxt_port_memory.c index 24a40406..33d3777e 100644 --- a/src/nxt_port_memory.c +++ b/src/nxt_port_memory.c @@ -111,7 +111,7 @@ nxt_port_mmap_buf_completion(nxt_task_t *task, void *obj, void *data) { u_char *p; nxt_mp_t *mp; - nxt_buf_t *b; + nxt_buf_t *b, *next; nxt_port_t *port; nxt_process_t *process; nxt_chunk_id_t c; @@ -124,11 +124,12 @@ nxt_port_mmap_buf_completion(nxt_task_t *task, void *obj, void *data) b = obj; - mp = b->data; - nxt_assert(data == b->parent); mmap_handler = data; + +complete_buf: + hdr = mmap_handler->hdr; if (nxt_slow_path(hdr->src_pid != nxt_pid && hdr->dst_pid != nxt_pid)) { @@ -184,8 +185,18 @@ release_buf: nxt_port_mmap_handler_use(mmap_handler, -1); + next = b->next; + mp = b->data; + nxt_mp_free(mp, b); nxt_mp_release(mp); + + if (next != NULL) { + b = next; + mmap_handler = b->parent; + + goto complete_buf; + } } -- cgit From e616d0915c513323affd938f7eb89d23d4e70df5 Mon Sep 17 00:00:00 2001 From: Igor Sysoev Date: Wed, 15 Apr 2020 14:54:09 +0300 Subject: Disabled epoll error processing when socket events are inactive. --- src/nxt_epoll_engine.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/nxt_epoll_engine.c b/src/nxt_epoll_engine.c index a944834e..d53df1bc 100644 --- a/src/nxt_epoll_engine.c +++ b/src/nxt_epoll_engine.c @@ -926,6 +926,13 @@ nxt_epoll_poll(nxt_event_engine_t *engine, nxt_msec_t timeout) error = ((events & (EPOLLERR | EPOLLHUP)) != 0); ev->epoll_error = error; + if (error + && ev->read <= NXT_EVENT_BLOCKED + && ev->write <= NXT_EVENT_BLOCKED) + { + error = 0; + } + #if (NXT_HAVE_EPOLL_EDGE) ev->epoll_eof = ((events & EPOLLRDHUP) != 0); -- cgit From 04143c8c7ee59d24aa1d6df0377e7900e96e3f72 Mon Sep 17 00:00:00 2001 From: Igor Sysoev Date: Wed, 15 Apr 2020 14:54:09 +0300 Subject: Fixed crash that occurs when idle connections are closed forcibly. --- src/nxt_conn_accept.c | 74 +++++++++++++++++++++++++++++------------------- src/nxt_h1proto.c | 37 ++++++++++++++++++++---- src/nxt_log_moderation.c | 1 + 3 files changed, 77 insertions(+), 35 deletions(-) diff --git a/src/nxt_conn_accept.c b/src/nxt_conn_accept.c index 4ad2d02f..d4c3942c 100644 --- a/src/nxt_conn_accept.c +++ b/src/nxt_conn_accept.c @@ -24,8 +24,10 @@ static void nxt_conn_listen_handler(nxt_task_t *task, void *obj, void *data); static nxt_conn_t *nxt_conn_accept_next(nxt_task_t *task, nxt_listen_event_t *lev); -static nxt_int_t nxt_conn_accept_close_idle(nxt_task_t *task, +static void nxt_conn_accept_close_idle(nxt_task_t *task, nxt_listen_event_t *lev); +static void nxt_conn_accept_close_idle_handler(nxt_task_t *task, void *obj, + void *data); static void nxt_conn_listen_event_error(nxt_task_t *task, void *obj, void *data); static void nxt_conn_listen_timer_handler(nxt_task_t *task, void *obj, @@ -230,60 +232,76 @@ nxt_conn_accept_next(nxt_task_t *task, nxt_listen_event_t *lev) lev->next = NULL; - do { - c = nxt_conn_accept_alloc(task, lev); + c = nxt_conn_accept_alloc(task, lev); - if (nxt_fast_path(c != NULL)) { - return c; - } + if (nxt_slow_path(c == NULL)) { + nxt_conn_accept_close_idle(task, lev); + } - } while (nxt_conn_accept_close_idle(task, lev) == NXT_OK); + return c; +} - nxt_alert(task, "no available connections, " - "new connections are not accepted within 1s"); - return NULL; +static void +nxt_conn_accept_close_idle(nxt_task_t *task, nxt_listen_event_t *lev) +{ + nxt_event_engine_t *engine; + + engine = task->thread->engine; + + nxt_work_queue_add(&engine->close_work_queue, + nxt_conn_accept_close_idle_handler, task, NULL, NULL); + + nxt_timer_add(engine, &lev->timer, 100); + + nxt_fd_event_disable_read(engine, &lev->socket); + + nxt_alert(task, "new connections are not accepted within 100ms"); } -static nxt_int_t -nxt_conn_accept_close_idle(nxt_task_t *task, nxt_listen_event_t *lev) +static void +nxt_conn_accept_close_idle_handler(nxt_task_t *task, void *obj, void *data) { + nxt_uint_t times; nxt_conn_t *c; nxt_queue_t *idle; - nxt_queue_link_t *link; + nxt_queue_link_t *link, *next; nxt_event_engine_t *engine; static nxt_log_moderation_t nxt_idle_close_log_moderation = { NXT_LOG_INFO, 2, "idle connections closed", NXT_LOG_MODERATION }; + times = 10; engine = task->thread->engine; - idle = &engine->idle_connections; for (link = nxt_queue_last(idle); link != nxt_queue_head(idle); - link = nxt_queue_next(link)) + link = next) { + next = nxt_queue_next(link); + c = nxt_queue_link_data(link, nxt_conn_t, link); + nxt_debug(c->socket.task, "idle connection: %d rdy:%d", + c->socket.fd, c->socket.read_ready); + if (!c->socket.read_ready) { nxt_log_moderate(&nxt_idle_close_log_moderation, NXT_LOG_INFO, task->log, "no available connections, " "close idle connection"); - nxt_queue_remove(link); - nxt_conn_close(engine, c); - return NXT_OK; - } - } + c->read_state->close_handler(c->socket.task, c, c->socket.data); - nxt_timer_add(engine, &lev->timer, 1000); + times--; - nxt_fd_event_disable_read(engine, &lev->socket); - - return NXT_DECLINED; + if (times == 0) { + break; + } + } + } } @@ -313,12 +331,10 @@ nxt_conn_accept_error(nxt_task_t *task, nxt_listen_event_t *lev, case ENFILE: case ENOBUFS: case ENOMEM: - if (nxt_conn_accept_close_idle(task, lev) != NXT_OK) { - nxt_alert(task, "%s(%d) failed %E, " - "new connections are not accepted within 1s", - accept_syscall, lev->socket.fd, err); - } + nxt_alert(task, "%s(%d) failed %E", + accept_syscall, lev->socket.fd, err); + nxt_conn_accept_close_idle(task, lev); return; default: diff --git a/src/nxt_h1proto.c b/src/nxt_h1proto.c index 5e3b2f82..c2e65397 100644 --- a/src/nxt_h1proto.c +++ b/src/nxt_h1proto.c @@ -74,6 +74,8 @@ static void nxt_h1p_idle_close(nxt_task_t *task, void *obj, void *data); static void nxt_h1p_idle_timeout(nxt_task_t *task, void *obj, void *data); static void nxt_h1p_idle_response(nxt_task_t *task, nxt_conn_t *c); static void nxt_h1p_idle_response_sent(nxt_task_t *task, void *obj, void *data); +static void nxt_h1p_idle_response_error(nxt_task_t *task, void *obj, + void *data); static void nxt_h1p_idle_response_timeout(nxt_task_t *task, void *obj, void *data); static nxt_msec_t nxt_h1p_idle_response_timer_value(nxt_conn_t *c, @@ -470,6 +472,8 @@ nxt_h1p_conn_request_init(nxt_task_t *task, void *obj, void *data) nxt_debug(task, "h1p conn request init"); + nxt_queue_remove(&c->link); + r = nxt_http_request_create(task); if (nxt_fast_path(r != NULL)) { @@ -1714,6 +1718,8 @@ nxt_h1p_conn_close(nxt_task_t *task, void *obj, void *data) nxt_debug(task, "h1p conn close"); + nxt_queue_remove(&c->link); + nxt_h1p_shutdown(task, c); } @@ -1727,6 +1733,8 @@ nxt_h1p_conn_error(nxt_task_t *task, void *obj, void *data) nxt_debug(task, "h1p conn error"); + nxt_queue_remove(&c->link); + nxt_h1p_shutdown(task, c); } @@ -1745,8 +1753,9 @@ nxt_h1p_conn_timer_value(nxt_conn_t *c, uintptr_t data) static void nxt_h1p_keepalive(nxt_task_t *task, nxt_h1proto_t *h1p, nxt_conn_t *c) { - size_t size; - nxt_buf_t *in; + size_t size; + nxt_buf_t *in; + nxt_event_engine_t *engine; nxt_debug(task, "h1p keepalive"); @@ -1762,10 +1771,13 @@ nxt_h1p_keepalive(nxt_task_t *task, nxt_h1proto_t *h1p, nxt_conn_t *c) c->sent = 0; + engine = task->thread->engine; + nxt_queue_insert_head(&engine->idle_connections, &c->link); + if (in == NULL) { c->read_state = &nxt_h1p_keepalive_state; - nxt_conn_read(task->thread->engine, c); + nxt_conn_read(engine, c); } else { size = nxt_buf_mem_used_size(&in->mem); @@ -1831,6 +1843,8 @@ nxt_h1p_idle_timeout(nxt_task_t *task, void *obj, void *data) c = nxt_read_timer_conn(timer); c->block_read = 1; + nxt_queue_remove(&c->link); + nxt_h1p_idle_response(task, c); } @@ -1898,7 +1912,7 @@ static const nxt_conn_state_t nxt_h1p_timeout_response_state nxt_aligned(64) = { .ready_handler = nxt_h1p_conn_sent, - .error_handler = nxt_h1p_conn_error, + .error_handler = nxt_h1p_idle_response_error, .timer_handler = nxt_h1p_idle_response_timeout, .timer_value = nxt_h1p_idle_response_timer_value, @@ -1918,6 +1932,19 @@ nxt_h1p_idle_response_sent(nxt_task_t *task, void *obj, void *data) } +static void +nxt_h1p_idle_response_error(nxt_task_t *task, void *obj, void *data) +{ + nxt_conn_t *c; + + c = obj; + + nxt_debug(task, "h1p response error"); + + nxt_h1p_shutdown(task, c); +} + + static void nxt_h1p_idle_response_timeout(nxt_task_t *task, void *obj, void *data) { @@ -2057,8 +2084,6 @@ nxt_h1p_conn_free(nxt_task_t *task, void *obj, void *data) nxt_debug(task, "h1p conn free"); - nxt_queue_remove(&c->link); - engine = task->thread->engine; nxt_sockaddr_cache_free(engine, c); diff --git a/src/nxt_log_moderation.c b/src/nxt_log_moderation.c index 7c2d7a50..95f9cbfe 100644 --- a/src/nxt_log_moderation.c +++ b/src/nxt_log_moderation.c @@ -61,6 +61,7 @@ nxt_log_moderate_allow(nxt_log_moderation_t *mod) mod->timer.work_queue = &thr->engine->fast_work_queue; mod->timer.handler = nxt_log_moderate_timer_handler; mod->timer.log = &nxt_main_log; + mod->timer.task = &nxt_main_task; nxt_timer_add(thr->engine, &mod->timer, 1000); } -- cgit From ee62736a11acc4b699102a1260c6a8c5f57c1fef Mon Sep 17 00:00:00 2001 From: Igor Sysoev Date: Wed, 15 Apr 2020 15:10:14 +0300 Subject: Fixed memory leak occurring upon failure to accept a connection. --- src/nxt_conn.h | 2 +- src/nxt_conn_accept.c | 21 ++++++++++----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/nxt_conn.h b/src/nxt_conn.h index 2c1d49a0..a443601f 100644 --- a/src/nxt_conn.h +++ b/src/nxt_conn.h @@ -106,7 +106,7 @@ typedef struct { nxt_work_handler_t accept; nxt_listen_socket_t *listen; - nxt_conn_t *next; /* STUB */ + nxt_conn_t *next; nxt_work_queue_t *work_queue; nxt_timer_t timer; diff --git a/src/nxt_conn_accept.c b/src/nxt_conn_accept.c index d4c3942c..6a89840c 100644 --- a/src/nxt_conn_accept.c +++ b/src/nxt_conn_accept.c @@ -101,12 +101,12 @@ nxt_conn_accept_alloc(nxt_task_t *task, nxt_listen_event_t *lev) goto fail; } - lev->next = c; c->socket.read_work_queue = lev->socket.read_work_queue; c->socket.write_ready = 1; c->remote = nxt_sockaddr_cache_alloc(engine, lev->listen); if (nxt_fast_path(c->remote != NULL)) { + lev->next = c; return c; } } @@ -199,6 +199,7 @@ nxt_conn_accept(nxt_task_t *task, nxt_listen_event_t *lev, nxt_conn_t *c) c->listen = lev; lev->count++; + lev->next = NULL; c->socket.data = NULL; c->read_work_queue = lev->work_queue; @@ -230,12 +231,14 @@ nxt_conn_accept_next(nxt_task_t *task, nxt_listen_event_t *lev) { nxt_conn_t *c; - lev->next = NULL; + c = lev->next; - c = nxt_conn_accept_alloc(task, lev); + if (c == NULL) { + c = nxt_conn_accept_alloc(task, lev); - if (nxt_slow_path(c == NULL)) { - nxt_conn_accept_close_idle(task, lev); + if (nxt_slow_path(c == NULL)) { + nxt_conn_accept_close_idle(task, lev); + } } return c; @@ -355,14 +358,10 @@ nxt_conn_listen_timer_handler(nxt_task_t *task, void *obj, void *data) timer = obj; lev = nxt_timer_data(timer, nxt_listen_event_t, timer); - c = lev->next; + c = nxt_conn_accept_next(task, lev); if (c == NULL) { - c = nxt_conn_accept_next(task, lev); - - if (c == NULL) { - return; - } + return; } nxt_fd_event_enable_accept(task->thread->engine, &lev->socket); -- cgit From 6bda9b5eeb2b6c99c54f5b314b8eb96d72af3542 Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Thu, 16 Apr 2020 17:09:23 +0300 Subject: Using malloc/free for the http fields hash. This is required due to lack of a graceful shutdown: there is a small gap between the runtime's memory pool release and router process's exit. Thus, a worker thread may start processing a request between these two operations, which may result in an http fields hash access and subsequent crash. To simplify issue reproduction, it makes sense to add a 2 sec sleep before exit() in nxt_runtime_exit(). --- src/nxt_controller.c | 5 +---- src/nxt_h1proto.c | 6 +++--- src/nxt_http.h | 6 +++--- src/nxt_http_parse.c | 26 ++++---------------------- src/nxt_http_parse.h | 4 ++-- src/nxt_http_request.c | 6 +++--- src/nxt_http_response.c | 4 ++-- src/nxt_router.c | 2 +- src/test/nxt_http_parse_test.c | 17 +++++------------ 9 files changed, 24 insertions(+), 52 deletions(-) diff --git a/src/nxt_controller.c b/src/nxt_controller.c index f4c3a00d..f9b2cf26 100644 --- a/src/nxt_controller.c +++ b/src/nxt_controller.c @@ -130,14 +130,11 @@ nxt_controller_start(nxt_task_t *task, void *data) nxt_mp_t *mp; nxt_int_t ret; nxt_str_t *json; - nxt_runtime_t *rt; nxt_conf_value_t *conf; nxt_conf_validation_t vldt; nxt_controller_init_t *init; - rt = task->thread->runtime; - - ret = nxt_http_fields_hash(&nxt_controller_fields_hash, rt->mem_pool, + ret = nxt_http_fields_hash(&nxt_controller_fields_hash, nxt_controller_request_fields, nxt_nitems(nxt_controller_request_fields)); diff --git a/src/nxt_h1proto.c b/src/nxt_h1proto.c index c2e65397..a139f611 100644 --- a/src/nxt_h1proto.c +++ b/src/nxt_h1proto.c @@ -186,16 +186,16 @@ static nxt_http_field_proc_t nxt_h1p_peer_fields[] = { nxt_int_t -nxt_h1p_init(nxt_task_t *task, nxt_runtime_t *rt) +nxt_h1p_init(nxt_task_t *task) { nxt_int_t ret; - ret = nxt_http_fields_hash(&nxt_h1p_fields_hash, rt->mem_pool, + ret = nxt_http_fields_hash(&nxt_h1p_fields_hash, nxt_h1p_fields, nxt_nitems(nxt_h1p_fields)); if (nxt_fast_path(ret == NXT_OK)) { ret = nxt_http_fields_hash(&nxt_h1p_peer_fields_hash, - rt->mem_pool, nxt_h1p_peer_fields, + nxt_h1p_peer_fields, nxt_nitems(nxt_h1p_peer_fields)); } diff --git a/src/nxt_http.h b/src/nxt_http.h index 638affc8..841f5b40 100644 --- a/src/nxt_http.h +++ b/src/nxt_http.h @@ -245,9 +245,9 @@ nxt_http_date(u_char *buf, struct tm *tm) } -nxt_int_t nxt_http_init(nxt_task_t *task, nxt_runtime_t *rt); -nxt_int_t nxt_h1p_init(nxt_task_t *task, nxt_runtime_t *rt); -nxt_int_t nxt_http_response_hash_init(nxt_task_t *task, nxt_runtime_t *rt); +nxt_int_t nxt_http_init(nxt_task_t *task); +nxt_int_t nxt_h1p_init(nxt_task_t *task); +nxt_int_t nxt_http_response_hash_init(nxt_task_t *task); void nxt_http_conn_init(nxt_task_t *task, void *obj, void *data); nxt_http_request_t *nxt_http_request_create(nxt_task_t *task); diff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c index 4c5d4936..22004cc1 100644 --- a/src/nxt_http_parse.c +++ b/src/nxt_http_parse.c @@ -22,8 +22,6 @@ static nxt_int_t nxt_http_parse_field_end(nxt_http_request_parse_t *rp, static nxt_int_t nxt_http_parse_complex_target(nxt_http_request_parse_t *rp); static nxt_int_t nxt_http_field_hash_test(nxt_lvlhsh_query_t *lhq, void *data); -static void *nxt_http_field_hash_alloc(void *pool, size_t size); -static void nxt_http_field_hash_free(void *pool, void *p); static nxt_int_t nxt_http_field_hash_collision(nxt_lvlhsh_query_t *lhq, void *data); @@ -1133,8 +1131,8 @@ const nxt_lvlhsh_proto_t nxt_http_fields_hash_proto nxt_aligned(64) = { NXT_LVLHSH_BUCKET_SIZE(64), { NXT_HTTP_FIELD_LVLHSH_SHIFT, 0, 0, 0, 0, 0, 0, 0 }, nxt_http_field_hash_test, - nxt_http_field_hash_alloc, - nxt_http_field_hash_free, + nxt_lvlhsh_alloc, + nxt_lvlhsh_free, }; @@ -1153,20 +1151,6 @@ nxt_http_field_hash_test(nxt_lvlhsh_query_t *lhq, void *data) } -static void * -nxt_http_field_hash_alloc(void *pool, size_t size) -{ - return nxt_mp_align(pool, size, size); -} - - -static void -nxt_http_field_hash_free(void *pool, void *p) -{ - nxt_mp_free(pool, p); -} - - static nxt_int_t nxt_http_field_hash_collision(nxt_lvlhsh_query_t *lhq, void *data) { @@ -1175,7 +1159,7 @@ nxt_http_field_hash_collision(nxt_lvlhsh_query_t *lhq, void *data) nxt_int_t -nxt_http_fields_hash(nxt_lvlhsh_t *hash, nxt_mp_t *mp, +nxt_http_fields_hash(nxt_lvlhsh_t *hash, nxt_http_field_proc_t items[], nxt_uint_t count) { u_char ch; @@ -1187,7 +1171,6 @@ nxt_http_fields_hash(nxt_lvlhsh_t *hash, nxt_mp_t *mp, lhq.replace = 0; lhq.proto = &nxt_http_fields_hash_proto; - lhq.pool = mp; for (i = 0; i < count; i++) { key = NXT_HTTP_FIELD_HASH_INIT; @@ -1214,7 +1197,7 @@ nxt_http_fields_hash(nxt_lvlhsh_t *hash, nxt_mp_t *mp, nxt_uint_t -nxt_http_fields_hash_collisions(nxt_lvlhsh_t *hash, nxt_mp_t *mp, +nxt_http_fields_hash_collisions(nxt_lvlhsh_t *hash, nxt_http_field_proc_t items[], nxt_uint_t count, nxt_bool_t level) { u_char ch; @@ -1229,7 +1212,6 @@ nxt_http_fields_hash_collisions(nxt_lvlhsh_t *hash, nxt_mp_t *mp, lhq.replace = 0; lhq.proto = &proto; - lhq.pool = mp; mask = level ? (1 << NXT_HTTP_FIELD_LVLHSH_SHIFT) - 1 : 0xFFFF; diff --git a/src/nxt_http_parse.h b/src/nxt_http_parse.h index d319c71d..0f888949 100644 --- a/src/nxt_http_parse.h +++ b/src/nxt_http_parse.h @@ -102,9 +102,9 @@ nxt_int_t nxt_http_parse_request(nxt_http_request_parse_t *rp, nxt_int_t nxt_http_parse_fields(nxt_http_request_parse_t *rp, nxt_buf_mem_t *b); -nxt_int_t nxt_http_fields_hash(nxt_lvlhsh_t *hash, nxt_mp_t *mp, +nxt_int_t nxt_http_fields_hash(nxt_lvlhsh_t *hash, nxt_http_field_proc_t items[], nxt_uint_t count); -nxt_uint_t nxt_http_fields_hash_collisions(nxt_lvlhsh_t *hash, nxt_mp_t *mp, +nxt_uint_t nxt_http_fields_hash_collisions(nxt_lvlhsh_t *hash, nxt_http_field_proc_t items[], nxt_uint_t count, nxt_bool_t level); nxt_int_t nxt_http_fields_process(nxt_list_t *fields, nxt_lvlhsh_t *hash, void *ctx); diff --git a/src/nxt_http_request.c b/src/nxt_http_request.c index 72aaa290..050587f7 100644 --- a/src/nxt_http_request.c +++ b/src/nxt_http_request.c @@ -36,17 +36,17 @@ nxt_time_string_t nxt_http_date_cache = { nxt_int_t -nxt_http_init(nxt_task_t *task, nxt_runtime_t *rt) +nxt_http_init(nxt_task_t *task) { nxt_int_t ret; - ret = nxt_h1p_init(task, rt); + ret = nxt_h1p_init(task); if (ret != NXT_OK) { return ret; } - return nxt_http_response_hash_init(task, rt); + return nxt_http_response_hash_init(task); } diff --git a/src/nxt_http_response.c b/src/nxt_http_response.c index 00ecff00..55a4686c 100644 --- a/src/nxt_http_response.c +++ b/src/nxt_http_response.c @@ -34,9 +34,9 @@ static nxt_http_field_proc_t nxt_response_fields[] = { nxt_int_t -nxt_http_response_hash_init(nxt_task_t *task, nxt_runtime_t *rt) +nxt_http_response_hash_init(nxt_task_t *task) { - return nxt_http_fields_hash(&nxt_response_fields_hash, rt->mem_pool, + return nxt_http_fields_hash(&nxt_response_fields_hash, nxt_response_fields, nxt_nitems(nxt_response_fields)); } diff --git a/src/nxt_router.c b/src/nxt_router.c index 2f4ea698..93b750a0 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -303,7 +303,7 @@ nxt_router_start(nxt_task_t *task, void *data) } #endif - ret = nxt_http_init(task, rt); + ret = nxt_http_init(task); if (nxt_slow_path(ret != NXT_OK)) { return ret; } diff --git a/src/test/nxt_http_parse_test.c b/src/test/nxt_http_parse_test.c index 8dcbc061..9630b21c 100644 --- a/src/test/nxt_http_parse_test.c +++ b/src/test/nxt_http_parse_test.c @@ -510,7 +510,7 @@ static nxt_str_t nxt_http_test_big_request = nxt_string( nxt_int_t nxt_http_parse_test(nxt_thread_t *thr) { - nxt_mp_t *mp, *mp_temp; + nxt_mp_t *mp_temp; nxt_int_t rc; nxt_uint_t i, colls, lvl_colls; nxt_lvlhsh_t hash; @@ -519,12 +519,7 @@ nxt_http_parse_test(nxt_thread_t *thr) nxt_thread_time_update(thr); - mp = nxt_mp_create(1024, 128, 256, 32); - if (mp == NULL) { - return NXT_ERROR; - } - - rc = nxt_http_fields_hash(&nxt_http_test_fields_hash, mp, + rc = nxt_http_fields_hash(&nxt_http_test_fields_hash, nxt_http_test_fields, nxt_nitems(nxt_http_test_fields)); if (rc != NXT_OK) { @@ -569,14 +564,14 @@ nxt_http_parse_test(nxt_thread_t *thr) nxt_memzero(&hash, sizeof(nxt_lvlhsh_t)); - colls = nxt_http_fields_hash_collisions(&hash, mp, + colls = nxt_http_fields_hash_collisions(&hash, nxt_http_test_bench_fields, nxt_nitems(nxt_http_test_bench_fields), 0); nxt_memzero(&hash, sizeof(nxt_lvlhsh_t)); - lvl_colls = nxt_http_fields_hash_collisions(&hash, mp, + lvl_colls = nxt_http_fields_hash_collisions(&hash, nxt_http_test_bench_fields, nxt_nitems(nxt_http_test_bench_fields), 1); @@ -587,7 +582,7 @@ nxt_http_parse_test(nxt_thread_t *thr) nxt_memzero(&hash, sizeof(nxt_lvlhsh_t)); - rc = nxt_http_fields_hash(&hash, mp, nxt_http_test_bench_fields, + rc = nxt_http_fields_hash(&hash, nxt_http_test_bench_fields, nxt_nitems(nxt_http_test_bench_fields)); if (rc != NXT_OK) { return NXT_ERROR; @@ -607,8 +602,6 @@ nxt_http_parse_test(nxt_thread_t *thr) return NXT_ERROR; } - nxt_mp_destroy(mp); - return NXT_OK; } -- cgit From e377820b6cc482bae6c4288e9020606572cd92a8 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Thu, 16 Apr 2020 17:49:09 +0300 Subject: Added version 1.17.0 CHANGES. --- CHANGES | 27 ++++++++++++++++++ docs/changes.xml | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/CHANGES b/CHANGES index c44466e6..3ffd06b7 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,31 @@ +Changes with Unit 1.17.0 16 Apr 2020 + + *) Feature: a "return" action with optional "location" for immediate + responses and external redirection. + + *) Feature: fractional weights support for upstream servers. + + *) Bugfix: accidental 502 "Bad Gateway" errors might have occurred in + applications under high load. + + *) Bugfix: memory leak in the router; the bug had appeared in 1.13.0. + + *) Bugfix: segmentation fault might have occurred in the router process + when reaching open connections limit. + + *) Bugfix: "close() failed (9: Bad file descriptor)" alerts might have + appeared in the log while processing large request bodies; the bug + had appeared in 1.16.0. + + *) Bugfix: existing application processes didn't reopen the log file. + + *) Bugfix: incompatibility with some Node.js applications. + + *) Bugfix: broken build on DragonFly BSD; the bug had appeared in + 1.16.0. + + Changes with Unit 1.16.0 12 Mar 2020 *) Feature: basic load-balancing support with round-robin. diff --git a/docs/changes.xml b/docs/changes.xml index 270b49d3..a883ccbb 100644 --- a/docs/changes.xml +++ b/docs/changes.xml @@ -5,6 +5,92 @@ + + + + +NGINX Unit updated to 1.17.0. + + + + + + + + + + +a "return" action with optional "location" for immediate responses and external +redirection. + + + + + +fractional weights support for upstream servers. + + + + + +accidental 502 "Bad Gateway" errors might have occurred in applications under +high load. + + + + + +memory leak in the router; the bug had appeared in 1.13.0. + + + + + +segmentation fault might have occurred in the router process when reaching +open connections limit. + + + + + +"close() failed (9: Bad file descriptor)" alerts might have appeared in the log +while processing large request bodies; the bug had appeared in 1.16.0. + + + + + +existing application processes didn't reopen the log file. + + + + + +incompatibility with some Node.js applications. + + + + + +broken build on DragonFly BSD; the bug had appeared in 1.16.0. + + + + + + " -ENV UNIT_VERSION 1.16.0-1~buster +ENV UNIT_VERSION 1.17.0-1~buster RUN set -x \ && apt-get update \ diff --git a/pkg/docker/Dockerfile.go1.11-dev b/pkg/docker/Dockerfile.go1.11-dev index ab9bb699..2b213836 100644 --- a/pkg/docker/Dockerfile.go1.11-dev +++ b/pkg/docker/Dockerfile.go1.11-dev @@ -2,7 +2,7 @@ FROM debian:buster-slim LABEL maintainer="NGINX Docker Maintainers " -ENV UNIT_VERSION 1.16.0-1~buster +ENV UNIT_VERSION 1.17.0-1~buster RUN set -x \ && apt-get update \ diff --git a/pkg/docker/Dockerfile.minimal b/pkg/docker/Dockerfile.minimal index 03fab2a2..af97aa4f 100644 --- a/pkg/docker/Dockerfile.minimal +++ b/pkg/docker/Dockerfile.minimal @@ -2,7 +2,7 @@ FROM debian:buster-slim LABEL maintainer="NGINX Docker Maintainers " -ENV UNIT_VERSION 1.16.0-1~buster +ENV UNIT_VERSION 1.17.0-1~buster RUN set -x \ && apt-get update \ diff --git a/pkg/docker/Dockerfile.perl5.28 b/pkg/docker/Dockerfile.perl5.28 index f9b596f2..793b48d1 100644 --- a/pkg/docker/Dockerfile.perl5.28 +++ b/pkg/docker/Dockerfile.perl5.28 @@ -2,7 +2,7 @@ FROM debian:buster-slim LABEL maintainer="NGINX Docker Maintainers " -ENV UNIT_VERSION 1.16.0-1~buster +ENV UNIT_VERSION 1.17.0-1~buster RUN set -x \ && apt-get update \ diff --git a/pkg/docker/Dockerfile.php7.3 b/pkg/docker/Dockerfile.php7.3 index e3c2bfbd..5e3f0e97 100644 --- a/pkg/docker/Dockerfile.php7.3 +++ b/pkg/docker/Dockerfile.php7.3 @@ -2,7 +2,7 @@ FROM debian:buster-slim LABEL maintainer="NGINX Docker Maintainers " -ENV UNIT_VERSION 1.16.0-1~buster +ENV UNIT_VERSION 1.17.0-1~buster RUN set -x \ && apt-get update \ diff --git a/pkg/docker/Dockerfile.python2.7 b/pkg/docker/Dockerfile.python2.7 index 065fc61b..9e3a431c 100644 --- a/pkg/docker/Dockerfile.python2.7 +++ b/pkg/docker/Dockerfile.python2.7 @@ -2,7 +2,7 @@ FROM debian:buster-slim LABEL maintainer="NGINX Docker Maintainers " -ENV UNIT_VERSION 1.16.0-1~buster +ENV UNIT_VERSION 1.17.0-1~buster RUN set -x \ && apt-get update \ diff --git a/pkg/docker/Dockerfile.python3.7 b/pkg/docker/Dockerfile.python3.7 index d80d5533..2517896b 100644 --- a/pkg/docker/Dockerfile.python3.7 +++ b/pkg/docker/Dockerfile.python3.7 @@ -2,7 +2,7 @@ FROM debian:buster-slim LABEL maintainer="NGINX Docker Maintainers " -ENV UNIT_VERSION 1.16.0-1~buster +ENV UNIT_VERSION 1.17.0-1~buster RUN set -x \ && apt-get update \ diff --git a/pkg/docker/Dockerfile.ruby2.5 b/pkg/docker/Dockerfile.ruby2.5 index 3d141335..7258bd28 100644 --- a/pkg/docker/Dockerfile.ruby2.5 +++ b/pkg/docker/Dockerfile.ruby2.5 @@ -2,7 +2,7 @@ FROM debian:buster-slim LABEL maintainer="NGINX Docker Maintainers " -ENV UNIT_VERSION 1.16.0-1~buster +ENV UNIT_VERSION 1.17.0-1~buster RUN set -x \ && apt-get update \ -- cgit From 9877087756144d3bdf343d0d4e91e1efbcc62c93 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Thu, 16 Apr 2020 18:21:09 +0300 Subject: Added tag 1.17.0 for changeset 4b13438632bc --- .hgtags | 1 + 1 file changed, 1 insertion(+) diff --git a/.hgtags b/.hgtags index ad451c6f..bae952d7 100644 --- a/.hgtags +++ b/.hgtags @@ -23,3 +23,4 @@ b391df5f0102aa6afe660cfc863729c1b1111c9e 1.12.0 6e28966ed1f26e119bf333229ea5e6686c60a469 1.14.0 801ac82f80fb2b2333f2c03ac9c3df6b7cec130a 1.15.0 8bab088952dd9d7caa3d04fd4b3026cef26fcf7d 1.16.0 +4b13438632bc37ca599113be90af64f6e2f09d83 1.17.0 -- cgit