From 366f367459a92498b71e02d332fd6434b29c6d3f Mon Sep 17 00:00:00 2001 From: user01010111 <12504630+user01010111@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:40:38 +1200 Subject: [PATCH 1/8] PyNUTClient: decode escaped descriptions in GetUPSList() Parse the quoted LIST UPS description without treating escaped quotes as field delimiters. Decode NUT escapes once while keeping dictionary keys and values as bytes and preserving GetUPSNames() conversion of names. Add deterministic regression coverage to make check, including escaped byte combinations, fragmented reads, consumers and existing errors. Reject malformed quoted descriptions with ValueError rather than silently truncating a description whose closing quote is missing. Closes: #3620 AI assistance: OpenAI Codex with gpt-6-astra (x-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 | 4 + scripts/python/module/Makefile.am | 7 +- scripts/python/module/PyNUT.py.in | 8 +- scripts/python/module/test_upslist.py | 186 ++++++++++++++++++++++++++ 4 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 scripts/python/module/test_upslist.py diff --git a/NEWS.adoc b/NEWS.adoc index 365c28d878..7ba3208c2c 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -494,6 +494,10 @@ https://github.com/networkupstools/nut/milestone/13 the overflows it addressed) in normal device interactions. [#3588] - NUT client libraries: + * `PyNUTClient`: fixed `GetUPSList()` rejecting UPS descriptions containing + escaped quotes. Description escapes are decoded once, preserving byte + keys and values and allowing `GetUPSNames()` to work with these entries. + [issue #3620] * Complete support for actions documented in `docs/net-protocol.txt` was implemented in C++, Python and PERL bindings in-tree, and for Java in link:https://github.com/networkupstools/jNut[jNut] nearby. Among diff --git a/scripts/python/module/Makefile.am b/scripts/python/module/Makefile.am index d171395ce6..0c10befdbd 100644 --- a/scripts/python/module/Makefile.am +++ b/scripts/python/module/Makefile.am @@ -14,13 +14,18 @@ all: PyNUTClient check-local: + @if test -n "$(PYTHON_DEFAULT)" && test "$(PYTHON_DEFAULT)" != no ; then \ + PYTHONPATH="$(builddir)" $(PYTHON_DEFAULT) "$(srcdir)/test_upslist.py"; \ + else \ + echo "SKIP: LIST UPS tests require a configured Python interpreter"; \ + fi @echo "You may want to set up a NUT data server and run 'make tox' here: `pwd`" # NOT tying into "make check" because a lot of stars must align for this test: tox: dist .pypi-tools-tox tox -EXTRA_DIST = tox.ini MANIFEST.in +EXTRA_DIST = tox.ini MANIFEST.in test_upslist.py NUT_SOURCE_GITREV_NUMERIC = @NUT_SOURCE_GITREV_NUMERIC@ PYTHON_DEFAULT = @PYTHON_DEFAULT@ diff --git a/scripts/python/module/PyNUT.py.in b/scripts/python/module/PyNUT.py.in index 387caa9941..f4e359bde7 100644 --- a/scripts/python/module/PyNUT.py.in +++ b/scripts/python/module/PyNUT.py.in @@ -1212,7 +1212,13 @@ which is of little concern for Python2 but is important in Python3 for line in result.split( b"\n" ) : if line[:3] == b"UPS" : - ups, desc = line[4:-1].split( b'"' ) + fields = re.match( b'([^"]*)"((?:[^"\\\\]|\\\\.)*)"$', line[4:] ) + if fields is None : + raise ValueError( "Invalid UPS list entry" ) + ups, desc = fields.groups() + # NUT escapes the next byte literally, including quotes, + # backslashes and '#'. Consume each escape exactly once. + desc = re.sub( b'\\\\(.)', b'\\1', desc ) ups_list[ ups.replace( b" ", b"" ) ] = desc return( ups_list ) diff --git a/scripts/python/module/test_upslist.py b/scripts/python/module/test_upslist.py new file mode 100644 index 0000000000..615551a785 --- /dev/null +++ b/scripts/python/module/test_upslist.py @@ -0,0 +1,186 @@ +#!/usr/bin/env python +# Test the byte contract of LIST UPS without a running server. +# SPDX-License-Identifier: GPL-3.0-or-later + +import itertools +import unittest + +import PyNUT + + +class RecordingSocket(object): + def __init__(self, chunks): + self.chunks = list(chunks) + self.sent = [] + + def sendall(self, data): + self.sent.append(data) + + def recv(self, size): + if not self.chunks: + return b'' + data = self.chunks.pop(0) + if len(data) > size: + self.chunks.insert(0, data[size:]) + return data[:size] + + def close(self): + pass + + +def listing(rows): + return b'BEGIN LIST UPS\n' + rows + b'END LIST UPS\n' + + +class UPSListTest(unittest.TestCase): + def client(self, chunks): + client = PyNUT.PyNUTClient(connect_now=False) + transport = RecordingSocket(chunks) + client._PyNUTClient__srv_handler = transport + return client, transport + + def check_description(self, encoded, expected): + response = listing(b'UPS dummy "' + encoded + b'"\n') + client, transport = self.client([response]) + result = client.GetUPSList() + self.assertEqual(result, {b'dummy': expected}) + self.assertEqual(type(list(result.keys())[0]), type(b'')) + self.assertEqual(type(result[b'dummy']), type(b'')) + self.assertEqual(transport.sent, [b'LIST UPS\n']) + + def test_ordinary(self): + for desc in [b'Office UPS', b'', b'UPS', b' Office UPS ']: + self.check_description(desc, desc) + + def test_quotes(self): + for encoded, expected in [ + (b'Office \\"Main\\" UPS', b'Office "Main" UPS'), + (b'\\"Office', b'"Office'), + (b'Office\\"', b'Office"'), + (b'\\"', b'"'), + (b'\\"\\"', b'""'), + ]: + self.check_description(encoded, expected) + + def test_backslashes_and_literals(self): + for encoded, expected in [ + (b'\\\\', b'\\'), + (b'\\\\\\\\', b'\\\\'), + (b'\\\\\\"', b'\\"'), + (b'\\"\\\\', b'"\\'), + (b'\\\\n \\\\t \\\\x41', b'\\n \\t \\x41'), + (b"Owner's \\#1 = UPS; $HOME `a`", b"Owner's #1 = UPS; $HOME `a`"), + (b'\\n', b'n'), + ]: + self.check_description(encoded, expected) + + def test_escape_combinations(self): + # Literal encoded/decoded atoms, including adjacent escape boundaries. + atoms = [(b' ', b' '), (b'\\"', b'"'), + (b'\\\\', b'\\'), (b'\\#', b'#')] + for size in range(1, 5): + for parts in itertools.product(atoms, repeat=size): + self.check_description(b''.join(p[0] for p in parts), + b''.join(p[1] for p in parts)) + + def test_bytes(self): + for value in range(0x20, 0x80): + byte = chr(value).encode('latin-1') + encoded = b'\\' + byte if byte in (b'"', b'\\', b'#') else byte + self.check_description(encoded, byte) + # Synthetic wire bytes: upsd's config parser discards these, but the + # client must not introduce a Unicode decoding policy for descriptions. + self.check_description(b'\x80\xff\\"\xc3\xa9', b'\x80\xff"\xc3\xa9') + + def test_multiple_entries_and_names(self): + response = listing(b'UPS UPS_1-a.b "Office \\"Main\\""\n' + b'UPS second ""\n') + client, transport = self.client([response, response]) + self.assertEqual(client.GetUPSList(), + {b'UPS_1-a.b': b'Office "Main"', b'second': b''}) + names = client.GetUPSNames() + self.assertEqual(type(names), list) + self.assertEqual(sorted(names), ['UPS_1-a.b', 'second']) + for name in names: + self.assertEqual(type(name), type(b''.decode('ascii'))) + self.assertEqual(transport.sent, [b'LIST UPS\n', b'LIST UPS\n']) + + def test_empty_list(self): + client, transport = self.client([listing(b''), listing(b'')]) + self.assertEqual(client.GetUPSList(), {}) + self.assertEqual(client.GetUPSNames(), []) + + def test_fragmentation(self): + response = listing(b'UPS dummy "A\\\\\\"B\\#C"\n') + expected = {b'dummy': b'A\\"B#C'} + for split in range(1, len(response)): + client, transport = self.client([response[:split], response[split:]]) + self.assertEqual(client.GetUPSList(), expected) + client, transport = self.client([response[i:i+1] for i in range(len(response))]) + self.assertEqual(client.GetUPSList(), expected) + + def test_coalesced_responses(self): + # First recv includes part of the next response; __read_until must + # retain it through both the header and the end-of-list reads. + response = listing(b'UPS d "\\""\n') + client, transport = self.client([response + listing(b'') + b'OK\n']) + self.assertEqual(client.GetUPSList(), {b'd': b'"'}) + self.assertEqual(client.GetUPSList(), {}) + self.assertEqual(client._PyNUTClient__read_until(b'\n'), b'OK\n') + self.assertEqual(transport.chunks, []) + + def test_consumers(self): + response = listing(b'UPS dummy "Office \\"Main\\""\n') + client, transport = self.client([response + b'OK\n' + response + + b'BEGIN LIST CLIENT dummy\nCLIENT dummy 127.0.0.1\nEND LIST CLIENT dummy\n']) + self.assertEqual(client.DeviceLogin('dummy'), 'OK') + self.assertEqual(client.ListClients('dummy'), {b'dummy': [b'127.0.0.1']}) + self.assertEqual(transport.sent, [b'LIST UPS\n', b'LOGIN dummy\n', + b'LIST UPS\n', b'LIST CLIENT dummy\n']) + + def test_list_clients_fallback(self): + response = listing(b'UPS dummy "\\""\n') + client, transport = self.client([response + b'ERR INVALID-ARGUMENT\n' + response + + b'BEGIN LIST CLIENT dummy\nEND LIST CLIENT dummy\n']) + self.assertEqual(client.ListClients(), {}) + + def test_unknown_ups(self): + response = listing(b'UPS dummy "\\""\n') + for method in ['DeviceLogin', 'ListClients']: + client, transport = self.client([response]) + try: + getattr(client, method)('missing') + except PyNUT.PyNUTError as exc: + self.assertEqual(str(exc), 'ERR UNKNOWN-UPS') + else: + self.fail('Expected PyNUTError') + self.assertEqual(transport.sent, [b'LIST UPS\n']) + + def test_server_error(self): + client, transport = self.client([b'ERR ACCESS-DENIED\n']) + try: + client.GetUPSList() + except PyNUT.PyNUTError as exc: + self.assertEqual(str(exc), 'ERR ACCESS-DENIED') + else: + self.fail('Expected PyNUTError') + + def test_eof(self): + for response in [b'', b'BEGIN LIST UPS\nUPS dummy "unfinished']: + client, transport = self.client([response]) + self.assertRaises(EOFError, client.GetUPSList) + + def test_malformed_rows(self): + for row in [b'UPS dummy unquoted\n', b'UPS dummy "raw "quote""\n', + b'UPS dummy "desc" "extra"\n', b'UPS dummy "missing close\n', + b'UPS dummy "dangling\\"\n']: + client, transport = self.client([listing(row)]) + self.assertRaises(ValueError, client.GetUPSList) + + def test_ignored_lines(self): + client, transport = self.client([listing(b'OTHER line\n\nUPS dummy "Office"\n')]) + self.assertEqual(client.GetUPSList(), {b'dummy': b'Office'}) + + +if __name__ == '__main__': + unittest.main() From bbbdc221bc2234479c56cad741ac7f3047930aee Mon Sep 17 00:00:00 2001 From: user01010111 <12504630+user01010111@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:47:19 +1200 Subject: [PATCH 2/8] PyNUT: use command identifiers in description requests Decode LIST CMD identifiers as ASCII for GET CMDDESC requests and response offsets, preserving the public bytes dictionary and description fallback. Add request-sensitive regression coverage to make check and note the fix in NEWS.adoc. Validated with Python 2.6, 2.7 and 3.4 through 3.14, localhost upsd with dummy-ups, generated/installed/packaged modules, make check, spellcheck and distcheck-light. AI assistance: OpenAI Codex with gpt-6-astra (x-high reasoning). The human contributor remains responsible for reviewing and submitting the change. Closes: #3621 Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com> --- NEWS.adoc | 4 + scripts/python/module/PyNUT.py.in | 5 +- tests/Makefile.am | 8 ++ tests/pynut-commands-test.py | 157 ++++++++++++++++++++++++++++++ 4 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 tests/pynut-commands-test.py diff --git a/NEWS.adoc b/NEWS.adoc index 7ba3208c2c..bdf0b7c966 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -42,6 +42,10 @@ https://github.com/networkupstools/nut/milestone/13 to patterns defined in link:docs/nut-names.txt[] - Fix fallout of development in NUT v2.8.0 through v2.8.5: + * PyNUT: `GetUPSCommands()` now requests command descriptions with the + advertised identifiers on Python 3 and uses those identifiers when + extracting the response. Returned keys and descriptions remain byte + sequences. [issue #3621] * `nut-scanner` tool updates: - Mutexes used in parallelized scans were not properly released on failure code paths (impacts likely all 2.8.x until now). [PR #3551] diff --git a/scripts/python/module/PyNUT.py.in b/scripts/python/module/PyNUT.py.in index f4e359bde7..ddab7c87f7 100644 --- a/scripts/python/module/PyNUT.py.in +++ b/scripts/python/module/PyNUT.py.in @@ -1306,12 +1306,13 @@ of the command as value # For each var we try to get the available description try : - self.__send( ("GET CMDDESC %s %s\n" % ( ups, var )).encode('ascii') ) + command = var.decode('ascii') + self.__send( ("GET CMDDESC %s %s\n" % ( ups, command )).encode('ascii') ) temp = self.__read_until( b"\n" ) if temp[:7] != b"CMDDESC" : raise PyNUTError else : - off = len( ("CMDDESC %s %s " % ( ups, var )).encode('ascii') ) + off = len( ("CMDDESC %s %s " % ( ups, command )).encode('ascii') ) desc = temp[off:-1].split(b'"')[1] except : desc = var diff --git a/tests/Makefile.am b/tests/Makefile.am index 1d21744fc1..6e4f1704f6 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -17,6 +17,7 @@ all: $(TESTS) $(check_PROGRAMS) $(check_SCRIPTS) EXTRA_DIST = become-user-root-warning-test.sh nut-driver-enumerator-test.sh nut-driver-enumerator-test--ups.conf EXTRA_DIST += cppunit-warnings.h cppunit-warnings-end.h +EXTRA_DIST += pynut-commands-test.py TESTS = noinst_LTLIBRARIES = @@ -347,6 +348,13 @@ memcheck: endif !HAVE_VALGRIND CHECK_LOCAL_TARGETS = become-user-root-warning-test +if HAVE_PYTHON_DEFAULT +CHECK_LOCAL_TARGETS += pynut-commands-test + +pynut-commands-test: + $(AM_V_at)PYTHONPATH='$(abs_top_builddir)/scripts/python/module' @PYTHON_DEFAULT@ $(srcdir)/pynut-commands-test.py +endif HAVE_PYTHON_DEFAULT + if WITH_VALGRIND CHECK_LOCAL_TARGETS += memcheck endif WITH_VALGRIND diff --git a/tests/pynut-commands-test.py b/tests/pynut-commands-test.py new file mode 100644 index 0000000000..dfedbc9541 --- /dev/null +++ b/tests/pynut-commands-test.py @@ -0,0 +1,157 @@ +# Regression checks for PyNUT command enumeration (no running server needed). +# Run with PYTHONPATH pointing to the configured scripts/python/module directory. + +import socket +import unittest + +import PyNUT + + +class TranscriptSocket(object): + """Only release a response after its exact request has been sent.""" + + def __init__(self, exchanges, chunk_size=50): + self.exchanges = exchanges + self.chunk_size = chunk_size + self.sent = [] + self.pending = b'' + self.read_error = None + + def sendall(self, request): + index = len(self.sent) + self.sent.append(request) + expected, response = self.exchanges[index] + if request != expected: + raise AssertionError('Expected %r, received %r' % (expected, request)) + if isinstance(response, Exception): + raise response + self.pending += response + + def recv(self, size): + if self.read_error is not None and not self.pending: + raise self.read_error + size = min(size, self.chunk_size) + result, self.pending = self.pending[:size], self.pending[size:] + return result + + +def command_list(names): + return (b'BEGIN LIST CMD dummy\n' + + b''.join([b'CMD dummy ' + name + b'\n' for name in names]) + + b'END LIST CMD dummy\n') + + +def description_reply(name, description): + return b'CMDDESC dummy ' + name + b' "' + description + b'"\n' + + +class CommandTests(unittest.TestCase): + def client(self, exchanges, chunk_size=50): + transport = TranscriptSocket(exchanges, chunk_size) + client = PyNUT.PyNUTClient(connect_now=False) + client._PyNUTClient__srv_handler = transport + return client, transport + + def assert_requests(self, transport): + # GetUPSCommands catches even AssertionError during CMDDESC, so check + # the complete transcript outside the client's exception handler too. + self.assertEqual(transport.sent, [pair[0] for pair in transport.exchanges]) + self.assertEqual(transport.pending, b'') + + def test_descriptions(self): + pairs = [ + (b'load.off', b'Turn off the load immediately'), + (b'test.panel.start', b'Start testing the UPS panel'), + (b'upstream.load.off', b'Turn off the load immediately'), + (b'outlet.1.load.off', b'Turn off this outlet'), + (b'driver.reload-or-error', b'Reload driver configuration'), + (b'test.panel.stop', b''), + (b'experimental.example', b'Description unavailable'), + ] + exchanges = [(b'LIST CMD dummy\n', command_list([p[0] for p in pairs]))] + for name, description in pairs: + exchanges.append((b'GET CMDDESC dummy ' + name + b'\n', + description_reply(name, description))) + for chunk_size in (1, 7, 50): + client, transport = self.client(exchanges, chunk_size) + commands = client.GetUPSCommands('dummy') + self.assert_requests(transport) + self.assertEqual(commands, dict(pairs)) + for name, description in commands.items(): + self.assertEqual(type(name), bytes) + self.assertEqual(type(description), bytes) + # NUT-Monitor's Qt variants sort the keys and decode both fields. + labels = ['%s\n%s' % (name.decode('ascii'), commands[name].decode('ascii')) + for name in sorted(commands.keys())] + self.assertTrue('test.panel.start\nStart testing the UPS panel' in labels) + + def test_description_fallbacks(self): + name = b'test.panel.start' + for response in (b'ERR CMD-NOT-SUPPORTED\n', b'ERR DATA-STALE\n', + b'WRONG response\n', b'CMDDESC\n', + b'CMDDESC dummy test.panel.start missing-quotes\n'): + exchanges = [ + (b'LIST CMD dummy\n', command_list([name, b'load.off'])), + (b'GET CMDDESC dummy test.panel.start\n', response), + (b'GET CMDDESC dummy load.off\n', + description_reply(b'load.off', b'Turn off the load immediately')), + ] + client, transport = self.client(exchanges) + self.assertEqual(client.GetUPSCommands('dummy'), + {name: name, b'load.off': b'Turn off the load immediately'}) + self.assert_requests(transport) + + def test_list_error(self): + client, transport = self.client([(b'LIST CMD dummy\n', b'ERR UNKNOWN-UPS\n')]) + try: + client.GetUPSCommands('dummy') + except PyNUT.PyNUTError as error: + self.assertEqual(str(error), 'ERR UNKNOWN-UPS') + else: + self.fail('LIST CMD error did not propagate') + self.assert_requests(transport) + + def test_list_transport_errors(self): + for response in (b'', b'BEGIN LIST CMD dummy\nCMD dummy load.off\n'): + client, transport = self.client([(b'LIST CMD dummy\n', response)]) + self.assertRaises(EOFError, client.GetUPSCommands, 'dummy') + self.assert_requests(transport) + client, transport = self.client([(b'LIST CMD dummy\n', b'')]) + transport.read_error = socket.timeout('read timed out') + self.assertRaises(socket.timeout, client.GetUPSCommands, 'dummy') + self.assert_requests(transport) + client, transport = self.client([(b'LIST CMD dummy\n', socket.error('send failed'))]) + self.assertRaises(socket.error, client.GetUPSCommands, 'dummy') + self.assert_requests(transport) + + def test_description_transport_errors(self): + name = b'load.off' + for response, read_error in ((b'', None), (b'', socket.timeout('read timed out')), + (socket.error('send failed'), None)): + exchanges = [ + (b'LIST CMD dummy\n', command_list([name])), + (b'GET CMDDESC dummy load.off\n', response), + ] + client, transport = self.client(exchanges) + transport.read_error = read_error + self.assertEqual(client.GetUPSCommands('dummy'), {name: name}) + self.assert_requests(transport) + + def test_leftover_response(self): + # A coalesced LIST header/body leaves buffered data after the first + # line. Repeated calls must consume it without losing response boundaries. + name = b'load.off' + exchanges = [ + (b'LIST CMD dummy\n', command_list([name])), + (b'GET CMDDESC dummy load.off\n', description_reply(name, b'Load off')), + ] * 2 + [(b'LIST UPS\n', b'BEGIN LIST UPS\nUPS dummy "Test device"\nEND LIST UPS\n')] + client, transport = self.client(exchanges) + for unused in range(2): + self.assertEqual(client.GetUPSCommands('dummy'), {name: b'Load off'}) + self.assertEqual(client.GetUPSList(), {b'dummy': b'Test device'}) + self.assert_requests(transport) + self.assertEqual(client._PyNUTClient__recv_leftover, b'') + + +if __name__ == '__main__': + unittest.main() From 72962ca07e7054d81efb72b56d9043da542a3288 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 3/8] PyNUTClient: decode NUT values and escape authentication arguments Share NUT token decoding across Python response and configuration readers, including INCLUDE paths, and encode literal authentication arguments. Escape server DESC and CMDDESC responses with pconf_encode. Preserve return types and existing response limits; document configuration compatibility. Add deterministic and localhost integration coverage to existing tests. Validated across Python 2.6, 2.7 and 3.4-3.14, package/install paths, real localhost upsd/dummy-ups, documentation checks and distcheck-light. Closes: #3620 AI assistance: OpenAI Codex with gpt-6-astra (x-high and 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 | 13 +- UPGRADING.adoc | 13 ++ docs/man/nutauth.conf.txt | 7 + docs/nut.dict | 5 +- scripts/python/module/Makefile.am | 8 +- scripts/python/module/PyNUT.py.in | 151 +++++++++----- scripts/python/module/README.adoc | 23 ++- scripts/python/module/test_protocol.py | 275 +++++++++++++++++++++++++ server/netget.c | 8 +- tests/NIT/nit.sh | 156 ++++++++++++++ 10 files changed, 594 insertions(+), 65 deletions(-) create mode 100644 scripts/python/module/test_protocol.py diff --git a/NEWS.adoc b/NEWS.adoc index bdf0b7c966..8957158c7f 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -498,10 +498,12 @@ https://github.com/networkupstools/nut/milestone/13 the overflows it addressed) in normal device interactions. [#3588] - NUT client libraries: - * `PyNUTClient`: fixed `GetUPSList()` rejecting UPS descriptions containing - escaped quotes. Description escapes are decoded once, preserving byte - keys and values and allowing `GetUPSNames()` to work with these entries. - [issue #3620] + * `PyNUTClient`: fixed parsing of escaped descriptions and values in + server replies, including `GetUPSList()` and its consumers. Shared + parsing also handles `nutauth.conf` values, comments and included + filenames using NUT syntax. Authentication arguments are escaped + before transmission. Existing response types are preserved; single + quotes in configuration values are now literal. [issue #3620, PR #3629] * Complete support for actions documented in `docs/net-protocol.txt` was implemented in C++, Python and PERL bindings in-tree, and for Java in link:https://github.com/networkupstools/jNut[jNut] nearby. Among @@ -614,6 +616,9 @@ https://github.com/networkupstools/nut/milestone/13 * Added a `clean_exit()` handler similar to that in `upsmon`. [PR #3499] - `upsd` data server updates: + * Escape variable and command descriptions from `cmdvartab` when sending + `DESC` and `CMDDESC` responses, so embedded quotes, backslashes and + hashes are decoded correctly by clients. [PR #3629] * If we hit "Too many open files" during configuration reload, close the oldest client connection and retry. [issue #3365] * If the `MAXCONN` requested in the configuration file exceeds the OS diff --git a/UPGRADING.adoc b/UPGRADING.adoc index 0485efadcb..6f750594a3 100644 --- a/UPGRADING.adoc +++ b/UPGRADING.adoc @@ -31,6 +31,19 @@ command line, in order to quickly pick up any other removed option flags. Changes from 2.8.5 to 2.8.6 --------------------------- +- `PyNUTClient` now decodes NUT escapes in server descriptions and values + exactly once. Applications which worked around the old behavior by + removing escape characters themselves should stop doing so. The existing + bytes/string return types are unchanged. [PR #3629] + +- Python's `nutauth.conf` reader now follows NUT quoting rules: single + quotes are literal characters, not string delimiters. Replace single + quotes used for grouping with double quotes, and quote or escape spaces + within values. Backslashes introduce literal characters rather than + being retained, and unquoted hashes begin comments. Pass unescaped + usernames and passwords to `PyNUTClient`; the client handles their wire + escaping. [PR #3629] + - PLANNED: Keep track of any further API clean-up? - Potentially a breaking change for C++ clients that rushed to use the new diff --git a/docs/man/nutauth.conf.txt b/docs/man/nutauth.conf.txt index 2cb2761e43..1c233ca54d 100644 --- a/docs/man/nutauth.conf.txt +++ b/docs/man/nutauth.conf.txt @@ -86,6 +86,13 @@ should be double-quoted and/or use escape sequences, like in other NUT files. Blank lines and characters after an un-quoted hash (`#`) are ignored. +Single quotes are literal characters; only double quotes group a value +containing spaces. A backslash escapes the following character once, +including a space, quote, backslash or hash. It does not introduce C or +Python escape sequences. These rules also apply to `INCLUDE` filenames. +For compatibility with older NUT parsers, escape a hash with a backslash +even inside double quotes. + Example: # Global defaults diff --git a/docs/nut.dict b/docs/nut.dict index 4ca96e1119..957cf1a810 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3827 utf-8 +personal_ws-1.1 en 3830 utf-8 AAC AAS ABI @@ -480,11 +480,14 @@ Gathman Geerling Gembe Gert +GetEnumList GetRWVars +GetRangeList GetUPSCommands GetUPSList GetUPSNames GetUPSVars +GetVariableDescription Ghali Giese Gigabit diff --git a/scripts/python/module/Makefile.am b/scripts/python/module/Makefile.am index 0c10befdbd..27300de621 100644 --- a/scripts/python/module/Makefile.am +++ b/scripts/python/module/Makefile.am @@ -15,9 +15,11 @@ all: PyNUTClient check-local: @if test -n "$(PYTHON_DEFAULT)" && test "$(PYTHON_DEFAULT)" != no ; then \ - PYTHONPATH="$(builddir)" $(PYTHON_DEFAULT) "$(srcdir)/test_upslist.py"; \ + for TEST in test_upslist.py test_protocol.py ; do \ + PYTHONPATH="$(builddir)" $(PYTHON_DEFAULT) "$(srcdir)/$$TEST" || exit $$? ; \ + done ; \ else \ - echo "SKIP: LIST UPS tests require a configured Python interpreter"; \ + echo "SKIP: PyNUT parser tests require a configured Python interpreter"; \ fi @echo "You may want to set up a NUT data server and run 'make tox' here: `pwd`" @@ -25,7 +27,7 @@ check-local: tox: dist .pypi-tools-tox tox -EXTRA_DIST = tox.ini MANIFEST.in test_upslist.py +EXTRA_DIST = tox.ini MANIFEST.in test_upslist.py test_protocol.py NUT_SOURCE_GITREV_NUMERIC = @NUT_SOURCE_GITREV_NUMERIC@ PYTHON_DEFAULT = @PYTHON_DEFAULT@ diff --git a/scripts/python/module/PyNUT.py.in b/scripts/python/module/PyNUT.py.in index ddab7c87f7..41763f55b0 100644 --- a/scripts/python/module/PyNUT.py.in +++ b/scripts/python/module/PyNUT.py.in @@ -75,6 +75,82 @@ import re import os import sys + +def _nut_parse(lines): + """Yield logical lines of NUT tokens, preserving the input string type. + + Like parseconf, quotes only open at a token boundary, single quotes + are literal, and a backslash consumes the next character exactly once. + Keep protocol bytes intact; this is not a Unicode or shell decoder. + """ + fields = [] + word = [] + started = quoted = escaped = comment = False + for line in lines: + empty = line[:0] + for index in range(len(line)): + char = line[index:index + 1] + code = ord(char) + if comment: + if code == 10: + yield fields + fields = [] + comment = False + continue + if escaped: + if code != 10: + word.append(char) + escaped = False + continue + if quoted: + if code == 34: + fields.append(empty.join(word)) + word = [] + started = quoted = False + elif code == 92: + escaped = True + elif code != 10: + word.append(char) + continue + if code == 92: + started = escaped = True + elif code == 34 and not started: + started = quoted = True + elif code in (9, 10, 11, 12, 13, 32, 35, 61): + if started: + fields.append(empty.join(word)) + word = [] + started = False + if code == 35: + comment = True + elif code == 61: + fields.append(char) + elif code == 10: + yield fields + fields = [] + else: + word.append(char) + started = True + if quoted or escaped: + raise ValueError("Incomplete NUT quoted string or escape") + if started: + fields.append(empty.join(word)) + if fields: + yield fields + + +def _nut_tokens(line): + """Parse a single NUT response line or field.""" + return next(_nut_parse([line]), []) + + +def _nut_quote(value): + """Encode one ASCII protocol argument, including for older NUT peers.""" + if any(ord(char) < 32 or ord(char) == 127 for char in value): + raise ValueError("Control character in NUT argument") + return '"' + re.sub(r'([\\\\"#])', r'\\\1', value) + '"' + + ssl_available = False try: import ssl @@ -281,16 +357,12 @@ class AuthConf: try: AuthConf.printDebug( "readAuthConfFile(): Reading NUT AuthConf data from '%s'" % (filename) ) with open(filename, 'r') as f: - for line in f: - line = line.strip() - if not line or line.startswith('#'): + for fields in _nut_parse(f): + if not fields: continue + line = fields[0] if line.startswith('['): - # Chomp any trailing comments: - if '#' in line: - line = line[:line.index('#')].strip() - if not line.endswith(']'): raise PyNUTError("Invalid section header in '%s': '%s'" % (filename, line)) @@ -321,30 +393,19 @@ class AuthConf: continue # INCLUDE support - m = re.match(r'^(INCLUDE(?:_REQUIRED)?)\s+(.*)$', line, re.I) - if m: - inc_type = m.group(1).upper() - inc_file = m.group(2).strip() - if (inc_file.startswith('"') and inc_file.endswith('"')) or \ - (inc_file.startswith("'") and inc_file.endswith("'")): - inc_file = inc_file[1:-1] + inc_type = fields[0].upper() + if inc_type in ('INCLUDE', 'INCLUDE_REQUIRED') and len(fields) >= 2: + inc_file = fields[1] is_required = (inc_type == "INCLUDE_REQUIRED") AuthConf.printDebug( "readAuthConfFile(): INCLUDE '%s'" % (inc_file) ) AuthConf.readAuthConfFile(inc_file, is_required, (current_ac is None or current_ac == AuthConf.__global_defaults)) continue - if '=' in line: - key, value = line.split('=', 1) - key = key.strip() - keyUC = key.strip().upper() - value = value.strip() - - # FIXME: NUT parseconf for possibly escaped values is a bit more complicated than this: - # Remove quotes if present - if (value.startswith('"') and value.endswith('"')) or \ - (value.startswith("'") and value.endswith("'")): - value = value[1:-1] + if len(fields) >= 2 and fields[1] == '=': + key = fields[0] + keyUC = key.upper() + value = fields[2] if len(fields) >= 3 else '' if current_ac is None: if global_scope: @@ -952,24 +1013,16 @@ if something goes wrong. self.__use_ssl = False if self.__login != None : - self.__send( ("USERNAME %s\n" % self.__login).encode('ascii') ) + self.__send( ("USERNAME %s\n" % _nut_quote(self.__login)).encode('ascii') ) result = self.__read_until( b"\n" ) if result[:2] != b"OK" : raise PyNUTError( result.replace( b"\n", b"" ).decode('ascii') ) if self.__password != None : - self.__send( ("PASSWORD %s\n" % self.__password).encode('ascii') ) + self.__send( ("PASSWORD %s\n" % _nut_quote(self.__password)).encode('ascii') ) result = self.__read_until( b"\n" ) if result[:2] != b"OK" : - if result == b"ERR INVALID-ARGUMENT\n" : - # Quote the password (if it has whitespace etc) - # TODO: Escape special chard like NUT does? - self.__send( ("PASSWORD \"%s\"\n" % self.__password).encode('ascii') ) - result = self.__read_until( b"\n" ) - if result[:2] != b"OK" : - raise PyNUTError( result.replace( b"\n", b"" ).decode('ascii') ) - else: - raise PyNUTError( result.replace( b"\n", b"" ).decode('ascii') ) + raise PyNUTError( result.replace( b"\n", b"" ).decode('ascii') ) # NOTE: no-op if "None" self.__tracking = self.SetTrackingMode(self.__tracking_wanted) @@ -1128,7 +1181,7 @@ if something goes wrong. if result[:4] == b"DESC" : # DESC "" off = len( ("DESC %s %s " % ( ups, var )).encode('ascii') ) - return result[off:-1].split(b'"')[1].decode('ascii') + return _nut_tokens(result[off:-1])[0].decode('ascii') else : raise PyNUTError( result.replace( b"\n", b"" ).decode('ascii') ) @@ -1149,7 +1202,7 @@ if something goes wrong. end_offset = 0 - ( len( ("END LIST ENUM %s %s\n" % ( ups, var )).encode('ascii') ) + 1 ) for current in result[:end_offset].split( b"\n" ) : - enum_list.append( current[offset:].split( b'"' )[1].decode('ascii') ) + enum_list.append( _nut_tokens(current[offset:])[0].decode('ascii') ) return enum_list @@ -1171,8 +1224,8 @@ if something goes wrong. for current in result[:end_offset].split( b"\n" ) : # RANGE "" "" - ranges = current[offset:].split( b'"' ) - range_list.append( { 'min' : ranges[1].decode('ascii'), 'max' : ranges[3].decode('ascii') } ) + ranges = _nut_tokens(current[offset:]) + range_list.append( { 'min' : ranges[0].decode('ascii'), 'max' : ranges[1].decode('ascii') } ) return range_list @@ -1212,13 +1265,11 @@ which is of little concern for Python2 but is important in Python3 for line in result.split( b"\n" ) : if line[:3] == b"UPS" : - fields = re.match( b'([^"]*)"((?:[^"\\\\]|\\\\.)*)"$', line[4:] ) + fields = re.match( b'([^"]*)("(?:[^"\\\\]|\\\\.)*")$', line[4:] ) if fields is None : raise ValueError( "Invalid UPS list entry" ) ups, desc = fields.groups() - # NUT escapes the next byte literally, including quotes, - # backslashes and '#'. Consume each escape exactly once. - desc = re.sub( b'\\\\(.)', b'\\1', desc ) + desc = _nut_tokens(desc)[0] ups_list[ ups.replace( b" ", b"" ) ] = desc return( ups_list ) @@ -1259,8 +1310,7 @@ available vars. end_offset = 0 - ( len( ("END LIST VAR %s\n" % ups).encode('ascii') ) + 1 ) for current in result[:end_offset].split( b"\n" ) : - var = current[ offset: ].split( b'"' )[0].replace( b" ", b"" ) - data = current[ offset: ].split( b'"' )[1] + var, data = _nut_tokens(current[offset:]) ups_vars[ var ] = data return( ups_vars ) @@ -1302,7 +1352,7 @@ of the command as value end_offset = 0 - ( len( ("END LIST CMD %s\n" % ups).encode('ascii') ) + 1 ) for current in result[:end_offset].split( b"\n" ) : - var = current[ offset: ].split( b'"' )[0].replace( b" ", b"" ) + var = _nut_tokens(current[offset:])[0] # For each var we try to get the available description try : @@ -1313,7 +1363,9 @@ of the command as value raise PyNUTError else : off = len( ("CMDDESC %s %s " % ( ups, command )).encode('ascii') ) - desc = temp[off:-1].split(b'"')[1] + if temp[off:off + 1] != b'"': + raise PyNUTError + desc = _nut_tokens(temp[off:-1])[0] except : desc = var @@ -1341,8 +1393,7 @@ The result is presented as a dictionary containing 'key->val' pairs try : for current in result[:end_offset].split( b"\n" ) : - var = current[ offset: ].split( b'"' )[0].replace( b" ", b"" ) - data = current[ offset: ].split( b'"' )[1] + var, data = _nut_tokens(current[offset:]) rw_vars[ var ] = data except : diff --git a/scripts/python/module/README.adoc b/scripts/python/module/README.adoc index b98740a6d6..4497ca961e 100644 --- a/scripts/python/module/README.adoc +++ b/scripts/python/module/README.adoc @@ -33,9 +33,10 @@ versions 2.7, 3.4, 3.5, 3.7, 3.11 and 3.13. [NOTE] ====== -Text fields returned by methods are byte sequences (not locale-aware -strings), except NUT protocol error codes quoted into `PyNUTError` -exceptions which are decoded into strings as originally `ascii` text. +The dictionaries returned by `GetUPSList()`, `GetUPSVars()`, `GetRWVars()` +and `GetUPSCommands()` contain byte keys and values (not locale-aware +strings). NUT protocol error codes in `PyNUTError` exceptions are decoded +into strings as originally `ascii` text. This is of little concern for Python 2, but is important in Python 3. The `ups` argument to methods is handled as a string, so conversion may @@ -46,8 +47,8 @@ strUps = bUps.decode('ascii') bUps = strUps.encode('ascii') ---- -Only the names returned by `GetUPSNames()` method specifically are -converted into string type. +`GetUPSNames()`, `GetVariableDescription()`, `GetEnumList()` and the bounds +returned by `GetRangeList()` use ASCII-decoded strings. Since Python 3 support was added just recently, module code may later be converted to use string types like `str` or `unicode` in the returned @@ -58,6 +59,18 @@ Examples below *do not* specify the `b'some text'` markup that would be pedantically correct (for Python 3 at least). ====== +Description and value readers decode NUT escaping exactly once. Callers +should not remove backslashes from the returned data themselves: a literal +backslash in a value remains a backslash, and is not a Python escape. +Pass ordinary ASCII strings for the constructor's `login` and `password` +arguments, without adding NUT quotes or escapes. The client encodes these +arguments for transmission; control characters are rejected. + +The `nutauth.conf` reader uses NUT syntax for values and `INCLUDE` filenames: +double quotes group tokens, single quotes are literal, unquoted hashes +begin comments, and backslashes escape the next character. See +linkman:nutauth.conf[5] for configuration examples. + .List of methods in the class ------ class PyNUTClient : diff --git a/scripts/python/module/test_protocol.py b/scripts/python/module/test_protocol.py new file mode 100644 index 0000000000..3b457ccadd --- /dev/null +++ b/scripts/python/module/test_protocol.py @@ -0,0 +1,275 @@ +#!/usr/bin/env python +# Test NUT protocol and configuration quoting without external services. +# SPDX-License-Identifier: GPL-3.0-or-later + +import os +import shutil +import tempfile +import unittest + +import PyNUT +from test_upslist import RecordingSocket + + +class ReplySocket(RecordingSocket): + """ Only supply a reply after its exact request has been observed. """ + def __init__(self, exchanges): + RecordingSocket.__init__(self, []) + self.exchanges = list(exchanges) + + def sendall(self, data): + self.sent.append(data) + if not self.exchanges: + raise AssertionError('Unexpected request: %r' % data) + request, reply = self.exchanges.pop(0) + if request != data: + raise AssertionError('Expected %r, received %r' % (request, data)) + self.chunks.append(reply) + + def send(self, data): + self.sendall(data) + return len(data) + + +class ProtocolTest(unittest.TestCase): + def client(self, exchanges): + client = PyNUT.PyNUTClient(connect_now=False) + transport = ReplySocket(exchanges) + client._PyNUTClient__srv_handler = transport + return client, transport + + def check_value(self, encoded, expected): + cases = [ + ('GetVariableDescription', ('dummy', 'ups.id'), + b'GET DESC dummy ups.id\n', + b'DESC dummy ups.id "' + encoded + b'"\n', + expected.decode('ascii')), + ('GetEnumList', ('dummy', 'ups.id'), + b'LIST ENUM dummy ups.id\n', + b'BEGIN LIST ENUM dummy ups.id\nENUM dummy ups.id "' + encoded + + b'"\nEND LIST ENUM dummy ups.id\n', [expected.decode('ascii')]), + ('GetUPSVars', ('dummy',), b'LIST VAR dummy\n', + b'BEGIN LIST VAR dummy\nVAR dummy ups.id "' + encoded + + b'"\nEND LIST VAR dummy\n', {b'ups.id': expected}), + ('GetRWVars', ('dummy',), b'LIST RW dummy\n', + b'BEGIN LIST RW dummy\nRW dummy ups.id "' + encoded + + b'"\nEND LIST RW dummy\n', {b'ups.id': expected}), + ] + for method, args, request, reply, wanted in cases: + client, transport = self.client([(request, reply)]) + result = getattr(client, method)(*args) + self.assertEqual(result, wanted, method) + if isinstance(result, dict): + self.assertEqual(type(list(result.keys())[0]), type(b'')) + self.assertEqual(type(result[b'ups.id']), type(b'')) + elif isinstance(result, list): + self.assertEqual(type(result[0]), type(b''.decode('ascii'))) + else: + self.assertEqual(type(result), type(b''.decode('ascii'))) + self.assertEqual(transport.exchanges, []) + + def test_ordinary_readers(self): + for value in [b'Office UPS', b'', b' spaced ']: + self.check_value(value, value) + + def test_escaped_readers(self): + for encoded, expected in [ + (b'Office \\"Main\\" \\#1', b'Office "Main" #1'), + (b'\\\\n \\\\t \\\\x41', b'\\n \\t \\x41'), + (b'\\\\\\"\\\\', b'\\"\\'), + (b'\\q', b'q'), + (b"Owner's #1 = UPS", b"Owner's #1 = UPS"), + ]: + self.check_value(encoded, expected) + + def test_command_descriptions(self): + client, transport = self.client([ + (b'LIST CMD dummy\n', b'BEGIN LIST CMD dummy\n' + b'CMD dummy test.panel.start\nCMD dummy test.panel.stop\n' + b'END LIST CMD dummy\n'), + (b'GET CMDDESC dummy test.panel.start\n', + b'CMDDESC dummy test.panel.start "Start \\"panel\\" \\#1 \\\\n"\n'), + (b'GET CMDDESC dummy test.panel.stop\n', + b'CMDDESC dummy test.panel.stop "Stop panel"\n'), + ]) + result = client.GetUPSCommands('dummy') + self.assertEqual(result, {b'test.panel.start': b'Start "panel" #1 \\n', + b'test.panel.stop': b'Stop panel'}) + for key, value in result.items(): + self.assertEqual(type(key), type(b'')) + self.assertEqual(type(value), type(b'')) + self.assertEqual(transport.exchanges, []) + + def test_command_description_fallback(self): + client, transport = self.client([ + (b'LIST CMD dummy\n', b'BEGIN LIST CMD dummy\n' + b'CMD dummy test.panel.start\nEND LIST CMD dummy\n'), + (b'GET CMDDESC dummy test.panel.start\n', b'ERR UNKNOWN-COMMAND\n'), + ]) + self.assertEqual(client.GetUPSCommands('dummy'), + {b'test.panel.start': b'test.panel.start'}) + self.assertEqual(transport.exchanges, []) + + def test_range_text(self): + client, transport = self.client([ + (b'LIST RANGE dummy battery.charge.low\n', + b'BEGIN LIST RANGE dummy battery.charge.low\n' + b'RANGE dummy battery.charge.low "001" "010.50"\n' + b'RANGE dummy battery.charge.low "20" "100"\n' + b'END LIST RANGE dummy battery.charge.low\n'), + ]) + result = client.GetRangeList('dummy', 'battery.charge.low') + self.assertEqual(result, [{'min': '001', 'max': '010.50'}, + {'min': '20', 'max': '100'}]) + for limits in result: + for value in limits.values(): + self.assertEqual(type(value), type(b''.decode('ascii'))) + + def test_server_errors(self): + for method, args, request in [ + ('GetVariableDescription', ('dummy', 'ups.id'), b'GET DESC dummy ups.id\n'), + ('GetEnumList', ('dummy', 'ups.id'), b'LIST ENUM dummy ups.id\n'), + ('GetRangeList', ('dummy', 'ups.id'), b'LIST RANGE dummy ups.id\n'), + ('GetUPSVars', ('dummy',), b'LIST VAR dummy\n'), + ('GetRWVars', ('dummy',), b'LIST RW dummy\n'), + ('GetUPSCommands', ('dummy',), b'LIST CMD dummy\n'), + ]: + client, transport = self.client([(request, b'ERR UNKNOWN-UPS\n')]) + try: + getattr(client, method)(*args) + except PyNUT.PyNUTError as exc: + self.assertEqual(str(exc), 'ERR UNKNOWN-UPS') + else: + self.fail('Expected PyNUTError from ' + method) + + +class AuthConfTest(unittest.TestCase): + def setUp(self): + PyNUT.AuthConf.freeAuthConfList() + PyNUT.AuthConf.setDebug(False) + self.directory = tempfile.mkdtemp(prefix='nut-authconf-') + + def tearDown(self): + PyNUT.AuthConf.freeAuthConfList() + shutil.rmtree(self.directory) + + def write_config(self, name, contents): + filename = os.path.join(self.directory, name) + with open(filename, 'w') as stream: + stream.write(contents) + return filename + + def read_config(self, contents): + filename = self.write_config('nutauth.conf', contents) + PyNUT.AuthConf.readAuthConfFile(filename, fatal_errors=True) + return PyNUT.AuthConf.getAuthConf(host='localhost', port=3493) + + def test_value_syntax(self): + for source, expected in [ + ('ordinary', 'ordinary'), ('""', ''), + ('"Office \\"Main\\" \\#1"', 'Office "Main" #1'), + ('"Owner\'s #1" # comment', "Owner's #1"), + ("'literal'", "'literal'"), + ('"\\\\n \\\\t \\\\x41"', '\\n \\t \\x41'), + ('unquoted\\ space', 'unquoted space'), + ('escaped\\#hash # comment', 'escaped#hash'), + ('"equals=value"', 'equals=value'), + ('left=right', 'left'), + ('"first"second', 'first'), + ('mid"quote', 'mid"quote'), + ('one\\\ntwo', 'onetwo'), + ('"one\\\ntwo"', 'onetwo'), + ]: + PyNUT.AuthConf.freeAuthConfList() + result = self.read_config('PASSWORD=' + source + '\n') + self.assertEqual(result.password, expected, source) + + def test_defaults_and_section_precedence(self): + result = self.read_config('USERNAME=global\nPASSWORD=global\n' + 'CERTPATH="/global path"\n[@localhost:3493] # host defaults\n' + 'PASSWORD=host\n[alice@localhost:3493]\nUSERNAME=ignored\n' + 'PASSWORD=user\n') + self.assertEqual(result.password, 'host') + result = PyNUT.AuthConf.getAuthConf(user='alice', host='localhost', port=3493) + self.assertEqual(result.user, 'alice') + self.assertEqual(result.password, 'user') + self.assertEqual(result.certpath, '/global path') + + def test_included_filenames(self): + self.write_config('include "q"#1\\part.conf', 'PASSWORD="from include"\n') + encoded = os.path.join(self.directory, 'include \\"q\\"\\#1\\\\part.conf') + result = self.read_config('INCLUDE_REQUIRED "' + encoded + '" # comment\n') + self.assertEqual(result.password, 'from include') + PyNUT.AuthConf.freeAuthConfList() + self.write_config("owner's file.conf", 'CERTPATH="included path"\n') + encoded = os.path.join(self.directory, "owner's\\ file.conf") + result = self.read_config('INCLUDE ' + encoded + '\nPASSWORD=local\n') + self.assertEqual(result.certpath, 'included path') + self.assertEqual(result.password, 'local') + + def test_optional_and_required_missing_include(self): + missing = os.path.join(self.directory, 'missing.conf') + result = self.read_config('INCLUDE "' + missing + '"\nPASSWORD=local\n') + self.assertEqual(result.password, 'local') + PyNUT.AuthConf.freeAuthConfList() + self.assertRaises(PyNUT.PyNUTError, self.read_config, + 'INCLUDE_REQUIRED "' + missing + '"\n') + + def connect(self, login, password, transport): + original = PyNUT.socket.create_connection + PyNUT.socket.create_connection = lambda *args, **kwargs: transport + try: + return PyNUT.PyNUTClient(login=login, password=password, + use_ssl=False, tracking=None) + finally: + PyNUT.socket.create_connection = original + + def test_authenticate_decoded_credentials(self): + result = self.read_config('USERNAME="Office \\"Main\\" \\#1"\n' + 'PASSWORD="path\\\\next \\"key\\" \\#2"\n') + transport = ReplySocket([ + (b'USERNAME "Office \\"Main\\" \\#1"\n', b'OK\n'), + (b'PASSWORD "path\\\\next \\"key\\" \\#2"\n', b'OK\n'), + ]) + client = self.connect(result.user, result.password, transport) + self.assertEqual(transport.exchanges, []) + client.disconnect() + + def test_authenticate_ordinary_and_errors(self): + transport = ReplySocket([(b'USERNAME "user"\n', b'ERR ACCESS-DENIED\n')]) + try: + self.connect('user', 'pass', transport) + except PyNUT.PyNUTError as exc: + self.assertEqual(str(exc), 'ERR ACCESS-DENIED') + else: + self.fail('Expected username error') + self.assertEqual(transport.exchanges, []) + for reply in [b'OK\n', b'ERR ACCESS-DENIED\n', b'ERR INVALID-ARGUMENT\n']: + transport = ReplySocket([ + (b'USERNAME "user"\n', b'OK\n'), + (b'PASSWORD "pass"\n', reply), + ]) + if reply == b'OK\n': + client = self.connect('user', 'pass', transport) + client.disconnect() + else: + try: + self.connect('user', 'pass', transport) + except PyNUT.PyNUTError as exc: + self.assertEqual(str(exc), reply[:-1].decode('ascii')) + else: + self.fail('Expected authentication error') + self.assertEqual(transport.exchanges, []) + + def test_credential_line_breaks_rejected(self): + # A line-oriented argument must not become more than one request. + for value in ['first\nsecond', 'first\rsecond']: + for login, password in [(value, 'pass'), ('user', value)]: + transport = RecordingSocket([b'OK\n', b'OK\n']) + self.assertRaises(ValueError, self.connect, login, password, transport) + self.assertEqual(transport.sent, + [] if login == value else [b'USERNAME "user"\n']) + + +if __name__ == '__main__': + unittest.main() diff --git a/server/netget.c b/server/netget.c index de22d96ace..55478f4ff5 100644 --- a/server/netget.c +++ b/server/netget.c @@ -71,6 +71,7 @@ static void get_desc(nut_ctype_t *client, const char *upsname, const char *var) const upstype_t *ups; const char *varptr; const char *desc; + char esc[PCONF_DEFAULT_WORDLEN_LIMIT * 2 + 1]; ups = get_ups_ptr(upsname); @@ -93,7 +94,8 @@ static void get_desc(nut_ctype_t *client, const char *upsname, const char *var) desc = desc_get_var(varptr); if (desc) - sendback(client, "DESC %s %s \"%s\"\n", upsname, var, desc); + sendback(client, "DESC %s %s \"%s\"\n", upsname, var, + pconf_encode(desc, esc, sizeof(esc))); else sendback(client, "DESC %s %s \"Description unavailable\"\n", upsname, var); } @@ -103,6 +105,7 @@ static void get_cmddesc(nut_ctype_t *client, const char *upsname, const char *cm const upstype_t *ups; const char *cmdptr; const char *desc; + char esc[PCONF_DEFAULT_WORDLEN_LIMIT * 2 + 1]; ups = get_ups_ptr(upsname); @@ -125,7 +128,8 @@ static void get_cmddesc(nut_ctype_t *client, const char *upsname, const char *cm desc = desc_get_cmd(cmdptr); if (desc) - sendback(client, "CMDDESC %s %s \"%s\"\n", upsname, cmd, desc); + sendback(client, "CMDDESC %s %s \"%s\"\n", upsname, cmd, + pconf_encode(desc, esc, sizeof(esc))); else sendback(client, "CMDDESC %s %s \"Description unavailable\"\n", upsname, cmd); diff --git a/tests/NIT/nit.sh b/tests/NIT/nit.sh index d24a612c23..f42ed87571 100755 --- a/tests/NIT/nit.sh +++ b/tests/NIT/nit.sh @@ -4095,6 +4095,74 @@ testcase_sandbox_python_with_upsmon_credentials() { fi } +testcase_sandbox_python_escaping() { + log_separator + log_info "[testcase_sandbox_python_escaping] Check escaped data and credentials with PyNUT" + if $PYTHON - "${TOP_BUILDDIR}/scripts/python/module" << 'PY' +import os +import sys + +sys.path.insert(0, sys.argv[1]) +import PyNUT + +port = int(os.environ['NUT_PORT']) +value = b'Rack\'s "A" #1 \\n' +text = value.decode('ascii') +client = PyNUT.PyNUTClient(host='127.0.0.1', port=port, timeout=3) +try: + assert client.GetUPSList() == {b'dummy': value, b'ordinary': b'Ordinary UPS'} + assert sorted(client.GetUPSNames()) == ['dummy', 'ordinary'] + for result in (client.GetUPSVars('dummy'), client.GetRWVars('dummy')): + assert result[b'ups.model'] == value, result + assert result[b'ups.mfr'] == b'Ordinary Manufacturer', result + assert all(isinstance(k, bytes) and isinstance(v, bytes) for k, v in result.items()) + description = client.GetVariableDescription('dummy', 'ups.model') + assert description == text and isinstance(description, type(text)), description + assert client.GetVariableDescription('dummy', 'ups.mfr') == 'Ordinary variable' + commands = client.GetUPSCommands('dummy') + assert commands[b'load.off'] == value, commands + assert all(isinstance(k, bytes) and isinstance(v, bytes) for k, v in commands.items()) +finally: + client.disconnect() + +PyNUT.AuthConf.freeAuthConfList() +PyNUT.AuthConf.readAuthConfFile(os.path.join(os.environ['NUT_CONFPATH'], 'escape-auth.conf'), fatal_errors=True) +auth = PyNUT.AuthConf.getAuthConf(host='127.0.0.1', port=port) +assert auth.user == "nit'escape" and auth.password == text +client = PyNUT.PyNUTClient.from_authconf(auth, timeout=3) +try: + # USERNAME/PASSWORD alone acknowledge receipt; LOGIN verifies the credentials. + assert client.DeviceLogin('dummy') == 'OK' +finally: + client.disconnect() + +client = PyNUT.PyNUTClient(host='127.0.0.1', port=port, login='nit-plain', password='ordinary', timeout=3) +try: + assert client.DeviceLogin('ordinary') == 'OK' +finally: + client.disconnect() + +client = PyNUT.PyNUTClient(host='127.0.0.1', port=port, login=auth.user, password='wrong', timeout=3) +try: + try: + client.DeviceLogin('dummy') + except PyNUT.PyNUTError as error: + assert str(error) == 'ERR ACCESS-DENIED', str(error) + else: + raise AssertionError('Incorrect password was accepted') +finally: + client.disconnect() +print('Escaped values, descriptions, INCLUDE credentials and authentication passed') +PY + then + PASSED="`expr $PASSED + 1`" + else + log_error "[testcase_sandbox_python_escaping] Error: PyNUT regression check failed" + FAILED="`expr $FAILED + 1`" + FAILED_FUNCS="$FAILED_FUNCS testcase_sandbox_python_escaping" + fi +} + testcases_sandbox_python() { isTestablePython && [ -n "${PYTHON}" ] || { SKIPPED_FUNCS="${SKIPPED_FUNCS} testcase_sandbox_python_without_credentials testcase_sandbox_python_with_credentials testcase_sandbox_python_with_upsmon_credentials" @@ -4717,6 +4785,7 @@ testgroup_sandbox() { log_separator sandbox_forget_configs + testgroup_sandbox_python_escaping } testgroup_sandbox_python() { @@ -4726,6 +4795,93 @@ testgroup_sandbox_python() { log_separator sandbox_forget_configs + testgroup_sandbox_python_escaping +} + +testgroup_sandbox_python_escaping() { + isTestablePython && [ -n "${PYTHON}" ] || { + SKIPPED_FUNCS="$SKIPPED_FUNCS testcase_sandbox_python_escaping" + SKIPPED="`expr $SKIPPED + 1`" + return 0 + } + + # This fixture replaces the ordinary sandbox only after its other tests finish. + stop_daemons + generatecfg_upsd_trivial + generatecfg_ups_trivial + printf 'DATAPATH "%s"\n' "$NUT_CONFPATH" >> "$NUT_CONFPATH/upsd.conf" \ + || die "Failed to configure the temporary cmdvartab path" + cat >> "$NUT_CONFPATH/ups.conf" << 'EOF' +[dummy] + driver = dummy-ups + desc = "Rack's \"A\" \#1 \\n" + port = escaping.dev + mode = dummy-once +[ordinary] + driver = dummy-ups + desc = "Ordinary UPS" + port = escaping.dev + mode = dummy-once +EOF + [ $? = 0 ] || die "Failed to populate escaping ups.conf" + cat > "$NUT_CONFPATH/escaping.dev" << 'EOF' +ups.status: OL +ups.mfr: Ordinary Manufacturer +ups.model: "Rack's \"A\" \#1 \\n" +EOF + [ $? = 0 ] || die "Failed to populate escaping dummy data" + cat > "$NUT_CONFPATH/cmdvartab" << 'EOF' +VARDESC ups.model "Rack's \"A\" \#1 \\n" +VARDESC ups.mfr "Ordinary variable" +CMDDESC load.off "Rack's \"A\" \#1 \\n" +EOF + [ $? = 0 ] || die "Failed to populate escaping cmdvartab" + cat > "$NUT_CONFPATH/upsd.users" << 'EOF' +[nit'escape] + password = "Rack's \"A\" \#1 \\n" + upsmon secondary +[nit-plain] + password = ordinary + upsmon secondary +EOF + [ $? = 0 ] || die "Failed to populate escaping upsd.users" + printf 'INCLUDE_REQUIRED "%s/escape credentials.conf"\n' "$NUT_CONFPATH" \ + > "$NUT_CONFPATH/escape-auth.conf" || die "Failed to populate escaping INCLUDE" + printf '[@127.0.0.1:%s]\n' "$NUT_PORT" > "$NUT_CONFPATH/escape credentials.conf" \ + || die "Failed to populate escaping authconf section" + cat >> "$NUT_CONFPATH/escape credentials.conf" << 'EOF' +USERNAME = nit'escape +PASSWORD = "Rack's \"A\" \#1 \\n" +EOF + [ $? = 0 ] || die "Failed to populate escaping credentials" + if $I_AM_ROOT ; then + chmod 644 "$NUT_CONFPATH/upsd.users" + else + chmod 640 "$NUT_CONFPATH/upsd.users" + fi + chmod 644 "$NUT_CONFPATH/escaping.dev" "$NUT_CONFPATH/cmdvartab" + chmod 600 "$NUT_CONFPATH/escape-auth.conf" "$NUT_CONFPATH/escape credentials.conf" + + SANDBOX_CONFIG_GENERATED=true + sandbox_start_upsd || die "Failed to start escaping sandbox upsd" + execcmd dummy-ups -a dummy ${ARG_USER} ${ARG_FG} & + PID_DUMMYUPS="$!" + execcmd dummy-ups -a ordinary ${ARG_USER} ${ARG_FG} & + PID_DUMMYUPS1="$!" + COUNTDOWN=20 + while [ "$COUNTDOWN" -gt 0 ]; do + if runcmd upsc -A none -W 2 dummy@localhost:$NUT_PORT ups.mfr \ + && [ x"$CMDOUT" = x"Ordinary Manufacturer" ]; then + break + fi + sleep 1 + COUNTDOWN="`expr $COUNTDOWN - 1`" + done + [ "$COUNTDOWN" -gt 0 ] || die "Escaping dummy driver did not become ready" + + testcase_sandbox_python_escaping + log_separator + sandbox_forget_configs } testgroup_sandbox_perl() { From af77624f9d518a9bf449714f0e4fa6a5a251e6e3 Mon Sep 17 00:00:00 2001 From: user01010111 <12504630+user01010111@users.noreply.github.com> Date: Wed, 9 Sep 2026 13:44:07 +1200 Subject: [PATCH 4/8] PyNUTClient tests: make INCLUDE fixtures portable on Windows Use portable real filenames and escape complete paths in INCLUDE fixtures. Check quote, hash and backslash decoding separately through the AuthConf reader without creating a Windows-invalid filename. Preserve production behavior and existing escape coverage. Validated with all 36 tests across the documented macOS/Linux Python matrix and Windows CPython 3.13.7 under Wine, including a separate shared-test run with spaces and a hash in the temporary parent path. Build, make check, spelling/style and distcheck-light pass. Wine does not establish exact AppVeyor-image or Windows-kernel acceptance. Related: #3629 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> --- scripts/python/module/test_protocol.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/scripts/python/module/test_protocol.py b/scripts/python/module/test_protocol.py index 3b457ccadd..21f9e0ae11 100644 --- a/scripts/python/module/test_protocol.py +++ b/scripts/python/module/test_protocol.py @@ -164,6 +164,10 @@ def read_config(self, contents): PyNUT.AuthConf.readAuthConfFile(filename, fatal_errors=True) return PyNUT.AuthConf.getAuthConf(host='localhost', port=3493) + def config_path(self, name): + return os.path.join(self.directory, name).replace('\\', '\\\\').replace( + '"', '\\"').replace('#', '\\#') + def test_value_syntax(self): for source, expected in [ ('ordinary', 'ordinary'), ('""', ''), @@ -196,19 +200,33 @@ def test_defaults_and_section_precedence(self): self.assertEqual(result.certpath, '/global path') def test_included_filenames(self): - self.write_config('include "q"#1\\part.conf', 'PASSWORD="from include"\n') - encoded = os.path.join(self.directory, 'include \\"q\\"\\#1\\\\part.conf') + self.write_config('include #1.conf', 'PASSWORD="from include"\n') + encoded = self.config_path('include #1.conf') result = self.read_config('INCLUDE_REQUIRED "' + encoded + '" # comment\n') self.assertEqual(result.password, 'from include') PyNUT.AuthConf.freeAuthConfList() self.write_config("owner's file.conf", 'CERTPATH="included path"\n') - encoded = os.path.join(self.directory, "owner's\\ file.conf") + encoded = self.config_path("owner's file.conf").replace(' ', '\\ ') result = self.read_config('INCLUDE ' + encoded + '\nPASSWORD=local\n') self.assertEqual(result.certpath, 'included path') self.assertEqual(result.password, 'local') + def test_included_filename_decoding(self): + # Record the include path without creating a Windows-invalid filename. + filename = self.write_config('nutauth.conf', + 'INCLUDE_REQUIRED "include \\"q\\"\\#1\\\\part.conf" # comment\n') + original = PyNUT.AuthConf.readAuthConfFile + included = [] + try: + PyNUT.AuthConf.readAuthConfFile = staticmethod( + lambda name, required, *args: included.append((name, required))) + original(filename, fatal_errors=True) + finally: + PyNUT.AuthConf.readAuthConfFile = staticmethod(original) + self.assertEqual(included, [('include "q"#1\\part.conf', True)]) + def test_optional_and_required_missing_include(self): - missing = os.path.join(self.directory, 'missing.conf') + missing = self.config_path('missing.conf') result = self.read_config('INCLUDE "' + missing + '"\nPASSWORD=local\n') self.assertEqual(result.password, 'local') PyNUT.AuthConf.freeAuthConfList() From 40532f0f92ee8fc3bf964aa3853c5b4d7f709187 Mon Sep 17 00:00:00 2001 From: user01010111 <12504630+user01010111@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:41:32 +1200 Subject: [PATCH 5/8] NIT: use native paths in Python escaping fixtures Convert the escaping fixture directory with cygpath -m when available, then escape it for NUT configuration syntax. Use the resulting path in both DATAPATH and INCLUDE_REQUIRED so native Windows upsd can read the fixtures. Fail explicitly if conversion or escaping fails. Validated with deterministic path/parser checks across sh, bash and zsh; localhost upsd/dummy-ups on macOS and Linux; and native Windows binaries under Wine. The Wine tests model the documented cygpath mapping and do not establish exact AppVeyor/MSYS2-image or Windows-kernel acceptance. All 36 Python tests and 12 Automake tests pass, as do build, style, spelling and distcheck-light checks. Related: #3629 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 | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/NIT/nit.sh b/tests/NIT/nit.sh index f42ed87571..4028199828 100755 --- a/tests/NIT/nit.sh +++ b/tests/NIT/nit.sh @@ -4809,7 +4809,16 @@ testgroup_sandbox_python_escaping() { stop_daemons generatecfg_upsd_trivial generatecfg_ups_trivial - printf 'DATAPATH "%s"\n' "$NUT_CONFPATH" >> "$NUT_CONFPATH/upsd.conf" \ + # MSYS converts process arguments, but not paths written into config files. + ESCAPING_CONFPATH="$NUT_CONFPATH" + if command -v cygpath >/dev/null 2>&1 ; then + ESCAPING_CONFPATH="$(cygpath -m "$NUT_CONFPATH")" \ + || die "Failed to convert the escaping fixture path" + fi + ESCAPING_CONFPATH="$(printf '%s' "$ESCAPING_CONFPATH" | \ + sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' -e 's/#/\\#/g')" \ + || die "Failed to escape the fixture path for NUT configuration" + printf 'DATAPATH "%s"\n' "$ESCAPING_CONFPATH" >> "$NUT_CONFPATH/upsd.conf" \ || die "Failed to configure the temporary cmdvartab path" cat >> "$NUT_CONFPATH/ups.conf" << 'EOF' [dummy] @@ -4845,7 +4854,7 @@ EOF upsmon secondary EOF [ $? = 0 ] || die "Failed to populate escaping upsd.users" - printf 'INCLUDE_REQUIRED "%s/escape credentials.conf"\n' "$NUT_CONFPATH" \ + printf 'INCLUDE_REQUIRED "%s/escape credentials.conf"\n' "$ESCAPING_CONFPATH" \ > "$NUT_CONFPATH/escape-auth.conf" || die "Failed to populate escaping INCLUDE" printf '[@127.0.0.1:%s]\n' "$NUT_PORT" > "$NUT_CONFPATH/escape credentials.conf" \ || die "Failed to populate escaping authconf section" From 694afac5e27b6970910584b88fd870686ac68bdf Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 14 Sep 2026 13:19:33 +0200 Subject: [PATCH 6/8] scripts/python/module/test_protocol.py: consider a cleanly closed socket [#3627, #3629] Signed-off-by: Jim Klimov --- scripts/python/module/test_protocol.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/python/module/test_protocol.py b/scripts/python/module/test_protocol.py index 21f9e0ae11..ae0cc1e739 100644 --- a/scripts/python/module/test_protocol.py +++ b/scripts/python/module/test_protocol.py @@ -285,9 +285,14 @@ def test_credential_line_breaks_rejected(self): for login, password in [(value, 'pass'), ('user', value)]: transport = RecordingSocket([b'OK\n', b'OK\n']) self.assertRaises(ValueError, self.connect, login, password, transport) - self.assertEqual(transport.sent, + try: + # Did we also handle socket disconnection correctly? + self.assertEqual(transport.sent, + [b'LOGOUT\n'] if login == value else [b'USERNAME "user"\n', b'LOGOUT\n']) + except AssertionError as ignored: + # If the check below fails, we get a log with the transport.sent[] list: + self.assertEqual(transport.sent, [] if login == value else [b'USERNAME "user"\n']) - if __name__ == '__main__': unittest.main() From 953baad2a6fb8a6035673e465b6d687037385943 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 14 Sep 2026 13:22:58 +0200 Subject: [PATCH 7/8] scripts/python/module/test_nutclient.py.in: consider USERNAME sent double-quoted [#3629] Signed-off-by: Jim Klimov --- scripts/python/module/test_nutclient.py.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/python/module/test_nutclient.py.in b/scripts/python/module/test_nutclient.py.in index 69ad5ca10d..b6c8dab0a2 100755 --- a/scripts/python/module/test_nutclient.py.in +++ b/scripts/python/module/test_nutclient.py.in @@ -124,7 +124,7 @@ if HAVE_UNITTEST: socket.create_connection = lambda *args: sock self.assertRaises(PyNUT.PyNUTError, client.__init__, login="test") client.__del__() - self.assertEqual(sock.events, [b"USERNAME test\n", b"LOGOUT\n", "close"]) + self.assertEqual(sock.events, [b"USERNAME \"test\"\n", b"LOGOUT\n", "close"]) self.assertEqual(client._PyNUTClient__srv_handler, None) finally: socket.create_connection = create_connection From 5131bdb6f889245a1cf549213552eff4b82803ef Mon Sep 17 00:00:00 2001 From: user01010111 <12504630+user01010111@users.noreply.github.com> Date: Mon, 14 Sep 2026 16:46:56 +0000 Subject: [PATCH 8/8] docs: add possessive forms to spellcheck dictionary Add firmware's and kernel's, which Aspell rejects in existing documentation, and update the dictionary word count. Validated with the normal spelling and distcheck-light gates. AI assistance: OpenAI Codex with gpt-5.6-sol and gpt-6-astra. Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com> --- docs/nut.dict | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/nut.dict b/docs/nut.dict index 437911ea4f..69e271094a 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3831 utf-8 +personal_ws-1.1 en 3836 utf-8 AAC AAS ABI @@ -2228,6 +2228,7 @@ filenames filesystem filesystems firewalling +firmware's firmwares fixNSS fmt @@ -2514,6 +2515,7 @@ kVA kadets kaminski kde +kernel's kex kext keychain