Hi guys, as it stands the ticket, the user has implemented an iterator which easily accepts floats for seek and other iterator methods. Normal iterator can only work with integers as all the storage for positions is long or int. As result a statement like this
new LimitIterator(new ArrayIterator(array(42)), 10000000000000000000); will just convert that big float to some integer, so the original iterator wouldn't work properly. This is of course not the case with iterators implemented internally in SPL as they only work with long (ulong for instance with the ArrayIterator). But any user space iterators might break this. Just like in the ticket the iterator itself works as expected and gives 10000000000 10000000001 10000000002 Where passing it to the LimitIterator gives back 1410065408 1410065409 1410065410 Also please consider the fact that the documentation states both start and offset params of the LimitIterator as int. I'm on the way to revert this, but the issue stays. May be the patch should be just extended to check if it's a numeric string and try to convert it. The same for a float - check if it overflows the integer range. Or how would you solve this? Regards Anatoliy Am Mi, 11.07.2012, 23:25 schrieb Nikita Popov: > Hi Anatoliy! > > I'm not sure that this change is right. As it is now one could no > longer pass in "123" (as a string) or 123.0 (as a float) as > limit/offset. This goes against usual PHP semantics. > > Also I'm not exactly sure what the problem was in the first place. > > Nikita > > On Wed, Jul 11, 2012 at 10:25 PM, Anatoliy Belsky <a...@php.net> wrote: >> Commit: b383ddf1e5175abf1d000e887961fdcebae646a0 >> Author: Anatoliy Belsky <a...@php.net> Wed, 11 Jul 2012 >> 22:25:31 +0200 >> Parents: bcf5853eaa8b8be793d4a1bd325eaea68cfe57bb >> Branches: PHP-5.3 PHP-5.4 master >> >> Link: >> http://git.php.net/?p=php-src.git;a=commitdiff;h=b383ddf1e5175abf1d000e887961fdcebae646a0 >> >> Log: >> Fixed bug #62477 LimitIterator int overflow >> >> Bugs: >> https://bugs.php.net/62477 >> >> Changed paths: >> M ext/spl/spl_iterators.c >> M ext/spl/spl_iterators.h >> A ext/spl/tests/bug62477_1.phpt >> A ext/spl/tests/bug62477_2.phpt >> >> >> Diff: >> diff --git a/ext/spl/spl_iterators.c b/ext/spl/spl_iterators.c >> index eecd483..1cbb2e4 100755 >> --- a/ext/spl/spl_iterators.c >> +++ b/ext/spl/spl_iterators.c >> @@ -1380,12 +1380,31 @@ static spl_dual_it_object* >> spl_dual_it_construct(INTERNAL_FUNCTION_PARAMETERS, z >> intern->dit_type = dit_type; >> switch (dit_type) { >> case DIT_LimitIterator: { >> + zval *tmp_offset, *tmp_count; >> intern->u.limit.offset = 0; /* start at >> beginning */ >> intern->u.limit.count = -1; /* get all */ >> - if (zend_parse_parameters(ZEND_NUM_ARGS() >> TSRMLS_CC, "O|ll", &zobject, ce_inner, &intern->u.limit.offset, >> &intern->u.limit.count) == FAILURE) { >> + if (zend_parse_parameters(ZEND_NUM_ARGS() >> TSRMLS_CC, "O|zz", &zobject, ce_inner, &tmp_offset, &tmp_count) == >> FAILURE) { >> zend_restore_error_handling(&error_handling >> TSRMLS_CC); >> return NULL; >> } >> + if (tmp_offset && Z_TYPE_P(tmp_offset) != >> IS_NULL) { >> + if (Z_TYPE_P(tmp_offset) != IS_LONG) { >> + >> zend_throw_exception(spl_ce_OutOfRangeException, "offset param must be >> of type int", 0 TSRMLS_CC); >> + >> zend_restore_error_handling(&error_handling TSRMLS_CC); >> + return NULL; >> + } else { >> + intern->u.limit.offset = >> Z_LVAL_P(tmp_offset); >> + } >> + } >> + if (tmp_count && Z_TYPE_P(tmp_count) != IS_NULL) >> { >> + if (Z_TYPE_P(tmp_count) != IS_LONG) { >> + >> zend_throw_exception(spl_ce_OutOfRangeException, "count param must be of >> type int", 0 TSRMLS_CC); >> + >> zend_restore_error_handling(&error_handling TSRMLS_CC); >> + return NULL; >> + } else { >> + intern->u.limit.count = >> Z_LVAL_P(tmp_count); >> + } >> + } >> if (intern->u.limit.offset < 0) { >> >> zend_throw_exception(spl_ce_OutOfRangeException, >> "Parameter offset must be >= 0", 0 >> TSRMLS_CC); >> zend_restore_error_handling(&error_handling >> TSRMLS_CC); >> diff --git a/ext/spl/spl_iterators.h b/ext/spl/spl_iterators.h >> index 525a25c..9494b26 100755 >> --- a/ext/spl/spl_iterators.h >> +++ b/ext/spl/spl_iterators.h >> @@ -128,7 +128,7 @@ typedef struct _spl_dual_it_object { >> uint str_key_len; >> ulong int_key; >> int key_type; /* HASH_KEY_IS_STRING or >> HASH_KEY_IS_LONG */ >> - int pos; >> + long pos; >> } current; >> dual_it_type dit_type; >> union { >> diff --git a/ext/spl/tests/bug62477_1.phpt >> b/ext/spl/tests/bug62477_1.phpt >> new file mode 100644 >> index 0000000..1b768a7 >> --- /dev/null >> +++ b/ext/spl/tests/bug62477_1.phpt >> @@ -0,0 +1,12 @@ >> +--TEST-- >> +Bug #62477 LimitIterator int overflow when float is passed (1) >> +--FILE-- >> +<?php >> + >> +$it = new LimitIterator(new ArrayIterator(array(42)), >> 10000000000000000000); >> +--EXPECTF-- >> +Fatal error: Uncaught exception 'OutOfRangeException' with message >> 'offset param must be of type int' in %sbug62477_1.php:%d >> +Stack trace: >> +#0 %sbug62477_1.php(%d): >> LimitIterator->__construct(Object(ArrayIterator), %f) >> +#1 {main} >> + thrown in %sbug62477_1.php on line %d >> diff --git a/ext/spl/tests/bug62477_2.phpt >> b/ext/spl/tests/bug62477_2.phpt >> new file mode 100644 >> index 0000000..aa3468a >> --- /dev/null >> +++ b/ext/spl/tests/bug62477_2.phpt >> @@ -0,0 +1,12 @@ >> +--TEST-- >> +Bug #62477 LimitIterator int overflow when float is passed (2) >> +--FILE-- >> +<?php >> + >> +$it = new LimitIterator(new ArrayIterator(array(42)), 0, >> 10000000000000000000); >> +--EXPECTF-- >> +Fatal error: Uncaught exception 'OutOfRangeException' with message >> 'count param must be of type int' in %sbug62477_2.php:%d >> +Stack trace: >> +#0 %sbug62477_2.php(%d): >> LimitIterator->__construct(Object(ArrayIterator), 0, %f) >> +#1 {main} >> + thrown in %sbug62477_2.php on line %d >> >> >> -- >> PHP CVS Mailing List (http://www.php.net/) >> To unsubscribe, visit: http://www.php.net/unsub.php >> > -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
