From ca1cb1ba80ef18f5dccfb5b6ee7fa623ba6caab5 Mon Sep 17 00:00:00 2001 From: Jie Luo Date: Mon, 9 Jan 2023 15:55:17 -0800 Subject: [PATCH] Expect fail when serialize inf and nan for Value.number_value in json format. fixes #11259 Implemented in java, c++, python and upb. Also added conformance test. https://developers.google.com/protocol-buffers/docs/reference/google.protobuf#google.protobuf.Value where it says: attempting to serialize NaN or Infinity results in error. (We can't serialize these as string "NaN" or "Infinity" values like we do for regular fields, because they would parse as string_value, not number_value). PiperOrigin-RevId: 500828964 --- conformance/binary_json_conformance_suite.cc | 4 ++++ conformance/failure_list_ruby.txt | 2 ++ .../com/google/protobuf/util/JsonFormat.java | 16 ++++++++++++---- src/google/protobuf/json/internal/unparser.cc | 12 ++++++++++++ 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc index 239db3b7afc97..a915c29263807 100644 --- a/conformance/binary_json_conformance_suite.cc +++ b/conformance/binary_json_conformance_suite.cc @@ -3334,6 +3334,10 @@ void BinaryAndJsonConformanceSuite::RunJsonTestsForValue() { return value.empty(); }, true); + ExpectSerializeFailureForJson("ValueRejectNanNumberValue", RECOMMENDED, + "optional_value: { number_value: nan}"); + ExpectSerializeFailureForJson("ValueRejectInfNumberValue", RECOMMENDED, + "optional_value: { number_value: inf}"); } void BinaryAndJsonConformanceSuite::RunJsonTestsForAny() { diff --git a/conformance/failure_list_ruby.txt b/conformance/failure_list_ruby.txt index 3c744391cf754..8eb63b9a7d4f2 100644 --- a/conformance/failure_list_ruby.txt +++ b/conformance/failure_list_ruby.txt @@ -58,3 +58,5 @@ Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.PackedInput.UnpackedOu Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.UnpackedOutput.ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.UnpackedOutput.ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.UnpackedOutput.ProtobufOutput +Recommended.ValueRejectInfNumberValue.JsonOutput +Recommended.ValueRejectNanNumberValue.JsonOutput diff --git a/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java b/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java index 992980b0dc730..96bc38898f2d6 100644 --- a/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java +++ b/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java @@ -51,7 +51,6 @@ import com.google.protobuf.Descriptors.EnumDescriptor; import com.google.protobuf.Descriptors.EnumValueDescriptor; import com.google.protobuf.Descriptors.FieldDescriptor; -import com.google.protobuf.Descriptors.FieldDescriptor.Type; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.OneofDescriptor; import com.google.protobuf.DoubleValue; @@ -966,7 +965,16 @@ private void printValue(MessageOrBuilder message) throws IOException { throw new InvalidProtocolBufferException("Invalid Value type."); } for (Map.Entry entry : fields.entrySet()) { - printSingleFieldValue(entry.getKey(), entry.getValue()); + FieldDescriptor field = entry.getKey(); + if (field.getType() == FieldDescriptor.Type.DOUBLE) { + Double doubleValue = (Double) entry.getValue(); + if (doubleValue.isNaN() || doubleValue.isInfinite()) { + throw new IllegalArgumentException( + "google.protobuf.Value cannot encode double values for " + + "infinity or nan, because they would be parsed as a string."); + } + } + printSingleFieldValue(field, entry.getValue()); } } @@ -1683,7 +1691,7 @@ private void mergeMapField(FieldDescriptor field, JsonElement json, Message.Buil Object key = parseFieldValue(keyField, new JsonPrimitive(entry.getKey()), entryBuilder); Object value = parseFieldValue(valueField, entry.getValue(), entryBuilder); if (value == null) { - if (ignoringUnknownFields && valueField.getType() == Type.ENUM) { + if (ignoringUnknownFields && valueField.getType() == FieldDescriptor.Type.ENUM) { continue; } else { throw new InvalidProtocolBufferException("Map value cannot be null."); @@ -1724,7 +1732,7 @@ private void mergeRepeatedField( for (int i = 0; i < array.size(); ++i) { Object value = parseFieldValue(field, array.get(i), builder); if (value == null) { - if (ignoringUnknownFields && field.getType() == Type.ENUM) { + if (ignoringUnknownFields && field.getType() == FieldDescriptor.Type.ENUM) { continue; } else { throw new InvalidProtocolBufferException( diff --git a/src/google/protobuf/json/internal/unparser.cc b/src/google/protobuf/json/internal/unparser.cc index 5bda3dbf039f5..85fa403665522 100644 --- a/src/google/protobuf/json/internal/unparser.cc +++ b/src/google/protobuf/json/internal/unparser.cc @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -489,6 +490,17 @@ absl::Status WriteValue(JsonWriter& writer, const Msg& msg, if (Traits::GetSize(number_field, msg) > 0) { auto x = Traits::GetDouble(number_field, msg); RETURN_IF_ERROR(x.status()); + if (std::isnan(*x)) { + return absl::InvalidArgumentError( + "google.protobuf.Value cannot encode double values for nan, " + "because it would be parsed as a string"); + } + if (*x == std::numeric_limits::infinity() || + *x == -std::numeric_limits::infinity()) { + return absl::InvalidArgumentError( + "google.protobuf.Value cannot encode double values for " + "infinity, because it would be parsed as a string"); + } writer.Write(*x); return absl::OkStatus(); }