Skip to content

Commit a96939c

Browse files
Address code review feedback
- Use static imports for constants in test file - Consolidate SUCCESS/NEED_MORE_BYTES/COMPLETED case handling in policy - Improve segment size validation against remaining message bytes - Simplify condition using getAvailableBytes() helper Co-authored-by: gunjansingh-msft <[email protected]>
1 parent 67b25ec commit a96939c

File tree

3 files changed

+13
-16
lines changed

3 files changed

+13
-16
lines changed

sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/structuredmessage/StructuredMessageDecoder.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -415,8 +415,10 @@ private boolean tryReadSegmentHeader(ByteBuffer buffer) {
415415
"Unexpected segment number. Expected: " + (currentSegmentNumber + 1) + ", got: " + segmentNum));
416416
}
417417

418-
// Validate segment size
419-
if (segmentSize < 0 || segmentSize > Integer.MAX_VALUE) {
418+
// Validate segment size - must be non-negative and reasonable
419+
// We can't have segments larger than the remaining message length
420+
long remainingMessageBytes = messageLength - messageOffset - V1_SEGMENT_HEADER_LENGTH;
421+
if (segmentSize < 0 || segmentSize > remainingMessageBytes) {
420422
LOGGER.error("Invalid segment length read: segmentLength={}, decoderOffset={}, lastCompleteSegment={}",
421423
segmentSize, messageOffset, lastCompleteSegmentStart);
422424
throw LOGGER.logExceptionAsError(new IllegalArgumentException(
@@ -626,8 +628,8 @@ public DecodeResult decodeChunk(ByteBuffer buffer) {
626628
"Decode completed");
627629
}
628630

629-
// If we couldn't read any bytes and buffer is empty, need more
630-
if (payloadRead == 0 && buffer.remaining() == 0 && pendingBytes.size() == 0) {
631+
// If we couldn't read any bytes and no data available, need more
632+
if (payloadRead == 0 && getAvailableBytes(buffer) == 0) {
631633
break;
632634
}
633635
}

sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/policy/StorageContentValidationDecoderPolicy.java

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -107,18 +107,10 @@ private Flux<ByteBuffer> decodeStream(Flux<ByteBuffer> encodedFlux, DecoderState
107107
switch (result.getStatus()) {
108108
case SUCCESS:
109109
case NEED_MORE_BYTES:
110-
// Update state counters
111-
state.totalEncodedBytesProcessed.set(state.decoder.getMessageOffset());
112-
state.totalBytesDecoded.set(state.decoder.getTotalDecodedPayloadBytes());
113-
state.decodedBytesAtLastCompleteSegment = state.decoder.getTotalDecodedPayloadBytes();
114-
115-
if (result.getDecodedPayload() != null && result.getDecodedPayload().hasRemaining()) {
116-
return Flux.just(result.getDecodedPayload());
117-
}
118-
return Flux.empty();
119-
120110
case COMPLETED:
121-
// Update state counters
111+
// All three cases update counters and return any decoded payload
112+
// SUCCESS and NEED_MORE_BYTES: partial decode, more data expected
113+
// COMPLETED: decode finished successfully
122114
state.totalEncodedBytesProcessed.set(state.decoder.getMessageOffset());
123115
state.totalBytesDecoded.set(state.decoder.getTotalDecodedPayloadBytes());
124116
state.decodedBytesAtLastCompleteSegment = state.decoder.getTotalDecodedPayloadBytes();

sdk/storage/azure-storage-common/src/test/java/com/azure/storage/common/implementation/structuredmessage/StructuredMessageDecoderTests.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
import java.nio.ByteOrder;
1111
import java.util.concurrent.ThreadLocalRandom;
1212

13+
import static com.azure.storage.common.implementation.structuredmessage.StructuredMessageConstants.CRC64_LENGTH;
14+
import static com.azure.storage.common.implementation.structuredmessage.StructuredMessageConstants.V1_HEADER_LENGTH;
15+
import static com.azure.storage.common.implementation.structuredmessage.StructuredMessageConstants.V1_SEGMENT_HEADER_LENGTH;
1316
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
1417
import static org.junit.jupiter.api.Assertions.assertEquals;
1518
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -175,7 +178,7 @@ public void resetToLastCompleteSegmentWorks() throws IOException {
175178

176179
// Parse first segment completely, then simulate interruption
177180
// First segment ends after: header(13) + segment_header(10) + content(256) + crc(8) = 287
178-
int firstSegmentEnd = 13 + 10 + 256 + 8;
181+
int firstSegmentEnd = V1_HEADER_LENGTH + V1_SEGMENT_HEADER_LENGTH + 256 + CRC64_LENGTH;
179182
ByteBuffer chunk1 = ByteBuffer.wrap(encodedBytes, 0, firstSegmentEnd + 5); // 5 bytes into second segment header
180183
chunk1.order(ByteOrder.LITTLE_ENDIAN);
181184

0 commit comments

Comments
 (0)