Skip to content

Commit f9863df

Browse files
FabioBatSilvacopybara-github
authored andcommitted
[php][ext] Prevent Seg fault if constructor is not called (#21240)
Should fix : #19978 Closes #21240 COPYBARA_INTEGRATE_REVIEW=#21240 from ChessCom:php-ext-fault-constructor 8772e96 PiperOrigin-RevId: 747660794
1 parent 454bf54 commit f9863df

File tree

2 files changed

+59
-2
lines changed

2 files changed

+59
-2
lines changed

php/ext/google/protobuf/message.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ static zend_object* Message_create(zend_class_entry* class_type) {
6565
Message_SuppressDefaultProperties(class_type);
6666
zend_object_std_init(&intern->std, class_type);
6767
intern->std.handlers = &message_object_handlers;
68+
intern->desc = NULL;
6869
Arena_Init(&intern->arena);
6970
return &intern->std;
7071
}
@@ -89,6 +90,15 @@ static void Message_dtor(zend_object* obj) {
8990
* Helper function to look up a field given a member name (as a string).
9091
*/
9192
static const upb_FieldDef* get_field(Message* msg, zend_string* member) {
93+
if (!msg || !msg->desc || !msg->desc->msgdef) {
94+
zend_throw_exception_ex(NULL, 0,
95+
"Couldn't find descriptor. "
96+
"The message constructor was likely bypassed, "
97+
"resulting in an uninitialized descriptor.");
98+
99+
return NULL;
100+
}
101+
92102
const upb_MessageDef* m = msg->desc->msgdef;
93103
const upb_FieldDef* f = upb_MessageDef_FindFieldByNameWithSize(
94104
m, ZSTR_VAL(member), ZSTR_LEN(member));

php/tests/GeneratedClassTest.php

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,27 @@
2121
use Php\Test\TestNamespace;
2222

2323
# This is not allowed, but we at least shouldn't crash.
24-
class C extends \Google\Protobuf\Internal\Message {
25-
public function __construct($data = null) {
24+
class C extends \Google\Protobuf\Internal\Message
25+
{
26+
public function __construct($data = null)
27+
{
2628
parent::__construct($data);
2729
}
2830
}
2931

32+
# This is not allowed, but we at least shouldn't crash.
33+
class TestMessageMockProxy extends TestMessage
34+
{
35+
public $_proxy_data = null;
36+
37+
public function __construct($data = null)
38+
{
39+
$this->_proxy_data = $data;
40+
// bypass parent constructor
41+
// This is common behavior by phpunit ond other mock/proxy libraries
42+
}
43+
}
44+
3045
class GeneratedClassTest extends TestBase
3146
{
3247

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

1920+
public function testNoSegfaultWithContructorBypass()
1921+
{
1922+
if (!extension_loaded('protobuf')) {
1923+
$this->markTestSkipped('PHP Protobuf extension is not loaded');
1924+
}
1925+
1926+
$this->expectException(Exception::class);
1927+
$this->expectExceptionMessage(
1928+
"Couldn't find descriptor. " .
1929+
"The message constructor was likely bypassed, resulting in an uninitialized descriptor."
1930+
);
1931+
1932+
$m = new TestMessageMockProxy(['optional_int32' => 123]);
1933+
1934+
/**
1935+
* At this point the message constructor was bypassed and the descriptor is not initialized.
1936+
* This is a common PHP pattern where a proxy/mock class extends a concrete class,
1937+
* frequently used in frameworks like PHPUnit, phpspec, and Mockery.
1938+
*
1939+
* When this happens, the message's internal descriptor is never initialized.
1940+
*
1941+
* Without proper handling, accessing properties via getters (like $this->getOptionalInt32())
1942+
* would cause the C extension to segfault when trying to access the uninitialized descriptor.
1943+
*
1944+
* Instead of segfaulting, we now detect this uninitialized state and throw an exception.
1945+
*
1946+
* See: https://github.com/protocolbuffers/protobuf/issues/19978
1947+
*/
1948+
1949+
$m->getOptionalInt32();
1950+
}
1951+
19051952
public function testNoExceptionWithVarDump()
19061953
{
19071954
$m = new Sub(['a' => 1]);

0 commit comments

Comments
 (0)