[Python-checkins] bpo-45401: Change shouldRollover() methods to only rollover regular f… (GH-28822) (#28867)
vstinner
webhook-mailer at python.org
Mon Oct 11 06:01:18 EDT 2021
https://github.com/python/cpython/commit/5aca34f17c4baf8e4882a7e8a827cff06ac6ef25
commit: 5aca34f17c4baf8e4882a7e8a827cff06ac6ef25
branch: 3.10
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: vstinner <vstinner at python.org>
date: 2021-10-11T12:01:12+02:00
summary:
bpo-45401: Change shouldRollover() methods to only rollover regular f… (GH-28822) (#28867)
…iles.
Also changed some historical return values from 1 -> True and 0 -> False.
(cherry picked from commit 62a667784ba7b84611ebd50fa8a1a464cde32235)
Co-authored-by: Vinay Sajip <vinay_sajip at yahoo.co.uk>
Co-authored-by: Vinay Sajip <vinay_sajip at yahoo.co.uk>
files:
M Lib/logging/handlers.py
M Lib/test/test_logging.py
diff --git a/Lib/logging/handlers.py b/Lib/logging/handlers.py
index 4cabc0d740c29..4dcbe4530fcfc 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 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()
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 os.path.exists(self.baseFilename) and 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 8356e6be24baf..1a5f8b60f5ccb 100644
--- a/Lib/test/test_logging.py
+++ b/Lib/test/test_logging.py
@@ -5190,6 +5190,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)
@@ -5284,6 +5291,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(
More information about the Python-checkins
mailing list