pw_protobuf: Enforce kMaxVarintSize
Updates the streaming encoder to enforce submessage size limits
introduced by a smaller kMaxVarintSize. Switches the unit test for this
configuration option to use the new StreamingEncoder interfaces.
Bug: 384
No-Docs-Update-Reason: Internal bugfix
Change-Id: Iec06858c88b4b402292076bb410328ad8693debd
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/46620
Reviewed-by: Alexei Frolov <frolv@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
Pigweed-Auto-Submit: Armando Montanez <amontanez@google.com>
diff --git a/pw_protobuf/public/pw_protobuf/config.h b/pw_protobuf/public/pw_protobuf/config.h
index c7a12eb..bafbf33 100644
--- a/pw_protobuf/public/pw_protobuf/config.h
+++ b/pw_protobuf/public/pw_protobuf/config.h
@@ -41,8 +41,8 @@
inline constexpr size_t kMaxVarintSize = PW_PROTOBUF_CFG_MAX_VARINT_SIZE;
-// TODO(frolv): This converts the configured varint length to the legacy encoder
-// SizeType. Remove this with the encoder rewrite.
+// TODO(pwbug/384): This converts the configured varint length to the legacy
+// encoder SizeType. Remove this with the encoder rewrite.
#if PW_PROTOBUF_CFG_MAX_VARINT_SIZE == 1
using SizeType = uint8_t;
#elif PW_PROTOBUF_CFG_MAX_VARINT_SIZE == 2
diff --git a/pw_protobuf/streaming_encoder.cc b/pw_protobuf/streaming_encoder.cc
index 8311a77..c763d64 100644
--- a/pw_protobuf/streaming_encoder.cc
+++ b/pw_protobuf/streaming_encoder.cc
@@ -39,12 +39,17 @@
size_t reserved_size = key_size + config::kMaxVarintSize;
size_t max_size = std::min(memory_writer_.ConservativeWriteLimit(),
writer_.ConservativeWriteLimit());
+ // Account for reserved bytes.
+ max_size = max_size > reserved_size ? max_size - reserved_size : 0;
+ // Cap based on max varint size.
+ max_size = std::min(varint::MaxValueInBytes(config::kMaxVarintSize),
+ static_cast<uint64_t>(max_size));
ByteSpan nested_buffer;
- if (reserved_size < max_size) {
+ if (max_size > 0) {
nested_buffer = ByteSpan(
memory_writer_.data() + reserved_size + memory_writer_.bytes_written(),
- max_size - reserved_size);
+ max_size);
} else {
nested_buffer = ByteSpan();
}
diff --git a/pw_protobuf/varint_size_test.cc b/pw_protobuf/varint_size_test.cc
index 246b265..c36c8b4 100644
--- a/pw_protobuf/varint_size_test.cc
+++ b/pw_protobuf/varint_size_test.cc
@@ -14,59 +14,62 @@
#include "gtest/gtest.h"
#include "pw_bytes/array.h"
-#include "pw_protobuf/encoder.h"
+#include "pw_protobuf/streaming_encoder.h"
namespace pw::protobuf {
namespace {
TEST(Encoder, SizeTypeIsConfigured) {
- static_assert(sizeof(Encoder::SizeType) == sizeof(uint8_t));
+ static_assert(config::kMaxVarintSize == sizeof(uint8_t));
}
TEST(Encoder, NestedWriteSmallerThanVarintSize) {
std::array<std::byte, 256> buffer;
- NestedEncoder<2, 2> encoder(buffer);
- encoder.Push(1);
+ // TODO(pwbug/384): Use Encoder when MemoryEncoder is renamed.
+ MemoryEncoder encoder(buffer);
+
+ StreamingEncoder nested = encoder.GetNestedEncoder(1);
// 1 byte key + 1 byte size + 125 byte value = 127 byte nested length.
- EXPECT_EQ(encoder.WriteBytes(2, bytes::Initialized<125>(0xaa)), OkStatus());
- encoder.Pop();
+ EXPECT_EQ(nested.WriteBytes(2, bytes::Initialized<125>(0xaa)), OkStatus());
+ nested.Finalize();
- auto result = encoder.Encode();
- EXPECT_EQ(result.status(), OkStatus());
+ EXPECT_EQ(encoder.status(), OkStatus());
}
-TEST(Encoder, NestedWriteLargerThanVarintSizeReturnsOutOfRange) {
+TEST(Encoder, NestedWriteLargerThanVarintSizeReturnsResourceExhausted) {
std::array<std::byte, 256> buffer;
- NestedEncoder<2, 2> encoder(buffer);
+
+ // TODO(pwbug/384): Use Encoder when MemoryEncoder is renamed.
+ MemoryEncoder encoder(buffer);
// Try to write a larger nested message than the max nested varint value.
- encoder.Push(1);
+ StreamingEncoder nested = encoder.GetNestedEncoder(1);
// 1 byte key + 1 byte size + 126 byte value = 128 byte nested length.
- EXPECT_EQ(encoder.WriteBytes(2, bytes::Initialized<126>(0xaa)),
- Status::OutOfRange());
- EXPECT_EQ(encoder.WriteUint32(3, 42), Status::OutOfRange());
- encoder.Pop();
+ EXPECT_EQ(nested.WriteBytes(2, bytes::Initialized<126>(0xaa)),
+ Status::ResourceExhausted());
+ EXPECT_EQ(nested.WriteUint32(3, 42), Status::ResourceExhausted());
+ nested.Finalize();
- auto result = encoder.Encode();
- EXPECT_EQ(result.status(), Status::OutOfRange());
+ EXPECT_EQ(encoder.status(), Status::ResourceExhausted());
}
-TEST(Encoder, NestedMessageLargerThanVarintSizeReturnsOutOfRange) {
+TEST(Encoder, NestedMessageLargerThanVarintSizeReturnsResourceExhausted) {
std::array<std::byte, 256> buffer;
- NestedEncoder<2, 2> encoder(buffer);
+
+ // TODO(pwbug/384): Use Encoder when MemoryEncoder is renamed.
+ MemoryEncoder encoder(buffer);
// Try to write a larger nested message than the max nested varint value as
// multiple smaller writes.
- encoder.Push(1);
- EXPECT_EQ(encoder.WriteBytes(2, bytes::Initialized<60>(0xaa)), OkStatus());
- EXPECT_EQ(encoder.WriteBytes(3, bytes::Initialized<60>(0xaa)), OkStatus());
- EXPECT_EQ(encoder.WriteBytes(4, bytes::Initialized<60>(0xaa)),
- Status::OutOfRange());
- encoder.Pop();
+ StreamingEncoder nested = encoder.GetNestedEncoder(1);
+ EXPECT_EQ(nested.WriteBytes(2, bytes::Initialized<60>(0xaa)), OkStatus());
+ EXPECT_EQ(nested.WriteBytes(3, bytes::Initialized<60>(0xaa)), OkStatus());
+ EXPECT_EQ(nested.WriteBytes(4, bytes::Initialized<60>(0xaa)),
+ Status::ResourceExhausted());
+ nested.Finalize();
- auto result = encoder.Encode();
- EXPECT_EQ(result.status(), Status::OutOfRange());
+ EXPECT_EQ(encoder.status(), Status::ResourceExhausted());
}
} // namespace
