diff --git a/Zend/tests/foreach_017.phpt b/Zend/tests/foreach_017.phpt index 55eeeb0891b..e27b04c934e 100644 --- a/Zend/tests/foreach_017.phpt +++ b/Zend/tests/foreach_017.phpt @@ -45,7 +45,7 @@ $done = 0; $a = [0,1,2,3,4]; foreach($a as &$v) { echo "$v\n"; - if ($done && $v == 3) { + if (!$done && $v == 3) { $done = 1; array_splice($a, 1, 2, $replacement); } diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 0740c063e96..685dd27092a 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -83,6 +83,7 @@ static inline spl_array_object *spl_array_from_obj(zend_object *obj) /* {{{ */ { #define Z_SPLARRAY_P(zv) spl_array_from_obj(Z_OBJ_P((zv))) static inline HashTable *spl_array_get_hash_table(spl_array_object* intern) { /* {{{ */ + //??? TODO: Delay duplication for arrays; only duplicate for write operations if (intern->ar_flags & SPL_ARRAY_IS_SELF) { if (!intern->std.properties) { rebuild_object_properties(&intern->std); @@ -91,8 +92,19 @@ static inline HashTable *spl_array_get_hash_table(spl_array_object* intern) { /* } else if (intern->ar_flags & SPL_ARRAY_USE_OTHER) { spl_array_object *other = Z_SPLARRAY_P(&intern->array); return spl_array_get_hash_table(other); + } else if (Z_TYPE(intern->array) == IS_ARRAY) { + return Z_ARRVAL(intern->array); } else { - return HASH_OF(&intern->array); + zend_object *obj = Z_OBJ(intern->array); + if (!obj->properties) { + rebuild_object_properties(obj); + } else if (GC_REFCOUNT(obj->properties) > 1) { + if (EXPECTED(!(GC_FLAGS(obj->properties) & IS_ARRAY_IMMUTABLE))) { + GC_REFCOUNT(obj->properties)--; + } + obj->properties = zend_array_dup(obj->properties); + } + return obj->properties; } } /* }}} */ @@ -1082,13 +1094,13 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar return; } - zval_ptr_dtor(&intern->array); - if (Z_TYPE_P(array) == IS_ARRAY) { //??? TODO: try to avoid array duplication + zval_ptr_dtor(&intern->array); ZVAL_DUP(&intern->array, array); } else { if (Z_OBJ_HT_P(array) == &spl_handler_ArrayObject || Z_OBJ_HT_P(array) == &spl_handler_ArrayIterator) { + zval_ptr_dtor(&intern->array); if (just_array) { spl_array_object *other = Z_SPLARRAY_P(array); ar_flags = other->ar_flags & ~SPL_ARRAY_INT_MASK; @@ -1102,18 +1114,13 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar } } else { zend_object_get_properties_t handler = Z_OBJ_HANDLER_P(array, get_properties); - if (handler != std_object_handlers.get_properties - || !spl_array_get_hash_table(intern)) { - ZVAL_UNDEF(&intern->array); - zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "Overloaded object of type %s is not compatible with %s", Z_OBJCE_P(array)->name, intern->std.ce->name); - } - //??? TODO: try to avoid array duplication - if (Z_OBJ_P(array)->properties && GC_REFCOUNT(Z_OBJ_P(array)->properties) > 1) { - if (EXPECTED(!(GC_FLAGS(Z_OBJ_P(array)->properties) & IS_ARRAY_IMMUTABLE))) { - GC_REFCOUNT(Z_OBJ_P(array)->properties)--; - } - Z_OBJ_P(array)->properties = zend_array_dup(Z_OBJ_P(array)->properties); + if (handler != std_object_handlers.get_properties) { + zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, + "Overloaded object of type %s is not compatible with %s", + ZSTR_VAL(Z_OBJCE_P(array)->name), ZSTR_VAL(intern->std.ce->name)); + return; } + zval_ptr_dtor(&intern->array); ZVAL_COPY(&intern->array, array); } } @@ -1267,6 +1274,11 @@ SPL_METHOD(Array, exchangeArray) return; } + if (intern->nApplyCount > 0) { + zend_error(E_WARNING, "Modification of ArrayObject during sorting is prohibited"); + return; + } + RETVAL_ARR(zend_array_dup(spl_array_get_hash_table(intern))); spl_array_set_array(object, intern, array, 0L, 1); } diff --git a/ext/spl/tests/ArrayObject_exchange_array_during_sorting.phpt b/ext/spl/tests/ArrayObject_exchange_array_during_sorting.phpt new file mode 100644 index 00000000000..225d42c1ed0 --- /dev/null +++ b/ext/spl/tests/ArrayObject_exchange_array_during_sorting.phpt @@ -0,0 +1,29 @@ +--TEST-- +Can't use exchangeArray() while ArrayObject is being sorted +--FILE-- +uasort(function($a, $b) use ($ao, &$i) { + if ($i++ == 0) { + $ao->exchangeArray([4, 5, 6]); + var_dump($ao); + } + return $a <=> $b; +}); + +?> +--EXPECTF-- +Warning: Modification of ArrayObject during sorting is prohibited in %s on line %d +object(ArrayObject)#1 (1) { + ["storage":"ArrayObject":private]=> + array(3) { + [0]=> + int(1) + [1]=> + int(2) + [2]=> + int(3) + } +} diff --git a/ext/spl/tests/ArrayObject_modify_shared_object_properties.phpt b/ext/spl/tests/ArrayObject_modify_shared_object_properties.phpt new file mode 100644 index 00000000000..24c247cabd9 --- /dev/null +++ b/ext/spl/tests/ArrayObject_modify_shared_object_properties.phpt @@ -0,0 +1,19 @@ +--TEST-- +Modifications to ArrayObjects should not affect shared properties tables +--FILE-- + 1, 'b' => 2]; +$ao = new ArrayObject($obj); +$arr = (array) $obj; +$ao['a'] = 42; +var_dump($arr); + +?> +--EXPECT-- +array(2) { + ["a"]=> + int(1) + ["b"]=> + int(2) +} diff --git a/ext/spl/tests/ArrayObject_overloaded_object_incompatible.phpt b/ext/spl/tests/ArrayObject_overloaded_object_incompatible.phpt new file mode 100644 index 00000000000..8c1121b8d01 --- /dev/null +++ b/ext/spl/tests/ArrayObject_overloaded_object_incompatible.phpt @@ -0,0 +1,27 @@ +--TEST-- +Objects with overloaded get_properties are incompatible with ArrayObject +--FILE-- +exchangeArray(new SplFixedArray); +} catch (Exception $e) { + echo $e->getMessage(), "\n"; +} +var_dump($ao); + +?> +--EXPECT-- +Overloaded object of type SplFixedArray is not compatible with ArrayObject +object(ArrayObject)#1 (1) { + ["storage":"ArrayObject":private]=> + array(3) { + [0]=> + int(1) + [1]=> + int(2) + [2]=> + int(3) + } +}