44 Commits
Author SHA1 Message Date
Alejandro ColomarandAlejandro Colomar 5a37171f73 Added default values for pathnames.
This allows one to simply run `./configure` and expect it to
produce sane defaults for an install.

Previously, without specifying `--prefix=...`, `make install`
would simply fail, recommending to set `--prefix` or `DESTDIR`,
but that recommendation was incomplete at best, since it didn't
set many of the subdirs needed for a good organization.

Setting `DESTDIR` was even worse, since that shouldn't even affect
an installation (it is required to be transparent to the
installation).

/usr/local is the historic Unix standard path to use for
installations from source made manually by the admin of the
system.  Some package managers (Homebrew, I'm looking specifically
at you) have abused that path to install their things, but 1) it's
not our fault that someone else incorrectly abuses that path (and
they seem to be fixing it for newer archs; e.g., they started
using /opt/homebrew for Apple Silicon), 2) there's no better path
than /usr/local, 3) we still allow changing it for systems where
this might not be the desired path (MacOS Intel with hombrew), and
4) it's _the standard_.

See a related conversation with Ingo (OpenBSD maintainer):

On 7/27/22 16:16, Ingo Schwarze wrote:
> Hi Alejandro,
[...]
>
> Alejandro Colomar wrote on Sun, Jul 24, 2022 at 07:07:18PM +0200:
>> On 7/24/22 16:57, Ingo Schwarze wrote:
>>> Alejandro Colomar wrote on Sun, Jul 24, 2022 at 01:20:46PM +0200:
>
>>>> /usr/local is for sysadmins to build from source;
>
>>> Doing that is *very* strongly discouraged on OpenBSD.
>
>> I guess that's why the directory was reused in the BSDs to install ports
>> (probably ports were installed by the sysadmin there, and by extension,
>> ports are now always installed there, but that's just a guess).
>
> Maybe.  In any case, the practice of using /usr/local for packages
> created from ports is significantly older than the recommendation
> to refrain from using upstream "make install" outside the ports
> framework.
>
>   * The FreeBSD ports framework was started by Jordan Hubbard in 1993.
>   * The ports framework was ported from FreeBSD to OpenBSD
>     by Niklas Hallqvist in 1996.
>   * NetBSD pkgsrc was forked from FreeBSD ports by Alistair G. Crooks
>     and Hubert Feyrer in 1997.
>
> I failed to quickly find Jordan's original version, but rev. 1.1
> of /usr/ports/infrastructure/mk/bsd.port.mk in OpenBSD (dated Jun 3
> 22:47:10 1996 UTC) already said
>
>    LOCALBASE ?= /usr/local
>    PREFIX    ?= ${LOCALBASE}
>
[...]
>> I had a discussion in NGINX Unit about it, and
>> the decission for now has been: "support prefix=/usr/local for default
>> manual installation through the Makefile, and let BSD users adjust to
>> their preferred path".
>
> That's an *excellent* solution for the task, thanks for doing it
> the right way.  By setting PREFIX=/usr/local by default in the
> upstream Makefile, you are minimizing the work for *BSD porters.
>
> The BSD ports frameworks will typically run the upstreak "make install"
> with the variable DESTDIR set to a custom value, for example
>
>    DESTDIR=/usr/ports/pobj/groff-1.23.0/fake-amd64
>
> so if the upstream Makefile sets PREFIX=/usr/local ,
> that's perfect, everything gets installed to the right place
> without an intervention by the person doing the porting.
>
> Of course, if the upstream Makefile would use some other PREFIX,
> that would not be a huge obstacle.  All we have to do in that case
> is pass the option --prefix=/usr/local to the ./configure script,
> or something equivalent if the software isn't using GNU configure.
>
>> We were concerned that we might get collisions
>> with the BSD port also installing in /usr/local, but that's the least
>> evil (and considering BSD users don't typically run `make install`, it's
>> not so bad).
>
> It's not bad at all.  It's perfect.
>
> Of course, if a user wants to install *without* the ports framework,
> they have to provide their own --prefix.  But that's not an issue
> because it is easy to do, and installing without a port is discouraged
> anyway.

===

Directory variables should never contain a trailing slash (I've
learned that the hard way, where some things would break
unexpectedly).  Especially, make(1) is likely to have problems
when things have double slashes or a trailing slash, since it
treats filenames as text strings.  I've removed the trailing slash
from the prefix, and added it to the derivate variables just after
the prefix.  pkg-config(1) also expects directory variables to have
no trailing slash.

===

I also removed the code that would set variables as depending on
the prefix if they didn't start with a slash, because that is a
rather non-obvious behavior, and things should not always depend
on prefix, but other dirs such as $(runstatedir), so if we keep
a similar behavior it would be very unreliable.  Better keep
variables intact if set, or use the default if unset.

===

Print the real defaults for ./configure --help, rather than the actual
values.

===

I used a subdirectory under the standard /var/lib for NXT_STATE,
instead of a homemade "state" dir that does the same thing.

===

Modified the Makefile to create some dirs that weren't being
created, and also remove those that weren't being removed in
uninstall, probably because someone forgot to add them.

===

Add new options for setting the new variables, and rename some to be
consistent with the standard names.  Keep the old ones at configuration
time for compatibility, but mark them as deprecated.  Don't keep the old
ones at exec time.

===

A summary of the default config is:

Unit configuration summary:

  bin directory: ............. "/usr/local/bin"
  sbin directory: ............ "/usr/local/sbin"
  lib directory: ............. "/usr/local/lib"
  include directory: ......... "/usr/local/include"
  man pages directory: ....... "/usr/local/share/man"
  modules directory: ......... "/usr/local/lib/unit/modules"
  state directory: ........... "/usr/local/var/lib/unit"
  tmp directory: ............. "/tmp"

  pid file: .................. "/usr/local/var/run/unit/unit.pid"
  log file: .................. "/usr/local/var/log/unit/unit.log"

  control API socket: ........ "unix:/usr/local/var/run/unit/control.unit.sock"

Link: <https://www.gnu.org/prep/standards/html_node/Directory-Variables.html>
Link: <https://refspecs.linuxfoundation.org/FHS_3.0/fhs/index.html>
Reviewed-by: Artem Konev <a.konev@f5.com>
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Tested-by: Andrew Clayton <a.clayton@nginx.com>
Reviewed-by: Konstantin Pavlov <thresh@nginx.com>
Signed-off-by: Alejandro Colomar <alx@nginx.com>
2023-01-31 23:47:53 +01:00
Alejandro ColomarandAndrew Clayton 0686740f20 PHP: Factored out code into a helper function.
We're going to use zend_stream_init_filename in a following commit.  To
reduce the diff of that change, move the current code that will be
replaced, to a function that has the same interface.

We use strlen(3) here to be able to use an interface without passing the
length, but we will remove that call in a following code, so it has no
performance issues.

Co-developed-by: Andrew Clayton <a.clayton@nginx.com>
Signed-off-by: Alejandro Colomar <alx@nginx.com>
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Cc: Andrei Zeliankou <zelenkov@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
2023-01-27 14:45:52 +00:00
Alejandro Colomar 7e4a8a5422 Disallowed abstract unix socket syntax in non-Linux systems.
The previous commit added/fixed support for abstract Unix domain sockets
on Linux with a leading '@' or '\0'.  To be consistent in all platforms,
treat those prefixes as markers for abstract sockets in all platforms,
and fail if abstract sockets are not supported by the platform.

That will avoid mistakes when copying a config file from a Linux system
and using it in non-Linux, which would surprisingly create a normal socket.
2022-08-18 18:58:41 +02:00
Alejandro Colomar e2aec6686a Storing abstract sockets with @ internally.
We accept both "\u0000socket-name" and "@socket-name" as abstract
unix sockets.  The first one is passed to the kernel pristine,
while the second is transformed '@'->'\0'.

The commit that added support for unix sockets accepts both
variants, but we internally stored it in the same way, using
"\u0000..." for both.

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 '@' for a string that will
be used internally (not user-visible).

This commit (indirectly) fixes a small bug, where we created
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.
2022-08-18 18:56:24 +02:00
Alejandro Colomar d8e0768a5b Fixed support for abstract Unix sockets.
Unix domain sockets are normally backed by files in the
filesystem.  This has historically been problematic when closing
and opening again such sockets, since SO_REUSEADDR is ignored for
Unix sockets (POSIX left the behavior of SO_REUSEADDR as
implementation-defined, and most --if not all-- implementations
decided to just ignore this flag).

Many solutions are available for this problem, but all of them
have important caveats:

- unlink(2) the file when it's not needed anymore.

  This is not easy, because the process that controls the fd may
  not be the same process that created the file, and may not have
  file permissions to remove it.

  Further solutions can be applied to that caveat:

  - unlink(2) the file right after creation.

    This will remove the pathname from the filesystem without
    closing the socket (it will continue to live until the last fd
    is closed).  This is not useful for us, since we need the
    pathname of the socket as its interface.

  - chown(2) or chmod(2) the directory that contains the socket.

    For removing a file from the filesystem, a process needs
    write permissions in the containing directory.  We could
    put sockets in dummy directories that can be chown(2)ed to
    nobody.  This could be dangerous, though, as we don't control
    the socket names.  It is our users who configure the socket
    name in their configuration, and so it's easy that they don't
    understand the many implications of not chosing an appropriate
    socket pathname.  A user could unknowingly put the socket in a
    directory that is not supposed to be owned by user nobody, and
    if we blindly chown(2) or chmod(2) the directory, we could be
    creating a big security hole.

  - Ask the main process to remove the socket.

    This would require a very complex communication mechanism with
    the main process, which is not impossible, but let's avoid it
    if there are simpler solutions.

  - Give the child process the CAP_DAC_OVERRIDE capability.

    That is one of the most powerful capabilities.  A process with
    that capability can be considered root for most practical
    aspects.  Even if the capability is disabled for most of the
    lifetime of the process, there's a slight chance that a
    malicious actor could activate it and then easily do serious
    damage to the system.

- unlink(2) the file right before calling bind(2).

  This is dangerous because another process (for example, another
  running instance of unitd(8)), could be using the socket, and
  removing the pathname from the filesystem would be problematic.
  To do this correctly, a lot of checks should be added before the
  actual unlink(2), which is error-prone, and difficult to do
  correctly, and atomically.

- Use abstract-namespace Unix domain sockets.

  This is the simplest solution, as it only requires accepting a
  slightly different syntax (basically a @ prefix) for the socket
  name, to transform it into a string starting with a null byte
  ('\0') that the kernel can understand.  The patch is minimal.

  Since abstract sockets live in an abstract namespace, they don't
  create files in the filesystem, so there's no need to remove
  them later.  The kernel removes the name when the last fd to it
  has been closed.

  One caveat is that only Linux currently supports this kind of
  Unix sockets.  Of course, a solution to that could be to ask
  other kernels to implement such a feature.

  Another caveat is that filesystem permissions can't be used to
  control access to the socket file (since, of course, there's no
  file).  Anyone knowing the socket name can access to it.  The
  only method to control access to it is by using
  network_namespaces(7).  Since in unitd(8) we're using 0666 file
  sockets, abstract sockets should be no more insecure than that
  (anyone can already read/write to the listener sockets).

- Ask the kernel to implement a simpler way to unlink(2) socket
  files when they are not needed anymore.  I've suggested that to
  the <linux-fsdevel@vger.kernel.org> mailing list, in:
<lore.kernel.org/linux-fsdevel/0bc5f919-bcfd-8fd0-a16b-9f060088158a@gmail.com/T>

In this commit, I decided to go for the easiest/simplest solution,
which is abstract sockets.  In fact, we already had partial
support.  This commit only fixes some small bug in the existing
code so that abstract Unix sockets work:

- Don't chmod(2) the socket if it's an abstract one.

This fixes the creation of abstract sockets, but doesn't make them
usable, since we produce them with a trailing '\0' in their name.
That will be fixed in the following commit.

This closes #669 issue on GitHub.
2022-08-18 18:55:14 +02:00
Alejandro Colomar 1c2f070ee2 Fixed include guard.
For consistency, use the same pattern as in the rest of the project.
2022-08-18 15:43:03 +02:00
Alejandro Colomar 22c5100666 Removed dead code.
nxt_sockaddr_ntop() stopped being used in commit (git) 029942f4eb.
It has been replaced mostly by nxt_sockaddr_text().

    commit 029942f4eb
    Author: Igor Sysoev <igor@sysoev.ru>
    Date:   Wed Feb 22 15:09:59 2017 +0300

        I/O operations refactoring.

nxt_job_sockaddr_parse() stopped being used in commit (git) 794248090a.

    commit 794248090a
    Author: Igor Sysoev <igor@sysoev.ru>
    Date:   Wed Mar 4 14:04:08 2020 +0300

        Legacy upstream code removed.

Also, remove functions and types used only by those two functions:

nxt_job_sockaddr_unix_parse()
nxt_job_sockaddr_inet6_parse()
nxt_job_sockaddr_inet_parse()
nxt_job_sockaddr_parse_t
nxt_job_resolve()
nxt_job_resolve_t
2022-08-11 18:43:12 +02:00
Alejandro Colomar d37b76232e Put changes entry in the correct position. 2022-08-08 12:13:28 +02:00
Alejandro Colomar c7543311ca Python: supporting UNIX sockets.
This closes #635 issue on GitHub.
2022-08-08 12:11:28 +02:00
Alejandro Colomar 418bc208d0 Rejecting non-Linux pivot_root(2).
Some non-Linux systems implement pivot_root(2), even if they
don't document that.  An example is MacOS:

$ grepc pivot_root / 2>/dev/null
.../sys/sysproto.h:3012:
int pivot_root(struct proc *, struct pivot_root_args *, int *);

Since the prototype of the syscall differs from that of Linux, we
can't use that syscall.  Let's make sure the test only detects
pivot_root(2) under Linux.  Also, rename the feature macro to make
clear that it's only about Linux's pivot_root(2).

This closes #737 issue on GitHub.
2022-08-02 19:50:10 +02:00
Alejandro Colomar 2c0888f69c Including <mntent.h> iff it exists.
With NXT_HAVE_PIVOT_ROOT, we had issues in MacOS.  Headers should
normally be included unconditionally, except of course if they
don't exist.

This fixes part of the #737 issue on GitHub.
2022-08-02 13:58:01 +02:00
Alejandro Colomar 9b4b4925b3 Ruby: fixed contents of SCRIPT_NAME.
Having the basename of the script pathname was incorrect.  While
we don't have something more accurate, the best thing to do is to
have it empty (which should be the right thing most of the time).

This closes #715 issue on GitHub.

The bug was introduced in git commit
0032543fa6
'Ruby: added the Rack environment parameter "SCRIPT_NAME".'.
2022-07-27 12:46:42 +02:00
Alejandro Colomar 91ffd08d11 Fixed line removed by accident.
When fixing conflicts in the changelog, a line was removed by accident.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2022-07-26 16:58:15 +02:00
Alejandro Colomar 6e36584a2e Supporting UNIX sockets in address matching.
This closes #645 issue on GitHub.

(Also moved a changelog line that was misplaced in a previous commit.)
2022-07-26 16:24:33 +02:00
Alejandro Colomar 3422e81ab6 Added missing inline keyword. 2022-07-18 19:09:30 +02:00
Alejandro Colomar 14857442c6 Added missing inline keyword. 2022-07-18 19:09:30 +02:00
Alejandro Colomar 6ba39ccf9b Fixed incorrect code.
The #endif was misplaced by accident during a refactor:
<https://github.com/nginx/unit/commit/029942f4eb7196c2cff0d0e26bc6ff274138f7d8>.

clang(1)'s -Wunreachable-code-break (implied by -Weverything) catches
that, but it is only produced for code compiled without support
for Unix sockets, which is probably the reason it was undetected:
no-one seems to be compiling Unit without Unix sockets support (at
least with clang(1)).
2022-07-18 19:09:30 +02:00
Alejandro Colomar 5015b05fc4 Replaced Linux syscall macros by libc macros.
User-space programs should use the SYS_*form, as documented in
syscall(2).  That also adds compatibility to non-Linux systems.
2022-07-18 19:09:30 +02:00
Alejandro Colomar 0d15cbd5b6 Removed unnecessary include.
Some OSes, as Linux, provide FIONBIO in <sys/ioctl.h>.  Others,
such as the BSDs and Illumos, provide it in <sys/filio.h>, but
they all include that header from <sys/ioctl.h>, so for this test,
we can simplify and just include <sys/ioctl.h>.
2022-07-18 19:09:30 +02:00
Alejandro Colomar c8d9106a0d Removed code used when NXT_HAVE_POSIX_SPAWN is false.
posix_spawn(3POSIX) was introduced by POSIX.1d
(IEEE Std 1003.1d-1999), and was later consolidated in
POSIX.1-2001, requiring it in all POSIX-compliant systems.
It's safe to assume it's always available, more than 20 years
after its standardization.

Link: <https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/spawn.h.html>
2022-07-18 19:09:30 +02:00
Alejandro Colomar 26a5af8591 Removed duplicate handling of './configure --help'.
That is already handled in auto/options.  It is better suited
there, since it's an option, and not a module, and also because it
makes use of variables declared there.
2022-07-18 18:33:59 +02:00
Alejandro Colomar c3e40ae932 Static: Fixed finding the file extension.
The code for finding the extension made a few assumptions that are
no longer true.  It didn't account for pathnames that didn't
contain '/', including the empty string, or the NULL string.  That
code was used with "share", which always had a '/', but now it's
also used with "index", which should not have a '/' in it.

This fix works by limiting the search to the beginning of the
string, so that if no '/' is found in it, it doesn't continue
searching before the beginning of the string.

This also happens to work for NULL.  It is technically Undefined
Behavior, as we rely on `NULL + 0 == NULL` and `NULL - NULL == 0`.
But that is the only sane behavior for an implementation, and all
existing POSIX implementations will Just Work for this code.

Relying on this UB is useful, because we don't need to add an
explicit check for NULL, and therefore we have faster code.
Although the current code can't have a NULL, I expect that when we
add support for variables in the index, it will be NULL in some
cases.

Link: <https://stackoverflow.com/q/67291052/6872717>

The same code seems to be defined behavior in C++, which normally
will share implementation in the compiler for these cases, and
therefore it is really unlikely to be in trouble.

Link: <https://stackoverflow.com/q/59409034/6872717>
2022-06-21 12:47:01 +02:00
Alejandro Colomar aee8b9bfb2 Tests: Added tests for $request_uri. 2022-05-31 12:41:11 +02:00
Alejandro Colomar 9bf614cd08 Var: Added $request_uri (as in NGINX).
This supports a new variable $request_uri that contains the path
and the query (See RFC 3986, section 3).  Its contents are percent
encoded.  This is useful for example to redirect HTTP to HTTPS:

{
    "return": "301",
    "location": "https://$host$request_uri"
}

When <http://example.com/foo%23bar?baz> is requested, the server
redirects to <https://example.com/foo%23bar?baz>.

===

Testing:

//diff --git a/src/nxt_http_return.c b/src/nxt_http_return.c
//index 82c9156..adeb3a1 100644
//--- a/src/nxt_http_return.c
//+++ b/src/nxt_http_return.c
//@@ -196,6 +196,7 @@ nxt_http_return_send_ready(nxt_task_t *task,
    void *obj, void *data)
//         field->value = ctx->encoded.start;
//         field->value_length = ctx->encoded.length;
//     }
//+    fprintf(stderr, "ALX: target[%1$i]: <%2$.*1$s>\n",
    (int)r->target.length, r->target.start);
//
//     r->state = &nxt_http_return_send_state;
//

{
	"listeners": {
		"*:81": {
			"pass": "routes/ru"
		}
	},

	"routes": {
		"ru": [{
			"action": {
				"return": 301,
				"location": "$request_uri"
			}
		}]
	}
}

$ curl -i http://localhost:81/*foo%2Abar?baz#arg
HTTP/1.1 301 Moved Permanently
Location: /*foo%2Abar?baz
Server: Unit/1.27.0
Date: Mon, 30 May 2022 16:04:30 GMT
Content-Length: 0

$ sudo cat /usr/local/unit.log | grep ALX
ALX: target[15]: </*foo%2Abar?baz>
2022-05-31 12:40:02 +02:00
Alejandro Colomar 8027e7ce0f Tests: added tests for "index" (string) option. 2022-05-30 12:42:18 +02:00
Alejandro Colomar 9af5f36951 Static: supporting new "index" option.
This supports a new option "index" that configures a custom index
file name to be served when a directory is requested.  This
initial support only allows a single fixed string.  An example:

{
	"share": "/www/data/static/$uri",
	"index": "lookatthis.htm"
}

When <example.com/foo/bar/> is requested,
</www/data/static/foo/bar/lookatthis.html> is served.

Default is "index.html".

===

nxt_conf_validator.c:

Accept "index" as a member of "share", and make sure it's a string.

===

I tried this feature in my own computer, where I tried the
following:

- Setting "index" to "lookatthis.htm", and check that the correct
  file is being served (check both a different name and a
  different extension).
- Not setting "index", and check that <index.html> is being
  served.
- Settind "index" to an array of strings, and check that the
  configuration fails:

{
	"error": "Invalid configuration.",
	"detail": "The \"index\" value must be a string, but not an array."
}
2022-05-30 12:42:18 +02:00
Alejandro Colomar 02f50533c4 Static: returning 404 when "index" is a non-regular file.
Before this patch, if "index" was a file, but not a regular file
nor a directory, so it may have been for example a FIFO, Unit
returned 404.  But if "index" was a directory, Unit returned 301.

For consistency, this patch makes Unit return 404 for every
non-regular file, including directories.
2022-05-26 19:31:08 +02:00
Alejandro Colomar 27ca67f0df Added const to remove unnecessary casts.
Casts are usually very dangerous, disabling most compiler warnings
and basically removing type safety.  This change adds 'const' to a
pointer where we don't need to write, improving type safety, and
that also allows removing some casts.
2022-05-26 14:11:12 +02:00
Alejandro Colomar 7662ec5f1b Wrapped debug code in '#if (NXT_DEBUG)'. 2022-05-17 12:41:18 +02:00
Alejandro Colomar ba20fa3939 Fixed memcpy(dest, NULL, 0) Undefined Behavior.
nxt_str_null() setted the loc.start pointer to NULL, which was
being passed to memcpy(3) through nxt_debug().  That caused
Undefined Behavior, so we now pass an empty string.
2022-05-17 12:39:41 +02:00
Alejandro Colomar c60f678485 Tests: Added tests for empty "location". 2022-05-16 12:57:37 +02:00
Alejandro Colomar 7066acb2ce Supporting empty Location URIs.
An empty string in Location was being handled specially by not sending a
Location header.  This may occur after variable resolution, so we need to
consider this scenario.

The obsolete RFC 2616 defined the Location header as consisting of an absolute
URI <https://www.rfc-editor.org/rfc/rfc2616#section-14.30>, which cannot be an
empty string.  However, the current RFC 7231 allows the Location to be a
relative URI <https://www.rfc-editor.org/rfc/rfc7231#section-7.1.2>, and a
relative URI may be an empty string <https://stackoverflow.com/a/43338457>.

Due to these considerations, this patch allows sending an empty Location header
without handling this case specially.  This behavior will probably be more
straightforward to users, too.  It also simplifies the code, which is now more
readable, fast, and conformant to the current RFC.  We're skipping an
allocation at request time in a common case such as "action": {"return": 404}
2022-05-16 12:57:37 +02:00
Alejandro Colomar 5302faace2 Renamed nxt_http_static_ctx_t field 'index' to 'share_idx'.
Having a configurable index filename will require adding an index
field to this structure.  The most natural name for that field is
'index', so the current index field should be renamed to allow for
that.  A sensible name is 'share_idx', since it's the index of the
shares array in 'nxt_http_static_conf_t'.

Instead of 'share_index' I opted for the shorter 'share_idx'.
Also, when 'index' allows an array of filenames in a following
commit, another similar variable 'index_idx' should be created,
and having a different prefix and suffix seems more readable than
for example 'index_index'.
2022-05-16 10:27:07 +02:00
Alejandro Colomar 9c276d4d60 Added .gitignore. 2022-05-03 12:41:36 +02:00
Alejandro Colomar 952bcc50bf Fixed #define style.
We had a mix of styles for declaring function-like macros:

Style A:
 #define                    \
 foo()                      \
     do {                   \
         ...                \
     } while (0)

Style B:
 #define foo()              \
     do {                   \
         ...                \
     } while (0)

We had a similar number of occurences of each style:

 $ grep -rnI '^\w*(.*\\' | wc -l
 244
 $ grep -rn 'define.*(.*)' | wc -l
 239

(Those regexes aren't perfect, but a very decent approximation.)

Real examples:

 $ find src -type f | xargs sed -n '/^nxt_double_is_zero/,/^$/p'
 nxt_double_is_zero(f)                                                         \
     (fabs(f) <= FLT_EPSILON)

 $ find src -type f | xargs sed -n '/define nxt_http_field_set/,/^$/p'
 #define nxt_http_field_set(_field, _name, _value)                             \
     do {                                                                      \
         (_field)->name_length = nxt_length(_name);                            \
         (_field)->value_length = nxt_length(_value);                          \
         (_field)->name = (u_char *) _name;                                    \
         (_field)->value = (u_char *) _value;                                  \
     } while (0)

I'd like to standardize on a single style for them, and IMO,
having the identifier in the same line as #define is a better
option for the following reasons:

- Programmers are used to `#define foo() ...` (readability).
- One less line of code.
- The program for finding them is really simple (see below).

 function grep_ngx_func()
 {
     if (($# != 1)); then
         >&2 echo "Usage: ${FUNCNAME[0]} <func>";
         return 1;
     fi;

     find src -type f \
     | grep '\.[ch]$' \
     | xargs grep -l "$1" \
     | sort \
     | xargs pcregrep -Mn "(?s)^\$[\w\s*]+?^$1\(.*?^}";

     find src -type f \
     | grep '\.[ch]$' \
     | xargs grep -l "$1" \
     | sort \
     | xargs pcregrep -Mn "(?s)define $1\(.*?^$" \
     | sed -E '1s/^[^:]+:[0-9]+:/&\n\n/';
 }

 $ grep_ngx_func
 Usage: grep_ngx_func <func>

 $ grep_ngx_func nxt_http_field_set
 src/nxt_http.h:98:

 #define nxt_http_field_set(_field, _name, _value)                             \
     do {                                                                      \
         (_field)->name_length = nxt_length(_name);                            \
         (_field)->value_length = nxt_length(_value);                          \
         (_field)->name = (u_char *) _name;                                    \
         (_field)->value = (u_char *) _value;                                  \
     } while (0)

 $ grep_ngx_func nxt_sprintf
 src/nxt_sprintf.c:56:

 u_char * nxt_cdecl
 nxt_sprintf(u_char *buf, u_char *end, const char *fmt, ...)
 {
     u_char   *p;
     va_list  args;

     va_start(args, fmt);
     p = nxt_vsprintf(buf, end, fmt, args);
     va_end(args);

     return p;
 }

................
Scripted change:
................

$ find src -type f \
  | grep '\.[ch]$' \
  | xargs sed -i '/define *\\$/{N;s/ *\\\n/ /;s/        //}'
2022-05-03 12:11:14 +02:00
Alejandro Colomar d929fbe1a4 Tests: Added tests for variables in "location". 2022-04-28 20:40:01 +02:00
Alejandro Colomar 6d017dfbe4 Tests: Changed tests to accept variables in "location". 2022-04-28 20:40:01 +02:00
Alejandro Colomar 6fb7777ce7 Supporting variables in "location".
............
Description:
............

Before this commit, the encoded URI could be calculated at
configuration time.  Now, since variables can only be resolved at
request time, we have different situations:

- "location" contains no variables:

  In this case, we still encode the URI in the conf structure, at
  configuration time, and then we just copy the resulting string
  to the ctx structure at request time.

- "location" contains variables:

  In this case, we compile the var string at configure time, then
  when we resolve it at request time, and then we encode the
  string.

In both cases, as was being done before, if the string is empty,
either before or after resolving variables, we skip the encoding.

...........
Usefulness:
...........

An example of why this feature may be useful is redirecting HTTP
to HTTPS with something like:

"action": {
    "return": 301,
    "location": "https://${host}${uri}"
}

.....
Bugs:
.....

This feature conflicts with the relevant RFCs in the following:

'$' is used for Unit variables, but '$' is a reserved character in
a URI, to be used as a sub-delimiter.  However, it's almost never
used as that, and in fact, other parts of Unit already conflict
with '$' being a reserved character for use as a sub-delimiter, so
this is at least consistent in that sense.  VBart suggested an
easy workaround if we ever need it: adding a variable '$sign'
which resolves to a literal '$'.

......
Notes:
......

An empty string is handled as if "location" wasn't specified at
all, so no Location header is sent.

This is incorrect, and the code is slightly misleading.

The Location header consists of a URI-reference[1], which might be
a relative one, which itself might consist of an empty string[2].

[1]: <https://www.rfc-editor.org/rfc/rfc7231#section-7.1.2>
[2]: <https://stackoverflow.com/a/43338457>

Now that we have variables, it's more likely that an empty
Location header will be requested, and we should handle it
correctly.

I think in a future commit we should modify the code to allow
differentiating between an unset "location" and an empty one,
which should be treated as any other "location" string.

.................
Testing (manual):
.................

{
  "listeners": {
    "*:80": {
      "pass": "routes/str"
    },
    "*:81": {
      "pass": "routes/empty"
    },
    "*:82": {
      "pass": "routes/var"
    },
    "*:83": {
      "pass": "routes/enc-str"
    },
    "*:84": {
      "pass": "routes/enc-var"
    }
  },
  "routes": {
    "str": [
      {
        "action": {
          "return": 301,
          "location": "foo"
        }
      }
    ],
    "empty": [
      {
        "action": {
          "return": 301,
          "location": ""
        }
      }
    ],
    "var": [
      {
        "action": {
          "return": 301,
          "location": "$host"
        }
      }
    ],
    "enc-str": [
      {
        "action": {
          "return": 301,
          "location": "f%23o#o"
        }
      }
    ],
    "enc-var": [
      {
        "action": {
          "return": 301,
          "location": "f%23o${host}#o"
        }
      }
    ]
  }
}

$ curl --dump-header - localhost:80
HTTP/1.1 301 Moved Permanently
Location: foo
Server: Unit/1.27.0
Date: Thu, 07 Apr 2022 23:30:06 GMT
Content-Length: 0

$ curl --dump-header - localhost:81
HTTP/1.1 301 Moved Permanently
Server: Unit/1.27.0
Date: Thu, 07 Apr 2022 23:30:08 GMT
Content-Length: 0

$ curl --dump-header - localhost:82
HTTP/1.1 301 Moved Permanently
Location: localhost
Server: Unit/1.27.0
Date: Thu, 07 Apr 2022 23:30:15 GMT
Content-Length: 0

$ curl --dump-header - -H "Host: bar" localhost:82
HTTP/1.1 301 Moved Permanently
Location: bar
Server: Unit/1.27.0
Date: Thu, 07 Apr 2022 23:30:23 GMT
Content-Length: 0

$ curl --dump-header - -H "Host: " localhost:82
HTTP/1.1 301 Moved Permanently
Server: Unit/1.27.0
Date: Thu, 07 Apr 2022 23:30:29 GMT
Content-Length: 0

$ curl --dump-header - localhost:83
HTTP/1.1 301 Moved Permanently
Location: f%23o#o
Server: Unit/1.27.0
Date: Sat, 09 Apr 2022 11:22:23 GMT
Content-Length: 0

$ curl --dump-header - -H "Host: " localhost:84
HTTP/1.1 301 Moved Permanently
Location: f%23o#o
Server: Unit/1.27.0
Date: Sat, 09 Apr 2022 11:22:44 GMT
Content-Length: 0

$ curl --dump-header - -H "Host: alx" localhost:84
HTTP/1.1 301 Moved Permanently
Location: f%23oalx#o
Server: Unit/1.27.0
Date: Sat, 09 Apr 2022 11:22:52 GMT
Content-Length: 0

$ curl --dump-header - -H "Host: a#l%23x" localhost:84
HTTP/1.1 301 Moved Permanently
Location: f%2523oa#l%2523x%23o
Server: Unit/1.27.0
Date: Sat, 09 Apr 2022 11:23:09 GMT
Content-Length: 0

$ curl --dump-header - -H "Host: b##ar" localhost:82
HTTP/1.1 301 Moved Permanently
Location: b#%23ar
Server: Unit/1.27.0
Date: Sat, 09 Apr 2022 11:25:01 GMT
Content-Length: 0
2022-04-28 20:40:01 +02:00
Alejandro Colomar 60a584cfab Workarounded Clang bug triggered by Ruby.
Add -fdeclspec to NXT_RUBY_CFLAGS for Clang, if it's available.

Clang incorrectly reports 1 for __has_declspec_attribute(x) in
some cases, such as MacOS or Cygwin.  That causes ruby code to
break.  ruby added -fdeclspec to their CFLAGS in 2019 to
workaround this bug, since it enables __declspec() and therefore,
the compiler behavior matches what it reports.

Since we don't know what are all the architectures that trigger
the clang bug, let's add the flag for all of them (especially
since it should be harmless).

Add this workaround only at the time of configuring the ruby
module.  This way we don't clutter the global NXT_CFLAGS with an
unnecessary flag.

Link: unit bug <https://github.com/nginx/unit/issues/653>
Link: ruby bug <https://bugs.ruby-lang.org/issues/18616>
Link: LLVM bug <https://github.com/llvm/llvm-project/issues/49958>
Commit: LLVM: Add -fdeclspec <d170c4b57a91adc74ca89c6d4af616a00323b12c>
Commit: ruby: Use -fdeclspec <0958e19ffb047781fe1506760c7cbd8d7fe74e57>
2022-04-28 20:02:30 +02:00
Alejandro Colomar 0b79735b50 Added NXT_MAYBE_UNUSED for __attribute__((__unused__)).
When testing some configurations of compilers and OSes, I noticed
that clang(1) 13 on Debian caused a function to be compiled but
unused, and the compiler triggered a compile error.

To avoid that error, use __attribute__((__unused__)).  Let's call
our wrapper NXT_MAYBE_UNUSED, since it describes itself more
precisely than the GCC attribute name.  It's also the name that
C2x (likely C23) has given to the standard attribute, which is
[[maybe_unused]], so it's also likely to be more readable because
of that name being in ISO C.
2022-04-27 01:14:22 +02:00
Alejandro Colomar a3d19f71a2 Fixed indentation.
Some lines (incorrectly) had an indentation of 3 or 5, or 7 or 9,
or 11 or 13, or 15 or 17 spaces instead of 4, 8, 12, or 16.  Fix them.

Found with:

$ find src -type f | xargs grep -n '^   [^ ]';
$ find src -type f | xargs grep -n '^     [^ *]';
$ find src -type f | xargs grep -n '^       [^ ]';
$ find src -type f | xargs grep -n '^         [^ *]';
$ find src -type f | xargs grep -n '^           [^ +]';
$ find src -type f | xargs grep -n '^             [^ *+]';
$ find src -type f | xargs grep -n '^               [^ +]';
$ find src -type f | xargs grep -n '^                 [^ *+]';
2022-04-26 12:38:48 +02:00
Alejandro Colomar bce0f432c4 Removed special cases for non-NXT_CONF_VALUE_ARRAY.
The previous commit added more generic APIs for handling
NXT_CONF_VALUE_ARRAY and non-NXT_CONF_VALUE_ARRAY together.
Modify calling code to remove special cases for arrays and
non-arrays, taking special care that the path for non arrays is
logically equivalent to the previous special cased code.
Use the now-generic array code only.
2022-04-26 12:38:48 +02:00
Alejandro Colomar e525605d05 Added new array APIs that also work with non-arrays.
Similar to how C pointers to variables can always be considered as
pointers to the first element of an array of size 1 (see the
following code for an example of how they are equivalent),
treating non-NXT_CONF_VALUE_ARRAY as if they were
NXT_CONF_VALUE_ARRAYs of size 1 allows for simpler and more
generic code.

	void foo(ptrdiff_t sz, int arr[sz])
	{
		for (ptrdiff_t i = 0; i < sz; i++)
			arr[i] = 0;
	}

	void bar(void)
	{
		int  x;
		int  y[1];

		foo(1, &x);
		foo(1, y);
	}

nxt_conf_array_elements_count_or_1():
	Similar to nxt_conf_array_elements_count().
	Return a size of 1 when input is non-array, instead of
	causing undefined behavior.  That value (1) makes sense
	because it will be used as the limiter of a loop that
	loops over the array and calls
	nxt_conf_get_array_element_or_itself(), which will return
	a correct element for such loops.

nxt_conf_get_array_element_or_itself():
	Similar to nxt_conf_get_array_element().
	Return the input pointer unmodified (i.e., a pointer to
	the unique element of a hypothetical array), instead of
	returning NULL, which wasn't very useful.

nxt_conf_array_qsort():
	Since it's a no-op for non-arrays, this API can be reused.
2022-04-26 12:38:48 +02:00
Alejandro Colomar 940d695f82 Added 'const' for read-only function parameter.
That parameter is not being modified in the function.  Make it
'const' to allow passing 'static const' variables.
2022-04-26 12:38:48 +02:00