This is an automated email from the ASF dual-hosted git repository.
achennaka pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 1500df981 [thirdparty] Upgrade protobuf to 3.21.9
1500df981 is described below
commit 1500df981e4a95a589d9b3ae611f60f3b529b29b
Author: Abhishek Chennaka <[email protected]>
AuthorDate: Wed Jan 22 17:06:30 2025 -0800
[thirdparty] Upgrade protobuf to 3.21.9
protobuf upgrade to 3.21.9 to address the CVE [1]
The function error_message() is changed to message() in the commit [2]
in the file [3]. Hence renamed the call sites in Kudu repo accordingly.
Replacing function calls ExtractSubrange() with UnsafeArenaExtractSubrange()
as per [4]. Refer [5] for more information of the commit in the file,
repeated_field.h.
We also add the needed mappings for IWYU which stem due to [6].
[1]
https://github.com/protocolbuffers/protobuf/security/advisories/GHSA-8gq9-2x98-w8hf
[2]
https://github.com/protocolbuffers/protobuf/commit/0894e07536af88065d462cdc9d8e807c0723ef4d#diff-26f14c21bd27b6500347fdacdeea49b8bccde636aab2ecae545515e76a5a48bd
[3] src/google/protobuf/stubs/status.h
[4]
https://github.com/protocolbuffers/protobuf/blob/2798a968c330de223b711e4fe504800280f333fb/src/google/protobuf/repeated_ptr_field.h#L1069-L1073
[5]
https://github.com/protocolbuffers/protobuf/commit/6ef984af4b0c63c1c33127a12dcfc8e6359f0c9e#diff-5cc9e8e347380b7bbc8fac8c06dc98100f3b5c076fcffa177e8ce11e753d4f7c
[6]
https://github.com/protocolbuffers/protobuf/blob/2798a968c330de223b711e4fe504800280f333fb/src/google/protobuf/repeated_ptr_field.h#L44
Note: There will be followup patches to introduce abseil and upgrade
protobuf to atleast v25.5
References:
https://gerrit.cloudera.org/#/c/22205/
https://gerrit.cloudera.org/#/c/22206/
Change-Id: I5274c5f4c681a864d126c51960fa3b41d4f568d7
Reviewed-on: http://gerrit.cloudera.org:8080/22373
Reviewed-by: Alexey Serbin <[email protected]>
Tested-by: Abhishek Chennaka <[email protected]>
---
build-support/iwyu/mappings/protobuf.imp | 20 ++++++++++++++++++++
src/kudu/consensus/consensus_peers.cc | 2 +-
src/kudu/consensus/consensus_queue-test.cc | 16 ++++++++--------
src/kudu/consensus/consensus_queue.cc | 2 +-
src/kudu/consensus/consensus_queue.h | 2 +-
src/kudu/consensus/log.cc | 2 +-
src/kudu/consensus/log_util.cc | 2 +-
src/kudu/subprocess/subprocess_protocol.cc | 4 ++--
src/kudu/tools/tool_action_pbc.cc | 2 +-
src/kudu/tools/tool_action_table.cc | 6 +++---
src/kudu/tools/tool_action_test.cc | 4 ++--
thirdparty/vars.sh | 2 +-
12 files changed, 42 insertions(+), 22 deletions(-)
diff --git a/build-support/iwyu/mappings/protobuf.imp
b/build-support/iwyu/mappings/protobuf.imp
new file mode 100644
index 000000000..089990b9a
--- /dev/null
+++ b/build-support/iwyu/mappings/protobuf.imp
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[
+ { include: ["\"net/proto2/public/repeated_field.h\"", private,
"<google/protobuf/repeated_field.h>", public] }
+]
diff --git a/src/kudu/consensus/consensus_peers.cc
b/src/kudu/consensus/consensus_peers.cc
index 9fa2bb39e..55d521e16 100644
--- a/src/kudu/consensus/consensus_peers.cc
+++ b/src/kudu/consensus/consensus_peers.cc
@@ -538,7 +538,7 @@ Peer::~Peer() {
}
// We don't own the ops (the queue does).
- request_.mutable_ops()->ExtractSubrange(0, request_.ops_size(), nullptr);
+ request_.mutable_ops()->UnsafeArenaExtractSubrange(0, request_.ops_size(),
nullptr);
}
RpcPeerProxy::RpcPeerProxy(HostPort hostport,
diff --git a/src/kudu/consensus/consensus_queue-test.cc
b/src/kudu/consensus/consensus_queue-test.cc
index d21a37783..0a64c1df1 100644
--- a/src/kudu/consensus/consensus_queue-test.cc
+++ b/src/kudu/consensus/consensus_queue-test.cc
@@ -450,7 +450,7 @@ TEST_F(ConsensusQueueTest, TestStartTrackingAfterStart) {
ASSERT_EQ(0, request.ops_size());
// extract the ops from the request to avoid double free
- request.mutable_ops()->ExtractSubrange(0, request.ops_size(), nullptr);
+ request.mutable_ops()->UnsafeArenaExtractSubrange(0, request.ops_size(),
nullptr);
}
// Tests that the peers gets the messages pages, with the size of a page
@@ -499,7 +499,7 @@ TEST_F(ConsensusQueueTest, TestGetPagedMessages) {
SCOPED_CLEANUP({
// Extract the ops from the request to avoid double free.
- request.mutable_ops()->ExtractSubrange(0, request.ops_size(), nullptr);
+ request.mutable_ops()->UnsafeArenaExtractSubrange(0, request.ops_size(),
nullptr);
});
OpId last;
@@ -591,7 +591,7 @@ TEST_F(ConsensusQueueTest,
TestPeersDontAckBeyondWatermarks) {
ASSERT_EQ(queue_->GetAllReplicatedIndex(), expected.index());
// extract the ops from the request to avoid double free
- request.mutable_ops()->ExtractSubrange(0, request.ops_size(), nullptr);
+ request.mutable_ops()->UnsafeArenaExtractSubrange(0, request.ops_size(),
nullptr);
}
TEST_F(ConsensusQueueTest, TestQueueAdvancesCommittedIndex) {
@@ -790,7 +790,7 @@ TEST_F(ConsensusQueueTest, TestQueueLoadsOperationsForPeer)
{
ASSERT_EQ(request.ops_size(), 50);
// The messages still belong to the queue so we have to release them.
- request.mutable_ops()->ExtractSubrange(0, request.ops().size(), nullptr);
+ request.mutable_ops()->UnsafeArenaExtractSubrange(0, request.ops().size(),
nullptr);
}
// This tests that the queue is able to handle operation overwriting, i.e.
when a
@@ -899,7 +899,7 @@ TEST_F(ConsensusQueueTest,
TestQueueHandlesOperationOverwriting) {
ASSERT_EQ(queue_->GetAllReplicatedIndex(), 21);
// The messages still belong to the queue so we have to release them.
- request.mutable_ops()->ExtractSubrange(0, request.ops().size(), nullptr);
+ request.mutable_ops()->UnsafeArenaExtractSubrange(0, request.ops().size(),
nullptr);
}
// Test for a bug where we wouldn't move any watermark back, when overwriting
@@ -1030,7 +1030,7 @@ TEST_F(ConsensusQueueTest,
TestOnlyAdvancesWatermarkWhenPeerHasAPrefixOfOurLog)
// Another request for this peer should get another page of messages. Still
not
// on the queue's term (and thus without advancing watermarks).
- request.mutable_ops()->ExtractSubrange(0, request.ops().size(), nullptr);
+ request.mutable_ops()->UnsafeArenaExtractSubrange(0, request.ops().size(),
nullptr);
ASSERT_OK(queue_->RequestForPeer(kPeerUuid, &request, &refs,
&needs_tablet_copy));
ASSERT_FALSE(needs_tablet_copy);
ASSERT_EQ(request.ops_size(), 9);
@@ -1048,7 +1048,7 @@ TEST_F(ConsensusQueueTest,
TestOnlyAdvancesWatermarkWhenPeerHasAPrefixOfOurLog)
// The last page of request should overwrite the peer's operations and the
// response should finally advance the watermarks.
- request.mutable_ops()->ExtractSubrange(0, request.ops().size(), nullptr);
+ request.mutable_ops()->UnsafeArenaExtractSubrange(0, request.ops().size(),
nullptr);
ASSERT_OK(queue_->RequestForPeer(kPeerUuid, &request, &refs,
&needs_tablet_copy));
ASSERT_FALSE(needs_tablet_copy);
ASSERT_EQ(request.ops_size(), 4);
@@ -1063,7 +1063,7 @@ TEST_F(ConsensusQueueTest,
TestOnlyAdvancesWatermarkWhenPeerHasAPrefixOfOurLog)
ASSERT_EQ(queue_->GetMajorityReplicatedIndexForTests(),
expected_majority_replicated);
ASSERT_EQ(queue_->GetAllReplicatedIndex(), expected_all_replicated);
- request.mutable_ops()->ExtractSubrange(0, request.ops().size(), nullptr);
+ request.mutable_ops()->UnsafeArenaExtractSubrange(0, request.ops().size(),
nullptr);
}
// Test that Tablet Copy is triggered when a "tablet not found" error occurs.
diff --git a/src/kudu/consensus/consensus_queue.cc
b/src/kudu/consensus/consensus_queue.cc
index 90d1c7930..d97de9656 100644
--- a/src/kudu/consensus/consensus_queue.cc
+++ b/src/kudu/consensus/consensus_queue.cc
@@ -665,7 +665,7 @@ Status PeerMessageQueue::RequestForPeer(const string& uuid,
peer_copy = *peer;
// Clear the requests without deleting the entries, as they may be in use
by other peers.
- request->mutable_ops()->ExtractSubrange(0, request->ops_size(), nullptr);
+ request->mutable_ops()->UnsafeArenaExtractSubrange(0, request->ops_size(),
nullptr);
// This is initialized to the queue's last appended op but gets set to the
id of the
// log entry preceding the first one in 'messages' if messages are found
for the peer.
diff --git a/src/kudu/consensus/consensus_queue.h
b/src/kudu/consensus/consensus_queue.h
index c20429dcc..1fa4a456f 100644
--- a/src/kudu/consensus/consensus_queue.h
+++ b/src/kudu/consensus/consensus_queue.h
@@ -268,7 +268,7 @@ class PeerMessageQueue {
// WARNING: In order to avoid copying the same messages to every peer,
// entries are added to 'request' via AddAllocated() methods.
// The owner of 'request' is expected not to delete the request prior
- // to removing the entries through ExtractSubRange() or any other method
+ // to removing the entries through UnsafeArenaExtractSubRange() or any other
method
// that does not delete the entries. The simplest way is to pass the same
// instance of ConsensusRequestPB to RequestForPeer(): the buffer will
// replace the old entries with new ones without de-allocating the old
diff --git a/src/kudu/consensus/log.cc b/src/kudu/consensus/log.cc
index f0073b44e..81d6f279a 100644
--- a/src/kudu/consensus/log.cc
+++ b/src/kudu/consensus/log.cc
@@ -1019,7 +1019,7 @@ Status Log::Append(LogEntryPB* entry) {
LogEntryBatchPB entry_batch_pb;
entry_batch_pb.mutable_entry()->UnsafeArenaAddAllocated(entry);
LogEntryBatch entry_batch(entry->type(), entry_batch_pb, &DoNothingStatusCB);
- entry_batch_pb.mutable_entry()->ExtractSubrange(0, 1, nullptr);
+ entry_batch_pb.mutable_entry()->UnsafeArenaExtractSubrange(0, 1, nullptr);
Status s = WriteBatch(&entry_batch);
if (s.ok()) {
s = Sync();
diff --git a/src/kudu/consensus/log_util.cc b/src/kudu/consensus/log_util.cc
index 27f0f0d1d..59eaaddbb 100644
--- a/src/kudu/consensus/log_util.cc
+++ b/src/kudu/consensus/log_util.cc
@@ -182,7 +182,7 @@ Status
LogEntryReader::ReadNextEntry(unique_ptr<LogEntryPB>* entry) {
}
recent_entries_.push_back({ offset_, entry->type(), op_id });
}
- current_batch.mutable_entry()->ExtractSubrange(
+ current_batch.mutable_entry()->UnsafeArenaExtractSubrange(
0, current_batch.entry_size(), nullptr);
}
diff --git a/src/kudu/subprocess/subprocess_protocol.cc
b/src/kudu/subprocess/subprocess_protocol.cc
index 2d3ca3de5..cbb863ee7 100644
--- a/src/kudu/subprocess/subprocess_protocol.cc
+++ b/src/kudu/subprocess/subprocess_protocol.cc
@@ -92,7 +92,7 @@ Status SubprocessProtocol::ReceiveMessage(M* message) {
if (!google_status.ok()) {
return Status::InvalidArgument(
Substitute("unable to parse JSON: $0", buf.ToString()),
- google_status.error_message().ToString());
+ google_status.message().ToString());
}
break;
}
@@ -154,7 +154,7 @@ Status SubprocessProtocol::SendMessage(const M& message) {
if (!google_status.ok()) {
return Status::InvalidArgument(Substitute(
"unable to serialize JSON: $0",
pb_util::SecureDebugString(message)),
-
google_status.error_message().ToString());
+ google_status.message().ToString());
}
buf.append(serialized);
diff --git a/src/kudu/tools/tool_action_pbc.cc
b/src/kudu/tools/tool_action_pbc.cc
index 88dc49672..41473ef4c 100644
--- a/src/kudu/tools/tool_action_pbc.cc
+++ b/src/kudu/tools/tool_action_pbc.cc
@@ -266,7 +266,7 @@ Status EditFile(const RunnerContext& context) {
if (!google_status.ok()) {
return Status::InvalidArgument(
Substitute("Unable to parse JSON text: $0", str),
- google_status.error_message().ToString());
+ google_status.message().ToString());
}
// Append the protobuf object to writer.
diff --git a/src/kudu/tools/tool_action_table.cc
b/src/kudu/tools/tool_action_table.cc
index 4261a22ae..6948fd59b 100644
--- a/src/kudu/tools/tool_action_table.cc
+++ b/src/kudu/tools/tool_action_table.cc
@@ -31,7 +31,7 @@
#include <gflags/gflags.h>
#include <glog/logging.h>
-#include <google/protobuf/stubs/common.h>
+#include <google/protobuf/stubs/common.h> // IWYU pragma: keep
#include <google/protobuf/stubs/status.h>
#include <google/protobuf/stubs/stringpiece.h>
#include <google/protobuf/util/json_util.h>
@@ -1242,7 +1242,7 @@ Status ModifyRangePartition(const RunnerContext& context,
PartitionAction action
hash_schema_str, &hash_schema, opts); !s.ok()) {
return Status::InvalidArgument(
Substitute("unable to parse JSON: $0", hash_schema_str),
- s.error_message().ToString());
+ s.message().ToString());
}
}
@@ -1851,7 +1851,7 @@ Status CreateTable(const RunnerContext& context) {
if (!google_status.ok()) {
return Status::InvalidArgument(
Substitute("unable to parse JSON: $0", json_str),
- google_status.error_message().ToString());
+ google_status.message().ToString());
}
client::sp::shared_ptr<KuduClient> client;
diff --git a/src/kudu/tools/tool_action_test.cc
b/src/kudu/tools/tool_action_test.cc
index c861bd6a7..c903e2270 100644
--- a/src/kudu/tools/tool_action_test.cc
+++ b/src/kudu/tools/tool_action_test.cc
@@ -28,7 +28,7 @@
#include <gflags/gflags.h>
#include <glog/logging.h>
-#include <google/protobuf/stubs/common.h>
+#include <google/protobuf/stubs/common.h> // IWYU pragma: keep
#include <google/protobuf/stubs/status.h>
#include <google/protobuf/stubs/stringpiece.h>
#include <google/protobuf/util/json_util.h>
@@ -451,7 +451,7 @@ string SerializeRequest(const ControlShellRequestPB& req) {
req, &serialized);
CHECK(google_status.ok()) << Substitute(
"unable to serialize JSON ($0): $1",
- google_status.error_message().ToString(),
pb_util::SecureDebugString(req));
+ google_status.message().ToString(), pb_util::SecureDebugString(req));
return serialized;
}
diff --git a/thirdparty/vars.sh b/thirdparty/vars.sh
index 2e837da2f..3bcf0d7f2 100644
--- a/thirdparty/vars.sh
+++ b/thirdparty/vars.sh
@@ -54,7 +54,7 @@ GPERFTOOLS_VERSION=2.13
GPERFTOOLS_NAME=gperftools-$GPERFTOOLS_VERSION
GPERFTOOLS_SOURCE=$TP_SOURCE_DIR/$GPERFTOOLS_NAME
-PROTOBUF_VERSION=3.14.0
+PROTOBUF_VERSION=3.21.9
PROTOBUF_NAME=protobuf-$PROTOBUF_VERSION
PROTOBUF_SOURCE=$TP_SOURCE_DIR/$PROTOBUF_NAME