[pull] master from php:master

Zend/tests/str_offset_005.phpt

@@ -0,0 +1,12 @@
+--TEST--
+string offset 005 indirect string modification by error handler
+--FILE--
+<?php
+set_error_handler(function(){$GLOBALS['a']=8;});
+$a='a';
+var_dump($a[$b]);
+var_dump($a);
+?>
+--EXPECT--
+string(1) "a"
+int(8)

Zend/zend_execute.c

@@ -2410,6 +2410,7 @@ static zend_always_inline void zend_fetch_dimension_address_read(zval *result, z
 		}
 	}
 	if (!is_list && EXPECTED(Z_TYPE_P(container) == IS_STRING)) {
+		zend_string *str = Z_STR_P(container);
 		zend_long offset;
 
 try_string_offset:
@@ -2435,14 +2436,34 @@ static zend_always_inline void zend_fetch_dimension_address_read(zval *result, z
 					return;
 				}
 				case IS_UNDEF:
+					/* The string may be destroyed while throwing the notice.
+					 * Temporarily increase the refcount to detect this situation. */
+					if (!(GC_FLAGS(str) & IS_ARRAY_IMMUTABLE)) {
+						GC_ADDREF(str);
+					}
 					ZVAL_UNDEFINED_OP2();
+					if (!(GC_FLAGS(str) & IS_ARRAY_IMMUTABLE) && GC_DELREF(str) == 0) {
+						zend_string_release_ex(str, 0);
+						ZVAL_NULL(result);
+						return;
+					}
 					ZEND_FALLTHROUGH;
 				case IS_DOUBLE:
 				case IS_NULL:
 				case IS_FALSE:
 				case IS_TRUE:
 					if (type != BP_VAR_IS) {
+						/* The string may be destroyed while throwing the notice.
+						 * Temporarily increase the refcount to detect this situation. */
+						if (!(GC_FLAGS(str) & IS_ARRAY_IMMUTABLE)) {
+							GC_ADDREF(str);
+						}
 						zend_error(E_WARNING, "String offset cast occurred");
+						if (!(GC_FLAGS(str) & IS_ARRAY_IMMUTABLE) && GC_DELREF(str) == 0) {
+							zend_string_release_ex(str, 0);
+							ZVAL_NULL(result);
+							return;
+						}
 					}
 					break;
 				case IS_REFERENCE:
@@ -2460,7 +2481,7 @@ static zend_always_inline void zend_fetch_dimension_address_read(zval *result, z
 		}
 		out:
 
-		if (UNEXPECTED(Z_STRLEN_P(container) < ((offset < 0) ? -(size_t)offset : ((size_t)offset + 1)))) {
+		if (UNEXPECTED(ZSTR_LEN(str) < ((offset < 0) ? -(size_t)offset : ((size_t)offset + 1)))) {
 			if (type != BP_VAR_IS) {
 				zend_error(E_WARNING, "Uninitialized string offset " ZEND_LONG_FMT, offset);
 				ZVAL_EMPTY_STRING(result);
@@ -2472,8 +2493,8 @@ static zend_always_inline void zend_fetch_dimension_address_read(zval *result, z
 			zend_long real_offset;
 
 			real_offset = (UNEXPECTED(offset < 0)) /* Handle negative offset */
-				? (zend_long)Z_STRLEN_P(container) + offset : offset;
-			c = (zend_uchar)Z_STRVAL_P(container)[real_offset];
+				? (zend_long)ZSTR_LEN(str) + offset : offset;
+			c = (zend_uchar)ZSTR_VAL(str)[real_offset];
 
 			ZVAL_CHAR(result, c);
 		}

ext/spl/spl_observer.c

@@ -44,10 +44,17 @@ PHPAPI zend_class_entry     *spl_ce_MultipleIterator;
 
 PHPAPI zend_object_handlers spl_handler_SplObjectStorage;
 
+/* Bit flags for marking internal functionality overridden by SplObjectStorage subclasses. */
+#define SOS_OVERRIDDEN_READ_DIMENSION  1
+#define SOS_OVERRIDDEN_WRITE_DIMENSION 2
+#define SOS_OVERRIDDEN_UNSET_DIMENSION 4
+
 typedef struct _spl_SplObjectStorage { /* {{{ */
 	HashTable         storage;
 	zend_long         index;
 	HashPosition      pos;
+	/* In SplObjectStorage, flags is a hidden implementation detail to optimize ArrayAccess handlers.
+	 * In MultipleIterator on a different class hierarchy, flags is a user settable value controlling iteration behavior. */
 	zend_long         flags;
 	zend_function    *fptr_get_hash;
 	zend_object       std;
@@ -76,7 +83,7 @@ void spl_SplObjectStorage_free_storage(zend_object *object) /* {{{ */
 } /* }}} */
 
 static int spl_object_storage_get_hash(zend_hash_key *key, spl_SplObjectStorage *intern, zend_object *obj) {
-	if (intern->fptr_get_hash) {
+	if (UNEXPECTED(intern->fptr_get_hash)) {
 		zval param;
 		zval rv;
 		ZVAL_OBJ(&param, obj);
@@ -125,8 +132,54 @@ static spl_SplObjectStorageElement* spl_object_storage_get(spl_SplObjectStorage
 	}
 } /* }}} */
 
+static spl_SplObjectStorageElement *spl_object_storage_create_element(zend_object *obj, zval *inf) /* {{{ */
+{
+	spl_SplObjectStorageElement *pelement = emalloc(sizeof(spl_SplObjectStorageElement));
+	pelement->obj = obj;
+	GC_ADDREF(obj);
+	if (inf) {
+		ZVAL_COPY(&pelement->inf, inf);
+	} else {
+		ZVAL_NULL(&pelement->inf);
+	}
+	return pelement;
+} /* }}} */
+
+/* A faster version of spl_object_storage_attach used when neither SplObjectStorage->getHash nor SplObjectStorage->offsetSet is overridden. */
+static spl_SplObjectStorageElement *spl_object_storage_attach_handle(spl_SplObjectStorage *intern, zend_object *obj, zval *inf) /* {{{ */
+{
+	uint32_t handle = obj->handle;
+	zval *entry_zv = zend_hash_index_lookup(&intern->storage, handle);
+	spl_SplObjectStorageElement *pelement;
+	ZEND_ASSERT(!(intern->flags & SOS_OVERRIDDEN_WRITE_DIMENSION));
+
+	if (Z_TYPE_P(entry_zv) != IS_NULL) {
+		zval zv_inf;
+		ZEND_ASSERT(Z_TYPE_P(entry_zv) == IS_PTR);
+		pelement = Z_PTR_P(entry_zv);
+		ZVAL_COPY_VALUE(&zv_inf, &pelement->inf);
+		if (inf) {
+			ZVAL_COPY(&pelement->inf, inf);
+		} else {
+			ZVAL_NULL(&pelement->inf);
+		}
+		/* Call the old value's destructor last, in case it moves the entry */
+		zval_ptr_dtor(&zv_inf);
+		return pelement;
+	}
+
+	pelement = spl_object_storage_create_element(obj, inf);
+	ZVAL_PTR(entry_zv, pelement);
+	return pelement;
+} /* }}} */
+
 spl_SplObjectStorageElement *spl_object_storage_attach(spl_SplObjectStorage *intern, zend_object *obj, zval *inf) /* {{{ */
 {
+	if (EXPECTED(!(intern->flags & SOS_OVERRIDDEN_WRITE_DIMENSION))) {
+		return spl_object_storage_attach_handle(intern, obj, inf);
+	}
+	/* getHash or offsetSet is overridden. */
+
 	spl_SplObjectStorageElement *pelement, element;
 	zend_hash_key key;
 	if (spl_object_storage_get_hash(&key, intern, obj) == FAILURE) {
@@ -136,13 +189,16 @@ spl_SplObjectStorageElement *spl_object_storage_attach(spl_SplObjectStorage *int
 	pelement = spl_object_storage_get(intern, &key);
 
 	if (pelement) {
-		zval_ptr_dtor(&pelement->inf);
+		zval zv_inf;
+		ZVAL_COPY_VALUE(&zv_inf, &pelement->inf);
 		if (inf) {
 			ZVAL_COPY(&pelement->inf, inf);
 		} else {
 			ZVAL_NULL(&pelement->inf);
 		}
 		spl_object_storage_free_hash(intern, &key);
+		/* Call the old value's destructor last, in case it moves the entry */
+		zval_ptr_dtor(&zv_inf);
 		return pelement;
 	}
 
@@ -164,6 +220,9 @@ spl_SplObjectStorageElement *spl_object_storage_attach(spl_SplObjectStorage *int
 
 static int spl_object_storage_detach(spl_SplObjectStorage *intern, zend_object *obj) /* {{{ */
 {
+	if (EXPECTED(!(intern->flags & SOS_OVERRIDDEN_UNSET_DIMENSION))) {
+		return zend_hash_index_del(&intern->storage, obj->handle);
+	}
 	int ret = FAILURE;
 	zend_hash_key key;
 	if (spl_object_storage_get_hash(&key, intern, obj) == FAILURE) {
@@ -189,6 +248,9 @@ void spl_object_storage_addall(spl_SplObjectStorage *intern, spl_SplObjectStorag
 	intern->index = 0;
 } /* }}} */
 
+#define SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, zstr_method) \
+	(((zend_function *)zend_hash_find_ptr(&(class_type)->function_table, ZSTR_KNOWN(zstr_method)))->common.scope != spl_ce_SplObjectStorage)
+
 static zend_object *spl_object_storage_new_ex(zend_class_entry *class_type, zend_object *orig) /* {{{ */
 {
 	spl_SplObjectStorage *intern;
@@ -207,10 +269,27 @@ static zend_object *spl_object_storage_new_ex(zend_class_entry *class_type, zend
 
 	while (parent) {
 		if (parent == spl_ce_SplObjectStorage) {
+			/* Possible optimization: Cache these results with a map from class entry to IS_NULL/IS_PTR.
+			 * Or maybe just a single item with the result for the most recently loaded subclass. */
 			if (class_type != spl_ce_SplObjectStorage) {
-				intern->fptr_get_hash = zend_hash_str_find_ptr(&class_type->function_table, "gethash", sizeof("gethash") - 1);
-				if (intern->fptr_get_hash->common.scope == spl_ce_SplObjectStorage) {
-					intern->fptr_get_hash = NULL;
+				zend_function *get_hash = zend_hash_str_find_ptr(&class_type->function_table, "gethash", sizeof("gethash") - 1);
+				if (get_hash->common.scope != spl_ce_SplObjectStorage) {
+					intern->fptr_get_hash = get_hash;
+				}
+				if (intern->fptr_get_hash != NULL ||
+					SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, ZEND_STR_OFFSETGET) ||
+					SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, ZEND_STR_OFFSETEXISTS)) {
+					intern->flags |= SOS_OVERRIDDEN_READ_DIMENSION;
+				}
+
+				if (intern->fptr_get_hash != NULL ||
+					SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, ZEND_STR_OFFSETSET)) {
+					intern->flags |= SOS_OVERRIDDEN_WRITE_DIMENSION;
+				}
+
+				if (intern->fptr_get_hash != NULL ||
+					SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, ZEND_STR_OFFSETUNSET)) {
+					intern->flags |= SOS_OVERRIDDEN_UNSET_DIMENSION;
 				}
 			}
 			break;
@@ -328,20 +407,21 @@ static zend_object *spl_SplObjectStorage_new(zend_class_entry *class_type)
 }
 /* }}} */
 
-int spl_object_storage_contains(spl_SplObjectStorage *intern, zend_object *obj) /* {{{ */
+/* Returns true if the SplObjectStorage contains an entry for getHash(obj), even if the corresponding value is null. */
+bool spl_object_storage_contains(spl_SplObjectStorage *intern, zend_object *obj) /* {{{ */
 {
-	int found;
+	if (EXPECTED(!intern->fptr_get_hash)) {
+		return zend_hash_index_find(&intern->storage, obj->handle) != NULL;
+	}
 	zend_hash_key key;
 	if (spl_object_storage_get_hash(&key, intern, obj) == FAILURE) {
-		return 0;
+		return true;
 	}
 
-	if (key.key) {
-		found = zend_hash_exists(&intern->storage, key.key);
-	} else {
-		found = zend_hash_index_exists(&intern->storage, key.h);
-	}
-	spl_object_storage_free_hash(intern, &key);
+	ZEND_ASSERT(key.key);
+	bool found = zend_hash_exists(&intern->storage, key.key);
+	zend_string_release_ex(key.key, 0);
+
 	return found;
 } /* }}} */
 
@@ -361,6 +441,68 @@ PHP_METHOD(SplObjectStorage, attach)
 	spl_object_storage_attach(intern, obj, inf);
 } /* }}} */
 
+static int spl_object_storage_has_dimension(zend_object *object, zval *offset, int check_empty)
+{
+	spl_SplObjectStorage *intern = spl_object_storage_from_obj(object);
+	if (UNEXPECTED(offset == NULL || Z_TYPE_P(offset) != IS_OBJECT || (intern->flags & SOS_OVERRIDDEN_READ_DIMENSION))) {
+		/* Can't optimize empty()/isset() check if getHash, offsetExists, or offsetGet is overridden */
+		return zend_std_has_dimension(object, offset, check_empty);
+	}
+	spl_SplObjectStorageElement *element = zend_hash_index_find_ptr(&intern->storage, Z_OBJ_HANDLE_P(offset));
+	if (!element) {
+		return 0;
+	}
+
+	if (check_empty) {
+		return i_zend_is_true(&element->inf);
+	}
+	/* NOTE: SplObjectStorage->offsetExists() is an alias of SplObjectStorage->contains(), so this returns true even if the value is null. */
+	return 1;
+}
+
+static zval *spl_object_storage_read_dimension(zend_object *object, zval *offset, int type, zval *rv)
+{
+	spl_SplObjectStorage *intern = spl_object_storage_from_obj(object);
+	if (UNEXPECTED(offset == NULL || Z_TYPE_P(offset) != IS_OBJECT || (intern->flags & SOS_OVERRIDDEN_READ_DIMENSION))) {
+		/* Can't optimize it if getHash, offsetExists, or offsetGet is overridden */
+		return zend_std_read_dimension(object, offset, type, rv);
+	}
+	spl_SplObjectStorageElement *element = zend_hash_index_find_ptr(&intern->storage, Z_OBJ_HANDLE_P(offset));
+
+	if (!element) {
+		if (type == BP_VAR_IS) {
+			return &EG(uninitialized_zval);
+		}
+		zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0, "Object not found");
+		return NULL;
+	} else {
+		/* This deliberately returns a non-reference, even for BP_VAR_W and BP_VAR_RW, to behave the same way as SplObjectStorage did when using the default zend_std_read_dimension behavior.
+		 * i.e. This prevents taking a reference to an entry of SplObjectStorage because offsetGet would return a non-reference. */
+		ZVAL_COPY_DEREF(rv, &element->inf);
+		return rv;
+	}
+}
+
+static void spl_object_storage_write_dimension(zend_object *object, zval *offset, zval *inf)
+{
+	spl_SplObjectStorage *intern = spl_object_storage_from_obj(object);
+	if (UNEXPECTED(offset == NULL || Z_TYPE_P(offset) != IS_OBJECT || (intern->flags & SOS_OVERRIDDEN_WRITE_DIMENSION))) {
+		zend_std_write_dimension(object, offset, inf);
+		return;
+	}
+	spl_object_storage_attach_handle(intern, Z_OBJ_P(offset), inf);
+}
+
+static void spl_object_storage_unset_dimension(zend_object *object, zval *offset)
+{
+	spl_SplObjectStorage *intern = spl_object_storage_from_obj(object);
+	if (UNEXPECTED(Z_TYPE_P(offset) != IS_OBJECT || (intern->flags & SOS_OVERRIDDEN_UNSET_DIMENSION))) {
+		zend_std_unset_dimension(object, offset);
+		return;
+	}
+	zend_hash_index_del(&intern->storage, Z_OBJ_HANDLE_P(offset));
+}
+
 /* {{{ Detaches an object from the storage */
 PHP_METHOD(SplObjectStorage, detach)
 {
@@ -851,6 +993,7 @@ PHP_METHOD(SplObjectStorage, __unserialize)
 				RETURN_THROWS();
 			}
 
+			ZVAL_DEREF(val);
 			spl_object_storage_attach(intern, Z_OBJ_P(key), val);
 			key = NULL;
 		} else {
@@ -1201,6 +1344,10 @@ PHP_MINIT_FUNCTION(spl_observer)
 	spl_handler_SplObjectStorage.clone_obj       = spl_object_storage_clone;
 	spl_handler_SplObjectStorage.get_gc          = spl_object_storage_get_gc;
 	spl_handler_SplObjectStorage.free_obj        = spl_SplObjectStorage_free_storage;
+	spl_handler_SplObjectStorage.read_dimension  = spl_object_storage_read_dimension;
+	spl_handler_SplObjectStorage.write_dimension = spl_object_storage_write_dimension;
+	spl_handler_SplObjectStorage.has_dimension   = spl_object_storage_has_dimension;
+	spl_handler_SplObjectStorage.unset_dimension = spl_object_storage_unset_dimension;
 
 	spl_ce_MultipleIterator = register_class_MultipleIterator(zend_ce_iterator);
 	spl_ce_MultipleIterator->create_object = spl_SplObjectStorage_new;