Fix UTF-8 Validation of string extensions in C++

Also adds a regression test

PiperOrigin-RevId: 862026607

Author: protobuf-github-bot

conformance/failure_list_cpp.txt

@@ -35,7 +35,6 @@ Recommended.*.JsonInput.FieldMaskInvalidCharacter
 Required.*.JsonInput.SingleValueForRepeatedFieldInt32                                                              # Should have failed to parse, but didn't.
 Required.*.JsonInput.SingleValueForRepeatedFieldMessage                                                            # Should have failed to parse, but didn't.
 Required.*.ProtobufInput.BadTag_FieldNumberSlightlyTooHigh                                                         # Should have failed to parse, but didn't.
-Recommended.Editions.ProtobufInput.RejectInvalidUtf8.String.Extension                                              # Should have failed to parse, but didn't.
 # TODO: Uncomment once conformance tests can express failures that are not expected to be fixed.
 # Recommended.Editions_Proto2.ProtobufInput.RejectInvalidUtf8.String.MapKey                                          # Should have failed to parse, but didn't.
 # Recommended.Editions_Proto2.ProtobufInput.RejectInvalidUtf8.String.MapValue                                        # Should have failed to parse, but didn't.

conformance/failure_list_rust_cc.txt

@@ -1,5 +1,4 @@
 Required.*.ProtobufInput.BadTag_FieldNumberSlightlyTooHigh                                                        # Should have failed to parse, but didn't.
-Recommended.Editions.ProtobufInput.RejectInvalidUtf8.String.Extension                                             # Should have failed to parse, but didn't.
 # TODO: Uncomment once conformance tests can express failures that are not expected to be fixed.
 # Recommended.Editions_Proto2.ProtobufInput.RejectInvalidUtf8.String.MapKey                                         # Should have failed to parse, but didn't.
 # Recommended.Editions_Proto2.ProtobufInput.RejectInvalidUtf8.String.MapValue.                                      # Should have failed to parse, but didn't.

src/google/protobuf/BUILD.bazel

@@ -1090,6 +1090,7 @@ filegroup(
         "unittest_redaction.proto",
         "unittest_retention.proto",
         "unittest_string_type.proto",
+        "unittest_utf8_string_extensions.proto",
         "unittest_well_known_types.proto",
     ],
     visibility = ["//:__subpackages__"],
@@ -1182,6 +1183,7 @@ protobuf_test_proto_library(
         "unittest_retention.proto",
         "unittest_string_type.proto",
         "unittest_string_view.proto",
+        "unittest_utf8_string_extensions.proto",
         "unittest_well_known_types.proto",
     ],
     option_deps = [
@@ -1858,13 +1860,15 @@ cc_test(
         ":port",
         ":protobuf",
         ":protobuf_lite",
+        ":test_textproto",
         ":test_util",
         ":test_util2",
         "//src/google/protobuf/io",
         "//src/google/protobuf/stubs",
         "//src/google/protobuf/testing",
         "//src/google/protobuf/testing:file",
         "//src/google/protobuf/util:differencer",
+        "//third_party/utf8_range:utf8_validity",
         "@abseil-cpp//absl/algorithm:container",
         "@abseil-cpp//absl/log:absl_check",
         "@abseil-cpp//absl/strings",

src/google/protobuf/compiler/cpp/extension.cc

@@ -237,6 +237,9 @@ void ExtensionGenerator::GenerateRegistration(io::Printer* p,
        DescriptorTableName(descriptor_->containing_type()->file(), options_)},
       {"extendee_index", find_index(descriptor_->containing_type())},
       {"preregister", priority == kInitPriority101},
+      {"is_utf8", descriptor_->requires_utf8_validation()
+                      ? "/*is_utf8=*/true"
+                      : "/*is_utf8=*/false"},
   }});
   switch (descriptor_->cpp_type()) {
     case FieldDescriptor::CPPTYPE_ENUM:
@@ -331,7 +334,7 @@ void ExtensionGenerator::GenerateRegistration(io::Printer* p,
             R"cc(
               ::_pbi::ExtensionSet::RegisterExtension(
                   &$extendee$::default_instance(), $number$, $field_type$,
-                  $repeated$, $packed$),
+                  $repeated$, $packed$, $is_utf8$),
             )cc");
       }
 

src/google/protobuf/extension_set.cc

@@ -135,11 +135,11 @@ bool GeneratedExtensionFinder::Find(int number, ExtensionInfo* output) {
 
 void ExtensionSet::RegisterExtension(const MessageLite* extendee, int number,
                                      FieldType type, bool is_repeated,
-                                     bool is_packed) {
+                                     bool is_packed, bool is_utf8) {
   ABSL_CHECK_NE(type, WireFormatLite::TYPE_ENUM);
   ABSL_CHECK_NE(type, WireFormatLite::TYPE_MESSAGE);
   ABSL_CHECK_NE(type, WireFormatLite::TYPE_GROUP);
-  ExtensionInfo info(extendee, number, type, is_repeated, is_packed);
+  ExtensionInfo info(extendee, number, type, is_repeated, is_packed, is_utf8);
   Register(info);
 }
 
@@ -148,7 +148,8 @@ void ExtensionSet::RegisterEnumExtension(const MessageLite* extendee,
                                          bool is_repeated, bool is_packed,
                                          const uint32_t* validation_data) {
   ABSL_CHECK_EQ(type, WireFormatLite::TYPE_ENUM);
-  ExtensionInfo info(extendee, number, type, is_repeated, is_packed);
+  ExtensionInfo info(extendee, number, type, is_repeated, is_packed,
+                     /*is_utf8=*/false);
   info.enum_validity_check.func = nullptr;
   info.enum_validity_check.arg = validation_data;
   Register(info);

src/google/protobuf/extension_set.h

@@ -122,14 +122,17 @@ enum class LazyAnnotation : int8_t {
 
 // Information about a registered extension.
 struct ExtensionInfo {
-  constexpr ExtensionInfo() : enum_validity_check() {}
+  constexpr ExtensionInfo()
+      : is_packed(false), is_utf8(false), enum_validity_check() {}
   constexpr ExtensionInfo(const MessageLite* extendee, int param_number,
-                          FieldType type_param, bool isrepeated, bool ispacked)
+                          FieldType type_param, bool isrepeated, bool ispacked,
+                          bool is_utf8)
       : message(extendee),
         number(param_number),
         type(type_param),
         is_repeated(isrepeated),
         is_packed(ispacked),
+        is_utf8(is_utf8),
         enum_validity_check() {}
   constexpr ExtensionInfo(const MessageLite* extendee, int param_number,
                           FieldType type_param, bool isrepeated, bool ispacked,
@@ -140,6 +143,7 @@ struct ExtensionInfo {
         type(type_param),
         is_repeated(isrepeated),
         is_packed(ispacked),
+        is_utf8(false),
         is_lazy(islazy),
         enum_validity_check(),
         lazy_eager_verify_func(verify_func)
@@ -151,7 +155,8 @@ struct ExtensionInfo {
 
   FieldType type = 0;
   bool is_repeated = false;
-  bool is_packed = false;
+  bool is_packed : 1;
+  bool is_utf8 : 1;  // validate UTF8 if true
   LazyAnnotation is_lazy = LazyAnnotation::kUndefined;
 
   struct EnumValidityCheck {
@@ -275,7 +280,7 @@ class PROTOBUF_EXPORT ExtensionSet {
   // methods do.
   static void RegisterExtension(const MessageLite* extendee, int number,
                                 FieldType type, bool is_repeated,
-                                bool is_packed);
+                                bool is_packed, bool is_utf8 = false);
   static void RegisterEnumExtension(const MessageLite* extendee, int number,
                                     FieldType type, bool is_repeated,
                                     bool is_packed,

src/google/protobuf/extension_set_inl.h

@@ -12,10 +12,12 @@
 #include <string>
 #include <utility>
 
+#include "absl/base/optimization.h"
 #include "google/protobuf/extension_set.h"
 #include "google/protobuf/metadata_lite.h"
 #include "google/protobuf/parse_context.h"
 #include "google/protobuf/wire_format_lite.h"
+#include "utf8_validity.h"
 
 namespace google {
 namespace protobuf {
@@ -150,7 +152,15 @@ const char* ExtensionSet::ParseFieldWithExtensionInfo(
                                 info.descriptor);
         int size = ReadSize(&ptr);
         GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
-        return ctx->ReadString(ptr, size, value);
+        if (info.is_utf8) {
+          ptr = ctx->ReadString(ptr, size, value);
+          if ABSL_PREDICT_FALSE (!utf8_range::IsStructurallyValid(*value)) {
+            return nullptr;
+          }
+          return ptr;
+        } else {
+          return ctx->ReadString(ptr, size, value);
+        }
       }
 
       case WireFormatLite::TYPE_GROUP: {

src/google/protobuf/extension_set_unittest.cc

@@ -44,9 +44,12 @@
 #include "google/protobuf/unittest_mset.pb.h"
 #include "google/protobuf/unittest_mset_wire_format.pb.h"
 #include "google/protobuf/unittest_proto3_extensions.pb.h"
+#include "google/protobuf/unittest_utf8_string_extensions.pb.h"
 #include "google/protobuf/wire_format.h"
 #include "google/protobuf/wire_format_lite.h"
+#include "utf8_validity.h"
 
+#include "google/protobuf/test_textproto.h"
 
 // Must be included last.
 #include "google/protobuf/port_def.inc"
@@ -1975,6 +1978,55 @@ INSTANTIATE_TEST_SUITE_P(
       return name;
     });
 
+TEST(ExtensionSet, StringWithInvalidUTF8FailsToParse) {
+  // Sanity check that the extension field is marked as requiring UTF-8
+  // validation. `requires_utf8_validation` can only be true for string fields
+  // where feature.utf8_validation = VERIFY.
+  const google::protobuf::DescriptorPool* pool = google::protobuf::DescriptorPool::generated_pool();
+  ASSERT_NE(pool, nullptr);
+  const google::protobuf::FieldDescriptor* string_ext_fd = pool->FindExtensionByName(
+      "proto2_unittest.optional_utf8_string_extension");
+  ASSERT_NE(string_ext_fd, nullptr);
+  ASSERT_TRUE(string_ext_fd->requires_utf8_validation());
+  std::string invalid_utf8 = "\xFF";
+  ASSERT_FALSE(utf8_range::IsStructurallyValid(invalid_utf8));
+
+  proto2_unittest::TestUtf8ValidationOfExtensions test_message;
+  // It is reasonable to debate that UTF-8 validation should be checked in the
+  // setter, but it is not currently done because the setter doesn't have a way
+  // to report errors.
+  test_message.SetExtension(proto2_unittest::optional_utf8_string_extension,
+                            invalid_utf8);
+  std::string data;
+  ASSERT_TRUE(test_message.SerializeToString(&data));
+  proto2_unittest::TestUtf8ValidationOfExtensions parsed_message;
+  ASSERT_FALSE(parsed_message.ParseFromString(data));
+}
+
+TEST(ExtensionSet, BytesWithInvalidUTF8Succeeds) {
+  // Sanity check that the extension field is not marked as requiring UTF-8
+  // validation. `requires_utf8_validation` can only be true for string fields.
+  const google::protobuf::DescriptorPool* pool = google::protobuf::DescriptorPool::generated_pool();
+  ASSERT_NE(pool, nullptr);
+  const google::protobuf::FieldDescriptor* string_ext_fd =
+      pool->FindExtensionByName("proto2_unittest.optional_bytes_extension");
+  ASSERT_NE(string_ext_fd, nullptr);
+  ASSERT_FALSE(string_ext_fd->requires_utf8_validation());
+  std::string invalid_utf8 = "\xFF";
+  ASSERT_FALSE(utf8_range::IsStructurallyValid(invalid_utf8));
+
+  proto2_unittest::TestAllExtensions test_message;
+  test_message.SetExtension(proto2_unittest::optional_bytes_extension,
+                            invalid_utf8);
+  std::string data;
+  ASSERT_TRUE(test_message.SerializeToString(&data));
+  proto2_unittest::TestAllExtensions parsed_message;
+  EXPECT_TRUE(parsed_message.ParseFromString(data));
+  EXPECT_THAT(parsed_message, google::protobuf::EqualsProto(R"pb(
+                [proto2_unittest.optional_bytes_extension]: "\xFF"
+              )pb"));
+}
+
 }  // namespace
 }  // namespace internal
 }  // namespace protobuf