pw_protobuf: Add helper for map entry write
Expose low level APIs in pw_protobuf for estimating field size and
writing length-delimited field key and length prefix. Use them to
implement a helper function in pw_software_update for writing proto
map<string, bytes> entries.
Context: UpdateBundle needs to construct the following Manifest
proto message from blob storage:
message Manifest {
optional SnapshotMetadata snapshot_metadata = 1;
map<string, TargetsMetadata> targets_metadata = 2;
}
For wire format generation, it is essentially equivalent to the
following definition with a nested `Entry` message.
message Entry {
string key = 1;
bytes value = 2;
}
message Manifest {
optional bytes snapshot_metadata = 1;
repeated Entry targets_metadata = 2;
}
Although protobuf::StreamEncoder has capability for nested message
encoding, it requires a scratch buffer that shall be at least the
largest sub-message size. In this case, it will be largest
target metadata in the update software bundle, which however, can be
fairly large and difficult to estimate. To avoid the issue, the CL
takes an approach to construct the message from lower level.
Specifically, the CL constructs the delimited field key and length
prefix for `Entry` on its own and write to output, then followed by
writing a regular string field of `key` and bytes field of `value` via
the normal StreamEncoder approach.
Change-Id: Ie5f10f483ebceb587660a4a36e5c6674a09ce096
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/59681
Reviewed-by: Armando Montanez <amontanez@google.com>
Commit-Queue: Yecheng Zhao <zyecheng@google.com>
diff --git a/pw_protobuf/BUILD.bazel b/pw_protobuf/BUILD.bazel
index ba64f26..c343ee4 100644
--- a/pw_protobuf/BUILD.bazel
+++ b/pw_protobuf/BUILD.bazel
@@ -38,12 +38,14 @@
"decoder.cc",
"encoder.cc",
"find.cc",
+ "helpers.cc",
"stream_decoder.cc",
],
hdrs = [
"public/pw_protobuf/decoder.h",
"public/pw_protobuf/encoder.h",
"public/pw_protobuf/find.h",
+ "public/pw_protobuf/helpers.h",
"public/pw_protobuf/serialized_size.h",
"public/pw_protobuf/stream_decoder.h",
"public/pw_protobuf/wire_format.h",
@@ -108,6 +110,15 @@
],
)
+pw_cc_test(
+ name = "helpers_test",
+ srcs = ["helpers_test.cc"],
+ deps = [
+ ":pw_protobuf",
+ "//pw_unit_test",
+ ],
+)
+
proto_library(
name = "codegen_test_proto",
srcs = [
diff --git a/pw_protobuf/BUILD.gn b/pw_protobuf/BUILD.gn
index 2c4b231..a35d2a6 100644
--- a/pw_protobuf/BUILD.gn
+++ b/pw_protobuf/BUILD.gn
@@ -56,6 +56,7 @@
"public/pw_protobuf/decoder.h",
"public/pw_protobuf/encoder.h",
"public/pw_protobuf/find.h",
+ "public/pw_protobuf/helpers.h",
"public/pw_protobuf/serialized_size.h",
"public/pw_protobuf/stream_decoder.h",
"public/pw_protobuf/wire_format.h",
@@ -64,6 +65,7 @@
"decoder.cc",
"encoder.cc",
"find.cc",
+ "helpers.cc",
"stream_decoder.cc",
]
}
@@ -84,6 +86,7 @@
":encoder_fuzzer",
":find_test",
":stream_decoder_test",
+ ":helpers_test",
":varint_size_test",
]
}
@@ -113,6 +116,11 @@
sources = [ "stream_decoder_test.cc" ]
}
+pw_test("helpers_test") {
+ deps = [ ":pw_protobuf" ]
+ sources = [ "helpers_test.cc" ]
+}
+
config("one_byte_varint") {
defines = [ "PW_PROTOBUF_CFG_MAX_VARINT_SIZE=1" ]
visibility = [ ":*" ]
diff --git a/pw_protobuf/docs.rst b/pw_protobuf/docs.rst
index 79efe91..7e2232a 100644
--- a/pw_protobuf/docs.rst
+++ b/pw_protobuf/docs.rst
@@ -409,6 +409,16 @@
// parent decoder can be used again.
}
+Protobuf helpers
+================
+
+Some additional helpers for encoding more complex but common protobuf
+submessages (e.g. map<string, bytes>) are provided in
+``pw_protobuf/helpers.h``.
+
+.. Note::
+ The helper API are currently in-development and may not remain stable.
+
Size report
===========
diff --git a/pw_protobuf/encoder.cc b/pw_protobuf/encoder.cc
index 93cc777..156469a 100644
--- a/pw_protobuf/encoder.cc
+++ b/pw_protobuf/encoder.cc
@@ -20,6 +20,7 @@
#include "pw_assert/check.h"
#include "pw_bytes/span.h"
+#include "pw_protobuf/serialized_size.h"
#include "pw_protobuf/wire_format.h"
#include "pw_status/status.h"
#include "pw_status/try.h"
@@ -119,10 +120,9 @@
Status StreamEncoder::WriteLengthDelimitedField(uint32_t field_number,
ConstByteSpan data) {
PW_TRY(UpdateStatusForWrite(field_number, WireType::kDelimited, data.size()));
- WriteVarint(FieldKey(field_number, WireType::kDelimited))
- .IgnoreError(); // TODO(pwbug/387): Handle Status properly
- WriteVarint(data.size_bytes())
- .IgnoreError(); // TODO(pwbug/387): Handle Status properly
+ status_.Update(WriteLengthDelimitedKeyAndLengthPrefix(
+ field_number, data.size(), writer_));
+ PW_TRY(status_);
if (Status status = writer_.Write(data); !status.ok()) {
status_ = status;
}
@@ -137,10 +137,8 @@
PW_CHECK_UINT_GT(
stream_pipe_buffer.size(), 0, "Transfer buffer cannot be 0 size");
PW_TRY(UpdateStatusForWrite(field_number, WireType::kDelimited, num_bytes));
- // Ignore the error until we explicitly check status_ below to minimize
- // the number of branches.
- WriteVarint(FieldKey(field_number, WireType::kDelimited)).IgnoreError();
- WriteVarint(num_bytes).IgnoreError();
+ status_.Update(
+ WriteLengthDelimitedKeyAndLengthPrefix(field_number, num_bytes, writer_));
PW_TRY(status_);
// Stream data from `bytes_reader` to `writer_`.
@@ -177,28 +175,6 @@
return status_;
}
-// Encodes a base-128 varint to the buffer. This function assumes the caller
-// has already checked UpdateStatusForWrite() to ensure the writer's
-// conservative write limit indicates the Writer has sufficient buffer space.
-Status StreamEncoder::WriteVarint(uint64_t value) {
- if (!status_.ok()) {
- return status_;
- }
-
- std::array<std::byte, varint::kMaxVarint64SizeBytes> varint_encode_buffer;
- size_t varint_size = pw::varint::EncodeLittleEndianBase128(
- value, std::span(varint_encode_buffer));
-
- if (Status status =
- writer_.Write(std::span(varint_encode_buffer).first(varint_size));
- !status.ok()) {
- status_ = status;
- return status_;
- }
-
- return OkStatus();
-}
-
Status StreamEncoder::WritePackedFixed(uint32_t field_number,
std::span<const std::byte> values,
size_t elem_size) {
@@ -236,23 +212,20 @@
WireType type,
size_t data_size) {
PW_CHECK(!nested_encoder_open());
- if (!status_.ok()) {
- return status_;
- }
+ PW_TRY(status_);
+
if (!ValidFieldNumber(field_number)) {
- status_ = Status::InvalidArgument();
- return status_;
+ return status_ = Status::InvalidArgument();
}
- size_t size = varint::EncodedSize(FieldKey(field_number, type));
- if (type == WireType::kDelimited) {
- size += varint::EncodedSize(data_size);
- }
- size += data_size;
+ const Result<size_t> field_size = SizeOfField(field_number, type, data_size);
+ status_.Update(field_size.status());
+ PW_TRY(status_);
- if (size > writer_.ConservativeWriteLimit()) {
+ if (field_size.value() > writer_.ConservativeWriteLimit()) {
status_ = Status::ResourceExhausted();
}
+
return status_;
}
diff --git a/pw_protobuf/helpers.cc b/pw_protobuf/helpers.cc
new file mode 100644
index 0000000..8bf074c
--- /dev/null
+++ b/pw_protobuf/helpers.cc
@@ -0,0 +1,87 @@
+// Copyright 2021 The Pigweed Authors
+//
+// Licensed 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
+//
+// https://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 "pw_protobuf/helpers.h"
+
+#include <cstddef>
+
+#include "pw_protobuf/encoder.h"
+#include "pw_protobuf/serialized_size.h"
+
+namespace pw::protobuf {
+
+// TODO(pwbug/456): Generalize and move this helper to pw_protobuf
+//
+// Note that a map<string, bytes> is essentially
+//
+// message Entry {
+// string key = 1;
+// bytes value = 2;
+// }
+//
+// message Msg {
+// repeated Entry map_field = <field_number>;
+// }
+Status WriteProtoStringToBytesMapEntry(uint32_t field_number,
+ stream::Reader& key,
+ size_t key_size,
+ stream::Reader& value,
+ size_t value_size,
+ ByteSpan stream_pipe_buffer,
+ stream::Writer& writer) {
+ constexpr uint32_t kMapKeyFieldNumber = 1;
+ constexpr uint32_t kMapValueFieldNumber = 2;
+
+ if (!protobuf::ValidFieldNumber(field_number) ||
+ key_size >= std::numeric_limits<uint32_t>::max() ||
+ value_size >= std::numeric_limits<uint32_t>::max()) {
+ return Status::InvalidArgument();
+ }
+
+ Result<size_t> key_field_size = protobuf::SizeOfField(
+ kMapKeyFieldNumber, protobuf::WireType::kDelimited, key_size);
+ PW_TRY(key_field_size.status());
+
+ Result<size_t> value_field_size = protobuf::SizeOfField(
+ kMapValueFieldNumber, protobuf::WireType::kDelimited, value_size);
+ PW_TRY(value_field_size.status());
+
+ size_t entry_payload_total_size =
+ key_field_size.value() + value_field_size.value();
+
+ Result<size_t> entry_field_total_size = protobuf::SizeOfField(
+ field_number, protobuf::WireType::kDelimited, entry_payload_total_size);
+ PW_TRY(entry_field_total_size.status());
+
+ if (entry_field_total_size.value() > writer.ConservativeWriteLimit()) {
+ return Status::ResourceExhausted();
+ }
+
+ // Write field key and length prefix for nested message `Entry`
+ PW_TRY(protobuf::WriteLengthDelimitedKeyAndLengthPrefix(
+ field_number, entry_payload_total_size, writer));
+
+ protobuf::StreamEncoder encoder(writer, {});
+
+ // Write Entry::key
+ PW_TRY(encoder.WriteStringFromStream(
+ kMapKeyFieldNumber, key, key_size, stream_pipe_buffer));
+ // Write Entry::value
+ PW_TRY(encoder.WriteBytesFromStream(
+ kMapValueFieldNumber, value, value_size, stream_pipe_buffer));
+
+ return OkStatus();
+}
+
+} // namespace pw::protobuf
diff --git a/pw_protobuf/helpers_test.cc b/pw_protobuf/helpers_test.cc
new file mode 100644
index 0000000..6d2f8cf
--- /dev/null
+++ b/pw_protobuf/helpers_test.cc
@@ -0,0 +1,152 @@
+// Copyright 2021 The Pigweed Authors
+//
+// Licensed 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
+//
+// https://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 "pw_protobuf/helpers.h"
+
+#include "gtest/gtest.h"
+#include "pw_protobuf/encoder.h"
+
+namespace pw::protobuf {
+
+TEST(ProtoHelper, WriteProtoStringToBytesMapEntry) {
+ // The following defines an instance of the message below:
+ //
+ // message Maps {
+ // map<string, string> map_a = 1;
+ // map<string, string> map_b = 2;
+ // }
+ //
+ // where
+ //
+ // Maps.map_a['key_foo'] = 'foo_a'
+ // Maps.map_a['key_bar'] = 'bar_a'
+ //
+ // Maps.map_b['key_foo'] = 'foo_b'
+ // Maps.map_b['key_bar'] = 'bar_b'
+ //
+ // clang-format off
+ std::uint8_t encoded_proto[] = {
+ // map_a["key_bar"] = "bar_a", key = 1
+ 0x0a, 0x10,
+ 0x0a, 0x07, 'k', 'e', 'y', '_', 'b', 'a', 'r', // map key
+ 0x12, 0x05, 'b', 'a', 'r', '_', 'a', // map value
+
+ // map_a["key_foo"] = "foo_a", key = 1
+ 0x0a, 0x10,
+ 0x0a, 0x07, 'k', 'e', 'y', '_', 'f', 'o', 'o',
+ 0x12, 0x05, 'f', 'o', 'o', '_', 'a',
+
+ // map_b["key_foo"] = "foo_b", key = 2
+ 0x12, 0x10,
+ 0x0a, 0x07, 'k', 'e', 'y', '_', 'f', 'o', 'o',
+ 0x12, 0x05, 'f', 'o', 'o', '_', 'b',
+
+ // map_b["key_bar"] = "bar_b", key = 2
+ 0x12, 0x10,
+ 0x0a, 0x07, 'k', 'e', 'y', '_', 'b', 'a', 'r',
+ 0x12, 0x05, 'b', 'a', 'r', '_', 'b',
+ };
+ // clang-format on
+
+ // Now construct the same message with WriteStringToBytesMapEntry
+ std::byte dst_buffer[sizeof(encoded_proto)];
+ stream::MemoryWriter writer(dst_buffer);
+
+ const struct {
+ uint32_t field_number;
+ std::string_view key;
+ std::string_view value;
+ } kMapData[] = {
+ {1, "key_bar", "bar_a"},
+ {1, "key_foo", "foo_a"},
+ {2, "key_foo", "foo_b"},
+ {2, "key_bar", "bar_b"},
+ };
+
+ std::byte stream_pipe_buffer[1];
+ for (auto ele : kMapData) {
+ stream::MemoryReader key_reader(std::as_bytes(std::span{ele.key}));
+ stream::MemoryReader value_reader(std::as_bytes(std::span{ele.value}));
+ ASSERT_TRUE(WriteProtoStringToBytesMapEntry(ele.field_number,
+ key_reader,
+ ele.key.size(),
+ value_reader,
+ ele.value.size(),
+ stream_pipe_buffer,
+ writer)
+ .ok());
+ }
+
+ ASSERT_EQ(memcmp(dst_buffer, encoded_proto, sizeof(dst_buffer)), 0);
+}
+
+TEST(ProtoHelper, WriteProtoStringToBytesMapEntryExceedsWriteLimit) {
+ // Construct an instance of the message below:
+ //
+ // message Maps {
+ // map<string, string> map_a = 1;
+ // }
+ //
+ // where
+ //
+ // Maps.map_a['key_bar'] = 'bar_a'. The needed buffer size is 18 in this
+ // case:
+ //
+ // {
+ // 0x0a, 0x10,
+ // 0x0a, 0x07, 'k', 'e', 'y', '_', 'b', 'a', 'r',
+ // 0x12, 0x05, 'b', 'a', 'r', '_', 'a',
+ // }
+ //
+ // Use a smaller buffer.
+ std::byte encode_buffer[17];
+ stream::MemoryWriter writer(encode_buffer);
+ constexpr uint32_t kFieldNumber = 1;
+ std::string_view key = "key_bar";
+ std::string_view value = "bar_a";
+ stream::MemoryReader key_reader(std::as_bytes(std::span{key}));
+ stream::MemoryReader value_reader(std::as_bytes(std::span{value}));
+ std::byte stream_pipe_buffer[1];
+ ASSERT_EQ(
+ WriteProtoStringToBytesMapEntry(kFieldNumber,
+ key_reader,
+ key_reader.ConservativeReadLimit(),
+ value_reader,
+ value_reader.ConservativeReadLimit(),
+ stream_pipe_buffer,
+ writer),
+ Status::ResourceExhausted());
+}
+
+TEST(ProtoHelper, WriteProtoStringToBytesMapEntryInvalidArgument) {
+ std::byte encode_buffer[17];
+ stream::MemoryWriter writer(encode_buffer);
+ std::string_view key = "key_bar";
+ std::string_view value = "bar_a";
+ stream::MemoryReader key_reader(std::as_bytes(std::span{key}));
+ stream::MemoryReader value_reader(std::as_bytes(std::span{value}));
+ std::byte stream_pipe_buffer[1];
+
+ ASSERT_EQ(
+ WriteProtoStringToBytesMapEntry(19091,
+ key_reader,
+ key_reader.ConservativeReadLimit(),
+ value_reader,
+ value_reader.ConservativeReadLimit(),
+ stream_pipe_buffer,
+ writer),
+ Status::InvalidArgument());
+}
+
+} // namespace pw::protobuf
diff --git a/pw_protobuf/public/pw_protobuf/encoder.h b/pw_protobuf/public/pw_protobuf/encoder.h
index 72b2147..a404a38 100644
--- a/pw_protobuf/public/pw_protobuf/encoder.h
+++ b/pw_protobuf/public/pw_protobuf/encoder.h
@@ -51,6 +51,48 @@
return max_message_size + max_nested_depth * config::kMaxVarintSize;
}
+// Write a varint value to the writer.
+//
+// Args:
+// value: The value of the varint to write
+// writer: The writer for writing to output.
+//
+// Returns:
+// OK - varint is written successfully
+//
+// Errors encountered by the `writer` will be returned as it is.
+inline Status WriteVarint(uint64_t value, stream::Writer& writer) {
+ std::array<std::byte, varint::kMaxVarint64SizeBytes> varint_encode_buffer;
+ const size_t varint_size =
+ pw::varint::EncodeLittleEndianBase128(value, varint_encode_buffer);
+ return writer.Write(std::span(varint_encode_buffer).first(varint_size));
+}
+
+// Write the field key and length prefix for a length-delimited field. It is
+// up to the caller to ensure that this will be followed by an exact number
+// of bytes written for the field in order to form a valid proto message.
+//
+// Args:
+// field_number: The field number for the field.
+// payload_size: The size of the payload.
+// writer: The output writer to write to
+//
+//
+// Returns:
+// OK - Field key is written successfully
+//
+// Errors encountered by the `writer` will be returned as it is.
+//
+// Precondition: The field_number must be a ValidFieldNumber.
+// Precondition: `data_size_bytes` must be smaller than
+// std::numeric_limits<uint32_t>::max()
+inline Status WriteLengthDelimitedKeyAndLengthPrefix(uint32_t field_number,
+ size_t payload_size,
+ stream::Writer& writer) {
+ PW_TRY(WriteVarint(FieldKey(field_number, WireType::kDelimited), writer));
+ return WriteVarint(payload_size, writer);
+}
+
// Forward declaration. StreamEncoder and MemoryEncoder are very tightly
// coupled.
class MemoryEncoder;
@@ -471,8 +513,14 @@
// Implementation for encoding all fixed-length integer types.
Status WriteFixed(uint32_t field_number, ConstByteSpan data);
- // Encodes a base-128 varint to the buffer.
- Status WriteVarint(uint64_t value);
+ // Encodes a base-128 varint to the buffer. This function assumes the caller
+ // has already checked UpdateStatusForWrite() to ensure the writer's
+ // conservative write limit indicates the Writer has sufficient buffer space.
+ Status WriteVarint(uint64_t value) {
+ PW_TRY(status_);
+ status_.Update(::pw::protobuf::WriteVarint(value, writer_));
+ return status_;
+ }
Status WriteZigzagVarint(int64_t value) {
return WriteVarint(varint::ZigZagEncode(value));
diff --git a/pw_protobuf/public/pw_protobuf/helpers.h b/pw_protobuf/public/pw_protobuf/helpers.h
new file mode 100644
index 0000000..3c4c74f
--- /dev/null
+++ b/pw_protobuf/public/pw_protobuf/helpers.h
@@ -0,0 +1,55 @@
+// Copyright 2021 The Pigweed Authors
+//
+// Licensed 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
+//
+// https://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.
+//
+// The header provides a set of helper utils for protobuf related operations.
+// The APIs may not be finalized yet.
+
+#pragma once
+
+#include <cstddef>
+
+#include "pw_status/status.h"
+#include "pw_stream/stream.h"
+
+namespace pw::protobuf {
+
+// The function writes an entry for the proto map<string, bytes> field type.
+//
+// Args:
+// field_number - The field number for the map.
+// key - The string payload for the key value of the entry.
+// key_size - Number of bytes in the key.
+// value - The value payload for the entry.
+// value_size - Number of bytes in the value.
+// stream_pipe_buffer - A non-zero size buffer for the function to read and
+// store data from the reader and write to the given writer.
+// writer - The output writer to write to.
+//
+// Returns:
+// OK - Entry is successfully written.
+// RESOURCE_EXHAUSTED - Entry would exceed write limit.
+// INVALID_ARGUMENT - Field number is invalid.
+//
+// Since all length-delimited fields can be treated as `bytes`,
+// it can be used to write any string to length-delimited field map, i.e.
+// map<string, message>, map<string, bytes> etc.
+Status WriteProtoStringToBytesMapEntry(uint32_t field_number,
+ stream::Reader& key,
+ size_t key_size,
+ stream::Reader& value,
+ size_t value_size,
+ ByteSpan stream_pipe_buffer,
+ stream::Writer& writer);
+
+} // namespace pw::protobuf
diff --git a/pw_protobuf/public/pw_protobuf/serialized_size.h b/pw_protobuf/public/pw_protobuf/serialized_size.h
index dc51491..a20ba04 100644
--- a/pw_protobuf/public/pw_protobuf/serialized_size.h
+++ b/pw_protobuf/public/pw_protobuf/serialized_size.h
@@ -44,10 +44,48 @@
inline constexpr size_t kMaxSizeOfLength = varint::kMaxVarint32SizeBytes;
+// Calculate the size of a proto field key in wire format, including the key
+// field number + wire type.
+// type).
+//
+// Args:
+// field_number: The field number for the field.
+//
+// Returns:
+// The size of the field key.
+//
+// Precondition: The field_number must be a ValidFieldNumber.
+// Precondition: The field_number must be a ValidFieldNumber.
constexpr size_t SizeOfFieldKey(uint32_t field_number) {
// The wiretype does not impact the serialized size, so use kVarint (0), which
// will be optimized out by the compiler.
return varint::EncodedSize(FieldKey(field_number, WireType::kVarint));
}
+// Calculate the size of a proto field in wire format. This is the size of a
+// final serialized protobuf entry, including the key (field number + wire
+// type), encoded payload size (for length-delimited types), and data.
+//
+// Args:
+// field_number: The field number for the field.
+// type: The wire type of the field
+// data_size: The size of the payload.
+//
+// Returns:
+// The size of the field.
+//
+// Precondition: The field_number must be a ValidFieldNumber.
+// Precondition: `data_size_bytes` must be smaller than
+// std::numeric_limits<uint32_t>::max()
+constexpr size_t SizeOfField(uint32_t field_number,
+ WireType type,
+ size_t data_size_bytes) {
+ size_t size = SizeOfFieldKey(field_number);
+ if (type == WireType::kDelimited) {
+ size += varint::EncodedSize(data_size_bytes);
+ }
+ size += data_size_bytes;
+ return size;
+}
+
} // namespace pw::protobuf
