Cloning of com objects is seriously broken

[cmb69]

Description

The following code:

<?php
$v = new variant("123");
$v2 = clone $v;

Resulted in this output:

Assertion failed: (!(((uintptr_t)((executor_globals.objects_store).object_buckets[handle])) & (1<<0))), file Zend\zend_objects_API.c, line 189

But I expected no output instead.

---

php-src/ext/com_dotnet/php_com_dotnet_internal.h

Lines 28 to 49 in b14469b

	typedef struct _php_com_dotnet_object {
	zend_object zo;
	VARIANT v;
	bool modified;
	int code_page;
	ITypeInfo *typeinfo;
	zend_class_entry *ce;
	/* associated event sink */
	IDispatch *sink_dispatch;
	GUID sink_id;
	DWORD sink_cookie;
	/* cache for method signatures */
	HashTable *method_cache;
	/* cache for name -> DISPID */
	HashTable *id_of_name_cache;
	} php_com_dotnet_object;

php-src/ext/com_dotnet/com_handlers.c

Lines 594 to 616 in b14469b

	zend_object* php_com_object_clone(zend_object *object)
	{
	php_com_dotnet_object *cloneobj, *origobject;
	origobject = (php_com_dotnet_object*) object;
	cloneobj = (php_com_dotnet_object*)emalloc(sizeof(php_com_dotnet_object));
	memcpy(cloneobj, origobject, sizeof(*cloneobj));
	/* VariantCopy will perform VariantClear; we don't want to clobber
	* the IDispatch that we memcpy'd, so we init a new variant in the
	* clone structure */
	VariantInit(&cloneobj->v);
	/* We use the Indirection-following version of the API since we
	* want to clone as much as possible */
	VariantCopyInd(&cloneobj->v, &origobject->v);
	if (cloneobj->typeinfo) {
	ITypeInfo_AddRef(cloneobj->typeinfo);
	}
	return (zend_object*)cloneobj;
	}

That memcpy() is a "bit" crude, and while we cater to v and typeinfo, we apparently missed sink_dispatch and possibly the elements of method_cache. modified seems to need an reset (not sure, though).

(I also wonder why we store the ce.)

Anyhow, I wonder if that's worth fixing, or whether we should just forbid cloning in the first place; we can still implement proper cloning if requested.

PHP Version

master (but likely since PHP 7, or even before)

Operating System

Windows

[Girgias]

Honestly, I'm not sure cloning makes sense. Yes "cloning" a scalar variant is easy, but I have concerns about how crude the implementation is to properly clone more complicated variants such as arrays.

And as you said we can always relax this constraint further down the line.

[cmb69]

[…] but I have concerns about how crude the implementation is to properly clone more complicated variants such as arrays.

Oh, indeed, that is not properly implemented. We would at least need to check the return value of VariantCopyInd() which may fail for different reasons, and in that case throw an exception.

Also 115ee49, where we prohibited serialization for good reasons. I think these operations have been hacked in, because PHP supports it generally, but were never properly implemented.