This commit refactors the CLI code to accept
multiple instances of the control socket flag.
All subcommands except for edit and save now
support being run against multiple specified
instances of unitd.
* control_socket_addresses CLI field is now a vector
* centralize error related logic into the error module
* wait_for_socket now returns a vector of sockets. all
sockets in vector are waited upon and validated
* extraneous code is removed
* applications, execute, import, listeners, and status
commands all run against N control sockets now
* edit and save commands return error when run against
a single control socket
Signed-off-by: Ava Hahn <a.hahn@f5.com>
Normally when the language modules are built, they are built directly
into the build/lib/unit/modules/ directory.
This then allows Unit to find them without being installed. This is
useful for things like the pytests.
This wasn't happening for the wasm-wasi-component language module. So we
now copy it over and give it the right name as part of the make/build
process.
Reported-by: Andrei Zeliankou <zelenkov@nginx.com>
Fixes: 4e6d7e876 ("Wasm-wc: Wire it up to the build system")
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
The default on Ubuntu 24.04 and newer is now -D_FORTIFY_SOURCE=3 which
clashes with our definition. We shouldnt be setting it for Ubuntus
anyway since _FORTIFY_SOURCE=2 for older distros is already handled by
the defaults in their gcc builds.
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>
Add the following tests cases:
1. When "factory" key is used inside the "targets" option.
2. When "factory" key is used at the root level of python application
config.
3. When factory returns invalid callable or When factory is invalid
callable
Link: <https://github.com/nginx/unit/pull/1336>
[ Commit subject & message formatting tweaks - Andrew ]
Signed-off-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>
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>
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>
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>
* new subcommand for "export" in CLI
* new cmd submodule for exporting config tarballs
* logic to also output to stdout
* README additions
* limitations documented
Signed-off-by: Ava Hahn <a.hahn@f5.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>
This directory should exist already in the system, and if not, it should
(and will) be created at run time, not at install time.
It triggered a warning in Alpine Linux's packaging system:
ERROR: unit*: Packages must not put anything under /var/run
Fixes: 5a37171f73 ("Added default values for pathnames.")
Fixes: 57fc9201cb ("Socket: Created control socket & pid file directories.")
Closes: <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>
Cc: Liam Crilly <liam@nginx.com>
Cc: Konstantin Pavlov <thresh@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: 57fc9201cb ("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: 57fc9201cb ("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: e2b53e16c6 ("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>
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>
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>
"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>
"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>
The various .bin files are in DOS format with trailing ^Ms (CRs) and
should be ignored for whitespace issues.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
Signed-off-by: Arjun <pkillarjun@protonmail.com>
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
[ Some small edits - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
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>
Commit 4fc50258b ("ci: Be more specific when to run the main Unit
checks") limited when the checks for the main ci run, on pushes to
master.
It should have done the same for pull-requests.
Fixes: 4fc50258b ("ci: Be more specific when to run the main Unit checks")
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
This patch does a number of things to help when failing to apply a new
configuration.
* The error body from the Unit control API is displayed which can have
useful troubleshooting information (when the version of curl supports it).
* When using the EDIT option, the temporary file with unapplied changes is
preserved so that the user can edit it again without losing their work.
* Editing JavaScript modules no longer requires that module to have been
enabled.
* Failure to apply edited JavaScript modules now rolls-back to the previous
configuration instead of deleting the module.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
