From 5dcc8f2392b24cd21a2287cda0bb4c2dbc0b2187 Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Fri, 10 Sep 2021 21:20:01 +0000 Subject: [PATCH 1/5] Add release notes. --- CHANGES.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index 835281ff56..29b581c7e1 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,7 @@ Unreleased Changes (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript) + Protocol Compiler + * Make proto2::Message::DiscardUnknownFields() non-virtual + Python * Removed Python 2.x support. * Pure python descriptor_pool.AddSerializedFile() will always build the @@ -11,10 +14,15 @@ Unreleased Changes (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript) C++ * Generated code now uses the c++11 standard integer types int{32,64}_t and uint{32,64}_t + * Reduce memory usage of the DescriptorPool type. Java * Add @deprecated javadoc for set/get/has methods * correctly decode \? escape sequence in text protos + * Avoid depending on Objects.requireNonNull() until we can verify that no users are depending on older Android versions. + + Kotlin + * Generated Kotlin code is Explicit API mode compatible Unreleased Changes (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript) C++ From 565294d79fc8d9061ebd5f2fa801a0717505cfa5 Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Sat, 11 Sep 2021 00:14:28 +0000 Subject: [PATCH 2/5] Remove duplicate build_ext class and get_ext_filename method --- python/setup.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/python/setup.py b/python/setup.py index 31f5406078..33d74203c3 100755 --- a/python/setup.py +++ b/python/setup.py @@ -157,21 +157,6 @@ class build_py(_build_py): return [(pkg, mod, fil) for (pkg, mod, fil) in modules if not any(fnmatch.fnmatchcase(fil, pat=pat) for pat in exclude)] -class build_ext(_build_ext): - def get_ext_filename(self, ext_name): - # since python3.5, python extensions' shared libraries use a suffix that corresponds to the value - # of sysconfig.get_config_var('EXT_SUFFIX') and contains info about the architecture the library targets. - # E.g. on x64 linux the suffix is ".cpython-XYZ-x86_64-linux-gnu.so" - # When crosscompiling python wheels, we need to be able to override this suffix - # so that the resulting file name matches the target architecture and we end up with a well-formed - # wheel. - filename = _build_ext.get_ext_filename(self, ext_name) - orig_ext_suffix = sysconfig.get_config_var("EXT_SUFFIX") - new_ext_suffix = os.getenv("PROTOCOL_BUFFERS_OVERRIDE_EXT_SUFFIX") - if new_ext_suffix and filename.endswith(orig_ext_suffix): - filename = filename[:-len(orig_ext_suffix)] + new_ext_suffix - return filename - class build_ext(_build_ext): def get_ext_filename(self, ext_name): From 6dd21763548ac15bc26399917c4d916f09771815 Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Mon, 13 Sep 2021 17:56:05 +0000 Subject: [PATCH 3/5] Sync from Piper @396393195 PROTOBUF_SYNC_PIPER --- BUILD | 5 ++- conformance/conformance_nodejs.js | 35 +--------------- .../google/protobuf/MapForProto2LiteTest.java | 33 +++++++++++++++ .../com/google/protobuf/MapForProto2Test.java | 41 +++++++++++++++---- .../java/com/google/protobuf/MapTest.java | 34 +++++++++++++++ .../protobuf/compiler/java/java_map_field.cc | 9 ++-- update_file_lists.sh | 22 ++++++---- 7 files changed, 125 insertions(+), 54 deletions(-) mode change 100644 => 100755 conformance/conformance_nodejs.js diff --git a/BUILD b/BUILD index e592465ea0..9fbf69dfca 100644 --- a/BUILD +++ b/BUILD @@ -133,6 +133,7 @@ cc_library( "src/google/protobuf/extension_set.cc", "src/google/protobuf/generated_enum_util.cc", "src/google/protobuf/generated_message_table_driven_lite.cc", + "src/google/protobuf/generated_message_tctable_lite.cc", "src/google/protobuf/generated_message_util.cc", "src/google/protobuf/implicit_weak_message.cc", "src/google/protobuf/inlined_string_field.cc", @@ -193,6 +194,7 @@ cc_library( "src/google/protobuf/generated_message_bases.cc", "src/google/protobuf/generated_message_reflection.cc", "src/google/protobuf/generated_message_table_driven.cc", + "src/google/protobuf/generated_message_tctable_full.cc", "src/google/protobuf/io/gzip_stream.cc", "src/google/protobuf/io/printer.cc", "src/google/protobuf/io/tokenizer.cc", @@ -222,7 +224,6 @@ cc_library( "src/google/protobuf/util/internal/protostream_objectsource.cc", "src/google/protobuf/util/internal/protostream_objectwriter.cc", "src/google/protobuf/util/internal/type_info.cc", - "src/google/protobuf/util/internal/type_info_test_helper.cc", "src/google/protobuf/util/internal/utility.cc", "src/google/protobuf/util/json_util.cc", "src/google/protobuf/util/message_differencer.cc", @@ -621,6 +622,8 @@ cc_proto_library( COMMON_TEST_SRCS = [ # AUTOGEN(common_test_srcs) "src/google/protobuf/arena_test_util.cc", + "src/google/protobuf/map_lite_test_util.cc", + "src/google/protobuf/test_util_lite.cc", "src/google/protobuf/map_test_util.inc", "src/google/protobuf/reflection_tester.cc", "src/google/protobuf/test_util.cc", diff --git a/conformance/conformance_nodejs.js b/conformance/conformance_nodejs.js old mode 100644 new mode 100755 index f6bbcf4fcc..95da893f71 --- a/conformance/conformance_nodejs.js +++ b/conformance/conformance_nodejs.js @@ -1,3 +1,4 @@ +#!/usr/bin/env node // Protocol Buffers - Google's data interchange format // Copyright 2008 Google Inc. All rights reserved. // https://developers.google.com/protocol-buffers/ @@ -28,40 +29,6 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -#!/usr/bin/env node - -/* - * Protocol Buffers - Google's data interchange format - * Copyright 2008 Google Inc. All rights reserved. - * https://developers.google.com/protocol-buffers/ - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - var conformance = require('conformance_pb'); var test_messages_proto3 = require('google/protobuf/test_messages_proto3_pb'); var test_messages_proto2 = require('google/protobuf/test_messages_proto2_pb'); diff --git a/java/core/src/test/java/com/google/protobuf/MapForProto2LiteTest.java b/java/core/src/test/java/com/google/protobuf/MapForProto2LiteTest.java index b143dce9b3..ed0773988c 100644 --- a/java/core/src/test/java/com/google/protobuf/MapForProto2LiteTest.java +++ b/java/core/src/test/java/com/google/protobuf/MapForProto2LiteTest.java @@ -32,6 +32,7 @@ package com.google.protobuf; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.fail; import map_lite_test.MapForProto2TestProto.BizarroTestMap; import map_lite_test.MapForProto2TestProto.TestMap; @@ -43,6 +44,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.Map; +import java.util.TreeMap; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -803,4 +805,35 @@ public final class MapForProto2LiteTest { // expected } } + + @Test + public void testPutAllWithNullStringValue() throws Exception { + TestMap.Builder sourceBuilder = TestMap.newBuilder(); + + // order preserving map used here to help test rollback + Map data = new TreeMap<>(); + data.put(7, "foo"); + data.put(8, "bar"); + data.put(9, null); + try { + sourceBuilder.putAllInt32ToStringField(data); + fail("allowed null string value"); + } catch (NullPointerException expected) { + // Verify rollback of previously added values. + // They all go in or none do. + assertThat(sourceBuilder.getInt32ToStringFieldMap()).isEmpty(); + } + } + + @Test + public void testPutNullStringValue() throws Exception { + TestMap.Builder sourceBuilder = TestMap.newBuilder(); + + try { + sourceBuilder.putInt32ToStringField(8, null); + fail("allowed null string value"); + } catch (NullPointerException expected) { + } + } + } diff --git a/java/core/src/test/java/com/google/protobuf/MapForProto2Test.java b/java/core/src/test/java/com/google/protobuf/MapForProto2Test.java index 821b93cf28..1d6a0ddea1 100644 --- a/java/core/src/test/java/com/google/protobuf/MapForProto2Test.java +++ b/java/core/src/test/java/com/google/protobuf/MapForProto2Test.java @@ -32,6 +32,8 @@ package com.google.protobuf; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; import com.google.protobuf.Descriptors.FieldDescriptor; import map_test.MapForProto2TestProto.BizarroTestMap; @@ -51,6 +53,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.TreeMap; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -484,13 +487,6 @@ public class MapForProto2Test { public void testPutChecksNullKeysAndValues() throws Exception { TestMap.Builder builder = TestMap.newBuilder(); - try { - builder.putInt32ToStringField(1, null); - assertWithMessage("expected exception").fail(); - } catch (NullPointerException e) { - // expected. - } - try { builder.putInt32ToBytesField(1, null); assertWithMessage("expected exception").fail(); @@ -1218,4 +1214,35 @@ public class MapForProto2Test { assertThat(message.getInt32ToEnumFieldMap()).isEqualTo(message.getInt32ToEnumFieldMap()); assertThat(message.getInt32ToMessageFieldMap()).isEqualTo(message.getInt32ToMessageFieldMap()); } + + @Test + public void testPutAllWithNullStringValue() throws Exception { + TestMap.Builder sourceBuilder = TestMap.newBuilder(); + + // order preserving map used here to help test rollback + Map data = new TreeMap<>(); + data.put(7, "foo"); + data.put(8, "bar"); + data.put(9, null); + try { + sourceBuilder.putAllInt32ToStringField(data); + fail("allowed null string value"); + } catch (NullPointerException expected) { + // Verify rollback of previously added values. + // They all go in or none do. + assertThat(sourceBuilder.getInt32ToStringFieldMap()).isEmpty(); + } + } + + @Test + public void testPutNullStringValue() throws Exception { + TestMap.Builder sourceBuilder = TestMap.newBuilder(); + + try { + sourceBuilder.putInt32ToStringField(8, null); + fail("allowed null string value"); + } catch (NullPointerException expected) { + assertNotNull(expected.getMessage()); + } + } } diff --git a/java/core/src/test/java/com/google/protobuf/MapTest.java b/java/core/src/test/java/com/google/protobuf/MapTest.java index cc7a1217db..34df94523d 100644 --- a/java/core/src/test/java/com/google/protobuf/MapTest.java +++ b/java/core/src/test/java/com/google/protobuf/MapTest.java @@ -32,6 +32,8 @@ package com.google.protobuf; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.EnumDescriptor; @@ -50,6 +52,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.TreeMap; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -464,6 +467,37 @@ public class MapTest { assertMapValuesSet(destination.build()); } + @Test + public void testPutAllWithNullStringValue() throws Exception { + TestMap.Builder sourceBuilder = TestMap.newBuilder(); + + // order preserving map used here to help test rollback + Map data = new TreeMap<>(); + data.put(7, "foo"); + data.put(8, "bar"); + data.put(9, null); + try { + sourceBuilder.putAllInt32ToStringField(data); + fail("allowed null string value"); + } catch (NullPointerException expected) { + // Verify rollback of previously added values. + // They all go in or none do. + assertThat(sourceBuilder.getInt32ToStringFieldMap()).isEmpty(); + } + } + + @Test + public void testPutNullStringValue() throws Exception { + TestMap.Builder sourceBuilder = TestMap.newBuilder(); + + try { + sourceBuilder.putInt32ToStringField(8, null); + fail("allowed null string value"); + } catch (NullPointerException expected) { + assertNotNull(expected.getMessage()); + } + } + @Test public void testPutAllForUnknownEnumValues() throws Exception { TestMap source = diff --git a/src/google/protobuf/compiler/java/java_map_field.cc b/src/google/protobuf/compiler/java/java_map_field.cc index 7ced11c00f..8a89100714 100644 --- a/src/google/protobuf/compiler/java/java_map_field.cc +++ b/src/google/protobuf/compiler/java/java_map_field.cc @@ -111,11 +111,13 @@ void SetMessageVariables(const FieldDescriptor* descriptor, int messageBitIndex, (*variables)["key_default_value"] = DefaultValue(key, true, name_resolver); (*variables)["key_null_check"] = IsReferenceType(keyJavaType) - ? "if (key == null) { throw new java.lang.NullPointerException(); }" + ? "if (key == null) { throw new NullPointerException(\"map key\"); }" : ""; (*variables)["value_null_check"] = - IsReferenceType(valueJavaType) - ? "if (value == null) { throw new java.lang.NullPointerException(); }" + valueJavaType != JAVATYPE_ENUM && IsReferenceType(valueJavaType) + ? "if (value == null) {\n" + " throw new NullPointerException(\"map value\");\n" + "}\n" : ""; if (valueJavaType == JAVATYPE_ENUM) { // We store enums as Integers internally. @@ -435,6 +437,7 @@ void ImmutableMapFieldGenerator::GenerateBuilderMembers( " $key_type$ key,\n" " $value_type$ value) {\n" " $key_null_check$\n" + " $value_null_check$\n" " internalGetMutable$capitalized_name$().getMutableMap()\n" " .put(key, value);\n" " return this;\n" diff --git a/update_file_lists.sh b/update_file_lists.sh index e0b446f867..1ec119523b 100755 --- a/update_file_lists.sh +++ b/update_file_lists.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/bash -u # This script copies source file lists from src/Makefile.am to cmake files. @@ -50,12 +50,12 @@ MAKEFILE=src/Makefile.am # Extract file lists from src/Makefile.am GZHEADERS=$(get_variable_value $MAKEFILE GZHEADERS) -HEADERS=$(get_variable_value $MAKEFILE nobase_include_HEADERS) -PUBLIC_HEADERS=$(sort_files $GZHEADERS $HEADERS) +LIBPROTOBUF_HEADERS=$(get_variable_value $MAKEFILE nobase_include_HEADERS | grep -v /compiler/) +LIBPROTOBUF_HEADERS=$(sort_files $GZHEADERS $LIBPROTOBUF_HEADERS) LIBPROTOBUF_LITE_SOURCES=$(get_source_files $MAKEFILE libprotobuf_lite_la_SOURCES) LIBPROTOBUF_SOURCES=$(get_source_files $MAKEFILE libprotobuf_la_SOURCES) LIBPROTOC_SOURCES=$(get_source_files $MAKEFILE libprotoc_la_SOURCES) -LIBPROTOC_HEADERS=$(get_header_files $MAKEFILE libprotoc_la_SOURCES) +LIBPROTOC_HEADERS=$(get_variable_value $MAKEFILE nobase_include_HEADERS | grep /compiler/) LITE_PROTOS=$(get_proto_files $MAKEFILE protoc_lite_outputs) PROTOS=$(get_proto_files $MAKEFILE protoc_outputs) PROTOS_BLACKLISTED=$(get_proto_files_blacklisted $MAKEFILE protoc_outputs) @@ -98,7 +98,11 @@ set_cmake_value() { print \$0; len = split(values, vlist, \" \"); for (i = 1; i <= len; ++i) { - printf(\" %s%s\\n\", prefix, vlist[i]); + printf(\" \"); + if (vlist[i] !~ /^\\\$/) { + printf(\"%s\", prefix); + } + printf(\"%s\\n\", vlist[i]); } next; } @@ -121,7 +125,7 @@ set_cmake_value $CMAKE_DIR/libprotoc.cmake libprotoc_files $CMAKE_PREFIX $LIBPRO set_cmake_value $CMAKE_DIR/libprotoc.cmake libprotoc_headers $CMAKE_PREFIX $LIBPROTOC_HEADERS set_cmake_value $CMAKE_DIR/tests.cmake lite_test_protos "" $LITE_PROTOS set_cmake_value $CMAKE_DIR/tests.cmake tests_protos "" $PROTOS_BLACKLISTED -set_cmake_value $CMAKE_DIR/tests.cmake common_test_files $CMAKE_PREFIX $COMMON_TEST_SOURCES +set_cmake_value $CMAKE_DIR/tests.cmake common_test_files $CMAKE_PREFIX '${common_lite_test_files}' $COMMON_TEST_SOURCES set_cmake_value $CMAKE_DIR/tests.cmake common_lite_test_files $CMAKE_PREFIX $COMMON_LITE_TEST_SOURCES set_cmake_value $CMAKE_DIR/tests.cmake tests_files $CMAKE_PREFIX $TEST_SOURCES set_cmake_value $CMAKE_DIR/tests.cmake non_msvc_tests_files $CMAKE_PREFIX $NON_MSVC_TEST_SOURCES @@ -130,14 +134,14 @@ set_cmake_value $CMAKE_DIR/tests.cmake lite_arena_test_files $CMAKE_PREFIX $LITE # Generate extract_includes.bat echo "mkdir include" > $EXTRACT_INCLUDES_BAT -for INCLUDE in $PUBLIC_HEADERS $WKT_PROTOS; do +for INCLUDE in $LIBPROTOBUF_HEADERS $LIBPROTOC_HEADERS $WKT_PROTOS; do INCLUDE_DIR=$(dirname "$INCLUDE") while [ ! "$INCLUDE_DIR" = "." ]; do echo "mkdir include\\${INCLUDE_DIR//\//\\}" INCLUDE_DIR=$(dirname "$INCLUDE_DIR") done done | sort | uniq >> $EXTRACT_INCLUDES_BAT -for INCLUDE in $PUBLIC_HEADERS $WKT_PROTOS; do +for INCLUDE in $(sort_files $LIBPROTOBUF_HEADERS $LIBPROTOC_HEADERS) $WKT_PROTOS; do WINPATH=${INCLUDE//\//\\} echo "copy \"\${PROTOBUF_SOURCE_WIN32_PATH}\\..\\src\\$WINPATH\" include\\$WINPATH" >> $EXTRACT_INCLUDES_BAT done @@ -186,7 +190,7 @@ if [ -f "$BAZEL_BUILD" ]; then set_bazel_value $BAZEL_BUILD lite_test_protos "" $LITE_PROTOS set_bazel_value $BAZEL_BUILD well_known_protos "" $WKT_PROTOS set_bazel_value $BAZEL_BUILD test_protos "" $PROTOS - set_bazel_value $BAZEL_BUILD common_test_srcs $BAZEL_PREFIX $COMMON_TEST_SOURCES + set_bazel_value $BAZEL_BUILD common_test_srcs $BAZEL_PREFIX $COMMON_LITE_TEST_SOURCES $COMMON_TEST_SOURCES set_bazel_value $BAZEL_BUILD test_srcs $BAZEL_PREFIX $TEST_SOURCES set_bazel_value $BAZEL_BUILD non_msvc_test_srcs $BAZEL_PREFIX $NON_MSVC_TEST_SOURCES set_bazel_value $BAZEL_BUILD test_plugin_srcs $BAZEL_PREFIX $TEST_PLUGIN_SOURCES From f286ae16f1a8a93b9b6e506d4a96730703def49e Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Mon, 13 Sep 2021 18:12:36 +0000 Subject: [PATCH 4/5] Update release notes. --- CHANGES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.txt b/CHANGES.txt index 29b581c7e1..fd20488e44 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -20,6 +20,7 @@ Unreleased Changes (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript) * Add @deprecated javadoc for set/get/has methods * correctly decode \? escape sequence in text protos * Avoid depending on Objects.requireNonNull() until we can verify that no users are depending on older Android versions. + * disallow null string map values in put and putAll Kotlin * Generated Kotlin code is Explicit API mode compatible From e56c930d39dcdd9736ff505a3ac80858da436ed7 Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Mon, 13 Sep 2021 22:18:07 +0000 Subject: [PATCH 5/5] Fix map_test.proto package name --- java/core/src/test/proto/com/google/protobuf/map_test.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/core/src/test/proto/com/google/protobuf/map_test.proto b/java/core/src/test/proto/com/google/protobuf/map_test.proto index f47ddb5908..240600f2de 100644 --- a/java/core/src/test/proto/com/google/protobuf/map_test.proto +++ b/java/core/src/test/proto/com/google/protobuf/map_test.proto @@ -30,7 +30,7 @@ syntax = "proto3"; -package map_lite_test; +package map_test; option java_package = "map_test"; option java_outer_classname = "MapTestProto";