Fix GH-17900 and GH-8084 #17908
Fix GH-17900 and GH-8084 #17908
Conversation
nielsdos
force-pushed
the
mysqli-no-double-construct
branch
from
229beda to
4205c31
Compare
Calling the constructor twice has no real world benefit. Block it to fix these two issues. We also clean up the constructor code a bit: - `in_ctor` implies `object` exist. - We surround the instance check with ZEND_DEBUG to avoid a runtime penalty. Closes phpGH-17900. Closes phpGH-8084.
ext/mysqli/mysqli_nonapi.c
Outdated
| @@ -85,7 +90,9 @@ void mysqli_common_connect(INTERNAL_FUNCTION_PARAMETERS, bool is_real_connect, b | |||
| } | |||
|
|
|||
| if (object) { | |||
| #if ZEND_DEBUG | |||
There was a problem hiding this comment.
This isn't part of the fix but initially I had changed the code in this if branch as well and put this in an #if to avoid the overhead, I can drop this or commit this separately
There was a problem hiding this comment.
I don't think the assert should be in #if.
There was a problem hiding this comment.
I don't think the assert should be in #if.
I can drop this. The motivation is that the call to instanceof_function_slow will still be done even in release builds, but it's not that big of a deal.
There was a problem hiding this comment.
It makes sense to me.
As an aside, __construct should be blocked the same way for the other 2 classes.
I also wonder why mysqli_common_connect even accepts in_ctor. Wouldn't it make more sense to have this check outside of this method in PHP_METHOD(mysqli, __construct)?
Perhaps, for a follow-up maybe.
If we do this prior to argument parsing, we create an arginfo/ZPP consistency violation; because if the wrong arguments are passed these should throw prior to the double constructor exception being thrown. |
But isn't the check you added done prior to parsing arguments anyway? |
Ah damn, I'm sorry. I thought I did this differently, I shouldn't work on multiple things at the same time... |
Calling the constructor twice has no real world benefit. Block it to fix these two issues.
We also clean up the constructor code a bit:
in_ctorimpliesobjectexists, and some conditions are only possible withobjectset.Closes GH-17900.
Closes GH-8084.