From 6fa3f2d79bcb5518e3453687fe10e3bbd16cbfe6 Mon Sep 17 00:00:00 2001 From: zhangskz Date: Wed, 18 Sep 2024 14:44:36 -0400 Subject: [PATCH] Fix cord handling in DynamicMessage and oneofs. (#18374) * Fix cord handling in DynamicMessage and oneofs. This fixes a memory corruption vulnerability for anyone using cord with dynamically built descriptor pools. * Move -Werror to our test/dev bazelrc files. (#17938) * Move -Werror to our test/dev bazelrc files. Putting it into BUILD files unintentionally forces it on all our downstream users. Instead, we just want to enable this during testing and let them choose for themselves in their builds. Note, that this expands the scope of -Werror to our entire repo for CI, so a bunch of fixes and opt-outs had to be applied to get this change passing. Closed #14714 PiperOrigin-RevId: 666903224 * Fix extra warnings on 28.x * Fix zlib issues on macos * Second try at zlib/macos issues * Only disable deprecated-non-prototype on macos-14 * Silence expected ubsan failures from absl::Cord * Fix test dependency * Fix python/upb warnings * Fix last upb warning --------- Co-authored-by: Mike Kruskal --- .bazelrc | 5 + .github/workflows/test_objectivec.yml | 3 +- .github/workflows/test_rust.yml | 3 +- .github/workflows/test_upb.yml | 2 +- build_defs/cpp_opts.bzl | 1 - ci/Linux.bazelrc | 1 + ci/common.bazelrc | 2 + ci/macOS.bazelrc | 1 + conformance/binary_json_conformance_suite.cc | 7 +- .../protobuf/CodedOutputStreamTest.java | 42 -- .../java/com/google/protobuf/TestUtil.java | 381 +++++++++++++----- .../com/google/protobuf/TextFormatTest.java | 39 +- .../protobuf/internal/descriptor_test.py | 2 +- .../protobuf/internal/field_mask_test.py | 2 +- .../google/protobuf/internal/message_test.py | 63 +-- python/google/protobuf/internal/test_util.py | 3 + .../protobuf/internal/unknown_fields_test.py | 6 +- python/map.c | 10 - python/message.c | 1 - python/protobuf.c | 2 +- ruby/ext/google/protobuf_c/convert.c | 3 +- src/google/protobuf/BUILD.bazel | 3 +- src/google/protobuf/arena_test_util.h | 40 -- src/google/protobuf/arena_unittest.cc | 7 - src/google/protobuf/compiler/BUILD.bazel | 1 + .../command_line_interface_unittest.cc | 28 +- src/google/protobuf/compiler/cpp/field.cc | 43 +- src/google/protobuf/compiler/cpp/field.h | 5 - .../cpp/field_generators/string_view_field.cc | 2 - src/google/protobuf/compiler/cpp/helpers.cc | 3 +- src/google/protobuf/compiler/cpp/tracker.cc | 10 +- src/google/protobuf/compiler/cpp/unittest.inc | 4 + src/google/protobuf/descriptor.cc | 23 ++ src/google/protobuf/descriptor.h | 19 +- .../protobuf/descriptor_database_unittest.cc | 8 +- src/google/protobuf/descriptor_lite.h | 11 + src/google/protobuf/descriptor_unittest.cc | 74 +++- src/google/protobuf/dynamic_message.cc | 72 +++- src/google/protobuf/edition_unittest.proto | 2 + .../protobuf/generated_message_reflection.cc | 213 ++++++---- .../protobuf/generated_message_tctable_gen.cc | 26 +- .../generated_message_tctable_lite.cc | 6 + src/google/protobuf/io/BUILD.bazel | 5 +- .../protobuf/io/zero_copy_stream_unittest.cc | 10 +- src/google/protobuf/lite_unittest.cc | 24 ++ src/google/protobuf/map_test.inc | 63 ++- src/google/protobuf/message.cc | 8 +- src/google/protobuf/message_unittest.inc | 18 +- src/google/protobuf/string_view_test.cc | 7 + src/google/protobuf/test_util.h | 17 + src/google/protobuf/test_util.inc | 12 + src/google/protobuf/testdata/golden_message | Bin 537 -> 0 bytes .../protobuf/testdata/golden_message_maps | Bin 13619 -> 0 bytes .../testdata/golden_message_oneof_implemented | Bin 521 -> 0 bytes .../protobuf/testdata/golden_message_proto3 | Bin 248 -> 0 bytes .../testdata/golden_packed_fields_message | Bin 142 -> 0 bytes .../testdata/text_format_unittest_data.txt | 1 + ...format_unittest_data_oneof_implemented.txt | 1 + .../text_format_unittest_data_pointy.txt | 1 + ...text_format_unittest_data_pointy_oneof.txt | 1 + .../text_format_unittest_extensions_data.txt | 1 + ...format_unittest_extensions_data_pointy.txt | 1 + src/google/protobuf/unittest.proto | 2 + src/google/protobuf/unittest_lite.proto | 2 + .../protobuf/unittest_proto3_arena.proto | 1 + .../protobuf/util/field_mask_util_test.cc | 2 +- src/google/protobuf/wire_format.cc | 8 +- third_party/zlib.BUILD | 4 + upb/io/zero_copy_stream_test.cc | 35 -- upb/wire/eps_copy_input_stream_test.cc | 2 +- 70 files changed, 822 insertions(+), 583 deletions(-) delete mode 100644 src/google/protobuf/testdata/golden_message delete mode 100644 src/google/protobuf/testdata/golden_message_maps delete mode 100644 src/google/protobuf/testdata/golden_message_oneof_implemented delete mode 100644 src/google/protobuf/testdata/golden_message_proto3 delete mode 100644 src/google/protobuf/testdata/golden_packed_fields_message diff --git a/.bazelrc b/.bazelrc index 65cc8c491e523..e1e5a8ba85bfb 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,5 +1,8 @@ build --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 +build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations" + + build:dbg --compilation_mode=dbg build:opt --compilation_mode=opt @@ -22,6 +25,8 @@ build:ubsan --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 # Workaround for the fact that Bazel links with $CC, not $CXX # https://github.com/bazelbuild/bazel/issues/11122#issuecomment-613746748 build:ubsan --copt=-fno-sanitize=function --copt=-fno-sanitize=vptr +# Abseil passes nullptr to memcmp with 0 size +build:ubsan --copt=-fno-sanitize=nonnull-attribute # TODO: migrate all dependencies from WORKSPACE to MODULE.bazel # https://github.com/protocolbuffers/protobuf/issues/14313 diff --git a/.github/workflows/test_objectivec.yml b/.github/workflows/test_objectivec.yml index 1db6c10c88abb..ab3a5af4b37ce 100644 --- a/.github/workflows/test_objectivec.yml +++ b/.github/workflows/test_objectivec.yml @@ -76,6 +76,7 @@ jobs: - OS: macos-14 PLATFORM: "visionos" XCODE: "15.2" + EXTRA_FLAGS: --copt="-Wno-deprecated-non-prototype" name: CocoaPods ${{ matrix.PLATFORM }} ${{ matrix.CONFIGURATION }} runs-on: ${{ matrix.OS }} steps: @@ -91,7 +92,7 @@ jobs: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} bazel-cache: cocoapods/${{ matrix.XCODE }} bash: | - ./regenerate_stale_files.sh $BAZEL_FLAGS --xcode_version="${{ matrix.XCODE }}" + ./regenerate_stale_files.sh $BAZEL_FLAGS ${{ matrix.EXTRA_FLAGS }} --xcode_version="${{ matrix.XCODE }}" pod lib lint --verbose \ --configuration=${{ matrix.CONFIGURATION }} \ --platforms=${{ matrix.PLATFORM }} \ diff --git a/.github/workflows/test_rust.yml b/.github/workflows/test_rust.yml index cab8c4d8744d0..790e78df9d7de 100644 --- a/.github/workflows/test_rust.yml +++ b/.github/workflows/test_rust.yml @@ -27,6 +27,7 @@ jobs: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} bazel-cache: rust_linux bazel: >- - test //rust:protobuf_upb_test //rust:protobuf_cpp_test + test --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 --copt="-Wno-return-type-c-linkage" + //rust:protobuf_upb_test //rust:protobuf_cpp_test //rust/test/rust_proto_library_unit_test:rust_upb_aspect_test //src/google/protobuf/compiler/rust/... diff --git a/.github/workflows/test_upb.yml b/.github/workflows/test_upb.yml index 3ae8705d53208..d09ec59f759c5 100644 --- a/.github/workflows/test_upb.yml +++ b/.github/workflows/test_upb.yml @@ -59,7 +59,7 @@ jobs: image: "us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:12.2-6.3.0-63dd26c0c7a808d92673a3e52e848189d4ab0f17" credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} bazel-cache: "upb-bazel-gcc" - bazel: test --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 -c opt //bazel/... //benchmarks/... //lua/... //protos/... //protos_generator/... //python/... //upb/... //upb_generator/... + bazel: test --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 -c opt --copt="-Wno-error=maybe-uninitialized" --copt="-Wno-error=attributes" //bazel/... //benchmarks/... //lua/... //protos/... //protos_generator/... //python/... //upb/... //upb_generator/... windows: strategy: diff --git a/build_defs/cpp_opts.bzl b/build_defs/cpp_opts.bzl index 46b60252f62ba..d8646d1681fe6 100644 --- a/build_defs/cpp_opts.bzl +++ b/build_defs/cpp_opts.bzl @@ -21,7 +21,6 @@ COPTS = select({ "-Woverloaded-virtual", "-Wno-sign-compare", "-Wno-nonnull", - "-Werror", ], }) diff --git a/ci/Linux.bazelrc b/ci/Linux.bazelrc index d5dcf5de005d2..b4ec98f8c7965 100644 --- a/ci/Linux.bazelrc +++ b/ci/Linux.bazelrc @@ -1,3 +1,4 @@ import common.bazelrc build --cxxopt=-std=c++14 --host_cxxopt=-std=c++14 +build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations" diff --git a/ci/common.bazelrc b/ci/common.bazelrc index b0501d5d202c5..ae483fa7cdfeb 100644 --- a/ci/common.bazelrc +++ b/ci/common.bazelrc @@ -27,6 +27,8 @@ build:ubsan --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 # Workaround for the fact that Bazel links with $CC, not $CXX # https://github.com/bazelbuild/bazel/issues/11122#issuecomment-613746748 build:ubsan --copt=-fno-sanitize=function --copt=-fno-sanitize=vptr +# Abseil passes nullptr to memcmp with 0 size +build:ubsan --copt=-fno-sanitize=nonnull-attribute # Workaround Bazel 7 remote cache issues. # See https://github.com/bazelbuild/bazel/issues/20161 diff --git a/ci/macOS.bazelrc b/ci/macOS.bazelrc index 465bf0957fdee..284ae690d9546 100644 --- a/ci/macOS.bazelrc +++ b/ci/macOS.bazelrc @@ -1,4 +1,5 @@ import common.bazelrc build --cxxopt=-std=c++14 --host_cxxopt=-std=c++14 +build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations" common --repo_env=BAZEL_NO_APPLE_CPP_TOOLCHAIN=1 diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc index f8430bceeadac..85c8e24b0beea 100644 --- a/conformance/binary_json_conformance_suite.cc +++ b/conformance/binary_json_conformance_suite.cc @@ -3449,12 +3449,11 @@ BinaryAndJsonConformanceSuiteImpl::GetFieldForOneofType( template std::string BinaryAndJsonConformanceSuiteImpl::SyntaxIdentifier() const { - if constexpr (std::is_same::value) { + if (std::is_same::value) { return "Proto2"; - } else if constexpr (std::is_same::value) { + } else if (std::is_same::value) { return "Proto3"; - } else if constexpr (std::is_same::value) { + } else if (std::is_same::value) { return "Editions_Proto2"; } else { return "Editions_Proto3"; diff --git a/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java b/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java index 51e66b9759ff4..1f17f24fe3c78 100644 --- a/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java +++ b/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java @@ -13,7 +13,6 @@ import com.google.protobuf.CodedOutputStream.OutOfSpaceException; import protobuf_unittest.UnittestProto.SparseEnumMessage; import protobuf_unittest.UnittestProto.TestAllTypes; -import protobuf_unittest.UnittestProto.TestPackedTypes; import protobuf_unittest.UnittestProto.TestSparseEnum; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -402,47 +401,6 @@ public void computeTagSize() { assertThat(CodedOutputStream.computeTagSize((1 << 30) + 1)).isEqualTo(1); } - /** Tests writing a whole message with every field type. */ - @Test - public void testWriteWholeMessage() throws Exception { - final byte[] expectedBytes = TestUtil.getGoldenMessage().toByteArray(); - TestAllTypes message = TestUtil.getAllSet(); - - for (OutputType outputType : OutputType.values()) { - Coder coder = outputType.newCoder(message.getSerializedSize()); - message.writeTo(coder.stream()); - coder.stream().flush(); - byte[] rawBytes = coder.toByteArray(); - assertEqualBytes(outputType, expectedBytes, rawBytes); - } - - // Try different block sizes. - for (int blockSize = 1; blockSize < 256; blockSize *= 2) { - Coder coder = OutputType.STREAM.newCoder(blockSize); - message.writeTo(coder.stream()); - coder.stream().flush(); - assertEqualBytes(OutputType.STREAM, expectedBytes, coder.toByteArray()); - } - } - - /** - * Tests writing a whole message with every packed field type. Ensures the wire format of packed - * fields is compatible with C++. - */ - @Test - public void testWriteWholePackedFieldsMessage() throws Exception { - byte[] expectedBytes = TestUtil.getGoldenPackedFieldsMessage().toByteArray(); - TestPackedTypes message = TestUtil.getPackedSet(); - - for (OutputType outputType : OutputType.values()) { - Coder coder = outputType.newCoder(message.getSerializedSize()); - message.writeTo(coder.stream()); - coder.stream().flush(); - byte[] rawBytes = coder.toByteArray(); - assertEqualBytes(outputType, expectedBytes, rawBytes); - } - } - /** * Test writing a message containing a negative enum value. This used to fail because the size was * not properly computed as a sign-extended varint. diff --git a/java/core/src/test/java/com/google/protobuf/TestUtil.java b/java/core/src/test/java/com/google/protobuf/TestUtil.java index 0f57ecc9acca2..598a086f71b6d 100644 --- a/java/core/src/test/java/com/google/protobuf/TestUtil.java +++ b/java/core/src/test/java/com/google/protobuf/TestUtil.java @@ -210,10 +210,6 @@ import protobuf_unittest.UnittestProto.TestPackedTypes; import protobuf_unittest.UnittestProto.TestRequired; import protobuf_unittest.UnittestProto.TestUnpackedTypes; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.io.RandomAccessFile; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -234,6 +230,281 @@ public final class TestUtil { private TestUtil() {} + public static final String ALL_FIELDS_SET_TEXT = + "" + + "optional_int32: 101\n" + + "optional_int64: 102\n" + + "optional_uint32: 103\n" + + "optional_uint64: 104\n" + + "optional_sint32: 105\n" + + "optional_sint64: 106\n" + + "optional_fixed32: 107\n" + + "optional_fixed64: 108\n" + + "optional_sfixed32: 109\n" + + "optional_sfixed64: 110\n" + + "optional_float: 111.0\n" + + "optional_double: 112.0\n" + + "optional_bool: true\n" + + "optional_string: \"115\"\n" + + "optional_bytes: \"116\"\n" + + "OptionalGroup {\n" + + " a: 117\n" + + "}\n" + + "optional_nested_message {\n" + + " bb: 118\n" + + "}\n" + + "optional_foreign_message {\n" + + " c: 119\n" + + "}\n" + + "optional_import_message {\n" + + " d: 120\n" + + "}\n" + + "optional_nested_enum: BAZ\n" + + "optional_foreign_enum: FOREIGN_BAZ\n" + + "optional_import_enum: IMPORT_BAZ\n" + + "optional_string_piece: \"124\"\n" + + "optional_cord: \"125\"\n" + + "optional_public_import_message {\n" + + " e: 126\n" + + "}\n" + + "optional_lazy_message {\n" + + " bb: 127\n" + + "}\n" + + "optional_unverified_lazy_message {\n" + + " bb: 128\n" + + "}\n" + + "repeated_int32: 201\n" + + "repeated_int32: 301\n" + + "repeated_int64: 202\n" + + "repeated_int64: 302\n" + + "repeated_uint32: 203\n" + + "repeated_uint32: 303\n" + + "repeated_uint64: 204\n" + + "repeated_uint64: 304\n" + + "repeated_sint32: 205\n" + + "repeated_sint32: 305\n" + + "repeated_sint64: 206\n" + + "repeated_sint64: 306\n" + + "repeated_fixed32: 207\n" + + "repeated_fixed32: 307\n" + + "repeated_fixed64: 208\n" + + "repeated_fixed64: 308\n" + + "repeated_sfixed32: 209\n" + + "repeated_sfixed32: 309\n" + + "repeated_sfixed64: 210\n" + + "repeated_sfixed64: 310\n" + + "repeated_float: 211.0\n" + + "repeated_float: 311.0\n" + + "repeated_double: 212.0\n" + + "repeated_double: 312.0\n" + + "repeated_bool: true\n" + + "repeated_bool: false\n" + + "repeated_string: \"215\"\n" + + "repeated_string: \"315\"\n" + + "repeated_bytes: \"216\"\n" + + "repeated_bytes: \"316\"\n" + + "RepeatedGroup {\n" + + " a: 217\n" + + "}\n" + + "RepeatedGroup {\n" + + " a: 317\n" + + "}\n" + + "repeated_nested_message {\n" + + " bb: 218\n" + + "}\n" + + "repeated_nested_message {\n" + + " bb: 318\n" + + "}\n" + + "repeated_foreign_message {\n" + + " c: 219\n" + + "}\n" + + "repeated_foreign_message {\n" + + " c: 319\n" + + "}\n" + + "repeated_import_message {\n" + + " d: 220\n" + + "}\n" + + "repeated_import_message {\n" + + " d: 320\n" + + "}\n" + + "repeated_nested_enum: BAR\n" + + "repeated_nested_enum: BAZ\n" + + "repeated_foreign_enum: FOREIGN_BAR\n" + + "repeated_foreign_enum: FOREIGN_BAZ\n" + + "repeated_import_enum: IMPORT_BAR\n" + + "repeated_import_enum: IMPORT_BAZ\n" + + "repeated_string_piece: \"224\"\n" + + "repeated_string_piece: \"324\"\n" + + "repeated_cord: \"225\"\n" + + "repeated_cord: \"325\"\n" + + "repeated_lazy_message {\n" + + " bb: 227\n" + + "}\n" + + "repeated_lazy_message {\n" + + " bb: 327\n" + + "}\n" + + "default_int32: 401\n" + + "default_int64: 402\n" + + "default_uint32: 403\n" + + "default_uint64: 404\n" + + "default_sint32: 405\n" + + "default_sint64: 406\n" + + "default_fixed32: 407\n" + + "default_fixed64: 408\n" + + "default_sfixed32: 409\n" + + "default_sfixed64: 410\n" + + "default_float: 411.0\n" + + "default_double: 412.0\n" + + "default_bool: false\n" + + "default_string: \"415\"\n" + + "default_bytes: \"416\"\n" + + "default_nested_enum: FOO\n" + + "default_foreign_enum: FOREIGN_FOO\n" + + "default_import_enum: IMPORT_FOO\n" + + "default_string_piece: \"424\"\n" + + "default_cord: \"425\"\n" + + "oneof_bytes: \"604\"\n"; + + public static final String ALL_EXTENSIONS_SET_TEXT = + "" + + "[protobuf_unittest.optional_int32_extension]: 101\n" + + "[protobuf_unittest.optional_int64_extension]: 102\n" + + "[protobuf_unittest.optional_uint32_extension]: 103\n" + + "[protobuf_unittest.optional_uint64_extension]: 104\n" + + "[protobuf_unittest.optional_sint32_extension]: 105\n" + + "[protobuf_unittest.optional_sint64_extension]: 106\n" + + "[protobuf_unittest.optional_fixed32_extension]: 107\n" + + "[protobuf_unittest.optional_fixed64_extension]: 108\n" + + "[protobuf_unittest.optional_sfixed32_extension]: 109\n" + + "[protobuf_unittest.optional_sfixed64_extension]: 110\n" + + "[protobuf_unittest.optional_float_extension]: 111.0\n" + + "[protobuf_unittest.optional_double_extension]: 112.0\n" + + "[protobuf_unittest.optional_bool_extension]: true\n" + + "[protobuf_unittest.optional_string_extension]: \"115\"\n" + + "[protobuf_unittest.optional_bytes_extension]: \"116\"\n" + + "[protobuf_unittest.optionalgroup_extension] {\n" + + " a: 117\n" + + "}\n" + + "[protobuf_unittest.optional_nested_message_extension] {\n" + + " bb: 118\n" + + "}\n" + + "[protobuf_unittest.optional_foreign_message_extension] {\n" + + " c: 119\n" + + "}\n" + + "[protobuf_unittest.optional_import_message_extension] {\n" + + " d: 120\n" + + "}\n" + + "[protobuf_unittest.optional_nested_enum_extension]: BAZ\n" + + "[protobuf_unittest.optional_foreign_enum_extension]: FOREIGN_BAZ\n" + + "[protobuf_unittest.optional_import_enum_extension]: IMPORT_BAZ\n" + + "[protobuf_unittest.optional_string_piece_extension]: \"124\"\n" + + "[protobuf_unittest.optional_cord_extension]: \"125\"\n" + + "[protobuf_unittest.optional_public_import_message_extension] {\n" + + " e: 126\n" + + "}\n" + + "[protobuf_unittest.optional_lazy_message_extension] {\n" + + " bb: 127\n" + + "}\n" + + "[protobuf_unittest.optional_unverified_lazy_message_extension] {\n" + + " bb: 128\n" + + "}\n" + + "[protobuf_unittest.repeated_int32_extension]: 201\n" + + "[protobuf_unittest.repeated_int32_extension]: 301\n" + + "[protobuf_unittest.repeated_int64_extension]: 202\n" + + "[protobuf_unittest.repeated_int64_extension]: 302\n" + + "[protobuf_unittest.repeated_uint32_extension]: 203\n" + + "[protobuf_unittest.repeated_uint32_extension]: 303\n" + + "[protobuf_unittest.repeated_uint64_extension]: 204\n" + + "[protobuf_unittest.repeated_uint64_extension]: 304\n" + + "[protobuf_unittest.repeated_sint32_extension]: 205\n" + + "[protobuf_unittest.repeated_sint32_extension]: 305\n" + + "[protobuf_unittest.repeated_sint64_extension]: 206\n" + + "[protobuf_unittest.repeated_sint64_extension]: 306\n" + + "[protobuf_unittest.repeated_fixed32_extension]: 207\n" + + "[protobuf_unittest.repeated_fixed32_extension]: 307\n" + + "[protobuf_unittest.repeated_fixed64_extension]: 208\n" + + "[protobuf_unittest.repeated_fixed64_extension]: 308\n" + + "[protobuf_unittest.repeated_sfixed32_extension]: 209\n" + + "[protobuf_unittest.repeated_sfixed32_extension]: 309\n" + + "[protobuf_unittest.repeated_sfixed64_extension]: 210\n" + + "[protobuf_unittest.repeated_sfixed64_extension]: 310\n" + + "[protobuf_unittest.repeated_float_extension]: 211.0\n" + + "[protobuf_unittest.repeated_float_extension]: 311.0\n" + + "[protobuf_unittest.repeated_double_extension]: 212.0\n" + + "[protobuf_unittest.repeated_double_extension]: 312.0\n" + + "[protobuf_unittest.repeated_bool_extension]: true\n" + + "[protobuf_unittest.repeated_bool_extension]: false\n" + + "[protobuf_unittest.repeated_string_extension]: \"215\"\n" + + "[protobuf_unittest.repeated_string_extension]: \"315\"\n" + + "[protobuf_unittest.repeated_bytes_extension]: \"216\"\n" + + "[protobuf_unittest.repeated_bytes_extension]: \"316\"\n" + + "[protobuf_unittest.repeatedgroup_extension] {\n" + + " a: 217\n" + + "}\n" + + "[protobuf_unittest.repeatedgroup_extension] {\n" + + " a: 317\n" + + "}\n" + + "[protobuf_unittest.repeated_nested_message_extension] {\n" + + " bb: 218\n" + + "}\n" + + "[protobuf_unittest.repeated_nested_message_extension] {\n" + + " bb: 318\n" + + "}\n" + + "[protobuf_unittest.repeated_foreign_message_extension] {\n" + + " c: 219\n" + + "}\n" + + "[protobuf_unittest.repeated_foreign_message_extension] {\n" + + " c: 319\n" + + "}\n" + + "[protobuf_unittest.repeated_import_message_extension] {\n" + + " d: 220\n" + + "}\n" + + "[protobuf_unittest.repeated_import_message_extension] {\n" + + " d: 320\n" + + "}\n" + + "[protobuf_unittest.repeated_nested_enum_extension]: BAR\n" + + "[protobuf_unittest.repeated_nested_enum_extension]: BAZ\n" + + "[protobuf_unittest.repeated_foreign_enum_extension]: FOREIGN_BAR\n" + + "[protobuf_unittest.repeated_foreign_enum_extension]: FOREIGN_BAZ\n" + + "[protobuf_unittest.repeated_import_enum_extension]: IMPORT_BAR\n" + + "[protobuf_unittest.repeated_import_enum_extension]: IMPORT_BAZ\n" + + "[protobuf_unittest.repeated_string_piece_extension]: \"224\"\n" + + "[protobuf_unittest.repeated_string_piece_extension]: \"324\"\n" + + "[protobuf_unittest.repeated_cord_extension]: \"225\"\n" + + "[protobuf_unittest.repeated_cord_extension]: \"325\"\n" + + "[protobuf_unittest.repeated_lazy_message_extension] {\n" + + " bb: 227\n" + + "}\n" + + "[protobuf_unittest.repeated_lazy_message_extension] {\n" + + " bb: 327\n" + + "}\n" + + "[protobuf_unittest.default_int32_extension]: 401\n" + + "[protobuf_unittest.default_int64_extension]: 402\n" + + "[protobuf_unittest.default_uint32_extension]: 403\n" + + "[protobuf_unittest.default_uint64_extension]: 404\n" + + "[protobuf_unittest.default_sint32_extension]: 405\n" + + "[protobuf_unittest.default_sint64_extension]: 406\n" + + "[protobuf_unittest.default_fixed32_extension]: 407\n" + + "[protobuf_unittest.default_fixed64_extension]: 408\n" + + "[protobuf_unittest.default_sfixed32_extension]: 409\n" + + "[protobuf_unittest.default_sfixed64_extension]: 410\n" + + "[protobuf_unittest.default_float_extension]: 411.0\n" + + "[protobuf_unittest.default_double_extension]: 412.0\n" + + "[protobuf_unittest.default_bool_extension]: false\n" + + "[protobuf_unittest.default_string_extension]: \"415\"\n" + + "[protobuf_unittest.default_bytes_extension]: \"416\"\n" + + "[protobuf_unittest.default_nested_enum_extension]: FOO\n" + + "[protobuf_unittest.default_foreign_enum_extension]: FOREIGN_FOO\n" + + "[protobuf_unittest.default_import_enum_extension]: IMPORT_FOO\n" + + "[protobuf_unittest.default_string_piece_extension]: \"424\"\n" + + "[protobuf_unittest.default_cord_extension]: \"425\"\n" + + "[protobuf_unittest.oneof_uint32_extension]: 601\n" + + "[protobuf_unittest.oneof_nested_message_extension] {\n" + + " bb: 602\n" + + "}\n" + + "[protobuf_unittest.oneof_string_extension]: \"603\"\n" + + "[protobuf_unittest.oneof_bytes_extension]: \"604\"\n"; + public static final TestRequired TEST_REQUIRED_UNINITIALIZED = TestRequired.newBuilder().setA(1).buildPartial(); public static final TestRequired TEST_REQUIRED_INITIALIZED = @@ -3764,108 +4035,6 @@ public void assertReflectionRepeatedSettersRejectNull(Message.Builder builder) } } - /** @param filePath The path relative to {@link #getTestDataDir}. */ - public static String readTextFromFile(String filePath) { - return readBytesFromFile(filePath) - .toStringUtf8() - .replace(System.getProperty("line.separator"), "\n"); - } - - private static File getTestDataDir() { - // Search each parent directory looking for "src/google/protobuf". - File ancestor = new File(System.getProperty("protobuf.dir", ".")); - String initialPath = ancestor.getAbsolutePath(); - try { - ancestor = ancestor.getCanonicalFile(); - } catch (IOException e) { - throw new RuntimeException("Couldn't get canonical name of working directory.", e); - } - - String srcRootCheck = "src/google/protobuf"; - - // If we're running w/ Bazel on Windows, we're not in a sandbox, so we - // we must change our source root check condition to find the true test data dir. - String testBinaryName = System.getenv("TEST_BINARY"); - if (testBinaryName != null && testBinaryName.endsWith(".exe")) { - srcRootCheck = srcRootCheck + "/descriptor.cc"; - } - - while (ancestor != null && ancestor.exists()) { - // Identify the true source root. - if (new File(ancestor, srcRootCheck).exists()) { - return new File(ancestor, "src/google/protobuf/testdata"); - } - ancestor = ancestor.getParentFile(); - } - - throw new RuntimeException( - "Could not find golden files. This test must be run from within the " - + "protobuf source package so that it can read test data files from the " - + "C++ source tree: " - + initialPath); - } - - /** @param filename The path relative to {@link #getTestDataDir}. */ - public static ByteString readBytesFromFile(String filename) { - File fullPath = new File(getTestDataDir(), filename); - try { - RandomAccessFile file = new RandomAccessFile(fullPath, "r"); - byte[] content = new byte[(int) file.length()]; - file.readFully(content); - return ByteString.copyFrom(content); - } catch (IOException e) { - // Throw a RuntimeException here so that we can call this function from - // static initializers. - throw new IllegalArgumentException("Couldn't read file: " + fullPath.getPath(), e); - } - } - // END FULL-RUNTIME - - private static ByteString readBytesFromResource(String name) { - try { - InputStream in = TestUtil.class.getResourceAsStream(name); - if (in == null) { // - throw new RuntimeException("Tests data file " + name + " is missing."); - } - return ByteString.readFrom(in); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - - /** - * Get the bytes of the "golden message". This is a serialized TestAllTypes with all fields set as - * they would be by {@link #setAllFields(TestAllTypes.Builder)}, but it is loaded from a file on - * disk rather than generated dynamically. The file is actually generated by C++ code, so testing - * against it verifies compatibility with C++. - */ - public static ByteString getGoldenMessage() { - if (goldenMessage == null) { - goldenMessage = - readBytesFromResource("/google/protobuf/testdata/golden_message_oneof_implemented"); - } - return goldenMessage; - } - - private static ByteString goldenMessage = null; - - /** - * Get the bytes of the "golden packed fields message". This is a serialized TestPackedTypes with - * all fields set as they would be by {@link #setPackedFields(TestPackedTypes.Builder)}, but it is - * loaded from a file on disk rather than generated dynamically. The file is actually generated by - * C++ code, so testing against it verifies compatibility with C++. - */ - public static ByteString getGoldenPackedFieldsMessage() { - if (goldenPackedFieldsMessage == null) { - goldenPackedFieldsMessage = - readBytesFromResource("/google/protobuf/testdata/golden_packed_fields_message"); - } - return goldenPackedFieldsMessage; - } - - private static ByteString goldenPackedFieldsMessage = null; - - // BEGIN FULL-RUNTIME /** * Mock implementation of {@link GeneratedMessage.BuilderParent} for testing. * diff --git a/java/core/src/test/java/com/google/protobuf/TextFormatTest.java b/java/core/src/test/java/com/google/protobuf/TextFormatTest.java index d66a8b0f981d3..54d0f3ba3b721 100644 --- a/java/core/src/test/java/com/google/protobuf/TextFormatTest.java +++ b/java/core/src/test/java/com/google/protobuf/TextFormatTest.java @@ -65,11 +65,6 @@ public class TextFormatTest { "\\\"A string with \\' characters \\n and \\r newlines " + "and \\t tabs and \\001 slashes \\\\"; - private static final String ALL_FIELDS_SET_TEXT = - TestUtil.readTextFromFile("text_format_unittest_data_oneof_implemented.txt"); - private static final String ALL_EXTENSIONS_SET_TEXT = - TestUtil.readTextFromFile("text_format_unittest_extensions_data.txt"); - private static final String EXOTIC_TEXT = "" + "repeated_int32: -1\n" @@ -145,12 +140,7 @@ public class TextFormatTest { public void testPrintMessage() throws Exception { String javaText = TextFormat.printer().printToString(TestUtil.getAllSet()); - // Java likes to add a trailing ".0" to floats and doubles. C printf - // (with %g format) does not. Our golden files are used for both - // C++ and Java TextFormat classes, so we need to conform. - javaText = javaText.replace(".0\n", "\n"); - - assertThat(javaText).isEqualTo(ALL_FIELDS_SET_TEXT); + assertThat(javaText).isEqualTo(TestUtil.ALL_FIELDS_SET_TEXT); } @Test @@ -165,12 +155,7 @@ public void testCharacterNotInUnicodeBlock() throws TextFormat.InvalidEscapeSequ public void testPrintMessageBuilder() throws Exception { String javaText = TextFormat.printer().printToString(TestUtil.getAllSetBuilder()); - // Java likes to add a trailing ".0" to floats and doubles. C printf - // (with %g format) does not. Our golden files are used for both - // C++ and Java TextFormat classes, so we need to conform. - javaText = javaText.replace(".0\n", "\n"); - - assertThat(javaText).isEqualTo(ALL_FIELDS_SET_TEXT); + assertThat(javaText).isEqualTo(TestUtil.ALL_FIELDS_SET_TEXT); } /** Print TestAllExtensions and compare with golden file. */ @@ -178,12 +163,7 @@ public void testPrintMessageBuilder() throws Exception { public void testPrintExtensions() throws Exception { String javaText = TextFormat.printer().printToString(TestUtil.getAllExtensionsSet()); - // Java likes to add a trailing ".0" to floats and doubles. C printf - // (with %g format) does not. Our golden files are used for both - // C++ and Java TextFormat classes, so we need to conform. - javaText = javaText.replace(".0\n", "\n"); - - assertThat(javaText).isEqualTo(ALL_EXTENSIONS_SET_TEXT); + assertThat(javaText).isEqualTo(TestUtil.ALL_EXTENSIONS_SET_TEXT); } // Creates an example unknown field set. @@ -359,13 +339,13 @@ public void testPrintMessageSet() throws Exception { @Test public void testMerge() throws Exception { TestAllTypes.Builder builder = TestAllTypes.newBuilder(); - TextFormat.merge(ALL_FIELDS_SET_TEXT, builder); + TextFormat.merge(TestUtil.ALL_FIELDS_SET_TEXT, builder); TestUtil.assertAllFieldsSet(builder.build()); } @Test public void testParse() throws Exception { - TestUtil.assertAllFieldsSet(TextFormat.parse(ALL_FIELDS_SET_TEXT, TestAllTypes.class)); + TestUtil.assertAllFieldsSet(TextFormat.parse(TestUtil.ALL_FIELDS_SET_TEXT, TestAllTypes.class)); } @Test @@ -405,14 +385,15 @@ public void testParseUninitialized() throws Exception { @Test public void testMergeReader() throws Exception { TestAllTypes.Builder builder = TestAllTypes.newBuilder(); - TextFormat.merge(new StringReader(ALL_FIELDS_SET_TEXT), builder); + TextFormat.merge(new StringReader(TestUtil.ALL_FIELDS_SET_TEXT), builder); TestUtil.assertAllFieldsSet(builder.build()); } @Test public void testMergeExtensions() throws Exception { TestAllExtensions.Builder builder = TestAllExtensions.newBuilder(); - TextFormat.merge(ALL_EXTENSIONS_SET_TEXT, TestUtil.getFullExtensionRegistry(), builder); + TextFormat.merge( + TestUtil.ALL_EXTENSIONS_SET_TEXT, TestUtil.getFullExtensionRegistry(), builder); TestUtil.assertAllExtensionsSet(builder.build()); } @@ -420,7 +401,9 @@ public void testMergeExtensions() throws Exception { public void testParseExtensions() throws Exception { TestUtil.assertAllExtensionsSet( TextFormat.parse( - ALL_EXTENSIONS_SET_TEXT, TestUtil.getFullExtensionRegistry(), TestAllExtensions.class)); + TestUtil.ALL_EXTENSIONS_SET_TEXT, + TestUtil.getFullExtensionRegistry(), + TestAllExtensions.class)); } @Test diff --git a/python/google/protobuf/internal/descriptor_test.py b/python/google/protobuf/internal/descriptor_test.py index ce16d0363ac01..0b396d0cdc173 100755 --- a/python/google/protobuf/internal/descriptor_test.py +++ b/python/google/protobuf/internal/descriptor_test.py @@ -726,7 +726,7 @@ def CheckDescriptorMapping(self, mapping): excepted_dict['new_key'] = 'new' self.assertNotEqual(mapping, excepted_dict) self.assertRaises(KeyError, mapping.__getitem__, 'key_error') - self.assertRaises(KeyError, mapping.__getitem__, len(mapping) + 1) + self.assertRaises(KeyError, mapping.__getitem__, len(mapping) * 2) # TODO: Add __repr__ support for DescriptorMapping. if api_implementation.Type() == 'cpp': self.assertEqual(str(mapping)[0], '<') diff --git a/python/google/protobuf/internal/field_mask_test.py b/python/google/protobuf/internal/field_mask_test.py index ed7c9ef60e264..ef286f8ba5d0b 100644 --- a/python/google/protobuf/internal/field_mask_test.py +++ b/python/google/protobuf/internal/field_mask_test.py @@ -53,7 +53,7 @@ def testDescriptorToFieldMask(self): mask = field_mask_pb2.FieldMask() msg_descriptor = unittest_pb2.TestAllTypes.DESCRIPTOR mask.AllFieldsFromDescriptor(msg_descriptor) - self.assertEqual(79, len(mask.paths)) + self.assertEqual(80, len(mask.paths)) self.assertTrue(mask.IsValidForDescriptor(msg_descriptor)) for field in msg_descriptor.fields: self.assertTrue(field.name in mask.paths) diff --git a/python/google/protobuf/internal/message_test.py b/python/google/protobuf/internal/message_test.py index 68f1999b16712..78b06765d0953 100755 --- a/python/google/protobuf/internal/message_test.py +++ b/python/google/protobuf/internal/message_test.py @@ -63,35 +63,6 @@ def testBadUtf8String(self, message_module): message_module.TestAllTypes.FromString(bad_utf8_data) self.assertIn('TestAllTypes.optional_string', str(context.exception)) - def testGoldenMessage(self, message_module): - # Proto3 doesn't have the "default_foo" members or foreign enums, - # and doesn't preserve unknown fields, so for proto3 we use a golden - # message that doesn't have these fields set. - if message_module is unittest_pb2: - golden_data = test_util.GoldenFileData('golden_message_oneof_implemented') - else: - golden_data = test_util.GoldenFileData('golden_message_proto3') - - golden_message = message_module.TestAllTypes() - golden_message.ParseFromString(golden_data) - if message_module is unittest_pb2: - test_util.ExpectAllFieldsSet(self, golden_message) - self.assertEqual(golden_data, golden_message.SerializeToString()) - golden_copy = copy.deepcopy(golden_message) - self.assertEqual(golden_data, golden_copy.SerializeToString()) - - def testGoldenPackedMessage(self, message_module): - golden_data = test_util.GoldenFileData('golden_packed_fields_message') - golden_message = message_module.TestPackedTypes() - parsed_bytes = golden_message.ParseFromString(golden_data) - all_set = message_module.TestPackedTypes() - test_util.SetAllPackedFields(all_set) - self.assertEqual(parsed_bytes, len(golden_data)) - self.assertEqual(all_set, golden_message) - self.assertEqual(golden_data, all_set.SerializeToString()) - golden_copy = copy.deepcopy(golden_message) - self.assertEqual(golden_data, golden_copy.SerializeToString()) - def testParseErrors(self, message_module): msg = message_module.TestAllTypes() self.assertRaises(TypeError, msg.FromString, 0) @@ -147,7 +118,9 @@ def __bool__(self): golden_message.SerializeToString(deterministic=BadArg()) def testPickleSupport(self, message_module): - golden_data = test_util.GoldenFileData('golden_message') + golden_message = message_module.TestAllTypes() + test_util.SetAllFields(golden_message) + golden_data = golden_message.SerializeToString() golden_message = message_module.TestAllTypes() golden_message.ParseFromString(golden_data) pickled_message = pickle.dumps(golden_message) @@ -1468,36 +1441,6 @@ def testCopyFromAllPackedExtensions(self): all_set.Extensions[unittest_pb2.packed_float_extension].extend([61.0, 71.0]) self.assertNotEqual(all_set, copy) - def testGoldenExtensions(self): - golden_data = test_util.GoldenFileData('golden_message') - golden_message = unittest_pb2.TestAllExtensions() - golden_message.ParseFromString(golden_data) - all_set = unittest_pb2.TestAllExtensions() - test_util.SetAllExtensions(all_set) - self.assertEqual(all_set, golden_message) - self.assertEqual(golden_data, golden_message.SerializeToString()) - golden_copy = copy.deepcopy(golden_message) - self.assertEqual(golden_message, golden_copy) - # Depend on a specific serialization order for extensions is not - # reasonable to guarantee. - if api_implementation.Type() != 'upb': - self.assertEqual(golden_data, golden_copy.SerializeToString()) - - def testGoldenPackedExtensions(self): - golden_data = test_util.GoldenFileData('golden_packed_fields_message') - golden_message = unittest_pb2.TestPackedExtensions() - golden_message.ParseFromString(golden_data) - all_set = unittest_pb2.TestPackedExtensions() - test_util.SetAllPackedExtensions(all_set) - self.assertEqual(all_set, golden_message) - self.assertEqual(golden_data, all_set.SerializeToString()) - golden_copy = copy.deepcopy(golden_message) - self.assertEqual(golden_message, golden_copy) - # Depend on a specific serialization order for extensions is not - # reasonable to guarantee. - if api_implementation.Type() != 'upb': - self.assertEqual(golden_data, golden_copy.SerializeToString()) - def testPickleIncompleteProto(self): golden_message = unittest_pb2.TestRequired(a=1) pickled_message = pickle.dumps(golden_message) diff --git a/python/google/protobuf/internal/test_util.py b/python/google/protobuf/internal/test_util.py index 53492d8a1587e..46059fe1eba24 100755 --- a/python/google/protobuf/internal/test_util.py +++ b/python/google/protobuf/internal/test_util.py @@ -77,6 +77,7 @@ def SetAllNonLazyFields(message): message.optional_string_piece = u'124' message.optional_cord = u'125' + message.optional_bytes_cord = b'optional bytes cord' # # Repeated fields. @@ -247,6 +248,7 @@ def SetAllExtensions(message): extensions[pb2.optional_string_piece_extension] = u'124' extensions[pb2.optional_cord_extension] = u'125' + extensions[pb2.optional_bytes_cord_extension] = b'optional bytes cord' # # Repeated fields. @@ -423,6 +425,7 @@ def ExpectAllFieldsSet(test_case, message): test_case.assertTrue(message.HasField('optional_string_piece')) test_case.assertTrue(message.HasField('optional_cord')) + test_case.assertTrue(message.HasField('optional_bytes_cord')) test_case.assertEqual(101, message.optional_int32) test_case.assertEqual(102, message.optional_int64) diff --git a/python/google/protobuf/internal/unknown_fields_test.py b/python/google/protobuf/internal/unknown_fields_test.py index 57224ec74c74e..33ad766537994 100755 --- a/python/google/protobuf/internal/unknown_fields_test.py +++ b/python/google/protobuf/internal/unknown_fields_test.py @@ -212,7 +212,7 @@ def testCheckUnknownFieldValue(self): unknown_field_set, (17, 0, 117)) - self.assertEqual(98, len(unknown_field_set)) + self.assertEqual(99, len(unknown_field_set)) def testCopyFrom(self): message = unittest_pb2.TestEmptyMessage() @@ -250,7 +250,7 @@ def testClear(self): self.empty_message.Clear() # All cleared, even unknown fields. self.assertEqual(self.empty_message.SerializeToString(), b'') - self.assertEqual(len(unknown_field_set), 98) + self.assertEqual(len(unknown_field_set), 99) @unittest.skipIf((sys.version_info.major, sys.version_info.minor) < (3, 4), 'tracemalloc requires python 3.4+') @@ -309,7 +309,7 @@ def testUnknownField(self): def testUnknownExtensions(self): message = unittest_pb2.TestEmptyMessageWithExtensions() message.ParseFromString(self.all_fields_data) - self.assertEqual(len(unknown_fields.UnknownFieldSet(message)), 98) + self.assertEqual(len(unknown_fields.UnknownFieldSet(message)), 99) self.assertEqual(message.SerializeToString(), self.all_fields_data) diff --git a/python/map.c b/python/map.c index 4b6e97e1932d4..1e2b3806b6a40 100644 --- a/python/map.c +++ b/python/map.c @@ -259,16 +259,6 @@ static Py_ssize_t PyUpb_MapContainer_Length(PyObject* _self) { return map ? upb_Map_Size(map) : 0; } -static PyUpb_MapContainer* PyUpb_MapContainer_Check(PyObject* _self) { - PyUpb_ModuleState* state = PyUpb_ModuleState_Get(); - if (!PyObject_TypeCheck(_self, state->message_map_container_type) && - !PyObject_TypeCheck(_self, state->scalar_map_container_type)) { - PyErr_Format(PyExc_TypeError, "Expected protobuf map, but got %R", _self); - return NULL; - } - return (PyUpb_MapContainer*)_self; -} - int PyUpb_Message_InitMapAttributes(PyObject* map, PyObject* value, const upb_FieldDef* f); diff --git a/python/message.c b/python/message.c index c0c0882d24c41..6ace35ca20bd4 100644 --- a/python/message.c +++ b/python/message.c @@ -1593,7 +1593,6 @@ static PyObject* PyUpb_Message_WhichOneof(PyObject* _self, PyObject* name) { } PyObject* DeepCopy(PyObject* _self, PyObject* arg) { - PyUpb_Message* self = (void*)_self; const upb_MessageDef* def = PyUpb_Message_GetMsgdef(_self); const upb_MiniTable* mini_table = upb_MessageDef_MiniTable(def); upb_Message* msg = PyUpb_Message_GetIfReified(_self); diff --git a/python/protobuf.c b/python/protobuf.c index 316b1f6603e05..8cdce61a37442 100644 --- a/python/protobuf.c +++ b/python/protobuf.c @@ -240,7 +240,7 @@ static void* upb_trim_allocfunc(upb_alloc* alloc, void* ptr, size_t oldsize, } } static upb_alloc trim_alloc = {&upb_trim_allocfunc}; -static const upb_alloc* global_alloc = &trim_alloc; +static upb_alloc* global_alloc = &trim_alloc; // end:github_only static upb_Arena* PyUpb_NewArena(void) { diff --git a/ruby/ext/google/protobuf_c/convert.c b/ruby/ext/google/protobuf_c/convert.c index 1fae52446f703..8231c53c89f11 100644 --- a/ruby/ext/google/protobuf_c/convert.c +++ b/ruby/ext/google/protobuf_c/convert.c @@ -204,7 +204,8 @@ upb_MessageValue Convert_RubyToUpb(VALUE value, const char* name, ret.uint64_val = NUM2ULL(value); break; default: - break; + rb_raise(cTypeError, "Convert_RubyToUpb(): Unexpected type %d", + (int)type_info.type); } break; default: diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index 6ac0549f5ceb9..032b2979a940c 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -1038,6 +1038,7 @@ cc_test( name = "string_view_test", srcs = ["string_view_test.cc"], deps = [ + ":port", ":protobuf", ":unittest_string_view_cc_proto", "@com_google_absl//absl/strings:string_view", @@ -1578,7 +1579,6 @@ cc_test( "message_unittest.inc", "message_unittest_legacy_apis.inc", ], - data = [":testdata"], deps = [ ":arena", ":cc_test_protos", @@ -1609,7 +1609,6 @@ cc_test( "edition_message_unittest.cc", "message_unittest.inc", ], - data = [":testdata"], deps = [ ":arena", ":cc_test_protos", diff --git a/src/google/protobuf/arena_test_util.h b/src/google/protobuf/arena_test_util.h index db21949d084c7..811c9dd8200a3 100644 --- a/src/google/protobuf/arena_test_util.h +++ b/src/google/protobuf/arena_test_util.h @@ -21,46 +21,6 @@ namespace google { namespace protobuf { - -template -void TestParseCorruptedString(const T& message) { - int success_count = 0; - std::string s; - { - // Map order is not deterministic. To make the test deterministic we want - // to serialize the proto deterministically. - io::StringOutputStream output(&s); - io::CodedOutputStream out(&output); - out.SetSerializationDeterministic(true); - message.SerializePartialToCodedStream(&out); - } -#if defined(PROTOBUF_ASAN) || defined(PROTOBUF_TSAN) || defined(PROTOBUF_MSAN) - // Make the test smaller in sanitizer mode. - const int kMaxIters = 200; -#else - const int kMaxIters = 900; -#endif - const int stride = s.size() <= kMaxIters ? 1 : s.size() / kMaxIters; - const int start = stride == 1 || use_arena ? 0 : (stride + 1) / 2; - for (int i = start; i < s.size(); i += stride) { - for (int c = 1 + (i % 17); c < 256; c += 2 * c + (i & 3)) { - s[i] ^= c; - Arena arena; - T* message = Arena::Create(use_arena ? &arena : nullptr); - if (message->ParseFromString(s)) { - ++success_count; - } - if (!use_arena) { - delete message; - } - s[i] ^= c; // Restore s to its original state. - } - } - // This next line is a low bar. But getting through the test without crashing - // due to use-after-free or other bugs is a big part of what we're checking. - ABSL_CHECK_GT(success_count, 0); -} - namespace internal { struct ArenaTestPeer { diff --git a/src/google/protobuf/arena_unittest.cc b/src/google/protobuf/arena_unittest.cc index 0752735a4836d..4e2dbbf38e0e0 100644 --- a/src/google/protobuf/arena_unittest.cc +++ b/src/google/protobuf/arena_unittest.cc @@ -1585,13 +1585,6 @@ TEST(ArenaTest, NoHeapAllocationsTest) { arena.Reset(); } -TEST(ArenaTest, ParseCorruptedString) { - TestAllTypes message; - TestUtil::SetAllFields(&message); - TestParseCorruptedString(message); - TestParseCorruptedString(message); -} - #if PROTOBUF_RTTI // Test construction on an arena via generic MessageLite interface. We should be // able to successfully deserialize on the arena without incurring heap diff --git a/src/google/protobuf/compiler/BUILD.bazel b/src/google/protobuf/compiler/BUILD.bazel index 02729619fdd47..3f5624d5f8da4 100644 --- a/src/google/protobuf/compiler/BUILD.bazel +++ b/src/google/protobuf/compiler/BUILD.bazel @@ -416,6 +416,7 @@ cc_test( "//src/google/protobuf:cc_test_protos", "//src/google/protobuf:port", "//src/google/protobuf:test_textproto", + "//src/google/protobuf:test_util", "//src/google/protobuf:test_util2", "//src/google/protobuf/compiler/cpp:names", "//src/google/protobuf/io", diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index 8870dabae30f5..d6341b58d384f 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -48,6 +48,7 @@ #include "google/protobuf/compiler/mock_code_generator.h" #include "google/protobuf/compiler/plugin.pb.h" #include "google/protobuf/test_textproto.h" +#include "google/protobuf/test_util.h" #include "google/protobuf/test_util2.h" #include "google/protobuf/unittest.pb.h" #include "google/protobuf/unittest_custom_options.pb.h" @@ -4146,7 +4147,16 @@ class EncodeDecodeTest : public testing::TestWithParam { std::string unittest_proto_descriptor_set_filename_; }; +static void WriteGoldenMessage(const std::string& filename) { + protobuf_unittest::TestAllTypes message; + TestUtil::SetAllFields(&message); + std::string golden = message.SerializeAsString(); + ABSL_CHECK_OK(File::SetContents(filename, golden, true)); +} + TEST_P(EncodeDecodeTest, Encode) { + std::string golden_path = absl::StrCat(TestTempDir(), "/golden_message"); + WriteGoldenMessage(golden_path); RedirectStdinFromFile(TestUtil::GetTestDataPath( "google/protobuf/" "testdata/text_format_unittest_data_oneof_implemented.txt")); @@ -4156,14 +4166,14 @@ TEST_P(EncodeDecodeTest, Encode) { } EXPECT_TRUE( Run(absl::StrCat(args, " --encode=protobuf_unittest.TestAllTypes"))); - ExpectStdoutMatchesBinaryFile(TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_message_oneof_implemented")); + ExpectStdoutMatchesBinaryFile(golden_path); ExpectStderrMatchesText(""); } TEST_P(EncodeDecodeTest, Decode) { - RedirectStdinFromFile(TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_message_oneof_implemented")); + std::string golden_path = absl::StrCat(TestTempDir(), "/golden_message"); + WriteGoldenMessage(golden_path); + RedirectStdinFromFile(golden_path); EXPECT_TRUE( Run("google/protobuf/unittest.proto" " --decode=protobuf_unittest.TestAllTypes")); @@ -4216,6 +4226,8 @@ TEST_P(EncodeDecodeTest, ProtoParseError) { } TEST_P(EncodeDecodeTest, EncodeDeterministicOutput) { + std::string golden_path = absl::StrCat(TestTempDir(), "/golden_message"); + WriteGoldenMessage(golden_path); RedirectStdinFromFile(TestUtil::GetTestDataPath( "google/protobuf/" "testdata/text_format_unittest_data_oneof_implemented.txt")); @@ -4225,14 +4237,14 @@ TEST_P(EncodeDecodeTest, EncodeDeterministicOutput) { } EXPECT_TRUE(Run(absl::StrCat( args, " --encode=protobuf_unittest.TestAllTypes --deterministic_output"))); - ExpectStdoutMatchesBinaryFile(TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_message_oneof_implemented")); + ExpectStdoutMatchesBinaryFile(golden_path); ExpectStderrMatchesText(""); } TEST_P(EncodeDecodeTest, DecodeDeterministicOutput) { - RedirectStdinFromFile(TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_message_oneof_implemented")); + std::string golden_path = absl::StrCat(TestTempDir(), "/golden_message"); + WriteGoldenMessage(golden_path); + RedirectStdinFromFile(golden_path); EXPECT_FALSE( Run("google/protobuf/unittest.proto" " --decode=protobuf_unittest.TestAllTypes --deterministic_output")); diff --git a/src/google/protobuf/compiler/cpp/field.cc b/src/google/protobuf/compiler/cpp/field.cc index c333a5818b3b9..6b354e740409e 100644 --- a/src/google/protobuf/compiler/cpp/field.cc +++ b/src/google/protobuf/compiler/cpp/field.cc @@ -130,7 +130,6 @@ FieldGeneratorBase::FieldGeneratorBase(const FieldDescriptor* field, break; case FieldDescriptor::CPPTYPE_STRING: is_string_ = true; - string_type_ = field->options().ctype(); is_inlined_ = IsStringInlined(field, options); is_bytes_ = field->type() == FieldDescriptor::TYPE_BYTES; has_default_constexpr_constructor_ = is_repeated_or_map; @@ -228,40 +227,6 @@ void FieldGeneratorBase::GenerateCopyConstructorCode(io::Printer* p) const { } namespace { -// Use internal types instead of ctype or string_type. -enum class StringType { - kView, - kString, - kCord, - kStringPiece, -}; - -StringType GetStringType(const FieldDescriptor& field) { - ABSL_CHECK_EQ(field.cpp_type(), FieldDescriptor::CPPTYPE_STRING); - - if (field.options().has_ctype()) { - switch (field.options().ctype()) { - case FieldOptions::CORD: - return StringType::kCord; - case FieldOptions::STRING_PIECE: - return StringType::kStringPiece; - default: - return StringType::kString; - } - } - - const pb::CppFeatures& cpp_features = - CppGenerator::GetResolvedSourceFeatures(field).GetExtension(::pb::cpp); - switch (cpp_features.string_type()) { - case pb::CppFeatures::CORD: - return StringType::kCord; - case pb::CppFeatures::VIEW: - return StringType::kView; - default: - return StringType::kString; - } -} - std::unique_ptr MakeGenerator(const FieldDescriptor* field, const Options& options, MessageSCCAnalyzer* scc) { @@ -278,7 +243,7 @@ std::unique_ptr MakeGenerator(const FieldDescriptor* field, case FieldDescriptor::CPPTYPE_MESSAGE: return MakeRepeatedMessageGenerator(field, options, scc); case FieldDescriptor::CPPTYPE_STRING: { - if (GetStringType(*field) == StringType::kView) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kView) { return MakeRepeatedStringViewGenerator(field, options, scc); } else { return MakeRepeatedStringGenerator(field, options, scc); @@ -302,10 +267,10 @@ std::unique_ptr MakeGenerator(const FieldDescriptor* field, case FieldDescriptor::CPPTYPE_ENUM: return MakeSinguarEnumGenerator(field, options, scc); case FieldDescriptor::CPPTYPE_STRING: { - switch (GetStringType(*field)) { - case StringType::kView: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kView: return MakeSingularStringViewGenerator(field, options, scc); - case StringType::kCord: + case FieldDescriptor::CppStringType::kCord: if (field->type() == FieldDescriptor::TYPE_BYTES) { if (field->real_containing_oneof()) { return MakeOneofCordGenerator(field, options, scc); diff --git a/src/google/protobuf/compiler/cpp/field.h b/src/google/protobuf/compiler/cpp/field.h index 984596d875acf..3944906b0b61d 100644 --- a/src/google/protobuf/compiler/cpp/field.h +++ b/src/google/protobuf/compiler/cpp/field.h @@ -92,9 +92,6 @@ class FieldGeneratorBase { // Returns true if the field API uses bytes (void) instead of chars. bool is_bytes() const { return is_bytes_; } - // Returns the public API string type for string fields. - FieldOptions::CType string_type() const { return string_type_; } - // Returns true if this field is part of a oneof field. bool is_oneof() const { return is_oneof_; } @@ -213,7 +210,6 @@ class FieldGeneratorBase { bool is_lazy_ = false; bool is_weak_ = false; bool is_oneof_ = false; - FieldOptions::CType string_type_ = FieldOptions::STRING; bool has_default_constexpr_constructor_ = false; }; @@ -262,7 +258,6 @@ class FieldGenerator { bool is_foreign() const { return impl_->is_foreign(); } bool is_string() const { return impl_->is_string(); } bool is_bytes() const { return impl_->is_bytes(); } - FieldOptions::CType string_type() const { return impl_->string_type(); } bool is_oneof() const { return impl_->is_oneof(); } bool is_inlined() const { return impl_->is_inlined(); } bool has_default_constexpr_constructor() const { diff --git a/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc b/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc index 6e73c0f5a37b1..805ce16813591 100644 --- a/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc +++ b/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc @@ -216,8 +216,6 @@ void SingularStringView::GenerateStaticMembers(io::Printer* p) const { } void SingularStringView::GenerateAccessorDeclarations(io::Printer* p) const { - ABSL_CHECK(!field_->options().has_ctype()); - auto vars = AnnotatedAccessors(field_, {"", "set_allocated_"}); vars.push_back(Sub{ "release_name", diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index 047a52709ea32..06710f412bc90 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -1574,7 +1574,8 @@ MessageAnalysis MessageSCCAnalyzer::GetSCCAnalysis(const SCC* scc) { switch (field->type()) { case FieldDescriptor::TYPE_STRING: case FieldDescriptor::TYPE_BYTES: { - if (field->options().ctype() == FieldOptions::CORD) { + if (field->cpp_string_type() == + FieldDescriptor::CppStringType::kCord) { result.contains_cord = true; } break; diff --git a/src/google/protobuf/compiler/cpp/tracker.cc b/src/google/protobuf/compiler/cpp/tracker.cc index 4c565c5e81868..b7a7a1956754d 100644 --- a/src/google/protobuf/compiler/cpp/tracker.cc +++ b/src/google/protobuf/compiler/cpp/tracker.cc @@ -214,7 +214,8 @@ Getters RepeatedFieldGetters(const FieldDescriptor* field, Getters StringFieldGetters(const FieldDescriptor* field, const Options& opts) { std::string member = FieldMemberName(field, ShouldSplit(field, opts)); - bool is_std_string = field->options().ctype() == FieldOptions::STRING; + bool is_std_string = + field->cpp_string_type() == FieldDescriptor::CppStringType::kString; Getters getters; if (is_std_string && !field->default_value_string().empty()) { @@ -234,7 +235,8 @@ Getters StringOneofGetters(const FieldDescriptor* field, ABSL_CHECK(oneof != nullptr); std::string member = FieldMemberName(field, ShouldSplit(field, opts)); - bool is_std_string = field->options().ctype() == FieldOptions::STRING; + bool is_std_string = + field->cpp_string_type() == FieldDescriptor::CppStringType::kString; std::string field_ptr = member; if (is_std_string) { @@ -251,8 +253,8 @@ Getters StringOneofGetters(const FieldDescriptor* field, } Getters getters; - if (field->default_value_string().empty() || - field->options().ctype() == FieldOptions::STRING_PIECE) { + if (field->default_value_string().empty() + ) { getters.base = absl::Substitute("$0 ? $1 : nullptr", has, field_ptr); } else { getters.base = diff --git a/src/google/protobuf/compiler/cpp/unittest.inc b/src/google/protobuf/compiler/cpp/unittest.inc index c4b27ae94a24e..ce805003c5a1f 100644 --- a/src/google/protobuf/compiler/cpp/unittest.inc +++ b/src/google/protobuf/compiler/cpp/unittest.inc @@ -425,6 +425,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { message1.set_optional_string("abc"); message1.mutable_optional_nested_message()->set_bb(1); message1.set_optional_nested_enum(UNITTEST::TestAllTypes::FOO); + message1.set_optional_bytes_cord("bytes cord"); message1.add_repeated_int32(1); message1.add_repeated_int32(2); message1.add_repeated_string("a"); @@ -438,6 +439,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { message2.set_optional_string("def"); message2.mutable_optional_nested_message()->set_bb(2); message2.set_optional_nested_enum(UNITTEST::TestAllTypes::BAR); + message2.set_optional_bytes_cord("bytes cord"); message2.add_repeated_int32(3); message2.add_repeated_string("c"); message2.add_repeated_nested_message()->set_bb(9); @@ -449,6 +451,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { EXPECT_EQ("def", message1.optional_string()); EXPECT_EQ(2, message1.optional_nested_message().bb()); EXPECT_EQ(UNITTEST::TestAllTypes::BAR, message1.optional_nested_enum()); + EXPECT_EQ(absl::Cord("bytes cord"), message1.optional_bytes_cord()); ASSERT_EQ(1, message1.repeated_int32_size()); EXPECT_EQ(3, message1.repeated_int32(0)); ASSERT_EQ(1, message1.repeated_string_size()); @@ -462,6 +465,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { EXPECT_EQ("abc", message2.optional_string()); EXPECT_EQ(1, message2.optional_nested_message().bb()); EXPECT_EQ(UNITTEST::TestAllTypes::FOO, message2.optional_nested_enum()); + EXPECT_EQ(absl::Cord("bytes cord"), message2.optional_bytes_cord()); ASSERT_EQ(2, message2.repeated_int32_size()); EXPECT_EQ(1, message2.repeated_int32(0)); EXPECT_EQ(2, message2.repeated_int32(1)); diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 9b4a8fec66fc1..65cd1d5065f86 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -3935,6 +3935,29 @@ bool FieldDescriptor::has_optional_keyword() const { is_optional() && !containing_oneof()); } +FieldDescriptor::CppStringType FieldDescriptor::cpp_string_type() const { + ABSL_DCHECK(cpp_type() == FieldDescriptor::CPPTYPE_STRING); + switch (features().GetExtension(pb::cpp).string_type()) { + case pb::CppFeatures::VIEW: + return CppStringType::kView; + case pb::CppFeatures::CORD: + // In open-source, protobuf CORD is only supported for singular bytes + // fields. + if (type() != FieldDescriptor::TYPE_BYTES || is_repeated() || + is_extension()) { + return CppStringType::kString; + } + return CppStringType::kCord; + case pb::CppFeatures::STRING: + return CppStringType::kString; + default: + // If features haven't been resolved, this is a dynamic build not for C++ + // codegen. Just use string type. + ABSL_DCHECK(!features().GetExtension(pb::cpp).has_string_type()); + return CppStringType::kString; + } +} + // Location methods =============================================== bool FileDescriptor::GetSourceLocation(const std::vector& path, diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index c7d2d595542b8..abff2e7373169 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -859,6 +859,10 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase, const char* cpp_type_name() const; // Name of the C++ type. Label label() const; // optional/required/repeated +#ifndef SWIG + CppStringType cpp_string_type() const; // The C++ string type of this field. +#endif + bool is_required() const; // shorthand for label() == LABEL_REQUIRED bool is_optional() const; // shorthand for label() == LABEL_OPTIONAL bool is_repeated() const; // shorthand for label() == LABEL_REPEATED @@ -2880,22 +2884,21 @@ PROTOBUF_EXPORT bool HasPreservingUnknownEnumSemantics( PROTOBUF_EXPORT bool HasHasbit(const FieldDescriptor* field); +#ifndef SWIG // For a string field, returns the effective ctype. If the actual ctype is // not supported, returns the default of STRING. template typename FieldOpts::CType EffectiveStringCType(const FieldDesc* field) { - ABSL_DCHECK(field->cpp_type() == FieldDescriptor::CPPTYPE_STRING); - // Open-source protobuf release only supports STRING ctype and CORD for - // sinuglar bytes. - if (field->type() == FieldDescriptor::TYPE_BYTES && !field->is_repeated() && - field->options().ctype() == FieldOpts::CORD && !field->is_extension()) { - return FieldOpts::CORD; + // TODO Replace this function with FieldDescriptor::string_type; + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + return FieldOpts::CORD; + default: + return FieldOpts::STRING; } - return FieldOpts::STRING; } -#ifndef SWIG enum class Utf8CheckMode : uint8_t { kStrict = 0, // Parsing will fail if non UTF-8 data is in string fields. kVerify = 1, // Only log an error but parsing will succeed. diff --git a/src/google/protobuf/descriptor_database_unittest.cc b/src/google/protobuf/descriptor_database_unittest.cc index 6211650e7e8da..bea6f12c16b14 100644 --- a/src/google/protobuf/descriptor_database_unittest.cc +++ b/src/google/protobuf/descriptor_database_unittest.cc @@ -447,14 +447,14 @@ TEST_P(DescriptorDatabaseTest, ConflictingExtensionError) { " extendee: \".Foo\" }"); } -INSTANTIATE_TEST_CASE_P( +INSTANTIATE_TEST_SUITE_P( Simple, DescriptorDatabaseTest, testing::Values(&SimpleDescriptorDatabaseTestCase::New)); -INSTANTIATE_TEST_CASE_P( +INSTANTIATE_TEST_SUITE_P( MemoryConserving, DescriptorDatabaseTest, testing::Values(&EncodedDescriptorDatabaseTestCase::New)); -INSTANTIATE_TEST_CASE_P(Pool, DescriptorDatabaseTest, - testing::Values(&DescriptorPoolDatabaseTestCase::New)); +INSTANTIATE_TEST_SUITE_P(Pool, DescriptorDatabaseTest, + testing::Values(&DescriptorPoolDatabaseTestCase::New)); #endif // GTEST_HAS_PARAM_TEST diff --git a/src/google/protobuf/descriptor_lite.h b/src/google/protobuf/descriptor_lite.h index db5805affee3b..e1a90b7fa60d5 100644 --- a/src/google/protobuf/descriptor_lite.h +++ b/src/google/protobuf/descriptor_lite.h @@ -78,6 +78,17 @@ class FieldDescriptorLite { MAX_LABEL = 3, // Constant useful for defining lookup tables // indexed by Label. }; + + // Identifies the storage type of a C++ string field. This corresponds to + // pb.CppFeatures.StringType, but is compatible with ctype prior to Edition + // 2024. 0 is reserved for errors. +#ifndef SWIG + enum class CppStringType { + kView = 1, + kCord = 2, + kString = 3, + }; +#endif }; } // namespace internal diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index f5e2c69ab8d2e..d385700ed5a6b 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -7518,7 +7518,7 @@ TEST_F(FeaturesTest, Proto2Features) { name: "cord" number: 8 label: LABEL_OPTIONAL - type: TYPE_STRING + type: TYPE_BYTES options { ctype: CORD } } field { @@ -7592,6 +7592,11 @@ TEST_F(FeaturesTest, Proto2Features) { EXPECT_TRUE(message->FindFieldByName("req")->is_required()); EXPECT_TRUE(file->enum_type(0)->is_closed()); + EXPECT_EQ(message->FindFieldByName("str")->cpp_string_type(), + FieldDescriptor::CppStringType::kString); + EXPECT_EQ(message->FindFieldByName("cord")->cpp_string_type(), + FieldDescriptor::CppStringType::kCord); + // Check round-trip consistency. FileDescriptorProto proto; file->CopyTo(&proto); @@ -9708,6 +9713,73 @@ TEST_F(FeaturesTest, EnumFeatureHelpers) { EXPECT_FALSE(HasPreservingUnknownEnumSemantics(field_legacy_closed)); } +TEST_F(FeaturesTest, FieldCppStringType) { + BuildDescriptorMessagesInTestPool(); + const std::string file_contents = absl::Substitute( + R"pb( + name: "foo.proto" + syntax: "editions" + edition: EDITION_2024 + message_type { + name: "Foo" + field { + name: "view" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_STRING + } + field { + name: "str" + number: 2 + label: LABEL_OPTIONAL + type: TYPE_STRING + options { + features { + [pb.cpp] { string_type: STRING } + } + } + } + field { + name: "cord" + number: 3 + label: LABEL_OPTIONAL + type: TYPE_STRING + options { + features { + [pb.cpp] { string_type: CORD } + } + } + } + field { + name: "cord_bytes" + number: 4 + label: LABEL_OPTIONAL + type: TYPE_BYTES + options { + features { + [pb.cpp] { string_type: CORD } + } + } + } $0 + } + )pb", + "" + ); + const FileDescriptor* file = BuildFile(file_contents); + const Descriptor* message = file->message_type(0); + const FieldDescriptor* view = message->field(0); + const FieldDescriptor* str = message->field(1); + const FieldDescriptor* cord = message->field(2); + const FieldDescriptor* cord_bytes = message->field(3); + + EXPECT_EQ(view->cpp_string_type(), FieldDescriptor::CppStringType::kView); + EXPECT_EQ(str->cpp_string_type(), FieldDescriptor::CppStringType::kString); + EXPECT_EQ(cord_bytes->cpp_string_type(), + FieldDescriptor::CppStringType::kCord); + EXPECT_EQ(cord->cpp_string_type(), FieldDescriptor::CppStringType::kString); + +} + TEST_F(FeaturesTest, MergeFeatureValidationFailed) { BuildDescriptorMessagesInTestPool(); BuildFileInTestPool(pb::TestFeatures::descriptor()->file()); diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc index 7927107815130..5b9faa88a9779 100644 --- a/src/google/protobuf/dynamic_message.cc +++ b/src/google/protobuf/dynamic_message.cc @@ -117,9 +117,11 @@ int FieldSpaceUsed(const FieldDescriptor* field) { } case FD::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + return sizeof(RepeatedField); + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return sizeof(RepeatedPtrField); } break; @@ -147,9 +149,11 @@ int FieldSpaceUsed(const FieldDescriptor* field) { return sizeof(Message*); case FD::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + return sizeof(absl::Cord); + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return sizeof(ArenaStringPtr); } break; @@ -401,9 +405,31 @@ void DynamicMessage::SharedCtor(bool lock_factory) { break; case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + if (!field->is_repeated()) { + if (field->has_default_value()) { + new (field_ptr) absl::Cord(field->default_value_string()); + } else { + new (field_ptr) absl::Cord; + } + if (arena != nullptr) { + // Cord does not support arena so here we need to notify arena + // to remove the data it allocated on the heap by calling its + // destructor. + arena->OwnDestructor(static_cast(field_ptr)); + } + } else { + new (field_ptr) RepeatedField(arena); + if (arena != nullptr) { + // Needs to destroy Cord elements. + arena->OwnDestructor( + static_cast*>(field_ptr)); + } + } + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (!field->is_repeated()) { ArenaStringPtr* asp = new (field_ptr) ArenaStringPtr(); asp->InitDefault(); @@ -493,9 +519,12 @@ DynamicMessage::~DynamicMessage() { if (*(reinterpret_cast(field_ptr)) == field->number()) { field_ptr = MutableOneofFieldRaw(field); if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) { - switch (field->options().ctype()) { - default: - case FieldOptions::STRING: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + delete *reinterpret_cast(field_ptr); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { reinterpret_cast(field_ptr)->Destroy(); break; } @@ -527,9 +556,13 @@ DynamicMessage::~DynamicMessage() { #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + reinterpret_cast*>(field_ptr) + ->~RepeatedField(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: reinterpret_cast*>(field_ptr) ->~RepeatedPtrField(); break; @@ -547,9 +580,12 @@ DynamicMessage::~DynamicMessage() { } } else if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + reinterpret_cast(field_ptr)->~Cord(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { reinterpret_cast(field_ptr)->Destroy(); break; } diff --git a/src/google/protobuf/edition_unittest.proto b/src/google/protobuf/edition_unittest.proto index 81848b87e4fe6..c0d0ebbbf768f 100644 --- a/src/google/protobuf/edition_unittest.proto +++ b/src/google/protobuf/edition_unittest.proto @@ -91,6 +91,7 @@ message TestAllTypes { string optional_string_piece = 24 [ctype=STRING_PIECE]; string optional_cord = 25 [ctype=CORD]; + bytes optional_bytes_cord = 86 [ctype=CORD]; // Defined in unittest_import_public.proto protobuf_unittest_import.PublicImportMessage @@ -266,6 +267,7 @@ extend TestAllExtensions { // TODO: ctype=CORD is not supported for extension. Add // ctype=CORD option back after it is supported. string optional_cord_extension = 25; + bytes optional_bytes_cord_extension = 86; protobuf_unittest_import.PublicImportMessage optional_public_import_message_extension = 26; diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 4d7d7481057bd..f0dbb847dde26 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -402,9 +402,13 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + total_size += GetRaw>(message, field) + .SpaceUsedExcludingSelfLong(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: total_size += GetRaw >(message, field) .SpaceUsedExcludingSelfLong(); @@ -443,8 +447,8 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { break; case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { total_size += GetField(message, field) ->EstimatedMemoryUsage(); @@ -456,8 +460,8 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { sizeof(absl::Cord); } break; - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { const std::string* ptr = &GetField(message, field).GetNoArena(); @@ -553,15 +557,14 @@ struct OneofFieldMover { to->SetString(from->GetString()); break; } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: to->SetCord(from->GetCord()); break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: to->SetArenaStringPtr(from->GetArenaStringPtr()); break; - } } break; default: @@ -624,9 +627,19 @@ template void SwapFieldHelper::SwapRepeatedStringField(const Reflection* r, Message* lhs, Message* rhs, const FieldDescriptor* field) { - switch (field->options().ctype()) { - default: - case FieldOptions::STRING: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: { + auto* lhs_cord = r->MutableRaw>(lhs, field); + auto* rhs_cord = r->MutableRaw>(rhs, field); + if (unsafe_shallow_swap) { + lhs_cord->InternalSwap(rhs_cord); + } else { + lhs_cord->Swap(rhs_cord); + } + break; + } + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { auto* lhs_string = r->MutableRaw(lhs, field); auto* rhs_string = r->MutableRaw(rhs, field); if (unsafe_shallow_swap) { @@ -690,14 +703,14 @@ template void SwapFieldHelper::SwapStringField(const Reflection* r, Message* lhs, Message* rhs, const FieldDescriptor* field) { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: // Always shallow swap for Cord. std::swap(*r->MutableRaw(lhs, field), *r->MutableRaw(rhs, field)); break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { if (r->IsInlined(field)) { SwapFieldHelper::SwapInlinedStrings(r, lhs, rhs, field); @@ -1169,7 +1182,9 @@ void Reflection::SwapFieldsImpl( // may depend on the information in has bits. if (!field->is_repeated()) { SwapBit(message1, message2, field); - if (field->options().ctype() == FieldOptions::STRING && + if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && + field->cpp_string_type() == + FieldDescriptor::CppStringType::kString && IsInlined(field)) { ABSL_DCHECK(!unsafe_shallow_swap || message1->GetArena() == message2->GetArena()); @@ -1284,9 +1299,10 @@ void Reflection::InternalSwap(Message* lhs, Message* rhs) const { int inlined_string_count = 0; for (int i = 0; i < descriptor_->field_count(); i++) { const FieldDescriptor* field = descriptor_->field(i); + if (field->cpp_type() != FieldDescriptor::CPPTYPE_STRING) continue; if (field->is_extension() || field->is_repeated() || schema_.InRealOneof(field) || - field->options().ctype() != FieldOptions::STRING || + field->cpp_string_type() != FieldDescriptor::CppStringType::kString || !IsInlined(field)) { continue; } @@ -1338,6 +1354,10 @@ int Reflection::FieldSize(const Message& message, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { + return GetRaw >(message, field).size(); + } + ABSL_FALLTHROUGH_INTENDED; case FieldDescriptor::CPPTYPE_MESSAGE: if (IsMapFieldInApi(field)) { const internal::MapFieldBase& map = @@ -1396,8 +1416,8 @@ void Reflection::ClearField(Message* message, break; case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (field->has_default_value()) { *MutableRaw(message, field) = field->default_value_string(); @@ -1405,8 +1425,8 @@ void Reflection::ClearField(Message* message, MutableRaw(message, field)->Clear(); } break; - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { // Currently, string with default value can't be inlined. So we // don't have to handle default value here. @@ -1453,9 +1473,12 @@ void Reflection::ClearField(Message* message, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + MutableRaw>(message, field)->Clear(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: MutableRaw >(message, field)->Clear(); break; } @@ -1503,9 +1526,12 @@ void Reflection::RemoveLast(Message* message, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + MutableRaw>(message, field)->RemoveLast(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: MutableRaw >(message, field) ->RemoveLast(); break; @@ -1597,6 +1623,12 @@ void Reflection::SwapElements(Message* message, const FieldDescriptor* field, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { + MutableRaw >(message, field) + ->SwapElements(index1, index2); + break; + } + ABSL_FALLTHROUGH_INTENDED; case FieldDescriptor::CPPTYPE_MESSAGE: if (IsMapFieldInApi(field)) { MutableRaw(message, field) @@ -1803,15 +1835,15 @@ std::string Reflection::GetString(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return field->default_value_string(); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { return std::string(*GetField(message, field)); } else { return std::string(GetField(message, field)); } - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { return GetField(message, field).GetNoArena(); } else { @@ -1819,6 +1851,7 @@ std::string Reflection::GetString(const Message& message, return str.IsDefault() ? field->default_value_string() : str.Get(); } } + internal::Unreachable(); } } @@ -1834,8 +1867,8 @@ const std::string& Reflection::GetStringReference(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return field->default_value_string(); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { absl::CopyCordToString(*GetField(message, field), scratch); @@ -1843,8 +1876,8 @@ const std::string& Reflection::GetStringReference(const Message& message, absl::CopyCordToString(GetField(message, field), scratch); } return *scratch; - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { return GetField(message, field).GetNoArena(); } else { @@ -1852,6 +1885,7 @@ const std::string& Reflection::GetStringReference(const Message& message, return str.IsDefault() ? field->default_value_string() : str.Get(); } } + internal::Unreachable(); } } @@ -1865,15 +1899,15 @@ absl::Cord Reflection::GetCord(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return absl::Cord(field->default_value_string()); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { return *GetField(message, field); } else { return GetField(message, field); } - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { return absl::Cord( GetField(message, field).GetNoArena()); @@ -1883,9 +1917,7 @@ absl::Cord Reflection::GetCord(const Message& message, : str.Get()); } } - - ABSL_LOG(FATAL) << "Can't get here."; - return absl::Cord(); // Make compiler happy. + internal::Unreachable(); } } @@ -1902,8 +1934,8 @@ absl::string_view Reflection::GetStringView(const Message& message, return field->default_value_string(); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: { const auto& cord = schema_.InRealOneof(field) ? *GetField(message, field) : GetField(message, field); @@ -1923,8 +1955,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, return MutableExtensionSet(message)->SetString( field->number(), field->type(), std::move(value), field); } else { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { if (!HasOneofField(*message, field)) { ClearOneof(message, field->containing_oneof()); @@ -1936,8 +1968,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, } *MutableField(message, field) = value; break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { if (IsInlined(field)) { const uint32_t index = schema_.InlinedStringIndex(field); ABSL_DCHECK_GT(index, 0u); @@ -1975,8 +2007,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, MutableExtensionSet(message)->MutableString( field->number(), field->type(), field)); } else { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { if (!HasOneofField(*message, field)) { ClearOneof(message, field->containing_oneof()); @@ -1988,8 +2020,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, *MutableField(message, field) = value; } break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { // Oneof string fields are never set as a default instance. // We just need to pass some arbitrary default string to make it work. // This allows us to not have the real default accessible from @@ -2025,11 +2057,14 @@ std::string Reflection::GetRepeatedString(const Message& message, if (field->is_extension()) { return GetExtensionSet(message).GetRepeatedString(field->number(), index); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + return std::string(GetRepeatedField(message, field, index)); + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetRepeatedPtrField(message, field, index); } + internal::Unreachable(); } } @@ -2041,11 +2076,16 @@ const std::string& Reflection::GetRepeatedStringReference( if (field->is_extension()) { return GetExtensionSet(message).GetRepeatedString(field->number(), index); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + absl::CopyCordToString( + GetRepeatedField(message, field, index), scratch); + return *scratch; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetRepeatedPtrField(message, field, index); } + internal::Unreachable(); } } @@ -2060,11 +2100,16 @@ absl::string_view Reflection::GetRepeatedStringView( return GetExtensionSet(message).GetRepeatedString(field->number(), index); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::STRING: - default: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: { + auto& cord = GetRepeatedField(message, field, index); + return scratch.CopyFromCord(cord); + } + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetRepeatedPtrField(message, field, index); } + internal::Unreachable(); } @@ -2076,9 +2121,12 @@ void Reflection::SetRepeatedString(Message* message, MutableExtensionSet(message)->SetRepeatedString(field->number(), index, std::move(value)); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + SetRepeatedField(message, field, index, absl::Cord(value)); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: MutableRepeatedField(message, field, index) ->assign(std::move(value)); break; @@ -2094,9 +2142,12 @@ void Reflection::AddString(Message* message, const FieldDescriptor* field, MutableExtensionSet(message)->AddString(field->number(), field->type(), std::move(value), field); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + AddField(message, field, absl::Cord(value)); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: AddField(message, field)->assign(std::move(value)); break; } @@ -2748,7 +2799,7 @@ static Type* AllocIfDefault(const FieldDescriptor* field, Type*& ptr, // be e.g. char). if (field->cpp_type() < FieldDescriptor::CPPTYPE_STRING || (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD)) { + field->cpp_string_type() == FieldDescriptor::CppStringType::kCord)) { ptr = reinterpret_cast(Arena::Create>(arena)); } else { @@ -2927,11 +2978,11 @@ bool Reflection::HasBit(const Message& message, // (which uses HasField()) needs to be consistent with this. switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_STRING: - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: return !GetField(message, field).empty(); - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { if (IsInlined(field)) { return !GetField(message, field) .GetNoArena() @@ -2941,7 +2992,7 @@ bool Reflection::HasBit(const Message& message, return GetField(message, field).Get().size() > 0; } } - return false; + internal::Unreachable(); case FieldDescriptor::CPPTYPE_BOOL: return GetRaw(message, field) != false; case FieldDescriptor::CPPTYPE_INT32: @@ -3042,12 +3093,12 @@ void Reflection::ClearOneof(Message* message, if (message->GetArena() == nullptr) { switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: delete *MutableRaw(message, field); break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { // Oneof string fields are never set as a default instance. // We just need to pass some arbitrary default string to make it // work. This allows us to not have the real default accessible diff --git a/src/google/protobuf/generated_message_tctable_gen.cc b/src/google/protobuf/generated_message_tctable_gen.cc index 577c6f50cb0f4..0b6a1c38a4cf2 100644 --- a/src/google/protobuf/generated_message_tctable_gen.cc +++ b/src/google/protobuf/generated_message_tctable_gen.cc @@ -168,10 +168,10 @@ TailCallTableInfo::FastFieldInfo::Field MakeFastFieldEntry( : field->is_repeated() ? PROTOBUF_PICK_FUNCTION(fn##R) \ : PROTOBUF_PICK_FUNCTION(fn##S)) -#define PROTOBUF_PICK_STRING_FUNCTION(fn) \ - (field->options().ctype() == FieldOptions::CORD \ - ? PROTOBUF_PICK_FUNCTION(fn##cS) \ - : options.is_string_inlined ? PROTOBUF_PICK_FUNCTION(fn##iS) \ +#define PROTOBUF_PICK_STRING_FUNCTION(fn) \ + (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord \ + ? PROTOBUF_PICK_FUNCTION(fn##cS) \ + : options.is_string_inlined ? PROTOBUF_PICK_FUNCTION(fn##iS) \ : PROTOBUF_PICK_REPEATABLE_FUNCTION(fn)) const FieldDescriptor* field = entry.field; @@ -304,10 +304,12 @@ bool IsFieldEligibleForFastParsing( switch (field->type()) { // Some bytes fields can be handled on fast path. case FieldDescriptor::TYPE_STRING: - case FieldDescriptor::TYPE_BYTES: - if (field->options().ctype() == FieldOptions::STRING) { + case FieldDescriptor::TYPE_BYTES: { + auto ctype = field->cpp_string_type(); + if (ctype == FieldDescriptor::CppStringType::kString || + ctype == FieldDescriptor::CppStringType::kView) { // strings are fine... - } else if (field->options().ctype() == FieldOptions::CORD) { + } else if (ctype == FieldDescriptor::CppStringType::kCord) { // Cords are worth putting into the fast table, if they're not repeated if (field->is_repeated()) return false; } else { @@ -321,6 +323,7 @@ bool IsFieldEligibleForFastParsing( aux_idx = entry.inlined_string_idx; } break; + } case FieldDescriptor::TYPE_ENUM: { uint8_t rmax_value; @@ -760,12 +763,13 @@ uint16_t MakeTypeCardForField( // Fill in extra information about string and bytes field representations. if (field->type() == FieldDescriptor::TYPE_BYTES || field->type() == FieldDescriptor::TYPE_STRING) { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: // `Cord` is always used, even for repeated fields. type_card |= fl::kRepCord; break; - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (field->is_repeated()) { // A repeated string field uses RepeatedPtrField // (unless it has a ctype option; see above). @@ -775,8 +779,6 @@ uint16_t MakeTypeCardForField( type_card |= fl::kRepAString; } break; - default: - Unreachable(); } } diff --git a/src/google/protobuf/generated_message_tctable_lite.cc b/src/google/protobuf/generated_message_tctable_lite.cc index 9db970e23fc71..4981946dca5f1 100644 --- a/src/google/protobuf/generated_message_tctable_lite.cc +++ b/src/google/protobuf/generated_message_tctable_lite.cc @@ -1641,6 +1641,12 @@ bool TcParser::ChangeOneof(const TcParseTableBase* table, field.Destroy(); break; } + case field_layout::kRepCord: { + if (msg->GetArena() == nullptr) { + delete RefAt(msg, current_entry->offset); + } + break; + } case field_layout::kRepSString: case field_layout::kRepIString: default: diff --git a/src/google/protobuf/io/BUILD.bazel b/src/google/protobuf/io/BUILD.bazel index 096ebf71b863b..fca18ed8d415d 100644 --- a/src/google/protobuf/io/BUILD.bazel +++ b/src/google/protobuf/io/BUILD.bazel @@ -191,9 +191,6 @@ cc_test( "zero_copy_stream_unittest.cc", ], copts = COPTS, - data = [ - "//src/google/protobuf:testdata", - ], deps = [ ":gzip_stream", ":io", @@ -202,7 +199,9 @@ cc_test( ":tokenizer", "//:protobuf", "//src/google/protobuf", + "//src/google/protobuf:cc_test_protos", "//src/google/protobuf:port", + "//src/google/protobuf:test_util", "//src/google/protobuf:test_util2", "//src/google/protobuf/stubs", "//src/google/protobuf/testing", diff --git a/src/google/protobuf/io/zero_copy_stream_unittest.cc b/src/google/protobuf/io/zero_copy_stream_unittest.cc index 453c825be17c1..c42ec4676ae65 100644 --- a/src/google/protobuf/io/zero_copy_stream_unittest.cc +++ b/src/google/protobuf/io/zero_copy_stream_unittest.cc @@ -56,12 +56,15 @@ #include "google/protobuf/io/coded_stream.h" #include "google/protobuf/io/io_win32.h" #include "google/protobuf/io/zero_copy_stream_impl.h" +#include "google/protobuf/test_util.h" #include "google/protobuf/test_util2.h" +#include "google/protobuf/unittest.pb.h" #if HAVE_ZLIB #include "google/protobuf/io/gzip_stream.h" #endif +#include "google/protobuf/test_util.h" // Must be included last. #include "google/protobuf/port_def.inc" @@ -557,10 +560,9 @@ std::string IoTest::Uncompress(const std::string& data) { TEST_F(IoTest, CompressionOptions) { // Some ad-hoc testing of compression options. - std::string golden_filename = - TestUtil::GetTestDataPath("google/protobuf/testdata/golden_message"); - std::string golden; - ABSL_CHECK_OK(File::GetContents(golden_filename, &golden, true)); + protobuf_unittest::TestAllTypes message; + TestUtil::SetAllFields(&message); + std::string golden = message.SerializeAsString(); GzipOutputStream::Options options; std::string gzip_compressed = Compress(golden, options); diff --git a/src/google/protobuf/lite_unittest.cc b/src/google/protobuf/lite_unittest.cc index 978307528a795..cfa65b6740503 100644 --- a/src/google/protobuf/lite_unittest.cc +++ b/src/google/protobuf/lite_unittest.cc @@ -957,6 +957,18 @@ TYPED_TEST(LiteTest, AllLite43) { EXPECT_TRUE(message2.MergeFromCodedStream(&input_stream)); EXPECT_EQ(17, message2.oneof_int32()); } + + // Bytes [ctype = CORD] + { + protobuf_unittest::TestOneofParsingLite message2; + message2.set_oneof_bytes_cord("bytes cord"); + io::CodedInputStream input_stream( + reinterpret_cast(serialized.data()), + serialized.size()); + EXPECT_TRUE(message2.MergeFromCodedStream(&input_stream)); + EXPECT_EQ(17, message2.oneof_int32()); + } + } // Verify that we can successfully parse fields of various types within oneof @@ -1043,6 +1055,18 @@ TYPED_TEST(LiteTest, AllLite44) { } } + // Bytes [ctype = CORD] + { + protobuf_unittest::TestOneofParsingLite original; + original.set_oneof_bytes_cord("bytes cord"); + std::string serialized; + EXPECT_TRUE(original.SerializeToString(&serialized)); + protobuf_unittest::TestOneofParsingLite parsed; + EXPECT_TRUE(parsed.MergeFromString(serialized)); + EXPECT_EQ("bytes cord", std::string(parsed.oneof_bytes_cord())); + EXPECT_TRUE(parsed.MergeFromString(serialized)); + } + std::cout << "PASS" << std::endl; } diff --git a/src/google/protobuf/map_test.inc b/src/google/protobuf/map_test.inc index cb396b79e0d6a..fe66d9c49fd1f 100644 --- a/src/google/protobuf/map_test.inc +++ b/src/google/protobuf/map_test.inc @@ -3923,16 +3923,35 @@ TEST(MapSerializationTest, Deterministic) { EXPECT_TRUE(util::MessageDifferencer::Equals(u, t)); } +static std::string GetGoldenMessageTextProto() { + static std::string* golden_message_textproto = [] { + std::string* textproto = new std::string; + ABSL_CHECK_OK(File::GetContents( + TestUtil::GetTestDataPath("google/protobuf/" + "testdata/map_test_data.txt"), + textproto, true)); + return textproto; + }(); + return *golden_message_textproto; +} + +static std::string GetGoldenMessageBinary() { + static std::string* golden_message_binary = [] { + UNITTEST::TestMaps t; + TextFormat::Parser parser; + parser.ParseFromString(GetGoldenMessageTextProto(), &t); + + std::string* result = new std::string; + t.SerializeToString(result); + return result; + }(); + return *golden_message_binary; +} + TEST(MapSerializationTest, DeterministicSubmessage) { UNITTEST::TestSubmessageMaps p; UNITTEST::TestMaps t; - const std::string filename = "golden_message_maps"; - std::string golden; - ABSL_CHECK_OK( - File::GetContents(TestUtil::GetTestDataPath(absl::StrCat( - "google/protobuf/testdata/", filename)), - &golden, true)); - t.ParseFromString(golden); + t.ParseFromString(GetGoldenMessageBinary()); *(p.mutable_m()) = t; std::vector v; // Use multiple attempts to increase the chance of a failure if something is @@ -3970,15 +3989,9 @@ TEST(TextFormatMapTest, DynamicMessage) { MapReflectionTester tester(message->GetDescriptor()); tester.SetMapFieldsViaReflection(message.get()); - std::string expected_text; - ABSL_CHECK_OK( - File::GetContents(TestUtil::GetTestDataPath("google/protobuf/" - "testdata/map_test_data.txt"), - &expected_text, true)); - std::string actual_text; TextFormat::PrintToString(*message, &actual_text); - EXPECT_EQ(actual_text, expected_text); + EXPECT_EQ(actual_text, GetGoldenMessageTextProto()); } TEST(TextFormatMapTest, Sorted) { @@ -3986,35 +3999,17 @@ TEST(TextFormatMapTest, Sorted) { MapReflectionTester tester(message.GetDescriptor()); tester.SetMapFieldsViaReflection(&message); - std::string expected_text; - ABSL_CHECK_OK( - File::GetContents(TestUtil::GetTestDataPath("google/protobuf/" - "testdata/map_test_data.txt"), - &expected_text, true)); - TextFormat::Printer printer; std::string actual_text; printer.PrintToString(message, &actual_text); - EXPECT_EQ(actual_text, expected_text); + EXPECT_EQ(actual_text, GetGoldenMessageTextProto()); // Test again on the reverse order. UNITTEST::TestMap message2; tester.SetMapFieldsViaReflection(&message2); tester.SwapMapsViaReflection(&message2); printer.PrintToString(message2, &actual_text); - EXPECT_EQ(actual_text, expected_text); -} - -TEST(TextFormatMapTest, ParseCorruptedString) { - std::string serialized_message; - ABSL_CHECK_OK(File::GetContents( - TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_message_maps"), - &serialized_message, true)); - UNITTEST::TestMaps message; - ABSL_CHECK(message.ParseFromString(serialized_message)); - TestParseCorruptedString(message); - TestParseCorruptedString(message); + EXPECT_EQ(actual_text, GetGoldenMessageTextProto()); } // Previously, serializing to text format will disable iterator from generated diff --git a/src/google/protobuf/message.cc b/src/google/protobuf/message.cc index caf941ad5b4a6..ffeac4da97137 100644 --- a/src/google/protobuf/message.cc +++ b/src/google/protobuf/message.cc @@ -456,9 +456,11 @@ const internal::RepeatedFieldAccessor* Reflection::RepeatedFieldAccessor( HANDLE_PRIMITIVE_TYPE(ENUM, int32_t) #undef HANDLE_PRIMITIVE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + ABSL_LOG(FATAL) << "Repeated cords are not supported."; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetSingleton(); } break; diff --git a/src/google/protobuf/message_unittest.inc b/src/google/protobuf/message_unittest.inc index 08b00a482f64a..76a59aa010a18 100644 --- a/src/google/protobuf/message_unittest.inc +++ b/src/google/protobuf/message_unittest.inc @@ -28,6 +28,8 @@ #include #include +#include "google/protobuf/testing/file.h" +#include "google/protobuf/testing/file.h" #include "google/protobuf/descriptor.pb.h" #include #include "google/protobuf/testing/googletest.h" @@ -142,8 +144,12 @@ TEST(MESSAGE_TEST_NAME, SerializeToBrokenOstream) { } TEST(MESSAGE_TEST_NAME, ParseFromFileDescriptor) { - std::string filename = - TestUtil::GetTestDataPath("google/protobuf/testdata/golden_message"); + std::string filename = absl::StrCat(TestTempDir(), "/golden_message"); + UNITTEST::TestAllTypes expected_message; + TestUtil::SetAllFields(&expected_message); + ABSL_CHECK_OK(File::SetContents( + filename, expected_message.SerializeAsString(), true)); + int file = open(filename.c_str(), O_RDONLY | O_BINARY); ASSERT_GE(file, 0); @@ -155,8 +161,12 @@ TEST(MESSAGE_TEST_NAME, ParseFromFileDescriptor) { } TEST(MESSAGE_TEST_NAME, ParsePackedFromFileDescriptor) { - std::string filename = TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_packed_fields_message"); + std::string filename = absl::StrCat(TestTempDir(), "/golden_message"); + UNITTEST::TestPackedTypes expected_message; + TestUtil::SetPackedFields(&expected_message); + ABSL_CHECK_OK(File::SetContents( + filename, expected_message.SerializeAsString(), true)); + int file = open(filename.c_str(), O_RDONLY | O_BINARY); ASSERT_GE(file, 0); diff --git a/src/google/protobuf/string_view_test.cc b/src/google/protobuf/string_view_test.cc index 1cdb860d94416..d348f1fbee871 100644 --- a/src/google/protobuf/string_view_test.cc +++ b/src/google/protobuf/string_view_test.cc @@ -12,6 +12,9 @@ #include "google/protobuf/text_format.h" #include "google/protobuf/unittest_string_view.pb.h" +// Must be included last. +#include "google/protobuf/port_def.inc" + namespace google { namespace protobuf { namespace { @@ -284,10 +287,12 @@ TEST(StringViewFieldTest, RepeatedSetAndGetByReflection) { } // MutableRepeatedPtrField(). + PROTOBUF_IGNORE_DEPRECATION_START; for (auto& it : *reflection->MutableRepeatedPtrField(&message, field)) { it.append(it); } + PROTOBUF_IGNORE_DEPRECATION_STOP; { const auto& rep_str = reflection->GetRepeatedFieldRef(message, field); @@ -309,3 +314,5 @@ TEST(StringViewFieldTest, RepeatedSetAndGetByReflection) { } // namespace } // namespace protobuf } // namespace google + +#include "google/protobuf/port_undef.inc" diff --git a/src/google/protobuf/test_util.h b/src/google/protobuf/test_util.h index c52c38fc4589b..4f6e07ed34936 100644 --- a/src/google/protobuf/test_util.h +++ b/src/google/protobuf/test_util.h @@ -223,6 +223,8 @@ inline void TestUtil::ReflectionTester::SetAllFieldsViaReflection( reflection->SetString(message, F("optional_string_piece"), "124"); reflection->SetString(message, F("optional_cord"), "125"); + reflection->SetString(message, F("optional_bytes_cord"), + "optional bytes cord"); sub_message = reflection->MutableMessage(message, F("optional_public_import_message")); @@ -491,6 +493,7 @@ inline void TestUtil::ReflectionTester::ExpectAllFieldsSetViaReflection1( EXPECT_TRUE(reflection->HasField(message, F("optional_string_piece"))); EXPECT_TRUE(reflection->HasField(message, F("optional_cord"))); + EXPECT_TRUE(reflection->HasField(message, F("optional_bytes_cord"))); EXPECT_EQ(101, reflection->GetInt32(message, F("optional_int32"))); EXPECT_EQ(102, reflection->GetInt64(message, F("optional_int64"))); @@ -552,6 +555,14 @@ inline void TestUtil::ReflectionTester::ExpectAllFieldsSetViaReflection1( EXPECT_EQ("125", reflection->GetStringReference(message, F("optional_cord"), &scratch)); + EXPECT_EQ("optional bytes cord", + reflection->GetString(message, F("optional_bytes_cord"))); + EXPECT_EQ("optional bytes cord", + reflection->GetStringReference(message, F("optional_bytes_cord"), + &scratch)); + EXPECT_EQ("optional bytes cord", + reflection->GetCord(message, F("optional_bytes_cord"))); + EXPECT_TRUE(reflection->HasField(message, F("oneof_bytes"))); EXPECT_EQ("604", reflection->GetString(message, F("oneof_bytes"))); @@ -912,6 +923,7 @@ inline void TestUtil::ReflectionTester::ExpectClearViaReflection( EXPECT_FALSE(reflection->HasField(message, F("optional_string_piece"))); EXPECT_FALSE(reflection->HasField(message, F("optional_cord"))); + EXPECT_FALSE(reflection->HasField(message, F("optional_bytes_cord"))); // Optional fields without defaults are set to zero or something like it. EXPECT_EQ(0, reflection->GetInt32(message, F("optional_int32"))); @@ -978,6 +990,11 @@ inline void TestUtil::ReflectionTester::ExpectClearViaReflection( EXPECT_EQ("", reflection->GetStringReference(message, F("optional_cord"), &scratch)); + EXPECT_EQ("", reflection->GetString(message, F("optional_bytes_cord"))); + EXPECT_EQ("", reflection->GetStringReference( + message, F("optional_bytes_cord"), &scratch)); + EXPECT_EQ("", reflection->GetCord(message, F("optional_bytes_cord"))); + // Repeated fields are empty. EXPECT_EQ(0, reflection->FieldSize(message, F("repeated_int32"))); EXPECT_EQ(0, reflection->FieldSize(message, F("repeated_int64"))); diff --git a/src/google/protobuf/test_util.inc b/src/google/protobuf/test_util.inc index 77ea70ba2e905..f58f951d324a4 100644 --- a/src/google/protobuf/test_util.inc +++ b/src/google/protobuf/test_util.inc @@ -137,6 +137,7 @@ inline void TestUtil::SetOptionalFields(UNITTEST::TestAllTypes* message) { message, message->GetDescriptor()->FindFieldByName("optional_cord"), "125"); #endif // !PROTOBUF_TEST_NO_DESCRIPTORS + message->set_optional_bytes_cord("optional bytes cord"); } // ------------------------------------------------------------------- @@ -352,6 +353,7 @@ inline void TestUtil::ExpectAllFieldsSet( EXPECT_TRUE(message.has_optional_string_piece()); EXPECT_TRUE(message.has_optional_cord()); #endif + EXPECT_TRUE(message.has_optional_bytes_cord()); EXPECT_EQ(101, message.optional_int32()); EXPECT_EQ(102, message.optional_int64()); @@ -381,6 +383,7 @@ inline void TestUtil::ExpectAllFieldsSet( EXPECT_EQ(UNITTEST::FOREIGN_BAZ, message.optional_foreign_enum()); EXPECT_EQ(UNITTEST_IMPORT::IMPORT_BAZ, message.optional_import_enum()); + EXPECT_EQ("optional bytes cord", message.optional_bytes_cord()); // ----------------------------------------------------------------- @@ -554,6 +557,7 @@ inline void TestUtil::ExpectClear(const UNITTEST::TestAllTypes& message) { EXPECT_FALSE(message.has_optional_string_piece()); EXPECT_FALSE(message.has_optional_cord()); + EXPECT_FALSE(message.has_optional_bytes_cord()); // Optional fields without defaults are set to zero or something like it. EXPECT_EQ(0, message.optional_int32()); @@ -594,6 +598,7 @@ inline void TestUtil::ExpectClear(const UNITTEST::TestAllTypes& message) { EXPECT_EQ(UNITTEST::FOREIGN_FOO, message.optional_foreign_enum()); EXPECT_EQ(UNITTEST_IMPORT::IMPORT_FOO, message.optional_import_enum()); + EXPECT_EQ("", message.optional_bytes_cord()); // Repeated fields are empty. EXPECT_EQ(0, message.repeated_int32_size()); @@ -975,6 +980,8 @@ inline void TestUtil::SetAllExtensions(UNITTEST::TestAllExtensions* message) { message->SetExtension(UNITTEST::optional_string_piece_extension, "124"); message->SetExtension(UNITTEST::optional_cord_extension, "125"); + message->SetExtension(UNITTEST::optional_bytes_cord_extension, + "optional bytes cord"); message->MutableExtension(UNITTEST::optional_public_import_message_extension) ->set_e(126); @@ -1204,6 +1211,7 @@ inline void TestUtil::ExpectAllExtensionsSet( EXPECT_TRUE(message.HasExtension(UNITTEST::optional_string_piece_extension)); EXPECT_TRUE(message.HasExtension(UNITTEST::optional_cord_extension)); + EXPECT_TRUE(message.HasExtension(UNITTEST::optional_bytes_cord_extension)); EXPECT_EQ(101, message.GetExtension(UNITTEST::optional_int32_extension)); EXPECT_EQ(102, message.GetExtension(UNITTEST::optional_int64_extension)); @@ -1242,6 +1250,8 @@ inline void TestUtil::ExpectAllExtensionsSet( EXPECT_EQ("124", message.GetExtension(UNITTEST::optional_string_piece_extension)); EXPECT_EQ("125", message.GetExtension(UNITTEST::optional_cord_extension)); + EXPECT_EQ("optional bytes cord", + message.GetExtension(UNITTEST::optional_bytes_cord_extension)); EXPECT_EQ( 126, message.GetExtension(UNITTEST::optional_public_import_message_extension) @@ -1490,6 +1500,7 @@ inline void TestUtil::ExpectExtensionsClear( EXPECT_FALSE(message.HasExtension(UNITTEST::optional_string_piece_extension)); EXPECT_FALSE(message.HasExtension(UNITTEST::optional_cord_extension)); + EXPECT_FALSE(message.HasExtension(UNITTEST::optional_bytes_cord_extension)); // Optional fields without defaults are set to zero or something like it. EXPECT_EQ(0, message.GetExtension(UNITTEST::optional_int32_extension)); @@ -1557,6 +1568,7 @@ inline void TestUtil::ExpectExtensionsClear( EXPECT_EQ("", message.GetExtension(UNITTEST::optional_string_piece_extension)); EXPECT_EQ("", message.GetExtension(UNITTEST::optional_cord_extension)); + EXPECT_EQ("", message.GetExtension(UNITTEST::optional_bytes_cord_extension)); // Repeated fields are empty. EXPECT_EQ(0, message.ExtensionSize(UNITTEST::repeated_int32_extension)); diff --git a/src/google/protobuf/testdata/golden_message b/src/google/protobuf/testdata/golden_message deleted file mode 100644 index 5825975ce07bde310f5618aab42bc578739a65f0..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 537 zcmXxfJxmlq6bJBm|2Mn0be5Kumh8uDXL5Qq1)?PEtPqJ%B(4+*LNb#9cY@vulDqNB$M^I{{ASSO0Cz7R-@%;ZZlRpEUL3O}^Z^Y0y zzip1i(2xJ3i*vew{Wfrc1!k{J*7sNvzmjzqCGi_s_mHtHuCP;Kx5{3X2X>f)AQHnM zdS|l{!@xA<{~u)ifM2}Kf4Ib5e$I)0!Kq$%i+aO-tzXI{Z*0jV)pfbwH?&HQ y3j4Y@+1_0KqgE}=k#E}8ICZG#=*-EjIUEJiy47^ubG?w0N%T^V_I^i;cA+cvq`u4VOJS%xYprl7H1=6px9w z%;U$`0bCY*>cZsUx%~J#=fV!gZ)v1p#27Pve0|KA?^{l9ADGmbIsEuK7;`dzv|@7b z!~FO<7<1xm%~c9UjA8llb!vAkFSrpoZeI& zH9V=y)A{jraJjwcK(_WwCD>sp`0;gcxv{q8E(Ig@oAcxA)EFyPrg(H>4oF#smyH2p z3%flO^Cn^}*qJ<8;T+k^gWS!p2F~EMOPmcm6kc0w zhC$@Kb_pPj9T|-%WHzr|0!Y%p?z2-cDYJO(57| z$!K;2B?bjZCOH38;KAZGe1IBOU4enq*YIu!g`-5|&Fl6S5AVe(J)uNPFTvg(Jb4Xgcd1 zJ4hvY(%jJ!T8T=fc?39;+nL#UevK%O=03PYnDk0eXExvvS{5=b2qlS?efaS9R#YfW zJjEhyRt1LD?1REXDpW7EKq{#Y|J*Y_Lo-3jlva4WQ^yL;c^d9pe5#d<2F|JojVL2iPG{mIEJ2vnyN?2txdw zt*uF4VMd7if~5@`RID=C87%GC#`b?1Q*G(M4yPQSg+*2`_beB&*W>&yqV+G15t!$f z7T1j?DJ4{Z?9VMqLK_7wO@M^eoGwHmG}XcUp}eTuW|*78g0ysG4}@R3XNLKymq8$~ zJT`O36ilEnEg>F>C0A3XV8c-jlnI`bjapLNW%u=@GY6q?lfD zc0&X+uk`IP$MTf&8bqeI?sB;~78yN41B@7iO_uVSp7u0lux#14Tz)!>pi%296yWI#G18qpRmN%rRk=gT>cFqYI9w_cgzc zSu9RO*m2>;=r^5_u@z`r7jqJr-MM$9AnsIC=?2Vfn!W(5jqa&Ec&X>?CJaW)3nF&3 zv@Y%R?Uv9$bUYIFI{;Vz*w>bRuQogw1q+!Lz{!3Qdz&u#7004rA*D>F9i>*E_({{?Urz;k{w0IjE&V zp0Rad%e!}sM3fx4tTC~xf^(g@(amuwnRhBs=V)tHE@F?@Bd$)R13L;cB}$CK zVgp=%XLr+dEO&)R0NO(|{iV}Q%Uh6In$y`d4U;OY2*3?)_ig4doG`UOyMKG_4;-dl zTmyj4+}08#!_Z=G2K2tW)i+Sq70*CmaYE7MN^{I&VIlx7H8?%W=)?iqORKVD%rINU z45qYV6$ZPA(wfcfY4SCY{7OeQqdy*IE973Hm-k8=c0_Zy7QN6@@&m~I__R^<5>ERH zK#tY_atxhSC>H_bhn~X`XcMCYAwYUU{I{YFjP^P3t8ZK95f<}HkJv$0O=w*!(Dtq?K*6&6dyNkMe4V;|HfepG= z{Y}xYoe{MtQP7a9T(reN0fua6{B+2)@%DnZs<9bp#+X43#hL zV<4YY&Az=mN8rs0xn>JX2mHJjS-!jS>hWlxXN2u_VFOCKxAS6DsDI~oD2?kKI|O-a z-%w9pa`vsaFrhS!0OnglMl)Zf)Y9hpJ4w5r#{3adCx|@MyYF1ozJ1r1 zVIpbLL5!l)legb(xs>f>G^qp z0~TW*3hNO(JaB6B<3>{m#Ff7=U4@AgrX>U-(Yk**76>uXx?9O3*SH&a*axMaA63$Z(wSXq_mf7rf&Ka zb4AD$U|rt}Eni~RQDy{0*}B=O7=pGuK-9Tb9>fr|SwLJ%xNqw)V}!j29L%|$-+(zN z?rNabEaYZ4rj^z`hzn5<^kd@0?CR+t^DTIUAs>T#heH}^v-ERZv+9tgmv$4`9d_fZAHa{uJ-v~cY6+`f3^g^XG07zgT2 z7GPojzq5X}SmBd{Stw)^IG^%WWiQDJGc;JGiu)uvpLK!f-O8_3U@p;K>E$f{MOomB zi4>E+m%ZF0EItA=Rmj2sd#3DSGzJsK98?d^EQ!Tbi}?{`ey^%Zz+?)08NfWR7N=q` zVG@DrKiWqg!c>ds!lMUR`-0F`3?@u> zQ2p19f9b?jiyNw!tz2fTIe1ygAANH^J9(;mImj;$m(DSo=3dtFCo}%O082wUGyq~K zs23f`XtMxBZNe`40?O3y*Q?9)d-#B}LR#~>+iZPeG8Bs!4zER>OhatodjF!^Em*vUwU zEp6K)Fi|wjK1*bzGCY=h?7jvLsOt!E_z&jG8ur2Q%z`WB1N+~19d+FDQGO@7h9Wh8G$+9VGb|of}CJdlRY9}>F z8!;43Z*YV#;uHD*Xv7>5vLXm1nYA%zek&%BW|xmrMsnx9m%ZCDNA&UsB$B$wurD6P zB+}aOagY&0ARmJhBOD@OLvw;N>7oz5lcw=S6gCd9l+=uCo5Ak}fIvcrH$Pru z^s507CPGl(uK&vTHv_;i(g{z#lw#r+17HK8`gNQ0O#EH|aSteL?H^Yg`dR?EM_QQo zMk}M=3IGcT#*4na04`J;B*3k2V+K#h;qksD;_yc)Qqcoyk#Lc8A3H=yC zFCj7?-oU(Q;?-B}!a=fXH{fX~<|(xkHY$|LGTGj59+10bDczt^9Jf<&^(7V?u(V+rX5(5RDK_ zDdWUE%sht$Q#jzN-MG0FI?*PhTcJHoWu)pg4A!3X9u3rsIXDLbB@s3bx96sS1DFBA z+oRfnlLE{*4w695yvRT_koHfnVk8M3fD9#p4(!O8%iY$^LRAZAe6=e#TMINLxOy9* z`OaV%Ys_Lg{;KUcmO!*2MI6(?mT}Wm%U7&ew$gRw(xt9g28rJU)SqzPf=v^x|8IXc zQ7O!6%b9r$4qDk%4 z5UZ5Qm=vfBSZ#!c0SjTM4u*+DhK`(D>dC#o_doZ(;U0JH(v7j3+e)da zdBN7q9bxwFAS-oEspIME7tPgdo)*~i%#sj2rG)N1qawk*UlJZvgojn)HOhRZ(XYgw zAGWNfBR7w`!g7z;jj2!HWY-X*MKL}i!FNjV2?hM10@jeC#3_2ZjJI6IA6>&wc|i>U zDOJbr@M(2~(%WiaZ=jLBiO%56rf7KIhEi8@V!V>ulPneA%KlG_*#po4R1fTXMhrZ2 zVsj)0Ui1%r{Gv11Zv$s|!t8d*x`F57Az3$36pzTdg_LD+nVm9cs_a&Ibe-AvLox8f zFE$%7@J&np|4r6y{N)#X%0=$;OOEv`PV{P~pg&~Z>DMyJdz}vFsp&V8>bf-S8(PKf nTvHDw+j^AU(W;|4^i0)CYT+ diff --git a/src/google/protobuf/testdata/golden_message_proto3 b/src/google/protobuf/testdata/golden_message_proto3 deleted file mode 100644 index bd646a0dc85e0b8c7cfcfbea8bb9ae0a8987883d..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 248 zcmXZSJ8r^25C-6xe>TCTkAio-u918M&X6W;E?`l*bm^pWiNr&c6o6=uoJavmpfag) zgjAVzGQ~IlJbqZ7W|+XVJVy!h&I?p9AG|~jQ%h$Wm^b+{Q$(Lcq1gdU SJG41$Vq;=t*u}=g$_4<-G94WN diff --git a/src/google/protobuf/testdata/text_format_unittest_data.txt b/src/google/protobuf/testdata/text_format_unittest_data.txt index 7a874b54cdba0..b218f291b067f 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data.txt @@ -126,6 +126,7 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_uint32: 601 oneof_nested_message { bb: 602 diff --git a/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt b/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt index 86389c93cbced..c477bbef1b917 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt @@ -129,4 +129,5 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_bytes: "604" diff --git a/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt b/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt index 788025c515306..d46ec7bb3a4f6 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt @@ -129,6 +129,7 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_uint32: 601 oneof_nested_message < bb: 602 diff --git a/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt b/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt index b2d3367098ce2..139e3df348e7c 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt @@ -129,4 +129,5 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_bytes: "604" diff --git a/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt b/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt index 5c3a03ac3708c..8ab4d2e85b0c0 100644 --- a/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt +++ b/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt @@ -129,6 +129,7 @@ [protobuf_unittest.default_import_enum_extension]: IMPORT_FOO [protobuf_unittest.default_string_piece_extension]: "424" [protobuf_unittest.default_cord_extension]: "425" +[protobuf_unittest.optional_bytes_cord_extension]: "optional bytes cord" [protobuf_unittest.oneof_uint32_extension]: 601 [protobuf_unittest.oneof_nested_message_extension] { bb: 602 diff --git a/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt b/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt index 4233ca78f3014..20b5ea1162921 100644 --- a/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt +++ b/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt @@ -129,6 +129,7 @@ [protobuf_unittest.default_import_enum_extension]: IMPORT_FOO [protobuf_unittest.default_string_piece_extension]: "424" [protobuf_unittest.default_cord_extension]: "425" +[protobuf_unittest.optional_bytes_cord_extension]: "optional bytes cord" [protobuf_unittest.oneof_uint32_extension]: 601 [protobuf_unittest.oneof_nested_message_extension] < bb: 602 diff --git a/src/google/protobuf/unittest.proto b/src/google/protobuf/unittest.proto index 90d4cf12c2bc8..a4f2143a5ab98 100644 --- a/src/google/protobuf/unittest.proto +++ b/src/google/protobuf/unittest.proto @@ -84,6 +84,7 @@ message TestAllTypes { optional string optional_string_piece = 24 [ctype=STRING_PIECE]; optional string optional_cord = 25 [ctype=CORD]; + optional bytes optional_bytes_cord = 86 [ctype=CORD]; // Defined in unittest_import_public.proto optional protobuf_unittest_import.PublicImportMessage @@ -251,6 +252,7 @@ extend TestAllExtensions { // TODO: ctype=CORD is not supported for extension. Add // ctype=CORD option back after it is supported. optional string optional_cord_extension = 25; + optional bytes optional_bytes_cord_extension = 86; optional protobuf_unittest_import.PublicImportMessage optional_public_import_message_extension = 26; diff --git a/src/google/protobuf/unittest_lite.proto b/src/google/protobuf/unittest_lite.proto index b3fcfa431c00a..cdafcf7933ba9 100644 --- a/src/google/protobuf/unittest_lite.proto +++ b/src/google/protobuf/unittest_lite.proto @@ -72,6 +72,7 @@ message TestAllTypesLite { string optional_string_piece = 24 [ctype = STRING_PIECE]; string optional_cord = 25 [ctype = CORD]; + bytes optional_bytes_cord = 86 [ctype = CORD]; // Defined in unittest_import_public.proto protobuf_unittest_import.PublicImportMessageLite @@ -261,6 +262,7 @@ extend TestAllExtensionsLite { // TODO: ctype=CORD is not supported for extension. Add // ctype=CORD option back after it is supported. string optional_cord_extension_lite = 25; + bytes optional_bytes_cord_extension_lite = 86; protobuf_unittest_import.PublicImportMessageLite optional_public_import_message_extension_lite = 26; TestAllTypesLite.NestedMessage optional_lazy_message_extension_lite = 27 diff --git a/src/google/protobuf/unittest_proto3_arena.proto b/src/google/protobuf/unittest_proto3_arena.proto index 032ce6ff67773..bb2e775b423ef 100644 --- a/src/google/protobuf/unittest_proto3_arena.proto +++ b/src/google/protobuf/unittest_proto3_arena.proto @@ -67,6 +67,7 @@ message TestAllTypes { string optional_string_piece = 24 [ctype=STRING_PIECE]; string optional_cord = 25 [ctype=CORD]; + bytes optional_bytes_cord = 86 [ctype=CORD]; // Defined in unittest_import_public.proto protobuf_unittest_import.PublicImportMessage diff --git a/src/google/protobuf/util/field_mask_util_test.cc b/src/google/protobuf/util/field_mask_util_test.cc index 79fe32c9dfda0..624d6ac613ce8 100644 --- a/src/google/protobuf/util/field_mask_util_test.cc +++ b/src/google/protobuf/util/field_mask_util_test.cc @@ -202,7 +202,7 @@ TEST(FieldMaskUtilTest, TestGetFieldMaskForAllFields) { EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("bb", mask)); mask = FieldMaskUtil::GetFieldMaskForAllFields(); - EXPECT_EQ(79, mask.paths_size()); + EXPECT_EQ(80, mask.paths_size()); EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_int32", mask)); EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_int64", mask)); EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_uint32", mask)); diff --git a/src/google/protobuf/wire_format.cc b/src/google/protobuf/wire_format.cc index 72e13c4ba7f75..36f8357e70ecc 100644 --- a/src/google/protobuf/wire_format.cc +++ b/src/google/protobuf/wire_format.cc @@ -560,7 +560,7 @@ bool WireFormat::ParseAndMergeField( } case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { absl::Cord value; if (!WireFormatLite::ReadBytes(input, &value)) return false; message_reflection->SetString(message, field, value); @@ -965,7 +965,7 @@ const char* WireFormat::_InternalParseAndMergeField( case FieldDescriptor::TYPE_BYTES: { int size = ReadSize(&ptr); if (ptr == nullptr) return nullptr; - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { absl::Cord value; ptr = ctx->ReadCord(ptr, size, &value); if (ptr == nullptr) return nullptr; @@ -1424,7 +1424,7 @@ uint8_t* WireFormat::InternalSerializeField(const FieldDescriptor* field, } case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { absl::Cord value = message_reflection->GetCord(message, field); target = stream->WriteString(field->number(), value, target); break; @@ -1725,7 +1725,7 @@ size_t WireFormat::FieldDataOnlyByteSize(const FieldDescriptor* field, // instead of copying. case FieldDescriptor::TYPE_STRING: case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { for (size_t j = 0; j < count; j++) { absl::Cord value = message_reflection->GetCord(message, field); data_size += WireFormatLite::StringSize(value); diff --git a/third_party/zlib.BUILD b/third_party/zlib.BUILD index faab8f5e8ba0c..85a97c5d6ab70 100644 --- a/third_party/zlib.BUILD +++ b/third_party/zlib.BUILD @@ -59,6 +59,10 @@ cc_library( hdrs = _ZLIB_PREFIXED_HEADERS, copts = select({ "@platforms//os:windows": [], + "@platforms//os:macos": [ + "-Wno-unused-variable", + "-Wno-implicit-function-declaration", + ], "//conditions:default": [ "-Wno-deprecated-non-prototype", "-Wno-unused-variable", diff --git a/upb/io/zero_copy_stream_test.cc b/upb/io/zero_copy_stream_test.cc index 392d924848b5f..b7378d402b614 100644 --- a/upb/io/zero_copy_stream_test.cc +++ b/upb/io/zero_copy_stream_test.cc @@ -45,12 +45,6 @@ class IoTest : public testing::Test { // WriteStuff() writes. void ReadStuff(upb_ZeroCopyInputStream* input, bool read_eof = true); - // Similar to WriteStuff, but performs more sophisticated testing. - int WriteStuffLarge(upb_ZeroCopyOutputStream* output); - // Reads and tests a stream that should have been written to - // via WriteStuffLarge(). - void ReadStuffLarge(upb_ZeroCopyInputStream* input); - static const int kBlockSizes[]; static const int kBlockSizeCount; }; @@ -157,35 +151,6 @@ void IoTest::ReadStuff(upb_ZeroCopyInputStream* input, bool read_eof) { } } -int IoTest::WriteStuffLarge(upb_ZeroCopyOutputStream* output) { - WriteString(output, "Hello world!\n"); - WriteString(output, "Some te"); - WriteString(output, "xt. Blah blah."); - WriteString(output, std::string(100000, 'x')); // A very long string - WriteString(output, std::string(100000, 'y')); // A very long string - WriteString(output, "01234567890123456789"); - - const int result = upb_ZeroCopyOutputStream_ByteCount(output); - EXPECT_EQ(result, 200055); - return result; -} - -// Reads text from an input stream and expects it to match what WriteStuff() -// writes. -void IoTest::ReadStuffLarge(upb_ZeroCopyInputStream* input) { - ReadString(input, "Hello world!\nSome text. "); - EXPECT_TRUE(upb_ZeroCopyInputStream_Skip(input, 5)); - ReadString(input, "blah."); - EXPECT_TRUE(upb_ZeroCopyInputStream_Skip(input, 100000 - 10)); - ReadString(input, std::string(10, 'x') + std::string(100000 - 20000, 'y')); - EXPECT_TRUE(upb_ZeroCopyInputStream_Skip(input, 20000 - 10)); - ReadString(input, "yyyyyyyyyy01234567890123456789"); - EXPECT_EQ(upb_ZeroCopyInputStream_ByteCount(input), 200055); - - uint8_t byte; - EXPECT_EQ(ReadFromInput(input, &byte, 1), 0); -} - // =================================================================== TEST_F(IoTest, ArrayIo) { diff --git a/upb/wire/eps_copy_input_stream_test.cc b/upb/wire/eps_copy_input_stream_test.cc index c28d1457e4743..4d3d94570699a 100644 --- a/upb/wire/eps_copy_input_stream_test.cc +++ b/upb/wire/eps_copy_input_stream_test.cc @@ -281,7 +281,7 @@ TEST(EpsCopyInputStreamTest, ZeroSize) { // } // // // Test with: -// // $ blaze run --config=fuzztest third_party/upb:eps_copy_input_stream_test \ +// // $ blaze run --config=fuzztest third_party/upb:eps_copy_input_stream_test // // -- --gunit_fuzz= // FUZZ_TEST(EpsCopyFuzzTest, TestAgainstFakeStream) // .WithDomains(ArbitraryEpsCopyTestScript());