Skip to content

Commit 8432b47

Browse files
KaiSernLimclaude
andcommitted
🤖 Address Copilot review comments: input validation and nullability docs
- Add key-length guard before Arrays.copyOf in chunked GlobalRtDiv manifest path to avoid NegativeArraySizeException on corrupted/short keys - Add METADATA_PARTITION_ID check to putGlobalRtDivChunk/getGlobalRtDivChunk for consistency with other metadata partition APIs - Annotate getGlobalRtDivChunk return type with @nullable and add Javadoc to make the null-when-missing contract explicit at the interface level Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 282dec3 commit 8432b47

3 files changed

Lines changed: 16 additions & 0 deletions

File tree

clients/da-vinci-client/src/main/java/com/linkedin/davinci/kafka/consumer/StoreIngestionTask.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4121,6 +4121,10 @@ protected void putGlobalRtDivStateInMetadata(int partition, byte[] keyBytes, Put
41214121
if (put.schemaId == CHUNK_MANIFEST_SCHEMA_ID) {
41224122
// Manifest for a chunked GlobalRtDiv: assemble all chunks from the metadata partition,
41234123
// decompress the result, then persist it in the metadata partition.
4124+
if (keyBytes.length < KEY_CHUNKING_SUFFIX_LENGTH) {
4125+
throw new VeniceException(
4126+
"GlobalRtDiv chunk manifest key too short to contain chunking suffix: " + Arrays.toString(keyBytes));
4127+
}
41244128
byte[] originalKeyBytes = Arrays.copyOf(keyBytes, keyBytes.length - KEY_CHUNKING_SUFFIX_LENGTH);
41254129
String key = new String(originalKeyBytes);
41264130
if (!key.startsWith(GLOBAL_RT_DIV_KEY_PREFIX)) {

clients/da-vinci-client/src/main/java/com/linkedin/davinci/store/AbstractStorageEngine.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,10 @@ public synchronized void putGlobalRtDivChunk(int partitionId, byte[] chunkKey, b
727727
if (!metadataPartitionCreated()) {
728728
throw new StorageInitializationException("Metadata partition not created!");
729729
}
730+
if (partitionId == METADATA_PARTITION_ID) {
731+
throw new IllegalArgumentException(
732+
"Metadata partition id should not be used as argument in putGlobalRtDivChunk.");
733+
}
730734
if (partitionId < 0) {
731735
throw new IllegalArgumentException("Invalid partition id argument in putGlobalRtDivChunk");
732736
}
@@ -738,6 +742,10 @@ public synchronized byte[] getGlobalRtDivChunk(int partitionId, byte[] chunkKey)
738742
if (!metadataPartitionCreated()) {
739743
throw new StorageInitializationException("Metadata partition not created!");
740744
}
745+
if (partitionId == METADATA_PARTITION_ID) {
746+
throw new IllegalArgumentException(
747+
"Metadata partition id should not be used as argument in getGlobalRtDivChunk.");
748+
}
741749
if (partitionId < 0) {
742750
throw new IllegalArgumentException("Invalid partition id argument in getGlobalRtDivChunk");
743751
}

clients/da-vinci-client/src/main/java/com/linkedin/davinci/store/StorageEngine.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.util.Optional;
1313
import java.util.Set;
1414
import java.util.function.Supplier;
15+
import javax.annotation.Nullable;
1516

1617

1718
public interface StorageEngine<Partition extends AbstractStoragePartition> extends Closeable {
@@ -176,7 +177,10 @@ void putWithReplicationMetadata(int partitionId, byte[] key, ByteBuffer value, b
176177

177178
/**
178179
* Retrieve a GlobalRtDiv intermediate chunk from the metadata partition.
180+
*
181+
* @return the chunk bytes, or {@code null} if the chunk is not present
179182
*/
183+
@Nullable
180184
byte[] getGlobalRtDivChunk(int partitionId, byte[] chunkKey);
181185

182186
/**

0 commit comments

Comments
 (0)