Fix IgnoreUnknownEnumStringValue php conformance

protocolbuffers · Jun 13, 2024 · 77bb25f · 77bb25f
1 parent 8ef3217
commit 77bb25f
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 10 deletions.
diff --git a/conformance/failure_list_php.txt b/conformance/failure_list_php.txt
@@ -5,16 +5,6 @@ Recommended.Proto2.JsonInput.FieldNameExtension.Validator
 Recommended.Proto3.JsonInput.BytesFieldBase64Url.JsonOutput
 Recommended.Proto3.JsonInput.BytesFieldBase64Url.ProtobufOutput
 Recommended.Proto3.JsonInput.FieldMaskInvalidCharacter
-Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
-Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
-Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
-Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
-Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
-Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
-Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
-Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
-Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
-Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
 Recommended.Proto3.ProtobufInput.ValidDataOneofBinary.MESSAGE.Merge.ProtobufOutput
 Required.Proto2.JsonInput.StoresDefaultPrimitive.Validator
 Required.Proto3.JsonInput.DoubleFieldTooSmall

diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php
@@ -1255,6 +1255,18 @@ private function mergeFromArrayJsonImpl($array, $ignore_unknown)
                         $tmp_value,
                         $value_field,
                         $ignore_unknown);
+
+                    // Mapped unknown enum string values should be silently
+                    // ignored if ignore_unknown is set.
+                    if ($value_field->getType() == GPBType::ENUM &&
+                        is_string($tmp_value) &&
+                        is_null(
+                          $value_field->getEnumType()->getValueByName($tmp_value)
+                        ) &&
+                        $ignore_unknown) {
+                        continue;
+                    }
+
                     self::kvUpdateHelper($field, $proto_key, $proto_value);
                 }
             } else if ($field->isRepeated()) {
@@ -1266,10 +1278,21 @@ private function mergeFromArrayJsonImpl($array, $ignore_unknown)
                         throw new \Exception(
                             "Repeated field elements cannot be null.");
                     }
+
                     $proto_value = $this->convertJsonValueToProtoValue(
                         $tmp,
                         $field,
                         $ignore_unknown);
+
+                    // Repeated unknown enum string values should be silently
+                    // ignored if ignore_unknown is set.
+                    if ($field->getType() == GPBType::ENUM &&
+                        is_string($tmp) &&
+                        is_null($field->getEnumType()->getValueByName($tmp)) &&
+                        $ignore_unknown) {
+                        continue;
+                    }
+
                     self::appendHelper($field, $proto_value);
                 }
             } else {

diff --git a/php/tests/EncodeDecodeTest.php b/php/tests/EncodeDecodeTest.php
@@ -205,6 +205,57 @@ public function testDecodeRepeatedStringValue()
         $this->assertSame("a", $m->getRepeatedField()[0]->getValue());
     }
 
+    public function testDecodeEnumMapWithUnknownStringValueThrows()
+    {
+        $this->expectException(Exception::class);
+
+        $m = new TestMessage();
+        $m->mergeFromJsonString("{\"map_int32_enum\":{\"1\": \"UNKNOWN_ENUM\"}}");
+    }
+
+    public function testDecodeEnumMapWithUnknownStringValueIgnored()
+    {
+        $m = new TestMessage();
+        $m->mergeFromJsonString(
+          "{\"map_int32_enum\":{
+            \"1\": \"ONE\",
+            \"2\": \"UNKNOWN_ENUM\",
+            \"3\": \"ZERO\"
+          }}",
+          true
+        );
+        $this->assertTrue($m->getMapInt32Enum()->offsetExists(1));
+
+        $this->assertSame(TestEnum::ONE, $m->getMapInt32Enum()["1"]);
+        $this->assertSame(TestEnum::ZERO, $m->getMapInt32Enum()["3"]);
+        $this->assertFalse($m->getMapInt32Enum()->offsetExists(2));
+    }
+
+    public function testDecodeRepeatedEnumWithUnknownStringValueThrows()
+    {
+        $this->expectException(Exception::class);
+
+        $m = new TestMessage();
+        $m->mergeFromJsonString("{\"repeated_enum\":[\"UNKNOWN_ENUM\"]}");
+    }
+
+    public function testDecodeRepeatedEnumWithUnknownStringValueIgnored()
+    {
+        $m = new TestMessage();
+        $m->mergeFromJsonString(
+          "{\"repeated_enum\":[
+            \"ONE\",
+            \"UNKNOWN_ENUM\",
+            \"ZERO\"
+          ]}",
+          true
+        );
+
+        $this->assertSame(2, count($m->getRepeatedEnum()));
+        $this->assertSame(TestEnum::ONE, $m->getRepeatedEnum()[0]);
+        $this->assertSame(TestEnum::ZERO, $m->getRepeatedEnum()[1]);
+    }
+
     public function testDecodeMapStringValue()
     {
         $m = new TestStringValue();