From 7dcd6c0ebacab6d78ecc34cbac347ef46f79f479 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Wed, 31 Jan 2024 15:20:33 +0000 Subject: Avoiding arithmetic ops with NULL pointer in nxt_http_arguments_parse Can be reproduced by test/test_variables.py::test_variables_dynamic_arguments with enabled UndefinedBehaviorSanitizer: src/nxt_http_request.c:961:17: runtime error: applying zero offset to null pointer #0 0x1050d95a4 in nxt_http_arguments_parse nxt_http_request.c:961 #1 0x105102bf8 in nxt_http_var_arg nxt_http_variables.c:621 #2 0x104f95d74 in nxt_var_interpreter nxt_var.c:507 #3 0x104f98c98 in nxt_tstr_query nxt_tstr.c:265 #4 0x1050abfd8 in nxt_router_access_log_writer nxt_router_access_log.c:194 #5 0x1050d81f4 in nxt_http_request_close_handler nxt_http_request.c:838 #6 0x104fcdc48 in nxt_event_engine_start nxt_event_engine.c:542 #7 0x104fba838 in nxt_thread_trampoline nxt_thread.c:126 #8 0x18133e030 in _pthread_start+0x84 (libsystem_pthread.dylib:arm64e+0x7030) #9 0x181338e38 in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1e38) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/nxt_http_request.c:961:17 Reviewed-by: Andrew Clayton --- src/nxt_http_request.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/nxt_http_request.c') diff --git a/src/nxt_http_request.c b/src/nxt_http_request.c index f8d8d887..425a4607 100644 --- a/src/nxt_http_request.c +++ b/src/nxt_http_request.c @@ -946,6 +946,10 @@ nxt_http_arguments_parse(nxt_http_request_t *r) return NULL; } + if (nxt_slow_path(r->args->start == NULL)) { + goto end; + } + hash = NXT_HTTP_FIELD_HASH_INIT; name = NULL; name_length = 0; @@ -1026,6 +1030,8 @@ nxt_http_arguments_parse(nxt_http_request_t *r) } } +end: + r->arguments = args; return args; -- cgit From 64f4c78bf441fa9e021d905a03d374d0a9e05e8d Mon Sep 17 00:00:00 2001 From: Zhidao HONG Date: Tue, 11 Jun 2024 17:59:00 +0800 Subject: http: Support chunked request bodies This is a temporary support for chunked request bodies by converting to Content-Length. This allows for processing of such requests until a more permanent solution is developed. A new configuration option "chunked_transform" has been added to enable this feature. The option can be set as follows: { "settings": { "chunked_transform": true } } By default, this option is set to false, which retains the current behaviour of rejecting chunked requests with a '411 Length Required' status code. Please note that this is an experimental implementation. Reviewed-by: Andrew Clayton --- src/nxt_http_request.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) (limited to 'src/nxt_http_request.c') diff --git a/src/nxt_http_request.c b/src/nxt_http_request.c index 425a4607..54d1bd27 100644 --- a/src/nxt_http_request.c +++ b/src/nxt_http_request.c @@ -540,15 +540,58 @@ static const nxt_http_request_state_t nxt_http_request_body_state }; +static nxt_int_t +nxt_http_request_chunked_transform(nxt_http_request_t *r) +{ + size_t size; + u_char *p, *end; + nxt_http_field_t *f; + + r->chunked_field->skip = 1; + + size = r->body->file_end; + + f = nxt_list_zero_add(r->fields); + if (nxt_slow_path(f == NULL)) { + return NXT_ERROR; + } + + nxt_http_field_name_set(f, "Content-Length"); + + p = nxt_mp_nget(r->mem_pool, NXT_OFF_T_LEN); + if (nxt_slow_path(p == NULL)) { + return NXT_ERROR; + } + + f->value = p; + end = nxt_sprintf(p, p + NXT_OFF_T_LEN, "%uz", size); + f->value_length = end - p; + + r->content_length = f; + r->content_length_n = size; + + return NXT_OK; +} + + static void nxt_http_request_ready(nxt_task_t *task, void *obj, void *data) { + nxt_int_t ret; nxt_http_action_t *action; nxt_http_request_t *r; r = obj; action = r->conf->socket_conf->action; + if (r->chunked) { + ret = nxt_http_request_chunked_transform(r); + if (nxt_slow_path(ret != NXT_OK)) { + nxt_http_request_error(task, r, NXT_HTTP_INTERNAL_SERVER_ERROR); + return; + } + } + nxt_http_request_action(task, r, action); } -- cgit From 2eecee7520558a8f72e98e839a77330712a946b4 Mon Sep 17 00:00:00 2001 From: Zhidao HONG Date: Thu, 18 Apr 2024 18:16:01 +0800 Subject: var: Restrict nxt_tstr_query() to only support synchronous operation Initially, variable query was designed to accomodate both synchronous and asynchronous operations. However, upon consideration of actual requirements, we recognized that asynchronous support was not needed. The refactoring ensures that the success or failure of the variable query operation is now directly indicated by its return value. This change streamlines the function's usage and enhances code clarity, as it facilitates immediate error handling without the need for asynchronous callbacks or additional error checking functions. Note the patch only works for Unit native variables but not njs variables. --- src/nxt_http_request.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src/nxt_http_request.c') diff --git a/src/nxt_http_request.c b/src/nxt_http_request.c index 54d1bd27..ccd2b141 100644 --- a/src/nxt_http_request.c +++ b/src/nxt_http_request.c @@ -936,9 +936,8 @@ nxt_http_request_access_log(nxt_task_t *task, nxt_http_request_t *r, 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))) { + ret = nxt_tstr_query(task, r->tstr_query, rtcf->log_expr, &str); + if (nxt_slow_path(ret != NXT_OK)) { return NXT_DECLINED; } } -- cgit From 82e168fe01306cf03f8eaba284f31f7285347424 Mon Sep 17 00:00:00 2001 From: Zhidao HONG Date: Fri, 16 Aug 2024 00:06:45 +0800 Subject: http: Refactor out nxt_tstr_cond_t from the access log module This nxt_tstr_cond_t will be reused for the feature of adding "if" option to the "match" object. The two "if" options have the same usage. --- src/nxt_http_request.c | 80 +++++++++++++++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 34 deletions(-) (limited to 'src/nxt_http_request.c') diff --git a/src/nxt_http_request.c b/src/nxt_http_request.c index ccd2b141..a65163d0 100644 --- a/src/nxt_http_request.c +++ b/src/nxt_http_request.c @@ -915,44 +915,11 @@ 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; - 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; - } - - ret = nxt_tstr_query(task, r->tstr_query, rtcf->log_expr, &str); - if (nxt_slow_path(ret != NXT_OK)) { - 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) { + if (nxt_http_cond_value(task, r, &rtcf->log_cond)) { access_log->handler(task, r, access_log, rtcf->log_format); return NXT_OK; } @@ -1364,3 +1331,48 @@ nxt_http_cookie_hash(nxt_mp_t *mp, nxt_str_t *name) { return nxt_http_field_hash(mp, name, 1, NXT_HTTP_URI_ENCODING_NONE); } + + +int +nxt_http_cond_value(nxt_task_t *task, nxt_http_request_t *r, + nxt_tstr_cond_t *cond) +{ + nxt_int_t ret; + nxt_str_t str; + nxt_bool_t expr; + nxt_router_conf_t *rtcf; + + rtcf = r->conf->socket_conf->router_conf; + + expr = 1; + + if (cond->expr != NULL) { + + if (nxt_tstr_is_const(cond->expr)) { + nxt_tstr_str(cond->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 -1; + } + + ret = nxt_tstr_query(task, r->tstr_query, cond->expr, &str); + if (nxt_slow_path(ret != NXT_OK)) { + return -1; + } + } + + 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; + } + } + + return cond->negate ^ expr; +} -- cgit From 57f939569d3c3db11d8bbc7dd0b45693cbe3344e Mon Sep 17 00:00:00 2001 From: Zhidao HONG Date: Fri, 16 Aug 2024 00:09:22 +0800 Subject: http: Get rid of nxt_http_request_access_log() --- src/nxt_http_request.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) (limited to 'src/nxt_http_request.c') diff --git a/src/nxt_http_request.c b/src/nxt_http_request.c index a65163d0..a7e9ff69 100644 --- a/src/nxt_http_request.c +++ b/src/nxt_http_request.c @@ -24,8 +24,6 @@ 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 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); @@ -861,12 +859,12 @@ 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; nxt_http_protocol_t protocol; nxt_socket_conf_joint_t *conf; + nxt_router_access_log_t *access_log; r = obj; proto.any = data; @@ -878,8 +876,10 @@ nxt_http_request_close_handler(nxt_task_t *task, void *obj, void *data) r->logged = 1; if (rtcf->access_log != NULL) { - ret = nxt_http_request_access_log(task, r, rtcf); - if (ret == NXT_OK) { + access_log = rtcf->access_log; + + if (nxt_http_cond_value(task, r, &rtcf->log_cond)) { + access_log->handler(task, r, access_log, rtcf->log_format); return; } } @@ -911,23 +911,6 @@ nxt_http_request_close_handler(nxt_task_t *task, void *obj, void *data) } -static nxt_int_t -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; - - if (nxt_http_cond_value(task, r, &rtcf->log_cond)) { - access_log->handler(task, r, access_log, rtcf->log_format); - return NXT_OK; - } - - return NXT_DECLINED; -} - - static u_char * nxt_http_date_cache_handler(u_char *buf, nxt_realtime_t *now, struct tm *tm, size_t size, const char *format) -- cgit