This feature allows users to specify conditions to control if access log
should be recorded. The "if" option supports a string and JavaScript code.
If its value is empty, 0, false, null, or undefined, the logs will not be
recorded. And the '!' as a prefix inverses the condition.
Example 1: Only log requests that sent a session cookie.
{
"access_log": {
"if": "$cookie_session",
"path": "..."
}
}
Example 2: Do not log health check requests.
{
"access_log": {
"if": "`${uri == '/health' ? false : true}`",
"path": "..."
}
}
Example 3: Only log requests when the time is before 22:00.
{
"access_log": {
"if": "`${new Date().getHours() < 22}`",
"path": "..."
}
}
or
{
"access_log": {
"if": "!`${new Date().getHours() >= 22}`",
"path": "..."
}
}
Closes: https://github.com/nginx/unit/issues/594
I hate having to type so much just for the useful help.
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
I've been using them for a long time, and they are quite useful and
stable. Let's say they're advanced instead of experimental.
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
On GH, @tonychuuy reported an issue when using Units 'share' action they
would get the following error in the unit log
2024/01/15 17:53:41 [error] 49#52 *103 file "/var/www/html/public/vendor/telescope/app.css" has changed while sending response to a client
This would happen when trying to serve files over a certain size and the
requested file would not be sent.
This is due to a somewhat bogus check in
nxt_http_static_buf_completion()
I say bogus because it's not clear what the check is trying to
accomplish and the error message is not entirely accurate either.
The check in question goes like
n = pread(file->fd, buf, size, offset);
return n;
...
if (n != size) {
if (n >= 0) {
/* log file changed error and finish */
/* >> Problem is here << */
}
/* log general error and finish */
}
If the number of bytes read is not what we asked for and is > -1 (i.e
not an error) then it says the file has changed, but really it only
checks if the file has _shrunk_ (we can't get back _more_ bytes than we
asked for) since it was stat'd.
This is what happens
recvfrom(22, "GET /tfile HTTP/1.1\r\nHost: local"..., 2048, 0, NULL, NULL) = 82
openat(AT_FDCWD, "/mnt/9p/tfile", O_RDONLY|O_NONBLOCK) = 23
newfstatat(23, "", {st_mode=S_IFREG|0644, st_size=149922, ...}, AT_EMPTY_PATH) = 0
We get a request from a client, open the requested file and stat(2) it to
get the file size.
We would then go into a pread/writev loop reading the file data and
sending it to the client until it's all been sent.
However what was happening in this case was this (showing a dummy file
of 149922 bytes)
pread64(23, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 131072, 0) = 61440
write(2, "2024/01/17 15:30:50 [error] 1849"..., 109) = 109
We wanted to read 131072 bytes but only read 61440 bytes, the above
check triggered and the file transfer was aborted and the above error
message logged.
Normally for a regular file you will only get less bytes than asked for
if the read call is interrupted by a signal or you're near the end of
file.
There is however at least another situation where this may happen, if
the file in question is being served from a network filesystem.
It turns out that was indeed the case here, the files where being served
over the 9P filesystem protocol. Unit was running in a docker container
in an Ubuntu VM under Windows/WSL2 and the files where being passed
through to the VM from Windows over 9P.
Whatever the intention of this check, it is clearly causing issues in
real world scenarios.
If it was really desired to check if the had changed since it was
opened/stat'd then it would require a different methodology and be a
patch for another day. But as it stands this current check does more
harm than good, so lets just remove it.
With it removed we now get for the above test file
recvfrom(22, "GET /tfile HTTP/1.1\r\nHost: local"..., 2048, 0, NULL, NULL) = 82
openat(AT_FDCWD, "/mnt/9p/tfile", O_RDONLY|O_NONBLOCK) = 23
newfstatat(23, "", {st_mode=S_IFREG|0644, st_size=149922, ...}, AT_EMPTY_PATH) = 0
mmap(NULL, 135168, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f367817b000
pread64(23, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 131072, 0) = 61440
pread64(23, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 18850, 61440) = 18850
writev(22, [{iov_base="HTTP/1.1 200 OK\r\nLast-Modified: "..., iov_len=171}, {iov_base="\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., iov_len=61440}, {iov_base="\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., iov_len=18850}], 3) = 80461
pread64(23, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 69632, 80290) = 61440
pread64(23, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 8192, 141730) = 8192
close(23) = 0
writev(22, [{iov_base="\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., iov_len=61440}, {iov_base="\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., iov_len=8192}], 2) = 69632
So we can see we do two pread(2)s's and a writev(2), then another two
pread(2)s and another writev(2) and all the file data has been read and
sent to the client.
Reported-by: tonychuuy <https://github.com/tonychuuy>
Link: <https://en.wikipedia.org/wiki/9P_(protocol)>
Fixes: 08a8d1510 ("Basic support for serving static files.")
Closes: https://github.com/nginx/unit/issues/1064
Reviewed-by: Zhidao Hong <z.hong@f5.com>
Reviewed-by: Andrei Zeliankou <zelenkov@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
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.
