Fix method calls for PHP objects wrapped in variant

[cmb69]

As is, methods of PHP can never be called, because we're first trying to read the property with the name of the method.

We fix this by first checking for DISPATCH_METHOD and treat that as method call, if the method would be callable. Only otherwise we try to access the respective property.

It needs to be noted that this breaks code which accesses a property of an object, which defines a method of the same name. However, instances of such classes should never be wrapped in variants, because this can't be distinguished by COM anyway.

---

This is a follow-up to #16331.

Note that this supports COM style where property access and function calls are not necessarily distinguished (VB doesn't, for example). Distinguishing method calls from property accesses would be possible, but might have issues regarding interoperability.

Also note that when accessing properties and calling functions, the names are taken unmodifed, and then looked up as they are stored as object property or method; that means variable need to use the case they've been declared with, and functions always needs to be lower-cased. Fixing this appears to be trivial, but would further reduce the available namespace (usually there can be different PHP properties with the same name in when using different case). Still, it might be a good idea to convert to lower-case, and to clearly document that such PHP objects are not supposed to have properties/methods with the same name in different letter case (since currently that already doesn't work cleanly; if there is a property and a method with the same name, only the property gets a disp ID, but one can still call the method).

 ext/com_dotnet/com_wrapper.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/ext/com_dotnet/com_wrapper.c b/ext/com_dotnet/com_wrapper.c
index 81c3196916..5181f0541d 100644
--- a/ext/com_dotnet/com_wrapper.c
+++ b/ext/com_dotnet/com_wrapper.c
@@ -152,20 +152,22 @@ static HRESULT STDMETHODCALLTYPE disp_getidsofnames(
 	FETCH_DISP("GetIDsOfNames");
 
 	for (i = 0; i < cNames; i++) {
-		zend_string *name;
+		zend_string *name, *lcname;
 		zval *tmp;
 
 		name = php_com_olestring_to_string(rgszNames[i], COMG(code_page));
+		lcname = zend_string_tolower(name);
+		zend_string_release_ex(name, /* persistent */ false);
 
 		/* Lookup the name in the hash */
-		if ((tmp = zend_hash_find(disp->name_to_dispid, name)) == NULL) {
+		if ((tmp = zend_hash_find(disp->name_to_dispid, lcname)) == NULL) {
 			ret = DISP_E_UNKNOWNNAME;
 			rgDispId[i] = 0;
 		} else {
 			rgDispId[i] = (DISPID)Z_LVAL_P(tmp);
 		}
 
-		zend_string_release_ex(name, /* persistent */ false);
+		zend_string_release_ex(lcname, /* persistent */ false);
 
 	}
 
@@ -449,7 +451,7 @@ static void generate_dispids(php_dispatchex *disp)
 				snprintf(namebuf, sizeof(namebuf), ZEND_ULONG_FMT, pid);
 				name = zend_string_init(namebuf, strlen(namebuf), 0);
 			} else {
-				zend_string_addref(name);
+				name = zend_string_tolower(name);
 			}
 
 			zend_hash_move_forward_ex(Z_OBJPROP(disp->object), &pos);

[Girgias]

I think the change of ordering makes sense.

Storing function/methods as lowercase may also make sense, but I would leave this as a follow-up.

[Girgias]

What may make sense as a follow-up is to pass an FCC to get rid of the call_user_function() call below as we would be able to just call it, instead of rechecking it.

Nor do I think the zend_try { } zend_catch { } is necessary (except if it needs to deal with Fatal Errors? But that doesn't seem to be what the comment is implying).

[cmb69]

Regarding the FCC refactoring, see #17001.

Regarding the try-catch: this is not related to the comment above (which still needs to be addressed), but it's likely necessary for out-of-process COM invocation, since even if the ZE bails out, the caller needs to be informed. However, the current implementation might not necessarily work. Might need to move the try-catch up the call stack, and retrigger zend_bailout() ASAP.

[Girgias]

Thank you for working on this, it is highly appreciated!

[cmb69]

One task completed, three new tasks in the backlog. Ugh. I'll see what I can do.