| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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>
|
|
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
|
|
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.
|
|
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.
|
|
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
|
|
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>
|
|
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>
|
|
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>
|
|
Found by UndefinedBehaviorSanitizer.
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Found by UndefinedBehaviorSanitizer.
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Can be reproduced by test/test_settings.py::test_settings_send_timeout
with enabled UndefinedBehaviorSanitizer.
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
When Unit receives a request, if the body of that request is greater
than a certain amount (16KiB by default) then it is written to a
temporary file.
When a language module goes to read the request body in such situations
it will end up using read(2).
The wasm-wasi-component language module was failing to properly read
request bodies of around 2GiB or more.
This is because (on Linux at least) read(2) (and other related system
calls) will only read (or write) at most 0x7ffff000 (2,147,479,552)
bytes, this is the case for both 32 and 64-bit systems.
Regardless, it's probably not a good idea doing IO in such large chunks
anyway.
This patch changes the wasm-wasi-component language module to read the
request buffer in 32MiB chunks (this matches the original 'wasm'
language module).
We are still limited to a 4GiB address space and can only upload files a
little under 4GiB.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Run from the repository root like
$ rustfmt --edition 2021 src/wasm-wasi-component/src/lib.rs
Also manually fix up some overly long comments.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This is the work of Alex Crichton.
This is written in Rust. The problem is that there is currently no
support on the C side of things for the component model, which is the
point of this module.
It talks to Unit via automatically generated bindings.
I've (Andrew) just made some minor tweaks to src/lib.rs, build.rs &
Cargo.toml to adjust some paths, adjust where we get the language module
config from and the module name and where it's located in the source
tree,
I also removed and disabled the tracking of the Cargo.lock file, this is
constantly changing and not tracking it seems right for 'libraries' and
dropped the README's...
Other than that I have tried to leave his work intact, subsequent
commits will make some larger changes, but I didn't want to intermix
them with Alex's work.
One such commit will update the module to use wasmtime 17 which brings
WASI 0.2.0 support.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This is required to actually _build_ the 'wasm-wasi-componet' language
module.
The nxt_wasm_wc_app_conf_t structure consists of the component name, e.g
my_component.wasm, this is required. It also consists of an object to
store the directories that are allowed access to by the component, this
is optional.
The bulk of the configuration infrastructure will be added in a
subsequent commit.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This is the first commit in adding WebAssembly Component Model language
module support.
This just adds a new NXT_APP_WASM_WC type, required by subsequent
commits.
The WC stands for WASI_COMPONENT
This new module will have a type of 'wasm-wasi-component'.
Link: <https://github.com/nginx/unit/issues/1098>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
@filiphanes requested support for bytearray
and memoryview in the request body here:
<https://github.com/nginx/unit/issues/648>
This patch implements bytearray body support only.
Memoryview body still need to be implemented.
|
|
|
|
In that particular issue the compiled nuxt files end up importing the
http module as node:http rather than http only. This bypasses unit's
custom loader implementation which only check for the http or unit-http
modules, and their websocket counterparts.
This changeset adds replace sources for both the node:http and
node:websocket import signatures.
Closes: https://github.com/nginx/unit/issues/1013
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This commit introduces the 'vars' JavaScript object to NJS,
enabling direct access to native variables such as $uri and $arg_foo.
The syntax is `${vars.var_name}` or `${'vars[var_name]'}`.
For example:
{
"action": {
"share": "`/www/html${vars.uri}`"
}
}
|
|
|
|
This commit is for subsequent commits that will support njs variable
accessing. In this commit, nxt_var_get() is introduced to extend
the variable handling capabilities. Concurrently, nxt_var_ref_get()
has been refactored to use in both configuration and request phases.
|
|
This commit enhances nxt_var_cache_value() to enable variable access
using string names, complementing the existing reference index method.
The modification ensures future compatibility with njs variable access.
|
|
|
|
|
|
This is a simple temporary fix (doesn't address the underlying problem)
for an issue reported by a user on GitHub whereby downloading of files
from a PHP application would cause the router process to crash.
This is actually a generic problem that will affect anything sending
data via nxt_unit_response_write().
This is just a simple fix for the 1.32 release, after which the full
correct fix will be worked out.
Link: <https://github.com/nginx/unit/issues/1125>
Reported-by: rustedsword <https://github.com/rustedsword>
Co-developed-by: rustedsword <https://github.com/rustedsword>
Tested-by: rustedsword <https://github.com/rustedsword>
Tested-by: Andrew Clayton <a.clayton@nginx.com>
Reviewed-by: Zhidao Hong <z.hong@f5.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Functionally identical, but marginally more idiomatic.
Refines: fbeb2065b180e2376088387ee150d3975dc08cd5
|
|
Several users in GitHub have asked for the ability to set the
permissions of the unitd UNIX Domain control socket.
This can of course be done externally, but can be done much cleaner by
Unit itself.
This commit adds three new options
--control-mode Set the mode of the socket, e.g 644
--control-user Set the user/owner of the socket, e.g unit
--control-group Set the group of the socket, e.g unit
Of course these only have an affect when using a UNIX Domain Socket for
the control socket.
Requested-by: michaelkosir <https://github.com/michaelkosir>
Requested-by: chopanovv <https://github.com/chopanovv>
Link: <https://github.com/nginx/unit/issues/254>
Link: <https://github.com/nginx/unit/issues/980>
Closes: https://github.com/nginx/unit/issues/840
Tested-by: Liam Crilly <liam.crilly@nginx.com>
Reviewed-by: Zhidao Hong <z.hong@f5.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This wraps chown(2) but takes the user/owner and group as strings.
It's a little long winded as it uses the thread safe versions of
getpwnam()/getgrname() which require a little more work.
This function will be used by the following commit that allows to set
the permissions of the Unix domain control socket.
We need to cast uid & gid to long in the call to nxt_thread_log_alert()
to appease clang-ast as it's adamant that uid/gid are unsigned ints, but
chown(2) takes -1 for these values to indicate don't change this item,
and it'd be nice to show them in the error message.
Note that getpwnam()/getgrname() don't define "not found" as an error as
per their man page
The formulation given above under "RETURN VALUE" is from POSIX.1-2001.
It does not call "not found" an error, and hence does not specify what
value errno might have in this situation. But that makes it impossible
to recognize errors. One might argue that according to POSIX errno
should be left unchanged if an entry is not found. Experiments on var‐
ious UNIX-like systems show that lots of different values occur in this
situation: 0, ENOENT, EBADF, ESRCH, EWOULDBLOCK, EPERM, and probably
others.
Thus if we log an error from these functions we can end up with the
slightly humorous error message
2024/02/12 15:15:12 [alert] 99404#99404 getpwnam_r("noddy", ...) failed (0: Success) (User not found) while creating listening socket on unix:/opt/unit/control.unit.sock
Reviewed-by: Zhidao Hong <z.hong@f5.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
* Take options as well as requestListener
Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.
This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.
* Add test file to start node application with options
* Add changes to docs/changes.xml
Closes: https://github.com/nginx/unit/issues/1043
|
|
It's an integer, not a floating number.
Fixes: 68c6b67ffc84 ("Configuration: support for rational numbers.")
Closes: https://github.com/nginx/unit/issues/1115
Link: <https://github.com/nginx/unit/pull/1116>
Reviewed-by: Zhidao Hong <z.hong@f5.com>
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Cc: Dan Callahan <d.callahan@f5.com>
Cc: Valentin Bartenev <vbartenev@gmail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
|
|
The commit that added support for Unix sockets accepts abstract sockets
using '@' in the config, but we stored it internally using '\0'.
We want to support abstract sockets transparently to the user, so that
if the user configures unitd with '@', if we receive a query about the
current configuration, the user should see the same exact thing that was
configured. So, this commit avoids the transformation in the internal
state file, storing user input pristine, and we only transform the '@'
in temporary strings.
This commit fixes another bug, where we try to connect to abstract
sockets with a trailing '\0' in their name due to calling twice
nxt_sockaddr_parse() on the same string. By calling that function only
once with each copy of the string, we have fixed that bug.
The following code was responsible for this bug, which the second time
it was called, considered these sockets as file-backed (not abstract)
Unix socket, and so appended a '\0' to the socket name.
$ grepc -tfd nxt_sockaddr_unix_parse . | grep -A10 @
if (path[0] == '@') {
path[0] = '\0';
socklen--;
#if !(NXT_LINUX)
nxt_thread_log_error(NXT_LOG_ERR,
"abstract unix domain sockets are not supported");
return NULL;
#endif
}
sa = nxt_sockaddr_alloc(mp, socklen, addr->length);
This bug was found thanks to some experiment about using 'const' for
some strings.
And here's some history:
- 9041d276fc6a ("nxt_sockaddr_parse() introducted.")
This commit introduced support for abstract Unix sockets, but they
only worked as "servers", and not as "listeners". We corrupted the
JSON config file, and stored a \u0000. This also caused calling
connect(2) with a bogus trailing null byte, which tried to connect to
a different abstract socket.
- d8e0768a5bae ("Fixed support for abstract Unix sockets.")
This commit (partially) fixed support for abstract Unix sockets, so
they they worked also as listeners. We still corrupted the JSON
config file, and stored a \u0000. This caused calling connect(2)
(and now bind(2) too) with a bogus trailing null byte.
- e2aec6686a4d ("Storing abstract sockets with @ internally.")
This commit fixed the problem by which we were corrupting the config
file, but only for "listeners", not for "servers". (It also fixes
the issue about the terminating '\0'.) We completely forgot about
"servers", and other callers of the same function.
To reproduce the problem, I used the following config:
```json
{
"listeners": {
"*:80": {
"pass": "routes/u"
},
"unix:@abstract": {
"pass": "routes/a"
}
},
"routes": {
"u": [{
"action": {
"pass": "upstreams/u"
}
}],
"a": [{
"action": {
"return": 302,
"location": "/i/am/not/at/home/"
}
}]
},
"upstreams": {
"u": {
"servers": {
"unix:@abstract": {}
}
}
}
}
```
And then check the state file:
$ sudo cat /opt/local/nginx/unit/master/var/lib/unit/conf.json \
| jq . \
| grep unix;
"unix:@abstract": {
"unix:\u0000abstract": {}
After this patch, the state file has a '@' as expected:
$ sudo cat /opt/local/nginx/unit/unix/var/lib/unit/conf.json \
| jq . \
| grep unix;
"unix:@abstract": {
"unix:@abstract": {}
Regarding the trailing null byte, here are some tests:
$ sudo strace -f -e 'bind,connect' /opt/local/nginx/unit/d8e0/sbin/unitd \
|& grep abstract;
[pid 22406] bind(10, {sa_family=AF_UNIX, sun_path=@"abstract\0"}, 12) = 0
[pid 22410] connect(134, {sa_family=AF_UNIX, sun_path=@"abstract\0"}, 12) = 0
^C
$ sudo killall unitd
$ sudo strace -f -e 'bind,connect' /opt/local/nginx/unit/master/sbin/unitd \
|& grep abstract;
[pid 22449] bind(10, {sa_family=AF_UNIX, sun_path=@"abstract"}, 11) = 0
[pid 22453] connect(134, {sa_family=AF_UNIX, sun_path=@"abstract\0"}, 12) = -1 ECONNREFUSED (Connection refused)
^C
$ sudo killall unitd
$ sudo strace -f -e 'bind,connect' /opt/local/nginx/unit/unix/sbin/unitd \
|& grep abstract;
[pid 22488] bind(10, {sa_family=AF_UNIX, sun_path=@"abstract"}, 11) = 0
[pid 22492] connect(134, {sa_family=AF_UNIX, sun_path=@"abstract"}, 11) = 0
^C
Fixes: 9041d276fc6a ("nxt_sockaddr_parse() introducted.")
Fixes: d8e0768a5bae ("Fixed support for abstract Unix sockets.")
Fixes: e2aec6686a4d ("Storing abstract sockets with @ internally.")
Link: <https://github.com/nginx/unit/pull/1108>
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Cc: Liam Crilly <liam.crilly@nginx.com>
Cc: Zhidao Hong <z.hong@f5.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
|
|
Refactor.
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Cc: Zhidao Hong <z.hong@f5.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
|
