gh-93205: When rotating logs with no namer specified, match whole extension (GH-93224)

Author: ilCatania

Lib/logging/handlers.py

@@ -369,32 +369,37 @@ 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
                 if (not fileName.startswith(baseName) and fileName.endswith(e) and
                     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:

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)

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.