Fix non-conformance in C++ JSON parsing of integer strings.

Integer fields are supposed to accept string values containing floats as long as they are equal to integers. PiperOrigin-RevId: 704785838
protocolbuffers · Dec 10, 2024 · cbed3b8 · cbed3b8
1 parent 7fbb3d2
commit cbed3b8
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 19 deletions.
diff --git a/conformance/failure_list_cpp.txt b/conformance/failure_list_cpp.txt
@@ -35,4 +35,3 @@ Recommended.*.FieldMaskNumbersDontRoundTrip.JsonOutput
 Recommended.*.FieldMaskPathsDontRoundTrip.JsonOutput                                                               # Should have failed to serialize, but didn't.
 Recommended.*.FieldMaskTooManyUnderscore.JsonOutput                                                                # Should have failed to serialize, but didn't.
 Recommended.*.JsonInput.FieldMaskInvalidCharacter                                                                  # Should have failed to parse, but didn't.
-Required.*.JsonInput.Int32FieldQuotedExponentialValue.*                                                            # Failed to parse input or produce output.
diff --git a/src/google/protobuf/json/BUILD.bazel b/src/google/protobuf/json/BUILD.bazel
@@ -223,6 +223,7 @@ cc_library(
     deps = [
         ":descriptor_traits",
         ":lexer",
+        ":zero_copy_buffered_stream",
         "//src/google/protobuf",
         "//src/google/protobuf:port",
         "//src/google/protobuf:protobuf_lite",

diff --git a/src/google/protobuf/json/internal/parser.cc b/src/google/protobuf/json/internal/parser.cc
@@ -40,6 +40,7 @@
 #include "google/protobuf/json/internal/descriptor_traits.h"
 #include "google/protobuf/json/internal/lexer.h"
 #include "google/protobuf/json/internal/parser_traits.h"
+#include "google/protobuf/json/internal/zero_copy_buffered_stream.h"
 #include "google/protobuf/message.h"
 #include "google/protobuf/util/type_resolver.h"
 #include "google/protobuf/stubs/status_macros.h"
@@ -169,6 +170,28 @@ absl::StatusOr<absl::Span<char>> DecodeBase64InPlace(absl::Span<char> base64) {
                           static_cast<size_t>(out - base64.data()));
 }
 
+template <typename T>
+static absl::Status ParseFloatStringAsInt(
+    const LocationWith<MaybeOwnedString>& x, T* out, double lo, double hi) {
+  double d;
+  if (!absl::SimpleAtod(x.value.AsView(), &d) || !std::isfinite(d)) {
+    return x.loc.Invalid(
+        absl::StrFormat("invalid number: '%s'", x.value.AsView()));
+  }
+
+  // Conversion overflow here would be UB.
+  if (lo > d || d > hi) {
+    return x.loc.Invalid("JSON number out of range for int");
+  }
+  *out = static_cast<T>(d);
+  if (d - static_cast<double>(*out) != 0) {
+    return x.loc.Invalid(
+        "expected integer, but JSON number had fractional part");
+  }
+
+  return absl::OkStatus();
+}
+
 template <typename T>
 absl::StatusOr<LocationWith<T>> ParseIntInner(JsonLexer& lex, double lo,
                                               double hi) {
@@ -185,37 +208,27 @@ absl::StatusOr<LocationWith<T>> ParseIntInner(JsonLexer& lex, double lo,
         break;
       }
 
-      double d;
-      if (!absl::SimpleAtod(x->value.AsView(), &d) || !std::isfinite(d)) {
-        return x->loc.Invalid(
-            absl::StrFormat("invalid number: '%s'", x->value.AsView()));
-      }
-
-      // Conversion overflow here would be UB.
-      if (lo > d || d > hi) {
-        return lex.Invalid("JSON number out of range for int");
-      }
-      n.value = static_cast<T>(d);
-      if (d - static_cast<double>(n.value) != 0) {
-        return lex.Invalid(
-            "expected integer, but JSON number had fractional part");
-      }
+      RETURN_IF_ERROR(ParseFloatStringAsInt<T>(*x, &n.value, lo, hi));
       break;
     }
     case JsonLexer::kStr: {
       absl::StatusOr<LocationWith<MaybeOwnedString>> str = lex.ParseUtf8();
       RETURN_IF_ERROR(str.status());
+
+      n.loc = str->loc;
+
       // SimpleAtoi will ignore leading and trailing whitespace, so we need
       // to check for it ourselves.
       for (char c : str->value.AsView()) {
         if (absl::ascii_isspace(c)) {
           return lex.Invalid("non-number characters in quoted number");
         }
       }
-      if (!absl::SimpleAtoi(str->value.AsView(), &n.value)) {
-        return str->loc.Invalid("non-number characters in quoted number");
+      if (absl::SimpleAtoi(str->value.AsView(), &n.value)) {
+        break;
       }
-      n.loc = str->loc;
+
+      RETURN_IF_ERROR(ParseFloatStringAsInt<T>(*str, &n.value, lo, hi));
       break;
     }
     default:

diff --git a/src/google/protobuf/json/json_test.cc b/src/google/protobuf/json/json_test.cc
@@ -347,6 +347,39 @@ TEST_P(JsonTest, QuotedEnumValue) {
   EXPECT_THAT(m->enum_value1(), proto3::BAR);
 }
 
+TEST_P(JsonTest, QuotedIntegerValue) {
+  auto m = ToProto<TestMessage>(R"json(
+    {"int32Value": "2"}
+  )json");
+  ASSERT_OK(m);
+  EXPECT_THAT(m->int32_value(), 2);
+}
+
+TEST_P(JsonTest, QuotedIntegerFloatValue) {
+  auto m = ToProto<TestMessage>(R"json(
+    {"int32Value": "2.0"}
+  )json");
+  ASSERT_OK(m);
+  EXPECT_THAT(m->int32_value(), 2);
+}
+
+TEST_P(JsonTest, QuotedIntegerExponentValue) {
+  auto m = ToProto<TestMessage>(R"json(
+    {"int32Value": "1e2"}
+  )json");
+  ASSERT_OK(m);
+  EXPECT_THAT(m->int32_value(), 100);
+}
+
+TEST_P(JsonTest, TestInvalidIntegerValue) {
+  EXPECT_THAT(ToProto<TestMessage>(R"("{"int32Value": ""})"),
+              StatusIs(absl::StatusCode::kInvalidArgument));
+  EXPECT_THAT(ToProto<TestMessage>(R"("{"int32Value": 1.001})"),
+              StatusIs(absl::StatusCode::kInvalidArgument));
+  EXPECT_THAT(ToProto<TestMessage>(R"("{"int32Value": "1.2e1"})"),
+              StatusIs(absl::StatusCode::kInvalidArgument));
+}
+
 TEST_P(JsonTest, WebSafeBytes) {
   auto m = ToProto<TestMessage>(R"json({
       "bytesValue": "-_"