8000 gh-95782: Fix io.BufferedReader.tell() etc. being able to return offs… · python/cpython@26800cf · GitHub
[go: up one dir, main page]
Skip to content

Commit 26800cf

Browse files
6t8k6t8k
authored
gh-95782: Fix io.BufferedReader.tell() etc. being able to return offsets < 0 (GH-99709)
lseek() always returns 0 for character pseudo-devices like `/dev/urandom` (for other non-regular files, e.g. `/dev/stdin`, it always returns -1, to which CPython reacts by raising appropriate exceptions). They are thus technically seekable despite not having seek semantics. When calling read() on e.g. an instance of `io.BufferedReader` that wraps such a file, `BufferedReader` reads ahead, filling its buffer, creating a discrepancy between the number of bytes read and the internal `tell()` always returning 0, which previously resulted in e.g. `BufferedReader.tell()` or `BufferedReader.seek()` being able to return positions < 0 even though these are supposed to be always >= 0. Invariably keep the return value non-negative by returning max(former_return_value, 0) instead, and add some corresponding tests.
1 parent d5a30a1 commit 26800cf

File tree

4 files changed

+62
-3
lines changed

4 files changed

+62
-3
lines changed

‎Lib/_pyio.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1197,7 +1197,8 @@ def _readinto(self, buf, read1):
11971197
return written
11981198

11991199
def tell(self):
1200-
return _BufferedIOMixin.tell(self) - len(self._read_buf) + self._read_pos
1200+
# GH-95782: Keep return value non-negative
1201+
return max(_BufferedIOMixin.tell(self) - len(self._read_buf) + self._read_pos, 0)
12011202

12021203
def seek(self, pos, whence=0):
12031204
if whence not in valid_seek_flags:

‎Lib/test/test_io.py

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,27 @@ class PyMockUnseekableIO(MockUnseekableIO, pyio.BytesIO):
257257
UnsupportedOperation = pyio.UnsupportedOperation
258258

259259

260+
class MockCharPseudoDevFileIO(MockFileIO):
261+
# GH-95782
262+
# ftruncate() does not work on these special files (and CPython then raises
263+
# appropriate exceptions), so truncate() does not have to be accounted for
264+
# here.
265+
def __init__(self, data):
266+
super().__init__(data)
267+
268+
def seek(self, *args):
269+
return 0
270+
271+
def tell(self, *args):
272+
return 0
273+
274+
class CMockCharPseudoDevFileIO(MockCharPseudoDevFileIO, io.BytesIO):
275+
pass
276+
277+
class PyMockCharPseudoDevFileIO(MockCharPseudoDevFileIO, pyio.BytesIO):
278+
pass
279+
280+
260281
class MockNonBlockWriterIO:
261282

262283
def __init__(self):
@@ -1648,6 +1669,30 @@ def test_truncate_on_read_only(self):
16481669
self.assertRaises(self.UnsupportedOperation, bufio.truncate)
16491670
self.assertRaises(self.UnsupportedOperation, bufio.truncate, 0)
16501671

1672+
def test_tell_character_device_file(self):
1673+
# GH-95782
1674+
# For the (former) bug in BufferedIO to manifest, the wrapped IO obj
1675+
# must be able to produce at least 2 bytes.
1676+
raw = self.MockCharPseudoDevFileIO(b"12")
1677+
buf = self.tp(raw)
1678+
self.assertEqual(buf.tell(), 0)
1679+
self.assertEqual(buf.read(1), b"1")
1680+
self.assertEqual(buf.tell(), 0)
1681+
1682+
def test_seek_character_device_file(self):
1683+
raw = self.MockCharPseudoDevFileIO(b"12")
1684+
buf = self.tp(raw)
1685+
self.assertEqual(buf.seek(0, io.SEEK_CUR), 0)
1686+
self.assertEqual(buf.seek(1, io.SEEK_SET), 0)
1687+
self.assertEqual(buf.seek(0, io.SEEK_CUR), 0)
1688+
self.assertEqual(buf.read(1), b"1")
1689+
1690+
# In the C implementation, tell() sets the BufferedIO's abs_pos to 0,
1691+
# which means that the next seek() could return a negative offset if it
1692+
# does not sanity-check:
1693+
self.assertEqual(buf.tell(), 0)
1694+
self.assertEqual(buf.seek(0, io.SEEK_CUR), 0)
1695+
16511696

16521697
class CBufferedReaderTest(BufferedReaderTest, SizeofTest):
16531698
tp = io.BufferedReader
@@ -4880,7 +4925,7 @@ def load_tests(loader, tests, pattern):
48804925
# classes in the __dict__ of each test.
48814926
mocks = (MockRawIO, MisbehavedRawIO, MockFileIO, CloseFailureIO,
48824927
MockNonBlockWriterIO, MockUnseekableIO, MockRawIOWithoutRead,
4883-
SlowFlushRawIO)
4928+
SlowFlushRawIO, MockCharPseudoDevFileIO)
48844929
all_members = io.__all__
48854930
c_io_ns = {name : getattr(io, name) for name in all_members}
48864931
py_io_ns = {name : getattr(pyio, name) for name in all_members}

‎Misc/NEWS.d/next/Library/2022-11-22-23-17-43.gh-issue-95782.an_and.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix :func:`io.BufferedReader.tell`, :func:`io.BufferedReader.seek`,
2+
:func:`_pyio.BufferedReader.tell`, :func:`io.BufferedRandom.tell`,
3+
:func:`io.BufferedRandom.seek` and :func:`_pyio.BufferedRandom.tell`
4+
being able to return negative offsets.

‎Modules/_io/bufferedio.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1325,7 +1325,11 @@ _io__Buffered_tell_impl(buffered *self)
13251325
if (pos == -1)
13261326
return NULL;
13271327
pos -= RAW_OFFSET(self);
1328-
/* TODO: sanity check (pos >= 0) */
1328+
1329+
// GH-95782
1330+
if (pos < 0)
1331+
pos = 0;
1332+
13291333
return PyLong_FromOff_t(pos);
13301334
}
13311335

@@ -1395,6 +1399,11 @@ _io__Buffered_seek_impl(buffered *self, PyObject *targetobj, int whence)
13951399
offset = target;
13961400
if (offset >= -self->pos && offset <= avail) {
13971401
self->pos += offset;
1402+
1403+
// GH-95782
1404+
if (current - avail + offset < 0)
1405+
return PyLong_FromOff_t(0);
1406+
13981407
return PyLong_FromOff_t(current - avail + offset);
13991408
}
14001409
}

0 commit comments

Comments
 (0)
0