pitrou commented on code in PR #50972:
URL: https://github.com/apache/arrow/pull/50972#discussion_r3850182827
##########
cpp/src/parquet/thrift_internal.h:
##########
@@ -586,57 +587,64 @@ class ThriftDeserializer {
container_size_limit_(container_size_limit) {}
// Deserialize a thrift message from buf/len. buf/len must at least contain
- // all the bytes needed to store the thrift message. On return, len will be
- // set to the actual length of the header.
+ // all the bytes needed to store the thrift message.
+ // The actual length of the header is returned.
template <class T>
- void DeserializeMessage(const uint8_t* buf, uint32_t* len, T*
deserialized_msg,
- Decryptor* decryptor = NULLPTR) {
+ int64_t DeserializeMessage(const uint8_t* buf, int64_t len, T*
deserialized_msg,
+ Decryptor* decryptor = NULLPTR) {
if (decryptor == NULLPTR) {
// thrift message is not encrypted
- DeserializeUnencryptedMessage(buf, len, deserialized_msg);
+ return DeserializeUnencryptedMessage(buf, len, deserialized_msg);
} else {
// thrift message is encrypted
- uint32_t clen;
- clen = *len;
- if (clen > static_cast<uint32_t>(std::numeric_limits<int32_t>::max())) {
+ if (len > std::numeric_limits<int32_t>::max()) {
std::stringstream ss;
- ss << "Cannot decrypt buffer with length " << clen << ", which
overflows int32\n";
+ ss << "Cannot decrypt buffer with length " << len << ", which
overflows int32\n";
throw ParquetException(ss.str());
}
// decrypt
auto decrypted_buffer = AllocateBuffer(
- decryptor->pool(),
decryptor->PlaintextLength(static_cast<int32_t>(clen)));
- std::span<const uint8_t> cipher_buf(buf, clen);
- uint32_t decrypted_buffer_len =
+ decryptor->pool(),
decryptor->PlaintextLength(static_cast<int32_t>(len)));
+ std::span<const uint8_t> cipher_buf(buf, len);
+ int32_t decrypted_buffer_len =
decryptor->Decrypt(cipher_buf,
decrypted_buffer->mutable_span_as<uint8_t>());
if (decrypted_buffer_len <= 0) {
throw ParquetException("Couldn't decrypt buffer\n");
}
- *len =
decryptor->CiphertextLength(static_cast<int32_t>(decrypted_buffer_len));
- DeserializeUnencryptedMessage(decrypted_buffer->data(),
&decrypted_buffer_len,
+ int64_t read_bytes = decryptor->CiphertextLength(decrypted_buffer_len);
+ ARROW_DCHECK_LE(read_bytes, len); // XXX should they be equal?
+ DeserializeUnencryptedMessage(decrypted_buffer->data(),
decrypted_buffer_len,
deserialized_msg);
+ return read_bytes;
}
}
private:
// On Thrift 0.14.0+, we want to use TConfiguration to raise the max message
size
// limit (ARROW-13655). If we wanted to protect against huge messages, we
could
// do it ourselves since we know the message size up front.
- std::shared_ptr<ThriftBuffer> CreateReadOnlyMemoryBuffer(uint8_t* buf,
uint32_t len) {
+ std::shared_ptr<ThriftBuffer> CreateReadOnlyMemoryBuffer(uint8_t* buf,
int64_t len) {
+ if (len >= static_cast<int64_t>(std::numeric_limits<uint32_t>::max())) {
+ std::stringstream ss;
+ ss << "Cannot decrypt deserialize Thrift message with length " << len
+ << ", which overflows uint32\n";
Review Comment:
Oops, you're right!
--
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]
