Storage - SeekableByteChannel Unnecessary Request Bugfix (Azure#49526)

ibrandes · Copilot · web-flow · commit 78bb953bb68f · 2026-06-18T15:12:14.000-07:00
* fixes and tests

* Apply suggestions from code review

Co-authored-by: Copilot Autofix powered by AI &lt;175728472+Copilot@users.noreply.github.com&gt;

* adding test recordings

* formatting

* style

* addressing comments

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI &lt;175728472+Copilot@users.noreply.github.com&gt;

---------

Co-authored-by: Copilot Autofix powered by AI &lt;175728472+Copilot@users.noreply.github.com&gt;
diff --git a/sdk/storage/azure-storage-blob/CHANGELOG.md b/sdk/storage/azure-storage-blob/CHANGELOG.md
@@ -7,6 +7,14 @@
 ### Breaking Changes
 
 ### Bugs Fixed
+- Fixed an issue where `BlobClientBase.openSeekableByteChannelRead` issued an unnecessary HTTP request (resulting
+  in an HTTP 416 response) after the entire blob had already been returned in the initial range download. When the
+  channel is opened with ETag consistency control (the default), the read behavior now short-circuits to end-of-file
+  once the known resource length is reached, avoiding the extra round trip.
+- Fixed an issue where a transport-level failure while streaming the body of the trailing HTTP 416 response from
+  `BlobClientBase.openSeekableByteChannelRead` (for example "Connection reset by peer") could propagate out of the
+  channel even though all of the blob's content had already been delivered to the caller. The read behavior now
+  logs a warning and signals end-of-file when such an error occurs at or past the known end of the resource.
 
 ### Other Changes
 
diff --git a/sdk/storage/azure-storage-blob/assets.json b/sdk/storage/azure-storage-blob/assets.json
@@ -2,5 +2,5 @@
   "AssetsRepo": "Azure/azure-sdk-assets",
   "AssetsRepoPrefixPath": "java",
   "TagPrefix": "java/storage/azure-storage-blob",
-  "Tag": "java/storage/azure-storage-blob_47f4243e59"
+  "Tag": "java/storage/azure-storage-blob_447cc62a15"
 }
diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehavior.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehavior.java
@@ -71,6 +71,15 @@ public int read(ByteBuffer dst, long sourceOffset) throws IOException {
             initialBufferPosition = null;
         }
 
+        // If the request is at or past the known end of the resource and the blob content is locked via an
+        // If-Match ETag, the blob cannot have grown without invalidating the precondition. Short-circuit and
+        // signal end-of-file rather than issuing a request that the service will reject with HTTP 416. This
+        // avoids an unnecessary round trip (and the failure modes that come with streaming the 416 response
+        // body, such as the connection being reset by the service).
+        if (sourceOffset >= resourceLength && isEtagLocked()) {
+            return -1;
+        }
+
         int initialPosition = dst.position();
 
         try (ByteBufferBackedOutputStreamUtil dstStream = new ByteBufferBackedOutputStreamUtil(dst)) {
@@ -90,7 +99,34 @@ public int read(ByteBuffer dst, long sourceOffset) throws IOException {
                 return sourceOffset < resourceLength ? 0 : -1;
             }
             throw LOGGER.logExceptionAsError(e);
+        } catch (RuntimeException e) {
+            // Reading the body of an HTTP 416 response can fail with transport-level errors (for example
+            // 'Connection reset by peer' wrapped in reactor's ReactiveException). When the requested offset is
+            // already at or past the known end of the resource, no data could have been returned anyway, so log
+            // the failure and signal end-of-file instead of propagating an exception to the caller that has
+            // already received all of the blob's content.
+            if (sourceOffset >= resourceLength && dst.position() == initialPosition) {
+                LOGGER.warning("Ignoring error encountered while issuing a read at or past the end of the blob; "
+                    + "treating as end-of-file because the resource length is already known.", e);
+                return -1;
+            }
+            throw LOGGER.logExceptionAsError(e);
+        }
+    }
+
+    /**
+     * @return Whether the request conditions on this behavior lock the blob's content via an If-Match ETag.
+     */
+    private boolean isEtagLocked() {
+        if (requestConditions == null) {
+            return false;
+        }
+
+        String ifMatch = requestConditions.getIfMatch();
+        if (CoreUtils.isNullOrEmpty(ifMatch)) {
+            return false;
         }
+        return !"*".equals(ifMatch.trim());
     }
 
     /**
diff --git a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehaviorTests.java b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehaviorTests.java
@@ -4,14 +4,18 @@
 package com.azure.storage.blob.specialized;
 
 import com.azure.core.http.HttpHeaders;
+import com.azure.core.http.HttpHeaderName;
+import com.azure.core.test.http.MockHttpResponse;
 import com.azure.core.test.utils.TestUtils;
 import com.azure.core.util.BinaryData;
 import com.azure.storage.blob.BlobTestBase;
 import com.azure.storage.blob.models.BlobDownloadAsyncResponse;
 import com.azure.storage.blob.models.BlobDownloadHeaders;
 import com.azure.storage.blob.models.BlobDownloadResponse;
+import com.azure.storage.blob.models.BlobErrorCode;
 import com.azure.storage.blob.models.BlobRange;
 import com.azure.storage.blob.models.BlobRequestConditions;
+import com.azure.storage.blob.models.BlobStorageException;
 import com.azure.storage.blob.models.PageRange;
 import com.azure.storage.common.implementation.Constants;
 import org.junit.jupiter.api.AfterEach;
@@ -35,9 +39,11 @@
 import java.util.stream.Stream;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
@@ -58,6 +64,10 @@ public void cleanup() {
         cc.deleteIfExists();
     }
 
+    private static BlobClientBase mockClient() {
+        return Mockito.mock(BlobClientBase.class);
+    }
+
     private BlobDownloadResponse createMockDownloadResponse(String contentRange) {
         String contentRangeHeader = "Content-Range";
         Map<String, String> headers = new HashMap<>();
@@ -70,7 +80,7 @@ private BlobDownloadResponse createMockDownloadResponse(String contentRange) {
     @MethodSource("readCallsToClientCorrectlySupplier")
     public void readCallsToClientCorrectly(long offset, int bufferSize, BlobRequestConditions conditions)
         throws IOException {
-        BlobClientBase client = Mockito.mock(BlobClientBase.class);
+        BlobClientBase client = mockClient();
         ArgumentCaptor<BlobRange> blobRangeCaptor = ArgumentCaptor.forClass(BlobRange.class);
         Mockito.when(client.downloadStreamWithResponse(any(), any(), any(), any(), anyBoolean(), any(), any()))
             .thenReturn(
@@ -100,7 +110,7 @@ private static Stream<Arguments> readCallsToClientCorrectlySupplier() {
     @MethodSource("readUsesCacheCorrectlySupplier")
     void readUsesCacheCorrectly(long offset, int bufferSize, int cacheSize) throws Exception {
         // given: "Behavior with a starting cached response"
-        BlobClientBase client = Mockito.mock(BlobClientBase.class);
+        BlobClientBase client = mockClient();
         ByteBuffer initialCache = getRandomData(cacheSize);
         StorageSeekableByteChannelBlobReadBehavior behavior
             = new StorageSeekableByteChannelBlobReadBehavior(client, initialCache, offset, Constants.MB, null);
@@ -273,4 +283,201 @@ void readDetectsBlobGrowth() throws IOException {
         assertEquals(buffer.capacity(), buffer.position());
         TestUtils.assertArraysEqual(data, halfLength, buffer.array(), 0, data.length - halfLength);
     }
+
+    /**
+     * The companion to {@link #readDetectsBlobGrowth()}: a blob can also shrink (e.g. it is overwritten with smaller
+     * content) while a channel is open. When a read targets an offset that is now past the (shrunk) end of the blob,
+     * the service responds with HTTP 416 {@code InvalidRange}. The behavior must surface the new, smaller resource
+     * length from the 416 response's {@code Content-Range} header and signal end-of-file.
+     */
+    @Test
+    void readDetectsBlobShrink() throws IOException {
+        // Given: data
+        int halfLength = 512;
+        byte[] data = getRandomByteArray(2 * halfLength);
+
+        // Blob initially at full size
+        String blockId1 = new String(Base64.getEncoder().encode("blockId1".getBytes()));
+        String blockId2 = new String(Base64.getEncoder().encode("blockId2".getBytes()));
+        blockBlobClient.stageBlock(blockId1, BinaryData.fromBytes(Arrays.copyOfRange(data, 0, halfLength)));
+        blockBlobClient.stageBlock(blockId2, BinaryData.fromBytes(Arrays.copyOfRange(data, halfLength, data.length)));
+        blockBlobClient.commitBlockList(Arrays.asList(blockId1, blockId2));
+
+        // behavior to read blob, initialized with the full resource length
+        StorageSeekableByteChannelBlobReadBehavior behavior = new StorageSeekableByteChannelBlobReadBehavior(
+            blockBlobClient, ByteBuffer.allocate(0), -1, 2 * halfLength, null);
+
+        // first half of blob read successfully
+        ByteBuffer buffer = ByteBuffer.allocate(halfLength);
+        int read = behavior.read(buffer, 0);
+
+        // behavior state as expected
+        assertEquals(halfLength, read);
+        assertEquals(2 * halfLength, behavior.getResourceLength());
+        assertEquals(buffer.capacity(), buffer.position());
+        TestUtils.assertArraysEqual(data, 0, buffer.array(), 0, halfLength);
+
+        // blob overwritten to half its previous size
+        blockBlobClient.commitBlockList(Collections.singletonList(blockId1), true);
+
+        // behavior reads at what used to be the middle of the blob, but is now past the end
+        buffer.clear();
+        read = behavior.read(buffer, halfLength);
+
+        // gracefully signal end of blob and update length to the new, smaller size
+        assertEquals(-1, read);
+        assertEquals(halfLength, behavior.getResourceLength());
+
+        // buffer unfilled
+        assertEquals(0, buffer.position());
+    }
+
+    /**
+     * Deterministically exercises the {@code InvalidRange} (HTTP 416) handling that detects shrink. When the service
+     * reports a smaller total size on the 416 {@code Content-Range} header, the behavior must adopt that length and
+     * either signal EOF (offset at/past the new end) or report zero bytes read (offset still within the new bounds).
+     */
+    @Test
+    public void readUpdatesLengthOnInvalidRangeResponse() throws IOException {
+        BlobClientBase client = mockClient();
+
+        // Channel was opened believing the blob was 2 KB; the service now reports it is only 1 KB.
+        long staleLength = 2 * Constants.KB;
+        long shrunkLength = Constants.KB;
+
+        // Case 1: offset is at/past the new end -> EOF (-1) and length updated.
+        Mockito.when(client.downloadStreamWithResponse(any(), any(), any(), any(), anyBoolean(), any(), any()))
+            .thenThrow(createInvalidRangeException("bytes */" + shrunkLength));
+
+        StorageSeekableByteChannelBlobReadBehavior behaviorAtEnd
+            = new StorageSeekableByteChannelBlobReadBehavior(client, ByteBuffer.allocate(0), -1, staleLength, null);
+
+        int readAtEnd = behaviorAtEnd.read(ByteBuffer.allocate(Constants.KB), shrunkLength);
+
+        assertEquals(-1, readAtEnd);
+        assertEquals(shrunkLength, behaviorAtEnd.getResourceLength());
+
+        // Case 2: offset is still within the new bounds -> zero bytes read (0) and length updated.
+        StorageSeekableByteChannelBlobReadBehavior behaviorWithin
+            = new StorageSeekableByteChannelBlobReadBehavior(client, ByteBuffer.allocate(0), -1, staleLength, null);
+
+        int readWithin = behaviorWithin.read(ByteBuffer.allocate(Constants.KB), shrunkLength - 100);
+
+        assertEquals(0, readWithin);
+        assertEquals(shrunkLength, behaviorWithin.getResourceLength());
+    }
+
+    private static BlobStorageException createInvalidRangeException(String contentRange) {
+        HttpHeaders headers = new HttpHeaders()
+            .set(Constants.HeaderConstants.ERROR_CODE_HEADER_NAME, BlobErrorCode.INVALID_RANGE.toString())
+            .set(HttpHeaderName.CONTENT_RANGE, contentRange);
+        return new BlobStorageException("The range specified is invalid.", new MockHttpResponse(null, 416, headers),
+            null);
+    }
+
+    /**
+     * When the request conditions lock the blob via an If-Match ETag, a read at or past the known end of the
+     * resource must short-circuit to EOF without issuing a service call (which the service would reject with an
+     * HTTP 416 response).
+     */
+    @Test
+    public void readPastEndShortCircuitsWhenETagLocked() throws IOException {
+        BlobClientBase client = mockClient();
+
+        long resourceLength = Constants.KB;
+        BlobRequestConditions conditions = new BlobRequestConditions().setIfMatch("0xETAG");
+        StorageSeekableByteChannelBlobReadBehavior behavior = new StorageSeekableByteChannelBlobReadBehavior(client,
+            ByteBuffer.allocate(0), -1, resourceLength, conditions);
+
+        // when: "Reading at the known end of the resource"
+        int readAtEnd = behavior.read(ByteBuffer.allocate(Constants.KB), resourceLength);
+
+        // then: "EOF is signaled without any service call"
+        assertEquals(-1, readAtEnd);
+        verify(client, never()).downloadStreamWithResponse(any(), any(), any(), any(), anyBoolean(), any(), any());
+
+        // when: "Reading past the known end of the resource"
+        int readPastEnd = behavior.read(ByteBuffer.allocate(Constants.KB), resourceLength + Constants.KB);
+
+        // then: "EOF is still signaled without any service call"
+        assertEquals(-1, readPastEnd);
+        verify(client, never()).downloadStreamWithResponse(any(), any(), any(), any(), anyBoolean(), any(), any());
+
+        // If-Match="*" is an existence precondition (not a specific ETag lock), so growth-detection behavior should be
+        // preserved and a request should still be issued.
+        Mockito.when(client.downloadStreamWithResponse(any(), any(), any(), any(), anyBoolean(), any(), any()))
+            .thenReturn(createMockDownloadResponse(
+                "bytes " + resourceLength + "-" + (resourceLength + Constants.KB - 1) + "/" + 2 * Constants.KB));
+
+        BlobRequestConditions ifMatchStar = new BlobRequestConditions().setIfMatch("*");
+        StorageSeekableByteChannelBlobReadBehavior starBehavior = new StorageSeekableByteChannelBlobReadBehavior(client,
+            ByteBuffer.allocate(0), -1, resourceLength, ifMatchStar);
+
+        starBehavior.read(ByteBuffer.allocate(Constants.KB), resourceLength);
+
+        verify(client, times(1)).downloadStreamWithResponse(any(), any(), any(), any(), anyBoolean(), any(), any());
+    }
+
+    /**
+     * When the blob is not ETag-locked, a read past the end must still issue a request so the behavior can
+     * detect blob growth (existing contract preserved).
+     */
+    @Test
+    public void readPastEndIssuesRequestWhenNotETagLocked() throws IOException {
+        BlobClientBase client = mockClient();
+        long resourceLength = Constants.KB;
+        Mockito.when(client.downloadStreamWithResponse(any(), any(), any(), any(), anyBoolean(), any(), any()))
+            .thenReturn(createMockDownloadResponse(
+                "bytes " + resourceLength + "-" + (resourceLength + Constants.KB - 1) + "/" + 2 * Constants.KB));
+
+        StorageSeekableByteChannelBlobReadBehavior behavior
+            = new StorageSeekableByteChannelBlobReadBehavior(client, ByteBuffer.allocate(0), -1, resourceLength, null);
+
+        // when: "Reading past the known end of the resource without an ETag lock"
+        behavior.read(ByteBuffer.allocate(Constants.KB), resourceLength);
+
+        // then: "A service call is issued (existing growth-detection behavior preserved)"
+        verify(client, times(1)).downloadStreamWithResponse(any(), any(), any(), any(), anyBoolean(), any(), any());
+    }
+
+    /**
+     * If the underlying download throws a non-{@code BlobStorageException} (for example, the connection is
+     * reset while streaming the body of a 416 response) and the caller is already at or past the known end of
+     * the resource, the behavior should swallow the error, log a warning, and signal EOF rather than throwing.
+     */
+    @Test
+    public void readPastEndSwallowsTransportErrorAndSignalsEof() throws IOException {
+        BlobClientBase client = mockClient();
+        long resourceLength = Constants.KB;
+        Mockito.when(client.downloadStreamWithResponse(any(), any(), any(), any(), anyBoolean(), any(), any()))
+            .thenThrow(new RuntimeException("Connection reset by peer"));
+
+        StorageSeekableByteChannelBlobReadBehavior behavior
+            = new StorageSeekableByteChannelBlobReadBehavior(client, ByteBuffer.allocate(0), -1, resourceLength, null);
+
+        // when: "Reading past the known end and the download fails with a transport-level error"
+        int read = behavior.read(ByteBuffer.allocate(Constants.KB), resourceLength);
+
+        // then: "EOF is signaled instead of the exception propagating"
+        assertEquals(-1, read);
+        verify(client, times(1)).downloadStreamWithResponse(any(), any(), any(), any(), anyBoolean(), any(), any());
+    }
+
+    /**
+     * Non-{@code BlobStorageException} failures that occur while reading within the known bounds of the resource
+     * must continue to surface as exceptions, since some bytes the caller asked for could not be retrieved.
+     */
+    @Test
+    public void readBeforeEndOfBlobPropagatesTransportError() {
+        BlobClientBase client = mockClient();
+        long resourceLength = Constants.KB;
+        Mockito.when(client.downloadStreamWithResponse(any(), any(), any(), any(), anyBoolean(), any(), any()))
+            .thenThrow(new RuntimeException("Connection reset by peer"));
+
+        StorageSeekableByteChannelBlobReadBehavior behavior
+            = new StorageSeekableByteChannelBlobReadBehavior(client, ByteBuffer.allocate(0), -1, resourceLength, null);
+
+        // Reading within the known resource range should still surface the error.
+        assertThrows(RuntimeException.class, () -> behavior.read(ByteBuffer.allocate(Constants.KB), 0));
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -2,5 +2,5 @@`
`2`	`2`	`"AssetsRepo": "Azure/azure-sdk-assets",`
`3`	`3`	`"AssetsRepoPrefixPath": "java",`
`4`	`4`	`"TagPrefix": "java/storage/azure-storage-blob",`
`5`		`- "Tag": "java/storage/azure-storage-blob_47f4243e59"`
	`5`	`+ "Tag": "java/storage/azure-storage-blob_447cc62a15"`
`6`	`6`	`}`