Handle optional fields correctly in Message_get method (#18982)

Refactor the Message_get method to accurately handle optional fields with presence checks by returning NULL if the field is not set. Also, add new unit tests in WellKnownTest.php to ensure correct behavior of reflection properties for optional fields with various data types. should fix #18966 Closes #18982 COPYBARA_INTEGRATE_REVIEW=#18982 from s2x:fix-property-reflection-values 9d7030b PiperOrigin-RevId: 701986724
protocolbuffers · Dec 2, 2024 · f1aa92a · f1aa92a
1 parent 5d0865c
commit f1aa92a
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 1 deletion.
diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c
@@ -312,7 +312,13 @@ static zval* Message_read_property(zend_object* obj, zend_string* member,
   const upb_FieldDef* f = get_field(intern, member);
 
   if (!f) return &EG(uninitialized_zval);
-  Message_get(intern, f, rv);
+
+  if (upb_FieldDef_IsOptional(f) && upb_FieldDef_HasPresence(f) &&
+      Message_has_property(obj, member, 0, cache_slot) == false) {
+    ZVAL_NULL(rv);
+  } else {
+    Message_get(intern, f, rv);
+  }
   return rv;
 }
 

diff --git a/php/tests/WellKnownTest.php b/php/tests/WellKnownTest.php
@@ -416,4 +416,44 @@ public function enumNameValueConversionDataProvider()
             ['\Google\Protobuf\Syntax'],
         ];
     }
+
+    /**
+     * @dataProvider optionalFieldsDataProvider
+     */
+    public function testReflectionProperty($property, $default)
+    {
+        $testMessage = new TestMessage();
+
+        $functionName = implode('', array_map('ucwords', explode('_', $property)));
+
+        $reflectionProperty = new \ReflectionProperty($testMessage, $property);
+
+        self::assertFalse(call_user_func([$testMessage, 'has'.$functionName]));
+        self::assertEquals($default, call_user_func([$testMessage, 'get'.$functionName]));
+        self::assertNull($reflectionProperty->getValue($testMessage));
+    }
+
+    public function optionalFieldsDataProvider()
+    {
+        return [
+            ['true_optional_int32', 0],
+            ['true_optional_int64', 0],
+            ['true_optional_uint32', 0],
+            ['true_optional_uint64', 0],
+            ['true_optional_sint32', 0],
+            ['true_optional_sint64', 0],
+            ['true_optional_fixed32', 0],
+            ['true_optional_fixed64', 0],
+            ['true_optional_sfixed32', 0],
+            ['true_optional_sfixed64', 0],
+            ['true_optional_float', 0.0],
+            ['true_optional_double', 0.0],
+            ['true_optional_bool', false],
+            ['true_optional_string', ''],
+            ['true_optional_bytes', ''],
+            ['true_optional_enum', null],
+            ['true_optional_message', null],
+            ['true_optional_included_message', null],
+        ];
+    }
 }