Truncating the number is the right way to do it (as far as PHP semantics
goes). It's consistent across all PHP functions that take integers as input.
Nuno
-----Original Message-----
From: Anatoliy Belsky
Sent: Thursday, July 12, 2012 8:50 AM
To: Nikita Popov ; Stas Malyshev
Cc: php-cvs@lists.php.net
Subject: Re: [PHP-CVS] com php-src: Fixed bug #62477 LimitIterator int
overflow: ext/spl/spl_iterators.c ext/spl/spl_iterators.h
ext/spl/tests/bug62477_1.phpt ext/spl/tests/bug62477_2.phpt
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
