yihua commented on code in PR #20321:
URL: https://github.com/apache/hudi/pull/20321#discussion_r4286739046
##########
hudi-io/src/main/java/org/apache/hudi/io/hfile/HFileBlock.java:
##########
@@ -137,6 +143,25 @@ protected HFileBlock(HFileContext context,
this.startOffsetInBuff + HFILEBLOCK_HEADER_SIZE +
uncompressedSizeWithoutHeader;
}
+ /**
+ * Rejects impossible header sizes before they are used to allocate buffers
or locate content.
+ */
+ private void validateHeader(byte[] byteBuff, int startOffsetInBuff) {
+ long blockEndInBuff = (long) startOffsetInBuff + HFILEBLOCK_HEADER_SIZE +
onDiskSizeWithoutHeader;
+ long unpackedSize = (long) HFILEBLOCK_HEADER_SIZE +
uncompressedSizeWithoutHeader + sizeCheckSum;
+ boolean uncompressedExceedsOnDisk =
CompressionCodec.NONE.equals(context.getCompressionCodec())
+ && uncompressedSizeWithoutHeader > onDiskSizeWithoutHeader;
+ if (onDiskSizeWithoutHeader < 0 || uncompressedSizeWithoutHeader < 0 ||
bytesPerChecksum <= 0
Review Comment:
Fixed. `validateHeader` now rejects `onDiskDataSizeWithHeader` outside
`[HFILEBLOCK_HEADER_SIZE, getOnDiskSizeWithHeader()]`, and `parse` checks that
a full header fits in the buffer before reading the magic. Both have tests: the
out-of-range sizes were accepted at parse time before, and the short buffer
threw `ArrayIndexOutOfBoundsException`.
##########
hudi-io/src/main/java/org/apache/hudi/io/hfile/HFileBlock.java:
##########
@@ -122,7 +126,9 @@ protected HFileBlock(HFileContext context,
byteBuff, startOffsetInBuff +
Header.UNCOMPRESSED_SIZE_WITHOUT_HEADER_INDEX);
this.bytesPerChecksum = readInt(
byteBuff, startOffsetInBuff + Header.BYTES_PER_CHECKSUM_INDEX);
- this.sizeCheckSum = numChecksumBytes(getOnDiskSizeWithHeader(),
bytesPerChecksum);
+ this.sizeCheckSum = bytesPerChecksum > 0
Review Comment:
The placeholder is gone: the raw header sizes are validated first and the
checksum size is computed after.
##########
hudi-io/src/main/java/org/apache/hudi/io/hfile/HFileBlock.java:
##########
@@ -295,8 +320,15 @@ public ByteBuffer serialize() throws IOException {
dataOutputStream.write(blockType.getMagic(), 0, 8);
// 2. onDiskSizeWithoutHeader.
int compressedDataSize = compressedBlockData.limit();
- int onDiskDataSizeWithHeader = HFileBlock.HFILEBLOCK_HEADER_SIZE +
compressedDataSize;
- int numChecksumBytes = numChecksumBytes(onDiskDataSizeWithHeader,
DEFAULT_BYTES_PER_CHECKSUM);
+ long onDiskDataSizeWithHeaderLong = (long)
HFileBlock.HFILEBLOCK_HEADER_SIZE + compressedDataSize;
Review Comment:
Done, the `long` keeps the name and the narrowed value is cast at the write.
--
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]
