summaryrefslogtreecommitdiffhomepage
path: root/src (follow)
AgeCommit message (Collapse)AuthorFilesLines
2024-06-20http: Move chunked buffer pos pointer while parsingZhidao HONG2-9/+4
Previously, Unit didn't move the buffer pointer when parsing chunked data because the buffer was not used after sent. It's used for upstream response. With the new requirement to support request chunked body, the buffer might be used for pipeline request, so it's necessary to move the pos pointer while parsing. Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
2024-06-18Use octal instead of mode macrosAlejandro Colomar6-15/+9
They are more readable. And we had a mix of both styles; there wasn't really a consistent style. Tested-by: Andrew Clayton <a.clayton@nginx.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-06-18fs: Make the full directory path for the pid file and the control socketAlejandro Colomar4-5/+5
Build systems should not attempt to create $runstatedir (or anything under it). Doing so causes warnings in packaging systems, such as in Alpine Linux, as reported by Andy. But unitd(8) can be configured to be installed under /opt, or other trees, where no directories exist before hand. Expecting that the user creates the entire directory trees that unit will need is a bit unreasonable. Instead, let's just create any directories that we need, with all their parents, at run time. Fixes: 57fc9201cb91 ("Socket: Created control socket & pid file directories.") Link: <https://github.com/nginx/unit/issues/742> Reported-by: Andy Postnikov <apostnikov@gmail.com> Tested-by: Andy Postnikov <apostnikov@gmail.com> Tested-by: Andrew Clayton <a.clayton@nginx.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Acked-by: Konstantin Pavlov <thresh@nginx.com> Cc: Liam Crilly <liam@nginx.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-06-18fs: Correctly handle "/" in nxt_fs_mkdir_dirname()Alejandro Colomar1-1/+2
The previous code attempted to mkdir(""), that is an empty string. Since "/" necessarily exists, just goto out_free. Fixes: 57fc9201cb91 ("Socket: Created control socket & pid file directories.") Link: <https://github.com/nginx/unit/issues/742> Tested-by: Andy Postnikov <apostnikov@gmail.com> Tested-by: Andrew Clayton <a.clayton@nginx.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Cc: Liam Crilly <liam@nginx.com> Cc: Konstantin Pavlov <thresh@nginx.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-06-18fs: Invert logic to reduce indentation in nxt_fs_mkdir_dirname()Alejandro Colomar1-3/+6
This refactor isn't very appealing alone, but it prepares the code for the following commits. Link: <https://github.com/nginx/unit/issues/742> Tested-by: Andy Postnikov <apostnikov@gmail.com> Tested-by: Andrew Clayton <a.clayton@nginx.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Cc: Liam Crilly <liam@nginx.com> Cc: Konstantin Pavlov <thresh@nginx.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-06-18fs: Accept path names of length 1 in nxt_fs_mkdir_p()Alejandro Colomar1-1/+1
That is, accept "/", or relative path names of a single byte. Fixes: e2b53e16c60b ("Added "rootfs" feature.") Tested-by: Andy Postnikov <apostnikov@gmail.com> Tested-by: Andrew Clayton <a.clayton@nginx.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-06-18fs: Accept relative paths in nxt_fs_mkdir_p()Alejandro Colomar1-1/+1
Tested-by: Andy Postnikov <apostnikov@gmail.com> Tested-by: Andrew Clayton <a.clayton@nginx.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-06-18fs: Use a temporary variable in nxt_fs_mkdir_p()Alejandro Colomar1-6/+6
This avoids breaking a long line. Tested-by: Andy Postnikov <apostnikov@gmail.com> Tested-by: Andrew Clayton <a.clayton@nginx.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-06-18fs: Use branchless code in nxt_fs_mkdir_p()Alejandro Colomar1-5/+1
That branch was to avoid an infinite loop on the slash. However, we can achieve the same by using a +1 to make sure we advance at least 1 byte in each iteration. Tested-by: Andy Postnikov <apostnikov@gmail.com> Tested-by: Andrew Clayton <a.clayton@nginx.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-06-18fs: Rename nxt_fs_mkdir_all() => nxt_fs_mkdir_p()Alejandro Colomar4-4/+4
"all" is too generic of an attribute to be meaningful. In the context of mkdir(), "parents" is used for this meaning, as in mkdir -p, so it should be more straightforward to readers. Tested-by: Andy Postnikov <apostnikov@gmail.com> Tested-by: Andrew Clayton <a.clayton@nginx.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-06-18fs: Rename nxt_fs_mkdir_parent() => nxt_fs_mkdir_dirname()Alejandro Colomar4-4/+4
"dirname" is the usual way to refer to the directory part of a path name. See for example dirname(1), or the dirname builtin in several languages. Also, in the context of mkdir(), "parents" is used to refer to mkdir -p, which is too similar to "parent", so it can lead to confusion. Tested-by: Andy Postnikov <apostnikov@gmail.com> Tested-by: Andrew Clayton <a.clayton@nginx.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-06-14http: fix use-of-uninitialized-value bugArjun1-0/+1
This was found via MSan. In nxt_http_fields_hash() we setup a nxt_lvlhsh_query_t structure and initialise a couple of its members. At some point we call lhq->proto->alloc(lhq->pool, nxt_lvlhsh_bucket_size(lhq->proto)); Which in this case is void * nxt_lvlhsh_alloc(void *data, size_t size) { return nxt_memalign(size, size); } So even though lhq.ppol wasn't previously initialised we don't actually use it in that particular function. However MSan triggers on the fact that we are passing an uninitialised value into that function. Indeed, compilers will generally complain about such things, e.g /* u.c */ struct t { void *p; int len; }; static void test(void *p __attribute__((unused)), int len) { (void)len; } int main(void) { struct t t; t.len = 42; test(t.p, t.len); return 0; } GCC and Clang will produce a -Wuninitialized warning. But they won't catch the following... /* u2.c */ struct t { void *p; int len; }; static void _test(void *p __attribute__((unused)), int len) { (void)len; } static void test(struct t *t) { _test(t->p, t->len); } int main(void) { struct t t; t.len = 42; test(&t); return 0; } Which is why we don't get a compiler warning about lhq.pool. In this case initialising lhg.pool even though we don't use it here seems like the right thing to do and maybe compilers will start being able to catch these in the future. Actually GCC with -fanalyzer does catch the above $ gcc -Wall -Wextra -O0 -fanalyzer u2.c u2.c: In function ‘test’: u2.c:15:9: warning: use of uninitialized value ‘*t.p’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value] 15 | _test(t->p, t->len); | ^~~~~~~~~~~~~~~~~~~ ‘main’: events 1-3 | | 18 | int main(void) | | ^~~~ | | | | | (1) entry to ‘main’ | 19 | { | 20 | struct t t; | | ~ | | | | | (2) region created on stack here |...... | 23 | test(&t); | | ~~~~~~~~ | | | | | (3) calling ‘test’ from ‘main’ | +--> ‘test’: events 4-5 | | 13 | static void test(struct t *t) | | ^~~~ | | | | | (4) entry to ‘test’ | 14 | { | 15 | _test(t->p, t->len); | | ~~~~~~~~~~~~~~~~~~~ | | | | | (5) use of uninitialized value ‘*t.p’ here | Signed-off-by: Arjun <pkillarjun@protonmail.com> Link: <https://clang.llvm.org/docs/MemorySanitizer.html> [ Commit message - Andrew ] Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-05-24wasm: Add a missing 'const' qualifier in nxt_wasm_setup()Andrew Clayton1-1/+1
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-05-24tstr: Constify the 'str' parameter to nxt_tstr_compile()Andrew Clayton2-2/+2
This allows you to then define strings like static const nxt_str_t my_str = nxt_string("string"); Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-05-09http: Ensure REQUEST_URI immutabilityZhidao HONG1-27/+2
Previously, the REQUEST_URI within Unit could be modified, for example, during uri rewriting. We decide to make $request_uri immutable and pass constant REQUEST_URI to applications. Based on the new requirement, we remove `r->target` rewriting in the rewrite module. Closes: https://github.com/nginx/unit/issues/916 Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Signed-off-by: Zhidao HONG <z.hong@f5.com>
2024-05-09http: Use consistent target in nxt_h1p_peer_header_send()Zhidao HONG1-1/+1
This change is required for the next commit, after which target and r->target may be different. Before the next patch, target and r->target would be the same. No functional changes. Reviewed-by: Andrew Clayton <a.clayton@nginx.com> Signed-off-by: Zhidao HONG <z.hong@f5.com>
2024-05-07Convert 0-sized arrays to true flexible array membersAndrew Clayton4-10/+10
Declaring a 0-sized array (e.g 'char arr[0];') as the last member of a structure is a GNU extension that was used to implement flexible array members (FAMs) before they were standardised in C99 as simply '[]'. The GNU extension itself was introduced to work around a hack of declaring 1-sized arrays to mean a variable-length object. The advantage of the 0-sized (and true FAMs) is that they don't count towards the size of the structure. Unit already declares some true FAMs, but it also declared some 0-sized arrays. Converting these 0-sized arrays to true FAMs is not only good for consistency but will also allow better compiler checks now (as in a C99 FAM *must* be the last member of a structure and the compiler will warn otherwise) and in the future as doing this fixes a bunch of warnings (treated as errors in Unit by default) when compiled with -O2 -Warray-bounds -Wstrict-flex-arrays -fstrict-flex-arrays=3 (Note -Warray-bounds is enabled by -Wall and -Wstrict-flex-arrays seems to also be enabled via -Wall -Wextra, the -02 is required to make -fstrict-flex-arrays more effective, =3 is the default on at least GCC 14) such as CC build/src/nxt_upstream.o src/nxt_upstream.c: In function ‘nxt_upstreams_create’: src/nxt_upstream.c:56:18: error: array subscript i is outside array bounds of ‘nxt_upstream_t[0]’ {aka ‘struct nxt_upstream_s[]’} [-Werror=array-bounds=] 56 | string = nxt_str_dup(mp, &upstreams->upstream[i].name, &name); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from src/nxt_upstream.c:9: src/nxt_upstream.h:55:48: note: while referencing ‘upstream’ 55 | nxt_upstream_t upstream[0]; | ^~~~~~~~ Making our flexible array members proper C99 FAMs and ensuring any >0 sized trailing arrays in structures are really normal arrays will allow to enable various compiler options (such as the above and more) that will help keep our array usage safe. Changing 0-sized arrays to FAMs should have no effect on structure layouts/sizes (they both have a size of 0, although doing a sizeof() on a FAM will result in a compiler error). Looking at pahole(1) output for the nxt_http_route_ruleset_t structure for the [0] and [] cases... $ pahole -C nxt_http_route_ruleset_t /tmp/build/src/nxt_http_route.o typedef struct { uint32_t items; /* 0 4 */ /* XXX 4 bytes hole, try to pack */ nxt_http_route_rule_t * rule[]; /* 8 0 */ /* size: 8, cachelines: 1, members: 2 */ /* sum members: 4, holes: 1, sum holes: 4 */ /* last cacheline: 8 bytes */ } nxt_http_route_ruleset_t; $ pahole -C nxt_http_route_ruleset_t build/src/nxt_http_route.o typedef struct { uint32_t items; /* 0 4 */ /* XXX 4 bytes hole, try to pack */ nxt_http_route_rule_t * rule[]; /* 8 0 */ /* size: 8, cachelines: 1, members: 2 */ /* sum members: 4, holes: 1, sum holes: 4 */ /* last cacheline: 8 bytes */ } nxt_http_route_ruleset_t; Also checking with the size(1) command on the effected object files shows no changes to their sizes $ for file in build/src/nxt_upstream.o \ build/src/nxt_upstream_round_robin.o \ build/src/nxt_h1proto.o \ build/src/nxt_http_route.o \ build/src/nxt_http_proxy.o \ build/src/python/*.o; do \ size -G /tmp/${file} $file; echo; done text data bss total filename 640 418 0 1058 /tmp/build/src/nxt_upstream.o 640 418 0 1058 build/src/nxt_upstream.o text data bss total filename 929 351 0 1280 /tmp/build/src/nxt_upstream_round_robin.o 929 351 0 1280 build/src/nxt_upstream_round_robin.o text data bss total filename 11707 8281 16 20004 /tmp/build/src/nxt_h1proto.o 11707 8281 16 20004 build/src/nxt_h1proto.o text data bss total filename 8319 3101 0 11420 /tmp/build/src/nxt_http_route.o 8319 3101 0 11420 build/src/nxt_http_route.o text data bss total filename 1495 1056 0 2551 /tmp/build/src/nxt_http_proxy.o 1495 1056 0 2551 build/src/nxt_http_proxy.o text data bss total filename 4321 2895 0 7216 /tmp/build/src/python/nxt_python_asgi_http-python.o 4321 2895 0 7216 build/src/python/nxt_python_asgi_http-python.o text data bss total filename 4231 2266 0 6497 /tmp/build/src/python/nxt_python_asgi_lifespan-python.o 4231 2266 0 6497 build/src/python/nxt_python_asgi_lifespan-python.o text data bss total filename 12051 6090 8 18149 /tmp/build/src/python/nxt_python_asgi-python.o 12051 6090 8 18149 build/src/python/nxt_python_asgi-python.o text data bss total filename 28 1963 432 2423 /tmp/build/src/python/nxt_python_asgi_str-python.o 28 1963 432 2423 build/src/python/nxt_python_asgi_str-python.o text data bss total filename 5818 3518 0 9336 /tmp/build/src/python/nxt_python_asgi_websocket-python.o 5818 3518 0 9336 build/src/python/nxt_python_asgi_websocket-python.o text data bss total filename 4391 2089 168 6648 /tmp/build/src/python/nxt_python-python.o 4391 2089 168 6648 build/src/python/nxt_python-python.o text data bss total filename 9095 5909 152 15156 /tmp/build/src/python/nxt_python_wsgi-python.o 9095 5909 152 15156 build/src/python/nxt_python_wsgi-python.o Link: <https://lwn.net/Articles/908817/> Link: <https://people.kernel.org/kees/bounded-flexible-arrays-in-c> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-04-25Tighten up some string arraysAndrew Clayton3-10/+13
This is the normal way of declaring such things. Reviewed-by: Zhidao HONG <z.hong@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-04-25configuration: Constify more pointersAndrew Clayton2-24/+28
This continues the patch series constifying various pointers in the configuration sub-system. This is done as a separate commit as it involved a _slightly_ more invasive change in nxt_conf_get_string(). While it takes a value parameter that is never modified, simply making it const results in CC build/src/nxt_conf.o src/nxt_conf.c: In function ‘nxt_conf_get_string’: src/nxt_conf.c:170:20: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers] 170 | str->start = value->u.str.start; | ^ due to the assignment operator. Making value const will allow for numerous other constification and seeing as we are not modifying it, seems worthwhile. We can get around the warning by casting ->u.{str,string}.start Reviewed-by: Zhidao HONG <z.hong@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-04-25php: Constify some local static variablesAndrew Clayton1-6/+6
A common pattern was to declare variables in functions like static nxt_str_t ... Not sure why static, as they were being treated more like string literals, let's actually make them constants (qualifier wise). Reviewed-by: Zhidao HONG <z.hong@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-04-25Constify a bunch of static local variablesAndrew Clayton10-74/+79
A common pattern was to declare variables in functions like static nxt_str_t ... Not sure why static, as they were being treated more like string literals (and of course they are _not_ thread safe), let's actually make them constants (qualifier wise). This handles core code conversion. Reviewed-by: Zhidao HONG <z.hong@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-04-25configuration: Constify numerous pointersAndrew Clayton3-68/+74
Mark numerous function argument pointers as 'const' in the configuration sub-system. This also does the same with a few functions in src/nxt_conf_validation.c that are required to accomplish the below, attacking the rest is an exercise for another day... While this is a worthwhile hardening exercise in its own right, the main impetus for this is to 'constify' some local function variables which are currently defined with 'static' storage class and turn them into 'static const', which will be done in a subsequent patch. Reviewed-by: Zhidao HONG <z.hong@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-04-19wasm-wc: Bump the rustls crate from 0.21.10 to 0.21.11dependabot[bot]1-2/+2
Bumps <https://github.com/rustls/rustls> from 0.21.10 to 0.21.11. "This release corrects a denial-of-service condition in rustls::ConnectionCommon::complete_io(), reachable via network input. If a close_notify alert is received during a handshake, complete_io() did not terminate. Callers which do not call complete_io() are not affected." The wasm-wasi-component language module is not effected by this as it doesn't handle client connections, Unit does. Link: Release notes <https://github.com/rustls/rustls/releases> Link: Commits <https://github.com/rustls/rustls/compare/v/0.21.10...v/0.21.11> Signed-off-by: dependabot[bot] <support@github.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> [ Tweaked commit message/subject - Andrew ] Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-04-17Fixes: 64934e59f ("HTTP: Introduce quoted target marker in HTTP parsing")Zhidao HONG1-2/+2
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
2024-04-11HTTP: Rewrote url target section in nxt_h1p_peer_header_send()Zhidao HONG3-5/+74
Previously, proxy request was constructed based on the `r->target` field. However, r->target will remain unchanged in the future, even in cases of URL rewriting because of the requirement change for $request_uri that will be changed to constant. To accommodate this, the r->target should be designed to be constant, but Unit needs to pass a changeable URL to the upstream server. Based on the above, the proxy module can't depend on r->target.
2024-04-11HTTP: Introduce quoted target marker in HTTP parsingZhidao HONG2-12/+5
The quoted_target field is to indentify URLs containing percent-encoded characters. It can be used in places where you might need to generate new URL, such as in the proxy module. It will be used in the subsequent commit.
2024-04-10HTTP: Added variable validation to the response_headers optionZhidao HONG1-1/+12
This is to improve error messages for response headers configuration. Take the configuration as an example: { "response_headers": { "a": "$b" } } Previously, when applying it the user would see this error message: failed to apply previous configuration After this change, the user will see this improved error message: the previous configuration is invalid: Unknown variable "b" in the "a" value
2024-04-09Wasm-wc: Bump the h2 crate from 0.4.2 to 0.4.4dependabot[bot]1-2/+2
Bumps h2 <https://github.com/hyperium/h2> from 0.4.2 to 0.4.4. Limit number of CONTINUATION frames for misbehaving connections. Link: Changelog <https://github.com/hyperium/h2/blob/master/CHANGELOG.md> Link: Commits <https://github.com/hyperium/h2/compare/v0.4.2...v0.4.4> Signed-off-by: dependabot[bot] <support@github.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> [ Tweaked commit message/subject - Andrew ] Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-14Wasm-wc: Fix application restartsAndrew Clayton1-3/+12
Liam reported a problem when trying to restart wasm-wasi-component based applications using the /control/applications/APPLICATION_NAME/restart endpoint. The application would become unresponsive. What was happening was the old application process(es) weren't exit(3)ing and so while we were starting new application processes, the old ones were still hanging around in a non-functioning state. When we are terminating an application it must call exit(3). So that's what we do. We use the return value of nxt_unit_run() as the exit status. Due to exit(3)ing we also need to now explicitly handle the return on error case. Reported-by: Liam Crilly <liam@nginx.com> Fixes: 20ada4b5c ("Wasm-wc: Core of initial Wasm component model language module support") Closes: https://github.com/nginx/unit/issues/1179 Tested-by: Liam Crilly <liam@nginx.com> Tested-by: Danielle De Leo <d.deleo@f5.com> Co-developed-by: Dan Callahan <d.callahan@f5.com> Signed-off-by: Dan Callahan <d.callahan@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-12NJS: loader should be registered using njs_vm_set_module_loader()Andrei Zeliankou2-20/+8
This change makes NJS module incompatible with NJS older than 0.8.3. Therefore, the configuration version check has been adjusted accordingly. This change was introduced in NJS 0.8.3 here: <https://hg.nginx.com/njs/rev/ad1a7ad3c715>
2024-03-11Avoiding arithmetic ops with NULL pointer in nxt_unit_mmap_getAndrei Zeliankou1-0/+6
Found by UndefinedBehaviorSanitizer. Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-11Initialize port_impl only when it is neededAndrei Zeliankou1-1/+1
Found by UndefinedBehaviorSanitizer. Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-11Avoiding arithmetic ops with NULL pointer in nxt_port_mmap_getAndrei Zeliankou1-0/+6
Can be reproduced by test/test_settings.py::test_settings_send_timeout with enabled UndefinedBehaviorSanitizer. Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-11Avoiding arithmetic ops with NULL pointer in nxt_http_arguments_parseAndrei Zeliankou1-0/+6
Can be reproduced by test/test_variables.py::test_variables_dynamic_arguments with enabled UndefinedBehaviorSanitizer: src/nxt_http_request.c:961:17: runtime error: applying zero offset to null pointer #0 0x1050d95a4 in nxt_http_arguments_parse nxt_http_request.c:961 #1 0x105102bf8 in nxt_http_var_arg nxt_http_variables.c:621 #2 0x104f95d74 in nxt_var_interpreter nxt_var.c:507 #3 0x104f98c98 in nxt_tstr_query nxt_tstr.c:265 #4 0x1050abfd8 in nxt_router_access_log_writer nxt_router_access_log.c:194 #5 0x1050d81f4 in nxt_http_request_close_handler nxt_http_request.c:838 #6 0x104fcdc48 in nxt_event_engine_start nxt_event_engine.c:542 #7 0x104fba838 in nxt_thread_trampoline nxt_thread.c:126 #8 0x18133e030 in _pthread_start+0x84 (libsystem_pthread.dylib:arm64e+0x7030) #9 0x181338e38 in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1e38) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/nxt_http_request.c:961:17 Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-11Fixed undefined behaviour in left shift of int valueAndrei Zeliankou1-4/+4
Found by UndefinedBehaviorSanitizer: src/nxt_random.c:151:31: runtime error: left shift of 140 by 24 places cannot be represented in type 'int' #0 0x104f78968 in nxt_random nxt_random.c:151 #1 0x104f58a98 in nxt_shm_open nxt_port_memory.c:377 #2 0x10503e24c in nxt_controller_conf_send nxt_controller.c:617 #3 0x105041154 in nxt_controller_process_request nxt_controller.c:1109 #4 0x104fcdc48 in nxt_event_engine_start nxt_event_engine.c:542 #5 0x104f27254 in main nxt_main.c:35 #6 0x180fbd0dc (<unknown module>) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/nxt_random.c:151:31 Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-11NJS: avoiding arithmetic ops with NULL pointer in r->argsAndrei Zeliankou1-2/+3
Can be reproduced by test/test_rewrite.py::test_rewrite_njs with enabled UndefinedBehaviorSanitizer: src/nxt_http_js.c:169:52: runtime error: applying zero offset to null pointer #0 0x10255b044 in nxt_http_js_ext_get_args nxt_http_js.c:169 #1 0x102598ad0 in njs_value_property njs_value.c:1175 #2 0x10259c2c8 in njs_vm_object_prop njs_vm.c:1398 #3 0x102559d74 in nxt_js_call nxt_js.c:445 #4 0x1023c0da0 in nxt_tstr_query nxt_tstr.c:276 #5 0x102516ec4 in nxt_http_rewrite nxt_http_rewrite.c:56 #6 0x1024fd86c in nxt_http_request_action nxt_http_request.c:565 #7 0x1024d71b0 in nxt_h1p_request_body_read nxt_h1proto.c:998 #8 0x1023f5c48 in nxt_event_engine_start nxt_event_engine.c:542 #9 0x1023e2838 in nxt_thread_trampoline nxt_thread.c:126 #10 0x18133e030 in _pthread_start+0x84 (libsystem_pthread.dylib:arm64e+0x7030) #11 0x181338e38 in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1e38) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/nxt_http_js.c:169:52 Same fix was introduced in NJS: <http://hg.nginx.org/njs/rev/4fba78789fe4> Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-11Router: match when pattern and tested string are both zero lengthAndrei Zeliankou1-0/+4
Otherwise, undefined behaviour will be triggered. Can be reproduced by test/test_routing.py::test_routes_match_host_empty with enabled UndefinedBehaviorSanitizer: src/nxt_http_route.c:2141:17: runtime error: applying zero offset to null pointer #0 0x100562588 in nxt_http_route_test_rule nxt_http_route.c:2091 #1 0x100564ed8 in nxt_http_route_handler nxt_http_route.c:1574 #2 0x10055188c in nxt_http_request_action nxt_http_request.c:570 #3 0x10052b1a0 in nxt_h1p_request_body_read nxt_h1proto.c:998 #4 0x100449c38 in nxt_event_engine_start nxt_event_engine.c:542 #5 0x100436828 in nxt_thread_trampoline nxt_thread.c:126 #6 0x18133e030 in _pthread_start+0x84 (libsystem_pthread.dylib:arm64e+0x7030) #7 0x181338e38 in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1e38) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/nxt_http_route.c:2141:17 Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-11Wasm-wc: Bump the mio crate from 0.8.10 to 0.8.11dependabot[bot]1-2/+2
Bumps mio <https://github.com/tokio-rs/mio> from 0.8.10 to 0.8.11. Fixes receiving IOCP events after deregistering a Windows named pipe. Not that that effects Unit... Link: <https://github.com/nginx/unit/security/dependabot/1> Link: Changelog <https://github.com/tokio-rs/mio/blob/master/CHANGELOG.md> Link: Commits <https://github.com/tokio-rs/mio/compare/v0.8.10...v0.8.11> Signed-off-by: dependabot[bot] <support@github.com> Reviewed-by: Andrew Clayton <a.clayton@nginx.com> [ Tweaked commit message/subject - Andrew ] Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-09Remove support for Sun's Sun Studio/SunPro C compilerAndrew Clayton1-75/+0
We really only support building Unit with GCC and Clang. Cc: Dan Callahan <d.callahan@f5.com> Reviewed-by: Alejandro Colomar <alx@kernel.org> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-09Remove support for IBM's XL C compilerAndrew Clayton1-103/+1
We really only support building Unit with GCC and Clang. Cc: Dan Callahan <d.callahan@f5.com> Reviewed-by: Alejandro Colomar <alx@kernel.org> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-09Remove support for Microsoft's Visual C++ compilerAndrew Clayton1-1/+0
We don't run on Windows and only really support compiling Unit with GCC and Clang. Cc: Dan Callahan <d.callahan@f5.com> Co-developed-by: Alejandro Colomar <alx@kernel.org> Signed-off-by: Alejandro Colomar <alx@kernel.org> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-08Var: Fix cacheable issue for njs variable accessZhidao HONG1-1/+1
The variables accessed with JS template literal should not be cacheable. Since it is parsed by njs engine, Unit can't create indexes on these variables for caching purpose. For example: { "format": "`{bodyLength:\"${vars.body_bytes_sent}\",status:\"${vars.status}\"}\n`" } The variables like the above are not cacheable. Closes: https://github.com/nginx/unit/issues/1169
2024-03-05Avoid potential NULL pointer dereference in nxt_router_temp_conf()Andrew Clayton1-7/+9
In nxt_router_temp_conf() we have rtcf = nxt_mp_zget(mp, sizeof(nxt_router_conf_t)); if (nxt_slow_path(rtcf == NULL)) { goto fail; } If rtcf is NULL then we do fail: if (rtcf->tstr_state != NULL) { nxt_tstr_state_release(rtcf->tstr_state); } In which case we will dereference the NULL pointer rtcf. This patch re-works the goto labels to make them more specific to their intended purpose and ensures we are freeing things which have been allocated. This was found by the clang static analyser. Reviewed-by: Zhidao Hong <z.hong@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-05Var: Remove a dead assignment in nxt_var_interpreter()Andrew Clayton1-1/+1
p is not used again before returning from the function. Found by the clang static analyser. Reviewed-by: Zhidao Hong <z.hong@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-03-05Remove unused nxt_vector_t APIAndrew Clayton3-222/+0
This is unused, yet a community member just spent time finding and fixing a bug in it only to be told it's unused. Just get rid of the thing. Link: <https://github.com/nginx/unit/pull/963> Reviewed-by: Zhidao Hong <z.hong@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-02-29Configuration: Fix check in nxt_conf_json_parse_value()Andrew Clayton1-1/+1
If we compile Unit with -Wstrict-overflow=5 (as we do with clang) then we get the following warning cc -c -pipe -fPIC -fvisibility=hidden -O0 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wstrict-overflow=5 -Wmissing-prototypes -g -I src -I build/include \ \ \ -o build/src/nxt_conf.o \ -MMD -MF build/src/nxt_conf.dep -MT build/src/nxt_conf.o \ src/nxt_conf.c src/nxt_conf.c: In function ‘nxt_conf_json_parse_value’: src/nxt_conf.c:1444:5: warning: assuming signed overflow does not occur when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Wstrict-overflow] 1444 | if (nxt_fast_path((ch - '0') <= 9)) { | Does this actually cause an issue?... well, yes. Using this minimal test config to show the problem { "listeners": { "[::1]:8080": { "pass": --100 } } } With the above if () statement that triggers the warning, my assumption here is that we only want a digit now. '0' - '9'. ch is a u_char, however if ch is any character with an ASCII code < 48 ('0') e.g if ch is '-' (45) then we get 45 - 48 = -3, through arithmetic conversion, which makes the if () statement true (when it shouldn't) then at some point we get the following error returned from the controller { "error": "Memory allocation failed." } Instead of the expected { "error": "Invalid JSON.", "detail": "A valid JSON value is expected here. It must be either a literal (null, true, or false), a number, a string (in double quotes \"\"), an array (with brackets []), or an object (with braces {}).", "location": { "offset": 234, "line": 15, "column": 27 } } Casting the result of (ch - '0') to u_char resolves this issue, this makes the above calculation come out as 253 (relying on unsigned integer wraparound) which was probably the intended way for it to work. Reviewed-by: Zhidao Hong <z.hong@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-02-21Wasm-wc: Wire up the language module to the config systemAndrew Clayton3-0/+35
This exposes the various WebAssembly Component Model language module specific options. The application type is "wasm-wasi-component". There is a "component" option that is required, this specifies the full path to the WebAssembly component to be run. This component should be in binary format, i.e a .wasm file. There is also currently one optional option "access" Due to the sandboxed nature of WebAssembly, by default Wasm modules/components don't have any access to the underlying filesystem. There is however a capabilities based mechanism[0] for allowing such access. This adds a config option to the 'wasm-wasi-component' application type (same as for 'wasm'); 'access.filesystem' which takes an array of directory paths that are then made available to the wasm module/component. This access works recursively, i.e everything under a specific path is allowed access to. Example config might look like "applications": { "my-wasm-component": { "type": "wasm-wasi-component", "component": "/path/to/component.wasm", "access" { "filesystem": [ "/tmp", "/var/tmp" ] } } } The actual mechanism used allows directories to be mapped differently in the guest. But at the moment we don't support that and just map say /tmp to /tmp. This can be revisited if it's something users clamour for. [0]: <https://github.com/bytecodealliance/wasmtime/blob/main/docs/WASI-capabilities.md> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-02-21Wasm-wc: Add Cargo.lockAndrew Clayton2-2/+2293
It seems we do want to track this thing. This is just the latest version that cargo had generated for me. Cc: Dan Callahan <d.callahan@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-02-21Wasm-wc: Allow to use the 'reactor' adaptor againAndrew Clayton1-1/+2
With the initial port to wasmtime 17 we could no longer use the 'reactor' adaptor but had to switch to the more restrictive 'proxy' adaptor. This meant amongst other things (probably) we could no longer access the filesystem. Thanks to Joel Dice for pointing out the fix. With this we can go back to using the 'reactor' adaptor again and things are back to working as before. It's worth noting that you can use either the 'proxy' or 'reactor' adaptor depending on your requirements. Cc: Joel Dice <joel.dice@fermyon.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2024-02-21Wasm-wc: Upgrade to wasmtime 17Andrew Clayton2-16/+17
This brings WASI 0.2.0 support. Link: <https://github.com/bytecodealliance/wasmtime/releases/tag/v17.0.0> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
generated by cgit (git 2.34.1) at 2024-11-17 11:48:37 +0000