Skip to content

Commit 94fe944

Browse files
habermancopybara-github
authored andcommitted
Automated rollback of commit 8f569de.
PiperOrigin-RevId: 843812355
1 parent dcf3b28 commit 94fe944

File tree

2 files changed

+63
-5
lines changed

2 files changed

+63
-5
lines changed

php/ext/google/protobuf/message.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ 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-
Arena_Init(&intern->arena);
68+
intern->desc = NULL;
69+
ZVAL_NULL(&intern->arena);
6970
return &intern->std;
7071
}
7172

@@ -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));
@@ -530,6 +540,7 @@ bool Message_InitFromPhp(upb_Message* msg, const upb_MessageDef* m, zval* init,
530540
static void Message_Initialize(Message* intern, const Descriptor* desc) {
531541
intern->desc = desc;
532542
const upb_MiniTable* t = upb_MessageDef_MiniTable(desc->msgdef);
543+
Arena_Init(&intern->arena);
533544
intern->msg = upb_Message_New(t, Arena_Get(&intern->arena));
534545
ObjCache_Add(intern->msg, &intern->std);
535546
}
@@ -544,7 +555,6 @@ PHP_METHOD(Message, __construct) {
544555
Message* intern = (Message*)Z_OBJ_P(getThis());
545556
const Descriptor* desc;
546557
zend_class_entry* ce = Z_OBJCE_P(getThis());
547-
upb_Arena* arena = Arena_Get(&intern->arena);
548558
zval* init_arr = NULL;
549559

550560
// This descriptor should always be available, as the generated __construct
@@ -573,7 +583,8 @@ PHP_METHOD(Message, __construct) {
573583
}
574584

575585
if (init_arr) {
576-
Message_InitFromPhp(intern->msg, desc->msgdef, init_arr, arena);
586+
Message_InitFromPhp(intern->msg, desc->msgdef, init_arr,
587+
Arena_Get(&intern->arena));
577588
}
578589
}
579590

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

@@ -1964,6 +1979,38 @@ public function testNoSegfaultWithError()
19641979
new TestMessage(['optional_int32' => $this->throwIntendedException()]);
19651980
}
19661981

1982+
public function testNoSegfaultWithContructorBypass()
1983+
{
1984+
if (!extension_loaded('protobuf')) {
1985+
$this->markTestSkipped('PHP Protobuf extension is not loaded');
1986+
}
1987+
1988+
$this->expectException(Exception::class);
1989+
$this->expectExceptionMessage(
1990+
"Couldn't find descriptor. " .
1991+
"The message constructor was likely bypassed, resulting in an uninitialized descriptor."
1992+
);
1993+
1994+
$m = new TestMessageMockProxy(['optional_int32' => 123]);
1995+
1996+
/**
1997+
* At this point the message constructor was bypassed and the descriptor is not initialized.
1998+
* This is a common PHP pattern where a proxy/mock class extends a concrete class,
1999+
* frequently used in frameworks like PHPUnit, phpspec, and Mockery.
2000+
*
2001+
* When this happens, the message's internal descriptor is never initialized.
2002+
*
2003+
* Without proper handling, accessing properties via getters (like $this->getOptionalInt32())
2004+
* would cause the C extension to segfault when trying to access the uninitialized descriptor.
2005+
*
2006+
* Instead of segfaulting, we now detect this uninitialized state and throw an exception.
2007+
*
2008+
* See: https://github.com/protocolbuffers/protobuf/issues/19978
2009+
*/
2010+
2011+
$m->getOptionalInt32();
2012+
}
2013+
19672014
public function testNoExceptionWithVarDump()
19682015
{
19692016
$m = new Sub(['a' => 1]);

0 commit comments

Comments
 (0)