github-actions[bot] commented on code in PR #67013:
URL: https://github.com/apache/doris/pull/67013#discussion_r3826970825
##########
cloud/src/meta-store/blob_message.cpp:
##########
@@ -240,31 +246,47 @@ void BlobIterator::load_current_blob() {
}
}
-bool BlobIterator::extract_origin_key(std::string_view raw_key, std::string*
output,
- uint8_t* version, uint16_t* sequence) {
- // The suffix is 8 bytes: |version(1)|dummy(5)|sequence(2)|
+bool decode_blob_key(
+ std::string_view raw_key, std::string* origin_key, uint8_t* version,
uint16_t* sequence,
+ std::vector<std::tuple<std::variant<int64_t, std::string>, int, int>>*
fields) {
if (raw_key.size() < 9) {
- LOG_WARNING("failed to extract origin key").tag("key", hex(raw_key));
- error_code_ = TxnErrorCode::TXN_INVALID_DATA;
return false;
}
const size_t origin_key_size = raw_key.size() - 9;
- std::string_view origin_key = raw_key.substr(0, origin_key_size);
+ std::string_view decoded_origin_key = raw_key.substr(0, origin_key_size);
raw_key.remove_prefix(origin_key_size);
int64_t suffix = 0;
if (decode_int64(&raw_key, &suffix) != 0) {
- LOG_WARNING("failed to decode int64")
- .tag("key", hex(raw_key))
- .tag("origin_key", hex(origin_key));
- error_code_ = TxnErrorCode::TXN_INVALID_DATA;
return false;
}
+ if (fields != nullptr) {
+ if (decoded_origin_key.size() <= 1) {
+ return false;
+ }
+ auto encoded_origin_key = decoded_origin_key;
+ encoded_origin_key.remove_prefix(1);
+ if (decode_key(&encoded_origin_key, fields) != 0) {
Review Comment:
[P2] Support valid inline-versionstamp origin keys
This generic `decode_key` call rejects at least one canonical structured
origin. `versioned::snapshot_reference_key(...)` encodes a versionstamp
followed directly by the encoded `ref_instance_id`, while `decode_key`
unconditionally requires the post-versionstamp input to end in
`VERSIONSTAMP_END_TAG`. Consequently
`decode_blob_key(encode_blob_key(versioned::snapshot_reference_key(...)), ...,
&fields)` returns `false` for a valid Doris key. Please make fields validation
handle both legal versionstamp layouts (or route through a decoder that does)
and add a snapshot-reference round-trip test.
##########
cloud/src/meta-store/blob_message.cpp:
##########
@@ -240,31 +246,47 @@ void BlobIterator::load_current_blob() {
}
}
-bool BlobIterator::extract_origin_key(std::string_view raw_key, std::string*
output,
- uint8_t* version, uint16_t* sequence) {
- // The suffix is 8 bytes: |version(1)|dummy(5)|sequence(2)|
+bool decode_blob_key(
+ std::string_view raw_key, std::string* origin_key, uint8_t* version,
uint16_t* sequence,
+ std::vector<std::tuple<std::variant<int64_t, std::string>, int, int>>*
fields) {
if (raw_key.size() < 9) {
- LOG_WARNING("failed to extract origin key").tag("key", hex(raw_key));
- error_code_ = TxnErrorCode::TXN_INVALID_DATA;
return false;
}
const size_t origin_key_size = raw_key.size() - 9;
- std::string_view origin_key = raw_key.substr(0, origin_key_size);
+ std::string_view decoded_origin_key = raw_key.substr(0, origin_key_size);
raw_key.remove_prefix(origin_key_size);
int64_t suffix = 0;
if (decode_int64(&raw_key, &suffix) != 0) {
- LOG_WARNING("failed to decode int64")
- .tag("key", hex(raw_key))
- .tag("origin_key", hex(origin_key));
- error_code_ = TxnErrorCode::TXN_INVALID_DATA;
return false;
}
+ if (fields != nullptr) {
+ if (decoded_origin_key.size() <= 1) {
+ return false;
+ }
+ auto encoded_origin_key = decoded_origin_key;
+ encoded_origin_key.remove_prefix(1);
Review Comment:
[P2] Validate the key-space byte before discarding it
Passing `fields` is documented as enabling origin-key validation, but this
removes the first byte without checking it. For example, if a valid
`meta_delete_bitmap_key(...)` has only its leading `0x01` changed to `0x04`,
`decode_key` sees the same remaining bytes and this helper returns `true`, even
though `keys.h` defines only key spaces `0x01` through `0x03`. A caller using
this API to validate blob keys can therefore accept a corrupted or
out-of-domain origin key. Please reject unsupported key-space bytes before
`remove_prefix(1)` and add a mutated-prefix negative test.
--
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]
