[njs] Ignoring UndefinedBehaviorSanitizer warnings where appropriate.
details: https://hg.nginx.org/njs/rev/0490f1ae4cf5
branches:
changeset: 2258:0490f1ae4cf5
user: Dmitry Volyntsev
date: Sun Jul 30 10:21:51 2023 +0100
description:
Ignoring UndefinedBehaviorSanitizer warnings where appropriate.
Prodded by David Carlier and Ben Kallus.
diffstat:
auto/clang| 11 +++
src/njs_clang.h | 25 +
src/njs_number.c | 4 +++-
src/njs_number.h | 2 +-
src/njs_typed_array.c | 2 +-
5 files changed, 41 insertions(+), 3 deletions(-)
diffs (94 lines):
diff -r 275d785ab5bf -r 0490f1ae4cf5 auto/clang
--- a/auto/clangMon Jan 08 16:40:42 2024 -0800
+++ b/auto/clangSun Jul 30 10:21:51 2023 +0100
@@ -161,6 +161,17 @@ njs_feature_test="int main(int argc, cha
. auto/feature
+njs_feature="GCC __attribute__ no_sanitize"
+njs_feature_name=NJS_HAVE_GCC_ATTRIBUTE_NO_SANITIZE
+njs_feature_run=no
+njs_feature_path=
+njs_feature_libs=
+njs_feature_test="__attribute__((no_sanitize(\"undefined\"))) int main(void) {
+return 0;
+ }"
+. auto/feature
+
+
njs_feature="Address sanitizer"
njs_feature_name=NJS_HAVE_ADDRESS_SANITIZER
njs_feature_run=no
diff -r 275d785ab5bf -r 0490f1ae4cf5 src/njs_clang.h
--- a/src/njs_clang.h Mon Jan 08 16:40:42 2024 -0800
+++ b/src/njs_clang.h Sun Jul 30 10:21:51 2023 +0100
@@ -183,6 +183,31 @@ njs_leading_zeros64(uint64_t x)
#define njs_msan_unpoison(ptr, size)
#endif
+#if (NJS_HAVE_GCC_ATTRIBUTE_NO_SANITIZE)
+#define NJS_NOSANITIZE(options) __attribute__((no_sanitize(options)))
+#else
+#define NJS_NOSANITIZE(options)
+#endif
+
+
+njs_inline NJS_NOSANITIZE("float-cast-overflow") int64_t
+njs_unsafe_cast_double_to_int64(double num)
+{
+ /*
+* Casting NaN to integer is undefined behavior,
+* but it is fine in some cases where we do additional checks later.
+* For example:
+* int64_t i64 = njs_unsafe_cast_double_to_int64(num);
+* if (i64 == num) {
+*// num is integer
+* }
+*
+* We do this as inline function to avoid UndefinedBehaviorSanitizer
+* warnings.
+*/
+ return (int64_t) num;
+}
+
#if (NJS_HAVE_DENORMALS_CONTROL)
#include
diff -r 275d785ab5bf -r 0490f1ae4cf5 src/njs_number.c
--- a/src/njs_number.c Mon Jan 08 16:40:42 2024 -0800
+++ b/src/njs_number.c Sun Jul 30 10:21:51 2023 +0100
@@ -382,7 +382,9 @@ njs_number_is_safe_integer(njs_vm_t *vm,
if (nargs > 1 && njs_is_number(&args[1])) {
num = njs_number(&args[1]);
-if (num == (int64_t) num && fabs(num) <= NJS_MAX_SAFE_INTEGER) {
+if (num == njs_unsafe_cast_double_to_int64(num)
+&& fabs(num) <= NJS_MAX_SAFE_INTEGER)
+{
integer = 1;
}
}
diff -r 275d785ab5bf -r 0490f1ae4cf5 src/njs_number.h
--- a/src/njs_number.h Mon Jan 08 16:40:42 2024 -0800
+++ b/src/njs_number.h Sun Jul 30 10:21:51 2023 +0100
@@ -36,7 +36,7 @@ njs_int_t njs_number_parse_float(njs_vm_
njs_uint_t nargs, njs_index_t unused, njs_value_t *retval);
-njs_inline njs_bool_t
+njs_inline NJS_NOSANITIZE("float-cast-overflow") njs_bool_t
njs_number_is_integer_index(double num)
{
uint32_t u32;
diff -r 275d785ab5bf -r 0490f1ae4cf5 src/njs_typed_array.c
--- a/src/njs_typed_array.c Mon Jan 08 16:40:42 2024 -0800
+++ b/src/njs_typed_array.c Sun Jul 30 10:21:51 2023 +0100
@@ -1388,7 +1388,7 @@ njs_typed_array_prototype_index_of(njs_v
v = njs_number(njs_argument(args, 1));
-i64 = v;
+i64 = njs_unsafe_cast_double_to_int64(v);
integer = (v == i64);
buffer = array->buffer;
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 4 of 4] AIO operations now add timers (ticket #2162)
Hello! On Mon, Jan 08, 2024 at 01:31:11PM +, J Carter wrote: > On Mon, 8 Jan 2024 11:25:55 + > J Carter wrote: > > > Hello, > > > > On Mon, 27 Nov 2023 05:50:27 +0300 > > Maxim Dounin wrote: > > > > > # HG changeset patch > > > # User Maxim Dounin > > > # Date 1701050170 -10800 > > > # Mon Nov 27 04:56:10 2023 +0300 > > > # Node ID 00c3e7333145ddb5ea0eeaaa66b3d9c26973c9c2 > > > # Parent 61d08e4cf97cc073200ec32fc6ada9a2d48ffe51 > > > AIO operations now add timers (ticket #2162). > > > > > > Each AIO (thread IO) operation being run is now accompanied with 1-minute > > > timer. This timer prevents unexpected shutdown of the worker process > > > while > > > an AIO operation is running, and logs an alert if the operation is running > > > for too long. > > > > Shouldn't this timer's duration be set to match worker_shutdown_timeout's > > duration rather than being hard coded to 60s ? > > Ah nevermind, I understand. > > These timers will either expire from passing the 60s set duration, or > will expire as worker_process_timeout itself expires, kills the > connection and times out associated timers (including the aio timers). > > Setting it to worker_shutdown_timeout's duration would be pointless > (an 'infinite' timer would give the same result). > > So the only situation in which a different value for these AIO > timers would make sense is if these AIO operations are expected to > take longer 60s, but less than worker_shutdown_timeout (in cases > where it has been increased from it's own default of 60s). > > In that case the AIO operation's timeout would have to be one > (or more) of it's own directives, with a value less than > worker_shutdown_timeout. Not really. When worker_shutdown_timeout expires, it tries to terminate the request, but it can't as long as an AIO operation is running. When the AIO operation completes, the request will be actually terminated and the worker process will be allowed to exit. So far so good. But if the AIO operation never completes, the timer will expire after 1 minute, will log an alert, and the worker processes will be anyway allowed to exit (with the request still present). This might not be actually possible though - for example, ngx_thread_pool_exit_worker() will just block waiting for all pending operations to complete. In theory, the situation when an AIO operation never completes should never happen, and just a counter of pending AIO operations can be used instead to delay shutdown (which is essentially equivalent to an infinite timer). In practice, though, using a large enough guard timer might be better: it provides additional level of protection against bugs or system misbehaviour, and at least logs an alert if something really weird happens. It is also looks more universal and in line with current approach of using existing timers as an indicator that something is going on and shutdown should be delayed. The timer is expected to be "large enough", since we cannot do anything meaningful with an AIO operation which never completes, we can only complain loudly, so the timer should never expire unless there is something really wrong. This is not the case with worker_shutdown_timeout: it can be set to an arbitrary low value, which is not expected to mean that something is really wrong if the timer expires, but rather means that nginx shouldn't try to process remaining requests, but should instead close them as long as it can do so. That is, worker_shutdown_timeout does not fit semantically. Further, worker_shutdown_timeout is not set by default, so it simply cannot be used. The 1 minute was chosen as it matches default send_timeout, which typically accompanies AIO operations when sending responses (and also delays shutdown, so no "open socket left" alerts are normally seen). Still, send_timeout is also quite different semantically, and therefore a hardcoded value is used instead. I don't think there are valid cases when AIO operations can take longer than 1 minute, as these are considered to be small and fast operations, which are normally done synchronously within nginx event loop when not using AIO, and an operations which takes 1m would mean nginx is completely unresponsive. Still, if we'll found out that 1 minute is not large enough in some cases, we can just bump it to a larger value. [...] -- Maxim Dounin http://mdounin.ru/ ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] SSL: raised limit for upstream session size
Hello! On Tue, Dec 26, 2023 at 12:29:54AM +0400, Sergey Kandaurov wrote: > > On 23 Dec 2023, at 01:46, Maxim Dounin wrote: > > > > Hello! > > > > On Fri, Dec 22, 2023 at 06:28:34PM +0400, Sergey Kandaurov wrote: > > > >> # HG changeset patch > >> # User Sergey Kandaurov > >> # Date 1703255284 -14400 > >> # Fri Dec 22 18:28:04 2023 +0400 > >> # Node ID a463fb67e143c051fd373d1df94e5813a37d5cea > >> # Parent 44266e0651c44f530c4aa66e68c1b9464a9acee7 > >> SSL: raised limit for upstream session size. > >> > >> Unlike shared session cache used to store multiple client SSL sessions and > >> which may be per a single SSL connection, sessions saved from upstream are > >> per upstream server peer, so there is no such multiplier effect, but they > >> may be of noticeably larger size due to session tickets being used. > >> > >> It was observed that session tickets sent from JVM backends may result in > >> a decoded session size nearly the previous maximum session size limit of > >> 4096 or slightly beyond. Raising the limit allows to save such sessions. > > > > Session tickets are not expected to be larger than sessions > > itself, except by several bytes used for key identification and > > encryption overhead. I see no reasons why the limit should be > > different in different places. > > > > And 4096 for an SSL session looks a lot. The only justification I > > can assume here is an SSL session with the client certificate (or > > even certificate chain) being saved into the session. It might > > worth looking into what actually happens here. > > > > Indeed. Both local and peer certificate chains are serialized and > encrypted as part of constructing a session ticket. Per the original > change to support tickets, this is hardcoded and may not be adjusted: > https://hg.openjdk.org/jdk/jdk/rev/c2398053ee90#l4.352 > https://hg.openjdk.org/jdk/jdk/rev/c2398053ee90#l10.261 From my limited understanding of the JDK code, at least peerCerts seems to contain only certificates actually sent by the client, which is understandable (links to Github, since hg.openjdk.org used to be unresponsive when writing this, and returned 504 for almost all requests): https://github.com/openjdk/jdk/blob/4fc6b0ffa4f771991a5ebd982b5133d2e364fdae/src/java.base/share/classes/sun/security/ssl/CertificateMessage.java#L416 But localCerts seems to be always set on the server side, with all the certificates being sent to the client: https://github.com/openjdk/jdk/blob/4fc6b0ffa4f771991a5ebd982b5133d2e364fdae/src/java.base/share/classes/sun/security/ssl/CertificateMessage.java#L265 This looks like an issue on the JDK side: there are no reasons why server certificates needs to be saved into the session on the server, as they are readily available on the server. Further, relevant saving code seems to be commented as "Client identity", which suggests that these might be saved unintentionally with an assumption that the code is only used on the client (OTOH, I don't see why the client needs its own certificates in the session as well). Do you have an affected JVM backend on hand to confirm it's indeed the case? I tend to think this needs to be reported to JDK developers. Their current code results in sending server certificate chain to the client at least two times (once in the handshake itself, and at least once in the ticket; not to mention that there can be more than one ticket in TLSv1.3), and they might reconsider doing this. (Funny enough, they seems to be using cache to deserialize certificates from such tickets, see https://bugs.openjdk.org/browse/JDK-8286433 for details.) Meanwhile, we can consider implementing a workaround on our side (that is, raising the limit, though I don't think there should be separate limits; also, I'm somewhat concerned about using 8k buffers on stack, we currently don't use anything larger than 4k) or instead focus on providing some guidance to users of affected JVM backends (I guess switching off tickets and/or TLSv1.3 should be enough in most cases). -- Maxim Dounin http://mdounin.ru/ ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] Fixed RegExp.prototype.exec() when second argument is absent.
details: https://hg.nginx.org/njs/rev/275d785ab5bf
branches:
changeset: 2257:275d785ab5bf
user: Dmitry Volyntsev
date: Mon Jan 08 16:40:42 2024 -0800
description:
Fixed RegExp.prototype.exec() when second argument is absent.
Previously, when the second argument is undefined, NaN is casted to
unsigned which is undefined behavior.
Found by UndefinedBehaviorSanitizer.
diffstat:
src/njs_regexp.c | 11 +--
1 files changed, 9 insertions(+), 2 deletions(-)
diffs (28 lines):
diff -r 41d0de3ad198 -r 275d785ab5bf src/njs_regexp.c
--- a/src/njs_regexp.c Mon Jan 08 16:40:42 2024 -0800
+++ b/src/njs_regexp.c Mon Jan 08 16:40:42 2024 -0800
@@ -1235,6 +1235,7 @@ njs_int_t
njs_regexp_prototype_exec(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
njs_index_t unused, njs_value_t *retval)
{
+unsigned flags;
njs_int_tret;
njs_value_t *r, *s;
njs_value_t string_lvalue;
@@ -1253,8 +1254,14 @@ njs_regexp_prototype_exec(njs_vm_t *vm,
return ret;
}
-return njs_regexp_builtin_exec(vm, r, s,
- njs_number(njs_arg(args, nargs, 2)),
retval);
+if (nargs > 2) {
+flags = njs_number(njs_arg(args, nargs, 2));
+
+} else {
+flags = 0;
+}
+
+return njs_regexp_builtin_exec(vm, r, s, flags, retval);
}
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] Fixed initialization of external prototypes with object entry.
details: https://hg.nginx.org/njs/rev/ee4d396aa418
branches:
changeset: 2255:ee4d396aa418
user: Dmitry Volyntsev
date: Mon Jan 08 16:40:42 2024 -0800
description:
Fixed initialization of external prototypes with object entry.
When external was NULL (for example, when .u.object.properties is not
declared), an arithmetic operation was performed with NULL pointer which
is undefined behavior.
Found by UndefinedBehaviorSanitizer.
diffstat:
src/njs_extern.c | 4
1 files changed, 4 insertions(+), 0 deletions(-)
diffs (14 lines):
diff -r c43745da92cd -r ee4d396aa418 src/njs_extern.c
--- a/src/njs_extern.c Mon Jan 08 16:40:42 2024 -0800
+++ b/src/njs_extern.c Mon Jan 08 16:40:42 2024 -0800
@@ -34,6 +34,10 @@ njs_external_add(njs_vm_t *vm, njs_arr_t
hash = &slot->external_shared_hash;
njs_lvlhsh_init(hash);
+if (n == 0) {
+return NJS_OK;
+}
+
lhq.replace = 0;
lhq.proto = &njs_object_hash_proto;
lhq.pool = vm->mem_pool;
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] Improved array enumeration with length 0.
details: https://hg.nginx.org/njs/rev/41d0de3ad198
branches:
changeset: 2256:41d0de3ad198
user: Dmitry Volyntsev
date: Mon Jan 08 16:40:42 2024 -0800
description:
Improved array enumeration with length 0.
The fix eliminates an arithmetic operation with NULL pointer.
Found by UndefinedBehaviorSanitizer.
diffstat:
src/njs_object.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diffs (12 lines):
diff -r ee4d396aa418 -r 41d0de3ad198 src/njs_object.c
--- a/src/njs_object.c Mon Jan 08 16:40:42 2024 -0800
+++ b/src/njs_object.c Mon Jan 08 16:40:42 2024 -0800
@@ -591,7 +591,7 @@ njs_object_enumerate_array(njs_vm_t *vm,
njs_value_t *p, *start, *end;
njs_array_t *entry;
-if (!array->object.fast_array) {
+if (!array->object.fast_array || array->length == 0) {
return NJS_OK;
}
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] QueryString: fixed underflow in parse().
details: https://hg.nginx.org/njs/rev/c43745da92cd
branches:
changeset: 2254:c43745da92cd
user: Dmitry Volyntsev
date: Mon Jan 08 16:40:42 2024 -0800
description:
QueryString: fixed underflow in parse().
Previously, njs_query_string_append() might be provided with invalid
val_size value when value in a key-value pair was absent.
Found by UndefinedBehaviorSanitizer.
diffstat:
external/njs_query_string_module.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diffs (12 lines):
diff -r 9fadb2e9c6ea -r c43745da92cd external/njs_query_string_module.c
--- a/external/njs_query_string_module.cMon Jan 08 16:40:42 2024 -0800
+++ b/external/njs_query_string_module.cMon Jan 08 16:40:42 2024 -0800
@@ -506,7 +506,7 @@ njs_query_string_parser(njs_vm_t *vm, u_
size = val - key;
-if (val != end) {
+if (val != part) {
val += eq->length;
}
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] Fixed external values initialization in unit tests.
details: https://hg.nginx.org/njs/rev/9fadb2e9c6ea
branches:
changeset: 2253:9fadb2e9c6ea
user: Dmitry Volyntsev
date: Mon Jan 08 16:40:42 2024 -0800
description:
Fixed external values initialization in unit tests.
Since 0.8.0 modules can create their own constructors and prototypes.
A modules has two method: preinit() and init(). A module should
add its constructors and prototypes in preinit() and create its own
values in init(). Creating a value in preinit() results in an error.
The patch fixes the issue by creating an external value in init()
instead of preinit().
Found by UndefinedBehaviorSanitizer.
diffstat:
src/test/njs_externals_test.c | 136 ++---
1 files changed, 72 insertions(+), 64 deletions(-)
diffs (176 lines):
diff -r 721475693b80 -r 9fadb2e9c6ea src/test/njs_externals_test.c
--- a/src/test/njs_externals_test.c Mon Jan 08 16:40:42 2024 -0800
+++ b/src/test/njs_externals_test.c Mon Jan 08 16:40:42 2024 -0800
@@ -28,6 +28,7 @@ typedef struct {
static njs_int_t njs_externals_262_init(njs_vm_t *vm);
static njs_int_t njs_externals_shared_preinit(njs_vm_t *vm);
+static njs_int_t njs_externals_shared_init(njs_vm_t *vm);
njs_int_t njs_array_buffer_detach(njs_vm_t *vm, njs_value_t *args,
njs_uint_t nargs, njs_index_t unused, njs_value_t *retval);
@@ -47,7 +48,7 @@ njs_module_t njs_unit_test_262_module =
njs_module_t njs_unit_test_external_module = {
.name = njs_str("external"),
.preinit = njs_externals_shared_preinit,
-.init = NULL,
+.init = njs_externals_shared_init,
};
@@ -1253,77 +1254,15 @@ njs_externals_init_internal(njs_vm_t *vm
{
njs_int_t ret;
njs_uint_ti, j;
-njs_function_t*f;
-njs_opaque_value_tvalue;
njs_unit_test_req_t *requests;
njs_unit_test_prop_t *prop;
-static const njs_str_t external_ctor = njs_str("ExternalConstructor");
-static const njs_str_t external_null = njs_str("ExternalNull");
-static const njs_str_t external_error = njs_str("ExternalError");
-
-if (shared) {
-njs_external_r_proto_id = njs_vm_external_prototype(vm,
- njs_unit_test_r_external,
- njs_nitems(njs_unit_test_r_external));
-if (njs_slow_path(njs_external_r_proto_id < 0)) {
-njs_printf("njs_vm_external_prototype() failed\n");
-return NJS_ERROR;
-}
-
-f = njs_vm_function_alloc(vm, njs_unit_test_constructor, 1, 1);
-if (f == NULL) {
-njs_printf("njs_vm_function_alloc() failed\n");
-return NJS_ERROR;
-}
-
-njs_value_function_set(njs_value_arg(&value), f);
-
-ret = njs_vm_bind(vm, &external_ctor, njs_value_arg(&value), 1);
-if (njs_slow_path(ret != NJS_OK)) {
-njs_printf("njs_vm_bind() failed\n");
-return NJS_ERROR;
-}
-
-njs_external_null_proto_id = njs_vm_external_prototype(vm,
- njs_unit_test_null_external,
-
njs_nitems(njs_unit_test_null_external));
-if (njs_slow_path(njs_external_null_proto_id < 0)) {
-njs_printf("njs_vm_external_prototype() failed\n");
-return NJS_ERROR;
-}
-
-ret = njs_vm_external_create(vm, njs_value_arg(&value),
- njs_external_null_proto_id, NULL, 1);
-if (njs_slow_path(ret != NJS_OK)) {
-return NJS_ERROR;
-}
-
-ret = njs_vm_bind(vm, &external_null, njs_value_arg(&value), 1);
-if (njs_slow_path(ret != NJS_OK)) {
-njs_printf("njs_vm_bind() failed\n");
-return NJS_ERROR;
-}
-
-njs_external_error_ctor_id =
-njs_vm_external_constructor(vm, &external_error,
- njs_error_constructor, njs_unit_test_ctor_props,
- njs_nitems(njs_unit_test_ctor_props),
- njs_unit_test_proto_props,
- njs_nitems(njs_unit_test_proto_props));
-if (njs_slow_path(njs_external_error_ctor_id < 0)) {
-njs_printf("njs_vm_external_constructor() failed\n");
-return NJS_ERROR;
-}
-}
-
requests = njs_mp_zalloc(vm->mem_pool, n * sizeof(njs_unit_test_req_t));
if (njs_slow_path(requests == NULL)) {
return NJS_ERROR;
}
for (i = 0; i < n; i++) {
-
requests[i] = init[i].request;
ret = njs_vm_external_create(vm, njs_value_arg(&requests[i].value),
@@ -1396,7 +1335,76 @@ njs_externals_262_init(njs_vm_t *vm)
static njs_int_t
njs_externals_shared_preinit(njs_vm_t *vm)
{
-return njs_externals_init_internal(vm, njs_test_requests, 1, 1);
+static const njs_str_t external_error = njs_str("ExternalError");
+
+njs_external_r_proto_id = njs_vm_external_p
[njs] Unifying hash function prototypes.
details: https://hg.nginx.org/njs/rev/721475693b80
branches:
changeset: 2252:721475693b80
user: Dmitry Volyntsev
date: Mon Jan 08 16:40:42 2024 -0800
description:
Unifying hash function prototypes.
This fixes UndefinedBehaviorSanitizer warning "call to function through
pointer to incorrect function type".
Found by UndefinedBehaviorSanitizer.
diffstat:
external/njs_crypto_module.c | 71 +--
src/njs_hash.h | 32 +++
src/njs_main.h | 4 +-
src/njs_md5.c| 10 +++---
src/njs_md5.h| 23 --
src/njs_sha1.c | 10 +++---
src/njs_sha1.h | 24 --
src/njs_sha2.c | 10 +++---
src/njs_sha2.h | 24 --
9 files changed, 77 insertions(+), 131 deletions(-)
diffs (424 lines):
diff -r 57071ecadeb5 -r 721475693b80 external/njs_crypto_module.c
--- a/external/njs_crypto_module.c Mon Jan 08 16:40:27 2024 -0800
+++ b/external/njs_crypto_module.c Mon Jan 08 16:40:42 2024 -0800
@@ -6,16 +6,14 @@
#include
-#include
-#include
-#include
+#include
#include
#include
-typedef void (*njs_hash_init)(void *ctx);
-typedef void (*njs_hash_update)(void *ctx, const void *data, size_t size);
-typedef void (*njs_hash_final)(u_char *result, void *ctx);
+typedef void (*njs_hash_init)(njs_hash_t *ctx);
+typedef void (*njs_hash_update)(njs_hash_t *ctx, const void *data, size_t
size);
+typedef void (*njs_hash_final)(u_char result[32], njs_hash_t *ctx);
typedef njs_int_t (*njs_digest_encode)(njs_vm_t *vm, njs_value_t *value,
const njs_str_t *src);
@@ -31,24 +29,13 @@ typedef struct {
} njs_hash_alg_t;
typedef struct {
-union {
-njs_md5_t md5;
-njs_sha1_t sha1;
-njs_sha2_t sha2;
-} u;
-
+njs_hash_t ctx;
njs_hash_alg_t *alg;
} njs_digest_t;
typedef struct {
u_char opad[64];
-
-union {
-njs_md5_t md5;
-njs_sha1_t sha1;
-njs_sha2_t sha2;
-} u;
-
+njs_hash_t ctx;
njs_hash_alg_t *alg;
} njs_hmac_t;
@@ -85,25 +72,25 @@ static njs_hash_alg_t njs_hash_algorithm
{
njs_str("md5"),
16,
- (njs_hash_init) njs_md5_init,
- (njs_hash_update) njs_md5_update,
- (njs_hash_final) njs_md5_final
+ njs_md5_init,
+ njs_md5_update,
+ njs_md5_final
},
{
njs_str("sha1"),
20,
- (njs_hash_init) njs_sha1_init,
- (njs_hash_update) njs_sha1_update,
- (njs_hash_final) njs_sha1_final
+ njs_sha1_init,
+ njs_sha1_update,
+ njs_sha1_final
},
{
njs_str("sha256"),
32,
- (njs_hash_init) njs_sha2_init,
- (njs_hash_update) njs_sha2_update,
- (njs_hash_final) njs_sha2_final
+ njs_sha2_init,
+ njs_sha2_update,
+ njs_sha2_final
},
{
@@ -312,7 +299,7 @@ njs_crypto_create_hash(njs_vm_t *vm, njs
dgst->alg = alg;
-alg->init(&dgst->u);
+alg->init(&dgst->ctx);
return njs_vm_external_create(vm, retval, njs_crypto_hash_proto_id,
dgst, 0);
@@ -390,10 +377,10 @@ njs_hash_prototype_update(njs_vm_t *vm,
}
if (!hmac) {
-dgst->alg->update(&dgst->u, data.start, data.length);
+dgst->alg->update(&dgst->ctx, data.start, data.length);
} else {
-ctx->alg->update(&ctx->u, data.start, data.length);
+ctx->alg->update(&ctx->ctx, data.start, data.length);
}
njs_value_assign(retval, this);
@@ -450,17 +437,17 @@ njs_hash_prototype_digest(njs_vm_t *vm,
if (!hmac) {
alg = dgst->alg;
-alg->final(digest, &dgst->u);
+alg->final(digest, &dgst->ctx);
dgst->alg = NULL;
} else {
alg = ctx->alg;
-alg->final(hash1, &ctx->u);
+alg->final(hash1, &ctx->ctx);
-alg->init(&ctx->u);
-alg->update(&ctx->u, ctx->opad, 64);
-alg->update(&ctx->u, hash1, alg->size);
-alg->final(digest, &ctx->u);
+alg->init(&ctx->ctx);
+alg->update(&ctx->ctx, ctx->opad, 64);
+alg->update(&ctx->ctx, hash1, alg->size);
+alg->final(digest, &ctx->ctx);
ctx->alg = NULL;
}
@@ -562,9 +549,9 @@ njs_crypto_create_hmac(njs_vm_t *vm, njs
ctx->alg = alg;
if (key.length > sizeof(key_buf)) {
-alg->init(&ctx->u);
-alg->update(&ctx->u, key.start, key.length);
-alg->final(digest, &ctx->u);
+alg->init(&ctx->ctx);
+alg->update(&ctx->ctx, key.start, key.length);
+alg->final(digest, &ctx->ctx);
memcpy(key_buf, digest, alg->size);
njs_explicit_memzero(key_buf + alg->size, sizeof(key_buf) - alg->size);
@@ -583,8 +570,8 @@ njs_crypto_create_hmac(njs_vm_t *vm, njs
key_buf[i] ^= 0x36;
}
-alg->init(&ctx->u);
-alg->update(&ctx->u, key_buf,
[njs] Fixed Date constructor for overflows and with NaN values.
details: https://hg.nginx.org/njs/rev/57071ecadeb5
branches:
changeset: 2251:57071ecadeb5
user: Dmitry Volyntsev
date: Mon Jan 08 16:40:27 2024 -0800
description:
Fixed Date constructor for overflows and with NaN values.
Found by UndefinedBehaviorSanitizer.
diffstat:
src/njs_date.c | 8
1 files changed, 8 insertions(+), 0 deletions(-)
diffs (23 lines):
diff -r 4a15613f4e8b -r 57071ecadeb5 src/njs_date.c
--- a/src/njs_date.cTue Dec 19 12:37:05 2023 -0800
+++ b/src/njs_date.cMon Jan 08 16:40:27 2024 -0800
@@ -243,11 +243,19 @@ njs_make_date(int64_t tm[], njs_bool_t l
days = njs_make_day(tm[NJS_DATE_YR], tm[NJS_DATE_MON],
tm[NJS_DATE_DAY]);
+if (njs_slow_path(isnan(days))) {
+return NAN;
+}
+
time = ((tm[NJS_DATE_HR] * 60.0 + tm[NJS_DATE_MIN]) * 60.0
+ tm[NJS_DATE_SEC]) * 1000.0 + tm[NJS_DATE_MSEC];
time += days * 8640.0;
+if (time < -8.64e15 || time > 8.64e15) {
+return NAN;
+}
+
if (local) {
time += njs_tz_offset(time) * 6;
}
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 1 of 6] Stream: using ngx_stream_ssl_srv_conf_t *sscf naming convention
Hi,
On Fri, Dec 15, 2023 at 07:37:44PM +0400, Sergey Kandaurov wrote:
> # HG changeset patch
> # User Sergey Kandaurov
> # Date 1702646778 -14400
> # Fri Dec 15 17:26:18 2023 +0400
> # Node ID cb377d36446e1ce22b71848a4a138564b2e38719
> # Parent 763803589a36e3c67cbe39dd324b4e91fe57ecb7
> Stream: using ngx_stream_ssl_srv_conf_t *sscf naming convention.
>
> Originally, the stream module was developed based on the mail module,
> following the existing style. Then it was diverged to closely follow
> the http module development. This change updates style to use sscf
> naming convention troughout the stream module, which matches the http
> module code style. No functional changes.
>
> diff --git a/src/stream/ngx_stream_ssl_module.c
> b/src/stream/ngx_stream_ssl_module.c
> --- a/src/stream/ngx_stream_ssl_module.c
> +++ b/src/stream/ngx_stream_ssl_module.c
> @@ -40,12 +40,12 @@ static ngx_int_t ngx_stream_ssl_variable
> ngx_stream_variable_value_t *v, uintptr_t data);
>
> static ngx_int_t ngx_stream_ssl_add_variables(ngx_conf_t *cf);
> -static void *ngx_stream_ssl_create_conf(ngx_conf_t *cf);
> -static char *ngx_stream_ssl_merge_conf(ngx_conf_t *cf, void *parent,
> +static void *ngx_stream_ssl_create_srv_conf(ngx_conf_t *cf);
> +static char *ngx_stream_ssl_merge_srv_conf(ngx_conf_t *cf, void *parent,
> void *child);
>
> static ngx_int_t ngx_stream_ssl_compile_certificates(ngx_conf_t *cf,
> -ngx_stream_ssl_conf_t *conf);
> +ngx_stream_ssl_srv_conf_t *conf);
>
> static char *ngx_stream_ssl_password_file(ngx_conf_t *cf, ngx_command_t *cmd,
> void *conf);
> @@ -90,21 +90,21 @@ static ngx_command_t ngx_stream_ssl_com
>NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1,
>ngx_conf_set_msec_slot,
>NGX_STREAM_SRV_CONF_OFFSET,
> - offsetof(ngx_stream_ssl_conf_t, handshake_timeout),
> + offsetof(ngx_stream_ssl_srv_conf_t, handshake_timeout),
>NULL },
>
> { ngx_string("ssl_certificate"),
>NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1,
>ngx_conf_set_str_array_slot,
>NGX_STREAM_SRV_CONF_OFFSET,
> - offsetof(ngx_stream_ssl_conf_t, certificates),
> + offsetof(ngx_stream_ssl_srv_conf_t, certificates),
>NULL },
>
> { ngx_string("ssl_certificate_key"),
>NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1,
>ngx_conf_set_str_array_slot,
>NGX_STREAM_SRV_CONF_OFFSET,
> - offsetof(ngx_stream_ssl_conf_t, certificate_keys),
> + offsetof(ngx_stream_ssl_srv_conf_t, certificate_keys),
>NULL },
>
> { ngx_string("ssl_password_file"),
> @@ -118,63 +118,63 @@ static ngx_command_t ngx_stream_ssl_com
>NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1,
>ngx_conf_set_str_slot,
>NGX_STREAM_SRV_CONF_OFFSET,
> - offsetof(ngx_stream_ssl_conf_t, dhparam),
> + offsetof(ngx_stream_ssl_srv_conf_t, dhparam),
>NULL },
>
> { ngx_string("ssl_ecdh_curve"),
>NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1,
>ngx_conf_set_str_slot,
>NGX_STREAM_SRV_CONF_OFFSET,
> - offsetof(ngx_stream_ssl_conf_t, ecdh_curve),
> + offsetof(ngx_stream_ssl_srv_conf_t, ecdh_curve),
>NULL },
>
> { ngx_string("ssl_protocols"),
>NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_1MORE,
>ngx_conf_set_bitmask_slot,
>NGX_STREAM_SRV_CONF_OFFSET,
> - offsetof(ngx_stream_ssl_conf_t, protocols),
> + offsetof(ngx_stream_ssl_srv_conf_t, protocols),
>&ngx_stream_ssl_protocols },
>
> { ngx_string("ssl_ciphers"),
>NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1,
>ngx_conf_set_str_slot,
>NGX_STREAM_SRV_CONF_OFFSET,
> - offsetof(ngx_stream_ssl_conf_t, ciphers),
> + offsetof(ngx_stream_ssl_srv_conf_t, ciphers),
>NULL },
>
> { ngx_string("ssl_verify_client"),
>NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1,
>ngx_conf_set_enum_slot,
>NGX_STREAM_SRV_CONF_OFFSET,
> - offsetof(ngx_stream_ssl_conf_t, verify),
> + offsetof(ngx_stream_ssl_srv_conf_t, verify),
>&ngx_stream_ssl_verify },
>
> { ngx_string("ssl_verify_depth"),
>NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1,
>ngx_conf_set_num_slot,
>NGX_STREAM_SRV_CONF_OFFSET,
> - offsetof(ngx_stream_ssl_conf_t, verify_depth),
> + offsetof(ngx_stream_ssl_srv_conf_t, verify_depth),
>NULL },
>
> { ngx_string("ssl_client_certificate"),
>NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1,
>ngx_conf_set_str_slot,
>NGX_STREAM_SRV_CONF_OFFSET,
> - offsetof(ngx_stream_ssl_conf_t, client_certificate),
> + offsetof(ngx_stream_ssl_srv_conf_t, client_certificate),
>NULL },
>
> { ngx_string("ssl_trusted_certificate"),
>NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1,
>
Re: [PATCH 4 of 4] AIO operations now add timers (ticket #2162)
On Mon, 8 Jan 2024 11:25:55 + J Carter wrote: > Hello, > > On Mon, 27 Nov 2023 05:50:27 +0300 > Maxim Dounin wrote: > > > # HG changeset patch > > # User Maxim Dounin > > # Date 1701050170 -10800 > > # Mon Nov 27 04:56:10 2023 +0300 > > # Node ID 00c3e7333145ddb5ea0eeaaa66b3d9c26973c9c2 > > # Parent 61d08e4cf97cc073200ec32fc6ada9a2d48ffe51 > > AIO operations now add timers (ticket #2162). > > > > Each AIO (thread IO) operation being run is now accompanied with 1-minute > > timer. This timer prevents unexpected shutdown of the worker process while > > an AIO operation is running, and logs an alert if the operation is running > > for too long. > > Shouldn't this timer's duration be set to match worker_shutdown_timeout's > duration rather than being hard coded to 60s ? Ah nevermind, I understand. These timers will either expire from passing the 60s set duration, or will expire as worker_process_timeout itself expires, kills the connection and times out associated timers (including the aio timers). Setting it to worker_shutdown_timeout's duration would be pointless (an 'infinite' timer would give the same result). So the only situation in which a different value for these AIO timers would make sense is if these AIO operations are expected to take longer 60s, but less than worker_shutdown_timeout (in cases where it has been increased from it's own default of 60s). In that case the AIO operation's timeout would have to be one (or more) of it's own directives, with a value less than worker_shutdown_timeout. (doesn't seem like it's worth it from the ticket discussion, sorry for the noise). > > This fixes "open socket left" alerts during worker processes shutdown > > due to pending AIO (or thread IO) operations while corresponding requests > > have no timers. In particular, such errors were observed while reading > > cache headers (ticket #2162), and with worker_shutdown_timeout. > > [...] ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 4 of 4] AIO operations now add timers (ticket #2162)
Hello, On Mon, 27 Nov 2023 05:50:27 +0300 Maxim Dounin wrote: > # HG changeset patch > # User Maxim Dounin > # Date 1701050170 -10800 > # Mon Nov 27 04:56:10 2023 +0300 > # Node ID 00c3e7333145ddb5ea0eeaaa66b3d9c26973c9c2 > # Parent 61d08e4cf97cc073200ec32fc6ada9a2d48ffe51 > AIO operations now add timers (ticket #2162). > > Each AIO (thread IO) operation being run is now accompanied with 1-minute > timer. This timer prevents unexpected shutdown of the worker process while > an AIO operation is running, and logs an alert if the operation is running > for too long. Shouldn't this timer's duration be set to match worker_shutdown_timeout's duration rather than being hard coded to 60s ? > This fixes "open socket left" alerts during worker processes shutdown > due to pending AIO (or thread IO) operations while corresponding requests > have no timers. In particular, such errors were observed while reading > cache headers (ticket #2162), and with worker_shutdown_timeout. [...] ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
