Jefffrey commented on code in PR #10959:
URL: https://github.com/apache/arrow-rs/pull/10959#discussion_r4043247707
##########
arrow-ipc/src/compression.rs:
##########
@@ -375,14 +386,17 @@ fn decompress_zstd(
))
}
-/// Get the uncompressed length
-/// Notes:
-/// LENGTH_NO_COMPRESSED_DATA: indicate that the data that follows is not
compressed
-/// 0: indicate that there is no data
-/// positive number: indicate the uncompressed length for the following data
-/// Returns an error if the input buffer is shorter than 8 bytes
+/// Reads the uncompressed length from a compressed IPC buffer.
+///
+/// A positive value is the exact expected uncompressed length, `0` indicates
+/// that there is no data, and `-1` indicates that the bytes following the
+/// prefix are not compressed.
+/// IPC decompression limits its output buffer to a positive advertised length
+/// and rejects output whose length differs.
+///
+/// Returns an error if the input buffer is shorter than the 8-byte prefix.
#[inline]
-fn read_uncompressed_size(buffer: &[u8]) -> Result<i64, ArrowError> {
+pub fn read_uncompressed_size(buffer: &[u8]) -> Result<i64, ArrowError> {
Review Comment:
im a bit hesistant on making this public, especially given its a relatively
simply function anyway. it just reads an i64 from a buffer, surely itd be
easier to vendor that wherever its needed?
##########
arrow-ipc/src/compression.rs:
##########
@@ -375,21 +386,27 @@ fn decompress_zstd(
))
}
-/// Get the uncompressed length
-/// Notes:
-/// LENGTH_NO_COMPRESSED_DATA: indicate that the data that follows is not
compressed
-/// 0: indicate that there is no data
-/// positive number: indicate the uncompressed length for the following data
-/// Returns an error if the input buffer is shorter than 8 bytes
+/// Reads the uncompressed length from a compressed IPC buffer.
+///
+/// A positive value is the exact expected uncompressed length, `0` indicates
+/// that there is no data, and `-1` indicates that the bytes following the
+/// prefix are not compressed.
+/// IPC decompression limits its output buffer to a positive advertised length
+/// and rejects output whose length differs.
+///
+/// Returns an error if the input buffer is shorter than the 8-byte prefix.
#[inline]
-fn read_uncompressed_size(buffer: &[u8]) -> Result<i64, ArrowError> {
+pub fn read_uncompressed_size(buffer: &[u8]) -> Result<i64, ArrowError> {
let len_buffer = buffer.get(..LENGTH_OF_PREFIX_DATA as
usize).ok_or_else(|| {
ArrowError::IpcError(format!(
"Compressed IPC buffer is too short: expected at least
{LENGTH_OF_PREFIX_DATA} bytes, got {}",
buffer.len()
))
})?;
- Ok(i64::from_le_bytes(len_buffer.try_into().unwrap()))
+ let len_buffer: [u8; 8] = len_buffer
+ .try_into()
+ .map_err(|e| ArrowError::IpcError(format!("Invalid uncompressed length
prefix: {e}")))?;
Review Comment:
is this error actually reachable? we already guarantee `len_buffer` has 8
bytes because of the `get()` above
--
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]
