Add option to C++ JSON Parser/Serializer to allow customers to affirmatively disable legacy bug-compatibilty behaviors.

Right now this flag already exists inside the implementation but is not exposed to callers. Since this implementation has now been stable and used for an extended period with this behavior, we did not continue working on adhoc burning down these nonconformant behaviors piecemeal, so by offering the flag 'publicly' it will enable users who want a more conformant implementation to get it much faster.

The default behavior is unchanged.

PiperOrigin-RevId: 791836360

Author: protobuf-github-bot

src/google/protobuf/json/BUILD.bazel

@@ -18,8 +18,10 @@ cc_library(
     strip_include_prefix = "/src",
     visibility = ["//visibility:public"],
     deps = [
+        ":lexer",
         ":parser",
         ":unparser",
+        ":writer",
         "//src/google/protobuf",
         "//src/google/protobuf:port",
         "//src/google/protobuf/io",

src/google/protobuf/json/internal/lexer.cc

@@ -376,7 +376,7 @@ absl::StatusOr<LocationWith<MaybeOwnedString>> JsonLexer::ParseUtf8() {
   // This is a non-standard extension accepted by the ESF parser that we will
   // need to accept for backwards-compat.
   bool is_single_quote = stream_.PeekChar() == '\'';
-  if (!options_.allow_legacy_syntax && is_single_quote) {
+  if (!options_.allow_legacy_nonconformant_behavior && is_single_quote) {
     return Invalid("expected '\"'");
   }
 
@@ -423,14 +423,16 @@ absl::StatusOr<LocationWith<MaybeOwnedString>> JsonLexer::ParseUtf8() {
 
         char c = stream_.PeekChar();
         RETURN_IF_ERROR(Advance(1));
-        if (c == 'u' || (c == 'U' && options_.allow_legacy_syntax)) {
+        if (c == 'u' ||
+            (c == 'U' && options_.allow_legacy_nonconformant_behavior)) {
           // Ensure there is actual space to scribble the UTF-8 onto.
           on_heap.resize(on_heap.size() + 4);
           auto written = ParseUnicodeEscape(&on_heap[on_heap.size() - 4]);
           RETURN_IF_ERROR(written.status());
           on_heap.resize(on_heap.size() - 4 + *written);
         } else {
-          char escape = ParseSimpleEscape(c, options_.allow_legacy_syntax);
+          char escape = ParseSimpleEscape(
+              c, options_.allow_legacy_nonconformant_behavior);
           if (escape == 0) {
             return Invalid(absl::StrFormat("invalid escape char: '%c'", c));
           }
@@ -444,7 +446,8 @@ absl::StatusOr<LocationWith<MaybeOwnedString>> JsonLexer::ParseUtf8() {
         // If people have newlines in their strings, that's their problem; it
         // is too difficult to support correctly in our location tracking, and
         // is out of spec, so users will get slightly wrong locations in errors.
-        if ((uc < 0x20 || uc == 0xff) && !options_.allow_legacy_syntax) {
+        if ((uc < 0x20 || uc == 0xff) &&
+            !options_.allow_legacy_nonconformant_behavior) {
           return Invalid(absl::StrFormat(
               "invalid control character 0x%02x in string", uc));
         }

src/google/protobuf/json/internal/lexer.h

@@ -9,6 +9,7 @@
 #ifndef GOOGLE_PROTOBUF_JSON_INTERNAL_LEXER_H__
 #define GOOGLE_PROTOBUF_JSON_INTERNAL_LEXER_H__
 
+#include <cstddef>
 #include <cstdint>
 #include <utility>
 
@@ -48,7 +49,7 @@ struct ParseOptions {
   // What those extensions were is explicitly not documented, beyond what exists
   // in the unit tests; we intend to remove this setting eventually. See
   // b/234868512.
-  bool allow_legacy_syntax = false;
+  bool allow_legacy_nonconformant_behavior = false;
 };
 
 // A position in JSON input, for error context.
@@ -271,7 +272,7 @@ absl::Status JsonLexer::VisitArray(F f) {
     has_comma = Peek(",");
   } while (!Peek("]"));
 
-  if (!options_.allow_legacy_syntax && has_comma) {
+  if (!options_.allow_legacy_nonconformant_behavior && has_comma) {
     return Invalid("expected ']'");
   }
 
@@ -302,7 +303,7 @@ absl::Status JsonLexer::VisitObject(F f) {
     absl::StatusOr<LocationWith<MaybeOwnedString>> key;
     if (stream_.PeekChar() == '"' || stream_.PeekChar() == '\'') {
       key = ParseUtf8();
-    } else if (options_.allow_legacy_syntax) {
+    } else if (options_.allow_legacy_nonconformant_behavior) {
       key = ParseBareWord();
     } else {
       return Invalid("expected '\"'");
@@ -315,7 +316,7 @@ absl::Status JsonLexer::VisitObject(F f) {
   } while (!Peek("}"));
   Pop();
 
-  if (!options_.allow_legacy_syntax && has_comma) {
+  if (!options_.allow_legacy_nonconformant_behavior && has_comma) {
     return Invalid("expected '}'");
   }
 

src/google/protobuf/json/internal/lexer_test.cc

@@ -231,7 +231,7 @@ void BadInner(absl::string_view json, ParseOptions opts = {}) {
 void DoLegacy(absl::string_view json, std::function<void(const Value&)> test) {
   Do(json, [&](io::ZeroCopyInputStream* stream) {
     ParseOptions options;
-    options.allow_legacy_syntax = true;
+    options.allow_legacy_nonconformant_behavior = true;
     auto value = Value::Parse(stream, options);
     ASSERT_OK(value);
     test(*value);
@@ -242,7 +242,7 @@ void DoLegacy(absl::string_view json, std::function<void(const Value&)> test) {
 // Like Bad, but ensures json fails to parse in both modes.
 void Bad(absl::string_view json) {
   ParseOptions options;
-  options.allow_legacy_syntax = true;
+  options.allow_legacy_nonconformant_behavior = true;
   BadInner(json, options);
   BadInner(json);
 }

src/google/protobuf/json/internal/parser.cc

@@ -482,7 +482,7 @@ absl::Status ParseSingular(JsonLexer& lex, Field<Traits> field,
           Traits::SetBool(field, msg, false);
           break;
         case JsonLexer::kStr: {
-          if (!lex.options().allow_legacy_syntax) {
+          if (!lex.options().allow_legacy_nonconformant_behavior) {
             goto bad;
           }
 
@@ -672,7 +672,7 @@ absl::Status ParseArray(JsonLexer& lex, Field<Traits> field, Msg<Traits>& msg) {
         return ParseSingular<Traits>(lex, field, msg);
       }
 
-      if (lex.options().allow_legacy_syntax) {
+      if (lex.options().allow_legacy_nonconformant_behavior) {
         RETURN_IF_ERROR(lex.Expect("null"));
         return EmitNull<Traits>(lex, field, msg);
       }
@@ -689,7 +689,7 @@ absl::Status ParseArray(JsonLexer& lex, Field<Traits> field, Msg<Traits>& msg) {
     // the custom parser handler.
     bool can_flatten =
         type != MessageType::kValue && type != MessageType::kList;
-    if (can_flatten && lex.options().allow_legacy_syntax &&
+    if (can_flatten && lex.options().allow_legacy_nonconformant_behavior &&
         lex.Peek(JsonLexer::kArr)) {
       // You read that right. In legacy mode, if we encounter an array within
       // an array, we just flatten it as part of the current array!
@@ -1023,7 +1023,7 @@ absl::Status ParseFieldMask(JsonLexer& lex, const Desc<Traits>& desc,
       } else if (absl::ascii_isupper(c)) {
         snake_path.push_back('_');
         snake_path.push_back(absl::ascii_tolower(c));
-      } else if (lex.options().allow_legacy_syntax) {
+      } else if (lex.options().allow_legacy_nonconformant_behavior) {
         snake_path.push_back(c);
       } else {
         return str->loc.Invalid("unexpected character in FieldMask");
@@ -1069,7 +1069,8 @@ absl::Status ParseAny(JsonLexer& lex, const Desc<Traits>& desc,
   // limit.
   JsonLexer any_lex(&in, lex.options(), &lex.path(), mark.loc);
 
-  if (!type_url.has_value() && !lex.options().allow_legacy_syntax) {
+  if (!type_url.has_value() &&
+      !lex.options().allow_legacy_nonconformant_behavior) {
     return mark.loc.Invalid("missing @type in Any");
   }
 
@@ -1085,7 +1086,7 @@ absl::Status ParseAny(JsonLexer& lex, const Desc<Traits>& desc,
         });
   } else {
     // Empty {} is accepted in legacy mode.
-    ABSL_DCHECK(lex.options().allow_legacy_syntax);
+    ABSL_DCHECK(lex.options().allow_legacy_nonconformant_behavior);
     RETURN_IF_ERROR(any_lex.VisitObject([&](auto&) {
       return mark.loc.Invalid(
           "in legacy mode, missing @type in Any is only allowed for an empty "
@@ -1253,9 +1254,9 @@ absl::Status ParseField(JsonLexer& lex, const Desc<Traits>& desc,
   auto pop = lex.path().Push(name, Traits::FieldType(*field),
                              Traits::FieldTypeName(*field));
 
-  if (Traits::HasParsed(
-          *field, msg,
-          /*allow_repeated_non_oneof=*/lex.options().allow_legacy_syntax) &&
+  if (Traits::HasParsed(*field, msg,
+                        /*allow_repeated_non_oneof=*/
+                        lex.options().allow_legacy_nonconformant_behavior) &&
       !lex.Peek(JsonLexer::kNull)) {
     return lex.Invalid(absl::StrFormat(
         "'%s' has already been set (either directly or as part of a oneof)",
@@ -1267,7 +1268,8 @@ absl::Status ParseField(JsonLexer& lex, const Desc<Traits>& desc,
   }
 
   if (Traits::IsRepeated(*field)) {
-    if (lex.options().allow_legacy_syntax && !lex.Peek(JsonLexer::kArr)) {
+    if (lex.options().allow_legacy_nonconformant_behavior &&
+        !lex.Peek(JsonLexer::kArr)) {
       // The original ESF parser permits a single element in place of an array
       // thereof.
       return ParseSingular<Traits>(lex, *field, msg);
@@ -1297,7 +1299,8 @@ absl::Status ParseMessage(JsonLexer& lex, const Desc<Traits>& desc,
     // It is not clear if this counts as out-of-spec, but we're treating it as
     // such.
     bool is_upcoming_object = lex.Peek(JsonLexer::kObj);
-    if (!(is_upcoming_object && lex.options().allow_legacy_syntax)) {
+    if (!(is_upcoming_object &&
+          lex.options().allow_legacy_nonconformant_behavior)) {
       switch (type) {
         case MessageType::kList:
           return ParseListValue<Traits>(lex, desc, msg);

src/google/protobuf/json/internal/unparser.cc

@@ -129,7 +129,7 @@ absl::Status WriteSingular(JsonWriter& writer, Field<Traits> field,
     case FieldDescriptor::TYPE_FLOAT: {
       auto x = Traits::GetFloat(field, std::forward<Args>(args)...);
       RETURN_IF_ERROR(x.status());
-      if (writer.options().allow_legacy_syntax && is_default &&
+      if (writer.options().allow_legacy_nonconformant_behavior && is_default &&
           !std::isfinite(*x)) {
         *x = 0;
       }
@@ -139,7 +139,7 @@ absl::Status WriteSingular(JsonWriter& writer, Field<Traits> field,
     case FieldDescriptor::TYPE_DOUBLE: {
       auto x = Traits::GetDouble(field, std::forward<Args>(args)...);
       RETURN_IF_ERROR(x.status());
-      if (writer.options().allow_legacy_syntax && is_default &&
+      if (writer.options().allow_legacy_nonconformant_behavior && is_default &&
           !std::isfinite(*x)) {
         *x = 0;
       }
@@ -203,7 +203,7 @@ absl::Status WriteSingular(JsonWriter& writer, Field<Traits> field,
       auto x = Traits::GetString(field, writer.ScratchBuf(),
                                  std::forward<Args>(args)...);
       RETURN_IF_ERROR(x.status());
-      if (writer.options().allow_legacy_syntax && is_default) {
+      if (writer.options().allow_legacy_nonconformant_behavior && is_default) {
         // Although difficult to verify, it appears that the original ESF parser
         // fails to unescape the contents of a
         // google.protobuf.Field.default_value, which may potentially be
@@ -417,7 +417,7 @@ absl::Status WriteField(JsonWriter& writer, const Msg<Traits>& msg,
     // with an uppercase letter, and the Json name does not, we uppercase it.
     absl::string_view original_name = Traits::FieldName(field);
     absl::string_view json_name = Traits::FieldJsonName(field);
-    if (writer.options().allow_legacy_syntax &&
+    if (writer.options().allow_legacy_nonconformant_behavior &&
         absl::ascii_isupper(original_name[0]) &&
         !absl::ascii_isupper(json_name[0])) {
       writer.Write(MakeQuoted(absl::ascii_toupper(original_name[0]),
@@ -712,10 +712,11 @@ absl::Status WriteFieldMask(JsonWriter& writer, const Msg<Traits>& msg,
       } else if (absl::ascii_isdigit(c) || absl::ascii_islower(c) || c == '.') {
         writer.Write(c);
       } else if (c == '_' &&
-                 (!saw_under || writer.options().allow_legacy_syntax)) {
+                 (!saw_under ||
+                  writer.options().allow_legacy_nonconformant_behavior)) {
         saw_under = true;
         continue;
-      } else if (!writer.options().allow_legacy_syntax) {
+      } else if (!writer.options().allow_legacy_nonconformant_behavior) {
         return absl::InvalidArgumentError("unexpected character in FieldMask");
       } else {
         if (saw_under) {
@@ -744,7 +745,8 @@ absl::Status WriteAny(JsonWriter& writer, const Msg<Traits>& msg,
     return absl::OkStatus();
   } else if (!has_type_url) {
     return absl::InvalidArgumentError("broken Any: missing type URL");
-  } else if (!has_value && !writer.options().allow_legacy_syntax) {
+  } else if (!has_value &&
+             !writer.options().allow_legacy_nonconformant_behavior) {
     return absl::InvalidArgumentError("broken Any: missing value");
   }
 

src/google/protobuf/json/internal/writer.h

@@ -8,6 +8,7 @@
 #ifndef GOOGLE_PROTOBUF_JSON_INTERNAL_WRITER_H__
 #define GOOGLE_PROTOBUF_JSON_INTERNAL_WRITER_H__
 
+#include <cstddef>
 #include <cstdint>
 #include <string>
 #include <tuple>
@@ -50,7 +51,7 @@ struct WriterOptions {
   // What those extensions were is explicitly not documented, beyond what exists
   // in the unit tests; we intend to remove this setting eventually. See
   // b/234868512.
-  bool allow_legacy_syntax = false;
+  bool allow_legacy_nonconformant_behavior = false;
 };
 
 template <typename Tuple, typename F, size_t... i>

src/google/protobuf/json/json.cc

@@ -12,10 +12,13 @@
 #include "absl/status/status.h"
 #include "absl/strings/string_view.h"
 #include "google/protobuf/io/zero_copy_stream.h"
+#include "google/protobuf/io/zero_copy_stream_impl_lite.h"
+#include "google/protobuf/json/internal/lexer.h"
 #include "google/protobuf/json/internal/parser.h"
 #include "google/protobuf/json/internal/unparser.h"
+#include "google/protobuf/json/internal/writer.h"
+#include "google/protobuf/message.h"
 #include "google/protobuf/util/type_resolver.h"
-#include "google/protobuf/stubs/status_macros.h"
 
 // Must be included last.
 #include "google/protobuf/port_def.inc"
@@ -38,7 +41,8 @@ absl::Status BinaryToJsonStream(google::protobuf::util::TypeResolver* resolver,
   opts.unquote_int64_if_possible = options.unquote_int64_if_possible;
 
   // TODO: Drop this setting.
-  opts.allow_legacy_syntax = true;
+  opts.allow_legacy_nonconformant_behavior =
+      options.allow_legacy_nonconformant_behavior;
 
   return google::protobuf::json_internal::BinaryToJsonStream(
       resolver, type_url, binary_input, json_output, opts);
@@ -65,7 +69,8 @@ absl::Status JsonToBinaryStream(google::protobuf::util::TypeResolver* resolver,
   opts.case_insensitive_enum_parsing = options.case_insensitive_enum_parsing;
 
   // TODO: Drop this setting.
-  opts.allow_legacy_syntax = true;
+  opts.allow_legacy_nonconformant_behavior =
+      options.allow_legacy_nonconformant_behavior;
 
   return google::protobuf::json_internal::JsonToBinaryStream(
       resolver, type_url, json_input, binary_output, opts);
@@ -100,7 +105,8 @@ absl::Status MessageToJsonStream(const Message& message,
   opts.unquote_int64_if_possible = options.unquote_int64_if_possible;
 
   // TODO: Drop this setting.
-  opts.allow_legacy_syntax = true;
+  opts.allow_legacy_nonconformant_behavior =
+      options.allow_legacy_nonconformant_behavior;
 
   return google::protobuf::json_internal::MessageToJsonStream(message, json_output, opts);
 }
@@ -119,7 +125,8 @@ absl::Status JsonStreamToMessage(io::ZeroCopyInputStream* input,
   opts.case_insensitive_enum_parsing = options.case_insensitive_enum_parsing;
 
   // TODO: Drop this setting.
-  opts.allow_legacy_syntax = true;
+  opts.allow_legacy_nonconformant_behavior =
+      options.allow_legacy_nonconformant_behavior;
 
   return google::protobuf::json_internal::JsonStreamToMessage(input, message, opts);
 }

src/google/protobuf/json/json.h

@@ -24,6 +24,11 @@ namespace google {
 namespace protobuf {
 namespace json {
 struct ParseOptions {
+  // Whether some legacy non-spec behaviors are accepted for bug
+  // compatibility reasons. Setting allow_legacy_nonconformant_behavior=false is
+  // recommended for new code and is expected to eventually become the default.
+  bool allow_legacy_nonconformant_behavior = true;
+
   // Whether to ignore unknown JSON fields during parsing
   bool ignore_unknown_fields = false;
 
@@ -36,6 +41,11 @@ struct ParseOptions {
 };
 
 struct PrintOptions {
+  // Whether some legacy non-spec behaviors are accepted for bug
+  // compatibility reasons. Setting allow_legacy_nonconformant_behavior=false is
+  // recommended for new code and is expected to eventually become the default.
+  bool allow_legacy_nonconformant_behavior = true;
+
   // Whether to add spaces, line breaks and indentation to make the JSON output
   // easy to read.
   bool add_whitespace = false;

src/google/protobuf/json/json_test.cc

@@ -247,6 +247,16 @@ TEST_P(JsonTest, TestAlwaysPrintFieldsWithNoPresence) {
           R"("repeatedStringPiece":[],"repeatedCord":[],"repeatedLazyMessage":[]})"));
 }
 
+TEST_P(JsonTest, TestDisableLegacyNonconformantBehavior) {
+  EXPECT_THAT(ToProto<TestMessage>("{\"repeated_bool_value\": true}"),
+              StatusIs(absl::StatusCode::kOk));
+
+  ParseOptions options;
+  options.allow_legacy_nonconformant_behavior = false;
+  EXPECT_THAT(ToProto<TestMessage>("{\"repeated_bool_value\": true}", options),
+              StatusIs(absl::StatusCode::kInvalidArgument));
+}
+
 TEST_P(JsonTest, TestPreserveProtoFieldNames) {
   TestMessage m;
   m.mutable_message_value();