From 113687a8381d6dde179aeede607bcbca5c09d182 Mon Sep 17 00:00:00 2001 From: Gabriele Catania Date: Wed, 21 Feb 2024 21:09:06 +0000 Subject: [PATCH] gh-93205: When rotating logs with no namer specified, match whole extension (GH-93224) --- Lib/logging/handlers.py | 43 +++++++++++-------- Lib/test/test_logging.py | 37 ++++++++++++++++ ...2-05-25-17-49-04.gh-issue-93205.DjhFVR.rst | 1 + 3 files changed, 62 insertions(+), 19 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-05-25-17-49-04.gh-issue-93205.DjhFVR.rst diff --git a/Lib/logging/handlers.py b/Lib/logging/handlers.py index e7f1322e4ba3d9..895f11c3392253 100644 --- a/Lib/logging/handlers.py +++ b/Lib/logging/handlers.py @@ -369,16 +369,20 @@ def getFilesToDelete(self): dirName, baseName = os.path.split(self.baseFilename) fileNames = os.listdir(dirName) result = [] - # See bpo-44753: Don't use the extension when computing the prefix. - n, e = os.path.splitext(baseName) - prefix = n + '.' - plen = len(prefix) - for fileName in fileNames: - if self.namer is None: - # Our files will always start with baseName - if not fileName.startswith(baseName): - continue - else: + if self.namer is None: + prefix = baseName + '.' + plen = len(prefix) + for fileName in fileNames: + if fileName[:plen] == prefix: + suffix = fileName[plen:] + if self.extMatch.match(suffix): + result.append(os.path.join(dirName, fileName)) + else: + # See bpo-44753: Don't use the extension when computing the prefix. + n, e = os.path.splitext(baseName) + prefix = n + '.' + plen = len(prefix) + for fileName in fileNames: # Our files could be just about anything after custom naming, but # likely candidates are of the form # foo.log.DATETIME_SUFFIX or foo.DATETIME_SUFFIX.log @@ -386,15 +390,16 @@ def getFilesToDelete(self): len(fileName) > (plen + 1) and not fileName[plen+1].isdigit()): continue - if fileName[:plen] == prefix: - suffix = fileName[plen:] - # See bpo-45628: The date/time suffix could be anywhere in the - # filename - parts = suffix.split('.') - for part in parts: - if self.extMatch.match(part): - result.append(os.path.join(dirName, fileName)) - break + if fileName[:plen] == prefix: + suffix = fileName[plen:] + # See bpo-45628: The date/time suffix could be anywhere in the + # filename + + parts = suffix.split('.') + for part in parts: + if self.extMatch.match(part): + result.append(os.path.join(dirName, fileName)) + break if len(result) < self.backupCount: result = [] else: diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index cf09bad4c9187b..d87142698efa8b 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -6193,6 +6193,43 @@ def test_compute_files_to_delete(self): self.assertTrue(fn.startswith(prefix + '.') and fn[len(prefix) + 2].isdigit()) + def test_compute_files_to_delete_same_filename_different_extensions(self): + # See GH-93205 for background + wd = pathlib.Path(tempfile.mkdtemp(prefix='test_logging_')) + self.addCleanup(shutil.rmtree, wd) + times = [] + dt = datetime.datetime.now() + n_files = 10 + for _ in range(n_files): + times.append(dt.strftime('%Y-%m-%d_%H-%M-%S')) + dt += datetime.timedelta(seconds=5) + prefixes = ('a.log', 'a.log.b') + files = [] + rotators = [] + for i, prefix in enumerate(prefixes): + backupCount = i+1 + rotator = logging.handlers.TimedRotatingFileHandler(wd / prefix, when='s', + interval=5, + backupCount=backupCount, + delay=True) + rotators.append(rotator) + for t in times: + files.append('%s.%s' % (prefix, t)) + # Create empty files + for f in files: + (wd / f).touch() + # Now the checks that only the correct files are offered up for deletion + for i, prefix in enumerate(prefixes): + backupCount = i+1 + rotator = rotators[i] + candidates = rotator.getFilesToDelete() + self.assertEqual(len(candidates), n_files - backupCount) + matcher = re.compile(r"^\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}(\.\w+)?$") + for c in candidates: + d, fn = os.path.split(c) + self.assertTrue(fn.startswith(prefix)) + suffix = fn[(len(prefix)+1):] + self.assertRegex(suffix, matcher) def secs(**kw): return datetime.timedelta(**kw) // datetime.timedelta(seconds=1) diff --git a/Misc/NEWS.d/next/Library/2022-05-25-17-49-04.gh-issue-93205.DjhFVR.rst b/Misc/NEWS.d/next/Library/2022-05-25-17-49-04.gh-issue-93205.DjhFVR.rst new file mode 100644 index 00000000000000..4a280b93d93347 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-05-25-17-49-04.gh-issue-93205.DjhFVR.rst @@ -0,0 +1 @@ +Fixed a bug in :class:`logging.handlers.TimedRotatingFileHandler` where multiple rotating handler instances pointing to files with the same name but different extensions would conflict and not delete the correct files.