From b65755458538e1c5396144356bb99c97db2efee9 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 24 Feb 2016 18:23:42 +0100 Subject: [PATCH 1/4] Fix typo in foreach_017.phpt --- Zend/tests/foreach_017.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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); } From 079f2f7eb31659569cd2663e2c5b4c9f9eaa32d7 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 24 Feb 2016 18:27:56 +0100 Subject: [PATCH 2/4] Forbid exchangeArray() during sorting Previously this would leak. --- ext/spl/spl_array.c | 5 ++++ ...yObject_exchange_array_during_sorting.phpt | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 ext/spl/tests/ArrayObject_exchange_array_during_sorting.phpt diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 0740c063e96..2de985bd224 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -1267,6 +1267,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) + } +} From 0aa716381643c2746ddef019681e762d4e3f4233 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 24 Feb 2016 19:08:18 +0100 Subject: [PATCH 3/4] Fix AO object properties separation --- ext/spl/spl_array.c | 21 ++++++++++++------- ...bject_modify_shared_object_properties.phpt | 19 +++++++++++++++++ 2 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 ext/spl/tests/ArrayObject_modify_shared_object_properties.phpt diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 2de985bd224..070df298520 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; } } /* }}} */ @@ -1107,13 +1119,6 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar 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); - } ZVAL_COPY(&intern->array, array); } } 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) +} From fd561505f4142730684eacaba6f0ac5f293a8f0e Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 24 Feb 2016 22:31:17 +0100 Subject: [PATCH 4/4] Fix construction of AO with overloaded object error a) Fix uses of zend_string in error message b) Don't assign the overloaded object as the backing storage, that sort of defeats the point. Instead leave the previous value. --- ext/spl/spl_array.c | 14 +++++----- ...Object_overloaded_object_incompatible.phpt | 27 +++++++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 ext/spl/tests/ArrayObject_overloaded_object_incompatible.phpt diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 070df298520..685dd27092a 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -1094,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; @@ -1114,11 +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); + 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); } } 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) + } +}