unit.git/src, branch 1.30.0

NJS: supported loadable modules.

2023-05-08T08:00:25+00:00

HTTP: added basic URI rewrite.

2023-04-20T15:20:41+00:00

This commit introduced the basic URI rewrite. It allows users to change request URI. Note the "rewrite" option ignores the contained query if any and the query from the request is preserverd.
An example:
"routes": [
    {
        "match": {
            "uri": "/v1/test"
        },
        "action": {
            "return": 200
        }
    },
    {
        "action": {
            "rewrite": "/v1$uri",
            "pass": "routes"
        }
    }
]

Reviewed-by: Alejandro Colomar

Allow to remove the version string in HTTP responses.

2023-04-25T12:59:43+00:00

Normally Unit responds to HTTP requests by including a header like

  Server: Unit/1.30.0

however it can sometimes be beneficial to withhold the version
information and in this case just respond with

  Server: Unit

This patch adds a new "settings.http" boolean option called
server_version, which defaults to true, in which case the full version
information is sent. However this can be set to false, e.g

  "settings": {
      "http": {
          "server_version": false
      }
  },

in which case Unit responds without the version information as the
latter example above shows.

Link: 
Closes: 
Reviewed-by: Alejandro Colomar 
Signed-off-by: Andrew Clayton

Decouple "Unit" from NXT_SERVER.

2023-04-25T12:59:25+00:00

Split out the "Unit" name from the NXT_SERVER #define into its own
NXT_NAME #define, then make NXT_SERVER a combination of that and
NXT_VERSION.

This is required for a subsequent commit where we may want the server
name on its own.

Reviewed-by: Alejandro Colomar 
Signed-off-by: Andrew Clayton

Remove an erroneous semi-colon.

2023-04-24T18:40:16+00:00

Reviewed-by: Alejandro Colomar 
Signed-off-by: Andrew Clayton

Don't conflate the error variable in nxt_kqueue_poll().

2023-04-24T18:40:16+00:00

In nxt_kqueue_poll() error is declared as a nxt_bool_t aka unsigned int
(on x86-64 anyway).

It is used both as a boolean and as the return storage for a bitwise AND
operation.

This has potential to go awry.

If nxt_bool_t was changed to be a u8 then we would have the following
issue

gcc12 -c -pipe -fPIC -fvisibility=hidden -O -W -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wmissing-prototypes -Werror -g -O2 -I src -I build     -I/usr/local/include  -o build/src/nxt_kqueue_engine.o  -MMD -MF build/src/nxt_kqueue_engine.dep -MT build/src/nxt_kqueue_engine.o  src/nxt_kqueue_engine.c
src/nxt_kqueue_engine.c: In function 'nxt_kqueue_poll':
src/nxt_kqueue_engine.c:728:17: error: overflow in conversion from 'int' to 'nxt_bool_t' {aka 'unsigned char'} changes value from '(int)kev->flags & 16384' to '0' [-Werror=overflow]
  728 |         error = (kev->flags & EV_ERROR);
      |                 ^
cc1: all warnings being treated as errors

EV_ERROR has the value 16384, after the AND operation error holds 16384,
however this overflows and wraps around (64 times) exactly to 0.

With nxt_bool_t defined as a u32, we would have a similar issue if
EV_ERROR ever became UINT_MAX + 1 (or a multiple thereof)...

Rather than conflating the use of error, keep error as a boolean (it is
used further down the function) but do the AND operation inside the
if ().

Reviewed-by: Alejandro Colomar 
Signed-off-by: Andrew Clayton

Remove a bunch of dead code.

2023-04-24T18:39:09+00:00

This removes a bunch of unused files that would have been touched by
subsequent commits that switch to using nxt_bool_t (AKA unit6_t) in
structures.

In auto/sources we have

  NXT_LIB_SRC0=" \
      src/nxt_buf_filter.c \
      src/nxt_job_file.c \
      src/nxt_stream_module.c \
      src/nxt_stream_source.c \
      src/nxt_upstream_source.c \
      src/nxt_http_source.c \
      src/nxt_fastcgi_source.c \
      src/nxt_fastcgi_record_parse.c \
  \
      src/nxt_mem_pool_cleanup.h \
      src/nxt_mem_pool_cleanup.c \
  "

None of these seem to actually be used anywhere (other than within
themselves). That variable is _not_ referenced anywhere else.

Also remove the unused related header files: src/nxt_buf_filter.h,
src/nxt_fastcgi_source.h, src/nxt_http_source.h, src/nxt_job_file.h,
src/nxt_stream_source.h and src/nxt_upstream_source.h

Also, these files do not seem to be used, no mention under auto/ or build/

  src/nxt_file_cache.c
  src/nxt_cache.c
  src/nxt_job_file_cache.c

src/nxt_cache.h is #included in src/nxt_main.h, but AFAICT is not
actually used.

With all the above removed

  $ ./configure --openssl --debug --tests && make -j && make -j tests &&
  make libnxt

all builds.

Buildbot passes.

NOTE: You may need to do a 'make clean' before the next build attempt.

Reviewed-by: Alejandro Colomar 
Signed-off-by: Andrew Clayton

HTTP: optimizing $request_line.

2023-04-12T09:50:56+00:00

Don't reconstruct a new string for the $request_line from the parsed
method, target, and HTTP version, but rather keep a pointer to the
original memory where the request line was received.

This will be necessary for implementing URI rewrites, since we want to
log the original request line, and not one constructed from the
rewritten target.

This implementation changes behavior (only for invalid requests) in the
following way:

Previous behavior was to log as many tokens from the request line as
were parsed validly, thus:

Request              -> access log              ; error log

"GET / HTTP/1.1"     -> "GET / HTTP/1.1"     OK ; =
"GET   / HTTP/1.1"   -> "GET / HTTP/1.1"    [1] ; =
"GET / HTTP/2.1"     -> "GET / HTTP/2.1"     OK ; =
"GET / HTTP/1."      -> "GET / HTTP/1."     [2] ; "GET / HTTP/1. [null]"
"GET / food"         -> "GET / food"        [2] ; "GET / food [null]"
"GET / / HTTP/1.1"   -> "GET / / HTTP/1.1"  [2] ; =
"GET /  / HTTP/1.1"  -> "GET /  / HTTP/1.1" [2] ; =
"GET food HTTP/1.1"  -> "GET"                   ; "GET [null] [null]"
"OPTIONS * HTTP/1.1" -> "OPTIONS"           [3] ; "OPTIONS [null] [null]"
"FOOBAR baz HTTP/1.1"-> "FOOBAR"                ; "FOOBAR [null] [null]"
"FOOBAR / HTTP/1.1"  -> "FOOBAR / HTTP/1.1"     ; =
"get / HTTP/1.1"     -> "-"                     ; " [null] [null]"
""                   -> "-"                     ; " [null] [null]"

This behavior was rather inconsistent.  We have several options to go
forward with this patch:

-  NGINX behavior.

   Log the entire request line, up to '\r' | '\n', even if it was
   invalid.

   This is the most informative alternative.  However, RFC-complying
   requests will probably not send invalid requests.

   This information would be interesting to users where debugging
   requests constructed manually via netcat(1) or a similar tool, or
   maybe for debugging a client, are important.  It might be interesting
   to support this in the future if our users are interested; for now,
   since this approach requires looping over invalid requests twice,
   that's an overhead that we better avoid.

-  Previous Unit behavior

   This is relatively fast (almost as fast as the next alternative, the
   one we chose), but the implementation is ugly, in that we need to
   perform the same operation in many places around the code.

   If we want performance, probably the next alternative is better; if
   we want to be informative, then the first one is better (maybe in
   combination with the third one too).

-  Chosen behavior

   Only logging request lines when the request is valid.  For any
   invalid request, or even unsupported ones, the request line will be
   logged as "-".  Thus:

   Request              -> access log [4]

   "GET / HTTP/1.1"     -> "GET / HTTP/1.1"     OK
   "GET   / HTTP/1.1"   -> "GET   / HTTP/1.1"  [1]
   "GET / HTTP/2.1"     -> "-"                 [3]
   "GET / HTTP/1."      -> "-"
   "GET / food"         -> "-"
   "GET / / HTTP/1.1"   -> "GET / / HTTP/1.1"  [2]
   "GET /  / HTTP/1.1"  -> "GET /  / HTTP/1.1" [2]
   "GET food HTTP/1.1"  -> "-"
   "OPTIONS * HTTP/1.1" -> "-"
   "FOOBAR baz HTTP/1.1"-> "-"
   "FOOBAR / HTTP/1.1"  -> "FOOBAR / HTTP/1.1"
   "get / HTTP/1.1"     -> "-"
   ""                   -> "-"

   This is less informative than previous behavior, but considering how
   inconsistent it was, and that RFC-complying agents will probably not
   send us such requests, we're ready to lose that information in the
   log.  This is of course the fastest and simplest implementation we
   can get.

   We've chosen to implement this alternative in this patch.  Since we
   modified the behavior, this patch also changes the affected tests.

[1]:  Multiple successive spaces as a token delimiter is allowed by the
      RFC, but it is discouraged, and considered a security risk.  It is
      currently supported by Unit, but we will probably drop support for
      it in the future.

[2]:  Unit currently supports spaces in the request-target.  This is
      a violation of the relevant RFC (linked below), and will be fixed
      in the future, and consider those targets as invalid, returning
      a 400 (Bad Request), and thus the log lines with the previous
      inconsistent behavior would be changed.

[3]:  Not yet supported.

[4]:  In the error log, regarding the "log_routes" conditional logging
      of the request line, we only need to log the request line if it
      was valid.  It doesn't make sense to log "" or "-" in case that
      the request was invalid, since this is only useful for
      understanding decisions of the router.  In this case, the access
      log is more appropriate, which shows that the request was invalid,
      and a 400 was returned.  When the request line is valid, it is
      printed in the error log exactly as in the access log.

Link: 
Suggested-by: Liam Crilly 
Reviewed-by: Zhidao Hong 
Cc: Timo Stark 
Cc: Andrei Zeliankou 
Cc: Andrew Clayton 
Cc: Artem Konev 
Signed-off-by: Alejandro Colomar

Add per-application logging.

2023-04-11T18:08:32+00:00

Currently when running in the foreground, unit application processes
will send stdout to the current TTY and stderr to the unit log file.

That behaviour won't change.

When running as a daemon, unit application processes will send stdout to
/dev/null and stderr to the unit log file.

This commit allows to alter the latter case of unit running as a daemon,
by allowing applications to redirect stdout and/or stderr to specific
log files. This is done via two new application options, 'stdout' &
'stderr', e.g

  "applications": {
      "myapp": {
          ...
          "stdout": "/path/to/log/unit/app/stdout.log",
          "stderr": "/path/to/log/unit/app/stderr.log"
      }
  }

These log files are created by the application processes themselves and
thus the log directories need to be writable by the user (and or group)
of the application processes.

E.g

  $ sudo mkdir -p /path/to/log/unit/app
  $ sudo chown APP_USER /path/to/log/unit/app

These need to be setup before starting unit with the above config.

Currently these log files do not participate in log-file rotation
(SIGUSR1), that may change in a future commit. In the meantime these
logs can be rotated using the traditional copy/truncate method.

NOTE:

You may or may not see stuff printed to stdout as stdout was
traditionally used by CGI applications to communicate with the
webserver.

Closes: 
Closes: 
Reviewed-by: Alejandro Colomar 
Signed-off-by: Andrew Clayton

Add nxt_file_stdout().

2023-04-11T18:08:12+00:00

This is analogous to the nxt_file_stderr() function and will be used in
a subsequent commit.

This function redirects stdout to a given file descriptor.

Reviewed-by: Alejandro Colomar 
Signed-off-by: Andrew Clayton