Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 0 additions & 10 deletions php/ext/google/protobuf/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ static zend_object* Message_create(zend_class_entry* class_type) {
Message_SuppressDefaultProperties(class_type);
zend_object_std_init(&intern->std, class_type);
intern->std.handlers = &message_object_handlers;
intern->desc = NULL;
Arena_Init(&intern->arena);
return &intern->std;
}
Expand All @@ -90,15 +89,6 @@ static void Message_dtor(zend_object* obj) {
* Helper function to look up a field given a member name (as a string).
*/
static const upb_FieldDef* get_field(Message* msg, zend_string* member) {
if (!msg || !msg->desc || !msg->desc->msgdef) {
zend_throw_exception_ex(NULL, 0,
"Couldn't find descriptor. "
"The message constructor was likely bypassed, "
"resulting in an uninitialized descriptor.");

return NULL;
}

const upb_MessageDef* m = msg->desc->msgdef;
const upb_FieldDef* f = upb_MessageDef_FindFieldByNameWithSize(
m, ZSTR_VAL(member), ZSTR_LEN(member));
Expand Down
51 changes: 2 additions & 49 deletions php/tests/GeneratedClassTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,12 @@
use Php\Test\TestNamespace;

# This is not allowed, but we at least shouldn't crash.
class C extends \Google\Protobuf\Internal\Message
{
public function __construct($data = null)
{
class C extends \Google\Protobuf\Internal\Message {
public function __construct($data = null) {
parent::__construct($data);
}
}

# This is not allowed, but we at least shouldn't crash.
class TestMessageMockProxy extends TestMessage
{
public $_proxy_data = null;

public function __construct($data = null)
{
$this->_proxy_data = $data;
// bypass parent constructor
// This is common behavior by phpunit ond other mock/proxy libraries
}
}

class GeneratedClassTest extends TestBase
{

Expand Down Expand Up @@ -1917,38 +1902,6 @@ public function testNoSegfaultWithError()
new TestMessage(['optional_int32' => $this->throwIntendedException()]);
}

public function testNoSegfaultWithContructorBypass()
{
if (!extension_loaded('protobuf')) {
$this->markTestSkipped('PHP Protobuf extension is not loaded');
}

$this->expectException(Exception::class);
$this->expectExceptionMessage(
"Couldn't find descriptor. " .
"The message constructor was likely bypassed, resulting in an uninitialized descriptor."
);

$m = new TestMessageMockProxy(['optional_int32' => 123]);

/**
* At this point the message constructor was bypassed and the descriptor is not initialized.
* This is a common PHP pattern where a proxy/mock class extends a concrete class,
* frequently used in frameworks like PHPUnit, phpspec, and Mockery.
*
* When this happens, the message's internal descriptor is never initialized.
*
* Without proper handling, accessing properties via getters (like $this->getOptionalInt32())
* would cause the C extension to segfault when trying to access the uninitialized descriptor.
*
* Instead of segfaulting, we now detect this uninitialized state and throw an exception.
*
* See: https://github.com/protocolbuffers/protobuf/issues/19978
*/

$m->getOptionalInt32();
}

public function testNoExceptionWithVarDump()
{
$m = new Sub(['a' => 1]);
Expand Down
Loading