Refactor SplFixedArray (#7168)

* Move spl_offset_convert_to_long() to spl_fixedarray.c

It is only used there, which explains its weird offset semantics

* Refactor SplFixedArray offset handling
- Implement warning for resource type
- Throw a proper TypeError instead of a RuntimeException

* Use a proper Error to signal that [] cannot be used with SplFixedArray

* Refactor SplFixedArray has_dimension helper

* Drop some ZPP tests

ext/spl/config.m4

@@ -1,4 +1,4 @@
-PHP_NEW_EXTENSION(spl, php_spl.c spl_functions.c spl_engine.c spl_iterators.c spl_array.c spl_directory.c spl_exceptions.c spl_observer.c spl_dllist.c spl_heap.c spl_fixedarray.c, no,, -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1)
+PHP_NEW_EXTENSION(spl, php_spl.c spl_functions.c spl_iterators.c spl_array.c spl_directory.c spl_exceptions.c spl_observer.c spl_dllist.c spl_heap.c spl_fixedarray.c, no,, -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1)
 PHP_INSTALL_HEADERS([ext/spl], [php_spl.h spl_array.h spl_directory.h spl_engine.h spl_exceptions.h spl_functions.h spl_iterators.h spl_observer.h spl_dllist.h spl_heap.h spl_fixedarray.h])
 PHP_ADD_EXTENSION_DEP(spl, pcre, true)
 PHP_ADD_EXTENSION_DEP(spl, standard, true)

ext/spl/config.w32

@@ -1,5 +1,5 @@
 // vim:ft=javascript
 
-EXTENSION("spl", "php_spl.c spl_functions.c spl_engine.c spl_iterators.c spl_array.c spl_directory.c spl_exceptions.c spl_observer.c spl_dllist.c spl_heap.c spl_fixedarray.c", false /*never shared */, "/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1");
+EXTENSION("spl", "php_spl.c spl_functions.c spl_iterators.c spl_array.c spl_directory.c spl_exceptions.c spl_observer.c spl_dllist.c spl_heap.c spl_fixedarray.c", false /*never shared */, "/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1");
 PHP_SPL="yes";
 PHP_INSTALL_HEADERS("ext/spl", "php_spl.h spl_array.h spl_directory.h spl_engine.h spl_exceptions.h spl_functions.h spl_iterators.h spl_observer.h spl_dllist.h spl_heap.h spl_fixedarray.h");

ext/spl/spl_engine.c

@@ -1,59 +0,0 @@
-/*
-   +----------------------------------------------------------------------+
-   | Copyright (c) The PHP Group                                          |
-   +----------------------------------------------------------------------+
-   | This source file is subject to version 3.01 of the PHP license,      |
-   | that is bundled with this package in the file LICENSE, and is        |
-   | available through the world-wide-web at the following url:           |
-   | https://www.php.net/license/3_01.txt                                 |
-   | If you did not receive a copy of the PHP license and are unable to   |
-   | obtain it through the world-wide-web, please send a note to          |
-   | license@php.net so we can mail you a copy immediately.               |
-   +----------------------------------------------------------------------+
-   | Authors: Marcus Boerger <helly@php.net>                              |
-   +----------------------------------------------------------------------+
- */
-
-#ifdef HAVE_CONFIG_H
-# include "config.h"
-#endif
-
-#include "php.h"
-#include "php_ini.h"
-#include "ext/standard/info.h"
-#include "zend_interfaces.h"
-
-#include "php_spl.h"
-#include "spl_functions.h"
-#include "spl_engine.h"
-
-#include "spl_array.h"
-
-PHPAPI zend_long spl_offset_convert_to_long(zval *offset) /* {{{ */
-{
-	zend_ulong idx;
-
-try_again:
-	switch (Z_TYPE_P(offset)) {
-	case IS_STRING:
-		if (ZEND_HANDLE_NUMERIC(Z_STR_P(offset), idx)) {
-			return idx;
-		}
-		break;
-	case IS_DOUBLE:
-		return zend_dval_to_lval_safe(Z_DVAL_P(offset));
-	case IS_LONG:
-		return Z_LVAL_P(offset);
-	case IS_FALSE:
-		return 0;
-	case IS_TRUE:
-		return 1;
-	case IS_REFERENCE:
-		offset = Z_REFVAL_P(offset);
-		goto try_again;
-	case IS_RESOURCE:
-		return Z_RES_HANDLE_P(offset);
-	}
-	return -1;
-}
-/* }}} */

ext/spl/spl_engine.h

@@ -21,8 +21,6 @@
 #include "php_spl.h"
 #include "zend_interfaces.h"
 
-PHPAPI zend_long spl_offset_convert_to_long(zval *offset);
-
 static inline void spl_instantiate_arg_ex1(zend_class_entry *pce, zval *retval, zval *arg1)
 {
 	object_init_ex(retval, pce);

ext/spl/spl_fixedarray.c

@@ -309,6 +309,38 @@ static zend_object *spl_fixedarray_object_clone(zend_object *old_object)
 	return new_object;
 }
 
+static zend_long spl_offset_convert_to_long(zval *offset) /* {{{ */
+{
+	try_again:
+	switch (Z_TYPE_P(offset)) {
+		case IS_STRING: {
+			zend_ulong index;
+			if (ZEND_HANDLE_NUMERIC(Z_STR_P(offset), index)) {
+				return (zend_long) index;
+			}
+			break;
+		}
+		case IS_DOUBLE:
+			return zend_dval_to_lval_safe(Z_DVAL_P(offset));
+		case IS_LONG:
+			return Z_LVAL_P(offset);
+		case IS_FALSE:
+			return 0;
+		case IS_TRUE:
+			return 1;
+		case IS_REFERENCE:
+			offset = Z_REFVAL_P(offset);
+			goto try_again;
+		case IS_RESOURCE:
+			zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)",
+				Z_RES_HANDLE_P(offset), Z_RES_HANDLE_P(offset));
+			return Z_RES_HANDLE_P(offset);
+	}
+
+	zend_type_error("Illegal offset type");
+	return 0;
+}
+
 static zval *spl_fixedarray_object_read_dimension_helper(spl_fixedarray_object *intern, zval *offset)
 {
 	zend_long index;
@@ -316,17 +348,17 @@ static zval *spl_fixedarray_object_read_dimension_helper(spl_fixedarray_object *
 	/* we have to return NULL on error here to avoid memleak because of
 	 * ZE duplicating uninitialized_zval_ptr */
 	if (!offset) {
-		zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
+		zend_throw_error(NULL, "[] operator not supported for SplFixedArray");
 		return NULL;
 	}
 
-	if (Z_TYPE_P(offset) != IS_LONG) {
 	index = spl_offset_convert_to_long(offset);
-	} else {
-		index = Z_LVAL_P(offset);
+	if (EG(exception)) {
+		return NULL;
 	}
 
 	if (index < 0 || index >= intern->array.size) {
+		// TODO Change error message and use OutOfBound SPL Exception?
 		zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
 		return NULL;
 	} else {
@@ -368,17 +400,17 @@ static void spl_fixedarray_object_write_dimension_helper(spl_fixedarray_object *
 
 	if (!offset) {
 		/* '$array[] = value' syntax is not supported */
-		zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
+		zend_throw_error(NULL, "[] operator not supported for SplFixedArray");
 		return;
 	}
 
-	if (Z_TYPE_P(offset) != IS_LONG) {
 	index = spl_offset_convert_to_long(offset);
-	} else {
-		index = Z_LVAL_P(offset);
+	if (EG(exception)) {
+		return;
 	}
 
 	if (index < 0 || index >= intern->array.size) {
+		// TODO Change error message and use OutOfBound SPL Exception?
 		zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
 		return;
 	} else {
@@ -410,13 +442,13 @@ static void spl_fixedarray_object_unset_dimension_helper(spl_fixedarray_object *
 {
 	zend_long index;
 
-	if (Z_TYPE_P(offset) != IS_LONG) {
 	index = spl_offset_convert_to_long(offset);
-	} else {
-		index = Z_LVAL_P(offset);
+	if (EG(exception)) {
+		return;
 	}
 
 	if (index < 0 || index >= intern->array.size) {
+		// TODO Change error message and use OutOfBound SPL Exception?
 		zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
 		return;
 	} else {
@@ -439,28 +471,24 @@ static void spl_fixedarray_object_unset_dimension(zend_object *object, zval *off
 	spl_fixedarray_object_unset_dimension_helper(intern, offset);
 }
 
-static int spl_fixedarray_object_has_dimension_helper(spl_fixedarray_object *intern, zval *offset, int check_empty)
+static bool spl_fixedarray_object_has_dimension_helper(spl_fixedarray_object *intern, zval *offset, bool check_empty)
 {
 	zend_long index;
-	int retval;
 
-	if (Z_TYPE_P(offset) != IS_LONG) {
 	index = spl_offset_convert_to_long(offset);
-	} else {
-		index = Z_LVAL_P(offset);
+	if (EG(exception)) {
+		return false;
 	}
 
 	if (index < 0 || index >= intern->array.size) {
-		retval = 0;
-	} else {
-		if (check_empty) {
-			retval = zend_is_true(&intern->array.elements[index]);
-		} else {
-			retval = Z_TYPE(intern->array.elements[index]) != IS_NULL;
-		}
+		return false;
 	}
 
-	return retval;
+	if (check_empty) {
+		return zend_is_true(&intern->array.elements[index]);
+	}
+
+	return Z_TYPE(intern->array.elements[index]) != IS_NULL;
 }
 
 static int spl_fixedarray_object_has_dimension(zend_object *object, zval *offset, int check_empty)

ext/spl/tests/fixedarray_001.phpt

@@ -7,17 +7,17 @@ $a = new SplFixedArray(0);
 try {
     $a[0] = "value1";
 } catch (RuntimeException $e) {
-    echo "Exception: ".$e->getMessage()."\n";
+    echo $e::class, ': ', $e->getMessage(), "\n";
 }
 try {
     var_dump($a["asdf"]);
-} catch (RuntimeException $e) {
-    echo "Exception: ".$e->getMessage()."\n";
+} catch (\TypeError $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
 }
 try {
     unset($a[-1]);
 } catch (RuntimeException $e) {
-    echo "Exception: ".$e->getMessage()."\n";
+    echo $e::class, ': ', $e->getMessage(), "\n";
 }
 $a->setSize(10);
 
@@ -45,9 +45,9 @@ $a[0] = "valueNew";
 var_dump($b[0]);
 ?>
 --EXPECT--
-Exception: Index invalid or out of range
-Exception: Index invalid or out of range
-Exception: Index invalid or out of range
+RuntimeException: Index invalid or out of range
+TypeError: Illegal offset type
+RuntimeException: Index invalid or out of range
 string(6) "value0"
 string(6) "value2"
 string(6) "value3"

ext/spl/tests/fixedarray_002.phpt

@@ -34,17 +34,17 @@ $a = new A;
 try {
     $a[0] = "value1";
 } catch (RuntimeException $e) {
-    echo "Exception: ".$e->getMessage()."\n";
+    echo $e::class, ': ', $e->getMessage(), "\n";
 }
 try {
     var_dump($a["asdf"]);
-} catch (RuntimeException $e) {
-    echo "Exception: ".$e->getMessage()."\n";
+} catch (\TypeError $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
 }
 try {
     unset($a[-1]);
 } catch (RuntimeException $e) {
-    echo "Exception: ".$e->getMessage()."\n";
+    echo $e::class, ': ', $e->getMessage(), "\n";
 }
 $a->setSize(10);
 
@@ -69,11 +69,11 @@ var_dump(count($a), $a->getSize(), count($a) == $a->getSize());
 ?>
 --EXPECT--
 A::offsetSet
-Exception: Index invalid or out of range
+RuntimeException: Index invalid or out of range
 A::offsetGet
-Exception: Index invalid or out of range
+TypeError: Illegal offset type
 A::offsetUnset
-Exception: Index invalid or out of range
+RuntimeException: Index invalid or out of range
 A::offsetSet
 A::offsetSet
 A::offsetSet

ext/spl/tests/fixedarray_003.phpt

@@ -0,0 +1,222 @@
+--TEST--
+SPL: FixedArray: Non integer offset handling
+--FILE--
+<?php
+$o = new SplFixedArray(10);
+$r = fopen('php://memory', 'r+');
+
+
+echo 'Write context', \PHP_EOL;
+$o[false] = 'a';
+$o[true] = 'b';
+$o[2.5] = 'c';
+
+try {
+    $o[[]] = 'd';
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    $o[new stdClass()] = 'e';
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    $o[$r] = 'f';
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+$o['3'] = 'g';
+
+try {
+    $o['3.5'] = 'h';
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    $o['03'] = 'i';
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    $o[' 3'] = 'j';
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+
+echo 'Read context', \PHP_EOL;
+var_dump($o[false]);
+var_dump($o[true]);
+var_dump($o[2.5]);
+
+try {
+    var_dump($o[[]]);
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump($o[new stdClass()]);
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump($o[$r]);
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+var_dump($o['3']);
+
+try {
+    var_dump($o['3.5']);
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump($o['03']);
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump($o[' 3']);
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+
+echo 'isset()', \PHP_EOL;
+var_dump(isset($o[false]));
+var_dump(isset($o[true]));
+var_dump(isset($o[2.5]));
+
+try {
+    var_dump(isset($o[[]]));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump(isset($o[new stdClass()]));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump(isset($o[$r]));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+var_dump(isset($o['3']));
+
+try {
+    var_dump(isset($o['3.5']));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump(isset($o['03']));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump(isset($o[' 3']));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+echo 'empty()', \PHP_EOL;
+var_dump(empty($o[false]));
+var_dump(empty($o[true]));
+var_dump(empty($o[2.5]));
+
+try {
+    var_dump(empty($o[[]]));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump(empty($o[new stdClass()]));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump(empty($o[$r]));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+var_dump(empty($o['3']));
+
+try {
+    var_dump(empty($o['3.5']));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump(empty($o['03']));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump(empty($o[' 3']));
+} catch (\TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+?>
+--EXPECTF--
+Write context
+
+Deprecated: Implicit conversion from float 2.5 to int loses precision in %s on line %d
+Illegal offset type
+Illegal offset type
+
+Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
+Illegal offset type
+Illegal offset type
+Illegal offset type
+Read context
+string(1) "a"
+string(1) "b"
+
+Deprecated: Implicit conversion from float 2.5 to int loses precision in %s on line %d
+string(1) "c"
+Illegal offset type
+Illegal offset type
+
+Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
+string(1) "f"
+string(1) "g"
+Illegal offset type
+Illegal offset type
+Illegal offset type
+isset()
+bool(true)
+bool(true)
+
+Deprecated: Implicit conversion from float 2.5 to int loses precision in %s on line %d
+bool(true)
+Illegal offset type
+Illegal offset type
+
+Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
+bool(true)
+bool(true)
+Illegal offset type
+Illegal offset type
+Illegal offset type
+empty()
+bool(false)
+bool(false)
+
+Deprecated: Implicit conversion from float 2.5 to int loses precision in %s on line %d
+bool(false)
+Illegal offset type
+Illegal offset type
+
+Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
+bool(false)
+bool(false)
+Illegal offset type
+Illegal offset type
+Illegal offset type

ext/spl/tests/fixedarray_004.phpt

@@ -7,10 +7,10 @@ $a = new SplFixedArray(10);
 
 try {
     $a[] = 1;
-} catch (Exception $e) {
-    var_dump($e->getMessage());
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
 }
 
 ?>
 --EXPECT--
-string(29) "Index invalid or out of range"
+[] operator not supported for SplFixedArray

ext/spl/tests/fixedarray_005.phpt

@@ -1,28 +0,0 @@
---TEST--
-SPL: FixedArray: Invalid arguments
---FILE--
-<?php
-
-try {
-    $a = new SplFixedArray(new stdClass);
-} catch (TypeError $iae) {
-    echo "Ok - ".$iae->getMessage().PHP_EOL;
-}
-
-try {
-    $a = new SplFixedArray('FOO');
-} catch (TypeError $iae) {
-    echo "Ok - ".$iae->getMessage().PHP_EOL;
-}
-
-try {
-    $a = new SplFixedArray('');
-} catch (TypeError $iae) {
-    echo "Ok - ".$iae->getMessage().PHP_EOL;
-}
-
-?>
---EXPECT--
-Ok - SplFixedArray::__construct(): Argument #1 ($size) must be of type int, stdClass given
-Ok - SplFixedArray::__construct(): Argument #1 ($size) must be of type int, string given
-Ok - SplFixedArray::__construct(): Argument #1 ($size) must be of type int, string given

ext/spl/tests/fixedarray_006.phpt

@@ -10,7 +10,7 @@ try {
     for ($i = 0; $i < 100; $i++) {
         $a[] = new stdClass;
     }
-} catch (Exception $e) {
+} catch (Error $e) {
     echo $e->getMessage(), "\n";
 }
 
@@ -18,5 +18,5 @@ print "ok\n";
 
 ?>
 --EXPECT--
-Index invalid or out of range
+[] operator not supported for SplFixedArray
 ok

ext/spl/tests/fixedarray_009.phpt

@@ -1,13 +0,0 @@
---TEST--
-SPL: FixedArray: Trying to instantiate passing string to constructor parameter
---FILE--
-<?php
-
-try {
-    $a = new SplFixedArray('FOO');
-} catch (TypeError $iae) {
-    echo "Ok - ".$iae->getMessage().PHP_EOL;
-}
-?>
---EXPECT--
-Ok - SplFixedArray::__construct(): Argument #1 ($size) must be of type int, string given

ext/spl/tests/fixedarray_012.phpt

@@ -7,7 +7,7 @@ $a = new SplFixedArray(100);
 
 try {
     $b = &$a[];
-} catch (Exception $e) {
+} catch (Error $e) {
     echo $e->getMessage(), "\n";
 }
 
@@ -15,5 +15,5 @@ print "ok\n";
 
 ?>
 --EXPECT--
-Index invalid or out of range
+[] operator not supported for SplFixedArray
 ok

ext/spl/tests/fixedarray_013.phpt

@@ -12,10 +12,10 @@ function test(SplFixedArray &$arr) {
 
 try {
     test($a[]);
-} catch (Exception $e) {
+} catch (\Error $e) {
     echo $e->getMessage(), "\n";
 }
 
 ?>
 --EXPECT--
-Index invalid or out of range
+[] operator not supported for SplFixedArray

ext/spl/tests/fixedarray_015.phpt

@@ -1,16 +0,0 @@
---TEST--
-SPL: FixedArray: accessing uninitialized array
---FILE--
-<?php
-
-try {
-    $a = new SplFixedArray('');
-} catch (TypeError $iae) {
-    echo "Ok - ".$iae->getMessage().PHP_EOL;
-}
-
-echo "Done\n";
-?>
---EXPECT--
-Ok - SplFixedArray::__construct(): Argument #1 ($size) must be of type int, string given
-Done