| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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.
|
|
|
|
|
|
|
|
|
|
|
|
Initially, variable query was designed to accomodate both synchronous
and asynchronous operations. However, upon consideration of actual
requirements, we recognized that asynchronous support was not needed.
The refactoring ensures that the success or failure of the variable
query operation is now directly indicated by its return value. This
change streamlines the function's usage and enhances code clarity,
as it facilitates immediate error handling without the need for
asynchronous callbacks or additional error checking functions.
Note the patch only works for Unit native variables but not njs
variables.
|
|
@oopsoop2 on GitHub reported a performance issue related to the default
listen(2) backlog size of 511 on nginx. They found that increasing it
helped, nginx has a config option to configure this.
They would like to be able to do the same on Unit (which also defaults
to 511 on some systems). This seems reasonable.
NOTE: On Linux before commit 97c15fa38 ("socket: Use a default listen
backlog of -1 on Linux") we defaulted to 511. Since that commit we
default to the Kernels default, which before 5.4 is 128 and after is
4096.
This adds a new per-listener 'backlog' config option, e.g
{
"listeners": {
"[::1]:8080": {
"pass": "routes",
"backlog": 1024
},
}
...
}
This doesn't effect the control socket.
Closes: https://github.com/nginx/unit/issues/1384
Reported-by: <https://github.com/oopsoop2>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
On FreeBSD, OpenBSD & macOS we use a default listen(2) backlog of -1
which means use the OS's default value.
On Linux (and others) we used a hard coded value of 511, presumably due
to this comment
/* Linux, Solaris, and NetBSD treat negative value as 0. */
On Linux (at least since 2.4), this is wrong, Linux treats -1 (and so
on) as use the OS's default (net.core.somaxconn). See this code in
net/socket.c::__sys_listen()
if ((unsigned int)backlog > somaxconn)
backlog = somaxconn;
On Linux prior to 5.4 somaxconn defaulted to 128, since 5.4 it defaults
to 4096.
We've had complaints that a listen backlog of 511 is too small. This
would help in those cases.
Unless they are on an old Kernel, in which case it's worse, but then the
plan is to also make this configurable. This would effect RHEL 8, which
is based on 4.10, however they seem to set somaxconn to 2048, so that's
fine.
Another advantage of using -1 is that we will automatically keep up to
date with the kernels default value.
Before this change
$ ss -tunxlp | grep unit
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port Process
u_str LISTEN 0 511 /opt/unit/control.unit.sock.tmp 4302333 * 0 users:(("unitd",pid=18290,fd=6),("unitd",pid=18289,fd=6),("unitd",pid=18287,fd=6))
tcp LISTEN 0 511 127.0.0.1:8080 0.0.0.0:* users:(("unitd",pid=18290,fd=12))
tcp LISTEN 0 511 [::1]:8080 [::]:* users:(("unitd",pid=18290,fd=11))
After
$ ss -tunxlp | grep unit
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port Process
u_str LISTEN 0 4096 /opt/unit/control.unit.sock.tmp 5408464 * 0 users:(("unitd",pid=132442,fd=6),("unitd",pid=132441,fd=6),("unitd",pid=132439,fd=6))
tcp LISTEN 0 4096 127.0.0.1:8080 0.0.0.0:* users:(("unitd",pid=132442,fd=12))
tcp LISTEN 0 4096 [::1]:8080 [::]:* users:(("unitd",pid=132442,fd=11))
Link: <https://github.com/nginx/unit/issues/1384>
Link: <https://lore.kernel.org/netdev/20191030163620.140387-1-edumazet@google.com/>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Unit generally creates an extra number of router threads (to handle
client connections, not incl the main thread) to match the number of
available CPUs.
There are cases when this can go wrong, e.g on a high CPU count machine
and Unit is being effectively limited to a few CPUs via the cgroups cpu
controller. So Unit may create a large number of router threads when
they are only going to effectively run on a couple of CPUs or so.
There may be other cases where you would like to tweak the number of
router threads, depending on your workload.
As it turns out it looks like it was intended to be made configurable
but was just never hooked up to the config system.
This adds a new '/settings/listen_threads' config option which can be
set like
{
"listen": {
...
},
"settings": {
"listen_threads": 2,
...
},
...
}
Before this patch (on a four cpu system)
$ ps -efL | grep router
andrew 419832 419829 419832 0 5 Aug12 pts/10 00:00:00 unit: router
andrew 419832 419829 419833 0 5 Aug12 pts/10 00:00:00 unit: router
andrew 419832 419829 419834 0 5 Aug12 pts/10 00:00:00 unit: router
andrew 419832 419829 445145 0 5 03:31 pts/10 00:00:00 unit: router
andrew 419832 419829 445146 0 5 03:31 pts/10 00:00:00 unit: router
After, with a threads setting of 2
$ ps -efL | grep router
andrew 419832 419829 419832 0 3 Aug12 pts/10 00:00:00 unit: router
andrew 419832 419829 419833 0 3 Aug12 pts/10 00:00:00 unit: router
andrew 419832 419829 419834 0 3 Aug12 pts/10 00:00:00 unit: router
Closes: https://github.com/nginx/unit/issues/1042
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
At startup, the unit router process creates a number of threads, it
tries to create the same number of threads (not incl the main thread) as
there are 'cpus' in the system.
On Linux the number of available cpus is determined via a call to
sysconf(_SC_NPROCESSORS_ONLN);
in a lot of cases this produces the right result, i.e. on a four cpu
system this will return 4.
However this can break down if unit has been restricted in the cpus it's
allowed to run on via something like cpuset()'s and/or
sched_setaffinity(2).
For example, on a four 'cpu' system, starting unit will create an extra
4 router threads
$ /opt/unit/sbin/unitd
$ ps -efL | grep router
andrew 234102 234099 234102 0 5 17:00 pts/10 00:00:00 unit: router
andrew 234102 234099 234103 0 5 17:00 pts/10 00:00:00 unit: router
andrew 234102 234099 234104 0 5 17:00 pts/10 00:00:00 unit: router
andrew 234102 234099 234105 0 5 17:00 pts/10 00:00:00 unit: router
andrew 234102 234099 234106 0 5 17:00 pts/10 00:00:00 unit: router
Say we want to limit unit to two cpus, i.e.
$ taskset -a -c 2-3 /opt/unit/sbin/unitd
$ ps -efL | grep router
andrew 235772 235769 235772 0 5 17:08 pts/10 00:00:00 unit: router
andrew 235772 235769 235773 0 5 17:08 pts/10 00:00:00 unit: router
andrew 235772 235769 235774 0 5 17:08 pts/10 00:00:00 unit: router
andrew 235772 235769 235775 0 5 17:08 pts/10 00:00:00 unit: router
andrew 235772 235769 235776 0 5 17:08 pts/10 00:00:00 unit: router
So despite limiting unit to two cpus
$ grep Cpus_allowed_list /proc/235772/status
Cpus_allowed_list: 2-3
It still created 4 threads, probably not such an issue in this case, but
if we had a 64 'cpu' system and wanted to limit unit two cpus, then we'd
have 64 threads vying to run on two cpus and with our spinlock
implementation this can cause a lot of thread scheduling and congestion
overhead.
Besides, our intention is currently to create nr router threads == nr
cpus.
To resolve this, on Linux at least, this patch makes use of
sched_getaffinity(2) to determine what cpus unit is actually allowed to
run on.
We still use the result of
sysconf(_SC_NPROCESSORS_ONLN);
as a fallback, we also use its result to allocate the required cpuset
size (where sched_getaffinity() will store its result) as the standard
cpu_set_t only has space to store 1023 cpus.
So with this patch if we try to limit unit to two cpus we now get
$ taskset -a -c 2-3 /opt/unit/sbin/unitd
$ ps -efL | grep router
andrew 236887 236884 236887 0 3 17:20 pts/10 00:00:00 unit: router
andrew 236887 236884 236888 0 3 17:20 pts/10 00:00:00 unit: router
andrew 236887 236884 236889 0 3 17:20 pts/10 00:00:00 unit: router
This also applies to the likes of docker, if you run docker with the
--cpuset-cpus="" option, unit will now create a number of router threads
that matches the cpu count specified.
Perhaps useful if you are running a number of unit docker instances on a
high cpu count machine.
Link: <https://github.com/nginx/unit/issues/1042>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Fixes: 707f4ef8 ("status: Show list of loaded language modules")
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
As the comment for 'Memory-only buffers' says
"... it is equal to offsetof(nxt_buf_t, file.pos)"
and
"... that is it is nxt_buf_t without file and mmap part"
Those are at odds with each other, 'file.pos' comes _after_ 'file' in
the nxt_buf_t structure.
Fix the 'offset()' bit of the comment to reflect that and to match the
relevant macro
#define NXT_BUF_MEM_SIZE offsetof(nxt_buf_t, file)
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
When querying the '/status' node in the control API, display the list of
currently loaded modules.
So we now get something like
{
"modules": {
"python": [
{
"version": "3.12.3",
"lib": "/opt/unit/modules/python.unit.so"
},
{
"version": "3.12.1",
"lib": "/opt/unit/modules/python-3.12.1.unit.so"
}
],
"wasm": {
"version": "0.1",
"lib": "/opt/unit/modules/wasm.unit.so"
},
"wasm-wasi-component": {
"version": "0.1",
"lib": "/opt/unit/modules/wasm_wasi_component.unit.so"
}
},
...
}
This can be useful for debugging to show exactly what modules Unit has
loaded _and_ from where.
Closes: https://github.com/nginx/unit/issues/1343
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
The nxt_app_lang_module_t structure contains various bits of information
as obtained from the nxt_app_module_t structure that language modules
define.
One bit of information that is in the nxt_app_module_t but not in the
nxt_app_lang_module_t is the language module name.
Having this name flowed through will be useful for displaying the loaded
language modules in the /status endpoint.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
In nxt_status_get() call nxt_conf_set_member() multiple times to set the
main /status json sections.
Previously this used hard coded values, 0, 1, 2 etc, if you wanted to
change the order or insert new sections it could mean renumbering all
these.
Instead use a variable to track this index which starts at 0 and is
simply incremented in each call of nxt_conf_set_member().
Currently this is only for the main outer sections, but can be
replicated for inner sections if required.
This is a preparatory patch for adding a new "modules" section at the
top.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This is yet more missed constification, due in this case to me searching
for 'static nxt_str_t ' but these only having a single space after the
type...
Anyway no problem, this can be a preparatory patch for adding further
/status information...
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
These somehow got missed in my previous constification patches...
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Previously, the certificate deletion only handled string type
certificates, causing issues when certificates were specified
as an array in the configuration.
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Adds support for the app factory pattern to the Python language module.
A factory is a callable that returns a WSGI or ASGI application object.
Unit does not support passing arguments to factories.
Setting the `factory` option to `true` instructs Unit to treat the
configured `callable` as a factory.
For example:
"my-app": {
"type": "python",
"path": "/srv/www/",
"module": "hello",
"callable": "create_app",
"factory": true
}
This is similar to other WSGI / ASGI servers. E.g.,
$ uvicorn --factory hello:create_app
$ gunicorn 'hello:create_app()'
The factory setting defaults to false.
Closes: https://github.com/nginx/unit/issues/1106
Link: <https://github.com/nginx/unit/pull/1336#issuecomment-2179381605>
[ Commit message - Dan / Minor code tweaks - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
njs changed strings API so now instead of njs_vm_value_string_set() used
njs_vm_value_string_create() as a drop-in replacement.
Link: <https://github.com/nginx/njs/commit/5730d5ffe23a4965c001d873695d22005fcfa588>
|
|
This issue was found with oss-fuzz.
==18420==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x55dd798a5797 in nxt_vsprintf unit/src/nxt_sprintf.c:163:31
#1 0x55dd798d5bdb in nxt_conf_vldt_error unit/src/nxt_conf_validation.c:1525:11
#2 0x55dd798dd4cd in nxt_conf_vldt_var unit/src/nxt_conf_validation.c:1560:16
#3 0x55dd798dd4cd in nxt_conf_vldt_if unit/src/nxt_conf_validation.c:1592:16
#4 0x55dd798d55f4 in nxt_conf_vldt_object unit/src/nxt_conf_validation.c:2815:23
#5 0x55dd798d6f84 in nxt_conf_vldt_access_log unit/src/nxt_conf_validation.c:3426:11
#6 0x55dd798d55f4 in nxt_conf_vldt_object unit/src/nxt_conf_validation.c:2815:23
#7 0x55dd798d47bd in nxt_conf_validate unit/src/nxt_conf_validation.c:1421:11
#8 0x55dd79871c82 in LLVMFuzzerTestOneInput unit/fuzzing/nxt_json_fuzz.c:67:5
#9 0x55dd79770620 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
#10 0x55dd7975adb4 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:327:6
#11 0x55dd7976084a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:862:9
#12 0x55dd7978cc42 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#13 0x7e8192213082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/libc-start.c:308:16
#14 0x55dd7975188d in _start
Uninitialized value was created by an allocation of 'error.i' in the stack frame
#0 0x55dd798dd42b in nxt_conf_vldt_var unit/src/nxt_conf_validation.c:1557:5
#1 0x55dd798dd42b in nxt_conf_vldt_if unit/src/nxt_conf_validation.c:1592:16
The issue was in nxt_tstr_test() where we create an error message with
nxt_sprintf(), where this error message is then later used with the
'%s' format specifier which expects a nul-terminated string, but by
default nxt_sprintf() doesn't nul-terminate, you must use the '%Z'
specifier to signify a '\0' at the end of the string.
Signed-off-by: Arjun <pkillarjun@protonmail.com>
Co-developed-by: Zhidao HONG <z.hong@f5.com>
Signed-off-by: Zhidao HONG <z.hong@f5.com>
Link: <https://github.com/google/oss-fuzz>
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
[ Commit message/subject - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
These somehow got missed in my previous constification patches...
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
These somehow got missed in my previous constification patches...
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This is a temporary support for chunked request bodies by converting
to Content-Length. This allows for processing of such requests until
a more permanent solution is developed.
A new configuration option "chunked_transform" has been added to enable
this feature. The option can be set as follows:
{
"settings": {
"chunked_transform": true
}
}
By default, this option is set to false, which retains the current
behaviour of rejecting chunked requests with a '411 Length Required'
status code.
Please note that this is an experimental implementation.
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
|
|
It's prepared for the subsequent patch.
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
