| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
This is to store the MIME type of the response which will be used by the
HTTP compression patches as part of determining whether or not to
compress the response.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
valgrind(1) was producing the following alert
==166470== Syscall param sendmsg(msg.msg_control) points to uninitialised byte(s)
==166470== at 0x4AE6514: sendmsg (sendmsg.c:28)
==166470== by 0x42D86C: nxt_sendmsg (nxt_socket_msg.c:32)
==166470== by 0x4FE6695: nxt_unit_sendmsg (nxt_unit.c:6013)
==166470== by 0x4FEB6E2: nxt_unit_ready (nxt_unit.c:963)
==166470== by 0x4FEB6E2: nxt_unit_init (nxt_unit.c:557)
==166470== by 0x4FEEC56: nxt_php_start (nxt_php_sapi.c:507)
==166470== by 0x426BA0: nxt_app_setup (nxt_application.c:1029)
==166470== by 0x403153: nxt_process_do_start (nxt_process.c:718)
==166470== by 0x4042A3: nxt_process_whoami_ok (nxt_process.c:846)
==166470== by 0x407A28: nxt_port_rpc_handler (nxt_port_rpc.c:347)
==166470== by 0x407E42: nxt_port_handler (nxt_port.c:184)
==166470== by 0x40501B: nxt_port_read_msg_process (nxt_port_socket.c:1271)
==166470== by 0x4055B3: nxt_port_read_handler (nxt_port_socket.c:778)
==166470== Address 0x1ffefffc9c is on thread 1's stack
==166470== in frame #3, created by nxt_unit_init (nxt_unit.c:428)
==166470== Uninitialised value was created by a stack allocation
==166470== at 0x4FEABFE: nxt_unit_init (nxt_unit.c:436)
This was due to the nxt_send_oob_t oob structure not being fully
initialised.
Given the name and intention of this function lets *fully*
empty-initialise this structure.
Link: <https://en.cppreference.com/w/c/language/initialization#Empty_initialization>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
valgrind(1) was producing the following alerts
==166470== Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s)
==166470== at 0x4AE6514: sendmsg (sendmsg.c:28)
==166470== by 0x42D86C: nxt_sendmsg (nxt_socket_msg.c:32)
==166470== by 0x4FE6695: nxt_unit_sendmsg (nxt_unit.c:6013)
==166470== by 0x4FEB6E2: nxt_unit_ready (nxt_unit.c:963)
==166470== by 0x4FEB6E2: nxt_unit_init (nxt_unit.c:557)
==166470== by 0x4FEEC56: nxt_php_start (nxt_php_sapi.c:507)
==166470== by 0x426BA0: nxt_app_setup (nxt_application.c:1029)
==166470== by 0x403153: nxt_process_do_start (nxt_process.c:718)
==166470== by 0x4042A3: nxt_process_whoami_ok (nxt_process.c:846)
==166470== by 0x407A28: nxt_port_rpc_handler (nxt_port_rpc.c:347)
==166470== by 0x407E42: nxt_port_handler (nxt_port.c:184)
==166470== by 0x40501B: nxt_port_read_msg_process (nxt_port_socket.c:1271)
==166470== by 0x4055B3: nxt_port_read_handler (nxt_port_socket.c:778)
==166470== Address 0x1ffefffc7f is on thread 1's stack
==166470== in frame #3, created by nxt_unit_init (nxt_unit.c:428)
==166470== Uninitialised value was created by a stack allocation
==166470== at 0x4FEABFE: nxt_unit_init (nxt_unit.c:436)
==166690== Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s)
==166690== at 0x4AE6514: sendmsg (sendmsg.c:28)
==166690== by 0x42D871: nxt_sendmsg (nxt_socket_msg.c:32)
==166690== by 0x4FE6695: nxt_unit_sendmsg (nxt_unit.c:6009)
==166690== by 0x4FE69C8: nxt_unit_port_send (nxt_unit.c:5939)
==166690== by 0x4FE8C77: nxt_unit_request_done (nxt_unit.c:3309)
==166690== by 0x4FEE13B: nxt_php_execute (nxt_php_sapi.c:1257)
==166690== by 0x4FEE2F1: nxt_php_dynamic_request (nxt_php_sapi.c:1128)
==166690== by 0x4FEE79E: nxt_php_request_handler (nxt_php_sapi.c:1023)
==166690== by 0x4FE92AD: nxt_unit_process_ready_req (nxt_unit.c:4846)
==166690== by 0x4FED1B4: nxt_unit_run_once_impl (nxt_unit.c:4605)
==166690== by 0x4FED3AE: nxt_unit_run (nxt_unit.c:4548)
==166690== by 0x4FEEC2A: nxt_php_start (nxt_php_sapi.c:514)
==166690== Address 0x1ffeffea5f is on thread 1's stack
==166690== in frame #3, created by nxt_unit_port_send (nxt_unit.c:5907)
==166690== Uninitialised value was created by a stack allocation
==166690== at 0x4FE8C05: nxt_unit_request_done (nxt_unit.c:3255)
These were due to the nxt_port_msg_t msg struct in nxt_unit_ready() and
nxt_unit_request_done() not being fully initialised.
Whether or not this is an actual problem an obviously correct thing to
do is to fully empty-initialise the structure and then we don't need to
explicitly set any members to 0 afterwards providing a nice cleanup as
well.
Link: <https://en.cppreference.com/w/c/language/initialization#Empty_initialization>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Update (cargo update) the rust crates for wasm-wasi-component, otel &
unitctl.
This will fix build issues with wasm-wasi-component & rustc 1.86+.
It will also fix dependabot issues in otel and unitctl.
Link: <https://github.com/bytecodealliance/wasmtime/issues/10184>
Link: <https://github.com/nginx/unit/pull/1585>
Link: <https://github.com/nginx/unit/pull/1589>
Link: <https://github.com/nginx/unit/pull/1570>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
In Unit we have a number of character arrays which are intentionally not
NUL terminated.
With GCC 15 this
static const char hex[16] = "0123456789ABCDEF";
will trigger a warning like
$ gcc -Wextra -c nonstring.c
nonstring.c: In function ‘hexit’:
nonstring.c:9:37: warning: initializer-string for array of ‘char’ truncates NUL terminator but destination lacks ‘nonstring’ attribute (17 chars into 16 available) [-Wunterminated-string-initialization]
9 | static const char hex[16] = "0123456789ABCDEF";
| ^~~~~~~~~~~~~~~~~~
By adding NXT_NONSTRING like
static const char hex[16] NXT_NONSTRING = "0123456789ABCDEF";
we no longer get the warning.
Cc: Alejandro Colomar <alx@kernel.org>
Co-authored-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This is a wrapper around __attribute__ ((__nonstring__)). Traditionally
this was used to mark char array variables that intentionally lacked a
terminating NUL byte, this would then cause warning to either be quelled
or emitted for various memory/string functions.
GCC 15 introduced a new warning, Wunterminated-string-initialization,
which will always warn on things like
static const char hex[16] = "0123456789ABCDEF";
However this is very much intentionally not NUL terminated.
When the Wunterminated-string-initialization patch went in, the
"nonstriong" attribute didn't quell this warning, however a patch has
since gone in (prior to the GCC 15 release) to enable this attribute to
quell this warning.
In Unit we disabled this new warning with an eye to being able to
re-enable it again, this patch is the first in a series to do just that.
So the above example would become
static const char hex[16] NXT_NONSTRING = "0123456789ABCDEF";
Link: <https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=44c9403ed1833ae71a59e84f9e37af3182be0df5>
Link: <https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=622968990beee7499e951590258363545b4a3b57>
Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117178#c21>
Cc: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
<https://bz.apache.org/bugzilla/show_bug.cgi?id=64563>
Patch taken from <https://github.com/apache/tomcat/commit/1c1c77b0efb667cea80b532440b44cea1dc427c3.patch>
[ Subject / message tweak - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Patch taken from <https://github.com/apache/tomcat/commit/1cddae8da4ecb4ac04575d3b5fba2daa2e0c8ead.patch>
[ Subject / message tweak - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Note: This may not be *specific* to Django 5.x but is where the issue
showed up.
@codedoga on GitHub reported an issue with Unit and Django 5.x
When trying to perform a simple POST/PUT request with body data, Unit
was throwing the following error
2025/02/16 11:07:14 [error] 6#6 [unit] #9: Python failed to call 'future.result()'
Traceback (most recent call last):
File "/usr/local/lib/python3.13/site-packages/django/core/handlers/asgi.py", line 162, in __call__
await self.handle(scope, receive, send)
File "/usr/local/lib/python3.13/site-packages/django/core/handlers/asgi.py", line 208, in handle
task.result()
~~~~~~~~~~~^^
File "/usr/local/lib/python3.13/site-packages/django/core/handlers/asgi.py", line 239, in listen_for_disconnect
assert False, "Invalid ASGI message after request body: %s" % message["type"]
^^^^^
AssertionError: Invalid ASGI message after request body: http.request
There is no such issue with Django 4.x
The issue was caused when Django started doing an 'async receive()' just
after we have handled the initial request and passed it to the
application. Django is then looking to see if/when we send it a
'http.disconnect' message.
We were not prepared for this and would go through all the motions of
handling the request again which would result in the erroneous
'http.request' message.
What we need to do is track when we've handled the initial request. We
can then use that information coupled with the fact if we get a request
with 0 content length then we basically have nothing to do.
For this we create a new nxt_py_asgi_http_t member, request_received.
We can repurpose 'empty_body_received' for this if we rename it and
change where we set it as now if 'request_received' is true then so
would 'empty_body_received'.
'empty_body_received' was actually part of a previous commit that was
addressing various receive() issues. I've checked that the provided
reproducer application still works.
Link: <https://github.com/django/django/commit/1d1ddffc27cd55c011298cd09bfa4de3fa73cf7a>
Link: <https://github.com/nginx/unit/issues/564>
Fixes: 567545213 ("Python: fixing ASGI receive() issues.")
Closes: https://github.com/nginx/unit/issues/1561
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Firefox (going back a couple of years at least) was unable to open a
WebSocket connection to Unit due to it sending a Connection header of
Connection: keep-alive, Upgrade
However in Unit we were expecting only a single value in the header.
Fix the 'Connection' parsing in nxt_h1p_connection() to address this.
Closes: https://github.com/nginx/unit/issues/772
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Bumps <https://github.com/sfackler/rust-openssl> from 0.10.68 to
0.10.70.
Signed-off-by: dependabot[bot] <support@github.com>
Link: Release notes <https://github.com/sfackler/rust-openssl/releases>
Link: Commits <https://github.com/sfackler/rust-openssl/compare/openssl-v0.10.68...openssl-v0.10.70>
[ Tweaked commit message/subject - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
It was reported on GitHub that Unit was unable to work with WebSockets
under Litestar Python applications.
This was due to Unit sending a 'method' variable in the WebSocket's
connection scope, which Litestar was interpreting as being a normal HTTP
connection.
The ASGI WebSocket specification makes no mention about setting a
'method', so let's not send it on WebSockets.
Also tested this change with basic ASGI WebSockets and FastAPI
WebSockets and obviously pytests still pass.
Closes: https://github.com/nginx/unit/issues/1507
Link: <https://asgi.readthedocs.io/en/latest/specs/www.html#websocket-connection-scope>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Ruby 3.4 started to actually mark some deprecated functions as
*deprecated* now resulting in compiler warnings (which due to -Werror we
treat as errors and thus the build fails).
The *new* functions were actually introduced back in Ruby 1.9.2, so have
been around for quite some time. We claim support for Ruby 2.0 onwards
so this is more than fine.
The new API replaces the old 'mark' and 'free' parameters with a struct
that allows for more fine tuning/configuration. We never made use of
either of those parameters and so the only members of this struct we
*need* to set is the structure wrapper name and the dsize function
pointer which is passed a pointer to the underlying wrapped structure to
calculate its memory usage. While this is *not* required the
documentation *recommends* setting it (though it doesn't say how it's
used).
Ruby pytests still pass after this change...
Closes: https://github.com/nginx/unit/issues/1525
Link: <https://bugs.ruby-lang.org/issues/19998>
Link: <https://docs.ruby-lang.org/en/3.4/extension_rdoc.html#label-C+struct+to+Ruby+object>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Associate file extension `.mjs` with `application/javascript`.
Context: common output of static site generators. There's little risk of
ambiguity for this extension, so might as well support it out of the
box.
[ Subject tweak - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
The superfluous else condition in nxt_otel_propagate_header was dead code.
This commit removes it.
Signed-off-by: Ava Hahn <a.hahn@f5.com>
|
|
This commit adds NULL checks for the request->otel object that
were missed in the Traceparent and Tracestate routines.
Closes: https://github.com/nginx/unit/issues/1523
Closes: https://github.com/nginx/unit/issues/1526
Fixes: 9d3dcb800 ("otel: add build tooling to include otel code")
Signed-off-by: Ava Hahn <a.hahn@f5.com>
|
|
Run 'cargo update' to get the latest version of the required crates in
preparation for the 1.34.0 release.
This resolves a dependabot notification regarding 'idna'.
Link: <https://github.com/nginx/unit/security/dependabot/13>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Run 'cargo update' to get the latest version of the required crates in
preparation for the 1.34.0 release.
The rustls update fixes a panic in `rustls::server::Acceptor::accept()`,
but Unit does not use this code path and was not affected.
Link: <https://rustsec.org/advisories/RUSTSEC-2024-0399.html>
Link: <https://github.com/nginx/unit/security/dependabot/11>
Closes: <https://github.com/nginx/unit/issues/1503>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
For no real reason other than to be on the latest release for the next
release of Unit...
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
When compiling OTEL support with rustc 1.83.0 we started getting the
following warning
Compiling otel v0.1.0 (/home/andrew/src/unit/src/otel)
warning: creating a mutable reference to mutable static is discouraged
--> src/lib.rs:42:9
|
42 | SPAN_TX.take();
| ^^^^^^^^^^^^^^ mutable reference to mutable static
|
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
= note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives
= note: `#[warn(static_mut_refs)]` on by default
warning: `otel` (lib) generated 1 warning
Finished `release` profile [optimized] target(s) in 1m 07s
However it *seems* our usage is OK, so we can disable this warning
(which it seems will soon turn into a hard error), fortunately we only
need to disable it for the nxt_otel_rs_span_tx() function.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Wasmtime 27.0.0 adjusted the C API to start flowing through the
directory and file permission bits of the underlying rust
wasi_config_preopen_dir() implementation.
The directory permissions control whether a directory is read-only or
whether you can create/modify files within.
You always need at least WASMTIME_WASI_DIR_PERMS_READ.
The file permissions control whether you can read or read/write files.
WASMTIME_WASI_FILE_PERMS_WRITE seems to imply
WASMTIME_WASI_FILE_PERMS_READ (but we add both just to make it clear
what we want)
[ Permissions tweak and commit message - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
nxt_cpymem() is basically mempcpy(3)
Like mempcpy() nxt_cpymem() returns a void *.
nxt_cpymem() is implemented as a wrapper around memcpy(3), however
before returning the new pointer value we cast the return of memcpy(3)
to a u_char *, then add the length parameter to it.
I guess this was done to support compilers that do not support
arithmetic on void pointers as the C standard forbids it.
However since we removed support for compilers other than GCC and Clang
(ending in commit 9cd11133 ("Remove support for Sun's Sun Studio/SunPro
C compiler")) this is no longer an issue as both GCC and Clang support
arithmetic on void pointers (without the -pedantic option) by treating
the size of a void as 1.
While removing the unnecessary casting in this case doesn't necessarily
improve type-safety (as we're dealing with void *'s in and out), it does
just make the code that little more readable.
Oh and for interest we have actually already been relying on this
extension
src/nxt_array.c:143:40: warning: arithmetic on a pointer to void is a GNU extension [-Wgnu-pointer-arith]
143 | nxt_memcpy(data, src->elts + (i * size), size);
| ~~~~~~~~~ ^
src/nxt_string.h:45:24: note: expanded from macro 'nxt_memcpy'
45 | (void) memcpy(dst, src, length)
| ^~~
which was introduced in e2b53e16 ("Added "rootfs" feature.") back in
2020.
Link: <https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html>
Link: <https://clang.llvm.org/docs/LanguageExtensions.html#introduction>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Allow format to be an object to generate JSON logs. The object keys
become JSON field names, and values support string, variable, and JS.
Note that when there is no JS in the format values, the object will
be pre-serialized to a JSON template string at configuration phase
for better performance.
Example config:
{
"access_log": {
"path": "/tmp/access.log",
"format": {
"remote_addr": "$remote_addr",
"time_local": "$time_local",
"request_line": "$request_line",
"status": "$status",
"body_bytes_sent": "$body_bytes_sent",
"header_referer": "$header_referer",
"header_user_agent": "$header_user_agent"
}
}
}
|
|
This is a preparatory refactoring for upcoming JSON format support
in access log. We will extend format option to access object for
JSON support.
No functional changes.
|
|
This is a preparatory refactoring for upcoming JSON format support
in access log.
No functional changes.
|
|
This is a preparatory refactoring for upcoming JSON format support
in access log.
No functional changes.
|
|
Adds code responsible for users to apply the `telemetry` configuration
options.
configuration snippet as follows:
{
"settings": {
"telemetry": {
"batch_size": 20,
"endpoint": "http://lgtm:4318/v1/traces",
"protocol": "http",
"sampling_ratio": 1
}
},
"listeners": {
"*:80": {
"pass": "routes"
}
},
"routes": [
{
"match": {
"headers": {
"accept": "*text/html*"
}
},
"action": {
"share": "/usr/share/unit/welcome/welcome.html"
}
},
{
"action": {
"share": "/usr/share/unit/welcome/welcome.md"
}
}
]
}
Signed-off-by: Ava Hahn <a.hahn@f5.com>
Signed-off-by: Gabor Javorszky <g.javorszky@f5.com>
|
|
Enables Unit to parse the tracestate and traceparent headers and add it
to the list, as well as calls to nxt_otel_test_and_call_state.
Signed-off-by: Ava Hahn <a.hahn@f5.com>
Signed-off-by: Gabor Javorszky <g.javorszky@f5.com>
|
|
Adds the --otel flag to the configure command and the various build time
variables and checks that are needed in this flow.
It also includes the nxt_otel.c and nxt_otel.h files that are needed for
the rest of Unit to talk to the compiled static library that's generated
from the rust crate.
Signed-off-by: Ava Hahn <a.hahn@f5.com>
Co-authored-by: Gabor Javorszky <g.javorszky@f5.com>
Signed-off-by: Gabor Javorszky <g.javorszky@f5.com>
|
|
This is purely the source code of the rust end of opentelemetry. It does
not have build tooling wired up yet, nor is this used from the C code.
Signed-off-by: Ava Hahn <a.hahn@f5.com>
Signed-off-by: Gabor Javorszky <g.javorszky@f5.com>
|
|
This fixes an issue we had with wasm-wasi-component failing to load
components with
2024/11/06 21:08:50 [alert] 107196#107196 failed to create initial state
Caused by:
0: failed to compile component
1: WebAssembly translation error
2: Invalid input WebAssembly code at offset 15936: zero byte expected
Which was a symptom of
<https://github.com/bytecodealliance/wasmtime/issues/9130>
Closes: https://github.com/nginx/unit/issues/1477
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
nxt_log_alert() was missing the nxt_str_t parameter as required by the
%V format specifier.
This was found with the Unit clang-ast plugin.
Fixes: 7bf625394 ("Custom implementation of Base64 decoding function.")
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
sizeof() should never be used to get the size of an array. It is
very unsafe, since arrays easily decay to pointers, and sizeof()
applied to a pointer gives false results that compile and produce
silent bugs.
It's better to use nxt_items(), which implements sizeof()
division, which recent compilers warn when used with pointers.
This change would have caught a couple of bugs that were *almost*
introduced
First up is the _infamous_ ternary macro bug (yes, using the ternary
operator in a macro is of itself a bad idea)
nxt_str_set(&port, (r->tls ? "https://" : "http://"));
which in the macro expansion runs:
(&port)->length = nxt_length((r->tls ? : "https://" : "http://"));
which evaluates to:
port.length = sizeof(r->tls ? "https://" : "http://") - 1;
which evaluates to:
port.length = 8 - 1;
Of course, we didn't want a compile-time-constant 8 there, but
rather the length of the string.
The above bug is not obvious to the untrained eye, so let's show some
example programs that may give some more hints about the problem.
$ cat sizeof.c
#include <stdio.h>
int
main(void)
{
printf("%zu\n", sizeof("01"));
printf("%zu\n", sizeof("012"));
printf("%zu\n", sizeof(char *));
}
$ cc -Wall -Wextra sizeof.c
$ ./a.out
3
4
8
sizeof() returns the size in bytes of the array passed to it, which in
case of char strings, it is equivalent to the length of the string + 1
(for the terminating '\0').
However, arrays decay very easily in C, and they decay to a pointer to
the first element in the array. In case of strings, that is a 'char *'.
When sizeof() is given a pointer, it returns the size of the pointer,
which in most platforms is 8.
The ternary operator (?) performs default promotions (and other
nefarious stuff) that may surprise even the most experienced
programmers. It contrasts the __builtin_choose_expr() GCC builtin [1],
which performs almost equivalently, but without the unwanted effects of
the ternary operator.
[1]: <https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr>
$ cat ?.c
#include <stdio.h>
int
main(void)
{
printf("%zu\n", sizeof("01"));
printf("%zu\n", sizeof(__builtin_choose_expr(1, "01", "01")));
printf("%zu\n", sizeof(1 ? "01" : "01"));
printf("%zu\n", sizeof(char *));
}
$ cc -Wall -Wextra ?.c
$ ./a.out
3
3
8
8
In the above program, we can see how the ternary operator (?) decays
the array into a pointer, and makes it so that sizeof() will return a
constant 8.
As we can see, everything in the use of the macro would make it look
like it should work, but the combination of some seemingly-safe side
effects of various C features produces a completely unexpected bug.
Second up is a more straight forward case of simply calling nxt_length()
on a char * pointer.
Like the above this will generally result in a length of 7.
When you sit and think about it, you know very well sizeof(char *) is
probably 8 these days (but may be some other value like 4).
But when you're in the depths of code it's very easy to overlook this
when all you're thinking about is to get the length of some string.
Let's look at this patch in action
$ cat sdiv.c
#include <stdio.h>
#define nxt_nitems(x) (sizeof(x) / sizeof((x)[0]))
#define nxt_length(s) (nxt_nitems(s) - 1)
#define nxt_unsafe_length(s) (sizeof(s) - 1)
#define STR_LITERAL "1234567890"
static const char *str_lit = "1234567890";
int main(void)
{
printf("[STR_LITERAL] nxt_unsafe_length(\"1234567890\") [%lu]\n",
nxt_unsafe_length(STR_LITERAL));
printf("[STR_LITERAL] nxt_length(\"1234567890\") [%lu]\n",
nxt_length(STR_LITERAL));
printf("[char * ] nxt_unsafe_length(\"1234567890\") [%lu]\n",
nxt_unsafe_length(str_lit));
printf("[char * ] nxt_length(\"1234567890\") [%lu]\n",
nxt_length(str_lit));
return 0;
}
First lets compile it without any flags
$ make sdiv
$ ./sdiv
[STR_LITERAL] nxt_unsafe_length("1234567890") [10]
[STR_LITERAL] nxt_length("1234567890") [10]
[char * ] nxt_unsafe_length("1234567890") [7]
[char * ] nxt_length("1234567890") [7]
It compiled without error and runs, although with incorrect results for
the two char *'s.
Now lets build it with -Wsizeof-pointer-div (also enabled with -Wall)
$ CFLAGS="-Wsizeof-pointer-div" make sdiv
cc -Wsizeof-pointer-div nxt_nitems.c -o nxt_nitems
sdiv.c: In function ‘main’:
sdiv.c:3:44: warning: division ‘sizeof (const char *) / sizeof (char)’ does not compute the number of array elements [-Wsizeof-pointer-div]
3 | #define nxt_nitems(x) (sizeof(x) / sizeof((x)[0]))
| ^
nxt_nitems.c:4:34: note: in expansion of macro ‘nxt_nitems’
4 | #define nxt_length(s) (nxt_nitems(s) - 1)
| ^~~~~~~~~~
nxt_nitems.c:22:16: note: in expansion of macro ‘nxt_length’
22 | nxt_length(str_lit));
| ^~~~~~~~~~
nxt_nitems.c:10:20: note: first ‘sizeof’ operand was declared here
10 | static const char *str_lit = "1234567890";
| ^~~~~~~
So we now get a very loud compiler warning (coming from nxt_length(char
*), nxt_unsafe_length() of course didn't trigger any warnings), telling
us we're being daft.
The good news is this didn't find any existing bugs! Let's keep it that
way...
Link: <https://stackoverflow.com/a/57537491>
Cc: Andrew Clayton <a.clayton@nginx.com>
Signed-off-by: Alejandro Colomar <alx@nginx.com>
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Tested-by: Andrew Clayton <a.clayton@nginx.com>
[ Tweaked and expanded the commit message - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Mostly more 'static nxt_str_t ...'s
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
When using JS configuration for the "format" option, access log entries
were being written without newline characters. This commit adds the
missing newline character to each log entry.
Closes: https://github.com/nginx/unit/issues/1458
|
|
This commit introduces a new flag to control the addition of newline
characters in access log entries. This is prepared for fixing the issue
where log entries lack newlines when using JS configuration.
|
|
In the perl language module we create a new perl *module* on the fly
comprised of some preamble, the specified perl script and some
post-amble.
In the preamble we create a constructor called new(), however this can
clash with other constructors also called new.
While this can be worked around by instead of doing
... new CLASS
rather do
... CLASS->new()
While this constructor was added in commit 3b2c1d0e ("Perl: added
implementation delayed response and streaming body."), I don't see that
we actually use it anywhere (nor is it seemingly something we document)
and if we simply remove it then things still seem to work, including the
Perl pytests
...
test/test_perl_application.py::test_perl_streaming_body_multiple_responses[5.38.2] PASSED
...
test/test_perl_application.py::test_perl_delayed_response[5.38.2] PASSED
test/test_perl_application.py::test_perl_streaming_body[5.38.2] PASSED
...
Closes: https://github.com/nginx/unit/issues/1456
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Bumps <https://github.com/bytecodealliance/wasmtime> from 24.0.0 to
24.0.1.
Fixes:
a runtime crash when combining tail-calls with host imports that
capture a stack trace or trap. GHSA-q8hx-mm92-4wvg
a race condition could lead to WebAssembly control-flow integrity and
type safety violations. GHSA-7qmx-3fpx-r45m
Link: Release notes <https://github.com/bytecodealliance/wasmtime/releases>
Link: Changelog <https://github.com/bytecodealliance/wasmtime/blob/main/docs/WASI-some-possible-changes.md>
Link: Commits <https://github.com/bytecodealliance/wasmtime/compare/v24.0.0...v24.0.1>
Signed-off-by: dependabot[bot] <support@github.com>
[ Tweaked commit message/subject - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
There has been a long standing clang-analyzer issue in
nxt_process_check_pid_status(), well ever since I introduced this
function in commit b0e2d9d0a ("Isolation: Switch to fork(2) & unshare(2)
on Linux."),
It is complaining that there are cases where 'status' could be returned
with an undefined or garbage value.
Now I'm convinced this can't happen
If nxt_process_pipe_timer() returns NXT_OK
read(2) the status value
If the read(2) failed or if we got NXT_ERROR from
nxt_process_pipe_timer(), NXT_OK (0) and NXT_ERROR (-1) are the only
possible return values here, then we set status to -1
I don't see a scenario where status is either not set by the read(2) or
not set to -1.
Now I'm not a fan of initialising variables willy-nilly, however, in
this case if we initialise status to -1, then we can simply remove the
if (ret <= 0) {
check. So it closes this (non-)issue but also provides a little code
simplification.
NOTE: There is no need to check the return from the read(2) here. We are
reading a single byte, we either get it or don't.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
The function nxt_term_parse() is able to take strings with trailing
whitespace e.g. "1w1d ", add a test case to cover such things.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Both clang-analyzer and coverity flagged an issue in nxt_term_parse()
that we set 'state = st_letter' but then set it to 'state = st_space'
before using it.
While we could simply remove the first assignment and placate the
analyzers, upon further analysis it seems that there is some more
cleanup that could be done in this function.
This commit addresses the above issue, subsequent commits will continue
the cleanup.
To solve the unused assignment issue we can get rid of the
'state == st_letter'
assignment and unconditionally execute the code that was behind the
if (state != st_letter) {
guard. If we're not handling a space then we should have either a digit
or letter.
Also, perhaps more importantly, this if () statement would never be
false at this point as state would never == st_letter.
We may as well also remove the st_letter enum value.
The src/test/nxt_term_parse_test.c still passes
tests: [notice] term parse test passed
NOTE: Although this function is not currently used in Unit (only by
src/test/nxt_term_parse_test.c), it is probably worth cleaning it up and
solving one of the open clang-analyzer (and coverity) issues.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
On some Python 3.11 systems, 3.11.9 & 3.11.10, we were seeing a crash
triggered by Py_Finalize() in nxt_python_atexit() when running one of
our pytests, namely
test/test_python_factory.py::test_python_factory_invalid_callable_value
2024/09/12 15:07:29 [alert] 5452#5452 factory "wsgi_invalid_callable" in module "wsgi" can not be called to fetch callable
Fatal Python error: none_dealloc: deallocating None: bug likely caused by a refcount error in a C extension
Python runtime state: finalizing (tstate=0x00007f560b88a718)
Current thread 0x00007f560bde7ad0 (most recent call first):
<no Python frame>
2024/09/12 15:07:29 [alert] 5451#5451 app process 5452 exited on signal 6 (core dumped)
This was due to
obj = PyDict_GetItemString(PyModule_GetDict(module), callable);
in nxt_python_set_target() which returns a *borrowed* reference, then
due to the test meaning this is a `None` object we `goto fail` and call
Py_DECREF(obj);
which then causes `Py_Finalize()` to blow up.
The simple fix is to just increment its reference count before the `goto
fail`.
Note: This problem only showed up under (the various versions of Python
we test on); 3.11.9 & 3.11.10. It doesn't show up under; 3.6, 3.7, 3.9,
3.10, 3.12
Cc: Konstantin Pavlov <thresh@nginx.com>
Closes: https://github.com/nginx/unit/issues/1413
Fixes: a9aa9e76d ("python: Support application factories")
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
When the client closes the connection before the upstream,
the proxy's error handler was calling cleanup operation like
peer close and request close twice, this fix ensures the cleanup
is performed only once, improving proxy stability.
Closes: https://github.com/nginx/unit/issues/828
|
|
While the C based wasm language module inherits the process environment
the Rust based wasm-wasi-component language module did not.
One upshot of this is that with wasm-wasi-component you don't get access
to any environment variables specified in the Unit configuration.
wasm-wasi-component was based on wasmtime 17.0.0. This capability wasn't
added to the wasmtime-crate until version 20.0.0.
Now that wasm-wasi-component has been updated to a newer wasmtime-crate
we can enable this functionality.
Closes: https://github.com/nginx/unit/issues/1312
[ Commit message - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Signed-off-by: Ava Hahn <a.hahn@f5.com>
|
|
This was found via ASan.
Given a listener address like ":" (or any address where the first
character is a colon) we can end up under-reading the addr->start
buffer here
if (nxt_slow_path(*(buf + length - 1) == '.')) {
due to length (essentially the position of the ":" in the string) being
0.
Seeing as any address that starts with a ":" is invalid Unit config
wise, we should simply reject the address if length == 0 in
nxt_sockaddr_inet_parse().
Link: <https://clang.llvm.org/docs/AddressSanitizer.html>
Signed-off-by: Arjun <pkillarjun@protonmail.com>
[ Commit message - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This feature allows users to specify conditions to check if one
route is matched. It is used the same way as the "if" option in
the access log.
Example:
{
"match": {
"if": "`${headers['User-Agent'].split('/')[0] == 'curl'}`"
},
"action": {
"return": 204
}
}
|
|
|
|
This nxt_tstr_cond_t will be reused for the feature of adding "if"
option to the "match" object. The two "if" options have the same usage.
|
|
|
