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

Commit a5eaa14

Browse files
miss-islington6t8k
miss-islington
and
6t8k
authored
[3.11] gh-95782: Fix io.BufferedReader.tell() etc. being able to return offsets < 0 (GH-99709) (GH-115600)
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. (cherry picked from commit 26800cf) Co-authored-by: 6t8k <58048945+6t8k@users.noreply.github.com>
1 parent 2c39d00 commit a5eaa14

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
@@ -1224,7 +1224,8 @@ def _readinto(self, buf, read1):
12241224
return written
12251225

12261226
def tell(self):
1227-
return _BufferedIOMixin.tell(self) - len(self._read_buf) + self._read_pos
1227+
# GH-95782: Keep return value non-negative
1228+
return max(_BufferedIOMixin.tell(self) - len(self._read_buf) + self._read_pos, 0)
12281229

12291230
def seek(self, pos, whence=0):
12301231
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
@@ -263,6 +263,27 @@ class PyMockUnseekableIO(MockUnseekableIO, pyio.BytesIO):
263263
UnsupportedOperation = pyio.UnsupportedOperation
264264

265265

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

268289
def __init__(self):
@@ -1556,6 +1577,30 @@ def test_truncate_on_read_only(self):
15561577
self.assertRaises(self.UnsupportedOperation, bufio.truncate)
15571578
self.assertRaises(self.UnsupportedOperation, bufio.truncate, 0)
15581579

1580+
def test_tell_character_device_file(self):
1581+
# GH-95782
1582+
# For the (former) bug in BufferedIO to manifest, the wrapped IO obj
1583+
# must be able to produce at least 2 bytes.
1584+
raw = self.MockCharPseudoDevFileIO(b"12")
1585+
buf = self.tp(raw)
1586+
self.assertEqual(buf.tell(), 0)
1587+
self.assertEqual(buf.read(1), b"1")
1588+
self.assertEqual(buf.tell(), 0)
1589+
1590+
def test_seek_character_device_file(self):
1591+
raw = self.MockCharPseudoDevFileIO(b"12")
1592+
buf = self.tp(raw)
1593+
self.assertEqual(buf.seek(0, io.SEEK_CUR), 0)
1594+
self.assertEqual(buf.seek(1, io.SEEK_SET), 0)
1595+
self.assertEqual(buf.seek(0, io.SEEK_CUR), 0)
1596+
self.assertEqual(buf.read(1), b"1")
1597+
1598+
# In the C implementation, tell() sets the BufferedIO's abs_pos to 0,
1599+
# which means that the next seek() could return a negative offset if it
1600+
# does not sanity-check:
1601+
self.assertEqual(buf.tell(), 0)
1602+
self.assertEqual(buf.seek(0, io.SEEK_CUR), 0)
1603+
15591604

15601605
class CBufferedReaderTest(BufferedReaderTest, SizeofTest):
15611606
tp = io.BufferedReader
@@ -4790,7 +4835,7 @@ def load_tests(loader, tests, pattern):
47904835
# classes in the __dict__ of each test.
47914836
mocks = (MockRawIO, MisbehavedRawIO, MockFileIO, CloseFailureIO,
47924837
MockNonBlockWriterIO, MockUnseekableIO, MockRawIOWithoutRead,
4793-
SlowFlushRawIO)
4838+
SlowFlushRawIO, MockCharPseudoDevFileIO)
47944839
all_members = io.__all__
47954840
c_io_ns = {name : getattr(io, name) for name in all_members}
47964841
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
@@ -1196,7 +1196,11 @@ buffered_tell(buffered *self, PyObject *Py_UNUSED(ignored))
11961196
if (pos == -1)
11971197
return NULL;
11981198
pos -= RAW_OFFSET(self);
1199-
/* TODO: sanity check (pos >= 0) */
1199+
1200+
// GH-95782
1201+
if (pos < 0)
1202+
pos = 0;
1203+
12001204
return PyLong_FromOff_t(pos);
12011205
}
12021206

@@ -1263,6 +1267,11 @@ _io__Buffered_seek_impl(buffered *self, PyObject *targetobj, int whence)
12631267
offset = target;
12641268
if (offset >= -self->pos && offset <= avail) {
12651269
self->pos += offset;
1270+
1271+
// GH-95782
1272+
if (current - avail + offset < 0)
1273+
return PyLong_FromOff_t(0);
1274+
12661275
return PyLong_FromOff_t(current - avail + offset);
12671276
}
12681277
}

0 commit comments

Comments
 (0)
0