Override has_dimension handler for SplObjectStorage

Preserve existing behavior

Author: TysonAndre

ext/spl/spl_observer.c

@@ -399,20 +399,22 @@ static zend_object *spl_SplObjectStorage_new(zend_class_entry *class_type)
 }
 /* }}} */
 
+/* Returns 1 if the SplObjectStorage contains an entry for getHash(obj), even if the corresponding value is null. */
 int spl_object_storage_contains(spl_SplObjectStorage *intern, zend_object *obj) /* {{{ */
 {
+	if (EXPECTED(!intern->fptr_get_hash)) {
+		return zend_hash_index_find(&intern->storage, obj->handle) != NULL;
+	}
 	int found;
 	zend_hash_key key;
 	if (spl_object_storage_get_hash(&key, intern, obj) == FAILURE) {
 		return 0;
 	}
 
-	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);
+	found = zend_hash_exists(&intern->storage, key.key);
+	zend_string_release_ex(key.key, 0);
+
 	return found;
 } /* }}} */
 
@@ -432,6 +434,25 @@ 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);
@@ -1307,6 +1328,7 @@ PHP_MINIT_FUNCTION(spl_observer)
 	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_ce_MultipleIterator = register_class_MultipleIterator(zend_ce_iterator);
 	spl_ce_MultipleIterator->create_object = spl_SplObjectStorage_new;

ext/spl/tests/SplObjectStorage_coalesce.phpt

@@ -12,10 +12,22 @@ var_dump($s[$o1] ??= $i);
 var_dump($s[$o1] ??= null);
 var_dump($s[$o1] ??= false);
 var_dump($s[$o1] ?? $i);
-$s[$o2] = null;
+var_dump(isset($s[$o1]));
+var_dump(empty($s[$o1]));
+echo "o2\n";
 var_dump(isset($s[$o2]));
+var_dump(empty($s[$o2]));
+$s[$o2] = null;
 var_dump($s[$o2] ?? new stdClass());
+echo "check isset/empty/contains for null. offsetExists returns true as long as the entry is there.\n";
+var_dump(isset($s[$o2]));
+var_dump(empty($s[$o2]));
+var_dump($s->contains($o2));
+echo "check isset/empty/contains for false.\n";
+$s[$o2] = false;
 var_dump(isset($s[$o2]));
+var_dump(empty($s[$o2]));
+var_dump($s->contains($o2));
 try {
     $s['invalid'] = 123;
 } catch (Error $e) {
@@ -38,13 +50,24 @@ string(7) "dynamic"
 string(7) "dynamic"
 string(7) "dynamic"
 bool(true)
+bool(false)
+o2
+bool(false)
+bool(true)
 object(stdClass)#4 (0) {
 }
+check isset/empty/contains for null. offsetExists returns true as long as the entry is there.
+bool(true)
+bool(true)
+bool(true)
+check isset/empty/contains for false.
+bool(true)
+bool(true)
 bool(true)
 TypeError: SplObjectStorage::offsetSet(): Argument #1 ($object) must be of type object, string given
 TypeError: SplObjectStorage::offsetExists(): Argument #1 ($object) must be of type object, string given
 
-Notice: Indirect modification of overloaded element of SplObjectStorage has no effect in %s on line 26
+Notice: Indirect modification of overloaded element of SplObjectStorage has no effect in %s on line 38
 object(SplObjectStorage)#1 (1) {
   ["storage":"SplObjectStorage":private]=>
   array(2) {
@@ -62,7 +85,7 @@ object(SplObjectStorage)#1 (1) {
       object(stdClass)#3 (0) {
       }
       ["inf"]=>
-      NULL
+      bool(false)
     }
   }
 }