Refactor SplFixedArray offset handling

- Implement warning for resource type
- Relax restictions on a string offset to better match the normal conversion to int semantics as the offset is always an integer (similar to string offsets)
- NULL is still not support as an offset
- Throw a proper TypeError instead of a RuntimeException

Author: Girgias

ext/spl/spl_fixedarray.c

@@ -311,13 +311,13 @@ static zend_object *spl_fixedarray_object_clone(zend_object *old_object)
 
 static zend_long spl_offset_convert_to_long(zval *offset) /* {{{ */
 {
-	zend_ulong idx;
+	zend_long index;
 
 	try_again:
 	switch (Z_TYPE_P(offset)) {
 		case IS_STRING:
-			if (ZEND_HANDLE_NUMERIC(Z_STR_P(offset), idx)) {
-				return idx;
+			if (IS_LONG == is_numeric_str_function(Z_STR_P(offset), &index, NULL)) {
+				return index;
 			}
 			break;
 		case IS_DOUBLE:
@@ -332,9 +332,13 @@ static zend_long spl_offset_convert_to_long(zval *offset) /* {{{ */
 			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);
-		}
-	return -1;
+	}
+
+	zend_type_error("Illegal offset type");
+	return 0;
 }
 
 static zval *spl_fixedarray_object_read_dimension_helper(spl_fixedarray_object *intern, zval *offset)
@@ -348,13 +352,13 @@ static zval *spl_fixedarray_object_read_dimension_helper(spl_fixedarray_object *
 		return NULL;
 	}
 
-	if (Z_TYPE_P(offset) != IS_LONG) {
-		index = spl_offset_convert_to_long(offset);
-	} else {
-		index = Z_LVAL_P(offset);
+	index = spl_offset_convert_to_long(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 {
@@ -400,13 +404,13 @@ static void spl_fixedarray_object_write_dimension_helper(spl_fixedarray_object *
 		return;
 	}
 
-	if (Z_TYPE_P(offset) != IS_LONG) {
-		index = spl_offset_convert_to_long(offset);
-	} else {
-		index = Z_LVAL_P(offset);
+	index = spl_offset_convert_to_long(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 {
@@ -438,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);
+	index = spl_offset_convert_to_long(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 {
@@ -472,10 +476,9 @@ static int spl_fixedarray_object_has_dimension_helper(spl_fixedarray_object *int
 	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);
+	index = spl_offset_convert_to_long(offset);
+	if (EG(exception)) {
+		return 0;
 	}
 
 	if (index < 0 || index >= intern->array.size) {

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