Skip to content

Fix method calls for PHP objects wrapped in variant #16945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fix method calls for PHP objects wrapped in variant #16945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions ext/com_dotnet/com_wrapper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,13 +257,10 @@ static HRESULT STDMETHODCALLTYPE disp_invokeex(
/* TODO: if PHP raises an exception here, we should catch it
* 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, &rv);
ret = S_OK;
} else if (wFlags & DISPATCH_PROPERTYPUT) {
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) {
} else if (wFlags & DISPATCH_METHOD && zend_is_callable_ex(name, Z_OBJ(disp->object), 0, NULL, NULL, NULL)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

zend_try {
retval = &rv;
if (SUCCESS == call_user_function(NULL, &disp->object, name,
Expand All @@ -289,6 +286,9 @@ static HRESULT STDMETHODCALLTYPE disp_invokeex(
trace("something blew up\n");
ret = DISP_E_EXCEPTION;
} zend_end_try();
} else 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, &rv);
ret = S_OK;
} else {
trace("Don't know how to handle this invocation %08x\n", wFlags);
}
Expand All @@ -307,7 +307,9 @@ static HRESULT STDMETHODCALLTYPE disp_invokeex(
VariantInit(pvarRes);
php_com_variant_from_zval(pvarRes, retval, COMG(code_page));
}
// zval_ptr_dtor(retval); // TODO needed for function calls?
if (retval == &rv) {
zval_ptr_dtor(retval);
}
} else if (pvarRes) {
VariantInit(pvarRes);
}
Expand Down
41 changes: 41 additions & 0 deletions ext/com_dotnet/tests/variant_variation3.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
--TEST--
Testing reading properties and calling functions
--EXTENSIONS--
com_dotnet
--FILE--
<?php
class MyClass {
public $foo = "property";
public $bar = "bar";
public function foo() {
return "method";
}
public function stdClass() {
return new stdclass();
}
}

$o = new MyClass();
$v = new variant($o);
var_dump($v->foo);
var_dump($v->foo());
var_dump($v->bar);
var_dump($v->bar());
var_dump($v->stdclass);
var_dump($v->stdclass());
try {
var_dump($v->qux);
} catch (com_exception $ex) {
echo $ex->getMessage(), "\n";
}
?>
--EXPECTF--
string(6) "method"
string(6) "method"
string(3) "bar"
string(3) "bar"
object(variant)#%d (0) {
}
object(variant)#%d (0) {
}
Unable to lookup `qux': %s