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

2012-07-12 Thread Anatoliy Belsky 
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)), 1000);

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

100
101
102

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;
 ulongint_key;
 int  key_type; /* HASH_KEY_IS_STRING or
 HASH_KEY_IS_LONG */

[PHP-CVS] com php-src: reverted changes for #62477: NEWS ext/spl/spl_iterators.c ext/spl/spl_iterators.h ext/spl/tests/bug62477_1.phpt ext/spl/tests/bug62477_2.phpt

2012-07-12 Thread Anatoliy Belsky 
Commit:ad7eeba3c1a4c77f439afc936fbf50b811a46a6b
Author:Anatoliy Belsky a...@php.net Thu, 12 Jul 2012 10:54:14 
+0200
Parents:   896d0fcd41df13fc852417de222aac2482d2704b
Branches:  PHP-5.3 PHP-5.4 master

Link:   
http://git.php.net/?p=php-src.git;a=commitdiff;h=ad7eeba3c1a4c77f439afc936fbf50b811a46a6b

Log:
reverted changes for #62477

Bugs:
https://bugs.php.net/62477

Changed paths:
  M  NEWS
  M  ext/spl/spl_iterators.c
  M  ext/spl/spl_iterators.h
  D  ext/spl/tests/bug62477_1.phpt
  D  ext/spl/tests/bug62477_2.phpt


Diff:
diff --git a/NEWS b/NEWS
index 2f60c43..902185c 100644
--- a/NEWS
+++ b/NEWS
@@ -204,7 +204,6 @@ PHP 
   NEWS
 having had its dtor callback called in between). (Gustavo)
   . Fixed bug #61347 (inconsistent isset behavior of Arrayobject). (Laruence)
   . Fixed bug #61326 (ArrayObject comparison). (Gustavo)
-  . Fixed bug #62477 LimitIterator int overflow. (Anatoliy)
 
 - SQLite3 extension:
   . Add createCollation() method. (Brad Dewar)
diff --git a/ext/spl/spl_iterators.c b/ext/spl/spl_iterators.c
index 1cbb2e4..eecd483 100755
--- a/ext/spl/spl_iterators.c
+++ b/ext/spl/spl_iterators.c
@@ -1380,31 +1380,12 @@ 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|zz, zobject, ce_inner, tmp_offset, tmp_count) == FAILURE) {
+   if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, 
O|ll, zobject, ce_inner, intern-u.limit.offset, intern-u.limit.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 9494b26..525a25c 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;
ulongint_key;
int  key_type; /* HASH_KEY_IS_STRING or 
HASH_KEY_IS_LONG */
-   long  pos;
+   int  pos;
} current;
dual_it_type dit_type;
union {
diff --git a/ext/spl/tests/bug62477_1.phpt b/ext/spl/tests/bug62477_1.phpt
deleted file mode 100644
index 1b768a7..000
--- a/ext/spl/tests/bug62477_1.phpt
+++ /dev/null
@@ -1,12 +0,0 @@
---TEST--
-Bug #62477 LimitIterator int overflow when float is passed (1)
---FILE--
-?php
-
-$it = new LimitIterator(new ArrayIterator(array(42)), 1000);
---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
deleted file mode 100644
index aa3468a..000
--- a/ext/spl/tests/bug62477_2.phpt
+++ /dev/null
@@ -1,12 +0,0 @@

[PHP-CVS] com php-src: reverted news about #62477: NEWS

2012-07-12 Thread Anatoliy Belsky 
Commit:ec7e479562506842766fcf2bc47f417e9da44813
Author:Anatoliy Belsky a...@php.net Thu, 12 Jul 2012 10:57:56 
+0200
Parents:   86428169d3c8d7d9a3dfd4e27fb48cf1314102dc
Branches:  PHP-5.4 master

Link:   
http://git.php.net/?p=php-src.git;a=commitdiff;h=ec7e479562506842766fcf2bc47f417e9da44813

Log:
reverted news about #62477

Bugs:
https://bugs.php.net/62477

Changed paths:
  M  NEWS


Diff:
diff --git a/NEWS b/NEWS
index 3281663..3d91e50 100644
--- a/NEWS
+++ b/NEWS
@@ -92,7 +92,6 @@ PHP   
 NEWS
 dot files). (Laruence)
   . Fixed bug #62262 (RecursiveArrayIterator does not implement Countable).
 (Nikita Popov)
-  . Fixed bug #62477 LimitIterator int overflow. (Anatoliy)
 
 - XML Writer:
   . Fixed bug #62064 (memory leak in the XML Writer module).


--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[PHP-CVS] com php-src: Merge branch 'PHP-5.3' into PHP-5.4: ext/spl/spl_iterators.c ext/spl/spl_iterators.h

2012-07-12 Thread Anatoliy Belsky 
Commit:86428169d3c8d7d9a3dfd4e27fb48cf1314102dc
Author:Anatoliy Belsky a...@php.net Thu, 12 Jul 2012 10:57:26 
+0200
Parents:   be8f089a13ef5e023ec27f14de8bc2453c75bb54 
ad7eeba3c1a4c77f439afc936fbf50b811a46a6b
Branches:  PHP-5.4 master

Link:   
http://git.php.net/?p=php-src.git;a=commitdiff;h=86428169d3c8d7d9a3dfd4e27fb48cf1314102dc

Log:
Merge branch 'PHP-5.3' into PHP-5.4

* PHP-5.3:
  reverted changes for #62477

Bugs:
https://bugs.php.net/62477

Changed paths:
  MM  ext/spl/spl_iterators.c
  MM  ext/spl/spl_iterators.h


Diff:



--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[PHP-CVS] com php-src: Merge branch 'PHP-5.4': ext/spl/spl_iterators.c

2012-07-12 Thread Anatoliy Belsky 
Commit:ed1f5b4dae81316ad53cee5c8cfa7e7f7746ad46
Author:Anatoliy Belsky a...@php.net Thu, 12 Jul 2012 11:02:33 
+0200
Parents:   7bd3d0bf8c130bb6478312a303fa309a31dcb3d1 
ec7e479562506842766fcf2bc47f417e9da44813
Branches:  master

Link:   
http://git.php.net/?p=php-src.git;a=commitdiff;h=ed1f5b4dae81316ad53cee5c8cfa7e7f7746ad46

Log:
Merge branch 'PHP-5.4'

* PHP-5.4:
  reverted news about #62477
  reverted changes for #62477

Conflicts:
NEWS

Bugs:
https://bugs.php.net/62477

Changed paths:
  MM  ext/spl/spl_iterators.c


Diff:



--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[PHP-CVS] com php-src: fixed the test for warnings changed: ext/fileinfo/tests/finfo_open_error-win32.phpt

2012-07-12 Thread Anatoliy Belsky 
Commit:cf91b163e1c16cfc0f6e4cd2b3ec78d82bc3ba1c
Author:Anatoliy Belsky a...@php.net Thu, 12 Jul 2012 18:24:38 
+0200
Parents:   ec7e479562506842766fcf2bc47f417e9da44813
Branches:  PHP-5.4 master

Link:   
http://git.php.net/?p=php-src.git;a=commitdiff;h=cf91b163e1c16cfc0f6e4cd2b3ec78d82bc3ba1c

Log:
fixed the test for warnings changed

Changed paths:
  M  ext/fileinfo/tests/finfo_open_error-win32.phpt


Diff:
diff --git a/ext/fileinfo/tests/finfo_open_error-win32.phpt 
b/ext/fileinfo/tests/finfo_open_error-win32.phpt
index e168b7f..0260ca5 100644
--- a/ext/fileinfo/tests/finfo_open_error-win32.phpt
+++ b/ext/fileinfo/tests/finfo_open_error-win32.phpt
@@ -43,13 +43,7 @@ bool(false)
 
 Warning: finfo_open() expects at most 2 parameters, 3 given in 
%sfinfo_open_error-win32.php on line %d
 bool(false)
-
-Warning: finfo_open(%smagic): failed to open stream: No such file or directory 
in %sfinfo_open_error-win32.php on line %d
-
-Warning: finfo_open(%smagic): failed to open stream: No such file or directory 
in %sfinfo_open_error-win32.php on line %d
-
-Warning: finfo_open(): Failed to load magic database at '%smagic'. in 
%sfinfo_open_error-win32.php on line %d
-bool(false)
+resource(6) of type (file_info)
 
 Warning: finfo_open() expects parameter 1 to be long, string given in 
%sfinfo_open_error-win32.php on line %d
 bool(false)


--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php