Skip to content

GH-141312: Allow only integers to longrangeiter_setstate state #141317

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+31 −0
Open

GH-141312: Allow only integers to longrangeiter_setstate state #141317

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aea82e8
Allow only integers to longrangeiter_setstate state
sergey-miryanov Nov 9, 2025
4eaee49
Check state instead of product
sergey-miryanov Nov 9, 2025
c11596e
Update tests
sergey-miryanov Nov 10, 2025
33190fe
Add extra check to rangeiter_setstate
sergey-miryanov Nov 10, 2025
8d08e5a
Update error message for longrangeiter_setstate
sergey-miryanov Nov 10, 2025
4011aa2
Add news
sergey-miryanov Nov 10, 2025
7e3c5af
Fix news
sergey-miryanov Nov 10, 2025
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: 9 additions & 0 deletions Lib/test/test_range.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,15 @@ def test_iterator_setstate(self):
it.__setstate__(2**64 - 7)
self.assertEqual(list(it), [12, 10])

msg = "'float' object cannot be interpreted as an integer"
with self.assertRaisesRegex(TypeError, msg):
it = iter(range(10, 100, 2))
it.__setstate__(1.0)
Copy link
Member

@picnixz picnixz Nov 9, 2025

Choose a reason for hiding this comment

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

  • Make a separate test method for setstate failures.
  • Only assert the exception when calling setstate.
  • Check that custom int subclasses are rejected. What is the current behavior with range objects not having exact ints?

Copy link
Contributor Author

@sergey-miryanov sergey-miryanov Nov 10, 2025

Choose a reason for hiding this comment

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

Range iter give following error messages before last changes:

  1. float: 'float' object cannot be interpreted as an integer
  2. I() (int-like object): no error message
  3. str: 'str' object cannot be interpreted as an integer


with self.assertRaisesRegex(TypeError, msg):
it = iter(range(10, 2**65, 2))
it.__setstate__(1.0)

def test_odd_bug(self):
# This used to raise a "SystemError: NULL result without error"
# because the range validation step was eating the exception
Expand Down
6 changes: 6 additions & 0 deletions Objects/rangeobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,12 @@ longrangeiter_reduce(PyObject *op, PyObject *Py_UNUSED(ignored))
static PyObject *
longrangeiter_setstate(PyObject *op, PyObject *state)
{
if (!PyLong_CheckExact(state)) {
PyErr_Format(PyExc_TypeError,
"'%T' object cannot be interpreted as an integer", state);
Copy link
Member

@serhiy-storchaka serhiy-storchaka Nov 10, 2025

Choose a reason for hiding this comment

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

Such error messages is usually used when the argument can be an int subclass of has the __index__ method. But this is not needed here, only int is supported.

Copy link
Member

@picnixz picnixz Nov 10, 2025

Choose a reason for hiding this comment

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

See my discussion here: #141317 (comment). Should we change rangeiter to match messages?

return NULL;
}
Comment on lines 1050 to 1053
Copy link
Member

@picnixz picnixz Nov 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
if (!PyLong_CheckExact(state)) {
PyErr_Format(PyExc_TypeError,
"'%T' object cannot be interpreted as an integer", state);
return NULL;
}
if (!PyLong_CheckExact(state)) {
PyErr_Format(PyExc_TypeError, "state must be an int, not %T", state);
return NULL;
}

Copy link
Contributor Author

@sergey-miryanov sergey-miryanov Nov 10, 2025

Choose a reason for hiding this comment

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

This is exactly what I added before checking what rangeiter returns for a wrong state. Is it ok if our messages will be different?

Copy link
Member

@picnixz picnixz Nov 10, 2025

Choose a reason for hiding this comment

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

Oh so that is the message of rangeiter. Ok keep it the same.

Copy link
Member

@serhiy-storchaka serhiy-storchaka Nov 10, 2025

Choose a reason for hiding this comment

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

The proposed message is better.

Copy link
Member

@serhiy-storchaka serhiy-storchaka Nov 10, 2025

Choose a reason for hiding this comment

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

rangeiter_setstate() supports int subclasses and classes with the __index__ method. If we want to support them here (__index__ is not currently supported), we should use PyNumber_Index().

But there is no good reason to do this. It would only complicate the code without adding any benefit.


longrangeiterobject *r = (longrangeiterobject*)op;
PyObject *zero = _PyLong_GetZero(); // borrowed reference
int cmp;
Expand Down
Loading