Fix up a mixture of different names/email addresses people have used.
You can always see the original names/addresses used by passing
--no-mailmap to the various git commands.
See gitmailmap(5)
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
Map her GitHub noreply address to her @f5 one.
You can always see the original address used by passing --no-mailmap to
the various git commands.
Note: We don't always need the name field, but we're keeping this file
consistent and alphabetically ordered on first name...
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
A RHEL 8 test was failing because it uses go1.16. The old style must
be retained for backwards compat.
Fixes: 9a36de84c ("Go: Use Homebrew include paths")
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Reviewed-by: Dylan Arbour <d.arbour@f5.com>
Signed-off-by: Danielle De Leo <d.deleo@f5.com>
This is to improve error messages for rewrite configuration.
Take the configuration as an example:
{
"rewrite": "`${a + "
}
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: "SyntaxError: Unexpected end of input in default:1" in the "rewrite" value.
Coverity picked up a potential issue with the previous commit d9f5f1fb7
("Ruby: Handle response field arrays") in that a size_t could wrap
around to SIZE_MAX - 1.
This would happen if we were given an empty array of header values.
Fixes: d9f5f1fb7 ("Ruby: Handle response field arrays")
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
@xeron on GitHub reported an issue whereby with a Rails 7.1 application
they were getting the following error
2023/10/22 20:57:28 [error] 56#56 [unit] #8: Ruby: Wrong header entry 'value' from application
2023/10/22 20:57:28 [error] 56#56 [unit] #8: Ruby: Failed to run ruby script
After some back and forth debugging it turns out rack was trying to send
back a header comprised of an array of values. E.g
app = Proc.new do |env|
["200", {
"Content-Type" => "text/plain",
"X-Array-Header" => ["Item-1", "Item-2"],
}, ["Hello World\n"]]
end
run app
It seems this became a possibility in rack v3.0[0]
So along with a header value type of T_STRING we need to also allow
T_ARRAY.
If we get a T_ARRAY we need to build up the header field using the given
values.
E.g
"X-Array-Header" => ["Item-1", "", "Item-3", "Item-4"],
becomes
X-Array-Header: Item-1; ; Item-3; Item-4
[0]: <https://github.com/rack/rack/blob/main/UPGRADE-GUIDE.md?plain=1#L26>
Reported-by: Ivan Larionov <xeron.oskom@gmail.com>
Closes: <https://github.com/nginx/unit/issues/974>
Link: <https://github.com/nginx/unit/pull/998>
Tested-by: Timo Stark <t.stark@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
Due to GH making a mess of merge commits, it used Danielle's personal
email address for the merge, it also used a generic GH address for the
committer but we can't do anything about that. However we can fix the
'Author' email address.
If for some reason you want to see the original names/addresses used you
can generally pass --no-mailmap to git commands.
Signed-off-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.
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 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>
