From 2cb99cab03745e93683644482f24c549d9a9f438 Mon Sep 17 00:00:00 2001 From: Pavel Zavadski Date: Thu, 25 Jan 2024 13:47:33 +0100 Subject: [PATCH 01/19] Add unit section to status endpoint Added unit section to /status endpoint. Unit section is about web-server version, config last load time and config update generation Response example below: {"unit":{"version":"1.32.0","load_time":"2024-01-25T13:24:08.000Z","generation":0},"connections":{"accepted":0,"active":0,"idle":0,"closed":0},"requests":{"total":0},"applications":{"laravel":{"processes":{"running":1,"starting":0,"idle":1},"requests":{"active":0}}}} Closes: https://github.com/nginx/unit/issues/928 --- src/nxt_controller.c | 18 +++++++++++++++--- src/nxt_runtime.c | 13 +++++++++++++ src/nxt_runtime.h | 3 +++ src/nxt_status.c | 32 ++++++++++++++++++++++++++------ src/nxt_status.h | 2 +- 5 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/nxt_controller.c b/src/nxt_controller.c index eb814321d..0980598f6 100644 --- a/src/nxt_controller.c +++ b/src/nxt_controller.c @@ -11,8 +11,6 @@ #include #include #include -#include - typedef struct { nxt_conf_value_t *root; @@ -1633,7 +1631,7 @@ nxt_controller_status_handler(nxt_task_t *task, nxt_port_recv_msg_t *msg, req = data; if (msg->port_msg.type == NXT_PORT_MSG_RPC_READY) { - status = nxt_status_get((nxt_status_report_t *) msg->buf->mem.pos, + status = nxt_status_get(task, (nxt_status_report_t *) msg->buf->mem.pos, req->conn->mem_pool); } else { status = NULL; @@ -2432,6 +2430,9 @@ nxt_controller_conf_store(nxt_task_t *task, nxt_conf_value_t *conf) nxt_buf_t *b; nxt_port_t *main_port; nxt_runtime_t *rt; + time_t rawtime; + u_char buffer[25]; + struct tm *timeinfo; rt = task->thread->runtime; @@ -2466,6 +2467,17 @@ nxt_controller_conf_store(nxt_task_t *task, nxt_conf_value_t *conf) NXT_PORT_MSG_CONF_STORE | NXT_PORT_MSG_CLOSE_FD, fd, 0, -1, b); + rt->conf_gen++; + + time(&rawtime); + timeinfo = gmtime(&rawtime); //convert to UTC + strftime((char*)buffer, 25, "%Y-%m-%dT%H:%M:%S.000Z", timeinfo); + + rt->conf_ltime.length = nxt_strlen(buffer); + rt->conf_ltime.start = nxt_malloc(rt->conf_ltime.length + 1); + + nxt_cpystr(rt->conf_ltime.start, buffer); + return; fail: diff --git a/src/nxt_runtime.c b/src/nxt_runtime.c index 9bfabc750..5d2e4b48b 100644 --- a/src/nxt_runtime.c +++ b/src/nxt_runtime.c @@ -777,6 +777,9 @@ nxt_runtime_conf_init(nxt_task_t *task, nxt_runtime_t *rt) nxt_sockaddr_t *sa; nxt_file_name_str_t file_name; const nxt_event_interface_t *interface; + time_t rawtime; + u_char buffer[25]; + struct tm *timeinfo; rt->daemon = 1; rt->engine_connections = 256; @@ -789,6 +792,16 @@ nxt_runtime_conf_init(nxt_task_t *task, nxt_runtime_t *rt) rt->state = NXT_STATEDIR; rt->control = NXT_CONTROL_SOCK; rt->tmp = NXT_TMPDIR; + rt->conf_gen = 0; + + time(&rawtime); + timeinfo = gmtime(&rawtime); //convert to UTC + strftime((char*)buffer, 25, "%Y-%m-%dT%H:%M:%S.000Z", timeinfo); + + rt->conf_ltime.length = nxt_strlen(buffer); + rt->conf_ltime.start = nxt_malloc(rt->conf_ltime.length + 1); + + nxt_cpystr(rt->conf_ltime.start, buffer); nxt_memzero(&rt->capabilities, sizeof(nxt_capabilities_t)); diff --git a/src/nxt_runtime.h b/src/nxt_runtime.h index 66ec0106c..a3397bc82 100644 --- a/src/nxt_runtime.h +++ b/src/nxt_runtime.h @@ -73,6 +73,9 @@ struct nxt_runtime_s { const char *control; const char *tmp; + nxt_int_t conf_gen; + nxt_str_t conf_ltime; + nxt_str_t certs; nxt_str_t scripts; diff --git a/src/nxt_status.c b/src/nxt_status.c index f8002e86e..e68a05a4b 100644 --- a/src/nxt_status.c +++ b/src/nxt_status.c @@ -9,14 +9,20 @@ nxt_conf_value_t * -nxt_status_get(nxt_status_report_t *report, nxt_mp_t *mp) +nxt_status_get(nxt_task_t *task, nxt_status_report_t *report, nxt_mp_t *mp) { size_t i; - nxt_str_t name; + nxt_runtime_t *rt; nxt_int_t ret; + nxt_str_t name; nxt_status_app_t *app; + nxt_str_t version; nxt_conf_value_t *status, *obj, *apps, *app_obj; + static nxt_str_t unit_str = nxt_string("unit"); + static nxt_str_t ver_str = nxt_string("version"); + static nxt_str_t gen_str = nxt_string("generation"); + static nxt_str_t lconf_str = nxt_string("last_configured"); static nxt_str_t conns_str = nxt_string("connections"); static nxt_str_t acc_str = nxt_string("accepted"); static nxt_str_t active_str = nxt_string("active"); @@ -29,17 +35,31 @@ nxt_status_get(nxt_status_report_t *report, nxt_mp_t *mp) static nxt_str_t run_str = nxt_string("running"); static nxt_str_t start_str = nxt_string("starting"); - status = nxt_conf_create_object(mp, 3); + rt = task->thread->runtime; + + status = nxt_conf_create_object(mp, 4); if (nxt_slow_path(status == NULL)) { return NULL; } + obj = nxt_conf_create_object(mp, 3); + if (nxt_slow_path(obj == NULL)) { + return NULL; + } + + nxt_conf_set_member(status, &unit_str, obj, 0); + + nxt_str_set(&version, NXT_VERSION); + nxt_conf_set_member_string(obj, &ver_str, &version, 0); + nxt_conf_set_member_string(obj, &lconf_str, &rt->conf_ltime, 1); + nxt_conf_set_member_integer(obj, &gen_str, rt->conf_gen, 2); + obj = nxt_conf_create_object(mp, 4); if (nxt_slow_path(obj == NULL)) { return NULL; } - nxt_conf_set_member(status, &conns_str, obj, 0); + nxt_conf_set_member(status, &conns_str, obj, 1); nxt_conf_set_member_integer(obj, &acc_str, report->accepted_conns, 0); nxt_conf_set_member_integer(obj, &active_str, report->accepted_conns @@ -53,7 +73,7 @@ nxt_status_get(nxt_status_report_t *report, nxt_mp_t *mp) return NULL; } - nxt_conf_set_member(status, &reqs_str, obj, 1); + nxt_conf_set_member(status, &reqs_str, obj, 2); nxt_conf_set_member_integer(obj, &total_str, report->requests, 0); @@ -62,7 +82,7 @@ nxt_status_get(nxt_status_report_t *report, nxt_mp_t *mp) return NULL; } - nxt_conf_set_member(status, &apps_str, apps, 2); + nxt_conf_set_member(status, &apps_str, apps, 3); for (i = 0; i < report->apps_count; i++) { app = &report->apps[i]; diff --git a/src/nxt_status.h b/src/nxt_status.h index a99ac7d0e..32a902fde 100644 --- a/src/nxt_status.h +++ b/src/nxt_status.h @@ -27,7 +27,7 @@ typedef struct { } nxt_status_report_t; -nxt_conf_value_t *nxt_status_get(nxt_status_report_t *report, nxt_mp_t *mp); +nxt_conf_value_t *nxt_status_get(nxt_task_t *task, nxt_status_report_t *report, nxt_mp_t *mp); #endif /* _NXT_STATUS_H_INCLUDED_ */ From 6452ca111c71188ab2813c763e6a0e86b48fbd56 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Thu, 25 Jan 2024 12:49:47 +0000 Subject: [PATCH 02/19] Node.js: fixed "httpVersion" variable format According to the Node.js documenation this variable should only include numbering scheme. Thanks to @dbit-xia. Closes: https://github.com/nginx/unit/issues/1085 --- docs/changes.xml | 7 +++++++ src/nodejs/unit-http/unit.cpp | 8 +++++++- test/test_node_application.py | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/changes.xml b/docs/changes.xml index 6b1aaf712..f226e4f4d 100644 --- a/docs/changes.xml +++ b/docs/changes.xml @@ -40,6 +40,13 @@ with Next.js. + + +ServerRequest.httpVersion variable format in Node.js module. + + + + diff --git a/src/nodejs/unit-http/unit.cpp b/src/nodejs/unit-http/unit.cpp index 7912d0acf..7d9395bb6 100644 --- a/src/nodejs/unit-http/unit.cpp +++ b/src/nodejs/unit-http/unit.cpp @@ -581,6 +581,7 @@ Unit::get_server_object() void Unit::create_headers(nxt_unit_request_info_t *req, napi_value request) { + char *p; uint32_t i; napi_value headers, raw_headers; napi_status status; @@ -602,7 +603,12 @@ Unit::create_headers(nxt_unit_request_info_t *req, napi_value request) set_named_property(request, "headers", headers); set_named_property(request, "rawHeaders", raw_headers); - set_named_property(request, "httpVersion", r->version, r->version_length); + + // trim the "HTTP/" protocol prefix + p = (char *) nxt_unit_sptr_get(&r->version); + p += 5; + + set_named_property(request, "httpVersion", create_string_latin1(p, 3)); set_named_property(request, "method", r->method, r->method_length); set_named_property(request, "url", r->target, r->target_length); diff --git a/test/test_node_application.py b/test/test_node_application.py index aaad2bcf3..cb7752105 100644 --- a/test/test_node_application.py +++ b/test/test_node_application.py @@ -80,7 +80,7 @@ def test_node_application_variables(date_to_sec_epoch, sec_epoch): 'Request-Method': 'POST', 'Request-Uri': '/', 'Http-Host': 'localhost', - 'Server-Protocol': 'HTTP/1.1', + 'Server-Protocol': '1.1', 'Custom-Header': 'blah', }, 'headers' assert resp['body'] == body, 'body' From 37abe2e4633d66241bf1766a740d2b2734c132fa Mon Sep 17 00:00:00 2001 From: Zhidao HONG Date: Tue, 23 Jan 2024 18:45:27 +0800 Subject: [PATCH 03/19] HTTP: refactored out nxt_http_request_access_log(). This is in preparation for adding conditional access logging. No functional changes. --- src/nxt_http_request.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/nxt_http_request.c b/src/nxt_http_request.c index e532baffa..2e39e4e88 100644 --- a/src/nxt_http_request.c +++ b/src/nxt_http_request.c @@ -24,6 +24,8 @@ static void nxt_http_request_proto_info(nxt_task_t *task, static void nxt_http_request_mem_buf_completion(nxt_task_t *task, void *obj, void *data); static void nxt_http_request_done(nxt_task_t *task, void *obj, void *data); +static void nxt_http_request_access_log(nxt_task_t *task, nxt_http_request_t *r, + nxt_router_conf_t *rtcf); static u_char *nxt_http_date_cache_handler(u_char *buf, nxt_realtime_t *now, struct tm *tm, size_t size, const char *format); @@ -816,26 +818,23 @@ nxt_http_request_error_handler(nxt_task_t *task, void *obj, void *data) void nxt_http_request_close_handler(nxt_task_t *task, void *obj, void *data) { - nxt_tstr_t *log_format; nxt_http_proto_t proto; + nxt_router_conf_t *rtcf; nxt_http_request_t *r; nxt_http_protocol_t protocol; nxt_socket_conf_joint_t *conf; - nxt_router_access_log_t *access_log; r = obj; proto.any = data; conf = r->conf; + rtcf = conf->socket_conf->router_conf; if (!r->logged) { r->logged = 1; - access_log = conf->socket_conf->router_conf->access_log; - log_format = conf->socket_conf->router_conf->log_format; - - if (access_log != NULL) { - access_log->handler(task, r, access_log, log_format); + if (rtcf->access_log != NULL) { + nxt_http_request_access_log(task, r, rtcf); return; } } @@ -866,6 +865,18 @@ nxt_http_request_close_handler(nxt_task_t *task, void *obj, void *data) } +static void +nxt_http_request_access_log(nxt_task_t *task, nxt_http_request_t *r, + nxt_router_conf_t *rtcf) +{ + nxt_router_access_log_t *access_log; + + access_log = rtcf->access_log; + + access_log->handler(task, r, access_log, rtcf->log_format); +} + + static u_char * nxt_http_date_cache_handler(u_char *buf, nxt_realtime_t *now, struct tm *tm, size_t size, const char *format) From 4c91bebb50d06b28e369d68b23022caa072cf62d Mon Sep 17 00:00:00 2001 From: Zhidao HONG Date: Tue, 23 Jan 2024 18:57:30 +0800 Subject: [PATCH 04/19] HTTP: enhanced access log with conditional filtering. This feature allows users to specify conditions to control if access log should be recorded. The "if" option supports a string and JavaScript code. If its value is empty, 0, false, null, or undefined, the logs will not be recorded. And the '!' as a prefix inverses the condition. Example 1: Only log requests that sent a session cookie. { "access_log": { "if": "$cookie_session", "path": "..." } } Example 2: Do not log health check requests. { "access_log": { "if": "`${uri == '/health' ? false : true}`", "path": "..." } } Example 3: Only log requests when the time is before 22:00. { "access_log": { "if": "`${new Date().getHours() < 22}`", "path": "..." } } or { "access_log": { "if": "!`${new Date().getHours() >= 22}`", "path": "..." } } Closes: https://github.com/nginx/unit/issues/594 --- src/nxt_conf_validation.c | 37 +++++++++++++++++++++++++ src/nxt_http_request.c | 54 ++++++++++++++++++++++++++++++++----- src/nxt_router.h | 2 ++ src/nxt_router_access_log.c | 25 +++++++++++++++++ 4 files changed, 112 insertions(+), 6 deletions(-) diff --git a/src/nxt_conf_validation.c b/src/nxt_conf_validation.c index 32ed4ffde..32124ee9b 100644 --- a/src/nxt_conf_validation.c +++ b/src/nxt_conf_validation.c @@ -77,6 +77,8 @@ static nxt_int_t nxt_conf_vldt_error(nxt_conf_validation_t *vldt, const char *fmt, ...); static nxt_int_t nxt_conf_vldt_var(nxt_conf_validation_t *vldt, nxt_str_t *name, nxt_str_t *value); +static nxt_int_t nxt_conf_vldt_if(nxt_conf_validation_t *vldt, + nxt_conf_value_t *value, void *data); nxt_inline nxt_int_t nxt_conf_vldt_unsupported(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, void *data) NXT_MAYBE_UNUSED; @@ -1369,6 +1371,10 @@ static nxt_conf_vldt_object_t nxt_conf_vldt_access_log_members[] = { }, { .name = nxt_string("format"), .type = NXT_CONF_VLDT_STRING, + }, { + .name = nxt_string("if"), + .type = NXT_CONF_VLDT_STRING, + .validator = nxt_conf_vldt_if, }, NXT_CONF_VLDT_END @@ -1538,6 +1544,37 @@ nxt_conf_vldt_var(nxt_conf_validation_t *vldt, nxt_str_t *name, } +static nxt_int_t +nxt_conf_vldt_if(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, + void *data) +{ + nxt_str_t str; + + static nxt_str_t if_str = nxt_string("if"); + + if (nxt_conf_type(value) != NXT_CONF_STRING) { + return nxt_conf_vldt_error(vldt, "The \"if\" must be a string"); + } + + nxt_conf_get_string(value, &str); + + if (str.length == 0) { + return NXT_OK; + } + + if (str.start[0] == '!') { + str.start++; + str.length--; + } + + if (nxt_is_tstr(&str)) { + return nxt_conf_vldt_var(vldt, &if_str, &str); + } + + return NXT_OK; +} + + typedef struct { nxt_mp_t *pool; nxt_str_t *type; diff --git a/src/nxt_http_request.c b/src/nxt_http_request.c index 2e39e4e88..f8d8d8877 100644 --- a/src/nxt_http_request.c +++ b/src/nxt_http_request.c @@ -24,8 +24,8 @@ static void nxt_http_request_proto_info(nxt_task_t *task, static void nxt_http_request_mem_buf_completion(nxt_task_t *task, void *obj, void *data); static void nxt_http_request_done(nxt_task_t *task, void *obj, void *data); -static void nxt_http_request_access_log(nxt_task_t *task, nxt_http_request_t *r, - nxt_router_conf_t *rtcf); +static nxt_int_t nxt_http_request_access_log(nxt_task_t *task, + nxt_http_request_t *r, nxt_router_conf_t *rtcf); static u_char *nxt_http_date_cache_handler(u_char *buf, nxt_realtime_t *now, struct tm *tm, size_t size, const char *format); @@ -818,6 +818,7 @@ nxt_http_request_error_handler(nxt_task_t *task, void *obj, void *data) void nxt_http_request_close_handler(nxt_task_t *task, void *obj, void *data) { + nxt_int_t ret; nxt_http_proto_t proto; nxt_router_conf_t *rtcf; nxt_http_request_t *r; @@ -834,8 +835,10 @@ nxt_http_request_close_handler(nxt_task_t *task, void *obj, void *data) r->logged = 1; if (rtcf->access_log != NULL) { - nxt_http_request_access_log(task, r, rtcf); - return; + ret = nxt_http_request_access_log(task, r, rtcf); + if (ret == NXT_OK) { + return; + } } } @@ -865,15 +868,54 @@ nxt_http_request_close_handler(nxt_task_t *task, void *obj, void *data) } -static void +static nxt_int_t nxt_http_request_access_log(nxt_task_t *task, nxt_http_request_t *r, nxt_router_conf_t *rtcf) { + nxt_int_t ret; + nxt_str_t str; + nxt_bool_t expr; nxt_router_access_log_t *access_log; access_log = rtcf->access_log; - access_log->handler(task, r, access_log, rtcf->log_format); + expr = 1; + + if (rtcf->log_expr != NULL) { + + if (nxt_tstr_is_const(rtcf->log_expr)) { + nxt_tstr_str(rtcf->log_expr, &str); + + } else { + ret = nxt_tstr_query_init(&r->tstr_query, rtcf->tstr_state, + &r->tstr_cache, r, r->mem_pool); + if (nxt_slow_path(ret != NXT_OK)) { + return NXT_DECLINED; + } + + nxt_tstr_query(task, r->tstr_query, rtcf->log_expr, &str); + + if (nxt_slow_path(nxt_tstr_query_failed(r->tstr_query))) { + return NXT_DECLINED; + } + } + + if (str.length == 0 + || nxt_str_eq(&str, "0", 1) + || nxt_str_eq(&str, "false", 5) + || nxt_str_eq(&str, "null", 4) + || nxt_str_eq(&str, "undefined", 9)) + { + expr = 0; + } + } + + if (rtcf->log_negate ^ expr) { + access_log->handler(task, r, access_log, rtcf->log_format); + return NXT_OK; + } + + return NXT_DECLINED; } diff --git a/src/nxt_router.h b/src/nxt_router.h index b14f84101..3e523001c 100644 --- a/src/nxt_router.h +++ b/src/nxt_router.h @@ -54,6 +54,8 @@ typedef struct { nxt_router_access_log_t *access_log; nxt_tstr_t *log_format; + nxt_tstr_t *log_expr; + uint8_t log_negate; /* 1 bit */ } nxt_router_conf_t; diff --git a/src/nxt_router_access_log.c b/src/nxt_router_access_log.c index ccbddb966..7fc599724 100644 --- a/src/nxt_router_access_log.c +++ b/src/nxt_router_access_log.c @@ -13,6 +13,7 @@ typedef struct { nxt_str_t path; nxt_str_t format; + nxt_conf_value_t *expr; } nxt_router_access_log_conf_t; @@ -53,6 +54,12 @@ static nxt_conf_map_t nxt_router_access_log_conf[] = { NXT_CONF_MAP_STR, offsetof(nxt_router_access_log_conf_t, format), }, + + { + nxt_string("if"), + NXT_CONF_MAP_PTR, + offsetof(nxt_router_access_log_conf_t, expr), + }, }; @@ -72,6 +79,8 @@ nxt_router_access_log_create(nxt_task_t *task, nxt_router_conf_t *rtcf, "[$time_local] \"$request_line\" $status $body_bytes_sent " "\"$header_referer\" \"$header_user_agent\""); + nxt_memzero(&alcf, sizeof(nxt_router_access_log_conf_t)); + alcf.format = log_format_str; if (nxt_conf_type(value) == NXT_CONF_STRING) { @@ -133,6 +142,22 @@ nxt_router_access_log_create(nxt_task_t *task, nxt_router_conf_t *rtcf, rtcf->access_log = access_log; rtcf->log_format = format; + if (alcf.expr != NULL) { + nxt_conf_get_string(alcf.expr, &str); + + if (str.length > 0 && str.start[0] == '!') { + rtcf->log_negate = 1; + + str.start++; + str.length--; + } + + rtcf->log_expr = nxt_tstr_compile(rtcf->tstr_state, &str, 0); + if (nxt_slow_path(rtcf->log_expr == NULL)) { + return NXT_ERROR; + } + } + return NXT_OK; } From dcbff27d9bc88c9e2049c52a441fa67d36ca7efc Mon Sep 17 00:00:00 2001 From: Zhidao HONG Date: Mon, 29 Jan 2024 20:07:53 +0800 Subject: [PATCH 05/19] Docs: Update changes.xml for conditional access logging --- docs/changes.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/changes.xml b/docs/changes.xml index f226e4f4d..508678569 100644 --- a/docs/changes.xml +++ b/docs/changes.xml @@ -26,6 +26,12 @@ NGINX Unit updated to 1.32.0. + + +conditional access logging. + + + $request_id variable contains a string that is formed using random data and From ad3645074e368e7277fa2c25d8f87ebd1f522e87 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Wed, 24 Jan 2024 16:09:41 +0000 Subject: [PATCH 06/19] Tests: "if" option in access logging. Conditional access logging was introduced here: https://github.com/nginx/unit/commit/4c91bebb50d06b28e369d68b23022caa072cf62d --- test/test_access_log.py | 62 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/test/test_access_log.py b/test/test_access_log.py index 873c941a5..1b0ec8ad1 100644 --- a/test/test_access_log.py +++ b/test/test_access_log.py @@ -28,6 +28,10 @@ def set_format(log_format): ), 'access_log format' +def set_if(condition): + assert 'success' in client.conf(f'"{condition}"', 'access_log/if') + + def test_access_log_keepalive(wait_for_record): load('mirror') @@ -308,6 +312,62 @@ def test_access_log_variables(wait_for_record): ), '$body_bytes_sent' +def test_access_log_if(search_in_file, wait_for_record): + load('empty') + set_format('$uri') + + def try_if(condition): + set_if(condition) + assert client.get(url=f'/{condition}')['status'] == 200 + + # const + + try_if('') + try_if('0') + try_if('false') + try_if('undefined') + try_if('!') + try_if('!null') + try_if('1') + + # variable + + set_if('$arg_foo') + assert client.get(url='/bar?bar')['status'] == 200 + assert client.get(url='/foo_empty?foo')['status'] == 200 + assert client.get(url='/foo?foo=1')['status'] == 200 + + # check results + + assert wait_for_record(r'^/foo$', 'access.log') is not None + + assert search_in_file(r'^/$', 'access.log') is None + assert search_in_file(r'^/0$', 'access.log') is None + assert search_in_file(r'^/false$', 'access.log') is None + assert search_in_file(r'^/undefined$', 'access.log') is None + assert search_in_file(r'^/!$', 'access.log') is not None + assert search_in_file(r'^/!null$', 'access.log') is not None + assert search_in_file(r'^/1$', 'access.log') is not None + + assert search_in_file(r'^/bar$', 'access.log') is None + assert search_in_file(r'^/foo_empty$', 'access.log') is None + + +def test_access_log_if_njs(require, search_in_file, wait_for_record): + require({'modules': {'njs': 'any'}}) + + load('empty') + set_format('$uri') + + set_if('`${args.foo == \'1\'}`') + + assert client.get(url='/foo_2?foo=2')['status'] == 200 + assert client.get(url='/foo_1?foo=1')['status'] == 200 + + assert wait_for_record(r'^/foo_1$', 'access.log') is not None + assert search_in_file(r'^/foo_2$', 'access.log') is None + + def test_access_log_incorrect(temp_dir, skip_alert): skip_alert(r'failed to apply new conf') @@ -323,3 +383,5 @@ def test_access_log_incorrect(temp_dir, skip_alert): }, 'access_log', ), 'access_log format incorrect' + + assert 'error' in client.conf('$arg_', 'access_log/if') From 9919b50aecb196ff9e005ab3d13689bc011233a7 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Wed, 24 Jan 2024 17:21:53 +0000 Subject: [PATCH 07/19] Isolation: Add a new nxt_cred_t type This is a generic type to represent a uid_t/gid_t on Linux when user namespaces are in use. Technically this only needs to be an unsigned int, but we make it an int64_t so we can make use of the existing NXT_CONF_MAP_INT64 type. This will be used in subsequent commits. Reviewed-by: Zhidao Hong Signed-off-by: Andrew Clayton --- src/nxt_clone.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/nxt_clone.h b/src/nxt_clone.h index 6cea1bd7e..1677dc776 100644 --- a/src/nxt_clone.h +++ b/src/nxt_clone.h @@ -9,6 +9,8 @@ #if (NXT_HAVE_CLONE_NEWUSER) +typedef int64_t nxt_cred_t; + typedef struct { nxt_int_t container; nxt_int_t host; From f7c9d3a8b3dbe083007e73c8c7b7e50094bf3763 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Wed, 24 Jan 2024 18:01:49 +0000 Subject: [PATCH 08/19] Isolation: Use an appropriate type for storing uid/gids Andrei reported an issue on arm64 where he was seeing the following error message when running the tests 2024/01/17 18:32:31.109 [error] 54904#54904 "gidmap" field has an entry with "size": 1, but for unprivileged unit it must be 1. This error message is guarded by the following if statement if (nxt_slow_path(m.size > 1) Turns out size was indeed > 1, in this case it was 289356276058554369, m.size is defined as a nxt_int_t, which on arm64 is actually 8 bytes, but was being printed as a signed int (4 bytes) and by chance/undefined behaviour comes out as 1. But why is size so big? In this case it should have just been 1 with a config of 'gidmap': [{'container': 0, 'host': os.getegid(), 'size': 1}], This is due to nxt_int_t being 64bits on arm64 but using a conf type of NXT_CONF_MAP_INT which means in nxt_conf_map_object() we would do (using our m.size variable as an example) ptr = nxt_pointer_to(data, map[i].offset); ... ptr->i = num; Where ptr is a union pointer and is now pointing at our m.size Next we set m.size to the value of num (which is 1 in this case), via ptr->i where i is a member of that union of type int. So here we are setting a 64bit memory location (nxt_int_t on arm64) through a 32bit (int) union alias, this means we are only setting the lower half (4) of the bytes. Whatever happens to be in the upper 4 bytes will remain, giving us our exceptionally large value. This is demonstrated by this program #include #include int main(void) { int64_t num = -1; /* All 1's in two's complement */ union { int32_t i32; int64_t i64; } *ptr; ptr = (void *)# ptr->i32 = 1; printf("num : %lu / %ld\n", num, num); ptr->i64 = 1; printf("num : %ld\n", num); return 0; } $ make union-32-64-issue cc union-32-64-issue.c -o union-32-64-issue $ ./union-32-64-issue num : 18446744069414584321 / -4294967295 num : 1 However that is not the only issue, because the members of nxt_clone_map_entry_t were specified as nxt_int_t's on the likes of x86_64 this would be a 32bit signed integer. However uid/gids on Linux at least are defined as unsigned integers, so a nxt_int_t would not be big enough to hold all potential values. We could make the nxt_uint_t's but then we're back to the above union aliasing problem. We could just set the memory for these variables to 0 and that would work, however that's really just papering over the problem. The right thing is to use a large enough sized type to store these things, hence the previously introduced nxt_cred_t. This is an int64_t which is plenty large enough. So we switch the nxt_clone_map_entry_t structure members over to nxt_cred_t's and use NXT_CONF_MAP_INT64 as the conf type, which then uses the right sized union member in nxt_conf_map_object() to set these variables. Reported-by: Andrei Zeliankou Reviewed-by: Zhidao Hong Signed-off-by: Andrew Clayton --- src/nxt_clone.c | 6 +++--- src/nxt_clone.h | 6 +++--- src/nxt_isolation.c | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/nxt_clone.c b/src/nxt_clone.c index 305f4261e..e78a7822a 100644 --- a/src/nxt_clone.c +++ b/src/nxt_clone.c @@ -143,7 +143,7 @@ nxt_clone_credential_map_set(nxt_task_t *task, const char* mapfile, pid_t pid, end = mapinfo + len; for (i = 0; i < map->size; i++) { - p = nxt_sprintf(p, end, "%d %d %d", map->map[i].container, + p = nxt_sprintf(p, end, "%L %L %L", map->map[i].container, map->map[i].host, map->map[i].size); if (nxt_slow_path(p == end)) { @@ -332,7 +332,7 @@ nxt_clone_vldt_credential_gidmap(nxt_task_t *task, if (nxt_slow_path((nxt_gid_t) m.host != nxt_egid)) { nxt_log(task, NXT_LOG_ERR, "\"gidmap\" field has an entry for " - "host gid %d but unprivileged unit can only map itself " + "host gid %L but unprivileged unit can only map itself " "(gid %d) into child namespaces.", m.host, nxt_egid); return NXT_ERROR; @@ -340,7 +340,7 @@ nxt_clone_vldt_credential_gidmap(nxt_task_t *task, if (nxt_slow_path(m.size > 1)) { nxt_log(task, NXT_LOG_ERR, "\"gidmap\" field has an entry with " - "\"size\": %d, but for unprivileged unit it must be 1.", + "\"size\": %L, but for unprivileged unit it must be 1.", m.size); return NXT_ERROR; diff --git a/src/nxt_clone.h b/src/nxt_clone.h index 1677dc776..bf28322ff 100644 --- a/src/nxt_clone.h +++ b/src/nxt_clone.h @@ -12,9 +12,9 @@ typedef int64_t nxt_cred_t; typedef struct { - nxt_int_t container; - nxt_int_t host; - nxt_int_t size; + nxt_cred_t container; + nxt_cred_t host; + nxt_cred_t size; } nxt_clone_map_entry_t; typedef struct { diff --git a/src/nxt_isolation.c b/src/nxt_isolation.c index cfa494a8f..ed5e0d76a 100644 --- a/src/nxt_isolation.c +++ b/src/nxt_isolation.c @@ -326,19 +326,19 @@ nxt_isolation_credential_map(nxt_task_t *task, nxt_mp_t *mp, static nxt_conf_map_t nxt_clone_map_entry_conf[] = { { nxt_string("container"), - NXT_CONF_MAP_INT, + NXT_CONF_MAP_INT64, offsetof(nxt_clone_map_entry_t, container), }, { nxt_string("host"), - NXT_CONF_MAP_INT, + NXT_CONF_MAP_INT64, offsetof(nxt_clone_map_entry_t, host), }, { nxt_string("size"), - NXT_CONF_MAP_INT, + NXT_CONF_MAP_INT64, offsetof(nxt_clone_map_entry_t, size), }, }; From eba7378d4f8816799032a0c086ab54d3c15157b3 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Wed, 24 Jan 2024 22:03:12 +0000 Subject: [PATCH 09/19] Configuration: Use the NXT_CONF_VLDT_REQUIRED flag for procmap Use the NXT_CONF_VLDT_REQUIRED flag on the app_procmap members. These three settings are required. These are for the uidmap & gidmap settings in the config. Suggested-by: Zhidao HONG Reviewed-by: Zhidao Hong Signed-off-by: Andrew Clayton --- src/nxt_conf_validation.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/nxt_conf_validation.c b/src/nxt_conf_validation.c index 32124ee9b..eb7ef530c 100644 --- a/src/nxt_conf_validation.c +++ b/src/nxt_conf_validation.c @@ -1327,12 +1327,15 @@ static nxt_conf_vldt_object_t nxt_conf_vldt_app_procmap_members[] = { { .name = nxt_string("container"), .type = NXT_CONF_VLDT_INTEGER, + .flags = NXT_CONF_VLDT_REQUIRED, }, { .name = nxt_string("host"), .type = NXT_CONF_VLDT_INTEGER, + .flags = NXT_CONF_VLDT_REQUIRED, }, { .name = nxt_string("size"), .type = NXT_CONF_VLDT_INTEGER, + .flags = NXT_CONF_VLDT_REQUIRED, }, NXT_CONF_VLDT_END From 990fbe7010526bb97f2d414db866050ef5e8f244 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Wed, 24 Jan 2024 22:17:02 +0000 Subject: [PATCH 10/19] Configuration: Remove procmap validation code With the previous commit which introduced the use of the NXT_CONF_VLDT_REQUIRED flag, we no longer need to do this separate validation, it's only purpose was to check if the three uidmap/gidmap settings had been provided. Reviewed-by: Zhidao Hong Signed-off-by: Andrew Clayton --- src/nxt_conf_validation.c | 73 ++------------------------------------- 1 file changed, 2 insertions(+), 71 deletions(-) diff --git a/src/nxt_conf_validation.c b/src/nxt_conf_validation.c index eb7ef530c..c843b2654 100644 --- a/src/nxt_conf_validation.c +++ b/src/nxt_conf_validation.c @@ -218,8 +218,6 @@ static nxt_int_t nxt_conf_vldt_clone_namespaces(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, void *data); #if (NXT_HAVE_CLONE_NEWUSER) -static nxt_int_t nxt_conf_vldt_clone_procmap(nxt_conf_validation_t *vldt, - const char* mapfile, nxt_conf_value_t *value); static nxt_int_t nxt_conf_vldt_clone_uidmap(nxt_conf_validation_t *vldt, nxt_conf_value_t *value); static nxt_int_t nxt_conf_vldt_clone_gidmap(nxt_conf_validation_t *vldt, @@ -3091,73 +3089,6 @@ nxt_conf_vldt_isolation(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, #if (NXT_HAVE_CLONE_NEWUSER) -typedef struct { - nxt_int_t container; - nxt_int_t host; - nxt_int_t size; -} nxt_conf_vldt_clone_procmap_conf_t; - - -static nxt_conf_map_t nxt_conf_vldt_clone_procmap_conf_map[] = { - { - nxt_string("container"), - NXT_CONF_MAP_INT32, - offsetof(nxt_conf_vldt_clone_procmap_conf_t, container), - }, - - { - nxt_string("host"), - NXT_CONF_MAP_INT32, - offsetof(nxt_conf_vldt_clone_procmap_conf_t, host), - }, - - { - nxt_string("size"), - NXT_CONF_MAP_INT32, - offsetof(nxt_conf_vldt_clone_procmap_conf_t, size), - }, - -}; - - -static nxt_int_t -nxt_conf_vldt_clone_procmap(nxt_conf_validation_t *vldt, const char *mapfile, - nxt_conf_value_t *value) -{ - nxt_int_t ret; - nxt_conf_vldt_clone_procmap_conf_t procmap; - - procmap.container = -1; - procmap.host = -1; - procmap.size = -1; - - ret = nxt_conf_map_object(vldt->pool, value, - nxt_conf_vldt_clone_procmap_conf_map, - nxt_nitems(nxt_conf_vldt_clone_procmap_conf_map), - &procmap); - if (ret != NXT_OK) { - return ret; - } - - if (procmap.container == -1) { - return nxt_conf_vldt_error(vldt, "The %s requires the " - "\"container\" field set.", mapfile); - } - - if (procmap.host == -1) { - return nxt_conf_vldt_error(vldt, "The %s requires the " - "\"host\" field set.", mapfile); - } - - if (procmap.size == -1) { - return nxt_conf_vldt_error(vldt, "The %s requires the " - "\"size\" field set.", mapfile); - } - - return NXT_OK; -} - - static nxt_int_t nxt_conf_vldt_clone_uidmap(nxt_conf_validation_t *vldt, nxt_conf_value_t *value) { @@ -3174,7 +3105,7 @@ nxt_conf_vldt_clone_uidmap(nxt_conf_validation_t *vldt, nxt_conf_value_t *value) return ret; } - return nxt_conf_vldt_clone_procmap(vldt, "uid_map", value); + return NXT_OK; } @@ -3194,7 +3125,7 @@ nxt_conf_vldt_clone_gidmap(nxt_conf_validation_t *vldt, nxt_conf_value_t *value) return ret; } - return nxt_conf_vldt_clone_procmap(vldt, "gid_map", value); + return NXT_OK; } #endif From ecd573924f5dc31e279f12249fb44e8d00e144a2 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Fri, 2 Feb 2024 23:29:48 +0100 Subject: [PATCH 11/19] Configuration: Add nxt_conf_get_string_dup() This function is like nxt_conf_get_string(), but creates a new copy, so that it can be modified without corrupting the configuration string. Reviewed-by: Andrew Clayton Cc: Zhidao Hong Signed-off-by: Alejandro Colomar --- src/nxt_conf.c | 11 +++++++++++ src/nxt_conf.h | 3 +++ 2 files changed, 14 insertions(+) diff --git a/src/nxt_conf.c b/src/nxt_conf.c index 664b54686..008cb9688 100644 --- a/src/nxt_conf.c +++ b/src/nxt_conf.c @@ -3,6 +3,7 @@ * Copyright (C) Igor Sysoev * Copyright (C) Valentin V. Bartenev * Copyright (C) NGINX, Inc. + * Copyright 2024, Alejandro Colomar */ #include @@ -174,6 +175,16 @@ nxt_conf_get_string(nxt_conf_value_t *value, nxt_str_t *str) } +nxt_str_t * +nxt_conf_get_string_dup(nxt_conf_value_t *value, nxt_mp_t *mp, nxt_str_t *str) +{ + nxt_str_t s; + + nxt_conf_get_string(value, &s); + return nxt_str_dup(mp, str, &s); +} + + void nxt_conf_set_string(nxt_conf_value_t *value, nxt_str_t *str) { diff --git a/src/nxt_conf.h b/src/nxt_conf.h index 1b13f5ae9..626b6d4d9 100644 --- a/src/nxt_conf.h +++ b/src/nxt_conf.h @@ -3,6 +3,7 @@ * Copyright (C) Igor Sysoev * Copyright (C) Valentin V. Bartenev * Copyright (C) NGINX, Inc. + * Copyright 2024, Alejandro Colomar */ #ifndef _NXT_CONF_INCLUDED_ @@ -116,6 +117,8 @@ void nxt_conf_json_position(u_char *start, const u_char *pos, nxt_uint_t *line, nxt_int_t nxt_conf_validate(nxt_conf_validation_t *vldt); NXT_EXPORT void nxt_conf_get_string(nxt_conf_value_t *value, nxt_str_t *str); +NXT_EXPORT nxt_str_t *nxt_conf_get_string_dup(nxt_conf_value_t *value, + nxt_mp_t *mp, 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, const nxt_str_t *str); From bb376c683877d2aec9ce4358536113ca5fd19d84 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Fri, 2 Feb 2024 23:43:13 +0100 Subject: [PATCH 12/19] Simplify, by calling nxt_conf_get_string_dup() Refactor. Reviewed-by: Andrew Clayton Cc: Zhidao Hong Signed-off-by: Alejandro Colomar --- src/nxt_http_static.c | 4 +--- src/nxt_router.c | 23 +++++++++-------------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/nxt_http_static.c b/src/nxt_http_static.c index c4caab3cf..ee25015e2 100644 --- a/src/nxt_http_static.c +++ b/src/nxt_http_static.c @@ -117,9 +117,7 @@ nxt_http_static_init(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, nxt_str_set(&conf->index, "index.html"); } else { - nxt_conf_get_string(acf->index, &str); - - ret = nxt_str_dup(mp, &conf->index, &str); + ret = nxt_conf_get_string_dup(acf->index, mp, &conf->index); if (nxt_slow_path(ret == NULL)) { return NXT_ERROR; } diff --git a/src/nxt_router.c b/src/nxt_router.c index 0b979575e..947836c84 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -2275,7 +2275,7 @@ nxt_router_conf_process_static(nxt_task_t *task, nxt_router_conf_t *rtcf, { uint32_t next, i; nxt_mp_t *mp; - nxt_str_t *type, exten, str; + nxt_str_t *type, exten, str, *s; nxt_int_t ret; nxt_uint_t exts; nxt_conf_value_t *mtypes_conf, *ext_conf, *value; @@ -2311,9 +2311,8 @@ nxt_router_conf_process_static(nxt_task_t *task, nxt_router_conf_t *rtcf, } if (nxt_conf_type(ext_conf) == NXT_CONF_STRING) { - nxt_conf_get_string(ext_conf, &str); - - if (nxt_slow_path(nxt_str_dup(mp, &exten, &str) == NULL)) { + s = nxt_conf_get_string_dup(ext_conf, mp, &exten); + if (nxt_slow_path(s == NULL)) { return NXT_ERROR; } @@ -2331,9 +2330,8 @@ nxt_router_conf_process_static(nxt_task_t *task, nxt_router_conf_t *rtcf, for (i = 0; i < exts; i++) { value = nxt_conf_get_array_element(ext_conf, i); - nxt_conf_get_string(value, &str); - - if (nxt_slow_path(nxt_str_dup(mp, &exten, &str) == NULL)) { + s = nxt_conf_get_string_dup(value, mp, &exten); + if (nxt_slow_path(s == NULL)) { return NXT_ERROR; } @@ -2425,14 +2423,11 @@ static nxt_int_t nxt_router_conf_forward_header(nxt_mp_t *mp, nxt_conf_value_t *conf, nxt_http_forward_header_t *fh) { - char c; - size_t i; - uint32_t hash; - nxt_str_t header; - - nxt_conf_get_string(conf, &header); + char c; + size_t i; + uint32_t hash; - fh->header = nxt_str_dup(mp, NULL, &header); + fh->header = nxt_conf_get_string_dup(conf, mp, NULL); if (nxt_slow_path(fh->header == NULL)) { return NXT_ERROR; } From 46cef09f296d9a3d54b98331d25920fc6322bea8 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Wed, 31 Jan 2024 15:34:57 +0100 Subject: [PATCH 13/19] Configuration: Don't corrupt abstract socket names The commit that added support for Unix sockets accepts abstract sockets using '@' in the config, but we stored it internally using '\0'. We want to support abstract sockets transparently to the user, so that if the user configures unitd with '@', if we receive a query about the current configuration, the user should see the same exact thing that was configured. So, this commit avoids the transformation in the internal state file, storing user input pristine, and we only transform the '@' in temporary strings. This commit fixes another bug, where we try to connect to abstract sockets with a trailing '\0' in their name due to calling twice nxt_sockaddr_parse() on the same string. By calling that function only once with each copy of the string, we have fixed that bug. The following code was responsible for this bug, which the second time it was called, considered these sockets as file-backed (not abstract) Unix socket, and so appended a '\0' to the socket name. $ grepc -tfd nxt_sockaddr_unix_parse . | grep -A10 @ if (path[0] == '@') { path[0] = '\0'; socklen--; #if !(NXT_LINUX) nxt_thread_log_error(NXT_LOG_ERR, "abstract unix domain sockets are not supported"); return NULL; #endif } sa = nxt_sockaddr_alloc(mp, socklen, addr->length); This bug was found thanks to some experiment about using 'const' for some strings. And here's some history: - 9041d276fc6a ("nxt_sockaddr_parse() introducted.") This commit introduced support for abstract Unix sockets, but they only worked as "servers", and not as "listeners". We corrupted the JSON config file, and stored a \u0000. This also caused calling connect(2) with a bogus trailing null byte, which tried to connect to a different abstract socket. - d8e0768a5bae ("Fixed support for abstract Unix sockets.") This commit (partially) fixed support for abstract Unix sockets, so they they worked also as listeners. We still corrupted the JSON config file, and stored a \u0000. This caused calling connect(2) (and now bind(2) too) with a bogus trailing null byte. - e2aec6686a4d ("Storing abstract sockets with @ internally.") This commit fixed the problem by which we were corrupting the config file, but only for "listeners", not for "servers". (It also fixes the issue about the terminating '\0'.) We completely forgot about "servers", and other callers of the same function. To reproduce the problem, I used the following config: ```json { "listeners": { "*:80": { "pass": "routes/u" }, "unix:@abstract": { "pass": "routes/a" } }, "routes": { "u": [{ "action": { "pass": "upstreams/u" } }], "a": [{ "action": { "return": 302, "location": "/i/am/not/at/home/" } }] }, "upstreams": { "u": { "servers": { "unix:@abstract": {} } } } } ``` And then check the state file: $ sudo cat /opt/local/nginx/unit/master/var/lib/unit/conf.json \ | jq . \ | grep unix; "unix:@abstract": { "unix:\u0000abstract": {} After this patch, the state file has a '@' as expected: $ sudo cat /opt/local/nginx/unit/unix/var/lib/unit/conf.json \ | jq . \ | grep unix; "unix:@abstract": { "unix:@abstract": {} Regarding the trailing null byte, here are some tests: $ sudo strace -f -e 'bind,connect' /opt/local/nginx/unit/d8e0/sbin/unitd \ |& grep abstract; [pid 22406] bind(10, {sa_family=AF_UNIX, sun_path=@"abstract\0"}, 12) = 0 [pid 22410] connect(134, {sa_family=AF_UNIX, sun_path=@"abstract\0"}, 12) = 0 ^C $ sudo killall unitd $ sudo strace -f -e 'bind,connect' /opt/local/nginx/unit/master/sbin/unitd \ |& grep abstract; [pid 22449] bind(10, {sa_family=AF_UNIX, sun_path=@"abstract"}, 11) = 0 [pid 22453] connect(134, {sa_family=AF_UNIX, sun_path=@"abstract\0"}, 12) = -1 ECONNREFUSED (Connection refused) ^C $ sudo killall unitd $ sudo strace -f -e 'bind,connect' /opt/local/nginx/unit/unix/sbin/unitd \ |& grep abstract; [pid 22488] bind(10, {sa_family=AF_UNIX, sun_path=@"abstract"}, 11) = 0 [pid 22492] connect(134, {sa_family=AF_UNIX, sun_path=@"abstract"}, 11) = 0 ^C Fixes: 9041d276fc6a ("nxt_sockaddr_parse() introducted.") Fixes: d8e0768a5bae ("Fixed support for abstract Unix sockets.") Fixes: e2aec6686a4d ("Storing abstract sockets with @ internally.") Link: Reviewed-by: Andrew Clayton Cc: Liam Crilly Cc: Zhidao Hong Signed-off-by: Alejandro Colomar --- src/nxt_conf_validation.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/nxt_conf_validation.c b/src/nxt_conf_validation.c index c843b2654..bf18cd1a3 100644 --- a/src/nxt_conf_validation.c +++ b/src/nxt_conf_validation.c @@ -2,6 +2,7 @@ /* * Copyright (C) Valentin V. Bartenev * Copyright (C) NGINX, Inc. + * Copyright 2024, Alejandro Colomar */ #include @@ -1936,10 +1937,13 @@ static nxt_int_t nxt_conf_vldt_proxy(nxt_conf_validation_t *vldt, nxt_conf_value_t *value, void *data) { - nxt_str_t name; + nxt_str_t name, *ret; nxt_sockaddr_t *sa; - nxt_conf_get_string(value, &name); + ret = nxt_conf_get_string_dup(value, vldt->pool, &name); + if (nxt_slow_path(ret == NULL)) { + return NXT_ERROR; + } if (nxt_str_start(&name, "http://", 7)) { name.length -= 7; @@ -2913,13 +2917,11 @@ nxt_conf_vldt_object_iterator(nxt_conf_validation_t *vldt, for ( ;; ) { member = nxt_conf_next_object_member(value, &name, &index); - if (member == NULL) { return NXT_OK; } ret = validator(vldt, &name, member); - if (ret != NXT_OK) { return ret; } @@ -3268,16 +3270,19 @@ nxt_conf_vldt_server(nxt_conf_validation_t *vldt, nxt_str_t *name, nxt_conf_value_t *value) { nxt_int_t ret; + nxt_str_t str; nxt_sockaddr_t *sa; ret = nxt_conf_vldt_type(vldt, name, value, NXT_CONF_VLDT_OBJECT); - if (ret != NXT_OK) { return ret; } - sa = nxt_sockaddr_parse(vldt->pool, name); + if (nxt_slow_path(nxt_str_dup(vldt->pool, &str, name) == NULL)) { + return NXT_ERROR; + } + sa = nxt_sockaddr_parse(vldt->pool, &str); if (sa == NULL) { return nxt_conf_vldt_error(vldt, "The \"%V\" is not valid " "server address.", name); From 9e986704480de0d6b74dafa5ebcf775eaa88333a Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Thu, 8 Feb 2024 11:15:34 +0100 Subject: [PATCH 14/19] Configuration: Fix validation of "processes" It's an integer, not a floating number. Fixes: 68c6b67ffc84 ("Configuration: support for rational numbers.") Closes: https://github.com/nginx/unit/issues/1115 Link: Reviewed-by: Zhidao Hong Reviewed-by: Andrew Clayton Cc: Dan Callahan Cc: Valentin Bartenev Signed-off-by: Alejandro Colomar --- src/nxt_conf_validation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nxt_conf_validation.c b/src/nxt_conf_validation.c index bf18cd1a3..caa068d28 100644 --- a/src/nxt_conf_validation.c +++ b/src/nxt_conf_validation.c @@ -2830,7 +2830,7 @@ 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_NUMBER) { + if (nxt_conf_type(value) == NXT_CONF_INTEGER) { int_value = nxt_conf_get_number(value); if (int_value < 1) { From 3a2687bb714226ab13111cac2149afc660fa70e7 Mon Sep 17 00:00:00 2001 From: Konstantin Pavlov Date: Tue, 17 Oct 2023 16:22:44 -0700 Subject: [PATCH 15/19] Packages: added Ubuntu 23.10 "mantic" support. --- docs/changes.xml | 29 +++++++- pkg/deb/Makefile | 17 +++++ pkg/deb/Makefile.jsc21 | 71 +++++++++++++++++++ pkg/deb/Makefile.python312 | 46 ++++++++++++ .../debian.module/unit.example-jsc21-config | 15 ++++ .../unit.example-python3.12-config | 16 +++++ 6 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 pkg/deb/Makefile.jsc21 create mode 100644 pkg/deb/Makefile.python312 create mode 100644 pkg/deb/debian.module/unit.example-jsc21-config create mode 100644 pkg/deb/debian.module/unit.example-python3.12-config diff --git a/docs/changes.xml b/docs/changes.xml index 508678569..428da0bab 100644 --- a/docs/changes.xml +++ b/docs/changes.xml @@ -5,16 +5,43 @@ + + + + +Initial release of Java 21 module for NGINX Unit. + + + + + + + + + + +Initial release of Python 3.12 module for NGINX Unit. + + + + + + Date: Fri, 20 Oct 2023 18:21:00 -0700 Subject: [PATCH 16/19] contrib: Bump libunit-wasm to 0.3.0. --- pkg/contrib/src/libunit-wasm/version | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/contrib/src/libunit-wasm/version b/pkg/contrib/src/libunit-wasm/version index 7ca15f983..60577d0ee 100644 --- a/pkg/contrib/src/libunit-wasm/version +++ b/pkg/contrib/src/libunit-wasm/version @@ -1,2 +1,2 @@ -LIBUNIT_WASM_VERSION := 0.1.0 -LIBUNIT_WASM_GITHASH := d6ed6a219b31a58526721f96195c80061d41ce54 +LIBUNIT_WASM_VERSION := 0.3.0 +LIBUNIT_WASM_GITHASH := 01c43784ec53aa1ff22aca7e7ae6f18b4591b514 From ca1bc0625a7c1c57f8f9fb9be66b29a96ac9d79d Mon Sep 17 00:00:00 2001 From: Konstantin Pavlov Date: Wed, 25 Oct 2023 12:37:10 -0700 Subject: [PATCH 17/19] contrib: updated njs to 0.8.2. --- pkg/contrib/src/njs/SHA512SUMS | 2 +- pkg/contrib/src/njs/version | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/contrib/src/njs/SHA512SUMS b/pkg/contrib/src/njs/SHA512SUMS index 3c3ce2100..437664871 100644 --- a/pkg/contrib/src/njs/SHA512SUMS +++ b/pkg/contrib/src/njs/SHA512SUMS @@ -1 +1 @@ -5038b4cd9e18de89c9cf7fe7b25a0a8a03c51cfb20b6ee5085e68f885113b104092baf5ac8fe80e9d1611b2f75e47448753e6b327bef2e706ea46f2d6299f927 njs-0.8.1.tar.gz +cc3110a0c6866dfc03d19c58745e5b75aa9792999db45bc55a752f7b04db8ae51322bfe0156b873109c8477c6c1a030c851c770697cf6791c6e89fb2fed0a2c5 njs-0.8.2.tar.gz diff --git a/pkg/contrib/src/njs/version b/pkg/contrib/src/njs/version index 73c524fbb..004534192 100644 --- a/pkg/contrib/src/njs/version +++ b/pkg/contrib/src/njs/version @@ -1 +1 @@ -NJS_VERSION := 0.8.1 +NJS_VERSION := 0.8.2 From bad2c181e124cc93cd839fce019bc31dab1b7e1b Mon Sep 17 00:00:00 2001 From: Konstantin Pavlov Date: Fri, 17 Nov 2023 16:29:18 -0800 Subject: [PATCH 18/19] Packages: Added Fedora 39 support. --- pkg/rpm/Makefile | 16 +++++- pkg/rpm/Makefile.jsc-common | 5 +- pkg/rpm/Makefile.jsc17 | 13 +++++ pkg/rpm/Makefile.python312 | 53 +++++++++++++++++++ .../SOURCES/unit.example-python312-config | 16 ++++++ 5 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 pkg/rpm/Makefile.python312 create mode 100644 pkg/rpm/rpmbuild/SOURCES/unit.example-python312-config diff --git a/pkg/rpm/Makefile b/pkg/rpm/Makefile index 7906fc24c..1f3bbd58b 100644 --- a/pkg/rpm/Makefile +++ b/pkg/rpm/Makefile @@ -22,8 +22,10 @@ else ifeq ($(shell rpm --eval "%{?amzn}"), 2023) OSVER = amazonlinux2023 else ifeq ($(shell test `rpm --eval '0%{?fedora} -ge 35 -a 0%{?fedora} -le 36'`; echo $$?),0) OSVER = fedora -else ifeq ($(shell test `rpm --eval '0%{?fedora} -ge 37'`; echo $$?),0) +else ifeq ($(shell test `rpm --eval '0%{?fedora} -ge 37 -a 0%{?fedora} -le 38'`; echo $$?),0) OSVER = fedora37 +else ifeq ($(shell test `rpm --eval '0%{?fedora} -ge 39'`; echo $$?),0) +OSVER = fedora39 endif BUILD_DEPENDS_unit = gcc rpm-build rpmlint @@ -124,6 +126,18 @@ include Makefile.jsc11 include Makefile.wasm endif +ifeq ($(OSVER), fedora39) +include Makefile.php +include Makefile.python312 +include Makefile.go +include Makefile.perl +include Makefile.ruby +include Makefile.jsc-common +include Makefile.jsc17 +include Makefile.wasm +endif + + CONFIGURE_ARGS_COMMON=\ --prefix=/usr \ --statedir=%{_sharedstatedir}/unit \ diff --git a/pkg/rpm/Makefile.jsc-common b/pkg/rpm/Makefile.jsc-common index a3c3a3da6..f77ca1e9f 100644 --- a/pkg/rpm/Makefile.jsc-common +++ b/pkg/rpm/Makefile.jsc-common @@ -10,16 +10,19 @@ JAVA_ARCH_jsc_common= $(shell /usr/lib/jvm/java-1.8.0/bin/java -XshowSettings 2> ifeq ($(OSVER),amazonlinux2023) MODULE_CONFARGS_jsc_common= java --home=/usr/lib/jvm/java-17-amazon-corretto --lib-path=/usr/lib/jvm/java-17-amazon-corretto/lib --jars=/usr/share/unit-jsc-common/ +else ifeq ($(OSVER),fedora39) +MODULE_CONFARGS_jsc_common= java --home=/usr/lib/jvm/java-17-openjdk --lib-path=/usr/lib/jvm/java-17-openjdk/lib --jars=/usr/share/unit-jsc-common/ else MODULE_CONFARGS_jsc_common= java --home=/usr/lib/jvm/java-1.8.0 --lib-path=/usr/lib/jvm/jre-1.8.0/lib/$(JAVA_ARCH_jsc_common) --jars=/usr/share/unit-jsc-common/ endif -MODULE_MAKEARGS_jsc_common= java MODULE_INSTARGS_jsc_common= java-shared-install MODULE_SOURCES_jsc_common= COPYRIGHT.unit-jsc-common ifeq ($(OSVER),amazonlinux2023) BUILD_DEPENDS_jsc_common= java-17-amazon-corretto-devel curl +else ifeq ($(OSVER),fedora39) +BUILD_DEPENDS_jsc_common= java-17-openjdk-devel curl else BUILD_DEPENDS_jsc_common= java-1.8.0-openjdk-devel curl endif diff --git a/pkg/rpm/Makefile.jsc17 b/pkg/rpm/Makefile.jsc17 index 7efdafaab..9a42c5a14 100644 --- a/pkg/rpm/Makefile.jsc17 +++ b/pkg/rpm/Makefile.jsc17 @@ -6,19 +6,32 @@ MODULE_SUMMARY_jsc17= Java 17 module for NGINX Unit MODULE_VERSION_jsc17= $(VERSION) MODULE_RELEASE_jsc17= 1 +ifeq ($(OSVER),amazonlinux2023) MODULE_CONFARGS_jsc17= java --module=java17 --home=/usr/lib/jvm/java-17-amazon-corretto --lib-path=/usr/lib/jvm/java-17-amazon-corretto/lib --jars=/usr/share/unit-jsc-common/ +else ifeq ($(OSVER),fedora39) +MODULE_CONFARGS_jsc17= java --module=java17 --home=/usr/lib/jvm/java-17-openjdk --lib-path=/usr/lib/jvm/java-17-openjdk/lib --jars=/usr/share/unit-jsc-common/ +endif MODULE_MAKEARGS_jsc17= java17 MODULE_INSTARGS_jsc17= java17-install MODULE_SOURCES_jsc17= unit.example-jsc-app \ unit.example-jsc17-config +ifeq ($(OSVER),amazonlinux2023) BUILD_DEPENDS_jsc17= java-17-amazon-corretto-devel +else ifeq ($(OSVER),fedora39) +BUILD_DEPENDS_jsc17= java-17-openjdk-devel BUILD_DEPENDS+= $(BUILD_DEPENDS_jsc17) +endif define MODULE_DEFINITIONS_jsc17 Requires: unit-jsc-common == $(MODULE_VERSION_jsc_common)-$(MODULE_RELEASE_jsc_common)%{?dist}.ngx +%if (0%{?amzn} == 2023) Requires: java-17-amazon-corretto-headless +%endif +%if (0%{?fedora} >= 39) +Requires: java-17-openjdk-headless +%endif endef export MODULE_DEFINITIONS_jsc17 diff --git a/pkg/rpm/Makefile.python312 b/pkg/rpm/Makefile.python312 new file mode 100644 index 000000000..c37069eb4 --- /dev/null +++ b/pkg/rpm/Makefile.python312 @@ -0,0 +1,53 @@ +MODULES+= python312 +MODULE_SUFFIX_python312= python3.12 + +MODULE_SUMMARY_python312= Python 3.12 module for NGINX Unit + +MODULE_VERSION_python312= $(VERSION) +MODULE_RELEASE_python312= 1 + +MODULE_CONFARGS_python312= python --config=python3.12-config +MODULE_MAKEARGS_python312= python3.12 +MODULE_INSTARGS_python312= python3.12-install + +MODULE_SOURCES_python312= unit.example-python-app \ + unit.example-python312-config + +BUILD_DEPENDS_python312= python3-devel + +BUILD_DEPENDS+= $(BUILD_DEPENDS_python312) + +define MODULE_PREINSTALL_python312 +%{__mkdir} -p %{buildroot}%{_datadir}/doc/unit-python312/examples/python-app +%{__install} -m 644 -p %{SOURCE100} \ + %{buildroot}%{_datadir}/doc/unit-python312/examples/python-app/wsgi.py +%{__install} -m 644 -p %{SOURCE101} \ + %{buildroot}%{_datadir}/doc/unit-python312/examples/unit.config +endef +export MODULE_PREINSTALL_python312 + +define MODULE_FILES_python312 +%{_libdir}/unit/modules/* +%{_libdir}/unit/debug-modules/* +endef +export MODULE_FILES_python312 + +define MODULE_POST_python312 +cat < Date: Thu, 25 Jan 2024 13:47:33 +0100 Subject: [PATCH 19/19] Add unit section to status endpoint Added unit section to /status endpoint. Unit section is about web-server version, config last load time and config update generation Response example below: {"unit":{"version":"1.32.0","load_time":"2024-01-25T13:24:08.000Z","generation":0},"connections":{"accepted":0,"active":0,"idle":0,"closed":0},"requests":{"total":0},"applications":{"laravel":{"processes":{"running":1,"starting":0,"idle":1},"requests":{"active":0}}}} Closes: https://github.com/nginx/unit/issues/928 --- src/nxt_controller.c | 18 +++++++++++++++--- src/nxt_runtime.c | 13 +++++++++++++ src/nxt_runtime.h | 3 +++ src/nxt_status.c | 32 ++++++++++++++++++++++++++------ src/nxt_status.h | 2 +- 5 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/nxt_controller.c b/src/nxt_controller.c index eb814321d..0980598f6 100644 --- a/src/nxt_controller.c +++ b/src/nxt_controller.c @@ -11,8 +11,6 @@ #include #include #include -#include - typedef struct { nxt_conf_value_t *root; @@ -1633,7 +1631,7 @@ nxt_controller_status_handler(nxt_task_t *task, nxt_port_recv_msg_t *msg, req = data; if (msg->port_msg.type == NXT_PORT_MSG_RPC_READY) { - status = nxt_status_get((nxt_status_report_t *) msg->buf->mem.pos, + status = nxt_status_get(task, (nxt_status_report_t *) msg->buf->mem.pos, req->conn->mem_pool); } else { status = NULL; @@ -2432,6 +2430,9 @@ nxt_controller_conf_store(nxt_task_t *task, nxt_conf_value_t *conf) nxt_buf_t *b; nxt_port_t *main_port; nxt_runtime_t *rt; + time_t rawtime; + u_char buffer[25]; + struct tm *timeinfo; rt = task->thread->runtime; @@ -2466,6 +2467,17 @@ nxt_controller_conf_store(nxt_task_t *task, nxt_conf_value_t *conf) NXT_PORT_MSG_CONF_STORE | NXT_PORT_MSG_CLOSE_FD, fd, 0, -1, b); + rt->conf_gen++; + + time(&rawtime); + timeinfo = gmtime(&rawtime); //convert to UTC + strftime((char*)buffer, 25, "%Y-%m-%dT%H:%M:%S.000Z", timeinfo); + + rt->conf_ltime.length = nxt_strlen(buffer); + rt->conf_ltime.start = nxt_malloc(rt->conf_ltime.length + 1); + + nxt_cpystr(rt->conf_ltime.start, buffer); + return; fail: diff --git a/src/nxt_runtime.c b/src/nxt_runtime.c index 9bfabc750..5d2e4b48b 100644 --- a/src/nxt_runtime.c +++ b/src/nxt_runtime.c @@ -777,6 +777,9 @@ nxt_runtime_conf_init(nxt_task_t *task, nxt_runtime_t *rt) nxt_sockaddr_t *sa; nxt_file_name_str_t file_name; const nxt_event_interface_t *interface; + time_t rawtime; + u_char buffer[25]; + struct tm *timeinfo; rt->daemon = 1; rt->engine_connections = 256; @@ -789,6 +792,16 @@ nxt_runtime_conf_init(nxt_task_t *task, nxt_runtime_t *rt) rt->state = NXT_STATEDIR; rt->control = NXT_CONTROL_SOCK; rt->tmp = NXT_TMPDIR; + rt->conf_gen = 0; + + time(&rawtime); + timeinfo = gmtime(&rawtime); //convert to UTC + strftime((char*)buffer, 25, "%Y-%m-%dT%H:%M:%S.000Z", timeinfo); + + rt->conf_ltime.length = nxt_strlen(buffer); + rt->conf_ltime.start = nxt_malloc(rt->conf_ltime.length + 1); + + nxt_cpystr(rt->conf_ltime.start, buffer); nxt_memzero(&rt->capabilities, sizeof(nxt_capabilities_t)); diff --git a/src/nxt_runtime.h b/src/nxt_runtime.h index 66ec0106c..a3397bc82 100644 --- a/src/nxt_runtime.h +++ b/src/nxt_runtime.h @@ -73,6 +73,9 @@ struct nxt_runtime_s { const char *control; const char *tmp; + nxt_int_t conf_gen; + nxt_str_t conf_ltime; + nxt_str_t certs; nxt_str_t scripts; diff --git a/src/nxt_status.c b/src/nxt_status.c index f8002e86e..e68a05a4b 100644 --- a/src/nxt_status.c +++ b/src/nxt_status.c @@ -9,14 +9,20 @@ nxt_conf_value_t * -nxt_status_get(nxt_status_report_t *report, nxt_mp_t *mp) +nxt_status_get(nxt_task_t *task, nxt_status_report_t *report, nxt_mp_t *mp) { size_t i; - nxt_str_t name; + nxt_runtime_t *rt; nxt_int_t ret; + nxt_str_t name; nxt_status_app_t *app; + nxt_str_t version; nxt_conf_value_t *status, *obj, *apps, *app_obj; + static nxt_str_t unit_str = nxt_string("unit"); + static nxt_str_t ver_str = nxt_string("version"); + static nxt_str_t gen_str = nxt_string("generation"); + static nxt_str_t lconf_str = nxt_string("last_configured"); static nxt_str_t conns_str = nxt_string("connections"); static nxt_str_t acc_str = nxt_string("accepted"); static nxt_str_t active_str = nxt_string("active"); @@ -29,17 +35,31 @@ nxt_status_get(nxt_status_report_t *report, nxt_mp_t *mp) static nxt_str_t run_str = nxt_string("running"); static nxt_str_t start_str = nxt_string("starting"); - status = nxt_conf_create_object(mp, 3); + rt = task->thread->runtime; + + status = nxt_conf_create_object(mp, 4); if (nxt_slow_path(status == NULL)) { return NULL; } + obj = nxt_conf_create_object(mp, 3); + if (nxt_slow_path(obj == NULL)) { + return NULL; + } + + nxt_conf_set_member(status, &unit_str, obj, 0); + + nxt_str_set(&version, NXT_VERSION); + nxt_conf_set_member_string(obj, &ver_str, &version, 0); + nxt_conf_set_member_string(obj, &lconf_str, &rt->conf_ltime, 1); + nxt_conf_set_member_integer(obj, &gen_str, rt->conf_gen, 2); + obj = nxt_conf_create_object(mp, 4); if (nxt_slow_path(obj == NULL)) { return NULL; } - nxt_conf_set_member(status, &conns_str, obj, 0); + nxt_conf_set_member(status, &conns_str, obj, 1); nxt_conf_set_member_integer(obj, &acc_str, report->accepted_conns, 0); nxt_conf_set_member_integer(obj, &active_str, report->accepted_conns @@ -53,7 +73,7 @@ nxt_status_get(nxt_status_report_t *report, nxt_mp_t *mp) return NULL; } - nxt_conf_set_member(status, &reqs_str, obj, 1); + nxt_conf_set_member(status, &reqs_str, obj, 2); nxt_conf_set_member_integer(obj, &total_str, report->requests, 0); @@ -62,7 +82,7 @@ nxt_status_get(nxt_status_report_t *report, nxt_mp_t *mp) return NULL; } - nxt_conf_set_member(status, &apps_str, apps, 2); + nxt_conf_set_member(status, &apps_str, apps, 3); for (i = 0; i < report->apps_count; i++) { app = &report->apps[i]; diff --git a/src/nxt_status.h b/src/nxt_status.h index a99ac7d0e..32a902fde 100644 --- a/src/nxt_status.h +++ b/src/nxt_status.h @@ -27,7 +27,7 @@ typedef struct { } nxt_status_report_t; -nxt_conf_value_t *nxt_status_get(nxt_status_report_t *report, nxt_mp_t *mp); +nxt_conf_value_t *nxt_status_get(nxt_task_t *task, nxt_status_report_t *report, nxt_mp_t *mp); #endif /* _NXT_STATUS_H_INCLUDED_ */