From b14076a4e689bf158827bac2a87204654ffd7cff Mon Sep 17 00:00:00 2001
From: "Christoph M. Becker" <cmbecker69@gmx.de>
Date: Wed, 9 Oct 2024 20:39:53 +0200
Subject: [PATCH] Fix property access of PHP objects wrapped in variant

First, we fix the long standing issue that property access throws a
`com_exception` ("0x80020003: member not found), because the `HRESULT`
was not properly set after accessing the property.

Next, we fix an issue introduced as of PHP 7.0.0, where the string
length for write access had been properly adapted, but the string
length for read access had been overlooked.

Then we fix an issue introduced as of PHP 8.0.0, where new `HashTable`s
no longer set `nNextFreeElement` to zero, but to `ZEND_LONG_MIN`.  This
doesn't work well with the `DISPID` lookup, which is a `LONG`.

Finally we fix a potential double-free due to erroneously destroying
the return value of `zend_read_property()`.

Closes GH-16331.
---
 NEWS                                         |  3 ++
 ext/com_dotnet/com_wrapper.c                 | 16 +++++-----
 ext/com_dotnet/tests/variant_variation2.phpt | 33 ++++++++++++++++++++
 3 files changed, 45 insertions(+), 7 deletions(-)
 create mode 100644 ext/com_dotnet/tests/variant_variation2.phpt

diff --git a/NEWS b/NEWS
index 24c1b6a15ec..c31f91c2178 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,9 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.5.0alpha1
 
+- COM:
+  . Fix property access of PHP objects wrapped in variant. (cmb)
+
 - DOM:
   . Added Dom\Element::$outerHTML. (nielsdos)
 
diff --git a/ext/com_dotnet/com_wrapper.c b/ext/com_dotnet/com_wrapper.c
index e505dc65402..42698a2e651 100644
--- a/ext/com_dotnet/com_wrapper.c
+++ b/ext/com_dotnet/com_wrapper.c
@@ -258,9 +258,11 @@ static HRESULT STDMETHODCALLTYPE disp_invokeex(
 		 * and expose it as a COM exception */
 
 		if (wFlags & DISPATCH_PROPERTYGET) {
-			retval = zend_read_property(Z_OBJCE(disp->object), Z_OBJ(disp->object), Z_STRVAL_P(name), Z_STRLEN_P(name)+1, 1, &rv);
+			retval = zend_read_property(Z_OBJCE(disp->object), Z_OBJ(disp->object), Z_STRVAL_P(name), Z_STRLEN_P(name), 1, &rv);
+			ret = S_OK;
 		} else if (wFlags & DISPATCH_PROPERTYPUT) {
 			zend_update_property(Z_OBJCE(disp->object), Z_OBJ(disp->object), Z_STRVAL_P(name), Z_STRLEN_P(name), &params[0]);
+			ret = S_OK;
 		} else if (wFlags & DISPATCH_METHOD) {
 			zend_try {
 				retval = &rv;
@@ -305,7 +307,7 @@ static HRESULT STDMETHODCALLTYPE disp_invokeex(
 				VariantInit(pvarRes);
 				php_com_variant_from_zval(pvarRes, retval, COMG(code_page));
 			}
-			zval_ptr_dtor(retval);
+			// zval_ptr_dtor(retval); // TODO needed for function calls?
 		} else if (pvarRes) {
 			VariantInit(pvarRes);
 		}
@@ -425,7 +427,7 @@ static void generate_dispids(php_dispatchex *disp)
 	zend_string *name = NULL;
 	zval *tmp, tmp2;
 	int keytype;
-	zend_ulong pid;
+	zend_long pid;
 
 	if (disp->dispid_to_name == NULL) {
 		ALLOC_HASHTABLE(disp->dispid_to_name);
@@ -458,8 +460,8 @@ static void generate_dispids(php_dispatchex *disp)
 
 			/* add the mappings */
 			ZVAL_STR_COPY(&tmp2, name);
-			pid = zend_hash_next_free_element(disp->dispid_to_name);
-			zend_hash_index_update(disp->dispid_to_name, pid, &tmp2);
+			zend_hash_next_index_insert(disp->dispid_to_name, &tmp2);
+			pid = zend_hash_next_free_element(disp->dispid_to_name) - 1;
 
 			ZVAL_LONG(&tmp2, pid);
 			zend_hash_update(disp->name_to_dispid, name, &tmp2);
@@ -493,8 +495,8 @@ static void generate_dispids(php_dispatchex *disp)
 
 			/* add the mappings */
 			ZVAL_STR_COPY(&tmp2, name);
-			pid = zend_hash_next_free_element(disp->dispid_to_name);
-			zend_hash_index_update(disp->dispid_to_name, pid, &tmp2);
+			zend_hash_next_index_insert(disp->dispid_to_name, &tmp2);
+			pid = zend_hash_next_free_element(disp->dispid_to_name) - 1;
 
 			ZVAL_LONG(&tmp2, pid);
 			zend_hash_update(disp->name_to_dispid, name, &tmp2);
diff --git a/ext/com_dotnet/tests/variant_variation2.phpt b/ext/com_dotnet/tests/variant_variation2.phpt
new file mode 100644
index 00000000000..5a93967b263
--- /dev/null
+++ b/ext/com_dotnet/tests/variant_variation2.phpt
@@ -0,0 +1,33 @@
+--TEST--
+Testing reading and writing of properties
+--EXTENSIONS--
+com_dotnet
+--FILE--
+<?php
+class MyClass {
+    public $foo = "foo";
+    public string $bar = "bar";
+}
+
+$o = new MyClass();
+$v = new variant($o);
+var_dump($v->foo);
+var_dump($v->bar);
+$v->foo = "new foo";
+var_dump($v->foo instanceof variant);
+var_dump((string) $v->foo);
+var_dump($o->foo instanceof variant);
+var_dump((string) $o->foo);
+$v->bar = "new bar";
+var_dump($v->bar);
+var_dump($o->bar);
+?>
+--EXPECT--
+string(3) "foo"
+string(3) "bar"
+bool(true)
+string(7) "new foo"
+bool(true)
+string(7) "new foo"
+string(7) "new bar"
+string(7) "new bar"