From de46b0bf6bc995678dd688ed1dd69f10ddb0595e Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 17 Aug 2025 16:18:53 -0400 Subject: [PATCH 001/290] close branch From bb19835178949bb40bc47e30917b812c969f93dd Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 17 Aug 2025 16:35:46 -0400 Subject: [PATCH 002/290] close duplicate head caused by identical merge in two working copies From 13134bd4631ff1f97372177cfb906504d98dd1c4 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 17 Aug 2025 16:44:22 -0400 Subject: [PATCH 003/290] document the repo cleanups and duplicate merge to get things cleaned up --- CHANGES.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index 122e8352..b181e066 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -23,6 +23,9 @@ Fixed: roundup.cgi.exceptions. Also it now inherits from HTTPException rather than Exception since it is an HTTP exception. (John Rouillard) +- cleaned up repo. Close obsolete branches and close a split head due + to an identical merge intwo different workicng copies. (John + Rouillard) Features: From 28b5642f812659cf5de1051a741c05f99ceebe7b Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Tue, 19 Aug 2025 22:32:46 -0400 Subject: [PATCH 004/290] feat: add support for using dictConfig to configure logging. Basic logging config (one level and one output file non-rotating) was always possible from config.ini. However the LOGGING_CONFIG setting could be used to load an ini fileConfig style file to set various channels (e.g. roundup.hyperdb) (also called qualname or tags) with their own logging level, destination (rotating file, socket, /dev/null) and log format. This is now a deprecated method in newer logging modules. The dictConfig format is preferred and allows disabiling other loggers as well as invoking new loggers in local code. This commit adds support for it reading the dict from a .json file. It also implements a comment convention so you can document the dictConfig. configuration.py: new code test_config.py: test added for the new code. admin_guide.txt, upgrading.txt CHANGES.txt: docs added upgrading references the section in admin_guid. --- CHANGES.txt | 2 + doc/admin_guide.txt | 275 +++++++++++++++++++++++++++++++++++++-- doc/upgrading.txt | 15 +++ roundup/configuration.py | 95 +++++++++++++- test/test_config.py | 195 +++++++++++++++++++++++++++ 5 files changed, 570 insertions(+), 12 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index b181e066..ac2c5422 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -32,6 +32,8 @@ Features: - add support for authorized changes. User can be prompted to enter their password to authorize a change. If the user's password is properly entered, the change is committed. (John Rouillard) +- add support for dictConfig style logging configuration. Ini/File + style configs will still be supported. (John Rouillard) 2025-07-13 2.5.0 diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index 538ad8fb..42e73e29 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -47,31 +47,284 @@ There's two "installations" that we talk about when using Roundup: in the tracker's config.ini. -Configuring Roundup's Logging of Messages For Sysadmins -======================================================= +Configuring Roundup Message Logging +=================================== -You may configure where Roundup logs messages in your tracker's config.ini -file. Roundup will use the standard Python (2.3+) logging implementation. +You can control how Roundup logs messages using your tracker's +config.ini file. Roundup uses the standard Python (2.3+) logging +implementation. The config file and ``roundup-server`` provide very +basic control over logging. -Configuration for standard "logging" module: - - tracker configuration file specifies the location of a logging - configration file as ``logging`` -> ``config`` - - ``roundup-server`` specifies the location of a logging configuration - file on the command line Configuration for "BasicLogging" implementation: - tracker configuration file specifies the location of a log file ``logging`` -> ``filename`` - tracker configuration file specifies the level to log to as ``logging`` -> ``level`` + - tracker configuration file lets you disable other loggers + (e.g. when running under a wsgi framework) with + ``logging`` -> ``disable_loggers``. - ``roundup-server`` specifies the location of a log file on the command line - - ``roundup-server`` specifies the level to log to on the command line + - ``roundup-server`` enable using the standard python logger with + the tag/channel ``roundup.http`` on the command line + +By supplying a standard log config file in ini or json (dictionary) +format, you get more control over the logs. You can set different +levels for logs (e.g. roundup.hyperdb can be set to WARNING while +other Roundup log channels are set to INFO and roundup.mailgw logs at +DEBUG level). You can also send the logs for roundup.mailgw to syslog, +and other roundup logs go to an automatically rotating log file, or +are submitted to your log aggregator over https. -(``roundup-mailgw`` always logs to the tracker's log file) +Configuration for standard "logging" module: + - tracker configuration file specifies the location of a logging + configuration file as ``logging`` -> ``config``. In both cases, if no logfile is specified then logging will simply be sent to sys.stderr with only logging of ERROR messages. +Standard Logging Setup +---------------------- + +You can specify your log configs in one of two formats: + + * `fileConfig format + `_ + in ini style + * `dictConfig format + `_ + using json with comment support + +The dictConfig allows more control over configuration including +loading your own log handlers and disabling existing handlers. If you +use the fileConfig format, the ``logging`` -> ``disable_loggers`` flag +in the tracker's config is used to enable/disable pre-existing loggers +as there is no way to do this in the logging config file. + +.. _`dictLogConfig`: + +dictConfig Based Logging Config +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +dictConfigs are specified in JSON format with support for comments. +The file name in the tracker's config for the ``logging`` -> ``config`` +setting must end with ``.json`` to choose the correct processing. + +Comments have to be in one of two forms: + +1. A ``#`` with preceding white space is considered a comment and is + stripped from the file before being passed to the json parser. This + is a "block comment". + +2. A ``#`` preceded by at least three + white space characters is stripped from the end of the line before + begin passed to the json parser. This is an "inline comment". + +Other than this the file is a standard json file that matches the +`Configuration dictionary schema +`_ +defined in the Python documentation. + + +Example dictConfig Logging Config +................................. + +Note that this file is not actually JSON format as it include comments. +So you can not use tools that expect JSON (linters, formatters) to +work with it. + +The config below works with the `Waitress wsgi server +`_ configured to use the +roundup.wsgi channel. It also controls the `TransLogger middleware +`_ configured to use +roundup.wsgi.translogger, to produce httpd style combined logs. The +log file is specified relative to the current working directory not +the tracker home. The tracker home is the subdirectory demo under the +current working directory. The commented config is:: + + { + "version": 1, # only supported version + "disable_existing_loggers": false, # keep the wsgi loggers + + "formatters": { + # standard format for Roundup messages + "standard": { + "format": "%(asctime)s %(levelname)s %(name)s:%(module)s %(msg)s" + }, + # used for waitress wsgi server to produce httpd style logs + "http": { + "format": "%(message)s" + } + }, + "handlers": { + # create an access.log style http log file + "access": { + "level": "INFO", + "formatter": "http", + "class": "logging.FileHandler", + "filename": "demo/access.log" + }, + # logging for roundup.* loggers + "roundup": { + "level": "DEBUG", + "formatter": "standard", + "class": "logging.FileHandler", + "filename": "demo/roundup.log" + }, + # print to stdout - fall through for other logging + "default": { + "level": "DEBUG", + "formatter": "standard", + "class": "logging.StreamHandler", + "stream": "ext://sys.stdout" + } + }, + "loggers": { + "": { + "handlers": [ + "default" + ], + "level": "DEBUG", + "propagate": false + }, + # used by roundup.* loggers + "roundup": { + "handlers": [ + "roundup" + ], + "level": "DEBUG", + "propagate": false # note pytest testing with caplog requires + # this to be true + }, + "roundup.hyperdb": { + "handlers": [ + "roundup" + ], + "level": "INFO", # can be a little noisy use INFO for production + "propagate": false + }, + "roundup.wsgi": { # using the waitress framework + "handlers": [ + "roundup" + ], + "level": "DEBUG", + "propagate": false + }, + "roundup.wsgi.translogger": { # httpd style logging + "handlers": [ + "access" + ], + "level": "DEBUG", + "propagate": false + }, + "root": { + "handlers": [ + "default" + ], + "level": "DEBUG", + "propagate": false + } + } + } + +fileConfig Based Logging Config +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The file config is an older and more limited method of configuring +logging. It is described by the `Configuration file format +`_ +in the Python documentation. The file name in the tracker's config for +the ``logging`` -> ``config`` setting must end with ``.ini`` to choose +the correct processing. + +Example fileConfig LoggingConfig +................................ + +This is an example .ini used with roundup-server configured to use +``roundup.http`` channel. It also includes some custom logging +qualnames/tags/channels for logging schema/permission detector and +extension output:: + + [loggers] + #other keys: roundup.hyperdb.backend + keys=root,roundup,roundup.http,roundup.hyperdb,actions,schema,extension,detector + + [logger_root] + #also for root where channlel is not set (NOTSET) aka all + level=DEBUG + handlers=rotate + + [logger_roundup] + # logger for all roundup.* not otherwise configured + level=DEBUG + handlers=rotate + qualname=roundup + propagate=0 + + [logger_roundup.http] + level=INFO + handlers=rotate_weblog + qualname=roundup.http + propagate=0 + + [logger_roundup.hyperdb] + level=WARNING + handlers=rotate + qualname=roundup.hyperdb + propagate=0 + + [logger_actions] + level=INFO + handlers=rotate + qualname=actions + propagate=0 + + [logger_detector] + level=INFO + handlers=rotate + qualname=detector + propagate=0 + + [logger_schema] + level=DEBUG + handlers=rotate + qualname=schema + propagate=0 + + [logger_extension] + level=INFO + handlers=rotate + qualname=extension + propagate=0 + + [handlers] + keys=basic,rotate,rotate_weblog + + [handler_basic] + class=StreamHandler + args=(sys.stderr,) + formatter=basic + + [handler_rotate] + class=logging.handlers.RotatingFileHandler + args=('roundup.log','a', 5120000, 2) + formatter=basic + + [handler_rotate_weblog] + class=logging.handlers.RotatingFileHandler + args=('httpd.log','a', 1024000, 2) + formatter=plain + + [formatters] + keys=basic,plain + + [formatter_basic] + format=%(asctime)s %(process)d %(name)s:%(module)s.%(funcName)s,%(levelname)s: %(message)s + datefmt=%Y-%m-%d %H:%M:%S + + [formatter_plain] + format=%(process)d %(message)s + Configuring roundup-server ========================== diff --git a/doc/upgrading.txt b/doc/upgrading.txt index 66faca56..ac919a5b 100644 --- a/doc/upgrading.txt +++ b/doc/upgrading.txt @@ -133,6 +133,21 @@ date, text etc.) do not need JavaScript to work. See :ref:`Confirming the User` in the reference manual for details. +Support for dictConfig Logging Configuration (optional) +------------------------------------------------------- + +Roundup's basic log configuration via config.ini has always had the +ability to use an ini style logging configuration to set levels per +log channel, control output file rotation etc. + +With Roundup 2.6 you can use a JSON like file to configure logging +using `dictConfig +`_. The +JSON file format as been enhanced to support comments that are +stripped before being processed by the logging system. + +You can read about the details in the :ref:`admin manual `. + .. index:: Upgrading; 2.4.0 to 2.5.0 Migrating from 2.4.0 to 2.5.0 diff --git a/roundup/configuration.py b/roundup/configuration.py index 421d91bc..51df14bd 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -43,6 +43,16 @@ def __str__(self): return self.args[0] +class LoggingConfigError(ConfigurationError): + def __init__(self, message, **attrs): + super().__init__(message) + for key, value in attrs.items(): + self.__setattr__(key, value) + + def __str__(self): + return self.args[0] + + class NoConfigError(ConfigurationError): """Raised when configuration loading fails @@ -2330,14 +2340,97 @@ def reset(self): self.detectors.reset() self.init_logging() + def load_config_dict_from_json_file(self, filename): + import json + comment_re = re.compile( + r"""^\s*\#.* # comment at beginning of line possibly indented. + | # or + ^(.*)\s\s\s\#.* # comment char preceeded by at least three spaces. + """, re.VERBOSE) + + config_list = [] + with open(filename) as config_file: + for line in config_file: + match = comment_re.search(line) + if match: + if match.lastindex: + config_list.append(match.group(1) + "\n") + else: + # insert blank line for comment line to + # keep line numbers in sync. + config_list.append("\n") + continue + config_list.append(line) + + try: + config_dict = json.loads("".join(config_list)) + except json.decoder.JSONDecodeError as e: + error_at_doc_line = e.lineno + # subtract 1 - zero index on config_list + # remove '\n' for display + line = config_list[error_at_doc_line - 1][:-1] + + hint = "" + if line.find('#') != -1: + hint = "\nMaybe bad inline comment, 3 spaces needed before #." + + raise LoggingConfigError( + 'Error parsing json logging dict (%(file)s) ' + 'near \n\n %(line)s\n\n' + '%(msg)s: line %(lineno)s column %(colno)s.%(hint)s' % + {"file": filename, + "line": line, + "msg": e.msg, + "lineno": error_at_doc_line, + "colno": e.colno, + "hint": hint}, + config_file=self.filepath, + source="json.loads" + ) + + return config_dict + def init_logging(self): _file = self["LOGGING_CONFIG"] - if _file and os.path.isfile(_file): + if _file and os.path.isfile(_file) and _file.endswith(".ini"): logging.config.fileConfig( _file, disable_existing_loggers=self["LOGGING_DISABLE_LOGGERS"]) return + if _file and os.path.isfile(_file) and _file.endswith(".json"): + config_dict = self.load_config_dict_from_json_file(_file) + try: + logging.config.dictConfig(config_dict) + except ValueError as e: + # docs say these exceptions: + # ValueError, TypeError, AttributeError, ImportError + # could be raised, but + # looking through the code, it looks like + # configure() maps all exceptions (including + # ImportError, TypeError) raised by functions to + # ValueError. + context = "No additional information available." + if hasattr(e, '__context__') and e.__context__: + # get additional error info. E.G. if INFO + # is replaced by MANGO, context is: + # ValueError("Unknown level: 'MANGO'") + # while str(e) is "Unable to configure handler 'access'" + context = e.__context__ + + raise LoggingConfigError( + 'Error loading logging dict from %(file)s.\n' + '%(msg)s\n%(context)s\n' % { + "file": _file, + "msg": type(e).__name__ + ": " + str(e), + "context": context + }, + config_file=self.filepath, + source="dictConfig" + ) + + return + _file = self["LOGGING_FILENAME"] # set file & level on the roundup logger logger = logging.getLogger('roundup') diff --git a/test/test_config.py b/test/test_config.py index aa3bb968..f7e630a7 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -25,6 +25,7 @@ import unittest from os.path import normpath +from textwrap import dedent from roundup import configuration from roundup.backends import get_backend, have_backend @@ -1046,3 +1047,197 @@ def testInvalidIndexerValue(self): print(string_rep) self.assertIn("nati", string_rep) self.assertIn("'whoosh'", string_rep) + + def testDictLoggerConfigViaJson(self): + + # test case broken, comment on version line misformatted + config1 = dedent(""" + { + "version": 1, # only supported version + "disable_existing_loggers": false, # keep the wsgi loggers + + "formatters": { + # standard Roundup formatter including context id. + "standard": { + "format": "%(asctime)s %(levelname)s %(name)s:%(module)s %(msg)s" + }, + # used for waitress wsgi server to produce httpd style logs + "http": { + "format": "%(message)s" + } + }, + "handlers": { + # create an access.log style http log file + "access": { + "level": "INFO", + "formatter": "http", + "class": "logging.FileHandler", + "filename": "demo/access.log" + }, + # logging for roundup.* loggers + "roundup": { + "level": "DEBUG", + "formatter": "standard", + "class": "logging.FileHandler", + "filename": "demo/roundup.log" + }, + # print to stdout - fall through for other logging + "default": { + "level": "DEBUG", + "formatter": "standard", + "class": "logging.StreamHandler", + "stream": "ext://sys.stdout" + } + }, + "loggers": { + "": { + "handlers": [ + "default" # used by wsgi/usgi + ], + "level": "DEBUG", + "propagate": false + }, + # used by roundup.* loggers + "roundup": { + "handlers": [ + "roundup" + ], + "level": "DEBUG", + "propagate": false # note pytest testing with caplog requires + # this to be true + }, + "roundup.hyperdb": { + "handlers": [ + "roundup" + ], + "level": "INFO", # can be a little noisy INFO for production + "propagate": false + }, + "roundup.wsgi": { # using the waitress framework + "handlers": [ + "roundup" + ], + "level": "DEBUG", + "propagate": false + }, + "roundup.wsgi.translogger": { # httpd style logging + "handlers": [ + "access" + ], + "level": "DEBUG", + "propagate": false + }, + "root": { + "handlers": [ + "default" + ], + "level": "DEBUG", + "propagate": false + } + } + } + """) + + log_config_filename = self.instance.tracker_home \ + + "/_test_log_config.json" + + # happy path + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(config1) + + config = self.db.config.load_config_dict_from_json_file( + log_config_filename) + self.assertIn("version", config) + self.assertEqual(config['version'], 1) + + # broken inline comment misformatted + test_config = config1.replace(": 1, #", ": 1, #") + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.load_config_dict_from_json_file( + log_config_filename) + self.assertEqual( + cm.exception.args[0], + ('Error parsing json logging dict ' + '(_test_instance/_test_log_config.json) near \n\n ' + '"version": 1, # only supported version\n\nExpecting ' + 'property name enclosed in double quotes: line 3 column 18.\n' + 'Maybe bad inline comment, 3 spaces needed before #.') + ) + + # broken trailing , on last dict element + test_config = config1.replace(' "ext://sys.stdout"', + ' "ext://sys.stdout",' + ) + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.load_config_dict_from_json_file( + log_config_filename) + self.assertEqual( + cm.exception.args[0], + ('Error parsing json logging dict ' + '(_test_instance/_test_log_config.json) near \n\n' + ' }\n\nExpecting property name enclosed in double ' + 'quotes: line 37 column 6.') + ) + + # happy path for init_logging() + + # verify preconditions + logger = logging.getLogger("roundup") + self.assertEqual(logger.level, 40) # error default from config.ini + self.assertEqual(logger.filters, []) + + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(config1) + + # file is made relative to tracker dir. + self.db.config["LOGGING_CONFIG"] = '_test_log_config.json' + config = self.db.config.init_logging() + self.assertIs(config, None) + + logger = logging.getLogger("roundup") + self.assertEqual(logger.level, 10) # debug + self.assertEqual(logger.filters, []) + + # broken invalid format type (int not str) + test_config = config1.replace('"format": "%(message)s"', + '"format": 1234',) + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + # file is made relative to tracker dir. + self.db.config["LOGGING_CONFIG"] = '_test_log_config.json' + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + self.assertEqual( + cm.exception.args[0], + ('Error loading logging dict from ' + '_test_instance/_test_log_config.json.\n' + "ValueError: Unable to configure formatter 'http'\n" + 'expected string or bytes-like object\n') + ) + + # broken invalid level MANGO + test_config = config1.replace( + ': "INFO", # can', + ': "MANGO", # can') + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + # file is made relative to tracker dir. + self.db.config["LOGGING_CONFIG"] = '_test_log_config.json' + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + self.assertEqual( + cm.exception.args[0], + ("Error loading logging dict from " + "_test_instance/_test_log_config.json.\nValueError: " + "Unable to configure logger 'roundup.hyperdb'\nUnknown level: " + "'MANGO'\n") + + ) From ab9a37b30a0520653e3751a00605dfd6a1fed5e7 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 20 Aug 2025 11:17:23 -0400 Subject: [PATCH 005/290] test: fix testDictLoggerConfigViaJson The path to the log file in the config did not exist. Change to use the tracker home. Also add a test where the log file directory does not exist. This reports the full path, so have to edit the full path in the error message before comparison. --- test/test_config.py | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/test/test_config.py b/test/test_config.py index f7e630a7..80c469c5 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -20,6 +20,7 @@ import logging import os import pytest +import re import shutil import sys import unittest @@ -1072,14 +1073,14 @@ def testDictLoggerConfigViaJson(self): "level": "INFO", "formatter": "http", "class": "logging.FileHandler", - "filename": "demo/access.log" + "filename": "_test_instance/access.log" }, # logging for roundup.* loggers "roundup": { "level": "DEBUG", "formatter": "standard", "class": "logging.FileHandler", - "filename": "demo/roundup.log" + "filename": "_test_instance/roundup.log" }, # print to stdout - fall through for other logging "default": { @@ -1241,3 +1242,29 @@ def testDictLoggerConfigViaJson(self): "'MANGO'\n") ) + + # broken invalid output directory + test_config = config1.replace( + ' "_test_instance/access.log"', + ' "not_a_test_instance/access.log"') + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + # file is made relative to tracker dir. + self.db.config["LOGGING_CONFIG"] = '_test_log_config.json' + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + + # error includes full path which is different on different + # CI and dev platforms. So munge the path using re.sub. + self.assertEqual( + re.sub("directory: \'/.*not_a", 'directory: not_a' , + cm.exception.args[0]), + ("Error loading logging dict from " + "_test_instance/_test_log_config.json.\n" + "ValueError: Unable to configure handler 'access'\n" + "[Errno 2] No such file or directory: " + "not_a_test_instance/access.log'\n" + ) + ) + From d510304e215d551d0be60536a04ff382b83c817e Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 20 Aug 2025 11:23:39 -0400 Subject: [PATCH 006/290] chore: bump actions/checkout as reported by dependabot. --- .github/workflows/anchore.yml | 2 +- .github/workflows/build-xapian.yml | 2 +- .github/workflows/ci-test.yml | 2 +- .github/workflows/codeql-analysis.yml | 2 +- .github/workflows/ossf-scorecard.yml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/anchore.yml b/.github/workflows/anchore.yml index 7eff99f1..3ab5b7cf 100644 --- a/.github/workflows/anchore.yml +++ b/.github/workflows/anchore.yml @@ -37,7 +37,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout the code - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - name: Build the Docker image run: docker pull python:3-alpine; docker build . --file scripts/Docker/Dockerfile --tag localbuild/testimage:latest - name: List the Docker image diff --git a/.github/workflows/build-xapian.yml b/.github/workflows/build-xapian.yml index 4d6edf6d..00e6fe09 100644 --- a/.github/workflows/build-xapian.yml +++ b/.github/workflows/build-xapian.yml @@ -42,7 +42,7 @@ jobs: # if: {{ false }} # continue running if step fails # continue-on-error: true - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 # Setup version of Python to use - name: Set Up Python 3.13 diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index 8e88da76..c0a66398 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -116,7 +116,7 @@ jobs: # if: {{ false }} # continue running if step fails # continue-on-error: true - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 # Setup version of Python to use - name: Set Up Python ${{ matrix.python-version }} diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 0b214cc7..d5a70850 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -49,7 +49,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL diff --git a/.github/workflows/ossf-scorecard.yml b/.github/workflows/ossf-scorecard.yml index c6a5547e..28075031 100644 --- a/.github/workflows/ossf-scorecard.yml +++ b/.github/workflows/ossf-scorecard.yml @@ -35,7 +35,7 @@ jobs: steps: - name: "Checkout code" - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: persist-credentials: false From 797154675cdfd8457c19bde09f83dc90d616b8f4 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 20 Aug 2025 12:53:14 -0400 Subject: [PATCH 007/290] test: more fixes for variations among python versions. reset logging. The loggers I installed are bleeding through to tests in other modules. Then handle recogniton of: trailing comma in json reporting line with comma not following line incorrect type value in dictConfig int where string should be 3.7 fils to detect. 3.10+ add "got 'int'" to reason. --- test/test_config.py | 68 +++++++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/test/test_config.py b/test/test_config.py index 80c469c5..ded20d23 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1051,7 +1051,7 @@ def testInvalidIndexerValue(self): def testDictLoggerConfigViaJson(self): - # test case broken, comment on version line misformatted + # good base test case config1 = dedent(""" { "version": 1, # only supported version @@ -1178,13 +1178,30 @@ def testDictLoggerConfigViaJson(self): with self.assertRaises(configuration.LoggingConfigError) as cm: config = self.db.config.load_config_dict_from_json_file( log_config_filename) - self.assertEqual( - cm.exception.args[0], - ('Error parsing json logging dict ' - '(_test_instance/_test_log_config.json) near \n\n' - ' }\n\nExpecting property name enclosed in double ' - 'quotes: line 37 column 6.') - ) + #pre 3.12?? + # FIXME check/remove when 3.13. is min supported version + if "property name" in cm.exception.args[0]: + self.assertEqual( + cm.exception.args[0], + ('Error parsing json logging dict ' + '(_test_instance/_test_log_config.json) near \n\n' + ' }\n\nExpecting property name enclosed in double ' + 'quotes: line 37 column 6.') + ) + + # 3.13+ diags FIXME + print('FINDME') + print(cm.exception.args[0]) + _junk = ''' + if "property name" not in cm.exception.args[0]: + self.assertEqual( + cm.exception.args[0], + ('Error parsing json logging dict ' + '(_test_instance/_test_log_config.json) near \n\n' + ' }\n\nExpecting property name enclosed in double ' + 'quotes: line 37 column 6.') + ) + ''' # happy path for init_logging() @@ -1213,15 +1230,25 @@ def testDictLoggerConfigViaJson(self): # file is made relative to tracker dir. self.db.config["LOGGING_CONFIG"] = '_test_log_config.json' - with self.assertRaises(configuration.LoggingConfigError) as cm: - config = self.db.config.init_logging() - self.assertEqual( - cm.exception.args[0], - ('Error loading logging dict from ' - '_test_instance/_test_log_config.json.\n' - "ValueError: Unable to configure formatter 'http'\n" - 'expected string or bytes-like object\n') - ) + + + # different versions of python have different errors + # (or no error for this case in 3.7) + # FIXME remove version check post 3.7 as minimum version + if sys.version_info > (3,7): + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + + # mangle args[0] to add got 'int' + # FIXME: remove mangle after 3.12 min version + self.assertEqual( + cm.exception.args[0].replace( + "object\n", "object, got 'int'\n"), + ('Error loading logging dict from ' + '_test_instance/_test_log_config.json.\n' + "ValueError: Unable to configure formatter 'http'\n" + "expected string or bytes-like object, got 'int'\n") + ) # broken invalid level MANGO test_config = config1.replace( @@ -1268,3 +1295,10 @@ def testDictLoggerConfigViaJson(self): ) ) + # rip down all the loggers leaving the root logger reporting to stdout. + # otherwise logger config is leaking to other tests + + from importlib import reload + logging.shutdown() + reload(logging) + From 24b82e46659ba007e5a588e42a35fa8e4be9e294 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 20 Aug 2025 16:13:52 -0400 Subject: [PATCH 008/290] test: more fun with logger leakage. I have disabled all calls to dictConfig. I can't see how to stop this from leaking. I even tried storing the root and roundup logger state (copying lists) before running anything and restoring afterwards. the funny part is if I removae all dictConfig calls and keep the state saving and logger reset and restoring code it sill fails. It passes if I don't restore the handler state for the root logger. However the test will fail even when I comment out the root logger config in the dict if I apply the dict. ???? --- test/test_config.py | 55 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/test/test_config.py b/test/test_config.py index ded20d23..a7f4a6ee 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1139,6 +1139,20 @@ def testDictLoggerConfigViaJson(self): } """) + # save roundup logger state + loggernames = ("", "roundup") + logger_state = {} + for name in loggernames: + logger_state[name] = {} + + roundup_logger = logging.getLogger("roundup") + for i in ("filters", "handlers", "level", "propagate"): + attr = getattr(roundup_logger, i) + if isinstance(attr, list): + logger_state[name][i] = attr.copy() + else: + logger_state[name][i] = getattr(roundup_logger, i) + log_config_filename = self.instance.tracker_home \ + "/_test_log_config.json" @@ -1203,6 +1217,16 @@ def testDictLoggerConfigViaJson(self): ) ''' + ''' + # comment out as it breaks the logging config for caplog + # on test_rest.py:testBadFormAttributeErrorException + # for all rdbms backends. + # the log ERROR check never gets any info + + # commenting out root logger in config doesn't make it work. + # storing root logger and roundup logger state and restoring it + # still fails. + # happy path for init_logging() # verify preconditions @@ -1295,9 +1319,38 @@ def testDictLoggerConfigViaJson(self): ) ) - # rip down all the loggers leaving the root logger reporting to stdout. + ''' + # rip down all the loggers leaving the root logger reporting + # to stdout. # otherwise logger config is leaking to other tests + roundup_loggers = [logging.getLogger(name) for name in + logging.root.manager.loggerDict + if name.startswith("roundup")] + + # cribbed from configuration.py:init_loggers + hdlr = logging.StreamHandler(sys.stdout) + formatter = logging.Formatter( + '%(asctime)s %(levelname)s %(message)s') + hdlr.setFormatter(formatter) + + for logger in roundup_loggers: + # no logging API to remove all existing handlers!?! + for h in logger.handlers: + h.close() + logger.removeHandler(h) + logger.handlers = [hdlr] + logger.setLevel("DEBUG") + logger.propagate = True + + for name in loggernames: + local_logger = logging.getLogger(name) + for attr in logger_state[name]: + # if I restore handlers state for root logger + # I break the test referenced above. -- WHY???? + if attr == "handlers" and name == "": continue + setattr(local_logger, attr, logger_state[name][attr]) + from importlib import reload logging.shutdown() reload(logging) From bf02f353d19be24dcf8ad7ee6ed2bb2aac7d19ad Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 20 Aug 2025 21:04:56 -0400 Subject: [PATCH 009/290] test: test dictLoggerConfig - working logging reset and windows Finally figured out why things weren't being restored. Bug in the code. Created a class fixture that stores and restores the logging config. Also using os.path.join and other machinations to make the tests run under windows and linux correctly. --- test/test_config.py | 187 ++++++++++++++++++++++++-------------------- 1 file changed, 101 insertions(+), 86 deletions(-) diff --git a/test/test_config.py b/test/test_config.py index a7f4a6ee..7d1ed541 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -415,8 +415,71 @@ def testOctalNumberOption(self): print(type(config._get_option('UMASK'))) +@pytest.mark.usefixtures("save_restore_logging") class TrackerConfig(unittest.TestCase): + @pytest.fixture(scope="class") + def save_restore_logging(self): + """Save logger state and try to restore it after all tests in + this class have finished. + + The primary test is testDictLoggerConfigViaJson which + can change the loggers and break tests that depend on caplog + """ + # Save logger state for root and roundup top level logger + loggernames = ("", "roundup") + + # The state attributes to save. Lists are shallow copied + state_to_save = ("filters", "handlers", "level", "propagate") + + logger_state = {} + for name in loggernames: + logger_state[name] = {} + roundup_logger = logging.getLogger(name) + + for i in state_to_save: + attr = getattr(roundup_logger, i) + if isinstance(attr, list): + logger_state[name][i] = attr.copy() + else: + logger_state[name][i] = getattr(roundup_logger, i) + + # run all class tests here + yield + + # rip down all the loggers leaving the root logger reporting + # to stdout. + # otherwise logger config is leaking to other tests + roundup_loggers = [logging.getLogger(name) for name in + logging.root.manager.loggerDict + if name.startswith("roundup")] + + # cribbed from configuration.py:init_loggers + hdlr = logging.StreamHandler(sys.stdout) + formatter = logging.Formatter( + '%(asctime)s %(levelname)s %(message)s') + hdlr.setFormatter(formatter) + + for logger in roundup_loggers: + # no logging API to remove all existing handlers!?! + for h in logger.handlers: + h.close() + logger.removeHandler(h) + logger.handlers = [hdlr] + logger.setLevel("WARNING") + logger.propagate = True # important as caplog requires this + + # Restore the info we stored before running tests + for name in loggernames: + local_logger = logging.getLogger(name) + for attr in logger_state[name]: + setattr(local_logger, attr, logger_state[name][attr]) + + # reset logging as well + from importlib import reload + logging.shutdown() + reload(logging) + backend = 'anydbm' def setUp(self): @@ -1139,22 +1202,8 @@ def testDictLoggerConfigViaJson(self): } """) - # save roundup logger state - loggernames = ("", "roundup") - logger_state = {} - for name in loggernames: - logger_state[name] = {} - - roundup_logger = logging.getLogger("roundup") - for i in ("filters", "handlers", "level", "propagate"): - attr = getattr(roundup_logger, i) - if isinstance(attr, list): - logger_state[name][i] = attr.copy() - else: - logger_state[name][i] = getattr(roundup_logger, i) - - log_config_filename = self.instance.tracker_home \ - + "/_test_log_config.json" + log_config_filename = os.path.join(self.instance.tracker_home, + "_test_log_config.json") # happy path with open(log_config_filename, "w") as log_config_file: @@ -1176,10 +1225,11 @@ def testDictLoggerConfigViaJson(self): self.assertEqual( cm.exception.args[0], ('Error parsing json logging dict ' - '(_test_instance/_test_log_config.json) near \n\n ' + '(%s) near \n\n ' '"version": 1, # only supported version\n\nExpecting ' 'property name enclosed in double quotes: line 3 column 18.\n' - 'Maybe bad inline comment, 3 spaces needed before #.') + 'Maybe bad inline comment, 3 spaces needed before #.' % + log_config_filename) ) # broken trailing , on last dict element @@ -1198,9 +1248,10 @@ def testDictLoggerConfigViaJson(self): self.assertEqual( cm.exception.args[0], ('Error parsing json logging dict ' - '(_test_instance/_test_log_config.json) near \n\n' - ' }\n\nExpecting property name enclosed in double ' - 'quotes: line 37 column 6.') + '(%s) near \n\n' + ' }\n\n' + 'Expecting property name enclosed in double ' + 'quotes: line 37 column 6.' % log_config_filename) ) # 3.13+ diags FIXME @@ -1211,22 +1262,12 @@ def testDictLoggerConfigViaJson(self): self.assertEqual( cm.exception.args[0], ('Error parsing json logging dict ' - '(_test_instance/_test_log_config.json) near \n\n' - ' }\n\nExpecting property name enclosed in double ' - 'quotes: line 37 column 6.') + '(%s) near \n\n' + ' "stream": "ext://sys.stdout"\n\n' + 'Expecting property name enclosed in double ' + 'quotes: line 37 column 6.' % log_config_filename) ) ''' - - ''' - # comment out as it breaks the logging config for caplog - # on test_rest.py:testBadFormAttributeErrorException - # for all rdbms backends. - # the log ERROR check never gets any info - - # commenting out root logger in config doesn't make it work. - # storing root logger and roundup logger state and restoring it - # still fails. - # happy path for init_logging() # verify preconditions @@ -1269,9 +1310,10 @@ def testDictLoggerConfigViaJson(self): cm.exception.args[0].replace( "object\n", "object, got 'int'\n"), ('Error loading logging dict from ' - '_test_instance/_test_log_config.json.\n' + '%s.\n' "ValueError: Unable to configure formatter 'http'\n" - "expected string or bytes-like object, got 'int'\n") + "expected string or bytes-like object, got 'int'\n" % + log_config_filename) ) # broken invalid level MANGO @@ -1288,9 +1330,9 @@ def testDictLoggerConfigViaJson(self): self.assertEqual( cm.exception.args[0], ("Error loading logging dict from " - "_test_instance/_test_log_config.json.\nValueError: " + "%s.\nValueError: " "Unable to configure logger 'roundup.hyperdb'\nUnknown level: " - "'MANGO'\n") + "'MANGO'\n" % log_config_filename) ) @@ -1298,6 +1340,8 @@ def testDictLoggerConfigViaJson(self): test_config = config1.replace( ' "_test_instance/access.log"', ' "not_a_test_instance/access.log"') + access_filename = os.path.join("not_a_test_instance", "access.log") + with open(log_config_filename, "w") as log_config_file: log_config_file.write(test_config) @@ -1307,51 +1351,22 @@ def testDictLoggerConfigViaJson(self): config = self.db.config.init_logging() # error includes full path which is different on different - # CI and dev platforms. So munge the path using re.sub. - self.assertEqual( - re.sub("directory: \'/.*not_a", 'directory: not_a' , - cm.exception.args[0]), - ("Error loading logging dict from " - "_test_instance/_test_log_config.json.\n" - "ValueError: Unable to configure handler 'access'\n" - "[Errno 2] No such file or directory: " - "not_a_test_instance/access.log'\n" - ) - ) - - ''' - # rip down all the loggers leaving the root logger reporting - # to stdout. - # otherwise logger config is leaking to other tests - - roundup_loggers = [logging.getLogger(name) for name in - logging.root.manager.loggerDict - if name.startswith("roundup")] - - # cribbed from configuration.py:init_loggers - hdlr = logging.StreamHandler(sys.stdout) - formatter = logging.Formatter( - '%(asctime)s %(levelname)s %(message)s') - hdlr.setFormatter(formatter) - - for logger in roundup_loggers: - # no logging API to remove all existing handlers!?! - for h in logger.handlers: - h.close() - logger.removeHandler(h) - logger.handlers = [hdlr] - logger.setLevel("DEBUG") - logger.propagate = True - - for name in loggernames: - local_logger = logging.getLogger(name) - for attr in logger_state[name]: - # if I restore handlers state for root logger - # I break the test referenced above. -- WHY???? - if attr == "handlers" and name == "": continue - setattr(local_logger, attr, logger_state[name][attr]) - - from importlib import reload - logging.shutdown() - reload(logging) + # CI and dev platforms. So munge the path using re.sub and + # replace. Windows needs replace as the full path for windows + # to the file has '\\\\' not '\\' when taken from __context__. + # E.G. + # ("Error loading logging dict from ' + # '_test_instance\\_test_log_config.json.\nValueError: ' + # "Unable to configure handler 'access'\n[Errno 2] No such file " + # "or directory: " + # "'C:\\\\tracker\\\\path\\\\not_a_test_instance\\\\access.log'\n") + # sigh..... + output = re.sub("directory: \'.*not_a", 'directory: not_a' , + cm.exception.args[0].replace(r'\\','\\')) + target = ("Error loading logging dict from " + "%s.\n" + "ValueError: Unable to configure handler 'access'\n" + "[Errno 2] No such file or directory: " + "%s'\n" % (log_config_filename, access_filename)) + self.assertEqual(output, target) From 4c31c3931097fa9b2927a968f5443d6d81f12bd7 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Thu, 21 Aug 2025 09:53:32 -0400 Subject: [PATCH 010/290] test: fix code that does not run a test on 3.7 --- test/test_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_config.py b/test/test_config.py index 7d1ed541..17ed0ba6 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1300,7 +1300,7 @@ def testDictLoggerConfigViaJson(self): # different versions of python have different errors # (or no error for this case in 3.7) # FIXME remove version check post 3.7 as minimum version - if sys.version_info > (3,7): + if sys.version_info >= (3, 8, 0): with self.assertRaises(configuration.LoggingConfigError) as cm: config = self.db.config.init_logging() From 22da3e23c1b0bc1b59169fcf3b97a71e9d9732f2 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 25 Aug 2025 20:30:51 -0400 Subject: [PATCH 011/290] doc: add check for db.tx_Source to Reauth examples Otherwise it triggers on roundup-admin et al --- doc/customizing.txt | 6 ++++++ doc/reference.txt | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/doc/customizing.txt b/doc/customizing.txt index 62faac18..c3aa5fb2 100644 --- a/doc/customizing.txt +++ b/doc/customizing.txt @@ -1836,6 +1836,12 @@ contents:: # the user has confirmed their identity return + if db.tx_Source not in ("web"): + # the user is using rest, xmlrpc, command line, + # email (unlikely) which don't support interactive + # verification + return + # if the password or email are changing, require id confirmation if 'password' in newvalues: raise Reauth('Add an optional message to the user') diff --git a/doc/reference.txt b/doc/reference.txt index 9713ae28..5d26493f 100644 --- a/doc/reference.txt +++ b/doc/reference.txt @@ -1321,6 +1321,12 @@ user to submit each sensitive property separately. For example:: 'at the same time is not allowed. Please ' 'submit two changes.') + if db.tx_Source not in ("web"): + # the user is using rest, xmlrpc, command line, + # email (unlikely) which don't support interactive + # verification + return + if 'password' in newvalues and not hasattr(db, 'reauth_done'): raise Reauth() From 0bc1ea3d4e4668dfa914780168b1afe2591b98b7 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 25 Aug 2025 20:32:14 -0400 Subject: [PATCH 012/290] doc: reformat markdown-note footnote --- doc/upgrading.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/upgrading.txt b/doc/upgrading.txt index ac919a5b..718a0114 100644 --- a/doc/upgrading.txt +++ b/doc/upgrading.txt @@ -949,9 +949,9 @@ bug. Splitting the large script into two parts: allows use of ``structure`` on the script with no replaced strings should it be required for your tracker. -.. [#markdown-note] If you are using markdown formatting for your tracker's notes, - the user will see the markdown label rather than the long - (suspicious) URL. You may want to add something like:: +.. [#markdown-note] If you are using markdown formatting for your + tracker's notes, the user will see the markdown label rather than + the long (suspicious) URL. You may want to add something like:: a[href*=\@template]::after { content: ' [' attr(href) ']'; From 98ef0037b4eff7f1254c1397077338c359700f7b Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 25 Aug 2025 20:44:42 -0400 Subject: [PATCH 013/290] doc: add disable saving roundup-admin history file for password changes --- doc/admin_guide.txt | 9 ++++++--- doc/upgrading.txt | 10 +++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index 42e73e29..c94ac59b 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -2151,13 +2151,16 @@ you can put the initialise command and password on the command line. But this allows others on the host to see the password (using the ps command). To initialise a tracker non-interactively without exposing the password, create a file (e.g init_tracker) set to mode -600 (so only the owner can read it) with the contents: +600 (so only the owner can read it) with the contents:: initialise admin_password -and feed it to roundup-admin on standard input. E.G. +and feed it to roundup-admin on standard input. E.G.:: - cat init_tracker | roundup-admin -i tracker_dir + cat init_tracker | roundup-admin -i tracker_dir -P history_features=2 + +setting the pragma ``history_features=2`` prevents storing the command +in the user's history file. (for more details see https://issues.roundup-tracker.org/issue2550789.) diff --git a/doc/upgrading.txt b/doc/upgrading.txt index 718a0114..e814a635 100644 --- a/doc/upgrading.txt +++ b/doc/upgrading.txt @@ -2188,7 +2188,7 @@ running:: roundup-admin -i table password,id,username Look for lines starting with ``{CRYPT}``. You can reset the user's -password using:: +password using [#history-pragma]_ :: roundup-admin -i roundup> set user16 password=somenewpassword @@ -2199,6 +2199,14 @@ prompt). This prevents the new password from showing up in the output of ps or shell history. The new password will be encrypted using the default encryption method (usually pbkdf2). +.. [#history-pragma] If your version of roundup-admin provides history + support, you should add ``-P history_features=2`` to the command + line or run ``pragma history_features=2`` at the ``roundup>`` + prompt. This will prevent the command line (and password) from being + saved to your history file (usually ``.roundup_admin_history`` in + your user's home directory. You can use ``roundup-admin -i + pragma list`` to see if pragmas are supported. + Enable performance improvement for wsgi mode (optional) ------------------------------------------------------- From 0c1dcc3ed37f8d78631526c218bc2d48a06c944d Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Tue, 26 Aug 2025 22:24:00 -0400 Subject: [PATCH 014/290] feat: change comment in dictConfig json file to // from # Emacs json mode at least will properly indent when using // as a comment character and not #. --- doc/admin_guide.txt | 54 ++++++++++++++++++++++------------------ roundup/configuration.py | 8 +++--- test/test_config.py | 38 ++++++++++++++-------------- 3 files changed, 53 insertions(+), 47 deletions(-) diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index c94ac59b..7228023d 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -110,15 +110,18 @@ dictConfigs are specified in JSON format with support for comments. The file name in the tracker's config for the ``logging`` -> ``config`` setting must end with ``.json`` to choose the correct processing. -Comments have to be in one of two forms: +Comments have to be in one of two forms based on javascript line +comments: -1. A ``#`` with preceding white space is considered a comment and is - stripped from the file before being passed to the json parser. This - is a "block comment". +1. A ``//`` possibly indented with whitespace on a line is considereda + a comment and is stripped from the file before being passed to the + json parser. This is a "line comment". -2. A ``#`` preceded by at least three - white space characters is stripped from the end of the line before - begin passed to the json parser. This is an "inline comment". +2. A ``//` with at least three white space characters before it is + stripped from the end of the line before begin passed to the json + parser. This is an "inline comment". + +Block style comments are not supported. Other than this the file is a standard json file that matches the `Configuration dictionary schema @@ -129,9 +132,12 @@ defined in the Python documentation. Example dictConfig Logging Config ................................. -Note that this file is not actually JSON format as it include comments. -So you can not use tools that expect JSON (linters, formatters) to -work with it. +Note that this file is not actually JSON format as it include +comments. However by using javascript style comments, some tools that +expect JSON (editors, linters, formatters) might work with it. A +command like ``sed -e 's#^\s*//.*##' -e 's#\s*\s\s\s//.*##' +logging.json`` can be used to strip comments for programs that need +it. The config below works with the `Waitress wsgi server `_ configured to use the @@ -143,35 +149,35 @@ the tracker home. The tracker home is the subdirectory demo under the current working directory. The commented config is:: { - "version": 1, # only supported version - "disable_existing_loggers": false, # keep the wsgi loggers + "version": 1, // only supported version + "disable_existing_loggers": false, // keep the wsgi loggers "formatters": { - # standard format for Roundup messages + // standard format for Roundup messages "standard": { - "format": "%(asctime)s %(levelname)s %(name)s:%(module)s %(msg)s" + "format": "%(asctime)s %(ctx_id)s %(levelname)s %(name)s:%(module)s %(msg)s" }, - # used for waitress wsgi server to produce httpd style logs + // used for waitress wsgi server to produce httpd style logs "http": { "format": "%(message)s" } }, "handlers": { - # create an access.log style http log file + // create an access.log style http log file "access": { "level": "INFO", "formatter": "http", "class": "logging.FileHandler", "filename": "demo/access.log" }, - # logging for roundup.* loggers + // logging for roundup.* loggers "roundup": { "level": "DEBUG", "formatter": "standard", "class": "logging.FileHandler", "filename": "demo/roundup.log" }, - # print to stdout - fall through for other logging + // print to stdout - fall through for other logging "default": { "level": "DEBUG", "formatter": "standard", @@ -187,30 +193,30 @@ current working directory. The commented config is:: "level": "DEBUG", "propagate": false }, - # used by roundup.* loggers + // used by roundup.* loggers "roundup": { "handlers": [ "roundup" ], "level": "DEBUG", - "propagate": false # note pytest testing with caplog requires - # this to be true + "propagate": false // note pytest testing with caplog requires + // this to be true }, "roundup.hyperdb": { "handlers": [ "roundup" ], - "level": "INFO", # can be a little noisy use INFO for production + "level": "INFO", // can be a little noisy use INFO for production "propagate": false }, - "roundup.wsgi": { # using the waitress framework + "roundup.wsgi": { // using the waitress framework "handlers": [ "roundup" ], "level": "DEBUG", "propagate": false }, - "roundup.wsgi.translogger": { # httpd style logging + "roundup.wsgi.translogger": { // httpd style logging "handlers": [ "access" ], diff --git a/roundup/configuration.py b/roundup/configuration.py index 51df14bd..fe37240f 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -2343,9 +2343,9 @@ def reset(self): def load_config_dict_from_json_file(self, filename): import json comment_re = re.compile( - r"""^\s*\#.* # comment at beginning of line possibly indented. + r"""^\s*//#.* # comment at beginning of line possibly indented. | # or - ^(.*)\s\s\s\#.* # comment char preceeded by at least three spaces. + ^(.*)\s\s\s\//.* # comment char preceeded by at least three spaces. """, re.VERBOSE) config_list = [] @@ -2371,8 +2371,8 @@ def load_config_dict_from_json_file(self, filename): line = config_list[error_at_doc_line - 1][:-1] hint = "" - if line.find('#') != -1: - hint = "\nMaybe bad inline comment, 3 spaces needed before #." + if line.find('//') != -1: + hint = "\nMaybe bad inline comment, 3 spaces needed before //." raise LoggingConfigError( 'Error parsing json logging dict (%(file)s) ' diff --git a/test/test_config.py b/test/test_config.py index 17ed0ba6..1c2834c5 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1117,35 +1117,35 @@ def testDictLoggerConfigViaJson(self): # good base test case config1 = dedent(""" { - "version": 1, # only supported version - "disable_existing_loggers": false, # keep the wsgi loggers + "version": 1, // only supported version + "disable_existing_loggers": false, // keep the wsgi loggers "formatters": { - # standard Roundup formatter including context id. + // standard Roundup formatter including context id. "standard": { "format": "%(asctime)s %(levelname)s %(name)s:%(module)s %(msg)s" }, - # used for waitress wsgi server to produce httpd style logs + // used for waitress wsgi server to produce httpd style logs "http": { "format": "%(message)s" } }, "handlers": { - # create an access.log style http log file + // create an access.log style http log file "access": { "level": "INFO", "formatter": "http", "class": "logging.FileHandler", "filename": "_test_instance/access.log" }, - # logging for roundup.* loggers + // logging for roundup.* loggers "roundup": { "level": "DEBUG", "formatter": "standard", "class": "logging.FileHandler", "filename": "_test_instance/roundup.log" }, - # print to stdout - fall through for other logging + // print to stdout - fall through for other logging "default": { "level": "DEBUG", "formatter": "standard", @@ -1156,35 +1156,35 @@ def testDictLoggerConfigViaJson(self): "loggers": { "": { "handlers": [ - "default" # used by wsgi/usgi + "default" // used by wsgi/usgi ], "level": "DEBUG", "propagate": false }, - # used by roundup.* loggers + // used by roundup.* loggers "roundup": { "handlers": [ "roundup" ], "level": "DEBUG", - "propagate": false # note pytest testing with caplog requires - # this to be true + "propagate": false // note pytest testing with caplog requires + // this to be true }, "roundup.hyperdb": { "handlers": [ "roundup" ], - "level": "INFO", # can be a little noisy INFO for production + "level": "INFO", // can be a little noisy INFO for production "propagate": false }, - "roundup.wsgi": { # using the waitress framework + "roundup.wsgi": { // using the waitress framework "handlers": [ "roundup" ], "level": "DEBUG", "propagate": false }, - "roundup.wsgi.translogger": { # httpd style logging + "roundup.wsgi.translogger": { // httpd style logging "handlers": [ "access" ], @@ -1215,7 +1215,7 @@ def testDictLoggerConfigViaJson(self): self.assertEqual(config['version'], 1) # broken inline comment misformatted - test_config = config1.replace(": 1, #", ": 1, #") + test_config = config1.replace(": 1, //", ": 1, //") with open(log_config_filename, "w") as log_config_file: log_config_file.write(test_config) @@ -1226,9 +1226,9 @@ def testDictLoggerConfigViaJson(self): cm.exception.args[0], ('Error parsing json logging dict ' '(%s) near \n\n ' - '"version": 1, # only supported version\n\nExpecting ' + '"version": 1, // only supported version\n\nExpecting ' 'property name enclosed in double quotes: line 3 column 18.\n' - 'Maybe bad inline comment, 3 spaces needed before #.' % + 'Maybe bad inline comment, 3 spaces needed before //.' % log_config_filename) ) @@ -1318,8 +1318,8 @@ def testDictLoggerConfigViaJson(self): # broken invalid level MANGO test_config = config1.replace( - ': "INFO", # can', - ': "MANGO", # can') + ': "INFO", // can', + ': "MANGO", // can') with open(log_config_filename, "w") as log_config_file: log_config_file.write(test_config) From 50a8b0069a7ab16313d90c9dca11cdef1a13d4c0 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Tue, 26 Aug 2025 23:06:40 -0400 Subject: [PATCH 015/290] refactor: also error on missing file or invalid extension Refactored the code to reuse check that logging config file is set and that the file exists. Now throws error and exits if file name does not end in .ini or .json. Now throws error if file doesn't exist. Before it would just configure default logging as though file wasn't specified. Added tests for these two cases. --- roundup/configuration.py | 85 +++++++++++++++++++++++----------------- test/test_config.py | 38 ++++++++++++++++++ 2 files changed, 86 insertions(+), 37 deletions(-) diff --git a/roundup/configuration.py b/roundup/configuration.py index fe37240f..d170eeed 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -2392,45 +2392,56 @@ def load_config_dict_from_json_file(self, filename): def init_logging(self): _file = self["LOGGING_CONFIG"] - if _file and os.path.isfile(_file) and _file.endswith(".ini"): - logging.config.fileConfig( - _file, - disable_existing_loggers=self["LOGGING_DISABLE_LOGGERS"]) - return - - if _file and os.path.isfile(_file) and _file.endswith(".json"): - config_dict = self.load_config_dict_from_json_file(_file) - try: - logging.config.dictConfig(config_dict) - except ValueError as e: - # docs say these exceptions: - # ValueError, TypeError, AttributeError, ImportError - # could be raised, but - # looking through the code, it looks like - # configure() maps all exceptions (including - # ImportError, TypeError) raised by functions to - # ValueError. - context = "No additional information available." - if hasattr(e, '__context__') and e.__context__: - # get additional error info. E.G. if INFO - # is replaced by MANGO, context is: - # ValueError("Unknown level: 'MANGO'") - # while str(e) is "Unable to configure handler 'access'" - context = e.__context__ - - raise LoggingConfigError( - 'Error loading logging dict from %(file)s.\n' - '%(msg)s\n%(context)s\n' % { - "file": _file, - "msg": type(e).__name__ + ": " + str(e), - "context": context - }, - config_file=self.filepath, - source="dictConfig" - ) - + if _file and os.path.isfile(_file): + if _file.endswith(".ini"): + logging.config.fileConfig( + _file, + disable_existing_loggers=self["LOGGING_DISABLE_LOGGERS"]) + elif _file.endswith(".json"): + config_dict = self.load_config_dict_from_json_file(_file) + try: + logging.config.dictConfig(config_dict) + except ValueError as e: + # docs say these exceptions: + # ValueError, TypeError, AttributeError, ImportError + # could be raised, but + # looking through the code, it looks like + # configure() maps all exceptions (including + # ImportError, TypeError) raised by functions to + # ValueError. + context = "No additional information available." + if hasattr(e, '__context__') and e.__context__: + # get additional error info. E.G. if INFO + # is replaced by MANGO, context is: + # ValueError("Unknown level: 'MANGO'") + # while str(e) is "Unable to configure handler 'access'" + context = e.__context__ + + raise LoggingConfigError( + 'Error loading logging dict from %(file)s.\n' + '%(msg)s\n%(context)s\n' % { + "file": _file, + "msg": type(e).__name__ + ": " + str(e), + "context": context + }, + config_file=self.filepath, + source="dictConfig" + ) + else: + raise OptionValueError( + self.options['LOGGING_CONFIG'], + _file, + "Unable to load logging config file. " + "File extension must be '.ini' or '.json'.\n" + ) + return + if _file: + raise OptionValueError(self.options['LOGGING_CONFIG'], + _file, + "Unable to find logging config file.") + _file = self["LOGGING_FILENAME"] # set file & level on the roundup logger logger = logging.getLogger('roundup') diff --git a/test/test_config.py b/test/test_config.py index 1c2834c5..130c9c41 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1370,3 +1370,41 @@ def testDictLoggerConfigViaJson(self): "%s'\n" % (log_config_filename, access_filename)) self.assertEqual(output, target) + def test_missing_logging_config_file(self): + saved_config = self.db.config['LOGGING_CONFIG'] + + self.db.config['LOGGING_CONFIG'] = 'logging.json' + + with self.assertRaises(configuration.OptionValueError) as cm: + self.db.config.init_logging() + + self.assertEqual(cm.exception.args[1], "_test_instance/logging.json") + self.assertEqual(cm.exception.args[2], + "Unable to find logging config file.") + + self.db.config['LOGGING_CONFIG'] = 'logging.ini' + + with self.assertRaises(configuration.OptionValueError) as cm: + self.db.config.init_logging() + + self.assertEqual(cm.exception.args[1], "_test_instance/logging.ini") + self.assertEqual(cm.exception.args[2], + "Unable to find logging config file.") + + self.db.config['LOGGING_CONFIG'] = saved_config + + def test_unknown_logging_config_file_type(self): + saved_config = self.db.config['LOGGING_CONFIG'] + + self.db.config['LOGGING_CONFIG'] = 'schema.py' + + + with self.assertRaises(configuration.OptionValueError) as cm: + self.db.config.init_logging() + + self.assertEqual(cm.exception.args[1], "_test_instance/schema.py") + self.assertEqual(cm.exception.args[2], + "Unable to load logging config file. " + "File extension must be '.ini' or '.json'.\n") + + self.db.config['LOGGING_CONFIG'] = saved_config From 0c3458bac0cbd3cab34cfb3a9d7e9feee91c34ad Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Tue, 26 Aug 2025 23:37:42 -0400 Subject: [PATCH 016/290] feat: add 'q' as alias to quit to exit interactive roundup-admin Also require no arguments to 'q', 'quit' or 'exit' before exiting. Now typing 'quit a' will get an unknown command error. Add to admin-guide how to get out of interactive mode. Also test 'q' and 'exit' commands. No upgrading docs added. Not that big a feature. Just noted in CHANGES. Reporting error if argument provided is unlikely to be an issue IMO, so no upgrading.txt entry. --- CHANGES.txt | 2 ++ doc/admin_guide.txt | 3 +++ roundup/admin.py | 3 ++- test/test_admin.py | 4 ++-- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index ac2c5422..f3d7cea3 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -34,6 +34,8 @@ Features: properly entered, the change is committed. (John Rouillard) - add support for dictConfig style logging configuration. Ini/File style configs will still be supported. (John Rouillard) +- add 'q' as alias for quit in roundup-admin interactive mode. (John + Rouillard) 2025-07-13 2.5.0 diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index 7228023d..bcd5101f 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -2067,6 +2067,9 @@ The basic usage is: Commands may be abbreviated as long as the abbreviation matches only one command, e.g. l == li == lis == list. +In interactive mode entering: ``q``, ``quit``, or ``exit`` alone on a +line will exit the program. + One thing to note, The ``-u user`` setting does not currently operate like a user logging in via the web. The user running roundup-admin must have read access to the tracker home directory. As a result the diff --git a/roundup/admin.py b/roundup/admin.py index 5fd1394b..f52aaa26 100644 --- a/roundup/admin.py +++ b/roundup/admin.py @@ -2415,7 +2415,8 @@ def interactive(self): except ValueError: continue # Ignore invalid quoted token if not args: continue # noqa: E701 - if args[0] in ('quit', 'exit'): break # noqa: E701 + if args[0] in ('q', 'quit', 'exit') and len(args) == 1: + break # noqa: E701 self.run_command(args) # exit.. check for transactions diff --git a/test/test_admin.py b/test/test_admin.py index e6387098..97018453 100644 --- a/test/test_admin.py +++ b/test/test_admin.py @@ -150,7 +150,7 @@ def testBasicInteractive(self): expected = 'ready for input.\nType "help" for help.' self.assertEqual(expected, out[-1*len(expected):]) - inputs = iter(["list user", "quit"]) + inputs = iter(["list user", "q"]) AdminTool.my_input = lambda _self, _prompt: next(inputs) @@ -1067,7 +1067,7 @@ def testPragma_reopen_tracker(self): # must set verbose to see _reopen_tracker hidden setting. # and to get "Reopening tracker" verbose log output - inputs = iter(["pragma verbose=true", "pragma list", "quit"]) + inputs = iter(["pragma verbose=true", "pragma list", "exit"]) AdminTool.my_input = lambda _self, _prompt: next(inputs) self.install_init() From 87adb70dca81d4f2e0e4a538799fa825638a2d7b Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Thu, 28 Aug 2025 11:13:24 -0400 Subject: [PATCH 017/290] build(chore): update bump codecov/codecov-action from 5.4.3 to 5.5.0 https://github.com/roundup-tracker/roundup/pull/61 by dependabot --- .github/workflows/ci-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index c0a66398..35a5a948 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -323,7 +323,7 @@ jobs: - name: Upload coverage to Codecov # see: https://github.com/codecov/codecov-action#usage - uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3 + uses: codecov/codecov-action@fdcc8476540edceab3de004e990f80d881c6cc00 # v5.5.0 with: verbose: true token: ${{ secrets.CODECOV_TOKEN }} From 8547740c034d54225f47d507b9d76c7d551c270d Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Thu, 28 Aug 2025 11:30:11 -0400 Subject: [PATCH 018/290] chore: update sha256 index for latest pyhton:3-alpine image. --- scripts/Docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/Docker/Dockerfile b/scripts/Docker/Dockerfile index 7ab53982..b3424cd1 100644 --- a/scripts/Docker/Dockerfile +++ b/scripts/Docker/Dockerfile @@ -26,7 +26,7 @@ ARG source=local # Note this is the index digest for the image, not the manifest digest. # The index digest is shared across archetectures (amd64, arm64 etc.) # while the manifest digest is unique per platform/arch. -ARG SHA256=9b4929a72599b6c6389ece4ecbf415fd1355129f22bb92bb137eea098f05e975 +ARG SHA256=9ba6d8cbebf0fb6546ae71f2a1c14f6ffd2fdab83af7fa5669734ef30ad48844 # Set to any non-empty value to enable shell debugging for troubleshooting ARG VERBOSE= From 2bdcc6b84772a00373fded524452760bfb299087 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Thu, 28 Aug 2025 12:39:38 -0400 Subject: [PATCH 019/290] test - fix parsing of integer param values CI broke on the string '1\r#' expecting a 400 but got a 200 in test_element_url_param_accepting_integer_values(). The #, & characters mark a url fragment or start of another parameter and not part of the value. In a couple of tests, I parse the hypothesis generated value to remove a # or & and anything after. Then I set the value to the preceding string. If the string starts with # or &, the value is set to "0" as the server ignores the parameter and returns 200. "0" is a value that asserts that status is 200. The code doing this parsing was different (and broken) between test_element_url_param_accepting_integer_values and test_class_url_param_accepting_integer_values It's now consistent and if it finds a & or #, it actually tests the resulting value/status rather than skipping the test. --- test/test_liveserver.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/test/test_liveserver.py b/test/test_liveserver.py index 4bec62b2..884439b0 100644 --- a/test/test_liveserver.py +++ b/test/test_liveserver.py @@ -256,6 +256,7 @@ class FuzzGetUrls(WsgiSetup, ClientSetup): @given(sampled_from(['@verbose', '@page_size', '@page_index']), text(min_size=1)) + @example("@verbose", "0\r#") @example("@verbose", "1#") @example("@verbose", "#1stuff") @example("@verbose", "0 #stuff") @@ -271,10 +272,18 @@ def test_class_url_param_accepting_integer_values(self, param, value): f = session.get(url, params=query) try: # test case '0 #', '0#', '12345#stuff' '12345&stuff' - match = re.match(r'(^[0-9]*\s*)[#&]', value) + # Normalize like a server does by breaking value at + # # or & as these mark a fragment or subsequent + # query arg and are not part of the value. + match = re.match(r'^(.*)[#&]', value) if match is not None: value = match[1] - elif int(value) >= 0: + # parameter is ignored by server if empty. + # so set it to 0 to force 200 status code. + if value == "": + value = "0" + + if int(value) >= 0: self.assertEqual(f.status_code, 200) except ValueError: # test case '#' '#0', '&', '&anything here really' @@ -285,6 +294,7 @@ def test_class_url_param_accepting_integer_values(self, param, value): self.assertEqual(f.status_code, 400) @given(sampled_from(['@verbose']), text(min_size=1)) + @example("@verbose", "0\r#") @example("@verbose", "10#") @example("@verbose", u'Ø\U000dd990') @settings(max_examples=_max_examples, @@ -298,10 +308,18 @@ def test_element_url_param_accepting_integer_values(self, param, value): f = session.get(url, params=query) try: # test case '0#' '12345#stuff' '12345&stuff' - match = re.match('(^[0-9]*)[#&]', value) + # Normalize like a server does by breaking value at + # # or & as these mark a fragment or subsequent + # query arg and are not part of the value. + match = re.match(r'^(.*)[#&]', value) if match is not None: value = match[1] - elif int(value) >= 0: + # parameter is ignored by server if empty. + # so set it to 0 to force 200 status code. + if value == "": + value = "0" + + if int(value) >= 0: self.assertEqual(f.status_code, 200) except ValueError: # test case '#' '#0', '&', '&anything here really' From c53b5cd6602edf1996a917f06bd5794668f566e8 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 31 Aug 2025 16:54:17 -0400 Subject: [PATCH 020/290] feat: add support for ! history and readline command in roundup-admin Ad support to change input mode emacs/vi using new 'readline' roundup-admin command. Also bind keys to command/input strings, List numbered history and allow rerunning a command with ! or allow user to edit it using !:p. admin_guide.txt: Added docs. admin.py: add functionality. Reconcile import commands to standard. Replace IOError with FileNotFoundError no that we have removed python 2.7 support. Add support for identifying backend used to supply line editing/history functions. Add support for saving commands sent on stdin to history to allow preloading of history. test_admin.py: Test code. Can't test mode changes as lack of pty when driving command line turns off line editing in readline/pyreadline3. Similarly can't test key bindings/settings. Some refactoring of test conditions that had to change because of additional output reporting backend library. --- CHANGES.txt | 3 + doc/admin_guide.txt | 51 +++++++ roundup/admin.py | 205 ++++++++++++++++++++++++-- test/test_admin.py | 350 +++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 586 insertions(+), 23 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index f3d7cea3..645983f5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -36,6 +36,9 @@ Features: style configs will still be supported. (John Rouillard) - add 'q' as alias for quit in roundup-admin interactive mode. (John Rouillard) +- add readline command to roundup-admin to list history, control input + mode etc. Also support bang (!) commands to rerun commands in history + or put them in the input buffer for editing. (John Rouillard) 2025-07-13 2.5.0 diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index bcd5101f..c08fee98 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -2202,6 +2202,57 @@ https://pythonhosted.org/pyreadline/usage.html#configuration-file. History is saved to the file ``.roundup_admin_history`` in your home directory (for windows usually ``\Users\``. +In Roundup 2.6.0 and newer, you can use the ``readline`` command to +make changes on the fly. + +* ``readline vi`` - change input mode to use vi key binding when + editing. It starts in entry mode. +* ``readline emacs`` - change input mode to emacs key bindings when + editing. This is also the default. +* ``readline reload`` - reloads the ``~/.roundup_admin_rlrc`` file so + you can test and use changes. +* ``readline history`` - dumps the history buffer and numbers all + commands. +* ``readline .inputrc_command_line`` can be used to make on the fly + key and key sequence bindings to readline commands. It can also be + used to change the internal readline settings using a set + command. For example:: + + readline set bell-style none + + will turn off a ``visible`` or ``audible`` bell. Single character + keybindings:: + + readline Control-o: dump-variables + + to list all the variables that can be set are supported. As are + multi-character bindings:: + + readline "\C-o1": "commit" + + will put "commit" on the input line when you type Control-o followed + by 1. See the `readline manual for details + `_ + on the command lines that can be used. + +Also a limited form of ``!`` (bang) history reference was added. The +reference must be at the start of the line. Typing ``!23`` will rerun +command number 23 from your history. + +Typing ``!23:p`` will load command 23 into the buffer so you can edit +and submit it. Using the bang feature will append the command to the +end of the history list. + +Pyreadline3 users can use ``readline history`` and the +bang commands (including ``:p``). Single character bindings can be +done. For example:: + + readline Control-w: history-search-backward + +The commands that are available are limited compared to Unix's +readline or libedit. Setting variables or entry mode (emacs, +vi) switching do not work in testing. + Using with the shell -------------------- diff --git a/roundup/admin.py b/roundup/admin.py index f52aaa26..2778c6b7 100644 --- a/roundup/admin.py +++ b/roundup/admin.py @@ -34,8 +34,8 @@ import roundup.instance from roundup import __version__ as roundup_version -from roundup import date, hyperdb, init, password, token_r -from roundup.anypy import scandir_ +from roundup import date, hyperdb, init, password, support, token_r +from roundup.anypy import scandir_ # noqa: F401 define os.scandir from roundup.anypy.my_input import my_input from roundup.anypy.strings import repr_export from roundup.configuration import ( @@ -49,7 +49,6 @@ ) from roundup.exceptions import UsageError from roundup.i18n import _, get_translation -from roundup import support try: from UserDict import UserDict @@ -93,6 +92,13 @@ class AdminTool: Additional help may be supplied by help_*() methods. """ + # import here to make AdminTool.readline accessible or + # mockable from tests. + try: + import readline # noqa: I001, PLC0415 + except ImportError: + readline = None + # Make my_input a property to allow overriding in testing. # my_input is imported in other places, so just set it from # the imported value rather than moving def here. @@ -1815,6 +1821,111 @@ def do_pragma(self, args): type(self.settings[setting]).__name__) self.settings[setting] = value + def do_readline(self, args): + ''"""Usage: readline initrc_line | 'emacs' | 'history' | 'reload' | 'vi' + + Using 'reload' will reload the file ~/.roundup_admin_rlrc. + 'history' will show (and number) all commands in the history. + + You can change input mode using the 'emacs' or 'vi' parameters. + The default is emacs. This is the same as using:: + + readline set editing-mode emacs + + or:: + + readline set editing-mode vi + + Any command that can be placed in a readline .inputrc file can + be executed using the readline command. You can assign + dump-variables to control O using:: + + readline Control-o: dump-variables + + Assigning multi-key values also works. + + pyreadline3 support on windows: + + Mode switching doesn't work, emacs only. + + Binding single key commands works with:: + + readline Control-w: history-search-backward + + Multiple key sequences don't work. + + Setting values may work. Difficult to tell because the library + has no way to view the live settings. + + """ + + # TODO: allow history 20 # most recent 20 commands + # history 100-200 # show commands 100-200 + + if not self.readline: + print(_("Readline support is not available.")) + return + # The if test allows pyreadline3 settings like: + # bind_exit_key("Control-z") get through to + # parse_and_bind(). It is not obvious that this form of + # command is supported. Pyreadline3 is supposed to parse + # readline style commands, so we use those for emacs/vi. + # Trying set-mode(...) as in the pyreadline3 init file + # didn't work in testing. + + if len(args) == 1 and args[0].find('(') == -1: + if args[0] == "vi": + self.readline.parse_and_bind("set editing-mode vi") + print(_("Enabled vi mode.")) + elif args[0] == "emacs": + self.readline.parse_and_bind("set editing-mode emacs") + print(_("Enabled emacs mode.")) + elif args[0] == "history": + print("history size", + self.readline.get_current_history_length()) + print('\n'.join([ + str("%3d " % (i + 1) + + self.readline.get_history_item(i + 1)) + for i in range( + self.readline.get_current_history_length()) + ])) + elif args[0] == "reload": + try: + # readline is a singleton. In testing previous + # tests using read_init_file are loading from ~ + # not the test directory because it doesn't + # matter. But for reload we want to test with the + # init file under the test directory. Calling + # read_init_file() calls with the ~/.. init + # location and I can't seem to reset it + # or the readline state. + # So call with explicit file here. + self.readline.read_init_file( + self.get_readline_init_file()) + except FileNotFoundError as e: + # If user invoked reload explicitly, report + # if file not found. + # + # DOES NOT WORK with pyreadline3. Exception + # is not raised if file is missing. + # + # Also e.filename is None under cygwin. A + # simple test case does set e.filename + # correctly?? sigh. So I just call + # get_readline_init_file again to get + # filename. + fn = e.filename or self.get_readline_init_file() + print(_("Init file %s not found.") % fn) + else: + print(_("File %s reloaded.") % + self.get_readline_init_file()) + else: + print(_("Unknown readline parameter %s") % args[0]) + return + + self.readline.parse_and_bind(" ".join(args)) + return + designator_re = re.compile('([A-Za-z]+)([0-9]+)$') designator_rng = re.compile('([A-Za-z]+):([0-9]+)-([0-9]+)$') @@ -2365,29 +2476,34 @@ def history_features(self, feature): # setting the bit disables the feature, so use not. return not self.settings['history_features'] & features[feature] + def get_readline_init_file(self): + return os.path.join(os.path.expanduser("~"), + ".roundup_admin_rlrc") + def interactive(self): """Run in an interactive mode """ print(_('Roundup %s ready for input.\nType "help" for help.') % roundup_version) - initfile = os.path.join(os.path.expanduser("~"), - ".roundup_admin_rlrc") + initfile = self.get_readline_init_file() histfile = os.path.join(os.path.expanduser("~"), ".roundup_admin_history") - try: - import readline + if self.readline: + # clear any history that might be left over from caller + # when reusing AdminTool from tests or program. + self.readline.clear_history() try: if self.history_features('load_rc'): - readline.read_init_file(initfile) - except IOError: # FileNotFoundError under python3 + self.readline.read_init_file(initfile) + except FileNotFoundError: # file is optional pass try: if self.history_features('load_history'): - readline.read_history_file(histfile) + self.readline.read_history_file(histfile) except IOError: # FileNotFoundError under python3 # no history file yet pass @@ -2397,19 +2513,75 @@ def interactive(self): # Pragma history_length allows setting on a per # invocation basis at startup if self.settings['history_length'] != -1: - readline.set_history_length( + self.readline.set_history_length( self.settings['history_length']) - except ImportError: - readline = None - print(_('Note: command history and editing not available')) + if hasattr(self.readline, 'backend'): + # FIXME after min 3.13 version; no backend prints pyreadline3 + print(_("Readline enabled using %s.") % self.readline.backend) + else: + print(_("Readline enabled using unknown library.")) + + else: + print(_('Command history and line editing not available')) + + autosave_enabled = sys.stdin.isatty() and sys.stdout.isatty() while 1: try: command = self.my_input('roundup> ') + # clear an input hook in case it was used to prefill + # buffer. + self.readline.set_pre_input_hook() except EOFError: print(_('exit...')) break if not command: continue # noqa: E701 + if command.startswith('!'): # Pull numbered command from history + print_only = command.endswith(":p") + try: + hist_num = int(command[1:]) \ + if not print_only else int(command[1:-2]) + command = self.readline.get_history_item(hist_num) + except ValueError: + # pass the unknown command + pass + else: + if autosave_enabled and \ + hasattr(self.readline, "replace_history_item"): + # history has the !23 input. Replace it if possible. + # replace_history_item not supported by pyreadline3 + # so !23 will show up in history not the command. + self.readline.replace_history_item( + self.readline.get_current_history_length() - 1, + command) + + if print_only: + # fill the edit buffer with the command + # the user selected. + + # from https://stackoverflow.com/questions/8505163/is-it-possible-to-prefill-a-input-in-python-3s-command-line-interface + # This triggers: + # B023 Function definition does not bind loop variable + # `command` + # in ruff. command will be the value of the command + # variable at the time the function is run. + # Not the value at define time. This is ok since + # hook is run before command is changed by the + # return from (readline) input. + def hook(): + self.readline.insert_text(command) # noqa: B023 + self.readline.redisplay() + self.readline.set_pre_input_hook(hook) + # we clear the hook after the next line is read. + continue + + if not autosave_enabled: + # needed to make testing work and also capture + # commands received on stdin from file/other command + # output. Disable saving with pragma on command line: + # -P history_features=2. + self.readline.add_history(command) + try: args = token_r.token_split(command) except ValueError: @@ -2426,8 +2598,9 @@ def interactive(self): self.db.commit() # looks like histfile is saved with mode 600 - if readline and self.history_features('save_history'): - readline.write_history_file(histfile) + if self.readline and self.history_features('save_history'): + self.readline.write_history_file(histfile) + return 0 def main(self): # noqa: PLR0912, PLR0911 diff --git a/test/test_admin.py b/test/test_admin.py index 97018453..9380115d 100644 --- a/test/test_admin.py +++ b/test/test_admin.py @@ -5,8 +5,17 @@ # from __future__ import print_function +import difflib +import errno import fileinput -import unittest, os, shutil, errno, sys, difflib, re +import io +import os +import platform +import pytest +import re +import shutil +import sys +import unittest from roundup.admin import AdminTool @@ -82,6 +91,10 @@ class AdminTest(object): def setUp(self): self.dirname = '_test_admin' + @pytest.fixture(autouse=True) + def inject_fixtures(self, monkeypatch): + self._monkeypatch = monkeypatch + def tearDown(self): try: shutil.rmtree(self.dirname) @@ -148,7 +161,9 @@ def testBasicInteractive(self): print(ret) self.assertTrue(ret == 0) expected = 'ready for input.\nType "help" for help.' - self.assertEqual(expected, out[-1*len(expected):]) + # back up by 30 to make sure 'ready for input' in slice. + self.assertIn(expected, + "\n".join(out.split('\n')[-3:-1])) inputs = iter(["list user", "q"]) @@ -161,8 +176,10 @@ def testBasicInteractive(self): print(ret) self.assertTrue(ret == 0) - expected = 'help.\n 1: admin\n 2: anonymous' - self.assertEqual(expected, out[-1*len(expected):]) + expected = ' 1: admin\n 2: anonymous' + + self.assertEqual(expected, + "\n".join(out.split('\n')[-2:])) AdminTool.my_input = orig_input @@ -1104,7 +1121,7 @@ def testPragma_reopen_tracker(self): print(ret) self.assertTrue(ret == 0) - self.assertEqual('Reopening tracker', out[2]) + self.assertEqual('Reopening tracker', out[3]) expected = ' _reopen_tracker=True' self.assertIn(expected, out) @@ -1133,7 +1150,7 @@ def testPragma(self): ret = self.admin.main() out = out.getvalue().strip().split('\n') - + print(ret) self.assertTrue(ret == 0) expected = ' verbose=True' @@ -1155,7 +1172,7 @@ def testPragma(self): ret = self.admin.main() out = out.getvalue().strip().split('\n') - + print(ret) self.assertTrue(ret == 0) expected = ' verbose=False' @@ -1810,6 +1827,325 @@ def testSetOnClass(self): self.assertEqual(out, expected) self.assertEqual(len(err), 0) + def testReadline(self): + ''' Note the tests will fail if you run this under pdb. + the context managers capture the pdb prompts and this screws + up the stdout strings with (pdb) prefixed to the line. + ''' + + '''history didn't work when testing. The commands being + executed aren't being sent into the history + buffer. Failed under both windows and linux. + + Explicitly using: readline.set_auto_history(True) in + roundup-admin setup had no effect. + + Looks like monkeypatching stdin is the issue since: + + printf... | roundup-admin | tee + + doesn't work either when printf uses + + "readline vi\nreadline emacs\nreadline history\nquit\n" + + Added explicit readline.add_history() if stdin or + stdout are not a tty to admin.py:interactive(). + + Still no way to drive editing with control/escape + chars to verify editing mode, check keybindings. Need + to trick Admintool to believe it's running on a + tty/pty/con in linux/windows to remove my hack. + ''' + + # Put the init file in the tracker test directory so + # we don't clobber user's actual init file. + original_home = None + if 'HOME' in os.environ: + original_home = os.environ['HOME'] + os.environ['HOME'] = self.dirname + + # same but for windows. + original_userprofile = None + if 'USERPROFILE' in os.environ: + # windows + original_userprofile = os.environ['USERPROFILE'] + os.environ['USERPROFILE'] = self.dirname + + inputs = ["readline vi", "readline emacs", "readline reload", "quit"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable loading and saving history + self.admin.settings['history_features'] = 3 + + # verify correct init file is being + self.assertIn(os.path.join(os.path.expanduser("~"), + ".roundup_admin_rlrc"), + self.admin.get_readline_init_file()) + + # No exception is raised for missing file + # under pyreadline3. Detect pyreadline3 looking for: + # readline.Readline + pyreadline = hasattr(self.admin.readline, "Readline") + + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = 'roundup> Enabled vi mode.' + self.assertIn(expected, out) + + expected = 'roundup> Enabled emacs mode.' + self.assertIn(expected, out) + + if not pyreadline: + expected = ('roundup> Init file %s ' + 'not found.' % self.admin.get_readline_init_file()) + self.assertIn(expected, out) + + # --- test 2 + + inputs = ["readline reload", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + with open(self.admin.get_readline_init_file(), + "w") as config_file: + # there is no config line that works for all + # pyreadline3 (windows), readline(*nix), or editline + # (mac). So write empty file. + config_file.write("") + + # disable loading and saving history + self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = ('roundup> File %s reloaded.' % + self.admin.get_readline_init_file()) + + self.assertIn(expected, out) + + # === cleanup + if original_home: + os.environ['HOME'] = original_home + if original_userprofile: + os.environ['USERPROFILE'] = original_userprofile + + def test_admin_history_save_load(self): + # To prevent overwriting/reading user's actual history, + # change HOME enviroment var. + original_home = None + if 'HOME' in os.environ: + original_home = os.environ['HOME'] + os.environ['HOME'] = self.dirname + os.environ['HOME'] = self.dirname + + # same idea but windows + original_userprofile = None + if 'USERPROFILE' in os.environ: + # windows + original_userprofile = os.environ['USERPROFILE'] + os.environ['USERPROFILE'] = self.dirname + + # -- history test + inputs = ["readline history", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # use defaults load/save history + self.admin.settings['history_features'] = 0 + + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = 'roundup> history size 1' + self.assertIn(expected, out) + + expected = ' 1 readline history' + self.assertIn(expected, out) + + # -- history test 3 reruns readline vi + inputs = ["readline vi", "readline history", "!3", + "readline history", "!23s", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + # preserve directory self.install_init() + self.admin=AdminTool() + + # default use all features + #self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + # 4 includes 2 commands in saved history + expected = 'roundup> history size 4' + self.assertIn(expected, out) + + expected = ' 4 readline history' + self.assertIn(expected, out) + + # Shouldn't work on windows. + if platform.system() != "Windows": + expected = ' 5 readline vi' + self.assertIn(expected, out) + else: + # PYREADLINE UNDER WINDOWS + # py3readline on windows can't replace + # command strings in history when connected + # to a console. (Console triggers autosave and + # I have to turn !3 into it's substituted value.) + # but in testing autosave is disabled so + # I don't get the !number but the actual command + # It should have + # + # expected = ' 5 !3' + # + # but it is the same as the unix case. + expected = ' 5 readline vi' + self.assertIn(expected, out) + + expected = ('roundup> Unknown command "!23s" ("help commands" ' + 'for a list)') + self.assertIn(expected, out) + + print(out) + # can't test !#:p mode as readline editing doesn't work + # if not in a tty. + + # === cleanup + if original_home: + os.environ['HOME'] = original_home + if original_userprofile: + os.environ['USERPROFILE'] = original_userprofile + + def test_admin_readline_history(self): + original_home = os.environ['HOME'] + # To prevent overwriting/reading user's actual history, + # change HOME enviroment var. + os.environ['HOME'] = self.dirname + + original_userprofile = None + if 'USERPROFILE' in os.environ: + # windows + original_userprofile = os.environ['USERPROFILE'] + os.environ['USERPROFILE'] = self.dirname + + # -- history test + inputs = ["readline history", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable loading, but save history + self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = 'roundup> history size 1' + self.assertIn(expected, out) + + expected = ' 1 readline history' + self.assertIn(expected, out) + + # -- history test + inputs = ["readline vi", "readline history", "!1", "!2", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable loading, but save history + self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = 'roundup> history size 2' + self.assertIn(expected, out) + + expected = ' 2 readline history' + self.assertIn(expected, out) + + # doesn't work on windows. + if platform.system() != "Windows": + expected = ' 4 readline history' + self.assertIn(expected, out) + else: + # See + # PYREADLINE UNDER WINDOWS + # elsewhere in this file for why I am not checking for + # expected = ' 4 !2' + expected = ' 4 readline history' + self.assertIn(expected, out) + + # can't test !#:p mode as readline editing doesn't work + # if not in a tty. + + # === cleanup + os.environ['HOME'] = original_home + if original_userprofile: + os.environ['USERPROFILE'] = original_userprofile + def testSpecification(self): ''' Note the tests will fail if you run this under pdb. the context managers capture the pdb prompts and this screws From 55857ec2b20495bd3f3940658f4df13311f3b2db Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 31 Aug 2025 20:59:04 -0400 Subject: [PATCH 021/290] bug, refactor, test: make pragma history_length work interactively history_length could be set interactively, but it was never used to set readline/pyreadline3's internal state. Using the pragma setting on the roundup-admin command line did set readline's state. Also refactored 2 calls to self.readline.get_current_history_length() into one call and storing in a variable. Also changed method for creating history strings for printing. Tests added for history_length pragma on cli and interactive use. Added test for exiting roundup-admin with EOF on input. Added test for 'readline nosuchdirective' error case. Added test to readline with a command directive to set an internal variable. This last one has no real test to see if it was successful because I can't emulate a real keyboard/tty which is needed to test. --- CHANGES.txt | 3 + roundup/admin.py | 14 +++-- test/test_admin.py | 139 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 151 insertions(+), 5 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 645983f5..fb3b06d3 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -26,6 +26,9 @@ Fixed: - cleaned up repo. Close obsolete branches and close a split head due to an identical merge intwo different workicng copies. (John Rouillard) +- in roundup-admin, using 'pragma history_length interactively now + sets readline history length. Using -P history_length=10 on the + command line always worked. (John Rouillard) Features: diff --git a/roundup/admin.py b/roundup/admin.py index 2778c6b7..eb717b73 100644 --- a/roundup/admin.py +++ b/roundup/admin.py @@ -1821,6 +1821,11 @@ def do_pragma(self, args): type(self.settings[setting]).__name__) self.settings[setting] = value + # history_length has to be pushed to readline to have any effect. + if setting == "history_length": + self.readline.set_history_length( + self.settings['history_length']) + def do_readline(self, args): ''"""Usage: readline initrc_line | 'emacs' | 'history' | 'reload' | 'vi' @@ -1881,13 +1886,12 @@ def do_readline(self, args): self.readline.parse_and_bind("set editing-mode emacs") print(_("Enabled emacs mode.")) elif args[0] == "history": - print("history size", - self.readline.get_current_history_length()) + history_size = self.readline.get_current_history_length() + print("history size", history_size) print('\n'.join([ - str("%3d " % (i + 1) + + "%3d %s" % ((i + 1), self.readline.get_history_item(i + 1)) - for i in range( - self.readline.get_current_history_length()) + for i in range(history_size) ])) elif args[0] == "reload": try: diff --git a/test/test_admin.py b/test/test_admin.py index 9380115d..74266d17 100644 --- a/test/test_admin.py +++ b/test/test_admin.py @@ -184,6 +184,31 @@ def testBasicInteractive(self): AdminTool.my_input = orig_input + # test EOF exit + inputs = ["help"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + # preserve directory self.install_init() + self.admin=AdminTool() + + # disable all features + self.admin.settings['history_features'] = 7 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + # 4 includes 2 commands in saved history + expected = 'roundup> exit...' + self.assertIn(expected, out) + def testGet(self): ''' Note the tests will fail if you run this under pdb. the context managers capture the pdb prompts and this screws @@ -1947,6 +1972,120 @@ def testReadline(self): self.assertIn(expected, out) + # --- test 3,4 - make sure readline gets history_length pragma. + # test CLI and interactive. + + inputs = ["pragma list", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable all config/history + self.admin.settings['history_features'] = 7 + sys.argv=['main', '-i', self.dirname, '-P', 'history_length=11'] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + self.assertEqual(self.admin.readline.get_history_length(), + 11) + + # 4 + inputs = ["pragma history_length=17", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable all config/history + self.admin.settings['history_features'] = 7 + # keep pragma in CLI. Make sure it's overridden by interactive + sys.argv=['main', '-i', self.dirname, '-P', 'history_length=11'] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + # should not be 11. + self.assertEqual(self.admin.readline.get_history_length(), + 17) + + # --- test 5 invalid single word parameter + + inputs = ["readline nosuchdirective", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable loading and saving history + self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = ('roundup> Unknown readline parameter nosuchdirective') + + self.assertIn(expected, out) + + # --- test 6 set keystroke command. + # FIXME: unable to test key binding/setting actually works. + # + # No errors seem to come back from readline or + # pyreadline3 even when the keybinding makes no + # sense. Errors are only reported when reading + # from init file. Using "set answer 42" does print + # 'readline: answer: unknown variable name' when + # attached to tty/pty and interactive, but not + # inside test case. Pyreadline3 doesn't + # report errors at all. + # + # Even if I set a keybidning, I can't invoke it + # because I am not running inside a pty, so + # editing is disabled and I have no way to + # simulate keyboard keystrokes for readline to act + # upon. + + inputs = ['readline set meaning 42', "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable loading and saving history + self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + # === cleanup if original_home: os.environ['HOME'] = original_home From 882c577fa3876bbe9345adeacdde15f33ad88eb1 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 1 Sep 2025 17:48:57 -0400 Subject: [PATCH 022/290] doc: fix typos. --- CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index fb3b06d3..68aa8d84 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -24,7 +24,7 @@ Fixed: rather than Exception since it is an HTTP exception. (John Rouillard) - cleaned up repo. Close obsolete branches and close a split head due - to an identical merge intwo different workicng copies. (John + to an identical merge in two different working copies. (John Rouillard) - in roundup-admin, using 'pragma history_length interactively now sets readline history length. Using -P history_length=10 on the From b8e186c852121e3051a68a52c03f381973a420b5 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 1 Sep 2025 20:35:54 -0400 Subject: [PATCH 023/290] fix missing formatting --- doc/admin_guide.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index c08fee98..bebf7402 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -117,7 +117,7 @@ comments: a comment and is stripped from the file before being passed to the json parser. This is a "line comment". -2. A ``//` with at least three white space characters before it is +2. A ``//`` with at least three white space characters before it is stripped from the end of the line before begin passed to the json parser. This is an "inline comment". From 27162c67396915f228b414031bed0afb045e37a9 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 1 Sep 2025 21:54:48 -0400 Subject: [PATCH 024/290] feat: allow admin to set logging format from config.ini This is prep work for adding a per thread logging variable that can be used to tie all logs for a single request together. This uses the same default logging format as before, just moves it to config.ini. Also because of configparser, the logging format has to have doubled % signs. So use: %%(asctime)s not '%(asctime)s' as configparser tries to interpolate that string and asctime is not defined in the configparser's scope. Using %%(asctime)s is not interpolated by configparser and is passed into Roundup. --- CHANGES.txt | 2 ++ doc/admin_guide.txt | 20 +++++++++++++++- roundup/configuration.py | 52 +++++++++++++++++++++++++++++++++++++--- test/test_config.py | 37 ++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 4 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 68aa8d84..4ec37e17 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -42,6 +42,8 @@ Features: - add readline command to roundup-admin to list history, control input mode etc. Also support bang (!) commands to rerun commands in history or put them in the input buffer for editing. (John Rouillard) +- add format to logging section in config.ini. Used to set default + logging format. (John Rouillard) 2025-07-13 2.5.0 diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index bebf7402..ad92a94a 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -63,6 +63,8 @@ Configuration for "BasicLogging" implementation: - tracker configuration file lets you disable other loggers (e.g. when running under a wsgi framework) with ``logging`` -> ``disable_loggers``. + - tracker configuration file can set the log format using + ``logging`` -> ``format``. See :ref:`logFormat` for more info. - ``roundup-server`` specifies the location of a log file on the command line - ``roundup-server`` enable using the standard python logger with @@ -83,10 +85,26 @@ Configuration for standard "logging" module: In both cases, if no logfile is specified then logging will simply be sent to sys.stderr with only logging of ERROR messages. +.. _logFormat: + +Defining the Log Format +----------------------- + +Starting with Roundup 2.6 you can specify the logging format. In the +``logging`` -> ``format`` setting of config.ini you can use any of the +`standard logging LogRecord attributes +`_. +However you must double any ``%`` format markers. The default value +is:: + + %%(asctime)s %%(levelname)s %%(message)s + Standard Logging Setup ---------------------- -You can specify your log configs in one of two formats: +If the settings in config.ini are not sufficient for your logging +requirements, you can specify a full logging configuration in one of +two formats: * `fileConfig format `_ diff --git a/roundup/configuration.py b/roundup/configuration.py index d170eeed..6da56200 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -273,7 +273,8 @@ def load_ini(self, config): try: if config.has_option(self.section, self.setting): self.set(config.get(self.section, self.setting)) - except configparser.InterpolationSyntaxError as e: + except (configparser.InterpolationSyntaxError, + configparser.InterpolationMissingOptionError) as e: raise ParsingOptionError( _("Error in %(filepath)s with section [%(section)s] at " "option %(option)s: %(message)s") % { @@ -575,6 +576,48 @@ def get(self): return None +class LoggingFormatOption(Option): + """Replace escaped % (as %%) with single %. + + Config file parsing allows variable interpolation using + %(keyname)s. However this is exactly the format that we need + for creating a logging format string. So we tell the user to + quote the string using %%(...). Then we turn %%( -> %( when + retrieved. + """ + + class_description = ("Allowed value: Python logging module named " + "attributes with % sign doubled.") + + def str2value(self, value): + """Check format of unquoted string looking for missing specifiers. + + This does a dirty check to see if a token is missing a + specifier. So "%(ascdate)s %(level) " would fail because of + the 's' missing after 'level)'. But "%(ascdate)s %(level)s" + would pass. + + Note that %(foo)s generates a error from the ini parser + with a less than wonderful message. + """ + unquoted_val = value.replace("%%(", "%(") + + # regexp matches all current logging record object attribute names. + scanned_result = re.sub(r'%\([A-Za-z_]+\)\S','', unquoted_val ) + if scanned_result.find('%(') != -1: + raise OptionValueError( + self, unquoted_val, + "Check that all substitution tokens have a format " + "specifier after the ). Unrecognized use of %%(...) in: " + "%s" % scanned_result) + + return str(unquoted_val) + + def _value2str(self, value): + """Replace %( with %%( to quote the format substitution param. + """ + return value.replace("%(", "%%(") + class OriginHeadersListOption(Option): """List of space seperated origin header values. @@ -1614,6 +1657,10 @@ def str2value(self, value): "Minimal severity level of messages written to log file.\n" "If above 'config' option is set, this option has no effect.\n" "Allowed values: DEBUG, INFO, WARNING, ERROR"), + (LoggingFormatOption, "format", + "%(asctime)s %(levelname)s %(message)s", + "Format of the logging messages with all '%' signs\n" + "doubled so they are not interpreted by the config file."), (BooleanOption, "disable_loggers", "no", "If set to yes, only the loggers configured in this section will\n" "be used. Yes will disable gunicorn's --access-logfile.\n"), @@ -2448,8 +2495,7 @@ def init_logging(self): hdlr = logging.FileHandler(_file) if _file else \ logging.StreamHandler(sys.stdout) - formatter = logging.Formatter( - '%(asctime)s %(levelname)s %(message)s') + formatter = logging.Formatter(self["LOGGING_FORMAT"]) hdlr.setFormatter(formatter) # no logging API to remove all existing handlers!?! for h in logger.handlers: diff --git a/test/test_config.py b/test/test_config.py index 130c9c41..b3547cdf 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1112,6 +1112,43 @@ def testInvalidIndexerValue(self): self.assertIn("nati", string_rep) self.assertIn("'whoosh'", string_rep) + def testLoggerFormat(self): + config = configuration.CoreConfig() + + # verify config is initalized to defaults + self.assertEqual(config['LOGGING_FORMAT'], + '%(asctime)s %(levelname)s %(message)s') + + # load config + config.load(self.dirname) + self.assertEqual(config['LOGGING_FORMAT'], + '%(asctime)s %(levelname)s %(message)s') + + # break config using an incomplete format specifier (no trailing 's') + self.munge_configini(mods=[ ("format = ", "%%(asctime)s %%(levelname) %%(message)s") ], section="[logging]") + + # load config + with self.assertRaises(configuration.OptionValueError) as cm: + config.load(self.dirname) + + self.assertIn('Unrecognized use of %(...) in: %(levelname)', + cm.exception.args[2]) + + # break config by not dubling % sign to quote it from configparser + self.munge_configini(mods=[ ("format = ", "%(asctime)s %%(levelname) %%(message)s") ], section="[logging]") + + with self.assertRaises( + configuration.ParsingOptionError) as cm: + config.load(self.dirname) + + self.assertEqual(cm.exception.args[0], + "Error in _test_instance/config.ini with section " + "[logging] at option format: Bad value substitution: " + "option 'format' in section 'logging' contains an " + "interpolation key 'asctime' which is not a valid " + "option name. Raw value: '%(asctime)s %%(levelname) " + "%%(message)s'") + def testDictLoggerConfigViaJson(self): # good base test case From cf70a3778c7b12caebeb367f9e490c90ba60c597 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 8 Sep 2025 09:07:12 -0400 Subject: [PATCH 025/290] chore: dependabot upgrade setup-python to 6.0.0 --- .github/workflows/build-xapian.yml | 2 +- .github/workflows/ci-test.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-xapian.yml b/.github/workflows/build-xapian.yml index 00e6fe09..01384615 100644 --- a/.github/workflows/build-xapian.yml +++ b/.github/workflows/build-xapian.yml @@ -46,7 +46,7 @@ jobs: # Setup version of Python to use - name: Set Up Python 3.13 - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6.0.0 with: python-version: 3.13 allow-prereleases: true diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index 35a5a948..c9dac0ce 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -120,7 +120,7 @@ jobs: # Setup version of Python to use - name: Set Up Python ${{ matrix.python-version }} - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6.0.0 with: python-version: ${{ matrix.python-version }} allow-prereleases: true From b9b8478fc2e37b77790b6c099e091b344c5ebe72 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 8 Sep 2025 09:08:14 -0400 Subject: [PATCH 026/290] chore: dependabot upgrade codecov to 5.5.1 --- .github/workflows/ci-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index c9dac0ce..a5498a88 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -323,7 +323,7 @@ jobs: - name: Upload coverage to Codecov # see: https://github.com/codecov/codecov-action#usage - uses: codecov/codecov-action@fdcc8476540edceab3de004e990f80d881c6cc00 # v5.5.0 + uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 # v5.5.1 with: verbose: true token: ${{ secrets.CODECOV_TOKEN }} From 4e4b409650289e011515fa4b71bdb431d1db9e3e Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Tue, 16 Sep 2025 22:53:00 -0400 Subject: [PATCH 027/290] feature: add thread local trace_id and trace_reason to logging. Added trace_id to default logging so that all logs for a given request share the same trace_id. This allows correlation of logs across a request. admin_guide.txt, upgrading.txt: add docs update sample configs to include trace_id. rewrite logging docs in admin_guide. Hopefully they are clearer now. clean up some stuff in the logging config file docs. admin.py: add decorators to run_command to enable trace_id. change calls to db.commit() to use run_command to get trace_id. configuration.py: clean up imports. update docstrings, comments and inline docs. add trace_id to default log format. add function for testing decorated with trace_id. add support for dumping stack trace in logging. add check for pytest in sys.modules to enable log propagation when pytest is running. Otherwise tests fail as the caplog logger doesn't see the roundup logs. logcontext.py: new file to handle thread local contextvar mangement. mailgw.py: add decorators for trace_id etc. scripts/roundup_xlmrpc_server.py: add decorators for trace_id etc. fix encoding bug turning bytes into a string. fix command line issue where we can't set encoding. (not sure if changing encoding via command line even works) cgi/client.py decorate two entry points for trace_id etc. cgi/wsgi_handler.py: decorate entry point for trace_id etc. test/test_config.py: add test for trace_id in new log format. test various cases for sinfo and errors in formating msg. --- CHANGES.txt | 5 + doc/admin_guide.txt | 349 ++++++++++++++++++----- doc/upgrading.txt | 29 ++ roundup/admin.py | 7 +- roundup/cgi/client.py | 8 +- roundup/cgi/wsgi_handler.py | 3 + roundup/configuration.py | 226 ++++++++++++++- roundup/logcontext.py | 208 ++++++++++++++ roundup/mailgw.py | 3 + roundup/scripts/roundup_xmlrpc_server.py | 25 +- test/test_config.py | 133 +++++++++ 11 files changed, 889 insertions(+), 107 deletions(-) create mode 100644 roundup/logcontext.py diff --git a/CHANGES.txt b/CHANGES.txt index 4ec37e17..26cbae60 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -44,6 +44,11 @@ Features: or put them in the input buffer for editing. (John Rouillard) - add format to logging section in config.ini. Used to set default logging format. (John Rouillard) +- the default logging format template includes an identifier unique + for a request. This identifier (trace_id) can be use to identify + logs for a specific transaction. Logging also supports a + trace_reason log token with the url for a web request. The logging + format can be changed in config.ini. (John Rouillard) 2025-07-13 2.5.0 diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index ad92a94a..50ee90b6 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -46,98 +46,284 @@ There's two "installations" that we talk about when using Roundup: both choosing your "tracker home" and the ``main`` -> ``database`` variable in the tracker's config.ini. +.. _rounduplogging: Configuring Roundup Message Logging =================================== You can control how Roundup logs messages using your tracker's -config.ini file. Roundup uses the standard Python (2.3+) logging -implementation. The config file and ``roundup-server`` provide very -basic control over logging. - -Configuration for "BasicLogging" implementation: - - tracker configuration file specifies the location of a log file - ``logging`` -> ``filename`` - - tracker configuration file specifies the level to log to as - ``logging`` -> ``level`` - - tracker configuration file lets you disable other loggers - (e.g. when running under a wsgi framework) with - ``logging`` -> ``disable_loggers``. - - tracker configuration file can set the log format using - ``logging`` -> ``format``. See :ref:`logFormat` for more info. - - ``roundup-server`` specifies the location of a log file on the command - line - - ``roundup-server`` enable using the standard python logger with - the tag/channel ``roundup.http`` on the command line - -By supplying a standard log config file in ini or json (dictionary) -format, you get more control over the logs. You can set different -levels for logs (e.g. roundup.hyperdb can be set to WARNING while -other Roundup log channels are set to INFO and roundup.mailgw logs at -DEBUG level). You can also send the logs for roundup.mailgw to syslog, -and other roundup logs go to an automatically rotating log file, or -are submitted to your log aggregator over https. - -Configuration for standard "logging" module: - - tracker configuration file specifies the location of a logging - configuration file as ``logging`` -> ``config``. - -In both cases, if no logfile is specified then logging will simply be sent -to sys.stderr with only logging of ERROR messages. +config.ini file. Roundup uses the standard Python logging +implementation. The config file and ``roundup-server`` provide +very basic control over logging. + +``roundup-server``'s logging is controlled from the command +line. You can: + +- specify the location of a log file or +- enable logging using the standard Python logging library under + the tag/channel ``roundup.http`` + +Configuration for "BasicLogging" implementation for your tracker +is done using the settings in the tracker's ``config.ini`` under +the ``logging`` section: + +- ``filename`` setting: specifies the location of a log file +- ``level`` setting: specifies the minimum level to log +- ``disable_loggers`` setting: disable other loggers (e.g. when + running under a wsgi framework) +- ``format`` setting: set the log format template. See + :ref:`logFormat` for more info. + +In either case, if logfile is not specified, logging is sent to +sys.stderr. If level is not set, only ERROR or higher priority +log messages will be reported. + +You can get more control over logging by using the ``config`` +setting in the tracker's ``config.ini``. Using a logging config +file overrides all the rest of the other logging settings in +``config.ini``. You get more control over the logs by supplying +a log config file in ini or json (dictionary) format. + +Using this, you can set different levels by channel. For example +roundup.hyperdb can be set to WARNING while other Roundup log +channels are set to INFO and the roundup.mailgw channel logs at +the DEBUG level. You can also control the distribution of +logs. For example roundup.mailgw logs to syslog, other channels +log to an automatically rotating log file, or are submitted to +your log aggregator over https. .. _logFormat: Defining the Log Format ----------------------- -Starting with Roundup 2.6 you can specify the logging format. In the -``logging`` -> ``format`` setting of config.ini you can use any of the -`standard logging LogRecord attributes -`_. -However you must double any ``%`` format markers. The default value -is:: +Starting with Roundup 2.6 you can specify the logging format in +config.ini. The ``logging`` -> ``format`` setting of config.ini +supports all of the `standard logging LogRecord attributes +`_ +or Roundup logging attributes. However you must double any ``%`` +format markers. The default value is:: - %%(asctime)s %%(levelname)s %%(message)s + %%(asctime)s %%(trace_id)s %%(levelname)s %%(message)s -Standard Logging Setup +Roundup Logging Attributes +-------------------------- + +The `logging package has a number of attributes +`_ +that can be expanded in the format template. In addition to the +ones supplied by Python's logging module, Roundup defines +additional attributes: + + trace_id + a unique string that is generated for each request. It is + unique per thread. + + trace_reason + a string describing the reason for the trace/request. + + * the URL for a web triggered (http, rest, xmlrpc) request + * the email message id for an email triggered request + * the roundup-admin os user and start of command. Only first + two words in command are printed so seting a password will + not be leaked to the logs. + + sinfo + the stack traceback information at the time the log call id + made. + + This must be intentionally activated by using the extras + parameter. For example calling:: + + logging.get_logger('roundup.something').warning( + "I am here\n%(sinfo)s", extra={"sinfo": 2}) + + in the function confirmid() of the file detectors/reauth.py + in your demo tracker will print 2 items on the stack + including the log call. It results in the following (5 lines + total in the log file):: + + 2025-09-14 23:07:58,668 Cm0ZPlBaklLZ3Mm6hAAgoC WARNING I am here + File "[...]/roundup/hyperdb.py", line 1924, in fireAuditors + audit(self.db, self, nodeid, newvalues) + File "demo/detectors/reauth.py", line 7, in confirmid + logging.getLogger('roundup.something').warning( + + Note that the output does not include the arguments to + ``warning`` because they are on the following line. If you + want arguments to the log call included, they have to be on + the same line. + + Setting ``sinfo`` to an integer value N includes N lines up + the stack ending with the logging call. Setting it to 0 + includes all the lines in the stack ending with the logging + call. + + If the value is less than 0, the stack dump doesn't end at + the logging call but continues to the function that + generates the stack report. So it includes functions inside + the logging module. + + Setting it to a number larger than the stack trace will + print the trace down to the log call. So using ``-1000`` + will print up to 1000 stack frames and start at the function + that generates the stack report. + + Setting ``sinfo`` to a non-integer value ``{"sinfo": None}`` + will produce 5 lines of the stack trace ending at the + logging call. + + pct_char + produces a single ``%`` sign in the log. The usual way of + embedding a percent sign in a formatted string is to double + it like: ``%%``. However when the format string is specified + in the config.ini file percent signs are manipulated. So + ``%%(pct_char)s`` can be used in config.ini to print a + percent sign. + +The default logging template is defined in config.ini in the +``logging`` -> ``format`` setting. It includes the ``trace_id``. +When searching logs, you can use the trace_id to see all the log +messages associated with a request. + +If you want to log from a detector, extension or other code, you +can use these tokens in your messages when calling the logging +functions. (Note that doubling ``%`` signs is only required when +defining the log format in a config file, not when defining a +msg.) For example:: + + logging.getLogger('roundup.myextension').error('problem with ' + '%(trace_reason)s') + +will include the url in the message when triggered from the +web. This also works with other log methods: ``warning()``, +``debug()``, .... + +Note you must **not** use positional arguments in your +message. Using:: + + logging.getLogger('roundup.myextension').error( + '%s problem with %(trace_reason)s', "a") + +will not properly substitute the argument. You must use mapping +key based arguments and define the local values as part of the +extra dictionary. For example:: + + logging.getLogger('roundup.myextension').error('%(article)s ' + 'problem with %(trace_reason)', + extra={"article": some_local_variable}) + +Also if you are logging any data supplied by a user, you must not +log it directly. If the variable ``url`` contains the url typed in +by the user, never use: + + logger.info(url) + +or + + logger.info("Url is %s" % url) + +Use: + + logger.info("Url is %s", url) + +or + + logger.info("Url is %(url)s", extra={"url": url) + +This prevents printf style tokens in ``url`` from being processed +where it can raise an exception. This could be used to prevent +the log message from being generated. + +More on trace_id +~~~~~~~~~~~~~~~~ + +The trace_id provides a unique token (a UUID4 encoded to make it +shorter) for each transaction in the database. It is unique to +each thread or transaction. A transaction: + + for the web interface is + each web, rest or xmlrpc request + + for the email interface is + each email request. Using pipe mode will generate one + transaction. Using pop/imap etc can generate multiple + transactions, one for each email. Logging that occurs prior + to processing an email transaction has the default + ``not_set`` value for trace_id + + for the roundup-admin interface is + each command in the interactive interface or on the command + line. Plus one transaction when/if a commit happens on + roundup-admin exit. + +When creating scripts written using the roundup package the entry +point should use the ``@gen_trace_id`` decorator. For example to +decorate the entry point that performs one transaction:: + + from roundup.logcontext import gen_trace_id + + # stuff ... + + @gen_trace_id() + def main(...): + ... + +If your script does multiple processing operations, decorate the entry +point for the processing operation:: + + from roundup.logcontext import gen_trace_id + + @gen_trace_id() + def process_one(thing): + ... + + def main(): + for thing in things: + process_one(thing) + + +Advanced Logging Setup ---------------------- If the settings in config.ini are not sufficient for your logging -requirements, you can specify a full logging configuration in one of -two formats: +requirements, you can specify a full logging configuration in one +of two formats: - * `fileConfig format - `_ - in ini style * `dictConfig format `_ using json with comment support + * `fileConfig format + `_ + in ini style -The dictConfig allows more control over configuration including -loading your own log handlers and disabling existing handlers. If you -use the fileConfig format, the ``logging`` -> ``disable_loggers`` flag -in the tracker's config is used to enable/disable pre-existing loggers -as there is no way to do this in the logging config file. +The dictConfig format allows more control over configuration +including loading your own log handlers and disabling existing +handlers. If you use the fileConfig format, the ``logging`` -> +``disable_loggers`` flag in the tracker's config is used to +enable/disable pre-existing loggers as there is no way to do this +in the logging config file. .. _`dictLogConfig`: dictConfig Based Logging Config ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -dictConfigs are specified in JSON format with support for comments. -The file name in the tracker's config for the ``logging`` -> ``config`` -setting must end with ``.json`` to choose the correct processing. +dictConfigs are specified in JSON format with support for +comments. The file name in the tracker's config for the +``logging`` -> ``config`` setting must end with ``.json`` to +choose the correct processing. Comments have to be in one of two forms based on javascript line comments: -1. A ``//`` possibly indented with whitespace on a line is considereda - a comment and is stripped from the file before being passed to the - json parser. This is a "line comment". +1. A ``//`` possibly indented with whitespace on a line is + considereda a comment and is stripped from the file before + being passed to the json parser. This is a "line comment". -2. A ``//`` with at least three white space characters before it is - stripped from the end of the line before begin passed to the json - parser. This is an "inline comment". +2. A ``//`` with at least three white space characters before it + is stripped from the end of the line before being passed to + the json parser. This is an "inline comment". Block style comments are not supported. @@ -151,20 +337,23 @@ Example dictConfig Logging Config ................................. Note that this file is not actually JSON format as it include -comments. However by using javascript style comments, some tools that -expect JSON (editors, linters, formatters) might work with it. A -command like ``sed -e 's#^\s*//.*##' -e 's#\s*\s\s\s//.*##' -logging.json`` can be used to strip comments for programs that need -it. +comments. However by using javascript style comments, some tools +that treat JSON like javascript (editors, linters, formatters) +might work with it. A command like:: + + sed -e 's#^\s*//.*##' -e 's#\s*\s\s\s//.*##' logging.json + +can be used to strip comments for programs that need it. The config below works with the `Waitress wsgi server `_ configured to use the -roundup.wsgi channel. It also controls the `TransLogger middleware -`_ configured to use -roundup.wsgi.translogger, to produce httpd style combined logs. The -log file is specified relative to the current working directory not -the tracker home. The tracker home is the subdirectory demo under the -current working directory. The commented config is:: +roundup.wsgi channel. It also controls the `TransLogger +middleware `_ configured to +use roundup.wsgi.translogger, to produce httpd style combined +logs. The log file is specified relative to the current working +directory not the tracker home. The tracker home is the +subdirectory demo under the current working directory. The +commented config is:: { "version": 1, // only supported version @@ -173,11 +362,12 @@ current working directory. The commented config is:: "formatters": { // standard format for Roundup messages "standard": { - "format": "%(asctime)s %(ctx_id)s %(levelname)s %(name)s:%(module)s %(msg)s" + "format": "%(asctime)s %(trace_id)s %(levelname)s %(name)s:%(module)s %(msg)s" }, // used for waitress wsgi server to produce httpd style logs "http": { - "format": "%(message)s" + "format": "%(message)s %(trace_id)" + } }, "handlers": { @@ -254,12 +444,13 @@ current working directory. The commented config is:: fileConfig Based Logging Config ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -The file config is an older and more limited method of configuring -logging. It is described by the `Configuration file format +The file config is an older and more limited method of +configuring logging. It is described by the `Configuration file +format `_ -in the Python documentation. The file name in the tracker's config for -the ``logging`` -> ``config`` setting must end with ``.ini`` to choose -the correct processing. +in the Python documentation. The file name in the tracker's +config for the ``logging`` -> ``config`` setting must end with +``.ini`` to choose the correct processing. Example fileConfig LoggingConfig ................................ @@ -343,7 +534,7 @@ extension output:: keys=basic,plain [formatter_basic] - format=%(asctime)s %(process)d %(name)s:%(module)s.%(funcName)s,%(levelname)s: %(message)s + format=%(asctime)s %(trace_id)s %(process)d %(name)s:%(module)s.%(funcName)s,%(levelname)s: %(message)s datefmt=%Y-%m-%d %H:%M:%S [formatter_plain] diff --git a/doc/upgrading.txt b/doc/upgrading.txt index e814a635..73d91a3f 100644 --- a/doc/upgrading.txt +++ b/doc/upgrading.txt @@ -108,6 +108,35 @@ your database. Migrating from 2.5.0 to 2.6.0 ============================= +Default Logs Include Unique Request Identifier (info) +----------------------------------------------------- + +The default logging format has been changed from:: + + %(asctime)s %(levelname)s %(message)s + +to:: + + %(asctime)s %(trace_id)s %(levelname)s %(message)s + +So logs now look like:: + + 2025-08-20 03:25:00,308 f6RPbT2s70vvJ2jFb9BQNF DEBUG get user1 cached + +which in the previous format would look like:: + + 2025-08-20 03:25:00,308 DEBUG get user1 cached + +The new format includes ``trace_id`` which is a thread and process +unique identifier for a single request. So you can link together all +of the log lines and determine where a slow down or other +problem occurred. + +The logging format is now a ``config.ini`` parameter in the +``logging`` section with the name ``format``. You can change it if you +would like the old logging format without having to create a logging +configuration file. See :ref:`rounduplogging` for details. + Support authorized changes in your tracker (optional) ----------------------------------------------------- diff --git a/roundup/admin.py b/roundup/admin.py index eb717b73..737afcbe 100644 --- a/roundup/admin.py +++ b/roundup/admin.py @@ -49,6 +49,7 @@ ) from roundup.exceptions import UsageError from roundup.i18n import _, get_translation +from roundup.logcontext import gen_trace_id, store_trace_reason try: from UserDict import UserDict @@ -2354,6 +2355,8 @@ def usageError_feedback(self, message, function): print(function.__doc__) return 1 + @gen_trace_id() + @store_trace_reason('admin') def run_command(self, args): """Run a single command """ @@ -2599,7 +2602,7 @@ def hook(): if self.db and self.db_uncommitted: commit = self.my_input(_('There are unsaved changes. Commit them (y/N)? ')) if commit and commit[0].lower() == 'y': - self.db.commit() + self.run_command(["commit"]) # looks like histfile is saved with mode 600 if self.readline and self.history_features('save_history'): @@ -2674,7 +2677,7 @@ def main(self): # noqa: PLR0912, PLR0911 self.interactive() else: ret = self.run_command(args) - if self.db: self.db.commit() # noqa: E701 + if self.db: self.run_command(["commit"]) # noqa: E701 return ret finally: if self.db: diff --git a/roundup/cgi/client.py b/roundup/cgi/client.py index ed091c50..32290f24 100644 --- a/roundup/cgi/client.py +++ b/roundup/cgi/client.py @@ -56,9 +56,9 @@ class SysCallError(Exception): UsageError, ) -from roundup.mlink_expr import ExpressionError - +from roundup.logcontext import gen_trace_id, store_trace_reason from roundup.mailer import Mailer, MessageSendError +from roundup.mlink_expr import ExpressionError logger = logging.getLogger('roundup') @@ -434,6 +434,8 @@ class Client: # content-type. precompressed_mime_types = ["image/png", "image/jpeg"] + @gen_trace_id() + @store_trace_reason('client') def __init__(self, instance, request, env, form=None, translator=None): # re-seed the random number generator. Is this is an instance of # random.SystemRandom it has no effect. @@ -581,6 +583,8 @@ def setTranslator(self, translator=None): self._ = self.gettext = translator.gettext self.ngettext = translator.ngettext + @gen_trace_id() + @store_trace_reason('client_main') def main(self): """ Wrap the real main in a try/finally so we always close off the db. """ diff --git a/roundup/cgi/wsgi_handler.py b/roundup/cgi/wsgi_handler.py index f730503d..f1b27a36 100644 --- a/roundup/cgi/wsgi_handler.py +++ b/roundup/cgi/wsgi_handler.py @@ -13,6 +13,7 @@ from roundup.anypy.strings import s2b from roundup.cgi import TranslationService from roundup.cgi.client import BinaryFieldStorage +from roundup.logcontext import gen_trace_id, store_trace_reason BaseHTTPRequestHandler = http_.server.BaseHTTPRequestHandler DEFAULT_ERROR_MESSAGE = http_.server.DEFAULT_ERROR_MESSAGE @@ -102,6 +103,8 @@ def __init__(self, home, debug=False, timing=False, lang=None, else: self.preload() + @gen_trace_id() + @store_trace_reason("wsgi") def __call__(self, environ, start_response): """Initialize with `apache.Request` object""" request = RequestHandler(environ, start_response) diff --git a/roundup/configuration.py b/roundup/configuration.py index 6da56200..906f9531 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -11,18 +11,19 @@ import errno import getopt import logging -import logging.config import os import re import smtplib import sys import time +import traceback import roundup.date from roundup.anypy import random_ from roundup.anypy.strings import b2s from roundup.backends import list_backends from roundup.i18n import _ +from roundup.logcontext import gen_trace_id, get_context_info if sys.version_info[0] > 2: import configparser # Python 3 @@ -577,33 +578,36 @@ def get(self): class LoggingFormatOption(Option): - """Replace escaped % (as %%) with single %. + """Escape/unescape logging format string '%(' <-> '%%(' Config file parsing allows variable interpolation using %(keyname)s. However this is exactly the format that we need for creating a logging format string. So we tell the user to quote the string using %%(...). Then we turn %%( -> %( when - retrieved. + retrieved and turn %( into %%( when saving the file. """ - class_description = ("Allowed value: Python logging module named " - "attributes with % sign doubled.") + class_description = ("Allowed value: Python LogRecord attribute named " + "formats with % *sign doubled*.\n" + "Also you can include the following attributes:\n" + " %%(trace_id)s %%(trace_reason)s and %%(pct_char)s" + ) def str2value(self, value): - """Check format of unquoted string looking for missing specifiers. + """Validate and convert value of logging format string. This does a dirty check to see if a token is missing a - specifier. So "%(ascdate)s %(level) " would fail because of - the 's' missing after 'level)'. But "%(ascdate)s %(level)s" + specifier. So "%%(ascdate)s %%(level) " would fail because of + the 's' missing after 'level)'. But "%%(ascdate)s %%(level)s" would pass. - Note that %(foo)s generates a error from the ini parser - with a less than wonderful message. + Note that '%(foo)s' (i.e. unescaped substitution) generates + a error from the ini parser with a less than wonderful message. """ unquoted_val = value.replace("%%(", "%(") # regexp matches all current logging record object attribute names. - scanned_result = re.sub(r'%\([A-Za-z_]+\)\S','', unquoted_val ) + scanned_result = re.sub(r'%\([A-Za-z_]+\)\S', '', unquoted_val) if scanned_result.find('%(') != -1: raise OptionValueError( self, unquoted_val, @@ -618,6 +622,7 @@ def _value2str(self, value): """ return value.replace("%(", "%%(") + class OriginHeadersListOption(Option): """List of space seperated origin header values. @@ -1658,7 +1663,7 @@ def str2value(self, value): "If above 'config' option is set, this option has no effect.\n" "Allowed values: DEBUG, INFO, WARNING, ERROR"), (LoggingFormatOption, "format", - "%(asctime)s %(levelname)s %(message)s", + "%(asctime)s %(trace_id)s %(levelname)s %(message)s", "Format of the logging messages with all '%' signs\n" "doubled so they are not interpreted by the config file."), (BooleanOption, "disable_loggers", "no", @@ -2379,6 +2384,18 @@ def _get_unset_options(self): def _get_name(self): return self["TRACKER_NAME"] + @gen_trace_id() + def _logging_test(self, sinfo, msg="test %(a)s\n%(sinfo)s", args=None): + """Test method for logging formatting. + + Not used in production. + """ + logger = logging.getLogger('roundup') + if args: + logger.info(msg, *args) + else: + logger.info(msg, extra={"a": "a_var", "sinfo": sinfo}) + def reset(self): Config.reset(self) if self.ext: @@ -2387,6 +2404,177 @@ def reset(self): self.detectors.reset() self.init_logging() + def gather_callstack(self, keep_full_stack=False): + # Locate logging call in stack + stack = traceback.extract_stack() + if keep_full_stack: + last_frame_index = len(stack) + else: + for last_frame_index, frame in enumerate(stack): + # Walk from the top of stack looking for + # "logging" in filename (really + # filepath). Can't use /logging/ as + # windows uses \logging\. + # + # Filtering by looking for "logging" in + # the filename isn't great. + if "logging" in frame.filename: + break + if not keep_full_stack: + stack = stack[0:last_frame_index] + return (stack, last_frame_index) + + def context_filter(self, record): + """Add context to record, expand context references in record.msg + Define pct_char as '%' + """ + + # This method can be called multiple times on different handlers. + # However it modifies the record on the first call and the changes + # persist in the record. So we only want to be called once per + # record. + if hasattr(record, "ROUNDUP_CONTEXT_FILTER_CALLED"): + return True + + for name, value in get_context_info(): + if not hasattr(record, name): + setattr(record, name, value) + record.pct_char = "%" + record.ROUNDUP_CONTEXT_FILTER_CALLED = True + + if hasattr(record, "sinfo"): + # sinfo has to be set via extras argument to logging commands + # to activate this. + # + # sinfo value can be: + # non-integer: "", None etc. Print 5 elements of + # stack before logging call + # integer N > 0: print N elements of stack before + # logging call + # 0: print whole stack before logging call + # integer N < 0: undocumented print stack starting at + # extract_stack() below. I.E. do not set bottom of + # stack to the logging call. + # if |N| is greater than stack height, print whole stack. + stack_height = record.sinfo + keep_full_stack = False + + if isinstance(stack_height, int): + if stack_height < 0: + keep_full_stack = True + stack_height = abs(stack_height) + if stack_height == 0: + # None will set value to actual stack height. + stack_height = None + else: + stack_height = 5 + + stack, last_frame_index = self.gather_callstack(keep_full_stack) + + if stack_height is None: + stack_height = last_frame_index + elif stack_height > last_frame_index: + stack_height = last_frame_index # start at frame 0 + + # report the stack info + record.sinfo = "".join( + traceback.format_list( + stack[last_frame_index - stack_height:last_frame_index] + ) + ) + + # if args are present, just return. Logging will + # expand the arguments. + if record.args: + return True + + # Since args not present, try formatting msg using + # named arguments. + try: + record.msg = record.msg % record.__dict__ + except (ValueError, TypeError): + # ValueError: means there is a % sign in the msg + # somewhere that is not meant to be a format token. So + # just leave msg unexpanded. + # + # TypeError - a positional format string is being + # handled without setting record.args. E.G. + # .info("result is %f") + # Leave message unexpanded. + pass + return True + + def add_logging_context_filter(self): + """Update log record with contextvar values and expand msg + + The contextvar values are stored as attributes on the log + record object in record.__dict__. They should not exist + when this is called. Do not overwrite them if they do exist + as they can be set in a logger call using:: + + logging.warning("a message", extra = {"trace_id": foo}) + + the extra argument/parameter. + + Attempt to expand msg using the variables in + record.__dict__. This makes:: + + logging.warning("the URL was: %(trace_reason)s") + + work and replaces the ``%(trace_reason)s`` token with the value. + Note that you can't use positional params and named params + together. For example:: + + logging.warning("user = %s and URL was: %(trace_reason)s", user) + + will result in an exception in logging when it formats the + message. + + Also ``%(pct_char)`` is defined to allow the addition of % + characters in the format string as bare % chars can't make + it past the configparser and %% encoded ones run into issue + with the format verifier. + + Calling a logger (.warning() etc.) with the argument:: + + extra={"sinfo": an_int} + + will result in a stack trace starting at the logger call + and going up the stack for `an_int` frames. Using "True" + in place of `an_int` will print only the call to the logger. + + Note that logging untrusted strings in the msg set by user + (untrusted) may be an issue. So don't do something like: + + .info("%s" % web_url) + + as web_url could include '%(trace_id)s'. Instead use: + + .info("%(url)s", extra=("url": web_url)) + + Even in the problem case, I think the damage is contained since: + + * data doesn't leak as the log string is not exposed to the + user. + + * the log string isn't executed or used internally. + + * log formating can raise an exception. But this won't + affect the application as the exception is swallowed in + the logging package. The raw message would be printed by + the fallback logging handler. + + but if it is a concern, make sure user data is added using + the extra dict when calling one of the logging functions. + """ + loggers = [logging.getLogger(name) for name in + logging.root.manager.loggerDict] + # append the root logger as it is not listed in loggerDict + loggers.append(logging.getLogger()) + for logger in loggers: + for hdlr in logger.handlers: + hdlr.addFilter(self.context_filter) + def load_config_dict_from_json_file(self, filename): import json comment_re = re.compile( @@ -2481,14 +2669,15 @@ def init_logging(self): "Unable to load logging config file. " "File extension must be '.ini' or '.json'.\n" ) - + + self.add_logging_context_filter() return if _file: raise OptionValueError(self.options['LOGGING_CONFIG'], _file, "Unable to find logging config file.") - + _file = self["LOGGING_FILENAME"] # set file & level on the roundup logger logger = logging.getLogger('roundup') @@ -2503,6 +2692,15 @@ def init_logging(self): logger.removeHandler(hdlr) logger.handlers = [hdlr] logger.setLevel(self["LOGGING_LEVEL"] or "ERROR") + if 'pytest' not in sys.modules: + # logger.propatgate is True by default. This is + # needed so that pytest caplog code will work. In + # production leaving it set to True relogs everything + # using the root logger with logging BASIC_FORMAT: + # "%(level)s:%(name)s:%(message)s + logger.propagate = False + + self.add_logging_context_filter() def validator(self, options): """ Validate options once all options are loaded. diff --git a/roundup/logcontext.py b/roundup/logcontext.py new file mode 100644 index 00000000..e88e3a22 --- /dev/null +++ b/roundup/logcontext.py @@ -0,0 +1,208 @@ +import contextvars +import functools +import logging +import os +import uuid + +logger = logging.getLogger("roundup.logcontext") + + +class SimpleSentinel: + """A hack to get a sentinel value where I can define __str__(). + + I was using sentinel = object(). However some code paths + resulted in the sentinel object showing up as an argument + to print or logging.warning|error|debug(...). In this case + seeing "" in the output isn't useful. + + So I created this class (with slots) as a fast method where + I could control the __str__ representation. + + """ + __slots__ = ("name", "str") + + def __init__(self, name=None, str_value=""): + self.name = name + self.str = str_value + + def __str__(self): + # Generate a string without whitespace. + # Used in logging where whitespace could be + # a field delimiter + return ("%s%s" % ( + self.name + "-" if self.name else "", + self.str)).replace(" ", "_") + + def __repr__(self): + return 'SimpleSentinel(name=%s, str_value="%s")' % ( + self.name, self.str) + + +# store the context variable names in a dict. Because +# contactvars.copy_context().items() returns nothing if set has +# not been called on a context var. I need the contextvar names +# even if they have not been set. +ctx_vars = {} + +# set up sentinel values that will print a suitable error value +# and the context vars they are associated with. +_SENTINEL_ID = SimpleSentinel("trace_id", "not set") +ctx_vars['trace_id'] = contextvars.ContextVar("trace_id", default=_SENTINEL_ID) + + +_SENTINEL_REASON = SimpleSentinel("trace_reason", "missing") +ctx_vars['trace_reason'] = contextvars.ContextVar("trace_reason", + default=_SENTINEL_REASON) + + +def shorten_int_uuid(uuid): + """Encode a UUID integer in a shorter form for display. + + A uuid is long. Make a shorter version that takes less room + in a log line. + """ + + alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890" + result = "" + while uuid: + uuid, t = divmod(uuid, len(alphabet)) + result += alphabet[t] + return result or "0" + + +def gen_trace_id(): + """Decorator to generate a trace id (encoded uuid4) and add to contextvar + + The logging routine uses this to label every log line. All + logs with the same trace_id should be generated from a + single request. + + This decorator is applied to an entry point for a request. + Different methods of invoking Roundup have different entry + points. As a result, this decorator can be called multiple + times as some entry points can traverse another entry point + used by a different invocation method. It will not set a + trace_id if one is already assigned. + + A uuid4() is used as the uuid, but to shorten the log line, + the uuid4 integer is encoded into a 62 character ascii + alphabet (A-Za-z0-9). + + This decorator may produce duplicate (colliding) trace_id's + when used with multiple processes on some platforms where + uuid.uuid4().is_safe is unknown. Probability of a collision + is unknown. + + """ + def decorator(func): + @functools.wraps(func) + def wrapper(*args, **kwargs): + prev = None + trace_id = ctx_vars['trace_id'] + if trace_id.get() is _SENTINEL_ID: + prev = trace_id.set(shorten_int_uuid(uuid.uuid4().int)) + try: + r = func(*args, **kwargs) + finally: + if prev: + trace_id.reset(prev) + return r + return wrapper + return decorator + + +def store_trace_reason(location=None): + """Decorator finds and stores a reason trace was started in contextvar. + + Record the url for a regular web triggered request. + Record the message id for an email triggered request. + Record a roundup-admin command/action for roundup-admin request. + + Because the reason can be stored in different locations + depending on where this is called, it is called with a + location hint to activate the right extraction method. + + If the reason has already been stored (and it's not "missing", + it tries to extract it again and verifies it's the same as the + stored reason. If it's not the same it logs an error. This + safety check may be removed in a future version of Roundup. + """ + def decorator(func): + @functools.wraps(func) + def wrapper(*args, **kwargs): + nonlocal location + + reason = None + prev_trace_reason = None + trace_reason = ctx_vars['trace_reason'] + stored_reason = trace_reason.get() + + # Fast return path. Not enabled to make sure SANITY + # CHECK below runs. If the CHECK fails, we have a a + # bad internal issue: contextvars shared between + # threads, roundup modifying reason within a request, ... + # + # if stored_reason is not _SENTINEL_REASON: + # return func(*args, **kwargs) + + # use location to determine how to extract the reason + if location == "wsgi" and 'REQUEST_URI' in args[1]: + reason = args[1]['REQUEST_URI'] + elif location == "client" and 'REQUEST_URI' in args[3]: + reason = args[3]['REQUEST_URI'] + elif location == "mailgw": + reason = args[1].get_header('message-id', "no_message_id") + elif location == "admin": + reason = "roundup-admin(%s): %s" % (os.getlogin(), args[1][:2]) + elif location.startswith("file://"): + reason = location + elif location == "client_main" and 'REQUEST_URI' in args[0].env: + reason = args[0].env['REQUEST_URI'] + elif location == "xmlrpc-server": + reason = args[0].path + + if reason is None: + pass + elif stored_reason is _SENTINEL_REASON: + # no value stored and reason is not none, update + prev_trace_reason = trace_reason.set(reason) + elif reason != stored_reason: # SANITY CHECK + # Throw an error we have mismatched REASON's which + # should never happen. + logger.error("Mismatched REASON's: stored: %s, new: %s at %s", + stored_reason, reason, location) + + try: + r = func(*args, **kwargs) + finally: + # reset context var in case thread is reused for + # another request. + if prev_trace_reason: + trace_reason.reset(prev_trace_reason) + return r + return wrapper + return decorator + + +def get_context_info(): + """Return list of context var tuples [(var_name, var_value), ...]""" + + return [(name, ctx.get()) for name, ctx in ctx_vars.items()] + + +#Is returning a dict for this info more pythonic? +def get_context_dict(): + """Return dict of context var tuples ["var_name": "var_value", ...}""" + return {name: ctx.get() for name, ctx in ctx_vars.items()} + +# Dummy no=op implementation of this module: +# +#def noop_decorator(*args, **kwargs): +# def decorator(func): +# return func +# return decorator +# +#def get_context_info(): +# return [ ("trace_id", "noop_trace_id"), +# ("trace_reason", "noop_trace_reason") ] +#gen_trace_id = store_trace_reason = noop_decorator diff --git a/roundup/mailgw.py b/roundup/mailgw.py index c12d4313..b221d7be 100644 --- a/roundup/mailgw.py +++ b/roundup/mailgw.py @@ -128,6 +128,7 @@ class node. Any parts of other types are each stored in separate files from roundup.anypy.strings import StringIO, b2s, u2s from roundup.hyperdb import iter_roles from roundup.i18n import _ +from roundup.logcontext import gen_trace_id, store_trace_reason from roundup.mailer import Mailer from roundup.dehtml import dehtml @@ -1646,6 +1647,8 @@ def main(self, fp): return self.handle_Message(message_from_binary_file(fp, RoundupMessage)) + @gen_trace_id() + @store_trace_reason("mailgw") def handle_Message(self, message): """Handle an RFC822 Message diff --git a/roundup/scripts/roundup_xmlrpc_server.py b/roundup/scripts/roundup_xmlrpc_server.py index e4c3645a..f9847aa5 100644 --- a/roundup/scripts/roundup_xmlrpc_server.py +++ b/roundup/scripts/roundup_xmlrpc_server.py @@ -8,8 +8,12 @@ # --- patch sys.path to make sure 'import roundup' finds correct version from __future__ import print_function -import sys + +import base64 +import getopt import os.path as osp +import socket +import sys thisdir = osp.dirname(osp.abspath(__file__)) rootdir = osp.dirname(osp.dirname(thisdir)) @@ -19,15 +23,14 @@ sys.path.insert(0, rootdir) # --/ - -import base64, getopt, os, sys, socket -from roundup.anypy import urllib_ -from roundup.xmlrpc import translate -from roundup.xmlrpc import RoundupInstance import roundup.instance -from roundup.instance import TrackerError +from roundup.anypy import urllib_, xmlrpc_ +from roundup.anypy.strings import b2s from roundup.cgi.exceptions import Unauthorised -from roundup.anypy import xmlrpc_ +from roundup.instance import TrackerError +from roundup.logcontext import gen_trace_id, store_trace_url +from roundup.xmlrpc import RoundupInstance, translate + SimpleXMLRPCServer = xmlrpc_.server.SimpleXMLRPCServer SimpleXMLRPCRequestHandler = xmlrpc_.server.SimpleXMLRPCRequestHandler @@ -64,7 +67,7 @@ def authenticate(self, tracker): scheme, challenge = authorization.split(' ', 1) if scheme.lower() == 'basic': - decoded = base64.b64decode(challenge) + decoded = b2s(base64.b64decode(challenge)) if ':' in decoded: username, password = decoded.split(':') else: @@ -85,6 +88,8 @@ def authenticate(self, tracker): db.setCurrentUser(username) return db + @gen_trace_id() + @store_trace_url("xmlrpc-server") def do_POST(self): """Extract username and password from authorization header.""" @@ -149,7 +154,7 @@ def run(): elif opt in ['-p', '--port']: port = int(arg) elif opt in ['-e', '--encoding']: - encoding = encoding + encoding = arg tracker_homes = {} for arg in args: diff --git a/test/test_config.py b/test/test_config.py index b3547cdf..f4229813 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -418,6 +418,10 @@ def testOctalNumberOption(self): @pytest.mark.usefixtures("save_restore_logging") class TrackerConfig(unittest.TestCase): + @pytest.fixture(autouse=True) + def inject_fixtures(self, caplog): + self._caplog = caplog + @pytest.fixture(scope="class") def save_restore_logging(self): """Save logger state and try to restore it after all tests in @@ -1009,7 +1013,136 @@ def testLoadConfigNoConfig(self): print(cm.exception) self.assertEqual(cm.exception.args[0], self.dirname) + def testFormattedLogging(self): + """Depends on using default logging format with %(trace_id)""" + + def find_file_occurances(string): + return len(re.findall(r'\bFile\b', string)) + + config = configuration.CoreConfig() + + config.LOGGING_LEVEL = "DEBUG" + config.init_logging() + + # format the record and verify the logformat/trace_id. + config._logging_test(None, msg="message") + tuple = self._caplog.record_tuples[0] + self.assertEqual(tuple[1], 20) + self.assertEqual("message", tuple[2]) + logger = logging.getLogger('roundup') + hdlr = logger.handlers[0] + log = hdlr.format(self._caplog.records[0]) + # verify that %(trace_id) was set and substituted + # Note: trace_id is not initialized in this test case + log_parts = log.split() + self.assertRegex(log_parts[2], r'^[A-Za-z0-9]{22}') + self._caplog.clear() + + # the rest check various values of sinfo and msg formating. + + # sinfo = 1 - one line of stack starting with log call + config._logging_test(1) + tuple = self._caplog.record_tuples[0] + self.assertEqual(tuple[1], 20) + self.assertIn("test a_var\n File", tuple[2]) + self.assertIn("in _logging_test", tuple[2]) + self.assertIn("logger.info(msg, extra=", tuple[2]) + self.assertEqual(find_file_occurances(tuple[2]), 1) + self._caplog.clear() + + # sinfo = None - 5 lines of stack starting with log call + config._logging_test(None) + tuple = self._caplog.record_tuples[0] + self.assertEqual(tuple[1], 20) + self.assertIn("test a_var\n File", tuple[2]) + self.assertIn("in _logging_test", tuple[2]) + self.assertIn("logger.info(msg, extra=", tuple[2]) + self.assertEqual(find_file_occurances(tuple[2]), 5) + self._caplog.clear() + + # sinfo = 0 - whole stack starting with log call + config._logging_test(0) + tuple = self._caplog.record_tuples[0] + self.assertEqual(tuple[1], 20) + self.assertIn("test a_var\n File", tuple[2]) + self.assertIn("in _logging_test", tuple[2]) + self.assertIn("logger.info(msg, extra=", tuple[2]) + # A file in 'pytest' directory should be at the top of stack. + self.assertIn("pytest", tuple[2]) + # no idea how deep the actual stack is, could change with python + # versions, but 3.12 is 33 so .... + self.assertTrue(find_file_occurances(tuple[2]) > 10) + self._caplog.clear() + + # sinfo = -1 - one line of stack starting with extract_stack() + config._logging_test(-1) + tuple = self._caplog.record_tuples[0] + self.assertEqual(tuple[1], 20) + self.assertIn("test a_var\n File", tuple[2]) + # The call to extract_stack should be included as the frame + # at bottom of stack. + self.assertIn("extract_stack()", tuple[2]) + # only one frame included + self.assertEqual(find_file_occurances(tuple[2]), 1) + self._caplog.clear() + + # sinfo = 1000 - whole stack starting with log call 1000>stack size + config._logging_test(1000) + tuple = self._caplog.record_tuples[0] + self.assertEqual(tuple[1], 20) + self.assertIn("test a_var\n File", tuple[2]) + self.assertIn("in _logging_test", tuple[2]) + self.assertIn("logger.info(msg, extra=", tuple[2]) + # A file in 'pytest' directory should be at the top of stack. + self.assertIn("pytest", tuple[2]) + # no idea how deep the actual stack is, could change with python + # versions, but 3.12 is 33 so .... + self.assertTrue(find_file_occurances(tuple[2]) > 10) + self._caplog.clear() + + # sinfo = -1000 - whole stack starting with extract_stack + config._logging_test(-1000) + tuple = self._caplog.record_tuples[0] + self.assertEqual(tuple[1], 20) + self.assertIn("test a_var\n File", tuple[2]) + self.assertIn("in _logging_test", tuple[2]) + self.assertIn("logger.info(msg, extra=", tuple[2]) + # The call to extract_stack should be included as the frame + # at bottom of stack. + self.assertIn("extract_stack()", tuple[2]) + # A file in 'pytest' directory should be at the top of stack. + # no idea how deep the actual stack is, could change with python + # versions, but 3.12 is 33 so .... + self.assertTrue(find_file_occurances(tuple[2]) > 10) + self.assertIn("pytest", tuple[2]) + self._caplog.clear() + + # pass args and compatible message + config._logging_test(None, args=(1,2,3), + msg="one: %s, two: %s, three: %s" + ) + tuple = self._caplog.record_tuples[0] + self.assertEqual(tuple[1], 20) + self.assertEqual('one: 1, two: 2, three: 3', tuple[2]) + self._caplog.clear() + + # error case for incorrect placeholder + config._logging_test(None, msg="%(a)") + tuple = self._caplog.record_tuples[0] + self.assertEqual(tuple[1], 20) + self.assertEqual("%(a)", tuple[2]) + self._caplog.clear() + + # error case for incompatible format record is the first argument + # and it can't be turned into floating point. + config._logging_test(None, msg="%f") + tuple = self._caplog.record_tuples[0] + self.assertEqual(tuple[1], 20) + self.assertEqual("%f", tuple[2]) + self._caplog.clear() + + def testXhtmlRaisesOptionError(self): self.munge_configini(mods=[ ("html_version = ", "xhtml") ]) From 97446ad58a3e79d34e98f619b82a9a8d45c27bc1 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 17 Sep 2025 00:45:04 -0400 Subject: [PATCH 028/290] bug, test: fix tests for trace_id; readd import logging.config Made save_restore_logging a test level fixture. It was a class level which worked fine until I started using caplog for tests in the same class. Due to loading config from dict, the roundup channel was set to not propagate which broke the new formatting test used for trace_id. Forgot to update some tests due to change in default format adding %(trace_id). Also re-added logging.config import which broke loading logging config files in configuration.py. --- roundup/configuration.py | 1 + test/test_config.py | 30 ++++++++++++++---------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/roundup/configuration.py b/roundup/configuration.py index 906f9531..1c9ba55d 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -11,6 +11,7 @@ import errno import getopt import logging +import logging.config import os import re import smtplib diff --git a/test/test_config.py b/test/test_config.py index f4229813..baae7522 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -422,7 +422,7 @@ class TrackerConfig(unittest.TestCase): def inject_fixtures(self, caplog): self._caplog = caplog - @pytest.fixture(scope="class") + @pytest.fixture(autouse=True) def save_restore_logging(self): """Save logger state and try to restore it after all tests in this class have finished. @@ -461,7 +461,7 @@ def save_restore_logging(self): # cribbed from configuration.py:init_loggers hdlr = logging.StreamHandler(sys.stdout) formatter = logging.Formatter( - '%(asctime)s %(levelname)s %(message)s') + '%(asctime)s %(trace_id)s %(levelname)s %(message)s') hdlr.setFormatter(formatter) for logger in roundup_loggers: @@ -1019,11 +1019,7 @@ def testFormattedLogging(self): def find_file_occurances(string): return len(re.findall(r'\bFile\b', string)) - config = configuration.CoreConfig() - - config.LOGGING_LEVEL = "DEBUG" - config.init_logging() - + config = configuration.CoreConfig(settings={"LOGGING_LEVEL": "DEBUG"}) # format the record and verify the logformat/trace_id. config._logging_test(None, msg="message") @@ -1036,7 +1032,9 @@ def find_file_occurances(string): # verify that %(trace_id) was set and substituted # Note: trace_id is not initialized in this test case log_parts = log.split() - self.assertRegex(log_parts[2], r'^[A-Za-z0-9]{22}') + # testing len(shorten_int_uuid(uuid.uuid4().int)) + # for 20000 tests gives range [19,22] + self.assertRegex(log_parts[2], r'^[A-Za-z0-9]{19,22}') self._caplog.clear() # the rest check various values of sinfo and msg formating. @@ -1250,25 +1248,25 @@ def testLoggerFormat(self): # verify config is initalized to defaults self.assertEqual(config['LOGGING_FORMAT'], - '%(asctime)s %(levelname)s %(message)s') + '%(asctime)s %(trace_id)s %(levelname)s %(message)s') # load config config.load(self.dirname) self.assertEqual(config['LOGGING_FORMAT'], - '%(asctime)s %(levelname)s %(message)s') + '%(asctime)s %(trace_id)s %(levelname)s %(message)s') # break config using an incomplete format specifier (no trailing 's') - self.munge_configini(mods=[ ("format = ", "%%(asctime)s %%(levelname) %%(message)s") ], section="[logging]") + self.munge_configini(mods=[ ("format = ", "%%(asctime)s %%(trace_id)s %%(levelname) %%(message)s") ], section="[logging]") # load config with self.assertRaises(configuration.OptionValueError) as cm: config.load(self.dirname) - self.assertIn('Unrecognized use of %(...) in: %(levelname)', + self.assertIn('Unrecognized use of %(...) in: %(levelname)', cm.exception.args[2]) - # break config by not dubling % sign to quote it from configparser - self.munge_configini(mods=[ ("format = ", "%(asctime)s %%(levelname) %%(message)s") ], section="[logging]") + # break config by not doubling % sign to quote it from configparser + self.munge_configini(mods=[ ("format = ", "%(asctime)s %%(trace_id)s %%(levelname) %%(message)s") ], section="[logging]") with self.assertRaises( configuration.ParsingOptionError) as cm: @@ -1279,8 +1277,8 @@ def testLoggerFormat(self): "[logging] at option format: Bad value substitution: " "option 'format' in section 'logging' contains an " "interpolation key 'asctime' which is not a valid " - "option name. Raw value: '%(asctime)s %%(levelname) " - "%%(message)s'") + "option name. Raw value: '%(asctime)s %%(trace_id)s " + "%%(levelname) %%(message)s'") def testDictLoggerConfigViaJson(self): From 41fa7cc503b8b736c568234889465b3d3787bc98 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 17 Sep 2025 01:05:35 -0400 Subject: [PATCH 029/290] fix: remove nolocal on unset param in logcontext.py flake8 was failing in ci on it. --- roundup/logcontext.py | 1 - 1 file changed, 1 deletion(-) diff --git a/roundup/logcontext.py b/roundup/logcontext.py index e88e3a22..8b2c72f4 100644 --- a/roundup/logcontext.py +++ b/roundup/logcontext.py @@ -130,7 +130,6 @@ def store_trace_reason(location=None): def decorator(func): @functools.wraps(func) def wrapper(*args, **kwargs): - nonlocal location reason = None prev_trace_reason = None From 0bd284b85d0bf5c2d151f075f9c9f73482fa5a74 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 17 Sep 2025 01:31:12 -0400 Subject: [PATCH 030/290] fix: os.getlogin fails with OSError in CI. --- roundup/logcontext.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/roundup/logcontext.py b/roundup/logcontext.py index 8b2c72f4..03226639 100644 --- a/roundup/logcontext.py +++ b/roundup/logcontext.py @@ -152,7 +152,11 @@ def wrapper(*args, **kwargs): elif location == "mailgw": reason = args[1].get_header('message-id', "no_message_id") elif location == "admin": - reason = "roundup-admin(%s): %s" % (os.getlogin(), args[1][:2]) + try: + login = os.getlogin() + except OSError: + login = "unknown" + reason = "roundup-admin(%s): %s" % (login, args[1][:2]) elif location.startswith("file://"): reason = location elif location == "client_main" and 'REQUEST_URI' in args[0].env: From 949af33cd5a324ef7e6a7a24ea91991835ddc22f Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 17 Sep 2025 01:33:09 -0400 Subject: [PATCH 031/290] fix: use getloing value --unknown-- on error. In case there is a real user with name unknown, Unlikely for account to to have dashes in name. --- roundup/logcontext.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roundup/logcontext.py b/roundup/logcontext.py index 03226639..0371e28d 100644 --- a/roundup/logcontext.py +++ b/roundup/logcontext.py @@ -155,7 +155,7 @@ def wrapper(*args, **kwargs): try: login = os.getlogin() except OSError: - login = "unknown" + login = "--unknown--" reason = "roundup-admin(%s): %s" % (login, args[1][:2]) elif location.startswith("file://"): reason = location From 43eb594d8ff2fecf21d9fdd3963ced98f2512703 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 17 Sep 2025 19:58:08 -0400 Subject: [PATCH 032/290] bug: fix json logging config file syntax exception/fix test for windows If the json logging config file has mimatched {} or [], it raises an IndexError. Handle that case and test it. Also handle embedded filenames in tests when testsare run on windows:(/ vs \ directory sep). --- roundup/configuration.py | 6 +++- test/test_config.py | 64 +++++++++++++++++++++++++--------------- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/roundup/configuration.py b/roundup/configuration.py index 1c9ba55d..736521a1 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -2604,7 +2604,11 @@ def load_config_dict_from_json_file(self, filename): error_at_doc_line = e.lineno # subtract 1 - zero index on config_list # remove '\n' for display - line = config_list[error_at_doc_line - 1][:-1] + try: + line = config_list[error_at_doc_line - 1][:-1] + except IndexError: + line = _("Error found at end of file. Maybe missing a " + "block closing '}'.") hint = "" if line.find('//') != -1: diff --git a/test/test_config.py b/test/test_config.py index baae7522..c32cd91e 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1169,14 +1169,15 @@ def testCopyConfig(self): self.assertEqual(config['STATIC_FILES'], None) # load config + STATIC_FILES = os.path.join(self.dirname, "html2") config.load(self.dirname) - self.assertEqual(config['STATIC_FILES'], ['_test_instance/html2']) + self.assertEqual(config['STATIC_FILES'], [ STATIC_FILES ]) # copy config config_copy = config.copy() # this should work - self.assertEqual(config_copy['STATIC_FILES'], ['_test_instance/html2']) + self.assertEqual(config_copy['STATIC_FILES'], [STATIC_FILES]) @skip_py2 def testConfigValueInterpolateError(self): @@ -1272,13 +1273,14 @@ def testLoggerFormat(self): configuration.ParsingOptionError) as cm: config.load(self.dirname) - self.assertEqual(cm.exception.args[0], - "Error in _test_instance/config.ini with section " + ini_path = os.path.join(self.dirname, "config.ini") + self.assertEqual(cm.exception.args[0],( + f"Error in {ini_path} with section " "[logging] at option format: Bad value substitution: " "option 'format' in section 'logging' contains an " "interpolation key 'asctime' which is not a valid " "option name. Raw value: '%(asctime)s %%(trace_id)s " - "%%(levelname) %%(message)s'") + "%%(levelname) %%(message)s'")) def testDictLoggerConfigViaJson(self): @@ -1538,26 +1540,41 @@ def testDictLoggerConfigViaJson(self): "%s'\n" % (log_config_filename, access_filename)) self.assertEqual(output, target) - def test_missing_logging_config_file(self): - saved_config = self.db.config['LOGGING_CONFIG'] - - self.db.config['LOGGING_CONFIG'] = 'logging.json' - - with self.assertRaises(configuration.OptionValueError) as cm: - self.db.config.init_logging() + # mess up '}' so json file block isn't properly closed. + test_config = config1.replace( + ' }', + ' ') - self.assertEqual(cm.exception.args[1], "_test_instance/logging.json") - self.assertEqual(cm.exception.args[2], - "Unable to find logging config file.") + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) - self.db.config['LOGGING_CONFIG'] = 'logging.ini' + # file is made relative to tracker dir. + self.db.config["LOGGING_CONFIG"] = '_test_log_config.json' + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() - with self.assertRaises(configuration.OptionValueError) as cm: - self.db.config.init_logging() + output = cm.exception.args[0].replace(r'\\','\\') + target = ("Error parsing json logging dict " + "(%s) near \n\n" + " Error found at end of file. Maybe missing a " + "block closing '}'.\n\n" + "Expecting ',' delimiter: line 86 column 1." % + (log_config_filename,)) + self.assertEqual(output, target) - self.assertEqual(cm.exception.args[1], "_test_instance/logging.ini") - self.assertEqual(cm.exception.args[2], - "Unable to find logging config file.") + def test_missing_logging_config_file(self): + saved_config = self.db.config['LOGGING_CONFIG'] + + for logging_file in ["logging.json", "logging.ini", "logging.foobar"]: + self.db.config['LOGGING_CONFIG'] = logging_file + + with self.assertRaises(configuration.OptionValueError) as cm: + self.db.config.init_logging() + + logging_configfile = os.path.join(self.dirname, logging_file) + self.assertEqual(cm.exception.args[1], logging_configfile) + self.assertEqual(cm.exception.args[2], + "Unable to find logging config file.") self.db.config['LOGGING_CONFIG'] = saved_config @@ -1566,11 +1583,12 @@ def test_unknown_logging_config_file_type(self): self.db.config['LOGGING_CONFIG'] = 'schema.py' - + with self.assertRaises(configuration.OptionValueError) as cm: self.db.config.init_logging() - self.assertEqual(cm.exception.args[1], "_test_instance/schema.py") + logging_configfile = os.path.join(self.dirname, "schema.py") + self.assertEqual(cm.exception.args[1], logging_configfile) self.assertEqual(cm.exception.args[2], "Unable to load logging config file. " "File extension must be '.ini' or '.json'.\n") From 29adc16b5beb6e978f071bd52fe0386c6d055ee3 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sat, 20 Sep 2025 16:49:38 -0400 Subject: [PATCH 033/290] bug: improve error reporting for errors for logging fileConfig. --- CHANGES.txt | 2 + roundup/configuration.py | 33 +++++- test/test_config.py | 236 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 266 insertions(+), 5 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 26cbae60..7e89ce71 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -29,6 +29,8 @@ Fixed: - in roundup-admin, using 'pragma history_length interactively now sets readline history length. Using -P history_length=10 on the command line always worked. (John Rouillard) +- enhanced error reporting for errors in ini style logging + configuration. (John Rouillard) Features: diff --git a/roundup/configuration.py b/roundup/configuration.py index 736521a1..07fbc38a 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -2634,9 +2634,36 @@ def init_logging(self): _file = self["LOGGING_CONFIG"] if _file and os.path.isfile(_file): if _file.endswith(".ini"): - logging.config.fileConfig( - _file, - disable_existing_loggers=self["LOGGING_DISABLE_LOGGERS"]) + try: + logging.config.fileConfig( + _file, + disable_existing_loggers=self[ + "LOGGING_DISABLE_LOGGERS"]) + except (ValueError, RuntimeError, + KeyError, NameError, ModuleNotFoundError) as e: + # configparser.DuplicateOptionError includes + # filename, line number and a useful error. + # so we don't have to augment it. + context = [] + if hasattr(e, '__context__') and e.__context__: + # get additional error info. + context.append(str(e.__context__)) + if hasattr(e, '__doc__') and e.__doc__: + context.append(e.__doc__) + + if isinstance(e, KeyError): + context.append("No section found with this name.") + if not context: + context = ["No additional information available."] + + raise LoggingConfigError( + "Error loading logging config from %(file)s.\n\n" + " %(msg)s\n\n%(context)s\n" % { + "file": _file, + "msg": e.args[0], + "context": " ".join(context), + } + ) elif _file.endswith(".json"): config_dict = self.load_config_dict_from_json_file(_file) try: diff --git a/test/test_config.py b/test/test_config.py index c32cd91e..9db9a947 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -15,6 +15,7 @@ # BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE, # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. +import configparser import errno import fileinput import logging @@ -424,8 +425,8 @@ def inject_fixtures(self, caplog): @pytest.fixture(autouse=True) def save_restore_logging(self): - """Save logger state and try to restore it after all tests in - this class have finished. + """Save logger state and try to restore it after each test + has finished. The primary test is testDictLoggerConfigViaJson which can change the loggers and break tests that depend on caplog @@ -484,6 +485,18 @@ def save_restore_logging(self): logging.shutdown() reload(logging) + def reset_logging(self): + """https://til.tafkas.net/posts/-resetting-python-logging-before-running-tests/""" + loggers = [logging.getLogger(name) for name in logging.root.manager.loggerDict] + loggers.append(logging.getLogger()) + for logger in loggers: + handlers = logger.handlers[:] + for handler in handlers: + logger.removeHandler(handler) + handler.close() + logger.setLevel(logging.NOTSET) + logger.propagate = True + backend = 'anydbm' def setUp(self): @@ -1562,6 +1575,225 @@ def testDictLoggerConfigViaJson(self): (log_config_filename,)) self.assertEqual(output, target) + def testIniFileLoggerConfig(self): + + # good base test case + config1 = dedent(""" + [loggers] + keys=root,roundup,roundup.http,roundup.hyperdb,actions,schema,extension,detector + + [logger_root] + #DEBUG, INFO, WARNING, ERROR, CRITICAL + #also for root only NOTSET (all) + level=DEBUG + handlers=basic + + [logger_roundup] + #DEBUG, INFO, WARNING, ERROR, CRITICAL + #also for root only NOTSET (all) + level=DEBUG + handlers=rotate + qualname=roundup + propagate=0 + + [logger_roundup.http] + level=INFO + handlers=rotate_weblog + qualname=roundup.http + propagate=0 + + [logger_roundup.hyperdb] + level=WARNING + handlers=rotate + qualname=roundup.hyperdb + propagate=0 + + [logger_actions] + #DEBUG, INFO, WARNING, ERROR, CRITICAL + #also for root only NOTSET (all) + level=DEBUG + handlers=rotate + qualname=actions + propagate=0 + + [logger_detector] + #DEBUG, INFO, WARNING, ERROR, CRITICAL + #also for root only NOTSET (all) + level=DEBUG + handlers=rotate + qualname=detector + propagate=0 + + [logger_schema] + level=DEBUG + handlers=rotate + qualname=schema + propagate=0 + + [logger_extension] + level=DEBUG + handlers=rotate + qualname=extension + propagate=0 + + [handlers] + keys=basic,rotate,rotate_weblog + + [handler_basic] + class=StreamHandler + args=(sys.stderr,) + formatter=basic + + [handler_rotate] + class=logging.handlers.RotatingFileHandler + args=('roundup.log','a', 512000, 2) + formatter=basic + + [handler_rotate_weblog] + class=logging.handlers.RotatingFileHandler + args=('httpd.log','a', 512000, 2) + formatter=plain + + [formatters] + keys=basic,plain + + [formatter_basic] + format=%(asctime)s %(name)s:%(module)s.%(funcName)s,%(levelname)s: %(message)s + datefmt=%Y-%m-%d %H:%M:%S + + [formatter_plain] + format=%(message)s + """) + + log_config_filename = os.path.join(self.instance.tracker_home, + "_test_log_config.ini") + + # happy path + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(config1) + + self.db.config.LOGGING_CONFIG = "_test_log_config.ini" + + # verify we have a clean environment + self.assertEqual(len(logging.getLogger('roundup.http').handlers), 0) + + # always returns None + self.db.config.init_logging() + + # verify that logging loaded and handler is set + # default log config doesn't define handlers for roundup.http + self.assertEqual(len(logging.getLogger('roundup.http').handlers), 1) + self.reset_logging() + + # use undefined enumeration + test_config = config1.replace("=DEBUG\n", "=DEBUF\n") + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + + # verify that logging was reset + # default log config doesn't define handlers for roundup.http + self.assertEqual(len(logging.getLogger('roundup.http').handlers), 0) + + self.assertEqual( + cm.exception.args[0].replace(r'\\','\\'), + ('Error loading logging config from %s.\n\n' + " Unknown level: 'DEBUF'\n\n" + 'Inappropriate argument value (of correct type).\n' % + log_config_filename) + ) + self.reset_logging() + + + # add a syntax error "= foo" + test_config = config1.replace("=DEBUG\n", "=DEBUG\n=foo\n", 1) + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + + # verify that logging was reset + # default log config doesn't define handlers for roundup.http + self.assertEqual(len(logging.getLogger('roundup.http').handlers), 0) + + self.assertEqual( + cm.exception.args[0].replace(r'\\','\\'), + ("Error loading logging config from %(filename)s.\n\n" + " %(filename)s is invalid: Source contains parsing errors: " + "'%(filename)s'\n\t[line 9]: '=foo\\n'\n\n" + "Source contains parsing errors: '%(filename)s'\n" + "\t[line 9]: '=foo\\n' Unspecified run-time error.\n" % + {"filename": log_config_filename}) + ) + self.reset_logging() + + # handler = basic to handler = basi + test_config = config1.replace("handlers=basic\n", "handlers=basi\n", 1) + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + + # verify that logging was reset + # default log config doesn't define handlers for roundup.http + self.assertEqual(len(logging.getLogger('roundup.http').handlers), 0) + + self.assertEqual( + cm.exception.args[0].replace(r'\\','\\'), + ("Error loading logging config from %(filename)s.\n\n" + " basi\n\n" + "Mapping key not found. No section found with this name.\n" % + {"filename": log_config_filename}) + ) + self.reset_logging() + + # Change class to missing class + test_config = config1.replace("class=StreamHandler\n", + "class=SHAndler\n", 1) + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + + # verify that logging was reset + # default log config doesn't define handlers for roundup.http + self.assertEqual(len(logging.getLogger('roundup.http').handlers), 0) + + self.assertEqual( + cm.exception.args[0].replace(r'\\','\\'), + ("Error loading logging config from %(filename)s.\n\n" + " No module named 'SHAndler'\n\n" + "name 'SHAndler' is not defined Module not found.\n" % + {"filename": log_config_filename}) + ) + self.reset_logging() + + # remove section to cause duplicate option definition + test_config = config1.replace("[logger_roundup.http]\n", + "\n", 1) + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + with self.assertRaises(configparser.DuplicateOptionError) as cm: + config = self.db.config.init_logging() + + # verify that logging was reset + # default log config doesn't define handlers for roundup.http + self.assertEqual(len(logging.getLogger('roundup.http').handlers), 0) + + self.assertEqual( + str(cm.exception).replace(r'\\','\\'), + ("While reading from '%(filename)s' [line 20]: " + "option 'level' in section 'logger_roundup' already exists" % + {"filename": log_config_filename}) + ) + self.reset_logging() + def test_missing_logging_config_file(self): saved_config = self.db.config['LOGGING_CONFIG'] From 044c744a0e1918e7eef3fa2f59317215ad705cb4 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 22 Sep 2025 12:37:21 -0400 Subject: [PATCH 034/290] chore(build): update anchore to 7.0.0 via dependabot --- .github/workflows/anchore.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/anchore.yml b/.github/workflows/anchore.yml index 3ab5b7cf..6e1c2135 100644 --- a/.github/workflows/anchore.yml +++ b/.github/workflows/anchore.yml @@ -43,7 +43,7 @@ jobs: - name: List the Docker image run: docker image ls - name: Run the Anchore scan action itself with GitHub Advanced Security code scanning integration enabled - uses: anchore/scan-action@1638637db639e0ade3258b51db49a9a137574c3e # 6.5.1 + uses: anchore/scan-action@f6601287cdb1efc985d6b765bbf99cb4c0ac29d8 # 7.0.0 id: scan with: image: "localbuild/testimage:latest" From 6c46419082fe2eb5e61357fa633bd6bc2cb8d350 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 22 Sep 2025 13:09:49 -0400 Subject: [PATCH 035/290] fix: python 3.{7,8,10} raise configfile.ParsingError not RuntimeError when adding broken syntax: =foo to logging config file. These version failing in CI. 3.9 and 3.11 are also affected. RuntimeError was added in 3.12. --- roundup/configuration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roundup/configuration.py b/roundup/configuration.py index 07fbc38a..f62a7e1b 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -2639,7 +2639,7 @@ def init_logging(self): _file, disable_existing_loggers=self[ "LOGGING_DISABLE_LOGGERS"]) - except (ValueError, RuntimeError, + except (ValueError, RuntimeError, configparser.ParsingError, KeyError, NameError, ModuleNotFoundError) as e: # configparser.DuplicateOptionError includes # filename, line number and a useful error. From 1ee6116b772a85d9858d0d3df80090b1f900d904 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 22 Sep 2025 13:57:52 -0400 Subject: [PATCH 036/290] fix: python < 3.12 returns ParsingError not RuntimeError; print exception RuntimeError added in 3.12. Support older configparser.ParsingError. Supporting older version also requires different test string for the two versions. Also use str(exception) rather than exception.args[0] for producing message to user. Include exception name in output. --- roundup/configuration.py | 4 ++-- test/test_config.py | 41 +++++++++++++++++++++++++++------------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/roundup/configuration.py b/roundup/configuration.py index 07fbc38a..6984aafc 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -2639,7 +2639,7 @@ def init_logging(self): _file, disable_existing_loggers=self[ "LOGGING_DISABLE_LOGGERS"]) - except (ValueError, RuntimeError, + except (ValueError, RuntimeError, configparser.ParsingError, KeyError, NameError, ModuleNotFoundError) as e: # configparser.DuplicateOptionError includes # filename, line number and a useful error. @@ -2660,7 +2660,7 @@ def init_logging(self): "Error loading logging config from %(file)s.\n\n" " %(msg)s\n\n%(context)s\n" % { "file": _file, - "msg": e.args[0], + "msg": type(e).__name__ + ": " + str(e), "context": " ".join(context), } ) diff --git a/test/test_config.py b/test/test_config.py index 9db9a947..e85b14f7 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1700,7 +1700,7 @@ def testIniFileLoggerConfig(self): self.assertEqual( cm.exception.args[0].replace(r'\\','\\'), ('Error loading logging config from %s.\n\n' - " Unknown level: 'DEBUF'\n\n" + " ValueError: Unknown level: 'DEBUF'\n\n" 'Inappropriate argument value (of correct type).\n' % log_config_filename) ) @@ -1718,16 +1718,31 @@ def testIniFileLoggerConfig(self): # verify that logging was reset # default log config doesn't define handlers for roundup.http self.assertEqual(len(logging.getLogger('roundup.http').handlers), 0) - - self.assertEqual( - cm.exception.args[0].replace(r'\\','\\'), - ("Error loading logging config from %(filename)s.\n\n" - " %(filename)s is invalid: Source contains parsing errors: " - "'%(filename)s'\n\t[line 9]: '=foo\\n'\n\n" - "Source contains parsing errors: '%(filename)s'\n" - "\t[line 9]: '=foo\\n' Unspecified run-time error.\n" % - {"filename": log_config_filename}) - ) + + output = cm.exception.args[0].replace(r'\\','\\') + + if sys.version_info >= (3, 12, 0): + expected = ( + "Error loading logging config from %(filename)s.\n\n" + " RuntimeError: %(filename)s is invalid: Source contains parsing errors: " + "'%(filename)s'\n\t[line 9]: '=foo\\n'\n\n" + "Source contains parsing errors: '%(filename)s'\n" + "\t[line 9]: '=foo\\n' Unspecified run-time error.\n" % + {"filename": log_config_filename}) + + else: # 3.7 <= x < 3.12.0 + + expected = ( + "Error loading logging config from %(filename)s.\n\n" + + " ParsingError: Source contains parsing errors: " + "'%(filename)s'\n" + "\t[line 9]: '=foo\\n'\n\n" + + "Raised when a configuration file does not follow legal " + "syntax.\n" % {"filename": log_config_filename}) + + self.assertEqual(output, expected) self.reset_logging() # handler = basic to handler = basi @@ -1745,7 +1760,7 @@ def testIniFileLoggerConfig(self): self.assertEqual( cm.exception.args[0].replace(r'\\','\\'), ("Error loading logging config from %(filename)s.\n\n" - " basi\n\n" + " KeyError: 'basi'\n\n" "Mapping key not found. No section found with this name.\n" % {"filename": log_config_filename}) ) @@ -1767,7 +1782,7 @@ def testIniFileLoggerConfig(self): self.assertEqual( cm.exception.args[0].replace(r'\\','\\'), ("Error loading logging config from %(filename)s.\n\n" - " No module named 'SHAndler'\n\n" + " ModuleNotFoundError: No module named 'SHAndler'\n\n" "name 'SHAndler' is not defined Module not found.\n" % {"filename": log_config_filename}) ) From cd75cc6d834da22df1156c621fd7a67eae41cbe7 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 22 Sep 2025 14:08:36 -0400 Subject: [PATCH 037/290] merge multiple fix: python < 3.12 returns ParsingError not RuntimeError; print exception --- roundup/configuration.py | 2 +- test/test_config.py | 39 +++++++++++++++++++++++++++------------ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/roundup/configuration.py b/roundup/configuration.py index f62a7e1b..6984aafc 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -2660,7 +2660,7 @@ def init_logging(self): "Error loading logging config from %(file)s.\n\n" " %(msg)s\n\n%(context)s\n" % { "file": _file, - "msg": e.args[0], + "msg": type(e).__name__ + ": " + str(e), "context": " ".join(context), } ) diff --git a/test/test_config.py b/test/test_config.py index 9db9a947..931d3f32 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1700,7 +1700,7 @@ def testIniFileLoggerConfig(self): self.assertEqual( cm.exception.args[0].replace(r'\\','\\'), ('Error loading logging config from %s.\n\n' - " Unknown level: 'DEBUF'\n\n" + " ValueError: Unknown level: 'DEBUF'\n\n" 'Inappropriate argument value (of correct type).\n' % log_config_filename) ) @@ -1718,16 +1718,31 @@ def testIniFileLoggerConfig(self): # verify that logging was reset # default log config doesn't define handlers for roundup.http self.assertEqual(len(logging.getLogger('roundup.http').handlers), 0) + + output = cm.exception.args[0].replace(r'\\','\\') + + if sys.version_info >= (3, 12, 0): + expected = ( + "Error loading logging config from %(filename)s.\n\n" + " %(filename)s is invalid: Source contains parsing errors: " + "'%(filename)s'\n\t[line 9]: '=foo\\n'\n\n" + "Source contains parsing errors: '%(filename)s'\n" + "\t[line 9]: '=foo\\n' Unspecified run-time error.\n" % + {"filename": log_config_filename}) + + else: # 3.7 <= x < 3.12.0 + + expected = ( + "Error loading logging config from %(filename)s.\n\n" + " ParsingError: Source contains parsing errors: " + "'%(filename)s'\n" + "\t[line 9]: '=foo\\n'\n\n" + "Raised when a configuration file does not follow legal " + "syntax.\n" % {"filename": log_config_filename}) + + print(output) - self.assertEqual( - cm.exception.args[0].replace(r'\\','\\'), - ("Error loading logging config from %(filename)s.\n\n" - " %(filename)s is invalid: Source contains parsing errors: " - "'%(filename)s'\n\t[line 9]: '=foo\\n'\n\n" - "Source contains parsing errors: '%(filename)s'\n" - "\t[line 9]: '=foo\\n' Unspecified run-time error.\n" % - {"filename": log_config_filename}) - ) + self.assertEqual(output, expected) self.reset_logging() # handler = basic to handler = basi @@ -1745,7 +1760,7 @@ def testIniFileLoggerConfig(self): self.assertEqual( cm.exception.args[0].replace(r'\\','\\'), ("Error loading logging config from %(filename)s.\n\n" - " basi\n\n" + " KeyError: 'basi'\n\n" "Mapping key not found. No section found with this name.\n" % {"filename": log_config_filename}) ) @@ -1767,7 +1782,7 @@ def testIniFileLoggerConfig(self): self.assertEqual( cm.exception.args[0].replace(r'\\','\\'), ("Error loading logging config from %(filename)s.\n\n" - " No module named 'SHAndler'\n\n" + " ModuleNotFoundError: No module named 'SHAndler'\n\n" "name 'SHAndler' is not defined Module not found.\n" % {"filename": log_config_filename}) ) From 1fa42d5083cf294bc988f18f03dc0857a459abb9 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Thu, 25 Sep 2025 23:30:07 -0400 Subject: [PATCH 038/290] fix: make user_src_input generate valid javascript user_src_input used to generate False if edit_ok == False in this statement: tal:attributes="onblur python:edit_ok and 'split_name(this)'; but False isn't a boolean in javascript, so it throws an error in the console. Changed to use: tal:attributes="onblur python:'split_name(this)' if edit_ok else ''; which generates an empty onblur if the field is not editable. --- share/roundup/templates/classic/html/page.html | 2 +- share/roundup/templates/devel/html/page.html | 2 +- share/roundup/templates/minimal/html/page.html | 2 +- share/roundup/templates/responsive/html/page.html | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/share/roundup/templates/classic/html/page.html b/share/roundup/templates/classic/html/page.html index cbd531c7..1ad23964 100644 --- a/share/roundup/templates/classic/html/page.html +++ b/share/roundup/templates/classic/html/page.html @@ -369,7 +369,7 @@

body title

diff --git a/share/roundup/templates/devel/html/page.html b/share/roundup/templates/devel/html/page.html index 7a43b32b..c21375f4 100644 --- a/share/roundup/templates/devel/html/page.html +++ b/share/roundup/templates/devel/html/page.html @@ -424,7 +424,7 @@

body title

diff --git a/share/roundup/templates/minimal/html/page.html b/share/roundup/templates/minimal/html/page.html index 106cafc0..0dbde6d8 100644 --- a/share/roundup/templates/minimal/html/page.html +++ b/share/roundup/templates/minimal/html/page.html @@ -331,7 +331,7 @@

body title

diff --git a/share/roundup/templates/responsive/html/page.html b/share/roundup/templates/responsive/html/page.html index 7651c2af..b7689ef5 100644 --- a/share/roundup/templates/responsive/html/page.html +++ b/share/roundup/templates/responsive/html/page.html @@ -439,7 +439,7 @@

body title

From fa26b6d106b40f72720700c1aaa6efb682bae480 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Fri, 26 Sep 2025 16:08:30 -0400 Subject: [PATCH 039/290] fix: update updating.txt doc for user_src_input bug I originally thought it was not worth documenting because the error case had no impact. But deployed copies of the trackers can be updated to reduce differences between deployed and distributed files. --- CHANGES.txt | 4 +++- doc/upgrading.txt | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 7e89ce71..07a800f5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -31,7 +31,9 @@ Fixed: command line always worked. (John Rouillard) - enhanced error reporting for errors in ini style logging configuration. (John Rouillard) - +- fix bogus javascript emitted by user_src_input macro. (John + Rouillard) + Features: - add support for authorized changes. User can be prompted to enter diff --git a/doc/upgrading.txt b/doc/upgrading.txt index 73d91a3f..9b3fcd7b 100644 --- a/doc/upgrading.txt +++ b/doc/upgrading.txt @@ -177,6 +177,26 @@ stripped before being processed by the logging system. You can read about the details in the :ref:`admin manual `. +Fix user.item.html template producing invalid Javascript (optional) +------------------------------------------------------------------- + +The html template ``page.html`` in the classic, devel, minimal, and +responsive tracker templates define a ``user_src_input`` macro. This +macro produces invalid javascript for the ``onblur`` event when used +by ``user.item.html``. The only effect from this bug is a javascript +error reported in the user's browser when the user does not have edit +permissions on the page. It doesn't have any user visible impact. + +If you want to fix this, replace:: + + tal:attributes="onblur python:edit_ok and 'split_name(this)'; + +with:: + + tal:attributes="onblur python:'split_name(this)' if edit_ok else ''; + +in the ``html/page.html`` file in your tracker. + .. index:: Upgrading; 2.4.0 to 2.5.0 Migrating from 2.4.0 to 2.5.0 From eedc6dec1dd7e8f76e6dfbd634519b6491e2dc41 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sat, 27 Sep 2025 17:26:32 -0400 Subject: [PATCH 040/290] fix: make version_check enforce version is 3.7+ --- roundup/version_check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/roundup/version_check.py b/roundup/version_check.py index d20840a2..35d6fe24 100644 --- a/roundup/version_check.py +++ b/roundup/version_check.py @@ -1,10 +1,10 @@ #!/usr/bin/env python -# Roundup requires Python 2.7+ as mentioned in doc\installation.txt +# Roundup requires Python 3.7+ as mentioned in doc\installation.txt from __future__ import print_function import sys -VERSION_NEEDED = (2, 7) +VERSION_NEEDED = (3, 7) if sys.version_info < VERSION_NEEDED: print("Content-Type: text/plain\n") From 678652a166875a35fc27f80877d3a37d6d919109 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Tue, 30 Sep 2025 18:08:14 -0400 Subject: [PATCH 041/290] doc: remove uWSGI references as it is in maint mode and not being updated Replaced with Waitress where appropriate. --- doc/admin_guide.txt | 8 ++++---- doc/installation.txt | 19 +------------------ 2 files changed, 5 insertions(+), 22 deletions(-) diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index 50ee90b6..a6730bbf 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -738,7 +738,7 @@ on the fly, compression is enabled by default, to disable it set:: dynamic_compression = No in the tracker's ``config.ini``. You should disable compression if -your proxy (e.g. nginx or apache) or wsgi server (uwsgi) is configured +your proxy (e.g. nginx or apache) is configured to compress responses on the fly. The python standard library includes gzip support. For brotli or zstd you will need to install packages. See the `installation documentation`_ for details. @@ -900,9 +900,9 @@ There are two ways to add a CSP: Fixed CSP --------- -If you are using a web server (Apache, Nginx) to run Roundup, you can -add a ``Content-Security-Policy`` header using that server. WSGI -servers like uWSGI can also be configured to add headers. An example +If you are using a web server (Apache, Nginx) to run Roundup, you +can add a ``Content-Security-Policy`` header using that +server. WSGI middleware can be written to add headers. An example header would look like:: Content-Security-Policy: default-src 'self' 'unsafe-inline' 'strict-dynamic'; diff --git a/doc/installation.txt b/doc/installation.txt index 14d5b008..24a97367 100644 --- a/doc/installation.txt +++ b/doc/installation.txt @@ -1283,7 +1283,7 @@ FastCGI (Cherokee, Hiawatha, lighttpd) The Hiawatha and lighttpd web servers can run Roundup using FastCGI. Cherokee can run FastCGI but it also supports wsgi directly using a -wsgi server like uWSGI, Gnuicorn etc. +wsgi server like Waitress, Gnuicorn etc. To run Roundup using FastCGI, the flup_ package can be used under Python 2 and Python 3. We don't have a detailed config for this, but @@ -1499,23 +1499,6 @@ including putting it behind a proxy, IPV6 support etc. .. _`See the Waitress docs`: https://docs.pylonsproject.org/projects/waitress/en/stable/ -.. index:: pair: web interface; uWSGI - single: wsgi; uWSGI - -uWSGI Installation -~~~~~~~~~~~~~~~~~~ - -For a basic roundup install using uWSGI behind a front end server, -install uwsgi and the python3 (or python) plugin. Then run:: - - uwsgi --http-socket 127.0.0.1:8917 \ - --plugin python3 --mount=/tracker=wsgi.py \ - --manage-script-name --callable app - -using the same wsgi.py as was used for Gunicorn. If you get path not -found errors, check the mount option. The /tracker entry must match -the path used for the [tracker] web value in the tracker's config.ini. - Configure an Email Interface ============================ From f677b91ed1bee9c69e754ccbe704ff6d17e8f353 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 6 Oct 2025 12:29:16 -0400 Subject: [PATCH 042/290] build: upgrade https://github.com/roundup-tracker/roundup/pull/65.patch --- .github/workflows/ossf-scorecard.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ossf-scorecard.yml b/.github/workflows/ossf-scorecard.yml index 28075031..acfba006 100644 --- a/.github/workflows/ossf-scorecard.yml +++ b/.github/workflows/ossf-scorecard.yml @@ -40,7 +40,7 @@ jobs: persist-credentials: false - name: "Run analysis" - uses: ossf/scorecard-action@05b42c624433fc40578a4040d5cf5e36ddca8cde # v5.2.1 + uses: ossf/scorecard-action@4eaacf0543bb3f2c246792bd56e8cdeffafb205a # v5.2.1 with: results_file: results.sarif results_format: sarif From 0fe1d65f6db22de26902e2f1af98fa8c96d667b7 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 6 Oct 2025 23:59:23 -0400 Subject: [PATCH 043/290] test: fix version test --- test/test_misc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_misc.py b/test/test_misc.py index 931dab43..0aee93fa 100644 --- a/test/test_misc.py +++ b/test/test_misc.py @@ -106,7 +106,7 @@ def test_Version_Check(self): # test for valid versions from roundup.version_check import VERSION_NEEDED - self.assertEqual((2, 7), VERSION_NEEDED) + self.assertEqual((3, 7), VERSION_NEEDED) del(sys.modules['roundup.version_check']) @@ -124,7 +124,7 @@ def test_Version_Check(self): sys.stdout = capturedOutput from roundup.version_check import VERSION_NEEDED sys.stdout = sys.__stdout__ - self.assertIn("Roundup requires Python 2.7", capturedOutput.getvalue()) + self.assertIn("Roundup requires Python 3.7", capturedOutput.getvalue()) # reset to valid values for future tests sys.exit = real_exit From c634b7997142c9c00a24e5a348adc0528e797c51 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Thu, 9 Oct 2025 13:49:07 -0400 Subject: [PATCH 044/290] doc: update url for irker to point to github. --- detectors/irker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/detectors/irker.py b/detectors/irker.py index 57ce2bfb..ff330cbb 100644 --- a/detectors/irker.py +++ b/detectors/irker.py @@ -1,6 +1,6 @@ # This detector sends notification on IRC through an irker daemon -# (http://www.catb.org/esr/irker/) when issues are created or messages -# are added. +# (https://gitlab.com/esr/irker formerly http://www.catb.org/esr/irker/) +# when issues are created or messages are added. # # Written by Ezio Melotti # From bc90fff8526272424f0f6cd01a013b63540d06fd Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sat, 11 Oct 2025 18:18:54 -0400 Subject: [PATCH 045/290] refactor: replace boolean expr with named function call Replace a confusing boolean expression with a the use_cached_tracker() method call. --- roundup/cgi/wsgi_handler.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/roundup/cgi/wsgi_handler.py b/roundup/cgi/wsgi_handler.py index f1b27a36..bfea660f 100644 --- a/roundup/cgi/wsgi_handler.py +++ b/roundup/cgi/wsgi_handler.py @@ -97,12 +97,16 @@ def __init__(self, home, debug=False, timing=False, lang=None, else: self.translator = None - if "cache_tracker" not in self.feature_flags or \ - self.feature_flags["cache_tracker"] is not False: + if self.use_cached_tracker(): self.tracker = roundup.instance.open(self.home, not self.debug) else: self.preload() + def use_cached_tracker(self): + return ( + "cache_tracker" not in self.feature_flags or + self.feature_flags["cache_tracker"] is not False) + @gen_trace_id() @store_trace_reason("wsgi") def __call__(self, environ, start_response): @@ -135,8 +139,7 @@ def __call__(self, environ, start_response): else: form = BinaryFieldStorage(fp=environ['wsgi.input'], environ=environ) - if "cache_tracker" not in self.feature_flags or \ - self.feature_flags["cache_tracker"] is not False: + if self.use_cached_tracker(): client = self.tracker.Client(self.tracker, request, environ, form, self.translator) try: From 27281f8df35bf5edb3f8eb2f8a46b186d9744568 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 20 Oct 2025 09:56:47 -0400 Subject: [PATCH 046/290] chore: dependabot update anchore 7.0.0 -> 7.0.2 --- .github/workflows/anchore.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/anchore.yml b/.github/workflows/anchore.yml index 6e1c2135..8dcbf3f8 100644 --- a/.github/workflows/anchore.yml +++ b/.github/workflows/anchore.yml @@ -43,7 +43,7 @@ jobs: - name: List the Docker image run: docker image ls - name: Run the Anchore scan action itself with GitHub Advanced Security code scanning integration enabled - uses: anchore/scan-action@f6601287cdb1efc985d6b765bbf99cb4c0ac29d8 # 7.0.0 + uses: anchore/scan-action@a5605eb0943e46279cb4fbd9d44297355d3520ab # 7.0.2 id: scan with: image: "localbuild/testimage:latest" From 3671c2f54de7f307b6df72c24448595bdca8b9bc Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Fri, 31 Oct 2025 20:55:01 -0400 Subject: [PATCH 047/290] fix: replace localhost with ip address in case localhost is ipv6 addr?? Norbert had an issue with his docker container where the healthcheck was timing out/failing. He diagnosed it as a ipv6 address bound to localhost. Not sure if this is the right fix. Might be better to determine where localhost is bound (V4 or V6 address) but I don't have an environment I can test with. --- CHANGES.txt | 4 +++- scripts/Docker/roundup_healthcheck | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 07a800f5..2422c627 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -33,7 +33,9 @@ Fixed: configuration. (John Rouillard) - fix bogus javascript emitted by user_src_input macro. (John Rouillard) - +- replaced hostname localhost with 127.0.0.1 in docker healthcheck + script. Found/patch by Norbert Schlemmer. (John Rouillard) + Features: - add support for authorized changes. User can be prompted to enter diff --git a/scripts/Docker/roundup_healthcheck b/scripts/Docker/roundup_healthcheck index dfa9dc25..426d6649 100755 --- a/scripts/Docker/roundup_healthcheck +++ b/scripts/Docker/roundup_healthcheck @@ -1,7 +1,10 @@ #! /bin/sh +# change this if you are not using +localhost=127.0.0.1 + # if there are multiple trackers, d=demo t=tracker ... # returns last one for testing that server is up. Does not test # each tracker. tracker=$(ps -ef | sed -ne '/roundup-server/s/^.*\s\(\w*\)=.*$/\1/p') -wget -q -O /dev/null --proxy off --no-verbose http://localhost:8080/"${tracker:-demo}"/ +wget -q -O /dev/null --proxy off --no-verbose http://$localhost:8080/"${tracker:-demo}"/ From dffd8f455b4c5cc57356eb27db093729bd8ee811 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Fri, 31 Oct 2025 20:57:26 -0400 Subject: [PATCH 048/290] chore: dependabot upgrade anchore scan from 7.0.2 to 7.1.0 --- .github/workflows/anchore.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/anchore.yml b/.github/workflows/anchore.yml index 8dcbf3f8..45e4b05d 100644 --- a/.github/workflows/anchore.yml +++ b/.github/workflows/anchore.yml @@ -43,7 +43,7 @@ jobs: - name: List the Docker image run: docker image ls - name: Run the Anchore scan action itself with GitHub Advanced Security code scanning integration enabled - uses: anchore/scan-action@a5605eb0943e46279cb4fbd9d44297355d3520ab # 7.0.2 + uses: anchore/scan-action@568b89d27fc18c60e56937bff480c91c772cd993 # 7.1.0 id: scan with: image: "localbuild/testimage:latest" From ba54d0f95f428bce8368de0b3604ad32c1bab293 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Fri, 31 Oct 2025 20:58:31 -0400 Subject: [PATCH 049/290] chore: dependabot upgrade upload-artifact from 4.6.2 to 5.0.0 pull #68 --- .github/workflows/ossf-scorecard.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ossf-scorecard.yml b/.github/workflows/ossf-scorecard.yml index acfba006..a1fc81dc 100644 --- a/.github/workflows/ossf-scorecard.yml +++ b/.github/workflows/ossf-scorecard.yml @@ -62,7 +62,7 @@ jobs: # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF # format to the repository Actions tab. - name: "Upload artifact" - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 with: name: SARIF file path: results.sarif From 998cc4c5837674d0ac120aeab84ec546cfadd8c4 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 2 Nov 2025 20:38:12 -0500 Subject: [PATCH 050/290] chore: update ci test to include 3.14/3.14t drop 3.12 Drop 3.12 to reduce number of runs/usage. Keep 3.10 IIRC there were some major changes in that version so it's a good canary. Swap out 3.13t for 3.14t. --- .github/workflows/ci-test.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index a5498a88..bb989035 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -52,14 +52,14 @@ jobs: # Run in all these versions of Python python-version: # - "2.7" + - "3.14" - "3.13" - # - 3.6 run via include on ubuntu 20.04 - # - "3.7" + # - "3.7" run via include for ubuntu-22.04 # - "3.8" run via include for ubuntu-22.04 # - "3.9" - "3.10" # - "3.11" - - "3.12" + # - "3.12" # use for multiple os or ubuntu versions #os: [ubuntu-latest, macos-latest, windows-latest] @@ -74,7 +74,7 @@ jobs: # example: if this version fails the jobs still succeeds # allow-prereleases in setup-python allows alpha/beta # releases to run. Also allow free threaded python testing - - python-version: 3.13t + - python-version: 3.14t os: ubuntu-24.04 experimental: true From 410691d6540eafb4cb084ffa01fa19270ea8fcad Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 2 Nov 2025 21:10:28 -0500 Subject: [PATCH 051/290] chore: update base image to python 3.14 alpine release. 3-alpine has changed to python 3.14. --- scripts/Docker/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/Docker/Dockerfile b/scripts/Docker/Dockerfile index b3424cd1..0567793e 100644 --- a/scripts/Docker/Dockerfile +++ b/scripts/Docker/Dockerfile @@ -26,7 +26,7 @@ ARG source=local # Note this is the index digest for the image, not the manifest digest. # The index digest is shared across archetectures (amd64, arm64 etc.) # while the manifest digest is unique per platform/arch. -ARG SHA256=9ba6d8cbebf0fb6546ae71f2a1c14f6ffd2fdab83af7fa5669734ef30ad48844 +ARG SHA256=8373231e1e906ddfb457748bfc032c4c06ada8c759b7b62d9c73ec2a3c56e710 # Set to any non-empty value to enable shell debugging for troubleshooting ARG VERBOSE= @@ -37,7 +37,7 @@ ARG appdir=/usr/src/app # Python version as a.b Used as path component for # installation directory and COPY from install dir # in second build stage. -ARG pythonversion=3.13 +ARG pythonversion=3.14 #FROM python:3-alpine via SHA256 sum FROM python@sha256:$SHA256 AS build From 6e43272e4e09c331c234c99431a1027b054ce21d Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 3 Nov 2025 00:13:04 -0500 Subject: [PATCH 052/290] refactor: change some classes to use __slots__ Speed up access to and reduce size of some low level classes. A few classes in security.py, rest.py are heavily used. But for all, it prevents adding random properties to lower level classes that people shouldn't be mucking with. While doing this I found some test cases accessing an invalid property name and this change caused the cases to crash. admin.py: Use new method Role.props_dict() and Permission.props_dict() where original code just referenced __dict__ when printing Role/Permission. mlink_expr.py: Add slots to multiple classes. Classes Binary and Unary set real properties/attributes. Classes that inherit from them (Equals, Empty, Not, Or, And) define empty slots tuple to eliminate need for __dict__. Class Expression also gets a slot. rate_limit.py: RateLimit and Gcra classes get slots. A couple of pep8 fixes: sort imports, remove trailing spaces on a line, remove unused noqa comment. rest.py: Add slots to class SimulateFieldStorageFromJson and FsValue classes. The memory savings from this could be useful as well as speedier access to the attributes. security.py: Add slots to Permission class. To prevent conflict between slot limit_perm_to_props_only and the class variable of the same name, rename the class variable to limit_perm_to_props_only_default. Also define method props_dict() to allow other code to get a dict to iterate over when checking permissions. Add slots to class Role along with props_dict() method. Add slots to class Security. Also have to add explicit __dict__ slot to support test override of the hasPermission() method. Add props_dict() method, currently unused, but added for symmetry. support.py: TruthDict and PrioList gets slots. test/test_cgi.py: Fix incorrect setting of permission property. Was setting permissions. So testing may not have been doing what we thought it was. Multiple places found with this typo. Remove setting of permissions in some places where it should have no effect on the test and looks like it was just copypasta. test/test_xmlrpc.py Remove setting of permissions in some places where it should have no effect on the test and looks like it was just copypasta. --- CHANGES.txt | 4 +++- roundup/admin.py | 6 +++--- roundup/mlink_expr.py | 16 ++++++++++++++++ roundup/rate_limit.py | 12 +++++++++--- roundup/rest.py | 9 +++++++++ roundup/security.py | 38 ++++++++++++++++++++++++++++++++++---- roundup/support.py | 6 ++++++ test/test_cgi.py | 5 ++--- test/test_xmlrpc.py | 1 - 9 files changed, 82 insertions(+), 15 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 2422c627..fe133a2e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -35,7 +35,9 @@ Fixed: Rouillard) - replaced hostname localhost with 127.0.0.1 in docker healthcheck script. Found/patch by Norbert Schlemmer. (John Rouillard) - +- change some internal classes to use __slots__ for hopefully a small + performance improvement. (John Rouillard) + Features: - add support for authorized changes. User can be prompted to enter diff --git a/roundup/admin.py b/roundup/admin.py index 737afcbe..14250694 100644 --- a/roundup/admin.py +++ b/roundup/admin.py @@ -250,7 +250,7 @@ def help_commands(self): if seq in h: commands.append(' ' + h.split(seq, 1)[1].lstrip()) break - + commands.sort() commands.append(_( """Commands may be abbreviated as long as the abbreviation @@ -2084,9 +2084,9 @@ def do_security(self, args): sys.stdout.write(_('New Email users get the Role "%(role)s"\n') % locals()) roles.sort() for _rolename, role in roles: - sys.stdout.write(_('Role "%(name)s":\n') % role.__dict__) + sys.stdout.write(_('Role "%(name)s":\n') % role.props_dict()) for permission in role.permission_list(): - d = permission.__dict__ + d = permission.props_dict() if permission.klass: if permission.properties: sys.stdout.write(_( diff --git a/roundup/mlink_expr.py b/roundup/mlink_expr.py index 19792179..0686e2da 100644 --- a/roundup/mlink_expr.py +++ b/roundup/mlink_expr.py @@ -60,6 +60,8 @@ def __repr__(self): class Binary: + __slots__ = ("x", "y") + def __init__(self, x, y): self.x = x self.y = y @@ -71,6 +73,8 @@ def visit(self, visitor): class Unary: + __slots__ = ("x",) + def __init__(self, x): self.x = x @@ -83,6 +87,8 @@ def visit(self, visitor): class Equals(Unary): + __slots__ = () + def evaluate(self, v): return self.x in v @@ -95,6 +101,8 @@ def __repr__(self): class Empty(Unary): + __slots__ = () + def evaluate(self, v): return not v @@ -107,6 +115,8 @@ def __repr__(self): class Not(Unary): + __slots__ = () + def evaluate(self, v): return not self.x.evaluate(v) @@ -119,6 +129,8 @@ def __repr__(self): class Or(Binary): + __slots__ = () + def evaluate(self, v): return self.x.evaluate(v) or self.y.evaluate(v) @@ -133,6 +145,8 @@ def __repr__(self): class And(Binary): + __slots__ = () + def evaluate(self, v): return self.x.evaluate(v) and self.y.evaluate(v) @@ -185,6 +199,8 @@ def compile_expression(opcodes): class Expression: + __slots__ = ("evaluate",) + def __init__(self, v, is_link=False): try: opcodes = [int(x) for x in v] diff --git a/roundup/rate_limit.py b/roundup/rate_limit.py index facf1fc2..fec23218 100644 --- a/roundup/rate_limit.py +++ b/roundup/rate_limit.py @@ -4,7 +4,7 @@ # set/get_tat and marshaling as string, support for testonly # and status method. -from datetime import timedelta, datetime +from datetime import datetime, timedelta try: # used by python 3.11 and newer use tz aware dates @@ -19,12 +19,15 @@ dt_epoch = datetime(1970, 1, 1) def fromisoformat(date): # only for naive dates - return datetime.strptime(date, "%Y-%m-%dT%H:%M:%S.%f") + return datetime.strptime(date, "%Y-%m-%dT%H:%M:%S.%f") from roundup.anypy.datetime_ import utcnow class RateLimit: # pylint: disable=too-few-public-methods + + __slots__ = ("count", "period") + def __init__(self, count, period): self.count = count self.period = period @@ -35,6 +38,9 @@ def inverse(self): class Gcra: + + __slots__ = ("memory",) + def __init__(self): self.memory = {} @@ -124,7 +130,7 @@ def status(self, key, limit): # One item is dequeued every limit.inverse seconds. ret['Retry-After'] = str(int(limit.inverse)) ret['Retry-After-Timestamp'] = "%s" % \ - (now + timedelta(seconds=limit.inverse)) # noqa: E127 + (now + timedelta(seconds=limit.inverse)) else: # if we are not rejected, the user can post another # attempt immediately. diff --git a/roundup/rest.py b/roundup/rest.py index 80078dcc..9de60b1a 100644 --- a/roundup/rest.py +++ b/roundup/rest.py @@ -426,6 +426,9 @@ def execute(cls, instance, path, method, input_payload): func = func_obj['func'] # zip the varlist into a dictionary, and pass it to the caller + # FIXME: 3.10 is min version- add strict=True to zip + # also wrap with try/except ValueError if different number of + # items (which should never happen). args = dict(zip(list_vars, match_obj.groups())) args['input_payload'] = input_payload return func(instance, **args) @@ -2741,6 +2744,9 @@ class SimulateFieldStorageFromJson(): string. ''' + + __slots__ = ("json_dict", "value") + def __init__(self, json_string): '''Parse the json string into an internal dict. @@ -2764,6 +2770,9 @@ def raise_error_on_constant(x): raise ValueError(e.args[0] + ". JSON is: " + json_string) class FsValue: + + __slots__ = ("name", "value") + '''Class that does nothing but response to a .value property ''' def __init__(self, name, val): self.name = u2s(name) diff --git a/roundup/security.py b/roundup/security.py index 09555ae2..3e5afb59 100644 --- a/roundup/security.py +++ b/roundup/security.py @@ -18,6 +18,7 @@ class Permission: - properties (optional) - check function (optional) - props_only (optional, internal field is limit_perm_to_props_only) + default value taken from Permission.limit_perm_to_props_only_default. - filter function (optional) returns filter arguments for determining which records are visible by the user. The filter function comes into play when determining if a set of nodes @@ -58,7 +59,18 @@ class Permission: with a True or False value. ''' - limit_perm_to_props_only = False + __slots__ = ( + "_properties_dict", + "check", + "check_version", + "description", + "filter", + "klass", + "limit_perm_to_props_only", + "name", + "properties") + + limit_perm_to_props_only_default = False def __init__(self, name='', description='', klass=None, properties=None, check=None, props_only=None, filter=None): @@ -80,7 +92,7 @@ def __init__(self, name='', description='', klass=None, # a == b will be true. if props_only is None: self.limit_perm_to_props_only = \ - Permission.limit_perm_to_props_only + Permission.limit_perm_to_props_only_default else: # see note on use of bool() in set_props_only_default() self.limit_perm_to_props_only = bool(props_only) @@ -123,6 +135,9 @@ def check(db, userid, itemid): return check + def props_dict(self): + return {name: getattr(self, name) for name in self.__slots__} + def test(self, db, permission, classname, property, userid, itemid): ''' Test permissions 5 args: permission - string like Edit, Register etc. Required, no wildcard. @@ -222,6 +237,9 @@ class Role: - description - permissions ''' + + __slots__ = ("_permissions", "description", "name") + def __init__(self, name='', description='', permissions=None): self.name = name.lower() self.description = description @@ -301,6 +319,9 @@ def permission_list(self): perm_list.sort(key=lambda x: (x.klass or '', x.name)) return perm_list + def props_dict(self): + return {name: getattr(self, name) for name in self.__slots__} + def searchable(self, classname, propname): for perm_name in 'View', 'Search': # Only permissions without a check method @@ -321,6 +342,12 @@ def searchable(self, classname, propname): class Security: + + # __dict__ is needed to allow mocking of db.security.hasPermission + # in test/test_templating.py. Define slots for properties used in + # production to increase speed. + __slots__ = ("__dict__", "db", "permission", "role") + def __init__(self, db): ''' Initialise the permission and role classes, and add in the base roles (for admin user). @@ -452,6 +479,9 @@ def is_filterable(self, permission, userid, classname): return False return True + def props_dict(self): + return {name: getattr(self, name) for name in self.__slots__} + def roleHasSearchPermission(self, classname, property, *rolenames): """ For each of the given roles, check the permissions. Property can be a transitive property. @@ -544,11 +574,11 @@ def set_props_only_default(self, props_only=None): # will be compared as part of tuple == tuple and # (3,) == (True,) is False even though 3 is a True value # in a boolean context. So use bool() to coerce value. - Permission.limit_perm_to_props_only = \ + Permission.limit_perm_to_props_only_default = \ bool(props_only) def get_props_only_default(self): - return Permission.limit_perm_to_props_only + return Permission.limit_perm_to_props_only_default def addPermissionToRole(self, rolename, permission, classname=None, properties=None, check=None, props_only=None): diff --git a/roundup/support.py b/roundup/support.py index f52d7bca..80e99998 100644 --- a/roundup/support.py +++ b/roundup/support.py @@ -11,6 +11,9 @@ class TruthDict: '''Returns True for valid keys, False for others. ''' + + __slots__ = ('keys',) + def __init__(self, keys): if keys: self.keys = {} @@ -49,6 +52,9 @@ class PrioList: 7 ''' + + __slots__ = ('key', 'list', 'sorted') + def __init__(self, key=None): self.list = [] self.key = key diff --git a/test/test_cgi.py b/test/test_cgi.py index 73f6ca71..b954b63b 100644 --- a/test/test_cgi.py +++ b/test/test_cgi.py @@ -1973,7 +1973,7 @@ def testClassPermission(self): actions.EditItemAction(cl).handle) def testCheckAndPropertyPermission(self): - self.db.security.permissions = {} + self.db.security.permission = {} def own_record(db, userid, itemid): return userid == itemid p = self.db.security.addPermission(name='Edit', klass='user', @@ -2004,7 +2004,7 @@ def own_record(db, userid, itemid): def testCreatePermission(self): # this checks if we properly differentiate between create and # edit permissions - self.db.security.permissions = {} + self.db.security.permission = {} self.db.security.addRole(name='UserAdd') # Don't allow roles p = self.db.security.addPermission(name='Create', klass='user', @@ -2061,7 +2061,6 @@ def testCreatePermission(self): def testSearchPermission(self): # this checks if we properly check for search permissions - self.db.security.permissions = {} self.db.security.addRole(name='User') self.db.security.addRole(name='Project') self.db.security.addPermissionToRole('User', 'Web Access') diff --git a/test/test_xmlrpc.py b/test/test_xmlrpc.py index 48464f8f..ae8a1692 100644 --- a/test/test_xmlrpc.py +++ b/test/test_xmlrpc.py @@ -228,7 +228,6 @@ def testAuthAllowedCreate(self): def testAuthFilter(self): # this checks if we properly check for search permissions - self.db.security.permissions = {} # self.db.security.set_props_only_default(props_only=False) self.db.security.addRole(name='User') self.db.security.addRole(name='Project') From a492c975be6e2cfa176f8731110e2ca0f83d21f7 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 3 Nov 2025 12:42:48 -0500 Subject: [PATCH 053/290] refactor: mke Date class use __slots__ This should be the last slotting for a while. Instrumenting roundup-admin and the Client class (running under the waitress wsgi server) shows object count output similar to: [(('IssueClass', 'roundup.backends.back_anydbm'), 128), (('Session', 'roundup.cgi.client'), 128), (('Mailer', 'roundup.mailer'), 128), (('Password', 'roundup.password'), 220), (('LiberalCookie', 'roundup.cgi.client'), 241), (('Database', 'roundup.backends.back_anydbm'), 256), (('FileClass', 'roundup.backends.back_anydbm'), 256), (('Number', 'roundup.hyperdb'), 256), (('PythonExpr', 'roundup.cgi.PageTemplates.PythonExpr'), 274), (('Proptree', 'roundup.hyperdb'), 276), (('Role', 'roundup.security'), 384), (('PathExpr', 'roundup.cgi.PageTemplates.Expressions'), 630), (('Class', 'roundup.backends.back_anydbm'), 640), (('SubPathExpr', 'roundup.cgi.PageTemplates.Expressions'), 645), (('Date', 'roundup.hyperdb'), 678), (('Link', 'roundup.hyperdb'), 934), (('Multilink', 'roundup.hyperdb'), 1024), (('Permission', 'roundup.security'), 6784), (('TruthDict', 'roundup.support'), 6795), (('PrioList', 'roundup.support'), 8192), (('Date', 'roundup.date'), 8610)] where each row is a tuple of (class, module) and the count of the number of object of that class sorted by number of objects. I think the major classes that meet the criteria below are all __slotted__ at this point: Is a native roundup class and not a subclass (e.g. LiberalCookie is subclass of http.cookies.SimpleCookie) Does not touch the database (not hyperdb or backend) Is not part of templating Is not a top level class that may need runtime attributes/methods overwritten e.g Session/Mailer/Password Some of the excluded classes can be slotted, but they are not low hanging fruit and requires more class heirarchy changes or more extensive testing. --- roundup/date.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/roundup/date.py b/roundup/date.py index 6515ee32..4bcd90ab 100644 --- a/roundup/date.py +++ b/roundup/date.py @@ -324,6 +324,9 @@ class Date: >>> test_fin(u) ''' + __slots__ = ("year", "month", "day", "hour", "minute", "second", + "_", "ngettext", "translator") + def __init__(self, spec='.', offset=0, add_granularity=False, translator=i18n): """Construct a date given a specification and a time zone offset. From fe5f88f9821f6cc107964ff123a22d86a12a33ed Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Fri, 7 Nov 2025 22:14:01 -0500 Subject: [PATCH 054/290] doc: spelling correction. --- doc/reference.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/reference.txt b/doc/reference.txt index 5d26493f..eb0759b7 100644 --- a/doc/reference.txt +++ b/doc/reference.txt @@ -1307,7 +1307,7 @@ is added to the db object. As a result, the ``hasattr`` call shown above will return True and the Reauth exception is not raised. (Note that the value of the ``reauth_done`` attribute is True, so ``getattr(db, "reauth_done", False)`` will return True when reauth is -done and the defaul value of False if the attribute is missing. If the +done and the defile value of False if the attribute is missing. If the default is not set, `getattr` raises an ``AttributeError`` which might be useful for flow control.) From 65b50f32ebb3923793a322ab73ee014752ef8e71 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sat, 8 Nov 2025 00:40:39 -0500 Subject: [PATCH 055/290] doc: removed line referencing API change in 1.6 Removed sentence: This may be fixed in Roundup 1.6 by introducing ``init_web(client)`` callback or a more flexible extension point mechanism. as I think RegisterAction can be used for both cgi/html interface (action inherits from roundup.cgi.actions.Action) and rest/xmlrpc interface (action inherits from roundup.actions.Action). --- doc/reference.txt | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/doc/reference.txt b/doc/reference.txt index eb0759b7..8fe8acb0 100644 --- a/doc/reference.txt +++ b/doc/reference.txt @@ -1368,10 +1368,8 @@ from this directory, at which point it calls ``init(instance)`` from each file supplying itself as a first argument. Note that at this point web interface is not loaded, but -extensions still can register actions for it in the tracker -instance. This may be fixed in Roundup 1.6 by introducing -``init_web(client)`` callback or a more flexible extension point -mechanism. +extensions can register actions for it in the tracker +instance. * ``instance.registerUtil`` is used for adding `templating utilities`_ (see `adding a time log to your issues @@ -1384,8 +1382,9 @@ mechanism. * ``instance.registerAction`` is used to add more actions to the instance and to web interface. See `Defining new web actions`_ - for details. Generic action can be added by inheriting from - ``action.Action`` instead of ``cgi.action.Action``. + for details. Generic action (used by xmlrpc or rest interfaces) can + be added by inheriting from ``action.Action`` instead of + ``cgi.action.Action``. .. _interfaces.py: .. _modifying the core of Roundup: From f67cf64157930438094cc0e4f88f0df2f1cb72e8 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sat, 8 Nov 2025 14:03:42 -0500 Subject: [PATCH 056/290] doc: update styles to WCAG AAA; add accessibility statement; reword Update to AA WCAG 2.2 contrast: in note admonitions, link contrast was too low in python code examples, comments were too low contrast Added accessability statement. It is only in the tree and not linked to the website currently. Updated install directions for future 2.6 release. Updated paragraph to make it clearer. --- doc/_static/style.css | 10 +++ .../accessibility-statement_2025-10-08.html | 75 +++++++++++++++++++ website/www/_static/style.css | 10 +++ website/www/index.txt | 9 ++- 4 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 doc/html_extra/accessibility-statement_2025-10-08.html diff --git a/doc/_static/style.css b/doc/_static/style.css index ef5215ca..60f673a2 100644 --- a/doc/_static/style.css +++ b/doc/_static/style.css @@ -186,6 +186,8 @@ div[class^=highlight-], div[class^=highlight-] * { width: /* style */ :link { color: rgb(220,0,0); text-decoration: none;} +/* improve contrast to AA */ +.admonition.note :link { color: rgb(170,1,1); text-decoration: none;} :link:hover { text-decoration: underline solid clamp(1px, .3ex, 4px); text-underline-position: under; @@ -438,6 +440,14 @@ dd > ul:first-child { white-space: break-spaces; }*/ +/* improve color contrast to AA against yellowish highlight bg */ +div.highlight .c1 { + color: rgb(3,114,3); +} +div.highlight .na { + color: rgb(220,2,2); +} + /* Forcing wrap in a pre leads to some confusing line breaks. Use a horizontal scroll. Indicate the scroll by using rounded scroll shadows. diff --git a/doc/html_extra/accessibility-statement_2025-10-08.html b/doc/html_extra/accessibility-statement_2025-10-08.html new file mode 100644 index 00000000..f4e67e00 --- /dev/null +++ b/doc/html_extra/accessibility-statement_2025-10-08.html @@ -0,0 +1,75 @@ + +

Accessibility Statement for Roundup Issue Tracker Main Website

+

+ This is an accessibility statement from Roundup Issue Tracker. +

+

Conformance status

+

+ The Web Content Accessibility Guidelines (WCAG) defines requirements for designers and developers to improve accessibility for people with disabilities. It defines three levels of conformance: Level A, Level AA, and Level AAA. + Roundup Issue Tracker Main Website + is + partially conformant + with + WCAG 2.2 level AA. + + Partially conformant + means that + some parts of the content do not fully conform to the accessibility standard. + +

+

Feedback

+

+ We welcome your feedback on the accessibility of + Roundup Issue Tracker Main Website. + Please let us know if you encounter accessibility barriers on + Roundup Issue Tracker Main Website: +

+ +

Technical specifications

+

+ Accessibility of + Roundup Issue Tracker Main Website + relies on the following technologies to work with the particular combination of web browser and any assistive technologies or plugins installed on your computer: +

+
    +
  • HTML
    • +
  • WAI-ARIA
    • +
  • CSS
    • +
+

These technologies are relied upon for conformance with the accessibility standards used.

+

Limitations and alternatives

+

+ Despite our best efforts to ensure accessibility of + Roundup Issue Tracker Main Website , there may be some limitations. Below is a description of known limitations, and potential solutions. Please contact us if you observe an issue not listed below. +

+

+ Known limitations for + Roundup Issue Tracker Main Website: +

+
    +
  1. Search input does not have a visible label: The visible label is in the [Search] button next to the input but it not tied to the input directly. because The button acts as a visible label from proximity. The input has an aria-label describing the input.
    2. +
  2. Multi-language tab panel examples do not scroll: Multi-language example panels require two tabs to get to the element that will scroll. The Sphinx tab extension is used to show different language examples for the same item. It adds a focusable container around the example container. So the outer container must be tabbed/focused through to scroll the example when using the keyboard.
    3. +
+

Assessment approach

+

+ Roundup Issue Tracker + assessed the accessibility of + Roundup Issue Tracker Main Website + by the following approaches: +

+
    +
  • Self-evaluation
    • +
+
+

Date

+

+ This statement was created on + 8 November 2025 + using the W3C Accessibility Statement Generator Tool. +

diff --git a/website/www/_static/style.css b/website/www/_static/style.css index aa319c4b..bcf51011 100644 --- a/website/www/_static/style.css +++ b/website/www/_static/style.css @@ -186,6 +186,8 @@ div[class^=highlight-], div[class^=highlight-] * { width: /* style */ :link { color: rgb(220,0,0); text-decoration: none;} +/* improve contrast to AA */ +.admonition.note :link { color: rgb(170,1,1); text-decoration: none;} :link:hover { text-decoration: underline solid clamp(1px, .3ex, 4px); text-underline-position: under; @@ -438,6 +440,14 @@ dd > ul:first-child { white-space: break-spaces; }*/ +/* improve color contrast to AA against yellowish highlight bg */ +div.highlight .c1 { + color: rgb(3,114,3); +} +div.highlight .na { + color: rgb(220,2,2); +} + /* Forcing wrap in a pre leads to some confusing line breaks. Use a horizontal scroll. Indicate the scroll by using rounded scroll shadows. diff --git a/website/www/index.txt b/website/www/index.txt index 6173f131..9906bcb8 100644 --- a/website/www/index.txt +++ b/website/www/index.txt @@ -197,17 +197,18 @@ install Roundup. After you've unpacked the source, just run "``python demo.py``" and load up the URL it prints out! Follow the source gratification mode with these steps (change the -``-2.4.0`` version identifier to match your downloaded file). +``-2.6.0`` version identifier to match the version of Roundup you want +to use). 1. ``python3 -m pip download roundup`` -2. ``tar -xzvf roundup-2.4.0.tar.gz`` +2. ``tar -xzvf roundup-2.6.0.tar.gz`` * if you don't have a tar command (e.g windows), use:: - python -c "import tarfile, sys; tarfile.open(sys.argv[1]).extractall();" roundup-2.4.0.tar.gz + python -c "import tarfile, sys; tarfile.open(sys.argv[1]).extractall();" roundup-2.6.0.tar.gz -3. ``cd roundup-2.4.0`` +3. ``cd roundup-2.6.0`` 4. ``python3 demo.py`` (The source download can also be used to `create a custom Docker From 7ef5b13ddcffb07d916ed2cb3ad3a2989bd41829 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 12 Nov 2025 22:28:16 -0500 Subject: [PATCH 057/290] doc: add docs for producing jsonl formatted output --- doc/admin_guide.txt | 50 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index a6730bbf..69c77f47 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -350,10 +350,19 @@ The config below works with the `Waitress wsgi server roundup.wsgi channel. It also controls the `TransLogger middleware `_ configured to use roundup.wsgi.translogger, to produce httpd style combined -logs. The log file is specified relative to the current working +logs. + +The log file is specified relative to the current working directory not the tracker home. The tracker home is the -subdirectory demo under the current working directory. The -commented config is:: +subdirectory demo under the current working directory. + +The config also expects the ``python-json-logger`` package to be +installed so that it can produce a jsonl (json line) formatted +output log file. This format is useful for sending to log +management/observability platforms like elasticsearch, splunk, +logly, or honeycomb. + +The commented config is:: { "version": 1, // only supported version @@ -364,6 +373,28 @@ commented config is:: "standard": { "format": "%(asctime)s %(trace_id)s %(levelname)s %(name)s:%(module)s %(msg)s" }, + // Used to dump all log requests in jsonl format. + // Each json object is on one line. Can be pretty printed + // using: + // python -m json.tool --json-lines --sort-keys < roundup.json.log + // jq --slurp --sort-keys . < roundup.json.log + // requires that you pip install python-json-logger + // * does not report the fields in reserved_attrs + // * example to remap a field in the log to traceID in + // the output json. (note trace_id_eg is not defined by + // logging + // * also adds the env atribute to json with the value of demo + "json": { + "()": "pythonjsonlogger.json.JsonFormatter", + "reserved_attrs": ["ROUNDUP_CONTEXT_FILTER_CALLED", + "msg", "pct_char", "relativeCreated"], + "rename_fields": { + "trace_id_eg": "traceID" + }, + "static_fields": { + "env": "demo" + } + }, // used for waitress wsgi server to produce httpd style logs "http": { "format": "%(message)s %(trace_id)" @@ -385,6 +416,13 @@ commented config is:: "class": "logging.FileHandler", "filename": "demo/roundup.log" }, + // handler for json output log file + "roundup_json": { + "level": "DEBUG", // "DEBUG", + "formatter": "json", + "class": "logging.FileHandler", + "filename": "demo/roundup.json.log" + }, // print to stdout - fall through for other logging "default": { "level": "DEBUG", @@ -396,7 +434,8 @@ commented config is:: "loggers": { "": { "handlers": [ - "default" + "default", + "roundup_json" // add json formatted logging ], "level": "DEBUG", "propagate": false @@ -404,7 +443,8 @@ commented config is:: // used by roundup.* loggers "roundup": { "handlers": [ - "roundup" + "roundup", + "roundup_json" ], "level": "DEBUG", "propagate": false // note pytest testing with caplog requires From 5c79282f52b53a9f7cf6e8bf4d1f8413befd3156 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sat, 15 Nov 2025 16:59:24 -0500 Subject: [PATCH 058/290] doc: initial attempt to document setup of pgp support for email. Used an AI assistant to help write this. Basic gpg commands seem to work, but I have not tested this totally. Docs basically follow the setup used for pgp testing in the test suite. It looks like roundup accepts signed emails as well as encrypted and signed emails. But it does not generate signed emails. Also it looks like there is no PGP support for alternate email addresses. Only primary addresses can do PGP emails. --- doc/admin_guide.txt | 81 +++++++++++++++++++++++++++++++++++++++++++++ doc/upgrading.txt | 2 ++ 2 files changed, 83 insertions(+) diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index 69c77f47..4b1d0ed5 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -1870,6 +1870,87 @@ Because environment variables can be inadvertently exposed in logs or process listings, Roundup does not currently support loading secrets from environment variables. +.. _pgpconfig: + +Configuring PGP Email Support +============================= + +.. note:: + This section was written with the help of the Devin/DeepWiki AI. + +You have to install the gpg module using pip. See :ref:`directions for +installing gpg ` +in the upgrading document for more information. + +In your tracker's config.ini configure the following settings in the +``[pgp]`` section:: + + enable = yes + homedir = /path/to/pgp/configdir + roles = admin + +This will allow any user with the admin role to send signed pgp +email. If ``roles`` is not set, all users will need to use signed +emails. If it is not signed it will be rejected. Note that ``homedir`` +must be an absolute path. Unlike other path settings, a relative path +is not interpreted relative to the tracker home. See the documentation +in config.ini for more information and other settings (e.g. to send +encrypted emails from the tracker). + +When PGP is enabled and a message is signed with a valid signature, +the database transaction source (db.tx_Source) is set to +``email-sig-openpgp`` instead of ``email``. This allows you to +restrict certain operations (e.g. changing a private flag) to +authenticated/signed emails. + +Creating GPG Keys for the Tracker +--------------------------------- + +To generate a keypair use:: + + gpg --homedir /path/to/pgp/configdir --gen-key + +where the homedir directory matches the one you set in +config.ini. Note the gpg homedir must be created before you run the +command. You will be prompted for the full name of your tracker and +the email address for your tracker. You also need to do with as the +user who runs roundup (aka the roundup user) and the roundup email +gateway. Do not encrypt the key. + +Roundup has no mechanism for reading the private key if it is +encrypted. So make sure the permissions on the homedir only allow the +roundup user to read the files. + +You can export the public key for use by clients using:: + + gpg --homedir /path/to/pgp/configdir --export -a tracker@example.com > tracker-public.key + +with homedir and email matching the values used to generate the +key. This will allow users to import the public key and encrypt emails +to the tracker. + +The public gpg key for each user's email address must be imported. To +do this, obtain the user's public key for their primary email address +and import it using:: + + gpg --homedir /path/to/tracker/gpg --import user-public-key.asc + +While Roundup supports multiple addresses for each user, only the +primary address supports PGP signed or encrypted messages. + +.. comment: + Questions: + + Can roundup send signed emails? (looks like no, why??) + + Why are alternate addresses not supported for receiving PGP emails? + + Does Roundup ever send an email to an alternate email address? + + Should there be some way for a user to upload their own public key? + If so what ui (paste armored asci cert in textbox, upload ascii + file from user page and process)? + Tasks ===== diff --git a/doc/upgrading.txt b/doc/upgrading.txt index 9b3fcd7b..8f84c093 100644 --- a/doc/upgrading.txt +++ b/doc/upgrading.txt @@ -785,6 +785,8 @@ request (tu.client.request), the translator for the current language You can find an example in :ref:`dynamic_csp`. +.. _gpginstall: + Directions for installing gpg (optional) ---------------------------------------- From c85105cfbbe18697115e9328195fd68bbaa68011 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 24 Nov 2025 11:50:19 -0500 Subject: [PATCH 059/290] chore: github actions/checkout upgrade 5.0.0 to 6.0.0 --- .github/workflows/anchore.yml | 2 +- .github/workflows/build-xapian.yml | 2 +- .github/workflows/ci-test.yml | 2 +- .github/workflows/codeql-analysis.yml | 2 +- .github/workflows/ossf-scorecard.yml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/anchore.yml b/.github/workflows/anchore.yml index 45e4b05d..dafe7a96 100644 --- a/.github/workflows/anchore.yml +++ b/.github/workflows/anchore.yml @@ -37,7 +37,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout the code - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 - name: Build the Docker image run: docker pull python:3-alpine; docker build . --file scripts/Docker/Dockerfile --tag localbuild/testimage:latest - name: List the Docker image diff --git a/.github/workflows/build-xapian.yml b/.github/workflows/build-xapian.yml index 01384615..a4e40ae6 100644 --- a/.github/workflows/build-xapian.yml +++ b/.github/workflows/build-xapian.yml @@ -42,7 +42,7 @@ jobs: # if: {{ false }} # continue running if step fails # continue-on-error: true - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 # Setup version of Python to use - name: Set Up Python 3.13 diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index bb989035..a41e5085 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -116,7 +116,7 @@ jobs: # if: {{ false }} # continue running if step fails # continue-on-error: true - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 # Setup version of Python to use - name: Set Up Python ${{ matrix.python-version }} diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index d5a70850..da136135 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -49,7 +49,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL diff --git a/.github/workflows/ossf-scorecard.yml b/.github/workflows/ossf-scorecard.yml index a1fc81dc..7e280c87 100644 --- a/.github/workflows/ossf-scorecard.yml +++ b/.github/workflows/ossf-scorecard.yml @@ -35,7 +35,7 @@ jobs: steps: - name: "Checkout code" - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 with: persist-credentials: false From 51fa3d68c1be4b541669dc2e8041aa6d73ebf816 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 24 Nov 2025 11:51:40 -0500 Subject: [PATCH 060/290] chore: anchore anchore/scan-action upgrade 7.1.0 to 7.2.0 --- .github/workflows/anchore.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/anchore.yml b/.github/workflows/anchore.yml index dafe7a96..ebce562c 100644 --- a/.github/workflows/anchore.yml +++ b/.github/workflows/anchore.yml @@ -43,7 +43,7 @@ jobs: - name: List the Docker image run: docker image ls - name: Run the Anchore scan action itself with GitHub Advanced Security code scanning integration enabled - uses: anchore/scan-action@568b89d27fc18c60e56937bff480c91c772cd993 # 7.1.0 + uses: anchore/scan-action@3aaf50d765cfcceafa51d322ccb790e40f6cd8c5 # 7.2.0 id: scan with: image: "localbuild/testimage:latest" From 235eb28234fce093661939e58e369265441b3237 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 24 Nov 2025 11:52:32 -0500 Subject: [PATCH 061/290] chore: coveralls coverallsapp/github-action upgrade 2.3.6 to 2.3.7 --- .github/workflows/ci-test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index a41e5085..e29abb29 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -331,7 +331,7 @@ jobs: - name: Upload coverage to Coveralls # python 2.7 and 3.6 versions of coverage can't produce lcov files. if: matrix.python-version != '2.7' && matrix.python-version != '3.6' - uses: coverallsapp/github-action@648a8eb78e6d50909eff900e4ec85cab4524a45b # v2.3.6 + uses: coverallsapp/github-action@5cbfd81b66ca5d10c19b062c04de0199c215fb6e # v2.3.7 with: github-token: ${{ secrets.GITHUB_TOKEN }} path-to-lcov: coverage.lcov @@ -367,7 +367,7 @@ jobs: steps: - name: Coveralls Finished - uses: coverallsapp/github-action@648a8eb78e6d50909eff900e4ec85cab4524a45b # v2.3.6 + uses: coverallsapp/github-action@5cbfd81b66ca5d10c19b062c04de0199c215fb6e # v2.3.7 with: github-token: ${{ secrets.github_token }} parallel-finished: true From 3bf7faaf214d6f9aee17ad610e4c9dde73363b65 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Fri, 28 Nov 2025 11:51:03 -0500 Subject: [PATCH 062/290] chore: anchore anchore/scan-action upgrade 7.1.0 to 7.2.0 --- .github/workflows/anchore.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/anchore.yml b/.github/workflows/anchore.yml index ebce562c..8401e691 100644 --- a/.github/workflows/anchore.yml +++ b/.github/workflows/anchore.yml @@ -43,7 +43,7 @@ jobs: - name: List the Docker image run: docker image ls - name: Run the Anchore scan action itself with GitHub Advanced Security code scanning integration enabled - uses: anchore/scan-action@3aaf50d765cfcceafa51d322ccb790e40f6cd8c5 # 7.2.0 + uses: anchore/scan-action@40a61b52209e9d50e87917c5b901783d546b12d0 # 7.2.1 id: scan with: image: "localbuild/testimage:latest" From 0b9722347806baceb44aeeeec087f70d3e0c29cc Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Tue, 2 Dec 2025 12:30:20 -0500 Subject: [PATCH 063/290] chore: bump actions/setup-python from 6.0.0 to 6.1.0 --- .github/workflows/build-xapian.yml | 2 +- .github/workflows/ci-test.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-xapian.yml b/.github/workflows/build-xapian.yml index a4e40ae6..36d837b7 100644 --- a/.github/workflows/build-xapian.yml +++ b/.github/workflows/build-xapian.yml @@ -46,7 +46,7 @@ jobs: # Setup version of Python to use - name: Set Up Python 3.13 - uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6.0.0 + uses: actions/setup-python@83679a892e2d95755f2dac6acb0bfd1e9ac5d548 # v6.1.0 with: python-version: 3.13 allow-prereleases: true diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index e29abb29..5e465aa2 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -120,7 +120,7 @@ jobs: # Setup version of Python to use - name: Set Up Python ${{ matrix.python-version }} - uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6.0.0 + uses: actions/setup-python@83679a892e2d95755f2dac6acb0bfd1e9ac5d548 # v6.1.0 with: python-version: ${{ matrix.python-version }} allow-prereleases: true From 3356e989ccf2cc71b1824b945da929e74ed67bc3 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 7 Dec 2025 16:28:08 -0500 Subject: [PATCH 064/290] chore: update base image to python 3.14.1-alpine3.23 release. New release is 3.14.1-alpine3.23. --- scripts/Docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/Docker/Dockerfile b/scripts/Docker/Dockerfile index 0567793e..1d7b1b57 100644 --- a/scripts/Docker/Dockerfile +++ b/scripts/Docker/Dockerfile @@ -26,7 +26,7 @@ ARG source=local # Note this is the index digest for the image, not the manifest digest. # The index digest is shared across archetectures (amd64, arm64 etc.) # while the manifest digest is unique per platform/arch. -ARG SHA256=8373231e1e906ddfb457748bfc032c4c06ada8c759b7b62d9c73ec2a3c56e710 +ARG SHA256=b80c82b1a282283bd3e3cd3c6a4c895d56d1385879c8c82fa673e9eb4d6d4aa5 # Set to any non-empty value to enable shell debugging for troubleshooting ARG VERBOSE= From 1b6bb479e7027489abd031447dc561eb6e51137b Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 7 Dec 2025 17:30:41 -0500 Subject: [PATCH 065/290] docs: key from keyserver, check key before import to production --- doc/admin_guide.txt | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index 4b1d0ed5..d4742f46 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -1935,9 +1935,28 @@ and import it using:: gpg --homedir /path/to/tracker/gpg --import user-public-key.asc +You may also be able to get it from a public keyserver using:: + + gpg --recv-keys KEYID + +where the ``KEYID`` is supplied by the roundup user. + While Roundup supports multiple addresses for each user, only the primary address supports PGP signed or encrypted messages. +You should verify that the public key is sane and has few signatures +attached. You can import a key into a throw away keystore:: + + mkdir throwaway + gpg --homedir throwaway -- import user-public-key.asc + gpg --homedir throwaway --list-sigs + +and verify that the number of sig lines is small (under 10 or so). If +it takes a long time to import you can kill the import without +affecting your production keystore. Large numbers of sig lines can +take a long time to import/access when compressed. See: +https://nvd.nist.gov/vuln/detail/CVE-2022-3219. + .. comment: Questions: From df79d29a571099c699f98e4131526e393b95fe52 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 7 Dec 2025 17:33:25 -0500 Subject: [PATCH 066/290] docs: update changelog with pgp doc addition. --- CHANGES.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index fe133a2e..c277eb5e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -57,6 +57,8 @@ Features: logs for a specific transaction. Logging also supports a trace_reason log token with the url for a web request. The logging format can be changed in config.ini. (John Rouillard) +- issue2551152 - added basic PGP setup/use info to admin_guide. (John + Rouillard) 2025-07-13 2.5.0 From 49137882b8a49cda6d8239e6012fdd41ac590b76 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 8 Dec 2025 00:23:14 -0500 Subject: [PATCH 067/290] feat: add nanoid pkg trace_id gen and decorator for setting processName nanoid is a shorter unique id generator and faster than uuid. I truncate nanoid id's to 12 chars to make it more readable. Also added decorator to allow setting the default processName definition in the logging module. admin.py and wsgi_handler now set processName. configuration.py knows how to overide the processName if set to the default MainProcess. Updated install docs to add nanoid as optional, how to switch to different trace_id output. pydoc generated docs include logcontext module and are referenced from admin.py. --- CHANGES.txt | 7 ++- doc/admin_guide.txt | 8 ++- doc/installation.txt | 7 ++- doc/pydoc.txt | 18 +++++- roundup/admin.py | 3 +- roundup/cgi/wsgi_handler.py | 3 +- roundup/configuration.py | 6 ++ roundup/logcontext.py | 120 +++++++++++++++++++++++++++++------- 8 files changed, 140 insertions(+), 32 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index fe133a2e..bb675bfb 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -54,9 +54,10 @@ Features: logging format. (John Rouillard) - the default logging format template includes an identifier unique for a request. This identifier (trace_id) can be use to identify - logs for a specific transaction. Logging also supports a - trace_reason log token with the url for a web request. The logging - format can be changed in config.ini. (John Rouillard) + logs for a specific transaction. Will use nanoid if installed, uses + uuid.uuid4 otherwise. Logging also supports a trace_reason log token + with the url for a web request. The logging format can be changed in + config.ini. (John Rouillard) 2025-07-13 2.5.0 diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index 4b1d0ed5..261eb4f7 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -239,8 +239,8 @@ More on trace_id ~~~~~~~~~~~~~~~~ The trace_id provides a unique token (a UUID4 encoded to make it -shorter) for each transaction in the database. It is unique to -each thread or transaction. A transaction: +shorter or a nanoid) for each transaction in the database. It is +unique to each thread or transaction. A transaction: for the web interface is each web, rest or xmlrpc request @@ -282,7 +282,11 @@ point for the processing operation:: for thing in things: process_one(thing) +You can change the format of the trace_id if required using the +tracker's interfaces.py file. See the :ref:`module docs for the +logcontext module ` for details. + Advanced Logging Setup ---------------------- diff --git a/doc/installation.txt b/doc/installation.txt index 24a97367..055ba66c 100644 --- a/doc/installation.txt +++ b/doc/installation.txt @@ -253,7 +253,11 @@ gpg version 2.0 of gpg from test.pypi.org. See the `gpg install directions in the upgrading document`_. - +nanoid + If nanoid_ is installed, it is used to generate short unique + ids to link all logging to a single request. If not installed, + uuid4's from the standard library are used. + jinja2 To use the jinja2 template (may still be experimental, check out its TEMPLATE-INFO.txt file) you need @@ -2432,6 +2436,7 @@ the test. .. _mod_python: https://github.com/grisha/mod_python .. _mod_wsgi: https://pypi.org/project/mod-wsgi/ .. _MySQLdb: https://pypi.org/project/mysqlclient/ +.. _nanoid: https://pypi.org/project/nanoid/ .. _Olson tz database: https://www.iana.org/time-zones .. _polib: https://polib.readthedocs.io .. _Psycopg2: https://www.psycopg.org/ diff --git a/doc/pydoc.txt b/doc/pydoc.txt index ef7bf849..36df7250 100644 --- a/doc/pydoc.txt +++ b/doc/pydoc.txt @@ -1,10 +1,14 @@ -================ -Pydocs from code -================ +================================ +Embedded documentation from code +================================ .. contents:: :local: +The following are embedded documentation selected from the Roundup +code base. You can see the same information using the ``help`` +function after importing the modules. + Client class ============ @@ -40,3 +44,11 @@ Templating Utils class .. autoclass:: roundup.cgi.templating::TemplatingUtils :members: + +Logcontext Module +================= +.. _logcontext_pydoc: + +.. automodule:: roundup.logcontext + :members: + :exclude-members: SimpleSentinel diff --git a/roundup/admin.py b/roundup/admin.py index 14250694..446b13f2 100644 --- a/roundup/admin.py +++ b/roundup/admin.py @@ -49,7 +49,7 @@ ) from roundup.exceptions import UsageError from roundup.i18n import _, get_translation -from roundup.logcontext import gen_trace_id, store_trace_reason +from roundup.logcontext import gen_trace_id, set_processName, store_trace_reason try: from UserDict import UserDict @@ -2355,6 +2355,7 @@ def usageError_feedback(self, message, function): print(function.__doc__) return 1 + @set_processName("roundup-admin") @gen_trace_id() @store_trace_reason('admin') def run_command(self, args): diff --git a/roundup/cgi/wsgi_handler.py b/roundup/cgi/wsgi_handler.py index bfea660f..46458891 100644 --- a/roundup/cgi/wsgi_handler.py +++ b/roundup/cgi/wsgi_handler.py @@ -13,7 +13,7 @@ from roundup.anypy.strings import s2b from roundup.cgi import TranslationService from roundup.cgi.client import BinaryFieldStorage -from roundup.logcontext import gen_trace_id, store_trace_reason +from roundup.logcontext import gen_trace_id, set_processName, store_trace_reason BaseHTTPRequestHandler = http_.server.BaseHTTPRequestHandler DEFAULT_ERROR_MESSAGE = http_.server.DEFAULT_ERROR_MESSAGE @@ -107,6 +107,7 @@ def use_cached_tracker(self): "cache_tracker" not in self.feature_flags or self.feature_flags["cache_tracker"] is not False) + @set_processName("wsgi_handler") @gen_trace_id() @store_trace_reason("wsgi") def __call__(self, environ, start_response): diff --git a/roundup/configuration.py b/roundup/configuration.py index 6984aafc..da5511ec 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -2440,6 +2440,12 @@ def context_filter(self, record): for name, value in get_context_info(): if not hasattr(record, name): setattr(record, name, value) + continue + if (name == "processName" and + isinstance(value, str) and + getattr(record, name) == "MainProcess"): + setattr(record, name, value) + record.pct_char = "%" record.ROUNDUP_CONTEXT_FILTER_CALLED = True diff --git a/roundup/logcontext.py b/roundup/logcontext.py index 0371e28d..51095a4f 100644 --- a/roundup/logcontext.py +++ b/roundup/logcontext.py @@ -1,9 +1,70 @@ +"""Generate and store thread local logging context including unique +trace id for request, request source etc. to be logged. + +Trace id generator can use nanoid or uuid.uuid4 stdlib function. +Nanoid is preferred if nanoid is installed using pip as nanoid is +faster and generates a shorter id. If nanoid is installed in the +tracker's lib subdirectory, it must be enabled using the tracker's +interfaces.py by adding:: + + # if nanoid is installed in tracker's lib directory or + # if you want to change the length of the nanoid from 12 + # to 14 chars use: + from functools import partial + from nanoid import generate + import roundup.logcontext + # change 14 to 12 to get the default nanoid size. + roundup.logcontext.idgen=partial(generate, size=14) + + # to force use of shortened uuid when nanoid is + # loaded by default + import roundup.logcontext + roundup.logcontext.idgen=roundup.logcontext.short_uuid + +""" import contextvars import functools import logging import os import uuid + +def short_uuid(): + """Encode a UUID integer in a shorter form for display. + + A uuid is long. Make a shorter version that takes less room + in a log line and is easier to store. + """ + alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890" + result = "" + alphabet_len = len(alphabet) + uuid_int = uuid.uuid4().int + while uuid_int: + uuid_int, t = divmod(uuid_int, alphabet_len) + result += alphabet[t] + return result or "0" + + +try: + from nanoid import generate + # With size=12 and the normal alphabet, it take ~4 months + # with 1000 nanoid's/sec to generate a collision with 1% + # probability. That's 100 users sec continously. These id's + # are used to link logging messages/traces that are all done + # in a few seconds. Collisions ae unlikely to happen in the + # same time period leading to confusion. + # + # nanoid is faster than shortened uuids. + # 1,000,000 generate(size=12) timeit.timeit at 25.4 seconds + # 1,000,000 generate(size=21) timeit.timeit at 33.7 seconds + + #: Variable used for setting the id generator. + idgen = functools.partial(generate, size=12) +except ImportError: + # 1,000,000 of short_uuid() timeit.timeit at 54.1 seconds + idgen = short_uuid #: :meta hide-value: + + logger = logging.getLogger("roundup.logcontext") @@ -46,6 +107,11 @@ def __repr__(self): # set up sentinel values that will print a suitable error value # and the context vars they are associated with. +_SENTINEL_PROCESSNAME = SimpleSentinel("processName", None) +ctx_vars['processName'] = contextvars.ContextVar("processName", + default=_SENTINEL_PROCESSNAME) + + _SENTINEL_ID = SimpleSentinel("trace_id", "not set") ctx_vars['trace_id'] = contextvars.ContextVar("trace_id", default=_SENTINEL_ID) @@ -55,23 +121,8 @@ def __repr__(self): default=_SENTINEL_REASON) -def shorten_int_uuid(uuid): - """Encode a UUID integer in a shorter form for display. - - A uuid is long. Make a shorter version that takes less room - in a log line. - """ - - alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890" - result = "" - while uuid: - uuid, t = divmod(uuid, len(alphabet)) - result += alphabet[t] - return result or "0" - - def gen_trace_id(): - """Decorator to generate a trace id (encoded uuid4) and add to contextvar + """Decorator to generate a trace id (nanoid or encoded uuid4) as contextvar The logging routine uses this to label every log line. All logs with the same trace_id should be generated from a @@ -84,15 +135,22 @@ def gen_trace_id(): used by a different invocation method. It will not set a trace_id if one is already assigned. - A uuid4() is used as the uuid, but to shorten the log line, - the uuid4 integer is encoded into a 62 character ascii - alphabet (A-Za-z0-9). + If a uuid4() is used as the id, the uuid4 integer is encoded + into a 62 character alphabet (A-Za-z0-9) to shorten + the log line. This decorator may produce duplicate (colliding) trace_id's when used with multiple processes on some platforms where uuid.uuid4().is_safe is unknown. Probability of a collision is unknown. + If nanoid is used to generate the id, it is 12 chars long and + uses a 64 char ascii alphabet, the 62 above with '_' and '-'. + The shorter nanoid has < 1% chance of collision in ~4 months + when generating 1000 id's per second. + + See the help text for the module to change how the id is + generated. """ def decorator(func): @functools.wraps(func) @@ -100,7 +158,7 @@ def wrapper(*args, **kwargs): prev = None trace_id = ctx_vars['trace_id'] if trace_id.get() is _SENTINEL_ID: - prev = trace_id.set(shorten_int_uuid(uuid.uuid4().int)) + prev = trace_id.set(idgen()) try: r = func(*args, **kwargs) finally: @@ -111,6 +169,26 @@ def wrapper(*args, **kwargs): return decorator +def set_processName(name): + """Decorator to set the processName used in the LogRecord + """ + def decorator(func): + @functools.wraps(func) + def wrapper(*args, **kwargs): + prev = None + processName = ctx_vars['processName'] + if processName.get() is _SENTINEL_PROCESSNAME: + prev = processName.set(name) + try: + r = func(*args, **kwargs) + finally: + if prev: + processName.reset(prev) + return r + return wrapper + return decorator + + def store_trace_reason(location=None): """Decorator finds and stores a reason trace was started in contextvar. @@ -195,7 +273,7 @@ def get_context_info(): #Is returning a dict for this info more pythonic? def get_context_dict(): - """Return dict of context var tuples ["var_name": "var_value", ...}""" + """Return dict of context var tuples {"var_name": "var_value", ...}""" return {name: ctx.get() for name, ctx in ctx_vars.items()} # Dummy no=op implementation of this module: From 0c60ec8bab86c94151db50422d6fe9d30d63cdb4 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 8 Dec 2025 23:07:57 -0500 Subject: [PATCH 068/290] chore: update actions/checkout from 6.0.0 to 6.1.1 pull74 --- .github/workflows/anchore.yml | 2 +- .github/workflows/build-xapian.yml | 2 +- .github/workflows/ci-test.yml | 2 +- .github/workflows/codeql-analysis.yml | 2 +- .github/workflows/ossf-scorecard.yml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/anchore.yml b/.github/workflows/anchore.yml index 8401e691..aa510fd5 100644 --- a/.github/workflows/anchore.yml +++ b/.github/workflows/anchore.yml @@ -37,7 +37,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout the code - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Build the Docker image run: docker pull python:3-alpine; docker build . --file scripts/Docker/Dockerfile --tag localbuild/testimage:latest - name: List the Docker image diff --git a/.github/workflows/build-xapian.yml b/.github/workflows/build-xapian.yml index 36d837b7..e71f6d08 100644 --- a/.github/workflows/build-xapian.yml +++ b/.github/workflows/build-xapian.yml @@ -42,7 +42,7 @@ jobs: # if: {{ false }} # continue running if step fails # continue-on-error: true - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 # Setup version of Python to use - name: Set Up Python 3.13 diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index 5e465aa2..d04d7708 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -116,7 +116,7 @@ jobs: # if: {{ false }} # continue running if step fails # continue-on-error: true - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 # Setup version of Python to use - name: Set Up Python ${{ matrix.python-version }} diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index da136135..c1702df5 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -49,7 +49,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL diff --git a/.github/workflows/ossf-scorecard.yml b/.github/workflows/ossf-scorecard.yml index 7e280c87..a1f84d88 100644 --- a/.github/workflows/ossf-scorecard.yml +++ b/.github/workflows/ossf-scorecard.yml @@ -35,7 +35,7 @@ jobs: steps: - name: "Checkout code" - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: persist-credentials: false From b96c66147e4fb3f29341cca5b7a625731fb31e81 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sat, 13 Dec 2025 23:02:53 -0500 Subject: [PATCH 069/290] issue2551413 - Broken MultiLink columns in CSV export cmeerw found a crash and bug. While investigating CSV export using names with a multilink field that doesn't have a label field (name or realname for user class). cmeerw's patch displays a multilink like messages as "['12', '23']" as when exporting csv with id. I changes code so this displays as "12;23" to match the list format output of the other fields in export csv (with names). --- CHANGES.txt | 4 ++++ roundup/cgi/actions.py | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index ac0d40ad..e09117b3 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -37,6 +37,10 @@ Fixed: script. Found/patch by Norbert Schlemmer. (John Rouillard) - change some internal classes to use __slots__ for hopefully a small performance improvement. (John Rouillard) +- issue2551413 - Broken MultiLink columns in CSV export. CSV export of + a multilink link "messages" that does not have a 'name' property + causes a crash. (found/fix by cmeerw; commit and better handling of + non-labeled multilink by John Rouillard) Features: diff --git a/roundup/cgi/actions.py b/roundup/cgi/actions.py index 7d12af4a..f6d73a85 100644 --- a/roundup/cgi/actions.py +++ b/roundup/cgi/actions.py @@ -1688,7 +1688,6 @@ def fct(arg): if isinstance(props[col], hyperdb.Multilink): cname = props[col].classname cclass = self.db.getclass(cname) - represent[col] = repr_list(cclass, 'name') if not self.hasPermission(self.permissionType, classname=cname): represent[col] = repr_no_right(cclass, 'name') else: @@ -1696,6 +1695,10 @@ def fct(arg): represent[col] = repr_list(cclass, 'name') elif cname == 'user': represent[col] = repr_list(cclass, 'realname') + else: + # handle cases like messages which have no + # useful label field issue2551413 + represent[col] = repr_list(cclass, 'id') if isinstance(props[col], hyperdb.Link): cname = props[col].classname cclass = self.db.getclass(cname) From 6a05a1d13d5c776514c7f9b204d2a9dd42476bfc Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 14 Dec 2025 22:40:46 -0500 Subject: [PATCH 070/290] feat: support justhtml parsing library to convert email to plain text justhtml is an pure python, fast, HTML5 compliant parser. It is now an option for converting html only emails to plain text. Its output format differs slightly from dehtml or beautifulsoup. Mostly by removing extra blank lines. dehtml.py: Using the stream parser of justhtml. Unable to get the full document parser to successfully strip script and style blocks. If I can fix this and use the standard parser, I can in theory generate markdown from the DOM tree generated by justhtml. Updated test case to include inline elements that should not cause a line break when they are encountered. Running dehtml as: `python roundup/dehtml.py foo.html` will load foo.html and parse it using all available parsers. configuration.py: justhtml is available as an option. docs: updated CHANGES.txt, doc/tracker_config.txt added beautifulsoup and justhtml to the optional software section of doc/installtion.txt. test_mailgw.py, .github/workflows/ci-test Updated tests and install justhtml as part of CI. --- .github/workflows/ci-test.yml | 2 +- CHANGES.txt | 4 + doc/installation.txt | 10 +++ doc/tracker_config.txt | 8 +- roundup/configuration.py | 20 ++--- roundup/dehtml.py | 152 ++++++++++++++++++++++++++++++++-- test/test_mailgw.py | 16 ++++ 7 files changed, 191 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index d04d7708..344fc727 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -240,7 +240,7 @@ jobs: # pygments for markdown2 to highlight code blocks pip install markdown2 pygments # docutils for ReStructuredText - pip install beautifulsoup4 brotli docutils jinja2 \ + pip install beautifulsoup4 justhtml brotli docutils jinja2 \ mistune==0.8.4 pyjwt pytz whoosh # gpg on PyPi is currently broken with newer OS platform # ubuntu 24.04 diff --git a/CHANGES.txt b/CHANGES.txt index e09117b3..a75bc93e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -64,6 +64,10 @@ Features: config.ini. (John Rouillard) - issue2551152 - added basic PGP setup/use info to admin_guide. (John Rouillard) +- add support for the 'justhtml' html 5 parser library. It is written + in pure Python. Used to convert html emails into plain text. Faster + then beautifulsoup4 and it passes the html 5 standard browser test + suite. Beautifulsoup is still supported. (John Rouillard) 2025-07-13 2.5.0 diff --git a/doc/installation.txt b/doc/installation.txt index 2f022747..78bf4c88 100644 --- a/doc/installation.txt +++ b/doc/installation.txt @@ -311,6 +311,14 @@ polib roundup-gettext, you must install polib_. See the `developer's guide`_ for details on translating your tracker. +beautifulsoup, justhtml + When HTML only email is received, Roundup can convert it into + plain text using the native dehtml parser. To convert HTML + email into plain text, beautifulsoup4_ or justhtml_ can also be + used. You can choose the converter in the tracker's + config. Note that justhtml is pure Python, fast and conforms to + HTML 5 standards. + pywin32 - Windows Service You can run Roundup as a Windows service if pywin32_ is installed. Otherwise it must be started manually. @@ -2423,6 +2431,7 @@ the test. .. _`adding MySQL users`: https://dev.mysql.com/doc/refman/8.0/en/creating-accounts.html .. _apache: https://httpd.apache.org/ +.. _beautifulsoup4: https://pypi.org/project/beautifulsoup4/ .. _brotli: https://pypi.org/project/Brotli/ .. _`developer's guide`: developers.html .. _defusedxml: https://pypi.org/project/defusedxml/ @@ -2430,6 +2439,7 @@ the test. .. _flup: https://pypi.org/project/flup/ .. _gpg: https://www.gnupg.org/software/gpgme/index.html .. _jinja2: https://palletsprojects.com/projects/jinja/ +.. _justhtml: https://pypi.org/project/justhtml/ .. _markdown: https://python-markdown.github.io/ .. _markdown2: https://github.com/trentm/python-markdown2 .. _mistune: https://pypi.org/project/mistune/ diff --git a/doc/tracker_config.txt b/doc/tracker_config.txt index 1d132660..a76220b0 100644 --- a/doc/tracker_config.txt +++ b/doc/tracker_config.txt @@ -1112,12 +1112,12 @@ # If an email has only text/html parts, use this module # to convert the html to text. Choose from beautifulsoup 4, - # dehtml - (internal code), or none to disable conversion. - # If 'none' is selected, email without a text/plain part - # will be returned to the user with a message. If + # justhtml, dehtml - (internal code), or none to disable + # conversion. If 'none' is selected, email without a text/plain + # part will be returned to the user with a message. If # beautifulsoup is selected but not installed dehtml will # be used instead. - # Allowed values: beautifulsoup, dehtml, none + # Allowed values: beautifulsoup, justhtml, dehtml, none # Default: none convert_htmltotext = none diff --git a/roundup/configuration.py b/roundup/configuration.py index d07b8bcd..1c470b4d 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -384,17 +384,17 @@ class HtmlToTextOption(Option): """What module should be used to convert emails with only text/html parts into text for display in roundup. Choose from beautifulsoup - 4, dehtml - the internal code or none to disable html to text - conversion. If beautifulsoup chosen but not available, dehtml will - be used. + 4, justhtml, dehtml - the internal code or none to disable html to + text conversion. If beautifulsoup or justhtml is chosen but not + available, dehtml will be used. """ - class_description = "Allowed values: beautifulsoup, dehtml, none" + class_description = "Allowed values: beautifulsoup, justhtml, dehtml, none" def str2value(self, value): _val = value.lower() - if _val in ("beautifulsoup", "dehtml", "none"): + if _val in ("beautifulsoup", "justhtml", "dehtml", "none"): return _val else: raise OptionValueError(self, value, self.class_description) @@ -1811,11 +1811,11 @@ def str2value(self, value): (HtmlToTextOption, "convert_htmltotext", "none", "If an email has only text/html parts, use this module\n" "to convert the html to text. Choose from beautifulsoup 4,\n" - "dehtml - (internal code), or none to disable conversion.\n" - "If 'none' is selected, email without a text/plain part\n" - "will be returned to the user with a message. If\n" - "beautifulsoup is selected but not installed dehtml will\n" - "be used instead."), + "justhtml, dehtml - (internal code), or none to disable\n" + "conversion. If 'none' is selected, email without a text/plain\n" + "part will be returned to the user with a message. If\n" + "beautifulsoup or justhtml is selected but not installed\n" + "dehtml will be used instead."), (BooleanOption, "keep_real_from", "no", "When handling emails ignore the Resent-From:-header\n" "and use the original senders From:-header instead.\n" diff --git a/roundup/dehtml.py b/roundup/dehtml.py index cd913a0d..ec73fb40 100644 --- a/roundup/dehtml.py +++ b/roundup/dehtml.py @@ -5,6 +5,10 @@ from roundup.anypy.strings import u2s, uchr +# ruff PLC0415 ignore imports not at top of file +# ruff RET505 ignore else after return +# ruff: noqa: PLC0415 RET505 + _pyver = sys.version_info[0] @@ -28,6 +32,108 @@ def html2text(html): return u2s(soup.get_text("\n", strip=True)) + self.html2text = html2text + elif converter == "justhtml": + from justhtml import stream + + def html2text(html): + # The below does not work. + # Using stream parser since I couldn't seem to strip + # 'script' and 'style' blocks. But stream doesn't + # have error reporting or stripping of text nodes + # and dropping empty nodes. Also I would like to try + # its GFM markdown output too even though it keeps + # tables as html and doesn't completely covert as + # this would work well for those supporting markdown. + # + # ctx used for for testing since I have a truncated + # test doc. It eliminates error from missing DOCTYPE + # and head. + # + #from justhtml import JustHTML + # from justhtml.context import FragmentContext + # + #ctx = FragmentContext('html') + #justhtml = JustHTML(html,collect_errors=True, + # fragment_context=ctx) + # I still have the text output inside style/script tags. + # with :not(style, script). I do get text contents + # with query("style, script"). + # + #return u2s("\n".join( + # [elem.to_text(separator="\n", strip=True) + # for elem in justhtml.query(":not(style, script)")]) + # ) + + # define inline elements so I can accumulate all unbroken + # text in a single line with embedded inline elements. + # 'br' is inline but should be treated it as a line break + # and element before/after should not be accumulated + # together. + inline_elements = ( + "a", + "address", + "b", + "cite", + "code", + "em", + "i", + "img", + "mark", + "q", + "s", + "small", + "span", + "strong", + "sub", + "sup", + "time") + + # each line is appended and joined at the end + text = [] + # the accumulator for all text in inline elements + text_accumulator = "" + # if set skip all lines till matching end tag found + # used to skip script/style blocks + skip_till_endtag = None + # used to force text_accumulator into text with added + # newline so we have a blank line between paragraphs. + _need_parabreak = False + + for event, data in stream(html): + if event == "end" and skip_till_endtag == data: + skip_till_endtag = None + continue + if skip_till_endtag: + continue + if (event == "start" and + data[0] in ('script', 'style')): + skip_till_endtag = data[0] + continue + if (event == "start" and + text_accumulator and + data[0] not in inline_elements): + # add accumulator to "text" + text.append(text_accumulator) + text_accumulator = "" + _need_parabreak = False + elif event == "text": + if not data.isspace(): + text_accumulator = text_accumulator + data + _need_parabreak = True + elif (_need_parabreak and + event == "start" and + data[0] == "p"): + text.append(text_accumulator + "\n") + text_accumulator = "" + _need_parabreak = False + + # save anything left in the accumulator at end of document + if text_accumulator: + # add newline to match dehtml and beautifulsoup + text.append(text_accumulator + "\n") + return u2s("\n".join(text)) + self.html2text = html2text else: raise ImportError @@ -96,6 +202,16 @@ def html2text(html): if __name__ == "__main__": + # ruff: noqa: B011 S101 + + try: + assert False + except AssertionError: + pass + else: + print("Error, assertions turned off. Test fails") + sys.exit(1) + html = """ {%- endfor %} + {%- if builder == 'web' %} @@ -130,7 +107,8 @@

{{ _('Quick search') }}

{%- endfor %} {%- else %} - {{ css() }} + + {%- endif %} @@ -171,7 +149,8 @@

{{ _('Quick search') }}

{%- block extrahead %} {% endblock %} -
Roundup
+
+
Roundup
{%- if pagename != "search" %} {%- endif %} -
+
+
{%- block content %}
@@ -199,15 +180,15 @@

{{ _('Quick search') }}

{% block body %} {% endblock %} {{ relbar('related-bottom') }}
- {%- endblock %} +{%- endblock %} {%- block footer %} -
+
{%- if hasdoc('copyright') %} {% trans path=pathto('copyright'), copyright=copyright|e %}© Copyright {{ copyright }}.{% endtrans %} {%- else %} {% trans copyright=copyright|e %}© Copyright {{ copyright }}.{% endtrans %} {%- endif %} -
+ {%- endblock %} diff --git a/doc/conf.py b/doc/conf.py index 7dc41f5d..fe5c5fab 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -21,7 +21,6 @@ import sys import os - # Read Roundup version by importing it from parent directory, # this ensures that 'unknown version' is inserted even if # `roundup` is importable from other location in sys.path @@ -51,11 +50,11 @@ # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom # ones. #extensions = ['toctree'] -extensions = ['sphinx_tabs.tabs', 'sphinx.ext.autodoc'] +extensions = ['sphinx.ext.autodoc', 'sphinx_tabs.tabs'] sphinx_tabs_valid_builders = ['linkcheck'] sphinx_tabs_disable_tab_closing = True - +autodoc_use_legacy_class_based = True # Add any paths that contain templates here, relative to this directory. templates_path = ['_templates'] @@ -119,7 +118,7 @@ #show_authors = False # The name of the Pygments (syntax highlighting) style to use. -pygments_style = 'sphinx' +pygments_style = 'borland' # disable permalinks from sphinx import version_info @@ -181,14 +180,14 @@ # directly to the root of the documentation. html_extra_path = ['html_extra'] -# If not '', a 'Last updated on:' timestamp is inserted at every page bottom, -# using the given strftime format. -html_last_updated_fmt = '%b %d, %Y' - # If true, SmartyPants will be used to convert quotes and dashes to # typographically correct entities. #html_use_smartypants = True +# If not '', a 'Last updated on:' timestamp is inserted at every page bottom, +# using the given strftime format. +html_last_updated_fmt = '%b %d, %Y' + # Custom sidebar templates, maps document names to template names. #html_sidebars = {} diff --git a/website/www/_templates/layout.html b/website/www/_templates/layout.html index ea1206b7..f2deee44 100644 --- a/website/www/_templates/layout.html +++ b/website/www/_templates/layout.html @@ -111,7 +111,7 @@ -
+
+ {%- block footer %}