Skip to content

Commit ac421c3

Browse files
bpo-45401: Change shouldRollover() methods to only rollover regular f… (GH-28822) (#28866)
…iles. Also changed some historical return values from 1 -> True and 0 -> False. (cherry picked from commit 62a6677) Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk> Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>
1 parent d57d33c commit ac421c3

File tree

2 files changed

+25
-4
lines changed

2 files changed

+25
-4
lines changed

Lib/logging/handlers.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,14 +185,17 @@ def shouldRollover(self, record):
185185
Basically, see if the supplied record would cause the file to exceed
186186
the size limit we have.
187187
"""
188+
# See bpo-45401: Never rollover anything other than regular files
189+
if os.path.exists(self.baseFilename) and not os.path.isfile(self.baseFilename):
190+
return False
188191
if self.stream is None: # delay was set...
189192
self.stream = self._open()
190193
if self.maxBytes > 0: # are we rolling over?
191194
msg = "%s\n" % self.format(record)
192195
self.stream.seek(0, 2) #due to non-posix-compliant Windows feature
193196
if self.stream.tell() + len(msg) >= self.maxBytes:
194-
return 1
195-
return 0
197+
return True
198+
return False
196199

197200
class TimedRotatingFileHandler(BaseRotatingHandler):
198201
"""
@@ -342,10 +345,13 @@ def shouldRollover(self, record):
342345
record is not used, as we are just comparing times, but it is needed so
343346
the method signatures are the same
344347
"""
348+
# See bpo-45401: Never rollover anything other than regular files
349+
if os.path.exists(self.baseFilename) and not os.path.isfile(self.baseFilename):
350+
return False
345351
t = int(time.time())
346352
if t >= self.rolloverAt:
347-
return 1
348-
return 0
353+
return True
354+
return False
349355

350356
def getFilesToDelete(self):
351357
"""

Lib/test/test_logging.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5066,6 +5066,13 @@ def test_should_not_rollover(self):
50665066
rh = logging.handlers.RotatingFileHandler(self.fn, maxBytes=0)
50675067
self.assertFalse(rh.shouldRollover(None))
50685068
rh.close()
5069+
# bpo-45401 - test with special file
5070+
# We set maxBytes to 1 so that rollover would normally happen, except
5071+
# for the check for regular files
5072+
rh = logging.handlers.RotatingFileHandler(
5073+
os.devnull, encoding="utf-8", maxBytes=1)
5074+
self.assertFalse(rh.shouldRollover(self.next_rec()))
5075+
rh.close()
50695076

50705077
def test_should_rollover(self):
50715078
rh = logging.handlers.RotatingFileHandler(self.fn, maxBytes=1)
@@ -5160,6 +5167,14 @@ def rotator(source, dest):
51605167
rh.close()
51615168

51625169
class TimedRotatingFileHandlerTest(BaseFileTest):
5170+
def test_should_not_rollover(self):
5171+
# See bpo-45401. Should only ever rollover regular files
5172+
fh = logging.handlers.TimedRotatingFileHandler(
5173+
os.devnull, 'S', encoding="utf-8", backupCount=1)
5174+
time.sleep(1.1) # a little over a second ...
5175+
r = logging.makeLogRecord({'msg': 'testing - device file'})
5176+
self.assertFalse(fh.shouldRollover(r))
5177+
51635178
# other test methods added below
51645179
def test_rollover(self):
51655180
fh = logging.handlers.TimedRotatingFileHandler(self.fn, 'S',

0 commit comments

Comments
 (0)