[issue22939] integer overflow in iterator object

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for your contribution Clement.

--
resolution:  -> fixed
stage: commit review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Roundup Robot]

Roundup Robot added the comment:

New changeset d6179accca20 by Serhiy Storchaka in branch '2.7':
Fixed issue number for issue #22939.
https://hg.python.org/cpython/rev/d6179accca20

New changeset 7fa2f4afcf5a by Serhiy Storchaka in branch '3.4':
Fixed issue number for issue #22939.
https://hg.python.org/cpython/rev/7fa2f4afcf5a

New changeset f23533fa6afa by Serhiy Storchaka in branch 'default':
Fixed issue number for issue #22939.
https://hg.python.org/cpython/rev/f23533fa6afa

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

If Raymond will not stop me, I'll commit the patch tomorrow.

--
assignee:  -> serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
versions: +Python 2.7, Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Clement Rouault]

Clement Rouault added the comment:

After a few months, I am back to you on this issue.
What should be the next step of the process ?

--
status: pending -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> That way a virtual sequence with PY_SSIZE_T_MAX-1 items would still work 
> (instead of failing unexpectedly).

Actually with PY_SSIZE_T_MAX+1 items (indices from 0 to PY_SSIZE_T_MAX 
inclusive).

If Raymond insists I can write more complicated patch, but I don't think that 
we should complicate the code for this pretty hypotetical case. I'm for 
committing issue22939v2.patch.

--
status: open -> pending

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Ronald Oussoren]

Ronald Oussoren added the comment:

Is it necessary to raise when it_index is PY_SSIZE_T_MAX? 

An alternative is to set it_index to -1 when there would be overflow and raise 
an exception on the next call to next(). That way a virtual sequence with 
PY_SSIZE_T_MAX-1 items would still work (instead of failing unexpectedly).

--
nosy: +ronaldoussoren

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Clement Rouault]

Clement Rouault added the comment:

> > The call to PySequence_GetItem() may be expensive, and we have to drop
> > the result if an OverflowError is raised after the call.

> You do realize that this error will be very rare and therefore 
> inconsequential.

The real question is: why would you call the iterator for a new value if it 
will be discarded anyway ? I think it could be very misleading to see  
_getitem__ being called and have an OverflowError being raised afterward.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Raymond Hettinger]

Raymond Hettinger added the comment:

> The call to PySequence_GetItem() may be expensive, and we have to drop
> the result if an OverflowError is raised after the call.

You do realize that this error will be very rare and therefore inconsequential.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[STINNER Victor]

STINNER Victor added the comment:

I prefer to raise an OverflowError *before* calling
PySequence_GetItem(). The call to PySequence_GetItem() may be
expensive, and we have to drop the result if an OverflowError is
raised after the call. At the end, the behaviour is the same: an
OverflowError is raised.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Clement Rouault]

Clement Rouault added the comment:

> Also, PY_SSIZE_T_MAX is a valid value to pass to PySequence_GetItem(), so it 
> shouldn't be blocked unless necessary.

I agree with you, that's why my first path was checking at the next call if 
it->it_index had overflowed. But then it relies on undefined behaviour.

> I would think that the PY_SSIZE_T_MAX check belongs inside the:
> 
> if (result != NULL) {
> it->it_index++;
> return result;
> }

If we raise the OverflowError when it->it_index really overflow (just after the 
getitem PY_SSIZE_T_MAX).
Is it really necessary to do the overflow check after the GetItem ? because the 
value returned by `PySequence_GetItem(seq, PY_SSIZE_T_MAX);` will be never used.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

This doesn't matter because next() will raise an exception if it_index == 
PY_SSIZE_T_MAX in any case. The code should be changed much more to allow 
yielding an item with index PY_SSIZE_T_MAX, use other (negative) signal value 
and change the behavior of __setstate__ for negative values.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Raymond Hettinger]

Raymond Hettinger added the comment:

I would think that the PY_SSIZE_T_MAX check belongs inside the:

if (result != NULL) {
it->it_index++;
return result;
}

just before the increment which could cause the overflow.  Also, PY_SSIZE_T_MAX 
is a valid value to pass to PySequence_GetItem(), so it shouldn't be blocked 
unless necessary.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Ah, __setstate__ already handles negative indices. Then the patch LGTM.

--
stage: needs patch -> commit review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

__setstate__ should check that an index is not negative. All values from 0 to 
PY_SSIZE_T_MAX are acceptable.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Clement Rouault]

Clement Rouault added the comment:

Thanks for the comments.
I corrected the patch and updated the test. I also added a test that check the 
behavior of setstate with negative indice.

Just one question:

>  __setstate__ must implement the same check.

Why should __setstate__ check for PY_SSIZE_T_MAX (throwing OverflowError when 
unpickling) if the check will be done when calling next on the resulting 
iterator ?

--
Added file: http://bugs.python.org/file37382/issue22939v2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> check if the index is greater or equal than PY_SSIZET_MAX - 1.

Just PY_SSIZET_MAX.

Added other comments on Rietveld (look the email in the Spam folder).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[STINNER Victor]

STINNER Victor added the comment:

You should not rely on undefined behaviour: check if the index is greater
or equal than PY_SSIZET_MAX - 1. __setstate__ must implement the same check.

You must always raise an error, not hide it in a second call to next (raise
StopIteration). Your unit test must check this behaviour.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Clement Rouault]

Clement Rouault added the comment:

Here is a first try for a patch.

There are two points I am not sure about:

1) The message for the OverflowError: is that explicit enough ?

2) The behaviour of the iterator after the raise of OverflowError. 
With this patch every call to `next(it)` where `it` have overflowed will raise 
`OverflowError` again.
Does this behaviour seems correct our should it raise StopIteration after the 
first OverflowError ?

--
keywords: +patch
Added file: http://bugs.python.org/file37375/issue22939.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> What is your use case?

Something like range(). I agree that it is very unlike to encounter this 
problem in real work, and we can live with implementation-related limitation 
(for which OverflowError exists).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[STINNER Victor]

STINNER Victor added the comment:

> I think OverflowError is good for maintained releases, but for 3.5 Clement's 
> idea with long index looks attractive to me.

What is your use case?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I think OverflowError is good for maintained releases, but for 3.5 Clement's 
idea with long index looks attractive to me. In any case an exception should be 
raised for negative argument in __setstate__(). Let split this issue on two 
parts. First fix the bug by raising exceptions, and then add long index (if 
anyone will need it).

--
nosy: +mark.dickinson

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[STINNER Victor]

STINNER Victor added the comment:

> The `it_index` attribute used by the iterator is a `Py_ssize_t` but overflow 
> is never checked.

Yes it is a bug. iter_iternext() must raises an OverflowError if it->it_index 
is equal to PY_SSIZE_T_MAX.

--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[R. David Murray]

R. David Murray added the comment:

For possibly relevant background information, see issue 21444 and the issues 
linked from it, and issue 14794.

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +rhettinger, serhiy.storchaka
stage:  -> needs patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22939] integer overflow in iterator object

[Clement Rouault]

New submission from Clement Rouault:

I found an interger overflow in the standard iterator object 
(Objects/iterobject.c)

The `it_index` attribute used by the iterator is a `Py_ssize_t` but overflow is 
never checked. So after the index `PY_SSIZE_T_MAX`, the iterator object will 
ask for the index `PY_SSIZE_T_MIN`.

Here is an example:

import sys

class Seq:
def __getitem__(self, item):
print("[-] Asked for item at <{0}>".format(item))
return 0

s = Seq()
i = iter(s)
# Manually set `it_index` to PY_SSIZE_T_MAX without a loop
i.__setstate__(sys.maxsize)

next(i)
[-] Asked for item at <9223372036854775807>
next(i)
[-] Asked for item at <-9223372036854775808>


I would be really interested in writing a patch but first I wanted to discuss 
the expected behaviour and fix.

The iterator could stop after `PY_SSIZE_T_MAX` but it seems strange as other 
iterator (like `enumobject`) handle values bigger than `PY_SSIZE_T_MAX`.

Or the same technique used in `enumobject` could be used: adding a field 
`PyObject* en_longindex` (a python long) which would be used for values bigger 
than `PY_SSIZE_T_MAX`

--
components: Interpreter Core
messages: 231651
nosy: hakril
priority: normal
severity: normal
status: open
title: integer overflow in iterator object
type: behavior
versions: Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com