diff --git a/Doc/library/logging.handlers.rst b/Doc/library/logging.handlers.rst index 059ab3d3a34448d..1182f43eb96ff0d 100644 --- a/Doc/library/logging.handlers.rst +++ b/Doc/library/logging.handlers.rst @@ -382,7 +382,8 @@ timed intervals. The system will save old log files by appending extensions to the filename. The extensions are date-and-time based, using the strftime format ``%Y-%m-%d_%H-%M-%S`` or a leading portion thereof, depending on the - rollover interval. + rollover interval. In the case that an existing file has the same name, + a numbered suffix will be added to the new file to prevent overwriting logs. When computing the next rollover time for the first time (when the handler is created), the last modification time of an existing log file, or else diff --git a/Lib/logging/handlers.py b/Lib/logging/handlers.py index b0d5885989ea426..26e839965468d23 100644 --- a/Lib/logging/handlers.py +++ b/Lib/logging/handlers.py @@ -255,6 +255,7 @@ def __init__(self, filename, when='h', interval=1, backupCount=0, raise ValueError("Invalid rollover interval specified: %s" % self.when) self.extMatch = re.compile(self.extMatch, re.ASCII) + self.file_number = re.compile(r'\(([0-9]+)\)$') self.interval = self.interval * interval # multiply by units requested # The following line added because the filename passed in could be a # path object (see Issue #27493), but self.baseFilename will be a string @@ -401,8 +402,12 @@ def doRollover(self): timeTuple = time.localtime(t + addend) dfn = self.rotation_filename(self.baseFilename + "." + time.strftime(self.suffix, timeTuple)) - if os.path.exists(dfn): - os.remove(dfn) + while os.path.exists(dfn): + if match := re.search(self.file_number, dfn): + name = re.sub(self.file_number, f'({int(match.groups()[0]) + 1})', dfn) + dfn = self.rotation_filename(name) + else: + dfn = self.rotation_filename(self.baseFilename + '.' + time.strftime(self.suffix, timeTuple) + ' (2)') self.rotate(self.baseFilename, dfn) if self.backupCount > 0: for s in self.getFilesToDelete(): diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index 6d111908e7c3951..dc578f31b3e8657 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -5331,6 +5331,53 @@ def test_rollover(self): print(tf.read()) self.assertTrue(found, msg=msg) + def test_overwrite_rollover(self): + # bpo-44186 test that pre-existing files aren't overwritten by rollover + now = datetime.datetime.now() + handler = logging.handlers.TimedRotatingFileHandler( + self.fn, 'midnight', atTime=now, encoding='utf-8', backupCount=1) + + # compute the filename + t = handler.rolloverAt - handler.interval + timeTuple = time.gmtime(t) + fn = handler.rotation_filename( + handler.baseFilename + '.' + time.strftime(handler.suffix, timeTuple)) + + # touch a file with the intended filename + pathlib.Path(fn).touch() + self.rmfiles.append(fn) + + r = logging.makeLogRecord({'msg': 'test msg'}) + + handler.emit(r) + handler.close() + + self.assertLogFile(fn + ' (2)') + + def test_lowest_filename_generated(self): + # test that when the filename is generated, the lowest possible suffix is used + # e.g. if the files "log.2021-06-19", "log.2021-06-19 (2)", and "log.2021-06-19 (4)" exist + # the filename generated should be "log.2021-06-19 (3)" + now = datetime.datetime.now() + fh = logging.handlers.TimedRotatingFileHandler( + self.fn, 'midnight', atTime=now, encoding='utf-8', backupCount=1) + + # compute the filename + t = fh.rolloverAt - fh.interval + timeTuple = time.gmtime(t) + fn = fh.rotation_filename( + fh.baseFilename + '.' + time.strftime(fh.suffix, timeTuple)) + + for filename in [fn, fn + ' (2)', fn + ' (4)']: + pathlib.Path(filename).touch() + self.rmfiles.append(filename) + + r = logging.makeLogRecord({'msg': 'test msg'}) + fh.emit(r) + fh.close() + + self.assertLogFile(fn + ' (3)') + def test_invalid(self): assertRaises = self.assertRaises assertRaises(ValueError, logging.handlers.TimedRotatingFileHandler, diff --git a/Misc/ACKS b/Misc/ACKS index 0cb738b3a12ee4c..b349430af8cd246 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1017,6 +1017,7 @@ John J. Lee Thomas Lee Robert Leenders Cooper Ry Lees +Harry Lees Yaron de Leeuw Tennessee Leeuwenburg Luc Lefebvre