From a59f5f320856926d5e61686d48bda277c6fe7919 Mon Sep 17 00:00:00 2001 From: Vinay Sajip Date: Fri, 8 Oct 2021 17:07:13 +0100 Subject: [PATCH 1/2] bpo-45401: Change shouldRollover() methods to only rollover regular files. Also changed some historical return values from 1->True and 0->False. --- Lib/logging/handlers.py | 14 ++++++++++---- Lib/test/test_logging.py | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/Lib/logging/handlers.py b/Lib/logging/handlers.py index f1a2e3b69986e9..0c78920f2fda6c 100644 --- a/Lib/logging/handlers.py +++ b/Lib/logging/handlers.py @@ -187,14 +187,17 @@ def shouldRollover(self, record): Basically, see if the supplied record would cause the file to exceed the size limit we have. """ + # See bpo-45401: Never rollover anything other than regular files + if not os.path.isfile(self.baseFilename): + return False if self.stream is None: # delay was set... self.stream = self._open() if self.maxBytes > 0: # are we rolling over? msg = "%s\n" % self.format(record) self.stream.seek(0, 2) #due to non-posix-compliant Windows feature if self.stream.tell() + len(msg) >= self.maxBytes: - return 1 - return 0 + return True + return False class TimedRotatingFileHandler(BaseRotatingHandler): """ @@ -345,10 +348,13 @@ def shouldRollover(self, record): record is not used, as we are just comparing times, but it is needed so the method signatures are the same """ + # See bpo-45401: Never rollover anything other than regular files + if not os.path.isfile(self.baseFilename): + return False t = int(time.time()) if t >= self.rolloverAt: - return 1 - return 0 + return True + return False def getFilesToDelete(self): """ diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index 7a80244047d0f1..b5885b985afd31 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -5216,6 +5216,13 @@ def test_should_not_rollover(self): self.fn, encoding="utf-8", maxBytes=0) self.assertFalse(rh.shouldRollover(None)) rh.close() + # bpo-45401 - test with special file + # We set maxBytes to 1 so that rollover would normally happen, except + # for the check for regular files + rh = logging.handlers.RotatingFileHandler( + os.devnull, encoding="utf-8", maxBytes=1) + self.assertFalse(rh.shouldRollover(self.next_rec())) + rh.close() def test_should_rollover(self): rh = logging.handlers.RotatingFileHandler(self.fn, encoding="utf-8", maxBytes=1) @@ -5310,6 +5317,14 @@ def rotator(source, dest): rh.close() class TimedRotatingFileHandlerTest(BaseFileTest): + def test_should_not_rollover(self): + # See bpo-45401. Should only ever rollover regular files + fh = logging.handlers.TimedRotatingFileHandler( + os.devnull, 'S', encoding="utf-8", backupCount=1) + time.sleep(1.1) # a little over a second ... + r = logging.makeLogRecord({'msg': 'testing - device file'}) + self.assertFalse(fh.shouldRollover(r)) + # other test methods added below def test_rollover(self): fh = logging.handlers.TimedRotatingFileHandler( From 97184f14fae0d4ee5757c069764609d29eadb1b4 Mon Sep 17 00:00:00 2001 From: Vinay Sajip Date: Fri, 8 Oct 2021 19:28:01 +0100 Subject: [PATCH 2/2] Change check to cover for file existence. --- Lib/logging/handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/logging/handlers.py b/Lib/logging/handlers.py index 0c78920f2fda6c..b613bec1c42705 100644 --- a/Lib/logging/handlers.py +++ b/Lib/logging/handlers.py @@ -188,7 +188,7 @@ def shouldRollover(self, record): the size limit we have. """ # See bpo-45401: Never rollover anything other than regular files - if not os.path.isfile(self.baseFilename): + if os.path.exists(self.baseFilename) and not os.path.isfile(self.baseFilename): return False if self.stream is None: # delay was set... self.stream = self._open() @@ -349,7 +349,7 @@ def shouldRollover(self, record): the method signatures are the same """ # See bpo-45401: Never rollover anything other than regular files - if not os.path.isfile(self.baseFilename): + if os.path.exists(self.baseFilename) and not os.path.isfile(self.baseFilename): return False t = int(time.time()) if t >= self.rolloverAt: