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 + 2 files changed, 32 insertions(+), 16 deletions(-) (limited to 'src') 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; -- 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(-) (limited to 'src') 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(-) (limited to 'src') 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 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(-) (limited to 'src') 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 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(-) (limited to 'src') 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. --- src/nxt_conf_validation.c | 38 ++++++++++++++++++++++++++++++++++---- src/nxt_http.h | 7 +++++++ src/nxt_http_return.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/nxt_http_route.c | 12 ++++++++++++ 4 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 src/nxt_http_return.c (limited to 'src') 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(-) (limited to 'src') 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(-) (limited to 'src') 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(-) (limited to 'src') 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 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(-) (limited to 'src') 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 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'src') 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: -- 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. --- src/nxt_unit.c | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-) (limited to 'src') 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; -- 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(-) (limited to 'src') 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(-) (limited to 'src') 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 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(-) (limited to 'src') 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 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(-) (limited to 'src') 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(-) (limited to 'src') 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(-) (limited to 'src') 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(-) (limited to 'src') 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(-) (limited to 'src') 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 +- 4 files changed, 89 insertions(+), 14 deletions(-) (limited to 'src') 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; } -- 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. --- src/nxt_process.c | 17 ++++++++++++----- src/nxt_process.h | 4 +++- src/nxt_router.c | 6 +++--- src/nxt_unit.c | 28 +++++++++++++++++++++++++--- 4 files changed, 43 insertions(+), 12 deletions(-) (limited to 'src') 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 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(-) (limited to 'src') 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(+) (limited to 'src') 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(-) (limited to 'src') 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(-) (limited to 'src') 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(-) (limited to 'src') 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