| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
For more information see:
https://github.com/rack/rack/commit/42aff22f708123839ba706cbe659d108b47c40c7
|
|
This closes #1006 issue on GitHub.
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
|
|
In nxt_php_execute() it is possible we could bail out before cleaning up
the FILE * representing the PHP script to execute.
At this point we only need to call fclose(3) on it.
We could have possibly moved the opening of this file to later in the
function, but it is probably good to bail out as early as possible if we
can't open it.
This was found by Coverity.
Fixes: bebc03c72 ("PHP: Implement better error handling.")
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This fixes some typos and grammatical errors in the comments of
src/nxt_unit.h
Link: <https://github.com/nginx/unit/pull/889>
[ Adjust summary and write commit message as this just contains the
fixes from the PR and not actual changes - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
The isb instruction fits for spin loops where it allows to save cpu
power.
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
On GitHub, @RomainMou reported an issue whereby HTTP header field values
where being incorrectly reported as non-ascii by the Python .isacii()
method.
For example, using the following test application
def application(environ, start_response):
t = environ['HTTP_ASCIITEST']
t = "'" + t + "'" + " (" + str(len(t)) + ")"
if t.isascii():
t = t + " [ascii]"
else:
t = t + " [non-ascii]"
resp = t + "\n\n"
start_response("200 OK", [("Content-Type", "text/plain")])
return (bytes(resp, 'latin1'))
You would see the following
$ curl -H "ASCIITEST: $" http://localhost:8080/
'$' (1) [non-ascii]
'$' has an ASCII code of 0x24 (36).
The initial idea was to adjust the second parameter to the
PyUnicode_New() call from 255 to 127. This unfortunately had the
opposite effect.
$ curl -H "ASCIITEST: $" http://localhost:8080/
'$' (1) [ascii]
Good. However...
$ curl -H "ASCIITEST: £" http://localhost:8080/
'£' (2) [ascii]
Not good. Let's take a closer look at this.
'£' is not in basic ASCII, but is in extended ASCII with a value of 0xA3
(163). Its UTF-8 encoding is 0xC2 0xA3, hence the length of 2 bytes
above.
$ strace -s 256 -e sendto,recvfrom curl -H "ASCIITEST: £" http://localhost:8080/
sendto(5, "GET / HTTP/1.1\r\nHost: localhost:8080\r\nUser-Agent: curl/8.0.1\r\nAccept: */*\r\nASCIITEST: \302\243\r\n\r\n", 92, MSG_NOSIGNAL, NULL, 0) = 92
recvfrom(5, "HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\nServer: Unit/1.30.0\r\nDate: Mon, 22 May 2023 12:44:11 GMT\r\nTransfer-Encoding: chunked\r\n\r\n12\r\n'\302\243' (2) [ascii]\n\n\r\n0\r\n\r\n", 102400, 0, NULL, NULL) = 160
'£' (2) [ascii]
So we can see curl sent it UTF-8 encoded '\302\243\' which is C octal
escaped UTF-8 for 0xC2 0xA3, and we got the same back. But it should not
be marked as ASCII.
When doing PyUnicode_New(size, 127) it sets the buffer as ASCII. So we
need to use another function and that function would appear to be
PyUnicode_DecodeCharmap()
Which creates an Unicode object with the correct ascii/non-ascii
properties based on the character encoding.
With this function we now get
$ curl -H "ASCIITEST: $" http://localhost:8080/
'$' (1) [ascii]
$ curl -H "ASCIITEST: £" http://localhost:8080/
'£' (2) [non-ascii]
and for good measure
$ curl -H "ASCIITEST: $ £" http://localhost:8080/
'$ £' (4) [non-ascii]
$ curl -H "ASCIITEST: $" -H "ASCIITEST: £" http://localhost:8080/
'$, £' (5) [non-ascii]
PyUnicode_DecodeCharmap() does require having the full string upfront so
we need to build up the potentially comma separated header field values
string before invoking this function.
I did not want to touch the Python 2.7 code (which may or may not even
be affected by this) so kept these changes completely isolated from
that, hence a slight duplication with the for () loop.
Python 2.7 was sunset on January 1st 2020[0], so this code will
hopefully just disappear soon anyway.
I also purposefully didn't touch other code that may well have similar
issues (such as the HTTP header field names) if we ever get issue
reports about them, we'll deal with them then.
[0]: <https://www.python.org/doc/sunset-python-2/>
Link: <https://docs.python.org/3/c-api/unicode.html>
Closes: <https://github.com/nginx/unit/issues/868>
Reported-by: RomainMou <https://github.com/RomainMou>
Tested-by: RomainMou <https://github.com/RomainMou>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This is a preparatory patch for fixing an issue with the encoding of
http header field values.
This patch simply moves the nxt_unit_sptr_get() to the top of the
function where we will need it in the next commit.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
After the launch of the project, the testing infrastructure was shared with
nginx project in some cases. To avoid port overlap, a decision was made
to shift the port range for Unit tests. This problem was resolved a long time
ago and is no longer relevant, so it is now safe to use port 8XXX range as the
default, as it is more appropriate for testing purposes.
|
|
|
|
This variable contains a string that is formed using random data and
can be used as a unique request identifier.
This closes #714 issue on GitHub.
|
|
|
|
|
|
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
|
|
|
|
|
|
|
|
|
|
Previously, the edit method created a temporary file that was then sent
to curl(1) as --data-binary @filename.tmp. This did not work with
remote instances because the temporary file is not on the remote host.
The edit method now passes the configuration to curl(1) using stdin, the
same way as for all other configuration changes.
|
|
|
|
|
|
|
|
Introduces a new remote host scheme docker:// that specifies a local
container ID. By default, the control socket is assumed to be in the default
location, as per the Docker Official Images for Unit. If not, the path to
the control socket can be appended to the container ID.
|
|
This test reproduces https://github.com/nginx/unit/issues/964.
|
|
|
|
This was inadvertently removed in 76086d6d ("Wasm: Allow to set the HTTP
response status.")
Fixes: 76086d6d ("Wasm: Allow to set the HTTP response status.")
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
|
|
Added --format option to manage configuration in other formats.
Initially, YAML is the only supported conversion format.
JSON/YAML conversion is performed with yq(1).
Suggested by: Torstein Krause Johansen <https://github.com/skybert>
Closes: #958 <https://github.com/nginx/unit/issues/958>
|
|
|
|
|
|
On Github, @rlandgrebe reported an issue when trying to rewrite URLs
that contained query strings.
With the PHP language module we were in fact segfaulting (SIGSEGV) in
libphp
[93960.462952] unitd[20940]: segfault at 7f307cef6476 ip 00007f2f81a94577 sp 00007fff28a777d0 error 4 in libphp-8.2.so[7f2f818df000+2fd000] likely on CPU 0 (core 0, socket 0)
#0 0x00007f2abd494577 in php_default_treat_data (arg=1, str=0x0,
destArray=<optimized out>)
at /usr/src/debug/php-8.2.10-1.fc38.x86_64/main/php_variables.c:488
488 if (c_var && *c_var) {
(gdb) p c_var
$1 = 0x7f2bb8880676 <error: Cannot access memory at address 0x7f2bb8880676>
This was when trying to get the query string which somehow is pointing
off into the woods.
This gdb debug session when doing rewrite basically shows the core of
the issue
(gdb) x /64bs req->fields
...
0x7f7eaaaa8090: "GET"
0x7f7eaaaa8094: "HTTP/1.1"
0x7f7eaaaa809d: "::1"
0x7f7eaaaa80a1: "::1"
0x7f7eaaaa80a5: "8080"
0x7f7eaaaa80aa: "localhost"
0x7f7eaaaa80b4: "/test?q=a"
0x7f7eaaaa80be: "/test"
...
(gdb) p target_pos
$4 = (void *) 0x7f7eaaaa80b4
(gdb) p query_pos
$6 = (void *) 0x7f7eaaaa6af6
(gdb) p r->args->start
$8 = (u_char *) 0x7f7ea4002b02 "q=a HTTP/1.1\r\nHost: localhost:8080\r\nUser-Agent: curl/8.0.1\r\nAccept: */*\r\n\r\n"
(gdb) p r->target.start
$9 = (u_char *) 0x7f7ea40040c0 "/test?q=a"
That last address, 0x7f7ea40040c0, looks out of wack, it should be
smaller than r->args->start.
That results in a calculation in nxt_router_prepare_msg()
if (r->args->start != NULL) {
query_pos = nxt_pointer_to(target_pos,
r->args->start - r->target.start);
nxt_unit_sptr_set(&req->query, query_pos);
} else {
that goes negative that then is stored in req->query.offset which is a
uint32_t and so wraps backwards from UINT_MAX to give us an offset of a
little under 4GiB, hence the above invalid memory access.
All this happens due to in nxt_http_rewrite() if we have a URL with a
query string, we create a new memory allocation to store the transformed
URL and query string.
We set r->target to point to this new allocation, but we also need to
point r->args->start to the start of the query string in this new
allocation.
Reported-by: René Landgrebe <https://github.com/rlandgrebe>
Tested-by: René Landgrebe <https://github.com/rlandgrebe>
Tested-by: Liam Crilly <liam.crilly@nginx.com>
Fixes: 14d6d97b ("HTTP: added basic URI rewrite.")
Closes: <https://github.com/nginx/unit/issues/964>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
|
|
|
|
head -c 0 does not work on macOS (invalid byte count) but tail(1) is happy
to accept zero bytes, and does not have a performance penalty.
|
|
This test reproduce https://github.com/nginx/unit/issues/923.
|
|
The default configuration previously contained just a listeners and
applications object. Since routes is now a principle configuration object,
and a recommended way of configurating Unit, it is now included in the
default configuration.
This change benefits new users because it explicitly introduces the three
principle configuration objects which leads more intuitively to the
documentation. Experienced users may choose to ignore or delete routes.
routes is defined as an array instead of an object because this change
is designed to assist new users, where the simpler form of routes is
easier to understand.
|
|
We need to take into account the size of the nxt_unit_response_t
structure itself when calculating where to start appending data to in
memory.
Closes: <https://github.com/nginx/unit/issues/923>
Reported-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Andrew Clayton <a.clayton@nginx.org>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This closes #871 issue on GitHub.
|
|
Starting from Node.js 15.0.0 the chunk parameter of the response.write()
can be a Uint8Array.
This closes #870 issue on GitHub.
|
|
Currently Wasm modules are limited to a 32bit address space (until at
least the memory64 work is completed). All the counters etc in the
request structure were u32's. Which matched with 32bit memory
limitation.
However there is really no need to not allow >4GiB uploads that can be
saved off to disk or some such.
To do this we just need to increase the ->content_len &
->total_content_sent members to u64's.
However because we need the request structure to have the exact same
layout on 32bit (for Wasm modules) as it does on 64bit we need to re-jig
the order of some of these members and add a four-byte padding member.
Thus the request structure now looks like on 64bit (as shown by
pahole(1))
struct nxt_wasm_request_s {
uint32_t method_off; /* 0 4 */
uint32_t method_len; /* 4 4 */
uint32_t version_off; /* 8 4 */
uint32_t version_len; /* 12 4 */
uint32_t path_off; /* 16 4 */
uint32_t path_len; /* 20 4 */
uint32_t query_off; /* 24 4 */
uint32_t query_len; /* 28 4 */
uint32_t remote_off; /* 32 4 */
uint32_t remote_len; /* 36 4 */
uint32_t local_addr_off; /* 40 4 */
uint32_t local_addr_len; /* 44 4 */
uint32_t local_port_off; /* 48 4 */
uint32_t local_port_len; /* 52 4 */
uint32_t server_name_off; /* 56 4 */
uint32_t server_name_len; /* 60 4 */
/* --- cacheline 1 boundary (64 bytes) --- */
uint64_t content_len; /* 64 8 */
uint64_t total_content_sent; /* 72 8 */
uint32_t content_sent; /* 80 4 */
uint32_t content_off; /* 84 4 */
uint32_t request_size; /* 88 4 */
uint32_t nfields; /* 92 4 */
uint32_t tls; /* 96 4 */
char __pad[4]; /* 100 4 */
nxt_wasm_http_field_t fields[]; /* 104 0 */
/* size: 104, cachelines: 2, members: 25 */
/* last cacheline: 40 bytes */
};
and the same structure (taken from unit-wasm) compiled as 32bit
struct luw_req {
u32 method_off; /* 0 4 */
u32 method_len; /* 4 4 */
u32 version_off; /* 8 4 */
u32 version_len; /* 12 4 */
u32 path_off; /* 16 4 */
u32 path_len; /* 20 4 */
u32 query_off; /* 24 4 */
u32 query_len; /* 28 4 */
u32 remote_off; /* 32 4 */
u32 remote_len; /* 36 4 */
u32 local_addr_off; /* 40 4 */
u32 local_addr_len; /* 44 4 */
u32 local_port_off; /* 48 4 */
u32 local_port_len; /* 52 4 */
u32 server_name_off; /* 56 4 */
u32 server_name_len; /* 60 4 */
/* --- cacheline 1 boundary (64 bytes) --- */
u64 content_len; /* 64 8 */
u64 total_content_sent; /* 72 8 */
u32 content_sent; /* 80 4 */
u32 content_off; /* 84 4 */
u32 request_size; /* 88 4 */
u32 nr_fields; /* 92 4 */
u32 tls; /* 96 4 */
char __pad[4]; /* 100 4 */
struct luw_hdr_field fields[]; /* 104 0 */
/* size: 104, cachelines: 2, members: 25 */
/* last cacheline: 40 bytes */
};
We can see the structures have the same layout, same size and no
padding.
We need the __pad member as otherwise I saw gcc and clang on Alpine
Linux automatically add the 'packed' attribute to the structure which
made the two structures not match.
Link: <https://github.com/WebAssembly/memory64>
Link: <https://github.com/nginx/unit-wasm>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
When trying to upload files to the luw-upload-reflector demo[0] above a
certain size that would mean Unit would need to make more than two calls
to the request_handler function in the Wasm module we would get the
following error from wasmtime and the upload would stall on the third
call to the request_handler
WASMTIME ERROR: failed to call function [->wasm_request_handler]
error while executing at wasm backtrace:
0: 0x5ce2 - <unknown>!memcpy
1: 0x7df - luw_req_buf_append
at /home/andrew/src/unit-wasm/src/c/libunit-wasm.c:308:14
2: 0x3a1 - luw_request_handler
at /home/andrew/src/unit-wasm/examples/c/luw-upload-reflector.c:110:3
Caused by:
wasm trap: out of bounds memory access
This was due to ->content_off (the offset of where the actual body
content starts in the request structure/memory) being some overly large
value.
This was largely down to me being an idiot!
Before calling the loop that makes the calls to the request_handler we
would calculate the new offset, which is now just the size of the
request structure as we don't re-send all the HTTP meta data and headers
etc. However because this value is in the request structure which is in
the shared memory and we use this same memory for requests and
responses, when we make a response we overwrite this request structure
with the response structure, so our ->content_off is now some wacked out
value when we make the next call to the request_handler.
To fix this we just need to reset ->content_off each time round the
loop.
There's also no point in setting ->nfields to 0, it has the same issue
as above, but doesn't get re-used by the Wasm module anyway.
[0]: <https://github.com/nginx/unit-wasm/blob/main/examples/c/luw-upload-reflector.c>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This commit enables WebAssembly modules to set the HTTP response status
to something other than the previously hard coded '200 OK'.
To do this they can make a call to nxt_wasm_set_resp_status() providing
the required status code.
If this function isn't called the status code defaults to '200 OK'. The
WebAssembly module can also return -1 from the request_handler function
as a short cut to signal a '500 Internal Server Error'.
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
As reported by @andypost on GitHub, if you try to build Unit on a system
that uses musl libc (such as Alpine Linux) with clang then you get the
following
clang -c -pipe -fPIC -fvisibility=hidden -O -W -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -fstrict-aliasing -Wstrict-overflow=5 -Wmissing-prototypes -Werror -g -I src -I build/include \
\
\
-o build/src/nxt_socketpair.o \
-MMD -MF build/src/nxt_socketpair.dep -MT build/src/nxt_socketpair.o \
src/nxt_socketpair.c
In file included from src/nxt_socketpair.c:8:
src/nxt_socket_msg.h:138:17: error: comparison of integers of different signs: 'unsigned long' and 'long' [-Werror,-Wsign-compare]
cmsg = CMSG_NXTHDR(&msg, cmsg))
^~~~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/socket.h:358:44: note: expanded from macro 'CMSG_NXTHDR'
__CMSG_LEN(cmsg) + sizeof(struct cmsghdr) >= __MHDR_END(mhdr) - (unsigned char *)(cmsg) \
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from src/nxt_socketpair.c:8:
src/nxt_socket_msg.h:177:17: error: comparison of integers of different signs: 'unsigned long' and 'long' [-Werror,-Wsign-compare]
cmsg = CMSG_NXTHDR(&msg, cmsg))
^~~~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/socket.h:358:44: note: expanded from macro 'CMSG_NXTHDR'
__CMSG_LEN(cmsg) + sizeof(struct cmsghdr) >= __MHDR_END(mhdr) - (unsigned char *)(cmsg) \
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.
make: *** [build/Makefile:261: build/src/nxt_socketpair.o] Error 1
GCC works fine, it seems to have some smarts so that it doesn't give
warnings on system header files.
This seems to be a long standing issue with musl libc (bad casting in
the CMSG_NXTHDR macro) and the workaround employed by several projects
is to disable the -Wsign-compare clang warning for the code in question.
So, that's what we do. We wrap the CMSG_NXTHDR macro in a function, so
we can use the pre-processor in it to selectively disable the warning.
Link: <https://github.com/dotnet/runtime/issues/16438>
Link: <https://git.openembedded.org/meta-openembedded/tree/meta-oe/recipes-devtools/breakpad/breakpad/0001-Turn-off-sign-compare-for-musl-libc.patch>
Link: <https://github.com/dotnet/corefx/blob/57ff88bb75a0/src/Native/Unix/System.Native/pal_networking.c#L811-L829>
Link: <https://patchwork.yoctoproject.org/project/oe/patch/20220407191438.3696227-1-stefan@datenfreihafen.org/>
Closes: <https://github.com/nginx/unit/issues/936>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Scripted change:
$ grep -ril recevied src/ | xargs sed -i s/recevied/received/
Reported-by: <https://github.com/jeffdafoe>
Closes: <https://github.com/nginx/unit/issues/920>
Cc: <https://github.com/meezaan>
Cc: Timo Stark <t.stark@nginx.com>
Signed-off-by: Alejandro Colomar <alx@nginx.com>
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Forgotten in f717dc15b47e.
|
|
|
|
|
|
|
|
|
|
|
