Skip to content

GH-120754: Make PY_READ_MAX smaller than max byteobject size #121633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

GH-120754: Make PY_READ_MAX smaller than max byteobject size #121633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5a6e0ea
GH-120754: Make PY_READ_MAX fit in byteobject size
cmaloney Jul 11, 2024
c60ae27
Add news blurb
cmaloney Jul 11, 2024
da8b55c
tweak news
cmaloney Jul 17, 2024
1f4d053
tweak news more
cmaloney Jul 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions Include/internal/pycore_fileutils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,13 @@ extern PyObject* _Py_device_encoding(int);
# define _PY_WRITE_MAX INT_MAX
#else
/* write() should truncate the input to PY_SSIZE_T_MAX bytes,
but it's safer to do it ourself to have a portable behaviour */
# define _PY_READ_MAX PY_SSIZE_T_MAX
but it's safer to do it ourself to have a portable behaviour

read() fills a PyBytes object, which has a capped size defined in
bytesobject.c. Prefer reading less data (meets the API spec) to causing
an overflow error.
*/
# define _PY_READ_MAX PY_SSIZE_T_MAX - 4096
# define _PY_WRITE_MAX PY_SSIZE_T_MAX
#endif

Expand Down
7 changes: 3 additions & 4 deletions Lib/test/test_os.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,17 +234,16 @@ def test_read(self):
self.assertEqual(s, b"spam")

@support.cpython_only
# Skip the test on 32-bit platforms: the number of bytes must fit in a
# Py_ssize_t type
@unittest.skipUnless(INT_MAX < PY_SSIZE_T_MAX,
"needs INT_MAX < PY_SSIZE_T_MAX")
@support.bigmemtest(size=INT_MAX + 10, memuse=1, dry_run=False)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue here is that this will result in a memory allocation error on 32 bit machines, and this bigmemtest effectively makes it not run on 32 bit machines (They are unlikely to have that much RAM)

def test_large_read(self, size):
self.addCleanup(os_helper.unlink, os_helper.TESTFN)
create_file(os_helper.TESTFN, b'test')

# Issue #21932: Make sure that os.read() does not raise an
# OverflowError for size larger than INT_MAX
#
# For 32 bit systems, it is expected the read doesn't read all the bytes
# but rather caps to a number it can reasonably read.
with open(os_helper.TESTFN, "rb") as fp:
data = os.read(fp.fileno(), size)

Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2024-07-11-13-57-50.gh-issue-120754.C1HedA.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
``os.read()`` caps read size smaller than the max bytes object size to avoid
getting an OverflowError that 'byte string is too large'. This makes it so the
read call is attempted (although still may fail for other reasons).
Loading