Let varint fields parse even if their value overflows the target integer or enum type. The overflow might happen if the field used to have a wider type. This aligns the semantics with native proto parsing. The cost is replacing a failure with silent data loss. Native proto parsing accepts that, so we can too. PiperOrigin-RevId: 907980660
diff --git a/riegeli/messages/BUILD b/riegeli/messages/BUILD index 12d85fe..459a3ae 100644 --- a/riegeli/messages/BUILD +++ b/riegeli/messages/BUILD
@@ -167,7 +167,6 @@ "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/status", - "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:cord", "@com_google_absl//absl/strings:string_view", ],
diff --git a/riegeli/messages/field_handlers.cc b/riegeli/messages/field_handlers.cc index 03d1bd6..344838d 100644 --- a/riegeli/messages/field_handlers.cc +++ b/riegeli/messages/field_handlers.cc
@@ -21,7 +21,6 @@ #include "absl/base/nullability.h" #include "absl/status/status.h" -#include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "riegeli/bytes/reader.h" @@ -37,71 +36,6 @@ } } -template <> -absl::Status VarintOverflowError<int32_t, field_handlers::VarintKind::kPlain>( - uint64_t repr) { - return absl::InvalidArgumentError( - absl::StrCat("int32 field overflow: ", repr)); -} - -template <> -absl::Status VarintOverflowError<uint32_t, field_handlers::VarintKind::kPlain>( - uint64_t repr) { - return absl::InvalidArgumentError( - absl::StrCat("uint32 field overflow: ", repr)); -} - -template <> -absl::Status VarintOverflowError<int32_t, field_handlers::VarintKind::kSigned>( - uint64_t repr) { - return absl::InvalidArgumentError( - absl::StrCat("sint32 field overflow: ", repr)); -} - -template <> -absl::Status VarintOverflowError<bool, field_handlers::VarintKind::kPlain>( - uint64_t repr) { - return absl::InvalidArgumentError( - absl::StrCat("bool field overflow: ", repr)); -} - -absl::Status EnumOverflowError(uint64_t repr) { - return absl::InvalidArgumentError( - absl::StrCat("enum field overflow: ", repr)); -} - -template <> -absl::Status VarintOverflowError<int32_t, field_handlers::VarintKind::kPlain>( - Reader& src, uint64_t repr) { - return src.StatusOrAnnotate( - VarintOverflowError<int32_t, field_handlers::VarintKind::kPlain>(repr)); -} - -template <> -absl::Status VarintOverflowError<uint32_t, field_handlers::VarintKind::kPlain>( - Reader& src, uint64_t repr) { - return src.StatusOrAnnotate( - VarintOverflowError<uint32_t, field_handlers::VarintKind::kPlain>(repr)); -} - -template <> -absl::Status VarintOverflowError<int32_t, field_handlers::VarintKind::kSigned>( - Reader& src, uint64_t repr) { - return src.StatusOrAnnotate( - VarintOverflowError<int32_t, field_handlers::VarintKind::kSigned>(repr)); -} - -absl::Status EnumOverflowError(Reader& src, uint64_t repr) { - return src.StatusOrAnnotate(EnumOverflowError(repr)); -} - -template <> -absl::Status VarintOverflowError<bool, field_handlers::VarintKind::kPlain>( - Reader& src, uint64_t repr) { - return src.StatusOrAnnotate( - VarintOverflowError<bool, field_handlers::VarintKind::kPlain>(repr)); -} - absl::Status ReadPackedVarintError() { return absl::InvalidArgumentError( "Could not read a varint element of a packed repeated field");
diff --git a/riegeli/messages/field_handlers.h b/riegeli/messages/field_handlers.h index 786469d..00080ec 100644 --- a/riegeli/messages/field_handlers.h +++ b/riegeli/messages/field_handlers.h
@@ -30,7 +30,6 @@ #include "absl/strings/cord.h" #include "absl/strings/string_view.h" #include "riegeli/base/arithmetic.h" -#include "riegeli/base/assert.h" #include "riegeli/base/chain.h" #include "riegeli/base/cord_iterator_span.h" #include "riegeli/base/types.h" @@ -86,10 +85,6 @@ // fields (packed or not), and it will be called also for singular fields // (which have the same wire representation as a non-packed repeated field). // -// For varint fields, in contrast to native proto parsing, 64-bit values which -// overflow the provided 32-bit type are reported as errors instead of being -// silently truncated. -// // Two kinds of field handlers are provided by these functions: // // * Static, which handle a single field number known at compile time. @@ -467,54 +462,6 @@ ABSL_ATTRIBUTE_COLD absl::Status AnnotateByReader(absl::Status status, Reader& reader); -ABSL_ATTRIBUTE_COLD absl::Status EnumOverflowError(uint64_t repr); - -template <typename Value, field_handlers::VarintKind kind> -absl::Status VarintOverflowError(uint64_t repr) { - RIEGELI_ASSERT(kind == field_handlers::VarintKind::kEnum) - << "Remaining VarintOverflowError() instantiations should be for enums"; - return EnumOverflowError(repr); -} -template <> -ABSL_ATTRIBUTE_COLD absl::Status -VarintOverflowError<int32_t, field_handlers::VarintKind::kPlain>(uint64_t repr); -template <> -ABSL_ATTRIBUTE_COLD absl::Status -VarintOverflowError<uint32_t, field_handlers::VarintKind::kPlain>( - uint64_t repr); -template <> -ABSL_ATTRIBUTE_COLD absl::Status -VarintOverflowError<int32_t, field_handlers::VarintKind::kSigned>( - uint64_t repr); -template <> -ABSL_ATTRIBUTE_COLD absl::Status -VarintOverflowError<bool, field_handlers::VarintKind::kPlain>(uint64_t repr); - -ABSL_ATTRIBUTE_COLD absl::Status EnumOverflowError(Reader& src, uint64_t repr); - -template <typename Value, field_handlers::VarintKind kind> -absl::Status VarintOverflowError(Reader& src, uint64_t repr) { - RIEGELI_ASSERT(kind == field_handlers::VarintKind::kEnum) - << "Remaining VarintOverflowError() instantiations should be for enums"; - return EnumOverflowError(src, repr); -} -template <> -ABSL_ATTRIBUTE_COLD absl::Status -VarintOverflowError<int32_t, field_handlers::VarintKind::kPlain>(Reader& src, - uint64_t repr); -template <> -ABSL_ATTRIBUTE_COLD absl::Status -VarintOverflowError<uint32_t, field_handlers::VarintKind::kPlain>( - Reader& src, uint64_t repr); -template <> -ABSL_ATTRIBUTE_COLD absl::Status -VarintOverflowError<int32_t, field_handlers::VarintKind::kSigned>( - Reader& src, uint64_t repr); -template <> -ABSL_ATTRIBUTE_COLD absl::Status -VarintOverflowError<bool, field_handlers::VarintKind::kPlain>(Reader& src, - uint64_t repr); - ABSL_ATTRIBUTE_COLD absl::Status ReadPackedVarintError(); ABSL_ATTRIBUTE_COLD absl::Status ReadPackedVarintError(Reader& src); @@ -536,22 +483,6 @@ Reader& src); template <typename Value, field_handlers::VarintKind kind> -inline bool VarintIsValid(uint64_t repr) { - if constexpr (std::disjunction_v<std::is_same<Value, uint64_t>, - std::is_same<Value, int64_t>>) { - return true; - } else if constexpr (kind == field_handlers::VarintKind::kSigned) { - return uint64_t{static_cast<std::make_unsigned_t<Value>>(repr)} == repr; - } else if constexpr (std::is_enum_v<Value>) { - static_assert(kind == field_handlers::VarintKind::kEnum); - return static_cast<uint64_t>( - static_cast<std::underlying_type_t<Value>>(repr)) == repr; - } else { - return static_cast<uint64_t>(static_cast<Value>(repr)) == repr; - } -} - -template <typename Value, field_handlers::VarintKind kind> inline Value DecodeVarint(uint64_t repr) { if constexpr (kind == field_handlers::VarintKind::kSigned) { return static_cast<Value>(DecodeVarintSigned64(repr)); @@ -585,10 +516,6 @@ std::enable_if_t<std::is_invocable_v<const Action&, Value, Context&...>, int> = 0> absl::Status HandleVarint(uint64_t repr, Context&... context) const { - if (ABSL_PREDICT_FALSE( - (!field_handlers_internal::VarintIsValid<Value, kind>(repr)))) { - return field_handlers_internal::VarintOverflowError<Value, kind>(repr); - } return action_(field_handlers_internal::DecodeVarint<Value, kind>(repr), context...); } @@ -1031,11 +958,6 @@ ScopedLimiter scoped_limiter(repr); uint64_t element; while (ReadVarint64(repr.reader(), element)) { - if (ABSL_PREDICT_FALSE( - (!field_handlers_internal::VarintIsValid<Value, kind>(element)))) { - return field_handlers_internal::VarintOverflowError<Value, kind>( - repr.reader(), element); - } if (absl::Status status = this->action()( field_handlers_internal::DecodeVarint<Value, kind>(element), context...); @@ -1078,10 +1000,6 @@ while (ReadVarint64(repr.iterator(), CordIteratorSpan::Remaining(repr.iterator()) - limit, element)) { - if (ABSL_PREDICT_FALSE( - (!field_handlers_internal::VarintIsValid<Value, kind>(element)))) { - return field_handlers_internal::VarintOverflowError<Value, kind>(element); - } if (absl::Status status = this->action()( field_handlers_internal::DecodeVarint<Value, kind>(element), context...); @@ -1109,10 +1027,6 @@ while (const size_t length = ReadVarint64(cursor, PtrDistance(cursor, limit), element)) { cursor += length; - if (ABSL_PREDICT_FALSE( - (!field_handlers_internal::VarintIsValid<Value, kind>(element)))) { - return field_handlers_internal::VarintOverflowError<Value, kind>(element); - } if (absl::Status status = this->action()( field_handlers_internal::DecodeVarint<Value, kind>(element), context...);