From 77bb25fb62c8744749ee50152d94f30a2166e18f Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Sat, 4 May 2024 15:06:26 +0200 Subject: [PATCH] Fix IgnoreUnknownEnumStringValue php conformance --- conformance/failure_list_php.txt | 10 ---- php/src/Google/Protobuf/Internal/Message.php | 23 +++++++++ php/tests/EncodeDecodeTest.php | 51 ++++++++++++++++++++ 3 files changed, 74 insertions(+), 10 deletions(-) diff --git a/conformance/failure_list_php.txt b/conformance/failure_list_php.txt index 5c09f3fc2f3e2..3232047aa759f 100644 --- 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 index 31e2f29d300e7..f6faf58c87352 100644 --- 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 index abf95cca00dbc..7cc1b2aadd541 100644 --- 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();