github-actions[bot] commented on code in PR #65676:
URL: https://github.com/apache/doris/pull/65676#discussion_r3609524120
##########
be/src/format/table/deletion_vector_reader.cpp:
##########
@@ -17,11 +17,77 @@
#include "format/table/deletion_vector_reader.h"
+#include <limits>
+
#include "rapidjson/document.h"
#include "rapidjson/stringbuffer.h"
#include "util/block_compression.h"
namespace doris {
+namespace {
+
+constexpr int64_t ICEBERG_DELETION_VECTOR_MIN_BYTES =
+ static_cast<int64_t>(ICEBERG_DELETION_VECTOR_BLOB_OVERHEAD_BYTES);
+constexpr int64_t PAIMON_DELETION_VECTOR_MIN_BYTES = 8;
+constexpr int64_t PAIMON_LENGTH_PREFIX_BYTES = 4;
+
+enum class DeletionVectorSizeLimitStatus {
+ DATA_QUALITY,
+ NOT_SUPPORTED,
+};
+
+Status validate_deletion_vector_read_range(const char* description, int64_t
offset, int64_t size,
+ int64_t min_size, int64_t max_size,
+ DeletionVectorSizeLimitStatus
size_limit_status,
+ size_t& bytes_read) {
+ if (offset < 0) {
+ return Status::DataQualityError("{} offset must be non-negative: {}",
description, offset);
+ }
+ if (size < min_size) {
+ return Status::DataQualityError("{} size too small: {}, minimum: {}",
description, size,
+ min_size);
+ }
+ if (size > max_size) {
+ if (size_limit_status == DeletionVectorSizeLimitStatus::NOT_SUPPORTED)
{
+ return Status::NotSupported("{} size exceeds Doris supported
limit: {}, limit: {}",
+ description, size, max_size);
+ }
+ return Status::DataQualityError("{} size exceeds limit: {}, limit:
{}", description, size,
+ max_size);
+ }
+ if (offset > std::numeric_limits<int64_t>::max() - size) {
+ return Status::DataQualityError("{} offset plus size overflows: offset
{}, size {}",
+ description, offset, size);
+ }
+ bytes_read = static_cast<size_t>(size);
Review Comment:
[P1] Check the opened file size before allocating the descriptor range
This success path only proves the descriptor arithmetic and cap. For
example, a Paimon deletion file can describe
`offset=0,length=MAX_PAIMON_DELETION_VECTOR_BYTES-4` while the referenced
object is only 8 bytes; FE forwards those values without a containing-file
size, so this returns a 1 GiB `bytes_read`. Every native cache-miss path then
calls `DeletionVectorReader::open()`, which already records the real
`_file_reader->size()`, but V1 Paimon, V2 `TableReader`, and the shared Iceberg
helper still allocate `std::vector<char>(bytes_read)` before `read_at()`
reports the short file. Thus one malformed in-cap descriptor can still force
about a 1 GiB allocation or `bad_alloc`, which defeats the validation's
stability goal. Please reject `_desc.start_offset/_desc.size` beyond the
authoritative size centrally in `open()` (with overflow-safe comparisons)
before returning to any caller, and cover a small-file/large-in-cap descriptor
in the common reader tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
