diff --git a/src/google/protobuf/compiler/cpp/file_unittest.cc b/src/google/protobuf/compiler/cpp/file_unittest.cc index f7cec1d2d9ba0..ad07b07dfc0f8 100644 --- a/src/google/protobuf/compiler/cpp/file_unittest.cc +++ b/src/google/protobuf/compiler/cpp/file_unittest.cc @@ -7,13 +7,13 @@ #include "google/protobuf/compiler/cpp/file.h" -#include -#include +#include #include +#include #include -#include "absl/strings/match.h" #include "absl/strings/string_view.h" +#include "absl/strings/strip.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/unittest.pb.h" @@ -94,6 +94,7 @@ TEST(FileTest, TopologicallyOrderedDescriptors) { "TestExtensionInsideTable", "TestEmptyMessageWithExtensions", "TestEmptyMessage", + "TestEagerlyVerifiedLazyMessage.LazyMessage", "TestDynamicExtensions.DynamicMessageType", "TestDupFieldNumber.Foo", "TestDupFieldNumber.Bar", @@ -149,6 +150,7 @@ TEST(FileTest, TopologicallyOrderedDescriptors) { "TestGroup", "TestForeignNested", "TestFieldOrderings", + "TestEagerlyVerifiedLazyMessage", "TestEagerMaybeLazy.NestedMessage", "TestDynamicExtensions", "TestDupFieldNumber", @@ -196,23 +198,14 @@ TEST(FileTest, TopologicallyOrderedDescriptors) { "TestLazyMessageRepeated", "TestNestedRequiredForeign", }; - static constexpr size_t kExpectedDescriptorCount = - std::end(kExpectedDescriptorOrder) - std::begin(kExpectedDescriptorOrder); - std::vector actual_descriptor_order = - FileGeneratorFriendForTesting::MessagesInTopologicalOrder(fgen); - EXPECT_TRUE(kExpectedDescriptorCount == actual_descriptor_order.size()) - << "Expected: " << kExpectedDescriptorCount - << ", got: " << actual_descriptor_order.size(); - - auto limit = - std::min(kExpectedDescriptorCount, actual_descriptor_order.size()); - for (auto i = 0u; i < limit; ++i) { - const Descriptor* desc = actual_descriptor_order[i]; - bool match = absl::EndsWith(desc->full_name(), kExpectedDescriptorOrder[i]); - EXPECT_TRUE(match) << "failed to match; expected " - << kExpectedDescriptorOrder[i] << ", got " - << desc->full_name(); + std::vector actual_order; + for (const Descriptor* desc : + FileGeneratorFriendForTesting::MessagesInTopologicalOrder(fgen)) { + actual_order.emplace_back( + absl::StripPrefix(desc->full_name(), "protobuf_unittest.")); } + EXPECT_THAT(actual_order, + ::testing::ElementsAreArray(kExpectedDescriptorOrder)); } } // namespace diff --git a/src/google/protobuf/edition_unittest.proto b/src/google/protobuf/edition_unittest.proto index c0d0ebbbf768f..794c1e5c277ed 100644 --- a/src/google/protobuf/edition_unittest.proto +++ b/src/google/protobuf/edition_unittest.proto @@ -1177,6 +1177,14 @@ message TestMessageSize { int64 m6 = 6; } +// Tests eager verification of a lazy message field. +message TestEagerlyVerifiedLazyMessage { + message LazyMessage { + bytes bytes_field = 1; + } + LazyMessage lazy_message = 1 [lazy = true]; +} + // Test that RPC services work. message FooRequest {} message FooResponse {} diff --git a/src/google/protobuf/message_unittest.inc b/src/google/protobuf/message_unittest.inc index 1aa329e43af6b..03b381e3866c5 100644 --- a/src/google/protobuf/message_unittest.inc +++ b/src/google/protobuf/message_unittest.inc @@ -51,6 +51,7 @@ #include "google/protobuf/message.h" #include "google/protobuf/reflection_ops.h" #include "google/protobuf/test_util2.h" +#include "google/protobuf/wire_format_lite.h" // Must be included last. @@ -364,6 +365,29 @@ TEST(MESSAGE_TEST_NAME, ExplicitLazyExceedRecursionLimit) { EXPECT_NE(parsed.lazy_child().child().payload().optional_int32(), -1); } +TEST(MESSAGE_TEST_NAME, ExplicitLazyBadLengthDelimitedSize) { + std::string serialized; + + // This is a regression test for a bug in lazy field verification. It + // requires invalid wire format to trigger the bug. + + // NestedMessage optional_lazy_message = 27 [lazy=true]; + uint32_t tag = internal::WireFormatLite::MakeTag( + 1, internal::WireFormatLite::WIRETYPE_LENGTH_DELIMITED); + ASSERT_LT(tag, INT8_MAX); + serialized.push_back(tag); + serialized.push_back(6); + + // bytes bytes_field = 1; + serialized.push_back(tag); + + // To trigger this bug, we need an overlong size. + serialized.append(5, 0xff); + + UNITTEST::TestLazyMessage parsed; + EXPECT_FALSE(parsed.ParseFromString(serialized)); +} + TEST(MESSAGE_TEST_NAME, NestedLazyRecursionLimit) { UNITTEST::NestedTestAllTypes original, parsed; original.mutable_lazy_child() diff --git a/src/google/protobuf/parse_context.h b/src/google/protobuf/parse_context.h index 52490d12c75e9..b69af7be570e8 100644 --- a/src/google/protobuf/parse_context.h +++ b/src/google/protobuf/parse_context.h @@ -8,13 +8,18 @@ #ifndef GOOGLE_PROTOBUF_PARSE_CONTEXT_H__ #define GOOGLE_PROTOBUF_PARSE_CONTEXT_H__ +#include +#include +#include #include #include +#include #include #include #include #include "absl/base/config.h" +#include "absl/base/prefetch.h" #include "absl/log/absl_check.h" #include "absl/log/absl_log.h" #include "absl/strings/cord.h" @@ -27,9 +32,11 @@ #include "google/protobuf/inlined_string_field.h" #include "google/protobuf/io/coded_stream.h" #include "google/protobuf/io/zero_copy_stream.h" +#include "google/protobuf/message_lite.h" #include "google/protobuf/metadata_lite.h" #include "google/protobuf/port.h" #include "google/protobuf/repeated_field.h" +#include "google/protobuf/repeated_ptr_field.h" #include "google/protobuf/wire_format_lite.h" @@ -103,7 +110,7 @@ class PROTOBUF_EXPORT EpsCopyInputStream { ABSL_DCHECK(ptr <= buffer_end_ + kSlopBytes); int count; if (next_chunk_ == patch_buffer_) { - count = static_cast(buffer_end_ + kSlopBytes - ptr); + count = BytesAvailable(ptr); } else { count = size_ + static_cast(buffer_end_ - ptr); } @@ -170,14 +177,14 @@ class PROTOBUF_EXPORT EpsCopyInputStream { } PROTOBUF_NODISCARD const char* Skip(const char* ptr, int size) { - if (size <= buffer_end_ + kSlopBytes - ptr) { + if (size <= BytesAvailable(ptr)) { return ptr + size; } return SkipFallback(ptr, size); } PROTOBUF_NODISCARD const char* ReadString(const char* ptr, int size, std::string* s) { - if (size <= buffer_end_ + kSlopBytes - ptr) { + if (size <= BytesAvailable(ptr)) { // Fundamentally we just want to do assign to the string. // However micro-benchmarks regress on string reading cases. So we copy // the same logic from the old CodedInputStream ReadString. Note: as of @@ -191,7 +198,7 @@ class PROTOBUF_EXPORT EpsCopyInputStream { } PROTOBUF_NODISCARD const char* AppendString(const char* ptr, int size, std::string* s) { - if (size <= buffer_end_ + kSlopBytes - ptr) { + if (size <= BytesAvailable(ptr)) { s->append(ptr, size); return ptr + size; } @@ -204,8 +211,7 @@ class PROTOBUF_EXPORT EpsCopyInputStream { PROTOBUF_NODISCARD const char* ReadCord(const char* ptr, int size, ::absl::Cord* cord) { - if (size <= std::min(static_cast(buffer_end_ + kSlopBytes - ptr), - kMaxCordBytesToCopy)) { + if (size <= std::min(BytesAvailable(ptr), kMaxCordBytesToCopy)) { *cord = absl::string_view(ptr, size); return ptr + size; } @@ -345,6 +351,14 @@ class PROTOBUF_EXPORT EpsCopyInputStream { // systems. TODO do we need to set this as build flag? enum { kSafeStringSize = 50000000 }; + int BytesAvailable(const char* ptr) const { + ABSL_DCHECK_NE(ptr, nullptr); + ptrdiff_t available = buffer_end_ + kSlopBytes - ptr; + ABSL_DCHECK_GE(available, 0); + ABSL_DCHECK_LE(available, INT_MAX); + return static_cast(available); + } + // Advances to next buffer chunk returns a pointer to the same logical place // in the stream as set by overrun. Overrun indicates the position in the slop // region the parse was left (0 <= overrun <= kSlopBytes). Returns true if at @@ -382,7 +396,7 @@ class PROTOBUF_EXPORT EpsCopyInputStream { template const char* AppendSize(const char* ptr, int size, const A& append) { - int chunk_size = static_cast(buffer_end_ + kSlopBytes - ptr); + int chunk_size = BytesAvailable(ptr); do { ABSL_DCHECK(size > chunk_size); if (next_chunk_ == nullptr) return nullptr; @@ -396,7 +410,7 @@ class PROTOBUF_EXPORT EpsCopyInputStream { ptr = Next(); if (ptr == nullptr) return nullptr; // passed the limit ptr += kSlopBytes; - chunk_size = static_cast(buffer_end_ + kSlopBytes - ptr); + chunk_size = BytesAvailable(ptr); } while (size > chunk_size); append(ptr, size); return ptr + size; @@ -411,7 +425,7 @@ class PROTOBUF_EXPORT EpsCopyInputStream { const char* AppendUntilEnd(const char* ptr, const A& append) { if (ptr - buffer_end_ > limit_) return nullptr; while (limit_ > kSlopBytes) { - size_t chunk_size = buffer_end_ + kSlopBytes - ptr; + size_t chunk_size = BytesAvailable(ptr); append(ptr, chunk_size); ptr = Next(); if (ptr == nullptr) return limit_end_; @@ -1163,7 +1177,7 @@ template const char* EpsCopyInputStream::ReadPackedFixed(const char* ptr, int size, RepeatedField* out) { GOOGLE_PROTOBUF_PARSER_ASSERT(ptr); - int nbytes = static_cast(buffer_end_ + kSlopBytes - ptr); + int nbytes = BytesAvailable(ptr); while (size > nbytes) { int num = nbytes / sizeof(T); int old_entries = out->size(); @@ -1181,7 +1195,7 @@ const char* EpsCopyInputStream::ReadPackedFixed(const char* ptr, int size, ptr = Next(); if (ptr == nullptr) return nullptr; ptr += kSlopBytes - (nbytes - block_size); - nbytes = static_cast(buffer_end_ + kSlopBytes - ptr); + nbytes = BytesAvailable(ptr); } int num = size / sizeof(T); int block_size = num * sizeof(T); diff --git a/src/google/protobuf/unittest.proto b/src/google/protobuf/unittest.proto index 14cf08e0961ce..b6bb06ed9fabe 100644 --- a/src/google/protobuf/unittest.proto +++ b/src/google/protobuf/unittest.proto @@ -1725,6 +1725,14 @@ message OpenEnumMessage { repeated ForeignEnum repeated_closed = 4; } +// Tests eager verification of a lazy message field. +message TestEagerlyVerifiedLazyMessage { + message LazyMessage { + bytes bytes_field = 1; + } + LazyMessage lazy_message = 1 [lazy = true]; +} + // Test that RPC services work. message FooRequest { }