Skip to content

Commit

Permalink
[7392] fix conformance test in PHP JSON parser (#19376)
Browse files Browse the repository at this point in the history
# Motivation

This PR fixes failing JSON conformance tests for php with name `IgnoreUnknownEnumStringValue*`.

The JSON parsing spec was discussed in #7392.

Recent similar changes in other languages:
- Python: 86abf35
- Swift: apple/swift-protobuf#1345
- C#: #15758
- C++: #16479

Note: this PR is equivalent to #16743. I had to create a new one since I lost access to noom/protobuf in the meantime (switched companies recently).

Closes #19376

COPYBARA_INTEGRATE_REVIEW=#19376 from antongrbin:anton--7392--php-newbranch 641a28a
PiperOrigin-RevId: 699989555
  • Loading branch information
antongrbin authored and copybara-github committed Nov 25, 2024
1 parent a02ec0f commit 1c98b5b
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 4 deletions.
4 changes: 0 additions & 4 deletions conformance/failure_list_php.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ Recommended.*.FieldMaskTooManyUnderscore.JsonOutput
Recommended.*.JsonInput.BytesFieldBase64Url.JsonOutput
Recommended.*.JsonInput.BytesFieldBase64Url.ProtobufOutput
Recommended.*.JsonInput.FieldMaskInvalidCharacter
Recommended.*.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
Recommended.*.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
Recommended.*.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
Recommended.*.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
Recommended.*.ProtobufInput.ValidDataOneofBinary.MESSAGE.Merge.ProtobufOutput
Recommended.*.ValueRejectInfNumberValue.JsonOutput # Should have failed to serialize, but didn't.
Recommended.*.ValueRejectNanNumberValue.JsonOutput # Should have failed to serialize, but didn't.
Expand Down
22 changes: 22 additions & 0 deletions php/src/Google/Protobuf/Internal/Message.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -1270,6 +1282,16 @@ private function mergeFromArrayJsonImpl($array, $ignore_unknown)
$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 {
Expand Down
62 changes: 62 additions & 0 deletions php/tests/EncodeDecodeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,68 @@ 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\": \"ONE\",
\"2\": \"UNKNOWN_ENUM\",
\"3\": \"ZERO\"
}}"
);
}

public function testDecodeEnumMapWithUnknownStringValueIgnored()
{
$m = new TestMessage();
$m->mergeFromJsonString(
"{\"map_int32_enum\":{
\"1\": \"ONE\",
\"2\": \"UNKNOWN_ENUM\",
\"3\": \"ZERO\"
}}",
true
);

$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\":[
\"ONE\",
\"UNKNOWN_ENUM\",
\"ZERO\"
]}"
);
}

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();
Expand Down

0 comments on commit 1c98b5b

Please sign in to comment.