diff --git a/Doc/library/logging.handlers.rst b/Doc/library/logging.handlers.rst index 059ab3d3a34448..1182f43eb96ff0 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 b0d5885989ea42..26e839965468d2 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 6d111908e7c395..dc578f31b3e865 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 0cb738b3a12ee4..b349430af8cd24 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