wombatu-kun commented on code in PR #20321:
URL: https://github.com/apache/hudi/pull/20321#discussion_r4278345426


##########
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:
   `validateHeader` leaves `onDiskDataSizeWithHeader` unchecked although 
`unpack` sizes the GZIP input from it, and a buffer with fewer than 
`HFILEBLOCK_HEADER_SIZE` bytes left at `startOffsetInBuff` still fails with 
`ArrayIndexOutOfBoundsException` in the header reads that run before this 
check. Worth covering both here or in a follow-up, e.g. rejecting 
`onDiskDataSizeWithHeader` outside `[HFILEBLOCK_HEADER_SIZE, 
getOnDiskSizeWithHeader()]` and checking in `parse` that the header fits before 
the magic is read?



-- 
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]

Reply via email to