bpo-45401: Change shouldRollover() methods to only rollover regular f… (GH-28822)

…iles.

Also changed some historical return values from 1 -> True and 0 -> False.
This commit is contained in:
Vinay Sajip 2021-10-10 16:15:24 +01:00 committed by GitHub
parent 0bcc5ade9b
commit 62a667784b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 4 deletions

View file

@ -187,14 +187,17 @@ class RotatingFileHandler(BaseRotatingHandler):
Basically, see if the supplied record would cause the file to exceed Basically, see if the supplied record would cause the file to exceed
the size limit we have. 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... if self.stream is None: # delay was set...
self.stream = self._open() self.stream = self._open()
if self.maxBytes > 0: # are we rolling over? if self.maxBytes > 0: # are we rolling over?
msg = "%s\n" % self.format(record) msg = "%s\n" % self.format(record)
self.stream.seek(0, 2) #due to non-posix-compliant Windows feature self.stream.seek(0, 2) #due to non-posix-compliant Windows feature
if self.stream.tell() + len(msg) >= self.maxBytes: if self.stream.tell() + len(msg) >= self.maxBytes:
return 1 return True
return 0 return False
class TimedRotatingFileHandler(BaseRotatingHandler): class TimedRotatingFileHandler(BaseRotatingHandler):
""" """
@ -345,10 +348,13 @@ class TimedRotatingFileHandler(BaseRotatingHandler):
record is not used, as we are just comparing times, but it is needed so record is not used, as we are just comparing times, but it is needed so
the method signatures are the same 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()) t = int(time.time())
if t >= self.rolloverAt: if t >= self.rolloverAt:
return 1 return True
return 0 return False
def getFilesToDelete(self): def getFilesToDelete(self):
""" """

View file

@ -5216,6 +5216,13 @@ class RotatingFileHandlerTest(BaseFileTest):
self.fn, encoding="utf-8", maxBytes=0) self.fn, encoding="utf-8", maxBytes=0)
self.assertFalse(rh.shouldRollover(None)) self.assertFalse(rh.shouldRollover(None))
rh.close() 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): def test_should_rollover(self):
rh = logging.handlers.RotatingFileHandler(self.fn, encoding="utf-8", maxBytes=1) rh = logging.handlers.RotatingFileHandler(self.fn, encoding="utf-8", maxBytes=1)
@ -5310,6 +5317,14 @@ class RotatingFileHandlerTest(BaseFileTest):
rh.close() rh.close()
class TimedRotatingFileHandlerTest(BaseFileTest): 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 # other test methods added below
def test_rollover(self): def test_rollover(self):
fh = logging.handlers.TimedRotatingFileHandler( fh = logging.handlers.TimedRotatingFileHandler(