summaryrefslogtreecommitdiffhomepage
path: root/src
diff options
context:
space:
mode:
authorAndrew Clayton <a.clayton@nginx.com>2024-02-28 20:46:18 +0000
committerAndrew Clayton <a.clayton@nginx.com>2024-02-29 13:05:16 +0000
commit8ff606fbca688072585325ee5a4ddb56cc034575 (patch)
tree9c74340840eac04c7b1e45cdb88d0645d86f45d2 /src
parent23e807dea3a68ead712cb2eab54fba08e545872b (diff)
downloadunit-8ff606fbca688072585325ee5a4ddb56cc034575.tar.gz
unit-8ff606fbca688072585325ee5a4ddb56cc034575.tar.bz2
Configuration: Fix check in nxt_conf_json_parse_value()
If we compile Unit with -Wstrict-overflow=5 (as we do with clang) then we get the following warning cc -c -pipe -fPIC -fvisibility=hidden -O0 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wstrict-overflow=5 -Wmissing-prototypes -g -I src -I build/include \ \ \ -o build/src/nxt_conf.o \ -MMD -MF build/src/nxt_conf.dep -MT build/src/nxt_conf.o \ src/nxt_conf.c src/nxt_conf.c: In function ‘nxt_conf_json_parse_value’: src/nxt_conf.c:1444:5: warning: assuming signed overflow does not occur when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Wstrict-overflow] 1444 | if (nxt_fast_path((ch - '0') <= 9)) { | Does this actually cause an issue?... well, yes. Using this minimal test config to show the problem { "listeners": { "[::1]:8080": { "pass": --100 } } } With the above if () statement that triggers the warning, my assumption here is that we only want a digit now. '0' - '9'. ch is a u_char, however if ch is any character with an ASCII code < 48 ('0') e.g if ch is '-' (45) then we get 45 - 48 = -3, through arithmetic conversion, which makes the if () statement true (when it shouldn't) then at some point we get the following error returned from the controller { "error": "Memory allocation failed." } Instead of the expected { "error": "Invalid JSON.", "detail": "A valid JSON value is expected here. It must be either a literal (null, true, or false), a number, a string (in double quotes \"\"), an array (with brackets []), or an object (with braces {}).", "location": { "offset": 234, "line": 15, "column": 27 } } Casting the result of (ch - '0') to u_char resolves this issue, this makes the above calculation come out as 253 (relying on unsigned integer wraparound) which was probably the intended way for it to work. Reviewed-by: Zhidao Hong <z.hong@f5.com> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
Diffstat (limited to 'src')
-rw-r--r--src/nxt_conf.c2
1 files changed, 1 insertions, 1 deletions
diff --git a/src/nxt_conf.c b/src/nxt_conf.c
index 008cb968..9ae25172 100644
--- a/src/nxt_conf.c
+++ b/src/nxt_conf.c
@@ -1441,7 +1441,7 @@ nxt_conf_json_parse_value(nxt_mp_t *mp, nxt_conf_value_t *value, u_char *start,
goto error;
}
- if (nxt_fast_path((ch - '0') <= 9)) {
+ if (nxt_fast_path((u_char)(ch - '0') <= 9)) {
p = nxt_conf_json_parse_number(mp, value, start, end, error);
if (nxt_slow_path(p == NULL)) {