https://github.com/python/cpython/commit/b76a4a5db7b4f97942b2cd7027f5a59ce7cbb523 commit: b76a4a5db7b4f97942b2cd7027f5a59ce7cbb523 branch: 3.13 author: Miss Islington (bot) <[email protected]> committer: Yhg1s <[email protected]> date: 2024-09-02T12:07:27Z summary:
[3.13] gh-116263: Do not rollover empty files in RotatingFileHandler (GH-122788) (#122814) gh-116263: Do not rollover empty files in RotatingFileHandler (GH-122788) (cherry picked from commit 6094c6fc2fc30eb9ee7c2f9f1088a851f71bf1b9) Co-authored-by: Serhiy Storchaka <[email protected]> files: A Misc/NEWS.d/next/Library/2024-08-07-17-41-16.gh-issue-116263.EcXir0.rst M Lib/logging/handlers.py M Lib/test/test_logging.py diff --git a/Lib/logging/handlers.py b/Lib/logging/handlers.py index 0fa40f56e998d5..1cba64fd554100 100644 --- a/Lib/logging/handlers.py +++ b/Lib/logging/handlers.py @@ -196,9 +196,12 @@ def shouldRollover(self, record): if self.stream is None: # delay was set... self.stream = self._open() if self.maxBytes > 0: # are we rolling over? + pos = self.stream.tell() + if not pos: + # gh-116263: Never rollover an empty file + return False 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: + if pos + len(msg) >= self.maxBytes: # See bpo-45401: Never rollover anything other than regular files if os.path.exists(self.baseFilename) and not os.path.isfile(self.baseFilename): return False diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index 1ebe0f623d23a0..72252e0ae2bb0e 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -6195,13 +6195,28 @@ def test_emit_after_closing_in_write_mode(self): self.assertEqual(fp.read().strip(), '1') class RotatingFileHandlerTest(BaseFileTest): - @unittest.skipIf(support.is_wasi, "WASI does not have /dev/null.") def test_should_not_rollover(self): - # If maxbytes is zero rollover never occurs + # If file is empty rollover never occurs + rh = logging.handlers.RotatingFileHandler( + self.fn, encoding="utf-8", maxBytes=1) + self.assertFalse(rh.shouldRollover(None)) + rh.close() + + # If maxBytes is zero rollover never occurs rh = logging.handlers.RotatingFileHandler( self.fn, encoding="utf-8", maxBytes=0) self.assertFalse(rh.shouldRollover(None)) rh.close() + + with open(self.fn, 'wb') as f: + f.write(b'\n') + rh = logging.handlers.RotatingFileHandler( + self.fn, encoding="utf-8", maxBytes=0) + self.assertFalse(rh.shouldRollover(None)) + rh.close() + + @unittest.skipIf(support.is_wasi, "WASI does not have /dev/null.") + def test_should_not_rollover_non_file(self): # bpo-45401 - test with special file # We set maxBytes to 1 so that rollover would normally happen, except # for the check for regular files @@ -6211,18 +6226,47 @@ def test_should_not_rollover(self): rh.close() def test_should_rollover(self): - rh = logging.handlers.RotatingFileHandler(self.fn, encoding="utf-8", maxBytes=1) + with open(self.fn, 'wb') as f: + f.write(b'\n') + rh = logging.handlers.RotatingFileHandler(self.fn, encoding="utf-8", maxBytes=2) self.assertTrue(rh.shouldRollover(self.next_rec())) rh.close() def test_file_created(self): # checks that the file is created and assumes it was created # by us + os.unlink(self.fn) rh = logging.handlers.RotatingFileHandler(self.fn, encoding="utf-8") rh.emit(self.next_rec()) self.assertLogFile(self.fn) rh.close() + def test_max_bytes(self, delay=False): + kwargs = {'delay': delay} if delay else {} + os.unlink(self.fn) + rh = logging.handlers.RotatingFileHandler( + self.fn, encoding="utf-8", backupCount=2, maxBytes=100, **kwargs) + self.assertIs(os.path.exists(self.fn), not delay) + small = logging.makeLogRecord({'msg': 'a'}) + large = logging.makeLogRecord({'msg': 'b'*100}) + self.assertFalse(rh.shouldRollover(small)) + self.assertFalse(rh.shouldRollover(large)) + rh.emit(small) + self.assertLogFile(self.fn) + self.assertFalse(os.path.exists(self.fn + ".1")) + self.assertFalse(rh.shouldRollover(small)) + self.assertTrue(rh.shouldRollover(large)) + rh.emit(large) + self.assertTrue(os.path.exists(self.fn)) + self.assertLogFile(self.fn + ".1") + self.assertFalse(os.path.exists(self.fn + ".2")) + self.assertTrue(rh.shouldRollover(small)) + self.assertTrue(rh.shouldRollover(large)) + rh.close() + + def test_max_bytes_delay(self): + self.test_max_bytes(delay=True) + def test_rollover_filenames(self): def namer(name): return name + ".test" @@ -6231,11 +6275,15 @@ def namer(name): rh.namer = namer rh.emit(self.next_rec()) self.assertLogFile(self.fn) + self.assertFalse(os.path.exists(namer(self.fn + ".1"))) rh.emit(self.next_rec()) self.assertLogFile(namer(self.fn + ".1")) + self.assertFalse(os.path.exists(namer(self.fn + ".2"))) rh.emit(self.next_rec()) self.assertLogFile(namer(self.fn + ".2")) self.assertFalse(os.path.exists(namer(self.fn + ".3"))) + rh.emit(self.next_rec()) + self.assertFalse(os.path.exists(namer(self.fn + ".3"))) rh.close() def test_namer_rotator_inheritance(self): diff --git a/Misc/NEWS.d/next/Library/2024-08-07-17-41-16.gh-issue-116263.EcXir0.rst b/Misc/NEWS.d/next/Library/2024-08-07-17-41-16.gh-issue-116263.EcXir0.rst new file mode 100644 index 00000000000000..167ca943b3527c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-08-07-17-41-16.gh-issue-116263.EcXir0.rst @@ -0,0 +1,2 @@ +:class:`logging.handlers.RotatingFileHandler` no longer rolls over empty log +files. _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3/lists/python-checkins.python.org/ Member address: [email protected]
