From 0ac9ac4422a789c0a81e3278c39027530c836b42 Mon Sep 17 00:00:00 2001 From: user01010111 <12504630+user01010111@users.noreply.github.com> Date: Wed, 9 Sep 2026 07:27:08 +1200 Subject: [PATCH 1/3] parseconf: accept literal hashes inside double quotes Accept quoted hashes in the shared parser while retaining unquoted comments and escaped output for older peers. Keep default upsc text output unchanged and document configuration, protocol and dummy-ups behavior. Add portable parser regressions and localhost text/JSON integration. Validated with C89/C99/C11 GCC and Clang builds, sanitizers, full NIT, package/install checks, documentation checks and distcheck-light. Related: #607, #1306, #3629 AI assistance: OpenAI Codex with gpt-6-astra (x-high reasoning) and gpt-daybreak-blue-latest (high reasoning). The human contributor remains responsible for reviewing and submitting the change. Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com> --- NEWS.adoc | 6 + common/parseconf.c | 17 --- docs/config-notes.txt | 36 ++++-- docs/man/dummy-ups.txt | 28 ++++- docs/net-protocol.txt | 18 +++ tests/Makefile.am | 4 + tests/NIT/Makefile.am | 1 + tests/NIT/nit.sh | 35 ++++++ tests/NIT/parseconf-test.py | 112 ++++++++++++++++++ tests/parseconf-test.c | 229 ++++++++++++++++++++++++++++++++++++ 10 files changed, 457 insertions(+), 29 deletions(-) create mode 100644 tests/NIT/parseconf-test.py create mode 100644 tests/parseconf-test.c diff --git a/NEWS.adoc b/NEWS.adoc index e54e3f1c81..a00eda370a 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -439,6 +439,12 @@ https://github.com/networkupstools/nut/milestone/13 not do so for single-driver runs -- addressed with this release. [#3302] - common code: + * The configuration and protocol parser now accepts a literal `#` + inside double quotes, including in UPS and `dummy-ups` outlet + descriptions. Unquoted `#` still starts a comment, and escaped + hashes remain supported. Encoded output continues to escape hashes + for older peers; default `upsc` text output is unchanged. + [issues #607, #1306] * POSIX daemons now warn if their real or effective UID remains zero after the common credential switch. [issue #3471, PR #3609] * Refactored `common::background()` method used by numerous NUT daemons diff --git a/common/parseconf.c b/common/parseconf.c index fad1b2fbe3..e0212bce6f 100644 --- a/common/parseconf.c +++ b/common/parseconf.c @@ -283,26 +283,9 @@ static int findeol(PCONF_CTX_t *ctx) return STATE_FINDEOL; } -/* set up the error reporting details */ -static void pconf_seterr(PCONF_CTX_t *ctx, const char *errmsg) -{ - snprintf(ctx->errmsg, PCONF_ERR_LEN, "%s", errmsg); - - ctx->error = 1; -} - /* quote characters inside a word bounded by "quotes" */ static int quotecollect(PCONF_CTX_t *ctx) { - /* user is trying to break us */ - if (ctx->ch == '#') { - pconf_seterr(ctx, "Unbalanced word due to unescaped # in quotes"); - endofword(ctx); - - /* this makes us drop all the way out of the caller */ - return STATE_PARSEERR; - } - /* another " means we're done with this word */ if (ctx->ch == '"') { endofword(ctx); diff --git a/docs/config-notes.txt b/docs/config-notes.txt index f06450c3e9..145cca14e7 100644 --- a/docs/config-notes.txt +++ b/docs/config-notes.txt @@ -35,8 +35,10 @@ Details about the configuration files Generalities ~~~~~~~~~~~~ -All configuration files within this package are parsed with a common -state machine, which means they all can use a number of extras described here. +Most configuration files within this package are parsed with a common +state machine, which means they can use a number of extras described here. +The service configuration file linkman:nut.conf[5] has its own syntax +requirements, described in its manual page. First, most of the programs use an upper-case word to declare a configuration directive. This may be something like MONITOR, NOTIFYCMD, @@ -45,13 +47,19 @@ or ACCESS. The case does matter here. "monitor" won't be recognized. Next, the parser does not care about whitespace between words. If you like to indent things with tabs or spaces, feel free to do it here. -If you need to set a value to something containing spaces, it has to be -contained within "quotes" to keep the parser from splitting up the line. +If you need to set a value to something containing spaces, put it within +double quotes or escape each space with a backslash to keep the parser +from splitting up the line. That is, you want to use something like this: SHUTDOWNCMD "/sbin/shutdown -h +0" -Without the quotes, it would only see the first word on the line. +Without quoting or escaping the spaces, a directive expecting one value +would only see the first word of that value. + +Double quotes must begin at the start of a token. A closing double quote +completes the token: `"one"two` yields two tokens, `one` and `two`. +Single quotes (apostrophes) are ordinary characters and do not group words. OK, so let's say you really need to embed that kind of quote within your configuration directive for some reason. You can do that too. @@ -65,15 +73,18 @@ string, you just escape it. NOTIFYCMD "/bin/notifyme c:\\dos\\style\\path" -The `\` can actually be used to escape any character, but you only really -need it for `\`, `"`, and `#` as they have special meanings to the parser. +The `\` can actually be used to escape any permitted character, inside or +outside double quotes. It quotes the next character once: `\n` means the +letter `n`, not a newline. It is useful for `\`, `"`, and, outside double +quotes, spaces, `=`, and `#`, which have special meanings to the parser. When using file names with space characters, you may end up having tricky things since you need to write them inside `""` which must be escaped: NOTIFYCMD "\"c:\\path with space\\notifyme\" \"c:\\path with space\\name\"" -`#` is the comment character. Anything after an unescaped `#` is ignored. +Outside double quotes, `#` is the comment character. Anything after an +unescaped `#` is ignored until the end of the physical line. Something like this... @@ -81,11 +92,16 @@ Something like this... will actually turn into `identity = my`, since the `#` stops the parsing. If you really need to have a `#` in your configuration, then -escape it. +escape it or put the value inside double quotes. identity = my\#1ups -Much better. +or: + + identity = "my#1ups" + +Older versions of the parser required `\#` even inside double quotes. +That spelling remains supported for compatibility with those versions. The `=` character should be used with care too. There should be only one "simple" `=` character in a line: between the parameter name and its value. diff --git a/docs/man/dummy-ups.txt b/docs/man/dummy-ups.txt index e7dedff4a1..a6cbd78ebc 100644 --- a/docs/man/dummy-ups.txt +++ b/docs/man/dummy-ups.txt @@ -141,8 +141,32 @@ This definition file, specified by the `port` argument in the example above, is generally named `something.dev` or `something.seq`. It contains a list of all valid variables and associated values (you can later use `upsrw` only to modify values of these variables), and has the same format as an -linkman:upsc[8] data dump (`: `). This means you can easily -create definition files from an existing UPS using `upsc > file.dev`. +linkman:upsc[8] data dump (`: `). You can use +`upsc > file.dev` as a starting point for a definition file. + +Values are read with the common NUT configuration parser. Unquoted value +tokens are joined with a single space; double quotes preserve spaces +inside a token. Single quotes (apostrophes) are literal characters. +An unescaped `#` outside double quotes starts a comment, while a `#` +inside double quotes is literal. For example: + + outlet.1.desc: Outlet #1 + outlet.2.desc: "Outlet \#2" + outlet.3.desc: "Outlet #3" + outlet.4.desc: Outlet \#4 + +These values become `Outlet`, `Outlet #2`, `Outlet #3`, and `Outlet #4`, +respectively. Older versions of the parser require `\#` even inside +double quotes. Backslashes quote the next permitted character once, so +`\n` becomes `n`, while `\\` becomes a single literal backslash. +A backslash followed by a physical newline continues the logical line. +An unescaped `=` outside double quotes is a separate token; quote or +escape it when it is part of a value. + +The default `upsc` output does not add quoting or escaping. Before reading +such a dump back with *dummy-ups*, quote or escape values containing these +special characters as needed. Its JSON output preserves string values +unambiguously, but JSON is not a *dummy-ups* definition file format. Note that the Network UPS project provides an extensive link:https://www.networkupstools.org/ddl/index.html[DDL (Devices Dumps Library)] diff --git a/docs/net-protocol.txt b/docs/net-protocol.txt index 46b3eefc8e..3bcac39971 100644 --- a/docs/net-protocol.txt +++ b/docs/net-protocol.txt @@ -44,6 +44,24 @@ Embedded quotes are escaped with backslashes. Embedded backslashes are also escaped by representing them as \\. This protocol is intended to be interpreted with parseconf (NUT parser) or something similar. +Double quotes start a quoted element only at the beginning of an element. +The closing quote completes that element, so `"one"two` produces two +elements, `one` and `two`. Single quotes (apostrophes) are ordinary +characters; they do not group words. Outside double quotes, unescaped +whitespace separates elements, and an unescaped `=` becomes a separate +element of its own. + +An unescaped `#` starts a comment only outside double quotes. Inside +double quotes it is literal, so `"Outlet #3"` represents `Outlet #3`. +For compatibility with older parsers which rejected an unescaped `#` +inside quotes, `pconf_encode()` continues to send it as `\#`. + +A backslash quotes the next character once, inside or outside double +quotes, subject to the character restrictions of the parser. For example, +`one\ two` is one element, while `\n` represents the letter `n`, not a +newline. A backslash followed by a physical newline continues the logical +line. These rules are NUT syntax, not shell or JSON string syntax. + Revision history ---------------- diff --git a/tests/Makefile.am b/tests/Makefile.am index 1d21744fc1..a9150988eb 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -99,6 +99,10 @@ nodist_test_cgilib_SOURCES = cgilib.c test_cgilib_CFLAGS = $(AM_CFLAGS) -I$(top_srcdir)/clients test_cgilib_LDADD = $(NUT_LIBCOMMON) +TESTS += parseconf-test +parseconf_test_SOURCES = parseconf-test.c +parseconf_test_LDADD = $(NUT_LIBCOMMON) + nutlogtest_SOURCES = nutlogtest.c nutlogtest_LDADD = $(NUT_LIBCOMMON) diff --git a/tests/NIT/Makefile.am b/tests/NIT/Makefile.am index 21d2319ee3..d8ab932100 100644 --- a/tests/NIT/Makefile.am +++ b/tests/NIT/Makefile.am @@ -10,6 +10,7 @@ @NUT_AM_MAKE_CAN_EXPORT@@NUT_AM_EXPORT_CCACHE_PATH@export PATH=@PATH_DURING_CONFIGURE@ EXTRA_DIST = nit.sh README.adoc +EXTRA_DIST += parseconf-test.py if WITH_CHECK_NIT check: check-NIT diff --git a/tests/NIT/nit.sh b/tests/NIT/nit.sh index d24a612c23..9b2280ce2d 100755 --- a/tests/NIT/nit.sh +++ b/tests/NIT/nit.sh @@ -4728,6 +4728,40 @@ testgroup_sandbox_python() { sandbox_forget_configs } +testgroup_sandbox_parseconf() { + # Start a fresh sandbox so these punctuation fixtures do not change + # the model, timer and language-binding tests' expected data. + if ! isTestablePython || [ -z "${PYTHON}" ]; then + if [ x"${NIT_CASE}" = xtestgroup_sandbox_parseconf ]; then + log_error "[testgroup_sandbox_parseconf] Python with the json module is required" + FAILED="`expr $FAILED + 1`" + FAILED_FUNCS="$FAILED_FUNCS testgroup_sandbox_parseconf:missing-python" + else + log_warn "[testgroup_sandbox_parseconf] SKIPPED: Python is unavailable" + SKIPPED="`expr $SKIPPED + 1`" + SKIPPED_FUNCS="$SKIPPED_FUNCS testgroup_sandbox_parseconf" + fi + return + fi + + stop_daemons + sandbox_forget_configs + sandbox_generate_configs + $PYTHON "${TOP_SRCDIR}/tests/NIT/parseconf-test.py" prepare "$NUT_CONFPATH" \ + || die "[testgroup_sandbox_parseconf] Could not prepare parser fixtures" + testcase_sandbox_start_drivers_after_upsd + + if $PYTHON "${TOP_SRCDIR}/tests/NIT/parseconf-test.py" \ + "${TOP_BUILDDIR}/clients/upsc${EXEEXT}" "$NUT_PORT" ; then + PASSED="`expr $PASSED + 1`" + log_info "[testgroup_sandbox_parseconf] PASSED: exact text and decoded JSON values" + else + FAILED="`expr $FAILED + 1`" + FAILED_FUNCS="$FAILED_FUNCS testgroup_sandbox_parseconf" + fi + sandbox_forget_configs +} + testgroup_sandbox_perl() { # Arrange for quick test iterations testcase_sandbox_start_drivers_after_upsd @@ -4854,6 +4888,7 @@ case "${NIT_CASE}" in testgroup_upsd_invalid_configs testgroup_upsd_questionable_configs testgroup_sandbox + testgroup_sandbox_parseconf ;; *) die "Unsupported NIT_CASE='$NIT_CASE' was requested" ;; esac diff --git a/tests/NIT/parseconf-test.py b/tests/NIT/parseconf-test.py new file mode 100644 index 0000000000..8aaf932d89 --- /dev/null +++ b/tests/NIT/parseconf-test.py @@ -0,0 +1,112 @@ +#!/usr/bin/env python +# Exact values through configuration, dummy-ups, upsd and upsc. +# Copyright (C) 2026 Network UPS Tools project +# SPDX-License-Identifier: GPL-2.0-or-later + +from __future__ import print_function + +import json +import os +import subprocess +import sys +import threading + + +# Input is NUT syntax; expected values are literal decoded strings. +VALUES = [ + ('Outlet #1', 'Outlet'), + ('"Outlet \\#2"', 'Outlet #2'), + ('"Outlet #3"', 'Outlet #3'), + ('Outlet \\#4', 'Outlet #4'), + ('"\\"edge\\" C:\\\\ups #5"', '"edge" C:\\ups #5'), + ('one\\ two \\n \\t \\x41', 'one two n t x41'), + ("'single quotes'", "'single quotes'"), + ('"a=b:c two spaces"', 'a=b:c two spaces'), + ('prefix\\\nsuffix', 'prefixsuffix'), + ('""', None), # dummy-ups deletes variables assigned an empty value. + ('"a\\\\#b"', 'a\\#b'), + ('"a\\\\\\"b"', 'a\\"b'), +] +DESCRIPTION_INPUT = '"Parser #1: \\"quoted\\" C:\\\\ups\'s"' +DESCRIPTION = 'Parser #1: "quoted" C:\\ups\'s' + + +def prepare(confpath): + path = os.path.join(confpath, 'ups.conf') + with open(path, 'r') as source: + config = source.read() + original = 'desc = "Crash Dummy"' + if config.count(original) != 1: + raise AssertionError('expected one fresh NIT dummy configuration') + with open(path, 'w') as target: + target.write(config.replace(original, 'desc = ' + DESCRIPTION_INPUT)) + with open(os.path.join(confpath, 'dummy.seq'), 'w') as target: + target.write('ups.status: OL\n') + for index, (value, expected) in enumerate(VALUES): + if expected is None: + target.write('outlet.%d.desc: before empty\n' % (index + 1)) + target.write('outlet.%d.desc: %s\n' % (index + 1, value)) + target.write('TIMER 60\n') + + +def query(upsc, *args): + process = subprocess.Popen([upsc] + list(args), stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + # Python 2.6 also runs NIT; communicate(timeout=...) is newer. + timer = threading.Timer(10, process.kill) + timer.start() + try: + output, error = process.communicate() + finally: + timer.cancel() + timer.join() + if process.returncode: + raise AssertionError('upsc %r returned %d: %r' % + (args, process.returncode, error)) + # Fixtures are ASCII. Normalize only the platform's output line ending. + return output.decode('ascii').replace('\r\n', '\n') + + +def equal(actual, expected): + if actual != expected: + raise AssertionError('got %r, expected %r' % (actual, expected)) + + +def check(upsc, port): + host = '127.0.0.1:' + port + ups = 'dummy@' + host + plain = query(upsc, ups).splitlines() + data = json.loads(query(upsc, '-j', ups)) + for index, (value, expected) in enumerate(VALUES): + key = 'outlet.%d.desc' % (index + 1) + if expected is None: + equal([line for line in plain if line.startswith(key + ':')], []) + equal(key in data, False) + print('PASS: empty value removes %s from text and JSON lists' % key) + continue + equal([line for line in plain if line.startswith(key + ':')], + [key + ': ' + expected]) + equal(data[key], expected) + equal(query(upsc, ups, key), expected + '\n') + equal(json.loads(query(upsc, '-j', ups, key)), expected) + print('PASS: %s = %r (text and JSON, list and single value)' % + (key, expected)) + + names = query(upsc, '-l', host).splitlines() + equal(json.loads(query(upsc, '-j', '-l', host)), names) + descriptions = json.loads(query(upsc, '-j', '-L', host)) + equal(sorted(descriptions), sorted(names)) + equal(descriptions['dummy'], DESCRIPTION) + equal([line for line in query(upsc, '-L', host).splitlines() + if line.startswith('dummy:')], ['dummy: ' + DESCRIPTION]) + # These read-only clients do not LOGIN to the UPS. + equal(query(upsc, '-c', ups).splitlines(), []) + equal(json.loads(query(upsc, '-j', '-c', ups)), []) + print('PASS: configured description, UPS lists and empty client lists') + + +if __name__ == '__main__': + if sys.argv[1] == 'prepare': + prepare(sys.argv[2]) + else: + check(sys.argv[1], sys.argv[2]) diff --git a/tests/parseconf-test.c b/tests/parseconf-test.c new file mode 100644 index 0000000000..0dc170f80a --- /dev/null +++ b/tests/parseconf-test.c @@ -0,0 +1,229 @@ +/* parseconf-test - configuration and protocol token boundaries + + Copyright (C) 2026 Network UPS Tools project + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. +*/ + +#include "config.h" /* should be first */ +#include "common.h" +#include "parseconf.h" + +typedef struct { + const char *name; + const char *input; + size_t numargs; + const char *args[8]; +} testcase_t; + +static const testcase_t cases[] = { + { "ordinary", "one two\n", 2, { "one", "two" } }, + { "empty", " \t\n", 0, { NULL } }, + { "comment", "# ignored\n", 0, { NULL } }, + { "unquoted hash", "Outlet #1\n", 1, { "Outlet" } }, + { "attached comment", "my#1ups\n", 1, { "my" } }, + { "quoted hash", "\"Outlet #3\"\n", 1, { "Outlet #3" } }, + { "quoted hash boundaries", "\"#\" \"##last#\"\n", 2, { "#", "##last#" } }, + { "escaped hash", "\"Outlet \\#2\" Outlet \\#4\n", 3, + { "Outlet #2", "Outlet", "#4" } }, + { "comment after quote", "\"# value\"# ignored\n", 1, { "# value" } }, + { "empty quotes", "\"\" \"\"\n", 2, { "", "" } }, + { "closing quote commits", "\"one\"two \"three\"\"four\"\n", 4, + { "one", "two", "three", "four" } }, + { "quote inside unquoted word", "a\"b\"c\n", 1, { "a\"b\"c" } }, + { "apostrophes", "it's 'two words' \"'literal'\"\n", 4, + { "it's", "'two", "words'", "'literal'" } }, + { "equal separators", "a=b==\"c=d\"\n", 6, { "a", "=", "b", "=", "=", "c=d" } }, + { "escaped spaces and equal", "one\\ two \\=value \"a\\ b\\=c\"\n", 3, + { "one two", "=value", "a b=c" } }, + { "escaped letters", "\\n\\t\\r\\x41 \"\\n\\t\\r\\x41\"\n", 2, + { "ntrx41", "ntrx41" } }, + { "escaped quotes", "\\\"edge\\\" \"\\\"edge\\\"\"\n", 2, + { "\"edge\"", "\"edge\"" } }, + { "escaped backslashes", "a\\\\b \"a\\\\b\\\\\\\\c\"\n", 2, + { "a\\b", "a\\b\\\\c" } }, + { "backslash before hash", "\"a\\\\#b\" \"a\\\\\\#b\"\n", 2, + { "a\\#b", "a\\#b" } }, + { "backslash before quote", "\"a\\\\\"b \"a\\\\\\\"b\"\n", 3, + { "a\\", "b", "a\\\"b" } }, + { "continuation", "one\\\ntwo \"three\\\nfour\"\n", 2, + { "onetwo", "threefour" } }, + { "quoted physical newline", "\"one\ntwo\"\n", 1, { "onetwo" } }, + { "character filter", "\"a\001\t\r\200\377b\177\"\n", 1, { "ab\177" } } +}; + +static int failures = 0; + +static void check_args(PCONF_CTX_t *ctx, const testcase_t *test, const char *mode) +{ + size_t i; + + if (pconf_parse_error(ctx)) { + fprintf(stderr, "%s/%s: unexpected parse error: %s\n", + test->name, mode, ctx->errmsg); + failures++; + } + if (ctx->numargs != test->numargs) { + fprintf(stderr, "%s/%s: got %" PRIuSIZE " arguments, expected %" PRIuSIZE "\n", + test->name, mode, ctx->numargs, test->numargs); + failures++; + return; + } + for (i = 0; i < test->numargs; i++) { + if (strcmp(ctx->arglist[i], test->args[i])) { + fprintf(stderr, "%s/%s: argument %" PRIuSIZE ": got <%s>, expected <%s>\n", + test->name, mode, i, ctx->arglist[i], test->args[i]); + failures++; + } + } +} + +static void check_result(int actual, int expected, const char *name) +{ + if (actual != expected) { + fprintf(stderr, "%s: got %d, expected %d\n", name, actual, expected); + failures++; + } +} + +static void file_input(PCONF_CTX_t *ctx, const char *input) +{ + /* tmpfile() gives each run a private stream and removes it on close. */ + ctx->f = tmpfile(); + if (!ctx->f) + fatal_with_errno(EXIT_FAILURE, "tmpfile"); + if (fputs(input, ctx->f) == EOF || fflush(ctx->f)) + fatal_with_errno(EXIT_FAILURE, "write test input"); + rewind(ctx->f); +} + +static void check_case(const testcase_t *test) +{ + PCONF_CTX_t ctx; + size_t i, len = strlen(test->input); + int ret; + + pconf_init(&ctx, NULL); + check_result(pconf_line(&ctx, test->input), 1, test->name); + check_args(&ctx, test, "line"); + pconf_finish(&ctx); + + pconf_init(&ctx, NULL); + file_input(&ctx, test->input); + check_result(pconf_file_next(&ctx), 1, test->name); + check_args(&ctx, test, "file"); + check_result(pconf_file_next(&ctx), 0, "file EOF"); + pconf_finish(&ctx); + + pconf_init(&ctx, NULL); + for (i = 0; i < len; i++) { + ret = pconf_char(&ctx, test->input[i]); + check_result(ret, (i == len - 1) ? 1 : 0, test->name); + if (ret < 0) + break; + } + check_args(&ctx, test, "char"); + pconf_finish(&ctx); +} + +static void check_boundaries(void) +{ + PCONF_CTX_t ctx; + static const testcase_t incomplete = { + "incomplete quote", "key \"unfinished", 2, { "key", "unfinished" } + }; + static const testcase_t limited = { + "limits", "abcde fghij discarded\n", 2, { "abcd", "fghi" } + }; + static const testcase_t trailing = { + "trailing backslash", "key value\\", 2, { "key", "value" } + }; + static const testcase_t next = { + "next line", "next\n", 1, { "next" } + }; + size_t i; + + /* These incomplete inputs historically return their collected words. */ + pconf_init(&ctx, NULL); + check_result(pconf_line(&ctx, incomplete.input), 1, incomplete.name); + check_args(&ctx, &incomplete, "line"); + check_result(pconf_line(&ctx, trailing.input), 1, trailing.name); + check_args(&ctx, &trailing, "line"); + ctx.arg_limit = 2; + ctx.wordlen_limit = 4; + check_result(pconf_line(&ctx, limited.input), 1, limited.name); + check_args(&ctx, &limited, "line"); + pconf_finish(&ctx); + + pconf_init(&ctx, NULL); + file_input(&ctx, incomplete.input); + check_result(pconf_file_next(&ctx), 1, incomplete.name); + check_args(&ctx, &incomplete, "file"); + check_result(pconf_file_next(&ctx), 0, "incomplete file EOF"); + pconf_finish(&ctx); + + pconf_init(&ctx, NULL); + for (i = 0; i < strlen(incomplete.input); i++) + check_result(pconf_char(&ctx, incomplete.input[i]), 0, "incomplete char input"); + check_result(pconf_parse_error(&ctx), 0, "incomplete char error"); + pconf_finish(&ctx); + + pconf_init(&ctx, NULL); + for (i = 0; i < 2; i++) { + check_result(pconf_char(&ctx, '"'), 0, "empty token start"); + check_result(pconf_char(&ctx, '"'), 0, "empty token end"); + check_result(pconf_char(&ctx, '\n'), 1, "char line end"); + check_result(pconf_line(&ctx, next.input), 1, "next line"); + check_args(&ctx, &next, "reused context"); + } + pconf_finish(&ctx); + check_result(pconf_line(&ctx, "ignored"), 0, "finished context"); + check_result(pconf_line(NULL, "ignored"), 0, "NULL context"); + check_result(pconf_char(NULL, 'x'), -1, "NULL char context"); +} + +static void check_encoder(void) +{ + static const char raw[] = "# \\\"' = end#"; + static const char encoded[] = "\\# \\\\\\\"' = end\\#"; + PCONF_CTX_t ctx; + char output[128], input[132]; + const testcase_t roundtrip = { "encoder roundtrip", NULL, 1, { raw } }; + const testcase_t short_result = { "short encoder roundtrip", NULL, 1, { "#" } }; + + check_result(pconf_encode(raw, output, sizeof(output)) == output, 1, "encoder return"); + check_result(strcmp(output, encoded), 0, "encoder bytes (including escaped hash)"); + snprintf(input, sizeof(input), "\"%s\"\n", output); + pconf_init(&ctx, NULL); + check_result(pconf_line(&ctx, input), 1, "encoder parse"); + check_args(&ctx, &roundtrip, "line"); + + /* A truncated encoding must not leave an unmatched escape character. */ + pconf_encode("##", output, 4); + check_result(strcmp(output, "\\#"), 0, "short encoder bytes"); + snprintf(input, sizeof(input), "\"%s\"\n", output); + check_result(pconf_line(&ctx, input), 1, "short encoder parse"); + check_args(&ctx, &short_result, "line"); + pconf_encode("#", output, 2); + check_result(output[0], '\0', "encoder cannot fit escape pair"); + output[0] = 'x'; + pconf_encode("#", output, 0); + check_result(output[0], 'x', "zero sized encoder destination"); + pconf_finish(&ctx); +} + +int main(void) +{ + size_t i; + + for (i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) + check_case(&cases[i]); + check_boundaries(); + check_encoder(); + printf("parseconf: %" PRIuSIZE " cases through line/file/char, boundary and encoder checks: %d failures\n", + sizeof(cases) / sizeof(cases[0]), failures); + return failures ? EXIT_FAILURE : EXIT_SUCCESS; +} From c925c962386466d92daf269cc789aba6f690e967 Mon Sep 17 00:00:00 2001 From: user01010111 <12504630+user01010111@users.noreply.github.com> Date: Thu, 10 Sep 2026 15:34:33 +1200 Subject: [PATCH 2/3] NIT: check JSON availability before parser integration tests Probe the selected Python interpreter for json before preparing the parser fixtures. Report a skip in the default suite if the prerequisite is absent, while preserving failure for an explicitly requested parser test group. Keep the existing parser and JSON assertions unchanged. Jenkins exposed a Python 3.7 installation unable to import its standard json module. The worker still needs repair to restore this integration coverage. Validated the missing-module failure and skip paths, native macOS and Linux parser integration, Linux make check (13 tests), style/spelling checks, and default make distcheck-light. The missing module was simulated locally; the exact Jenkins worker and physical UPS hardware were not tested. Related: #3633 AI assistance: OpenAI Codex with gpt-6-astra. The human contributor remains responsible for reviewing and submitting the change. Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com> --- tests/NIT/nit.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/NIT/nit.sh b/tests/NIT/nit.sh index 9b2280ce2d..b52a3476c5 100755 --- a/tests/NIT/nit.sh +++ b/tests/NIT/nit.sh @@ -4731,13 +4731,14 @@ testgroup_sandbox_python() { testgroup_sandbox_parseconf() { # Start a fresh sandbox so these punctuation fixtures do not change # the model, timer and language-binding tests' expected data. - if ! isTestablePython || [ -z "${PYTHON}" ]; then + if ! isTestablePython || [ -z "${PYTHON}" ] \ + || ! $PYTHON -c 'import json' >/dev/null 2>&1 ; then if [ x"${NIT_CASE}" = xtestgroup_sandbox_parseconf ]; then log_error "[testgroup_sandbox_parseconf] Python with the json module is required" FAILED="`expr $FAILED + 1`" - FAILED_FUNCS="$FAILED_FUNCS testgroup_sandbox_parseconf:missing-python" + FAILED_FUNCS="$FAILED_FUNCS testgroup_sandbox_parseconf:missing-prerequisites" else - log_warn "[testgroup_sandbox_parseconf] SKIPPED: Python is unavailable" + log_warn "[testgroup_sandbox_parseconf] SKIPPED: Python with the json module is unavailable" SKIPPED="`expr $SKIPPED + 1`" SKIPPED_FUNCS="$SKIPPED_FUNCS testgroup_sandbox_parseconf" fi From fd8c0f65bcf311f89a62524a61a5d82be145f2a2 Mon Sep 17 00:00:00 2001 From: user01010111 <12504630+user01010111@users.noreply.github.com> Date: Sun, 13 Sep 2026 09:39:04 +0000 Subject: [PATCH 3/3] NIT: clarify parser watchdog failures and address review feedback Report query starts, watchdog expiry and captured process output. Allow a bounded per-query NIT_PARSECONF_TIMEOUT, defaulting to 60 seconds, and preserve per-client debug settings in the named parser testcase. Retain the parser error helper under #if 0. Clarify dummy-ups token joining, escaped spaces, Python compatibility and the quoted-hash upgrade behaviour. Keep EOF, multiline and active parser semantics unchanged. AI assistance: OpenAI Codex with gpt-6-astra. Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com> --- UPGRADING.adoc | 7 ++++ common/parseconf.c | 10 +++++ docs/config-notes.txt | 4 ++ docs/man/dummy-ups.txt | 8 +++- tests/NIT/nit.sh | 60 ++++++++++++++++++++--------- tests/NIT/parseconf-test.py | 75 ++++++++++++++++++++++++++++++++++--- 6 files changed, 140 insertions(+), 24 deletions(-) diff --git a/UPGRADING.adoc b/UPGRADING.adoc index 0485efadcb..ac4d932de7 100644 --- a/UPGRADING.adoc +++ b/UPGRADING.adoc @@ -33,6 +33,13 @@ Changes from 2.8.5 to 2.8.6 - PLANNED: Keep track of any further API clean-up? +- The common configuration and protocol parser now accepts literal `#` + characters inside double quotes. An unescaped `#` outside double quotes + still starts a comment. Existing `\#` escapes remain supported and are + still needed when sharing configuration files with older NUT versions. + Encoded protocol output continues to escape hashes for older peers; + default `upsc` text output is unchanged. [issues #607, #1306, PR #3633] + - Potentially a breaking change for C++ clients that rushed to use the new SSL support options in `libnutclient`: for Mozilla NSS setup, the way to provide an expected `CERTHOST` address was missing. Now the API is fixed diff --git a/common/parseconf.c b/common/parseconf.c index e0212bce6f..a46888f106 100644 --- a/common/parseconf.c +++ b/common/parseconf.c @@ -283,6 +283,16 @@ static int findeol(PCONF_CTX_t *ctx) return STATE_FINDEOL; } +#if 0 +/* Retained for future syntax errors; quoted hashes are now valid. */ +static void pconf_seterr(PCONF_CTX_t *ctx, const char *errmsg) +{ + snprintf(ctx->errmsg, PCONF_ERR_LEN, "%s", errmsg); + + ctx->error = 1; +} +#endif + /* quote characters inside a word bounded by "quotes" */ static int quotecollect(PCONF_CTX_t *ctx) { diff --git a/docs/config-notes.txt b/docs/config-notes.txt index 145cca14e7..a84a4a1d52 100644 --- a/docs/config-notes.txt +++ b/docs/config-notes.txt @@ -54,6 +54,10 @@ That is, you want to use something like this: SHUTDOWNCMD "/sbin/shutdown -h +0" +or, with each space escaped: + + SHUTDOWNCMD /sbin/shutdown\ -h\ +0 + Without quoting or escaping the spaces, a directive expecting one value would only see the first word of that value. diff --git a/docs/man/dummy-ups.txt b/docs/man/dummy-ups.txt index a6cbd78ebc..b2f1648b15 100644 --- a/docs/man/dummy-ups.txt +++ b/docs/man/dummy-ups.txt @@ -156,7 +156,13 @@ inside double quotes is literal. For example: outlet.4.desc: Outlet \#4 These values become `Outlet`, `Outlet #2`, `Outlet #3`, and `Outlet #4`, -respectively. Older versions of the parser require `\#` even inside +respectively. The last example has two value tokens, `Outlet` and `#4`, +which *dummy-ups* joins with a space. Escaping that space makes one token +with the same value: + + outlet.4.desc: Outlet\ \#4 + +Older versions of the parser require `\#` even inside double quotes. Backslashes quote the next permitted character once, so `\n` becomes `n`, while `\\` becomes a single literal backslash. A backslash followed by a physical newline continues the logical line. diff --git a/tests/NIT/nit.sh b/tests/NIT/nit.sh index b52a3476c5..f4307b11d7 100755 --- a/tests/NIT/nit.sh +++ b/tests/NIT/nit.sh @@ -24,6 +24,7 @@ # NUT_DEBUG_LEVEL_UPSSCHED=3 to set debug level for particular # NUT daemons or tools # NUT_PORT=12345 custom port for upsd to listen and clients to query +# NIT_PARSECONF_TIMEOUT=60 per-query watchdog seconds for parser tests # NUT_FOREGROUND_WITH_PID=true default foregrounding is without # PID files, this option tells daemons to save them # TESTDIR=/tmp/nut-NIT to propose a location for "etc" and "run" @@ -4728,20 +4729,50 @@ testgroup_sandbox_python() { sandbox_forget_configs } -testgroup_sandbox_parseconf() { - # Start a fresh sandbox so these punctuation fixtures do not change - # the model, timer and language-binding tests' expected data. +testcase_sandbox_parseconf() { if ! isTestablePython || [ -z "${PYTHON}" ] \ || ! $PYTHON -c 'import json' >/dev/null 2>&1 ; then - if [ x"${NIT_CASE}" = xtestgroup_sandbox_parseconf ]; then - log_error "[testgroup_sandbox_parseconf] Python with the json module is required" + case "${NIT_CASE}" in + testcase_sandbox_parseconf|testgroup_sandbox_parseconf) + log_error "[testcase_sandbox_parseconf] Python with the json module is required" FAILED="`expr $FAILED + 1`" - FAILED_FUNCS="$FAILED_FUNCS testgroup_sandbox_parseconf:missing-prerequisites" - else - log_warn "[testgroup_sandbox_parseconf] SKIPPED: Python with the json module is unavailable" + FAILED_FUNCS="$FAILED_FUNCS testcase_sandbox_parseconf:missing-prerequisites" + ;; + *) + log_warn "[testcase_sandbox_parseconf] SKIPPED: Python with the json module is unavailable" SKIPPED="`expr $SKIPPED + 1`" - SKIPPED_FUNCS="$SKIPPED_FUNCS testgroup_sandbox_parseconf" + SKIPPED_FUNCS="$SKIPPED_FUNCS testcase_sandbox_parseconf" + ;; + esac + return + fi + + log_separator + log_info "[testcase_sandbox_parseconf] Check exact text and decoded JSON values with $PYTHON" + if ( + # Match execcmd's per-client debug setting without changing later tests. + if [ -n "${NUT_DEBUG_LEVEL_UPSC-}" ]; then + NUT_DEBUG_LEVEL="${NUT_DEBUG_LEVEL_UPSC}" + export NUT_DEBUG_LEVEL fi + $PYTHON "${TOP_SRCDIR}/tests/NIT/parseconf-test.py" \ + "${TOP_BUILDDIR}/clients/upsc${EXEEXT}" "$NUT_PORT" + ) ; then + PASSED="`expr $PASSED + 1`" + log_info "[testcase_sandbox_parseconf] PASSED: exact text and decoded JSON values" + else + log_error "[testcase_sandbox_parseconf] FAILED: parser checks complained, check above" + FAILED="`expr $FAILED + 1`" + FAILED_FUNCS="$FAILED_FUNCS testcase_sandbox_parseconf" + fi +} + +testgroup_sandbox_parseconf() { + # Start a fresh sandbox so these punctuation fixtures do not change + # the model, timer and language-binding tests' expected data. + if ! isTestablePython || [ -z "${PYTHON}" ] \ + || ! $PYTHON -c 'import json' >/dev/null 2>&1 ; then + testcase_sandbox_parseconf return fi @@ -4751,15 +4782,8 @@ testgroup_sandbox_parseconf() { $PYTHON "${TOP_SRCDIR}/tests/NIT/parseconf-test.py" prepare "$NUT_CONFPATH" \ || die "[testgroup_sandbox_parseconf] Could not prepare parser fixtures" testcase_sandbox_start_drivers_after_upsd - - if $PYTHON "${TOP_SRCDIR}/tests/NIT/parseconf-test.py" \ - "${TOP_BUILDDIR}/clients/upsc${EXEEXT}" "$NUT_PORT" ; then - PASSED="`expr $PASSED + 1`" - log_info "[testgroup_sandbox_parseconf] PASSED: exact text and decoded JSON values" - else - FAILED="`expr $FAILED + 1`" - FAILED_FUNCS="$FAILED_FUNCS testgroup_sandbox_parseconf" - fi + testcase_sandbox_parseconf + log_separator sandbox_forget_configs } diff --git a/tests/NIT/parseconf-test.py b/tests/NIT/parseconf-test.py index 8aaf932d89..50057c5601 100644 --- a/tests/NIT/parseconf-test.py +++ b/tests/NIT/parseconf-test.py @@ -1,11 +1,13 @@ #!/usr/bin/env python # Exact values through configuration, dummy-ups, upsd and upsc. +# nit.sh selects $PYTHON; keep this script compatible with Python 2.6+ and 3. # Copyright (C) 2026 Network UPS Tools project # SPDX-License-Identifier: GPL-2.0-or-later from __future__ import print_function import json +import math import os import subprocess import sys @@ -26,6 +28,7 @@ ('""', None), # dummy-ups deletes variables assigned an empty value. ('"a\\\\#b"', 'a\\#b'), ('"a\\\\\\"b"', 'a\\"b'), + ('Outlet\\ \\#4', 'Outlet #4'), ] DESCRIPTION_INPUT = '"Parser #1: \\"quoted\\" C:\\\\ups\'s"' DESCRIPTION = 'Parser #1: "quoted" C:\\ups\'s' @@ -50,19 +53,42 @@ def prepare(confpath): def query(upsc, *args): - process = subprocess.Popen([upsc] + list(args), stdout=subprocess.PIPE, + timeout = float(os.environ.get('NIT_PARSECONF_TIMEOUT', '60')) + if timeout <= 0 or math.isnan(timeout) or math.isinf(timeout): + raise ValueError('NIT_PARSECONF_TIMEOUT must be positive finite seconds') + command = [upsc] + list(args) + print('START: %r (watchdog %g seconds)' % (command, timeout)) + sys.stdout.flush() + process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + expired = threading.Event() + + def expire(): + if process.poll() is None: + try: + process.kill() + except OSError: + # The child may have exited between poll() and kill(). + if process.poll() is None: + raise + else: + expired.set() + # Python 2.6 also runs NIT; communicate(timeout=...) is newer. - timer = threading.Timer(10, process.kill) + timer = threading.Timer(timeout, expire) timer.start() try: output, error = process.communicate() finally: timer.cancel() timer.join() + if expired.is_set(): + raise AssertionError('%r timed out after %g seconds; watchdog killed ' + 'the process: stdout=%r stderr=%r' % + (command, timeout, output, error)) if process.returncode: - raise AssertionError('upsc %r returned %d: %r' % - (args, process.returncode, error)) + raise AssertionError('%r returned %d: stdout=%r stderr=%r' % + (command, process.returncode, output, error)) # Fixtures are ASCII. Normalize only the platform's output line ending. return output.decode('ascii').replace('\r\n', '\n') @@ -72,6 +98,43 @@ def equal(actual, expected): raise AssertionError('got %r, expected %r' % (actual, expected)) +def self_test(): + original = os.environ.get('NIT_PARSECONF_TIMEOUT') + child = ('import sys, time; sys.stdout.write("output\\n"); ' + 'sys.stdout.flush(); sys.stderr.write("diagnostic\\n"); ' + 'sys.stderr.flush(); ') + try: + os.environ['NIT_PARSECONF_TIMEOUT'] = '5' + equal(query(sys.executable, '-c', child + 'time.sleep(2)'), 'output\n') + for action, timeout, message in [('sys.exit(7)', '5', 'returned 7'), + ('time.sleep(2)', '1', 'timed out after 1 seconds')]: + os.environ['NIT_PARSECONF_TIMEOUT'] = timeout + try: + query(sys.executable, '-c', child + action) + except AssertionError as error: + text = str(error) + if (message not in text + or 'stdout=%r' % b'output\n' not in text + or 'stderr=%r' % b'diagnostic\n' not in text): + raise + else: + raise AssertionError('expected query failure: ' + message) + for timeout in ['0', '-1', 'nan', 'inf']: + os.environ['NIT_PARSECONF_TIMEOUT'] = timeout + try: + query(sys.executable, '-c', 'pass') + except ValueError: + pass + else: + raise AssertionError('accepted invalid watchdog: ' + timeout) + finally: + if original is None: + del os.environ['NIT_PARSECONF_TIMEOUT'] + else: + os.environ['NIT_PARSECONF_TIMEOUT'] = original + print('PASS: query output, process failure, watchdog and timeout validation') + + def check(upsc, port): host = '127.0.0.1:' + port ups = 'dummy@' + host @@ -106,7 +169,9 @@ def check(upsc, port): if __name__ == '__main__': - if sys.argv[1] == 'prepare': + if sys.argv[1] == 'self-test': + self_test() + elif sys.argv[1] == 'prepare': prepare(sys.argv[2]) else: check(sys.argv[1], sys.argv[2])