AlinsRan opened a new pull request, #13713:
URL: https://github.com/apache/apisix/pull/13713
### What this PR does
Makes `nginx_config.envs` entries whose value contains a space (or `;`, `{`,
`#`, tab) work instead of producing an nginx.conf that fails to parse.
Fixes #11467
### Root cause
`apisix/cli/ngx_tpl.lua` renders each entry verbatim into the `env`
directive, without quoting:
```
{% for _, name in ipairs(envs) do %}
env {*name*};
{% end %}
```
So `nginx_config.envs: [TEST=a b]` generates `env TEST=a b;`. nginx's
tokenizer splits that into two parameters, and `env` accepts exactly one:
```
[emerg] invalid number of arguments in "env" directive in conf/nginx.conf:32
```
This writing has been unchanged since 952450bd7 (2020-10). Only
`nginx_config.envs` entries written as `NAME=value` are affected; entries
exported via `${{VAR}}` render as a bare `env NAME;` and were never broken.
### This is not an nginx limitation
The issue thread concludes "This is also a limitation of nginx." That is
wrong — nginx handles env values with spaces fine, the value just has to arrive
as a single token.
Verified against nginx 1.29.2 `src/core/ngx_conf_file.c:
ngx_conf_read_token()`: inside a double-quoted token, space / `;` / `{` / `#` /
tab / newline are all taken literally and only `"` ends the token; the copy
stage turns `\"` into `"` and `\\` into `\`. Confirmed empirically too —
hand-editing the generated conf to `env "TEST=a b";` makes `nginx -t` pass, and
a worker-side `os.getenv("TEST")` returns `a b`.
So this is purely a template rendering bug on the APISIX side.
### Changes
1. `ngx_tpl.lua` — render as `env "{*name*}";`.
2. `ops.lua` — escape `\` and `"` in each entry, placed right before `-- fix
up lua path`, which is where all three envs sources (explicit
`nginx_config.envs`, `${{VAR}}` exported names, kubernetes discovery injected
names) have converged. Escaping is a no-op for bare `NAME` entries, so no
branching is needed.
3. `schema.lua` — `pattern = [[\A[^\x00-\x1f]*\z]]` on the array items.
Quoting is unconditional rather than "only when the value looks special".
Getting that character set wrong by one character reproduces the original bug,
and the tokenizer guarantees quoting is lossless for any non-control value, so
the simple version is also the correct one.
### Why `$` is not escaped
`env` is a core directive and does not compile its argument as a script, and
nginx's conf parser does no interpolation at tokenize time —
`ngx_conf_read_token()` only uses a `variable` flag so that `${...}` doesn't
break the token, then stores the bytes literally. A value like `TEST=$FOO`
reaches the environment as the literal `$FOO`, both before and after this
change.
### Why the control-character rejection
NUL cannot be carried through the C environ at all (it would silently
truncate), and other control characters have no sane config source. nginx would
technically accept a literal newline inside a quoted token, but a multi-line
`env` directive makes the generated conf unreadable and ungreppable, so
`\x00-\x1f` is rejected wholesale.
Note `\A...\z` rather than `^...$`: cli/schema.lua patterns are PCRE, where
`$` also matches before a trailing newline, so `^[^\x00-\x1f]*$` would let a
value ending in `\n` through.
### Test changes
`t/cli` has ~20 assertions grepping the generated conf for `env NAME;`.
Those patterns no longer match once the directive is quoted, so they are
updated to `env "NAME";` in `test_main.sh`, `test_standalone.sh` and
`test_kubernetes.sh`. Without this they would all fail on the fixed code — the
assertions are checking the rendered form, and the rendered form is exactly
what this PR changes.
One assertion is deliberately left alone: `test_main.sh` `env
APISIX_DEPLOYMENT_ETCD_HOST;` is a hardcoded line in the template, not part of
the `envs` loop.
New cases added to `test_main.sh` after "change default env": value with
spaces renders quoted and survives `nginx -t`; embedded `"` / `\` are escaped
and survive `nginx -t`; a value with a control character is rejected at `apisix
init` with `failed to validate config`.
### Compatibility
Configs that work today are unaffected — for a value with no `\`, `"` or
control character, the token nginx parses out of the quoted form is
byte-identical to the unquoted one. Configs that were guaranteed to fail at
startup now work. The one new hard failure is control characters, which move
from "generates an invalid nginx.conf and dies with an obscure nginx syntax
error" to a clear schema error at init time.
External tooling that greps the generated `conf/nginx.conf` for `env NAME;`
will stop matching. That file declares itself read-only and generated, and its
format is not a stability promise.
### Not covered
The template renders several other values unquoted (`user`, `error_log`,
`access_log`, `lua_ssl_trusted_certificate`, `ssl_certificate`, ...), which
have the same theoretical problem for paths containing spaces. No issue reports
on those and the fix surface is independent, so this PR deliberately stays
scoped to `envs`.
### Test status
Lua syntax checks and luacheck pass. The `t/cli` suite has not been run
locally; relying on CI for that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
