From 6482e46a6b35214967095169842e1d5403a01d4d Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Mon, 11 Aug 2025 19:08:07 +0100 Subject: http: compression: Set the temporary file name in n_h_c_c_s_r() When creating a new nxt_file_t structure in nxt_http_comp_compress_static_response() for the temporary compressed file be sure to set the *name* member. We don't generally need it, but I failed to notice that when calling nxt_file_close() if the close(2) fails then we log an error message containing the file name, which at best would have just printed junk. So set the file name for this particular error case... This issue was reported by coverity. Signed-off-by: Andrew Clayton --- src/nxt_http_compression.c | 20 ++++++++++---------- src/nxt_http_compression.h | 4 ++-- src/nxt_http_static.c | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/nxt_http_compression.c b/src/nxt_http_compression.c index 28e53a9d..4f4eec1a 100644 --- a/src/nxt_http_compression.c +++ b/src/nxt_http_compression.c @@ -232,14 +232,12 @@ nxt_http_comp_compress_app_response(nxt_task_t *task, nxt_http_request_t *r, nxt_int_t -nxt_http_comp_compress_static_response(nxt_task_t *task, nxt_file_t **f, - nxt_file_info_t *fi, - size_t static_buf_len, - size_t *out_total) +nxt_http_comp_compress_static_response(nxt_task_t *task, nxt_http_request_t *r, + nxt_file_t **f, nxt_file_info_t *fi, + size_t static_buf_len, size_t *out_total) { - char tmp_path[NXT_MAX_PATH_LEN]; size_t in_size, out_size, rest; - u_char *p; + char *tmp_path, *p; uint8_t *in, *out; nxt_int_t ret; nxt_file_t tfile; @@ -249,13 +247,14 @@ nxt_http_comp_compress_static_response(nxt_task_t *task, nxt_file_t **f, *out_total = 0; - if (nxt_slow_path(strlen(rt->tmp) + 1 + strlen(template) + 1 - > NXT_MAX_PATH_LEN)) - { + tmp_path = nxt_mp_nget(r->mem_pool, + strlen(rt->tmp) + 1 + strlen(template) + 1); + if (nxt_slow_path(tmp_path == NULL)) { return NXT_ERROR; } - p = nxt_cpymem(tmp_path, rt->tmp, strlen(rt->tmp)); + p = tmp_path; + p = nxt_cpymem(p, rt->tmp, strlen(rt->tmp)); *p++ = '/'; p = nxt_cpymem(p, template, strlen(template)); *p = '\0'; @@ -266,6 +265,7 @@ nxt_http_comp_compress_static_response(nxt_task_t *task, nxt_file_t **f, return NXT_ERROR; } unlink(tmp_path); + tfile.name = (nxt_file_name_t *)tmp_path; in_size = nxt_file_size(fi); out_size = nxt_http_comp_bound(in_size); diff --git a/src/nxt_http_compression.h b/src/nxt_http_compression.h index f178e984..99af8a66 100644 --- a/src/nxt_http_compression.h +++ b/src/nxt_http_compression.h @@ -93,8 +93,8 @@ extern const nxt_http_comp_operations_t nxt_http_comp_brotli_ops; extern nxt_int_t nxt_http_comp_compress_app_response(nxt_task_t *task, nxt_http_request_t *r, nxt_buf_t **b); extern nxt_int_t nxt_http_comp_compress_static_response(nxt_task_t *task, - nxt_file_t **f, nxt_file_info_t *fi, size_t static_buf_len, - size_t *out_total); + nxt_http_request_t *r, nxt_file_t **f, nxt_file_info_t *fi, + size_t static_buf_len, size_t *out_total); extern bool nxt_http_comp_wants_compression(void); extern bool nxt_http_comp_compressor_is_valid(const nxt_str_t *token); extern nxt_int_t nxt_http_comp_check_compression(nxt_task_t *task, diff --git a/src/nxt_http_static.c b/src/nxt_http_static.c index 78b1f150..8436b417 100644 --- a/src/nxt_http_static.c +++ b/src/nxt_http_static.c @@ -593,7 +593,7 @@ nxt_http_static_send(nxt_task_t *task, nxt_http_request_t *r, nxt_int_t ret; ret = nxt_http_comp_compress_static_response( - task, &f, &fi, + task, r, &f, &fi, NXT_HTTP_STATIC_BUF_SIZE, &out_total); if (ret == NXT_ERROR) { -- cgit From 5e97e44df4bf80ee6914c3fe42c57d6c674c3279 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Mon, 11 Aug 2025 22:26:31 +0100 Subject: http: compression: Add a missed nxt_http_comp_compress() return check In nxt_http_comp_compress_static_response() we should check the return value of the call to nxt_http_comp_compress() in case of error. Signed-off-by: Andrew Clayton --- src/nxt_http_compression.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src') diff --git a/src/nxt_http_compression.c b/src/nxt_http_compression.c index 4f4eec1a..f9a94d05 100644 --- a/src/nxt_http_compression.c +++ b/src/nxt_http_compression.c @@ -305,6 +305,12 @@ nxt_http_comp_compress_static_response(nxt_task_t *task, nxt_http_request_t *r, cbytes = nxt_http_comp_compress(out + *out_total, out_size - *out_total, in + in_size - rest, n, last); + if (cbytes == -1) { + nxt_file_close(task, &tfile); + nxt_mem_munmap(in, in_size); + nxt_mem_munmap(out, out_size); + return NXT_ERROR; + } *out_total += cbytes; rest -= n; -- cgit From 1701935ea4a55f3ce4b5da5db866fc1b11558e0c Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Tue, 12 Aug 2025 22:48:32 +0100 Subject: Don't leak file descriptor in nxt_main_port_access_log_handler() After opening a file and setting file.fd we _may_ call nxt_port_socket_write(). If so then the file is eventually closed via something like nxt_port_socket_write() nxt_port_socket_write2() nxt_port_write_handler() nxt_port_msg_close_fd() nxt_port_close_fds() Alternatively we may just return from the function and never close(2) file.fd. In which case we should call nxt_file_close(). This was reported by coverity. Signed-off-by: Andrew Clayton --- src/nxt_main_process.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src') diff --git a/src/nxt_main_process.c b/src/nxt_main_process.c index e942c1a8..25798ea0 100644 --- a/src/nxt_main_process.c +++ b/src/nxt_main_process.c @@ -1730,5 +1730,8 @@ nxt_main_port_access_log_handler(nxt_task_t *task, nxt_port_recv_msg_t *msg) if (nxt_fast_path(port != NULL)) { (void) nxt_port_socket_write(task, port, type, file.fd, msg->port_msg.stream, 0, NULL); + + } else { + nxt_file_close(task, &file); } } -- cgit