diff --git a/.bazelrc b/.bazelrc index eba2a89fb0a0..824d1af5bc0b 100644 --- a/.bazelrc +++ b/.bazelrc @@ -26,6 +26,8 @@ build:ubsan --copt=-DUNDEFINED_SANITIZER=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/ci/common.bazelrc b/ci/common.bazelrc index e5345cfd691d..c4888b5468f2 100644 --- a/ci/common.bazelrc +++ b/ci/common.bazelrc @@ -34,3 +34,5 @@ build:ubsan --copt=-DUNDEFINED_SANITIZER=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 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 3007c83a1f99..c68166daa9bb 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; @@ -327,47 +326,6 @@ public void testEncodeZigZag() throws Exception { .isEqualTo(-75123905439571256L); } - /** 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 0f57ecc9acca..598a086f71b6 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 d95ed4c8d9af..e2d637c54e5a 100644 --- a/java/core/src/test/java/com/google/protobuf/TextFormatTest.java +++ b/java/core/src/test/java/com/google/protobuf/TextFormatTest.java @@ -59,11 +59,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" @@ -139,12 +134,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 @@ -159,12 +149,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. */ @@ -172,12 +157,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. @@ -353,13 +333,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 @@ -399,14 +379,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()); } @@ -414,7 +395,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 1adbad286e84..cdac58d40509 100755 --- a/python/google/protobuf/internal/descriptor_test.py +++ b/python/google/protobuf/internal/descriptor_test.py @@ -719,7 +719,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 ed7c9ef60e26..ef286f8ba5d0 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 b0f1ae784fa6..3af128592875 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) @@ -1424,36 +1397,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 53492d8a1587..46059fe1eba2 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 57224ec74c74..33ad76653799 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/src/file_lists.cmake b/src/file_lists.cmake index d320be43a697..11c780a527e1 100644 --- a/src/file_lists.cmake +++ b/src/file_lists.cmake @@ -117,6 +117,7 @@ set(libprotobuf_hdrs ${protobuf_SOURCE_DIR}/src/google/protobuf/descriptor.pb.h ${protobuf_SOURCE_DIR}/src/google/protobuf/descriptor_database.h ${protobuf_SOURCE_DIR}/src/google/protobuf/descriptor_legacy.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/descriptor_lite.h ${protobuf_SOURCE_DIR}/src/google/protobuf/descriptor_visitor.h ${protobuf_SOURCE_DIR}/src/google/protobuf/dynamic_message.h ${protobuf_SOURCE_DIR}/src/google/protobuf/endian.h diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index 8961ca6af1a7..f57e6f660b83 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -238,6 +238,7 @@ cc_test( deps = [ ":port_def", ":varint_shuffle", + "@com_google_absl//absl/base:config", "@com_google_absl//absl/log:absl_check", "@com_google_googletest//:gtest", "@com_google_googletest//:gtest_main", @@ -436,6 +437,7 @@ cc_library( "//src/google/protobuf/io", "//src/google/protobuf/stubs:lite", "@com_google_absl//absl/base", + "@com_google_absl//absl/base:config", "@com_google_absl//absl/container:btree", "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/hash", @@ -482,6 +484,7 @@ cc_library( "descriptor.pb.h", "descriptor_database.h", "descriptor_legacy.h", + "descriptor_lite.h", "descriptor_visitor.h", "dynamic_message.h", "feature_resolver.h", @@ -1256,7 +1259,6 @@ cc_test( "message_unittest.cc", "message_unittest.inc", ], - data = [":testdata"], deps = [ ":cc_test_protos", ":protobuf", diff --git a/src/google/protobuf/arena_test_util.h b/src/google/protobuf/arena_test_util.h index e2144a866031..c74ba32c4959 100644 --- a/src/google/protobuf/arena_test_util.h +++ b/src/google/protobuf/arena_test_util.h @@ -18,41 +18,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); - } - const int kMaxIters = 900; - 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::CreateMessage(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 4a2d502cc1a6..7d1eee5610a2 100644 --- a/src/google/protobuf/arena_unittest.cc +++ b/src/google/protobuf/arena_unittest.cc @@ -1308,13 +1308,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 273dae00b40c..c26b104f99e6 100644 --- a/src/google/protobuf/compiler/BUILD.bazel +++ b/src/google/protobuf/compiler/BUILD.bazel @@ -323,6 +323,7 @@ cc_test( "//:protobuf", "//src/google/protobuf:cc_test_protos", "//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 23a1c7014a3c..6544a0030811 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -47,6 +47,7 @@ #include "google/protobuf/compiler/subprocess.h" #include "google/protobuf/io/io_win32.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" @@ -3998,7 +3999,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")); @@ -4008,14 +4018,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")); @@ -4068,6 +4078,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")); @@ -4077,14 +4089,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 e9ccf46bbdde..a3b9ec7ed108 100644 --- a/src/google/protobuf/compiler/cpp/field.cc +++ b/src/google/protobuf/compiler/cpp/field.cc @@ -115,7 +115,6 @@ FieldGeneratorBase::FieldGeneratorBase(const FieldDescriptor* descriptor, break; case FieldDescriptor::CPPTYPE_STRING: is_string_ = true; - string_type_ = descriptor->options().ctype(); is_inlined_ = IsStringInlined(descriptor, options); is_bytes_ = descriptor->type() == FieldDescriptor::TYPE_BYTES; has_default_constexpr_constructor_ = is_repeated_or_map; @@ -242,15 +241,18 @@ std::unique_ptr MakeGenerator(const FieldDescriptor* field, case FieldDescriptor::CPPTYPE_MESSAGE: return MakeSinguarMessageGenerator(field, options, scc); case FieldDescriptor::CPPTYPE_STRING: - if (field->type() == FieldDescriptor::TYPE_BYTES && - field->options().ctype() == FieldOptions::CORD) { - if (field->real_containing_oneof()) { - return MakeOneofCordGenerator(field, options, scc); - } else { - return MakeSingularCordGenerator(field, options, scc); - } - } else { - return MakeSinguarStringGenerator(field, options, scc); + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + if (field->type() == FieldDescriptor::TYPE_BYTES) { + if (field->real_containing_oneof()) { + return MakeOneofCordGenerator(field, options, scc); + } else { + return MakeSingularCordGenerator(field, options, scc); + } + } + ABSL_FALLTHROUGH_INTENDED; + default: + return MakeSinguarStringGenerator(field, options, scc); } case FieldDescriptor::CPPTYPE_ENUM: return MakeSinguarEnumGenerator(field, options, scc); diff --git a/src/google/protobuf/compiler/cpp/field.h b/src/google/protobuf/compiler/cpp/field.h index 1bba5f27b546..f4564851b579 100644 --- a/src/google/protobuf/compiler/cpp/field.h +++ b/src/google/protobuf/compiler/cpp/field.h @@ -84,9 +84,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_; } @@ -202,7 +199,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; }; @@ -249,7 +245,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/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index 2c8d27511404..e77d157ab41b 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -1351,7 +1351,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 2230b1c05fdb..6c9489e655c7 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 169fbb602164..aabfad361569 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 8fe803fba6f2..1a24a76fc562 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -3790,6 +3790,16 @@ bool FieldDescriptor::legacy_enum_field_treated_as_closed() const { enum_type()->is_closed()); } +FieldDescriptor::CppStringType FieldDescriptor::cpp_string_type() const { + ABSL_DCHECK(cpp_type() == FieldDescriptor::CPPTYPE_STRING); + switch (internal::cpp::EffectiveStringCType(this)) { + case FieldOptions::CORD: + return CppStringType::kCord; + default: + 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 ff178c31c967..9b17f6416eb8 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -49,6 +49,7 @@ #include "absl/strings/string_view.h" #include "absl/synchronization/mutex.h" #include "absl/types/optional.h" +#include "google/protobuf/descriptor_lite.h" #include "google/protobuf/extension_set.h" #include "google/protobuf/port.h" @@ -731,7 +732,8 @@ PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(Descriptor, 152); // - Given a DescriptorPool, call DescriptorPool::FindExtensionByNumber() or // DescriptorPool::FindExtensionByPrintableName(). // Use DescriptorPool to construct your own descriptors. -class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase { +class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase, + public internal::FieldDescriptorLite { public: typedef FieldDescriptorProto Proto; @@ -843,6 +845,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 diff --git a/src/google/protobuf/descriptor_lite.h b/src/google/protobuf/descriptor_lite.h new file mode 100644 index 000000000000..82349bd58f71 --- /dev/null +++ b/src/google/protobuf/descriptor_lite.h @@ -0,0 +1,36 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd +// +// This file contains definitions for the descriptors, so they can be used +// without importing descriptor.h + +#ifndef GOOGLE_PROTOBUF_DESCRIPTOR_LITE_H__ +#define GOOGLE_PROTOBUF_DESCRIPTOR_LITE_H__ + +namespace google { +namespace protobuf { +namespace internal { + +class FieldDescriptorLite { + public: + // 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 +} // namespace protobuf +} // namespace google + +#endif // GOOGLE_PROTOBUF_DESCRIPTOR_LITE_H__ diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 8986cf095d9c..5340282396b7 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -7298,6 +7298,20 @@ TEST_F(FeaturesTest, Proto2Features) { } field { name: "utf8" number: 6 label: LABEL_REPEATED type: TYPE_STRING } field { name: "req" number: 7 label: LABEL_REQUIRED type: TYPE_INT32 } + field { + name: "cord" + number: 8 + label: LABEL_OPTIONAL + type: TYPE_BYTES + options { ctype: CORD } + } + field { + name: "piece" + number: 9 + label: LABEL_OPTIONAL + type: TYPE_STRING + options { ctype: STRING_PIECE } + } } enum_type { name: "Foo2" @@ -7351,6 +7365,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); diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc index 64427553cccc..cb67cf65fa9d 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; @@ -345,6 +349,7 @@ void DynamicMessage::SharedCtor(bool lock_factory) { // constructor.) const Descriptor* descriptor = type_info_->type; + Arena* arena = GetArena(); // Initialize oneof cases. int oneof_count = 0; for (int i = 0; i < descriptor->oneof_decl_count(); ++i) { @@ -390,9 +395,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(); @@ -482,9 +509,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; } @@ -516,9 +546,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; @@ -536,9 +570,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/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 452fd5240d17..3fa975cc372b 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -385,9 +385,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(); @@ -426,8 +430,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(); @@ -439,8 +443,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(); @@ -536,15 +540,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: @@ -607,9 +610,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) { @@ -673,14 +686,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); @@ -1150,7 +1163,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()); @@ -1259,9 +1274,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; } @@ -1309,6 +1325,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 = @@ -1367,8 +1387,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(); @@ -1376,8 +1396,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. @@ -1424,9 +1444,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; } @@ -1474,9 +1497,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; @@ -1568,6 +1594,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) @@ -1774,15 +1806,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 { @@ -1790,6 +1822,7 @@ std::string Reflection::GetString(const Message& message, return str.IsDefault() ? field->default_value_string() : str.Get(); } } + internal::Unreachable(); } } @@ -1805,8 +1838,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); @@ -1814,8 +1847,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 { @@ -1823,6 +1856,7 @@ const std::string& Reflection::GetStringReference(const Message& message, return str.IsDefault() ? field->default_value_string() : str.Get(); } } + internal::Unreachable(); } } @@ -1836,15 +1870,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()); @@ -1854,9 +1888,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(); } } @@ -1868,8 +1900,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()); @@ -1881,8 +1913,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, 0); @@ -1920,8 +1952,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()); @@ -1933,8 +1965,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 @@ -1970,11 +2002,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(); } } @@ -1986,11 +2021,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(); } } @@ -2003,9 +2043,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; @@ -2021,9 +2064,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; } @@ -2688,9 +2734,9 @@ 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)) { - ptr = reinterpret_cast( - Arena::CreateMessage>(arena)); + field->cpp_string_type() == FieldDescriptor::CppStringType::kCord)) { + ptr = + reinterpret_cast(Arena::Create>(arena)); } else { ptr = reinterpret_cast( Arena::CreateMessage(arena)); @@ -2864,11 +2910,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() @@ -2878,7 +2924,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: @@ -2979,12 +3025,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 7c1736d63ebd..483893b5b70a 100644 --- a/src/google/protobuf/generated_message_tctable_gen.cc +++ b/src/google/protobuf/generated_message_tctable_gen.cc @@ -112,10 +112,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; @@ -248,10 +248,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 { @@ -265,6 +267,7 @@ bool IsFieldEligibleForFastParsing( aux_idx = entry.inlined_string_idx; } break; + } case FieldDescriptor::TYPE_ENUM: { uint8_t rmax_value; @@ -696,12 +699,12 @@ 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::kString: if (field->is_repeated()) { // A repeated string field uses RepeatedPtrField // (unless it has a ctype option; see above). @@ -711,8 +714,8 @@ uint16_t MakeTypeCardForField( type_card |= fl::kRepAString; } break; - default: - PROTOBUF_ASSUME(false); + case FieldDescriptor::CppStringType::kView: + break; } } diff --git a/src/google/protobuf/generated_message_tctable_lite.cc b/src/google/protobuf/generated_message_tctable_lite.cc index 531d0556a7c4..ef98f0b9a6c9 100644 --- a/src/google/protobuf/generated_message_tctable_lite.cc +++ b/src/google/protobuf/generated_message_tctable_lite.cc @@ -1664,6 +1664,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 0578742d62ce..7aaa568174fa 100644 --- a/src/google/protobuf/io/BUILD.bazel +++ b/src/google/protobuf/io/BUILD.bazel @@ -172,13 +172,12 @@ cc_test( "zero_copy_stream_unittest.cc", ], copts = COPTS, - data = [ - "//src/google/protobuf:testdata", - ], deps = [ ":gzip_stream", ":io", "//:protobuf", + "//src/google/protobuf", + "//src/google/protobuf:test_util", "//src/google/protobuf:test_util2", "//src/google/protobuf/testing", "@com_google_absl//absl/container:flat_hash_map", diff --git a/src/google/protobuf/io/zero_copy_stream_unittest.cc b/src/google/protobuf/io/zero_copy_stream_unittest.cc index a80bf6231eb7..d13846268d52 100644 --- a/src/google/protobuf/io/zero_copy_stream_unittest.cc +++ b/src/google/protobuf/io/zero_copy_stream_unittest.cc @@ -57,12 +57,14 @@ #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" #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" @@ -558,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/json/json_test.cc b/src/google/protobuf/json/json_test.cc index 2ff1e87a90fe..ef3aa62ecec5 100644 --- a/src/google/protobuf/json/json_test.cc +++ b/src/google/protobuf/json/json_test.cc @@ -250,7 +250,7 @@ TEST_P(JsonTest, TestDefaultValues) { R"("defaultSfixed64":"-50","defaultFloat":51.5,"defaultDouble":52000,"defaultBool":true,)" R"("defaultString":"hello","defaultBytes":"d29ybGQ=","defaultNestedEnum":"BAR",)" R"("defaultForeignEnum":"FOREIGN_BAR","defaultImportEnum":"IMPORT_BAR",)" - R"("defaultStringPiece":"abc","defaultCord":"123"})")); + R"("defaultStringPiece":"abc","defaultCord":"123","optionalBytesCord":""})")) << *ToJson(protobuf_unittest::TestAllTypes(), options); EXPECT_THAT( ToJson(protobuf_unittest::TestExtremeDefaultValues(), options), diff --git a/src/google/protobuf/lite_unittest.cc b/src/google/protobuf/lite_unittest.cc index 09f5ec60ccb1..1b7dcce1d821 100644 --- a/src/google/protobuf/lite_unittest.cc +++ b/src/google/protobuf/lite_unittest.cc @@ -956,6 +956,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 @@ -1042,6 +1054,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 c1b0029878f1..1cd789bbc783 100644 --- a/src/google/protobuf/map_test.inc +++ b/src/google/protobuf/map_test.inc @@ -3895,16 +3895,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 @@ -3942,15 +3961,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) { @@ -3958,35 +3971,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 b4b1a40c51d8..a98e0f923a8d 100644 --- a/src/google/protobuf/message.cc +++ b/src/google/protobuf/message.cc @@ -378,9 +378,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 4e07c693679a..25402c489754 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" @@ -110,8 +112,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); @@ -123,8 +129,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/port.h b/src/google/protobuf/port.h index 51e63e30e279..9aee882f6936 100644 --- a/src/google/protobuf/port.h +++ b/src/google/protobuf/port.h @@ -22,6 +22,7 @@ #include +#include "absl/base/config.h" #include "absl/meta/type_traits.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" @@ -201,6 +202,22 @@ inline constexpr bool DebugHardenStringValues() { #endif } +#if defined(NDEBUG) && ABSL_HAVE_BUILTIN(__builtin_unreachable) +[[noreturn]] ABSL_ATTRIBUTE_COLD PROTOBUF_ALWAYS_INLINE inline void +Unreachable() { + __builtin_unreachable(); +} +#elif ABSL_HAVE_BUILTIN(__builtin_FILE) && ABSL_HAVE_BUILTIN(__builtin_LINE) +[[noreturn]] ABSL_ATTRIBUTE_COLD inline void Unreachable( + const char* file = __builtin_FILE(), int line = __builtin_LINE()) { + protobuf_assumption_failed("Unreachable", file, line); +} +#else +[[noreturn]] ABSL_ATTRIBUTE_COLD inline void Unreachable() { + protobuf_assumption_failed("Unreachable", "", 0); +} +#endif + } // namespace internal } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index f6b10605d13a..a8d2c064acf4 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -1133,8 +1133,9 @@ static_assert(PROTOBUF_ABSL_MIN(20230125, 3), namespace google { namespace protobuf { namespace internal { -PROTOBUF_EXPORT void protobuf_assumption_failed(const char *pred, - const char *file, int line); +[[noreturn]] PROTOBUF_EXPORT void protobuf_assumption_failed(const char *pred, + const char *file, + int line); } // namespace internal } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/port_test.cc b/src/google/protobuf/port_test.cc index d1174c3a1b52..a8c6402819f4 100644 --- a/src/google/protobuf/port_test.cc +++ b/src/google/protobuf/port_test.cc @@ -5,10 +5,13 @@ // license that can be found in the LICENSE file or at // https://developers.google.com/open-source/licenses/bsd // +#include "google/protobuf/port.h" + #include #include #include +#include "absl/base/config.h" // Must be included last #include "google/protobuf/port_def.inc" @@ -28,6 +31,21 @@ TEST(PortTest, ProtobufAssume) { #endif } +TEST(PortTest, UnreachableTrapsOnDebugMode) { +#ifdef GTEST_HAS_DEATH_TEST +#if defined(NDEBUG) + // In NDEBUG we crash with a UD instruction, so we don't get the "Assumption + // failed" error. + GTEST_SKIP() << "Can't test __builtin_unreachable()"; +#elif ABSL_HAVE_BUILTIN(__builtin_FILE) + EXPECT_DEATH(Unreachable(), + "port_test\\.cc:.*Assumption failed: 'Unreachable'"); +#else + EXPECT_DEATH(Unreachable(), "Assumption failed: 'Unreachable'"); +#endif +#endif +} + } // namespace internal } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/test_util.h b/src/google/protobuf/test_util.h index c52c38fc4589..4f6e07ed3493 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 77ea70ba2e90..f58f951d324a 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 5825975ce07b..000000000000 Binary files a/src/google/protobuf/testdata/golden_message and /dev/null differ diff --git a/src/google/protobuf/testdata/golden_message_maps b/src/google/protobuf/testdata/golden_message_maps deleted file mode 100644 index c70a4d7cde5b..000000000000 Binary files a/src/google/protobuf/testdata/golden_message_maps and /dev/null differ diff --git a/src/google/protobuf/testdata/golden_message_oneof_implemented b/src/google/protobuf/testdata/golden_message_oneof_implemented deleted file mode 100644 index 794ca5e0d15e..000000000000 Binary files a/src/google/protobuf/testdata/golden_message_oneof_implemented and /dev/null differ diff --git a/src/google/protobuf/testdata/golden_message_proto3 b/src/google/protobuf/testdata/golden_message_proto3 deleted file mode 100644 index bd646a0dc85e..000000000000 Binary files a/src/google/protobuf/testdata/golden_message_proto3 and /dev/null differ diff --git a/src/google/protobuf/testdata/golden_packed_fields_message b/src/google/protobuf/testdata/golden_packed_fields_message deleted file mode 100644 index ee28d3883055..000000000000 Binary files a/src/google/protobuf/testdata/golden_packed_fields_message and /dev/null differ diff --git a/src/google/protobuf/testdata/text_format_unittest_data.txt b/src/google/protobuf/testdata/text_format_unittest_data.txt index 7a874b54cdba..b218f291b067 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 86389c93cbce..c477bbef1b91 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 788025c51530..d46ec7bb3a4f 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 b2d3367098ce..139e3df348e7 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 5c3a03ac3708..8ab4d2e85b0c 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 4233ca78f301..20b5ea116292 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 2970a6ba16c6..9bab28e08367 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 1848a2a1a242..0ea2cfcd35a1 100644 --- a/src/google/protobuf/unittest_lite.proto +++ b/src/google/protobuf/unittest_lite.proto @@ -69,6 +69,7 @@ message TestAllTypesLite { 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.PublicImportMessageLite @@ -223,7 +224,7 @@ extend TestAllExtensionsLite { // TODO: ctype=CORD is not supported for extension. Add // ctype=CORD option back after it is supported. optional string optional_cord_extension_lite = 25; - + optional bytes optional_bytes_cord_extension_lite = 86; optional protobuf_unittest_import.PublicImportMessageLite optional_public_import_message_extension_lite = 26; diff --git a/src/google/protobuf/unittest_proto3_arena.proto b/src/google/protobuf/unittest_proto3_arena.proto index 0d5e0b3ffe09..1cd4a64c62fc 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 16b3bd2fab2f..966ca9f0f878 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 452641719262..eb100a3fed07 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); @@ -964,7 +964,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; @@ -1423,7 +1423,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; @@ -1724,7 +1724,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);