From 50e825dcd1fa68e46d86747b86ec97c59c11c028 Mon Sep 17 00:00:00 2001 From: Isabelle <141270045+ibrandes@users.noreply.github.com> Date: Tue, 8 Oct 2024 13:16:58 -0700 Subject: [PATCH] V2 Encryption Bug 4MB file downloads (#41813) * testing, need to get this branch on my other workstation * adding broken test * updating tests * tests with logging * updating test and adding fix * making test paramaterized and adding change for only encryption v2 downloads * fixing imports * wip context addition * context implementation and sync / async tests * adding back unpulled tests * consolidating one of the async util methods * addressing all comments besides 1 * removing leftover changes and adding back test for V2 * kyle test * adjusting unencrypted blob length calculation * recording tests and fixing style * addressing comments * changelog update * fixing messed up recorded test * adding more test cases and updating adjustment method signature * adding missed recordings and fixing style * changing offsetAdjustment to a long to prevent int overflow, and added support for unencrypted blob length for blobinputstream * adding test for integer overflow, removing redundant test, adding helper method to blobinputstream * adjusting changelong * removing unused imports * changing blobinputstream util method to use long instead of Long --- .../CHANGELOG.md | 2 + .../assets.json | 2 +- .../EncryptedBlobAsyncClient.java | 9 +- .../cryptography/EncryptedBlobClient.java | 9 +- .../cryptography/EncryptedBlobLength.java | 39 ++++++++ .../cryptography/EncryptedBlobRange.java | 10 +- .../EncryptedBlobLengthTests.java | 99 +++++++++++++++++++ .../cryptography/EncryptedBlobRangeTests.java | 12 +-- .../EncryptedBlockBlobApiTests.java | 83 ++++++++++++++-- .../util/ChunkedDownloadUtils.java | 21 +++- .../blob/specialized/BlobAsyncClientBase.java | 2 +- .../blob/specialized/BlobInputStream.java | 14 ++- .../common/implementation/Constants.java | 2 + 13 files changed, 278 insertions(+), 26 deletions(-) create mode 100644 sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLength.java create mode 100644 sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLengthTests.java diff --git a/sdk/storage/azure-storage-blob-cryptography/CHANGELOG.md b/sdk/storage/azure-storage-blob-cryptography/CHANGELOG.md index 092a0362629dc..7dba7864d6be5 100644 --- a/sdk/storage/azure-storage-blob-cryptography/CHANGELOG.md +++ b/sdk/storage/azure-storage-blob-cryptography/CHANGELOG.md @@ -7,6 +7,8 @@ ### Breaking Changes ### Bugs Fixed +- Fixed a bug where downloadToFile and openInputStream was throwing an InvalidRange exception if the target file size was a multiple of the + authenticated region length. ### Other Changes diff --git a/sdk/storage/azure-storage-blob-cryptography/assets.json b/sdk/storage/azure-storage-blob-cryptography/assets.json index 42f543e03640c..41ce54ac19b8e 100644 --- a/sdk/storage/azure-storage-blob-cryptography/assets.json +++ b/sdk/storage/azure-storage-blob-cryptography/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "java", "TagPrefix": "java/storage/azure-storage-blob-cryptography", - "Tag": "java/storage/azure-storage-blob-cryptography_e245db55c0" + "Tag": "java/storage/azure-storage-blob-cryptography_9fd41be58a" } diff --git a/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobAsyncClient.java b/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobAsyncClient.java index 465b69ef21f48..9e069680efda1 100644 --- a/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobAsyncClient.java +++ b/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobAsyncClient.java @@ -750,9 +750,14 @@ private Mono populateRequestConditionsAndContext(BlobRequestConditions re String encryptionDataKey = StorageImplUtils.getEncryptionDataKey(response.getValue().getMetadata()); if (encryptionDataKey != null) { - result = result.contextWrite(context -> context.put(ENCRYPTION_DATA_KEY, - EncryptionData.getAndValidateEncryptionData(encryptionDataKey, requiresEncryption))); + EncryptionData encryptionData = EncryptionData.getAndValidateEncryptionData(encryptionDataKey, + requiresEncryption); + + result = result.contextWrite(context -> context.put(ENCRYPTION_DATA_KEY, encryptionData)) + .contextWrite(context -> context.put(Constants.ADJUSTED_BLOB_LENGTH_KEY, + EncryptedBlobLength.computeAdjustedBlobLength(encryptionData, response.getValue().getBlobSize()))); } + return result; }); } diff --git a/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobClient.java b/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobClient.java index 4ffc8b809478e..e3998d4a22773 100644 --- a/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobClient.java +++ b/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobClient.java @@ -489,8 +489,12 @@ private Context populateRequestConditionsAndContext(BlobRequestConditions reques String encryptionDataKey = StorageImplUtils.getEncryptionDataKey(initialProperties.getMetadata()); if (encryptionDataKey != null) { - context = context.addData(ENCRYPTION_DATA_KEY, EncryptionData.getAndValidateEncryptionData( - encryptionDataKey, encryptedBlobAsyncClient.isEncryptionRequired())); + EncryptionData encryptionData = EncryptionData.getAndValidateEncryptionData( + encryptionDataKey, encryptedBlobAsyncClient.isEncryptionRequired()); + + context = context.addData(ENCRYPTION_DATA_KEY, encryptionData) + .addData(Constants.ADJUSTED_BLOB_LENGTH_KEY, + EncryptedBlobLength.computeAdjustedBlobLength(encryptionData, initialProperties.getBlobSize())); } return context; } @@ -572,5 +576,4 @@ public BlobQueryResponse queryWithResponse(BlobQueryOptions queryOptions, BlobClientSideEncryptionOptions getClientSideEncryptionOptions() { return encryptedBlobAsyncClient.getClientSideEncryptionOptions(); } - } diff --git a/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLength.java b/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLength.java new file mode 100644 index 0000000000000..7b392b81008b1 --- /dev/null +++ b/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLength.java @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.storage.blob.specialized.cryptography; + +import com.azure.core.util.logging.ClientLogger; + +import static com.azure.storage.blob.specialized.cryptography.CryptographyConstants.ENCRYPTION_PROTOCOL_V1; +import static com.azure.storage.blob.specialized.cryptography.CryptographyConstants.ENCRYPTION_PROTOCOL_V2; +import static com.azure.storage.blob.specialized.cryptography.CryptographyConstants.ENCRYPTION_PROTOCOL_V2_1; +import static com.azure.storage.blob.specialized.cryptography.CryptographyConstants.NONCE_LENGTH; +import static com.azure.storage.blob.specialized.cryptography.CryptographyConstants.TAG_LENGTH; + +/** + * This class provides helper methods for adjusting encrypted downloads. + */ +final class EncryptedBlobLength { + private static final ClientLogger LOGGER = new ClientLogger(EncryptedBlobLength.class); + + static Long computeAdjustedBlobLength(EncryptionData encryptionData, Long encryptedLength) { + switch (encryptionData.getEncryptionAgent().getProtocol()) { + /* + Technically, the total unencrypted length may be different for v1, + but because this helper method is only used for partitioning ranged downloads, + the size does not need to be adjusted for v1. + */ + case ENCRYPTION_PROTOCOL_V1: + return encryptedLength; + case ENCRYPTION_PROTOCOL_V2: + case ENCRYPTION_PROTOCOL_V2_1: + long regionLength = encryptionData.getEncryptedRegionInfo().getDataLength(); + long region = (long) Math.ceil((double) encryptedLength / (double) (regionLength + NONCE_LENGTH + TAG_LENGTH)); + long offset = (NONCE_LENGTH + TAG_LENGTH) * region; + return encryptedLength - offset; + default: + throw LOGGER.logExceptionAsError(new IllegalArgumentException("Unexpected protocol version")); + } + } +} diff --git a/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobRange.java b/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobRange.java index 25af8f95d3f95..aa16d37306346 100644 --- a/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobRange.java +++ b/sdk/storage/azure-storage-blob-cryptography/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobRange.java @@ -35,7 +35,7 @@ final class EncryptedBlobRange { * 0-31 for v1 * 0-(4mb+12-1) */ - private final int offsetAdjustment; + private final Long offsetAdjustment; /** * How many bytes to download, including the adjustments for encryption block boundaries and the IV. @@ -78,7 +78,7 @@ static EncryptedBlobRange getEncryptedBlobRangeFromHeader(String stringRange, En EncryptedBlobRange(BlobRange originalRange, EncryptionData encryptionData) { if (originalRange == null) { this.originalRange = new BlobRange(0); - this.offsetAdjustment = 0; + this.offsetAdjustment = 0L; this.amountPlaintextToSkip = 0; // In cases where this block is executed, this value does not matter return; } @@ -111,7 +111,7 @@ static EncryptedBlobRange getEncryptedBlobRangeFromHeader(String stringRange, En } } - this.offsetAdjustment = tempOffsetAdjustment; + this.offsetAdjustment = (long) tempOffsetAdjustment; /* Align adjustedDownloadCount with encryption block boundary at the end of the range. Note that it is impossible @@ -153,7 +153,7 @@ static EncryptedBlobRange getEncryptedBlobRangeFromHeader(String stringRange, En } // Offset adjustment is difference in two starting values - this.offsetAdjustment = (int) (originalRange.getOffset() - regionStartOffset); + this.offsetAdjustment = (originalRange.getOffset() - regionStartOffset); break; default: @@ -171,7 +171,7 @@ BlobRange getOriginalRange() { /** * @return Offset from beginning of BlobRange, 0-31. */ - int getOffsetAdjustment() { + Long getOffsetAdjustment() { return this.offsetAdjustment; } diff --git a/sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLengthTests.java b/sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLengthTests.java new file mode 100644 index 0000000000000..1dbbd8360a03f --- /dev/null +++ b/sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLengthTests.java @@ -0,0 +1,99 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.storage.blob.specialized.cryptography; + +import com.azure.storage.common.implementation.Constants; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.stream.Stream; + +import static com.azure.storage.blob.specialized.cryptography.CryptographyConstants.GCM_ENCRYPTION_REGION_LENGTH; +import static com.azure.storage.blob.specialized.cryptography.CryptographyConstants.NONCE_LENGTH; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class EncryptedBlobLengthTests extends BlobCryptographyTestBase { + + //to prevent having to cast int -> long everywhere + private static final long FOUR_MB = 4 * Constants.MB; + private static final long SIXTEEN_MB = 16 * Constants.MB; + private static final long ONE_MB = Constants.MB; + private static final long ONE_KB = Constants.KB; + + @ParameterizedTest + @MethodSource("correctAdjustedLengthV1Supplier") + public void correctAdjustedLengthV1(Long encryptedLength) { + EncryptionData encryptionData = new EncryptionData(); + encryptionData.setEncryptionAgent(new EncryptionAgent("1.0", null)); + + Long newLength = EncryptedBlobLength.computeAdjustedBlobLength(encryptionData, encryptedLength); + assertEquals(encryptedLength, newLength); + } + + private static Stream correctAdjustedLengthV1Supplier() { + return Stream.of( + Arguments.of(FOUR_MB), + Arguments.of(SIXTEEN_MB) + ); + } + + @ParameterizedTest + @MethodSource("correctAdjustedLengthV2Supplier") + public void correctAdjustedLengthV2(Long encryptedLength, Long expectedLength) { + EncryptionData encryptionData = new EncryptionData(); + encryptionData.setEncryptionAgent(new EncryptionAgent("2.0", null)); + encryptionData.setEncryptedRegionInfo(new EncryptedRegionInfo(GCM_ENCRYPTION_REGION_LENGTH, NONCE_LENGTH)); + + Long newLength = EncryptedBlobLength.computeAdjustedBlobLength(encryptionData, encryptedLength); + assertEquals(expectedLength, newLength); + } + + private static Stream correctAdjustedLengthV2Supplier() { + return Stream.of( + Arguments.of(FOUR_MB + 28, FOUR_MB), + Arguments.of(FOUR_MB + 57, FOUR_MB + 1), + Arguments.of(SIXTEEN_MB + 112, SIXTEEN_MB), + Arguments.of(28L, 0L), + Arguments.of(0L, 0L), + Arguments.of(ONE_MB + 28, ONE_MB) + ); + } + + @ParameterizedTest + @MethodSource("correctAdjustedLengthVariableRegionSupplier") + public void correctAdjustedLengthVariableRegion(Long encryptedLength, Long expectedLength, Long regionLength) { + EncryptionData encryptionData = new EncryptionData(); + encryptionData.setEncryptionAgent(new EncryptionAgent("2.1", null)); + encryptionData.setEncryptedRegionInfo(new EncryptedRegionInfo(regionLength, NONCE_LENGTH)); + + Long newLength = EncryptedBlobLength.computeAdjustedBlobLength(encryptionData, encryptedLength); + assertEquals(expectedLength, newLength); + } + + private static Stream correctAdjustedLengthVariableRegionSupplier() { + return Stream.of( + Arguments.of(FOUR_MB + 112, FOUR_MB, ONE_MB), + Arguments.of(SIXTEEN_MB + 448, SIXTEEN_MB, ONE_MB), + Arguments.of(FOUR_MB + 114688, FOUR_MB, ONE_KB), + Arguments.of(SIXTEEN_MB + 458752, SIXTEEN_MB, ONE_KB), + Arguments.of(FOUR_MB + 448, FOUR_MB, 256 * ONE_KB), + Arguments.of(SIXTEEN_MB + 1792, SIXTEEN_MB, 256 * ONE_KB), + Arguments.of(ONE_MB + 28672, ONE_MB, ONE_KB), + Arguments.of(ONE_MB + 28, ONE_MB, ONE_MB) + ); + } + + @Test + public void badProtocol() { + EncryptionData encryptionData = new EncryptionData(); + encryptionData.setEncryptionAgent(new EncryptionAgent("garbage", null)); + + assertThrows(IllegalArgumentException.class, () -> EncryptedBlobLength.computeAdjustedBlobLength(encryptionData, + null)); + } + +} diff --git a/sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobRangeTests.java b/sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobRangeTests.java index 8410cfae3ac68..1f933780a1f18 100644 --- a/sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobRangeTests.java +++ b/sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobRangeTests.java @@ -114,8 +114,7 @@ public void encryptedBlobRangeFromEncryptionData(int regionLength) { @ParameterizedTest @MethodSource("provideRanges") - public void testAdjustedBlobRange(int originalOffset, long originalCount, int expectedNewOffset, long expectedNewCount) { - int regionLength = 16; + public void testAdjustedBlobRange(long originalOffset, long originalCount, long expectedNewOffset, long expectedNewCount, int regionLength) { EncryptionData encryptionData = new EncryptionData() .setEncryptionAgent(new EncryptionAgent(ENCRYPTION_PROTOCOL_V2, EncryptionAlgorithm.AES_GCM_256)) .setEncryptedRegionInfo(new EncryptedRegionInfo(regionLength, NONCE_LENGTH)); @@ -131,10 +130,11 @@ public void testAdjustedBlobRange(int originalOffset, long originalCount, int ex private static Stream provideRanges() { return Stream.of( - Arguments.of(5, 10, 0, 44), // Entirely within a single region, adjustment includes nonce and tag - Arguments.of(16, 16, 44, 44), // Exactly one region - Arguments.of(15, 35, 0, 176), // Straddles across four regions - Arguments.of(32, 15, 88, 44) // Starts exactly at the second region + Arguments.of(5L, 10L, 0L, 44L, 16), // Entirely within a single region, adjustment includes nonce and tag + Arguments.of(16L, 16L, 44L, 44L, 16), // Exactly one region + Arguments.of(15L, 35L, 0L, 176L, 16), // Straddles across four regions + Arguments.of(32L, 15L, 88L, 44L, 16), // Starts exactly at the second region + Arguments.of(3674210304L, 10066367L, 8818104720L, 24159312L, 20) // Tests for integer overflow with a very large blob ); } } diff --git a/sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlockBlobApiTests.java b/sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlockBlobApiTests.java index 1f8d4ad310a81..32ce98e3ba5eb 100644 --- a/sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlockBlobApiTests.java +++ b/sdk/storage/azure-storage-blob-cryptography/src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlockBlobApiTests.java @@ -14,6 +14,7 @@ import com.azure.core.http.policy.HttpPipelinePolicy; import com.azure.core.http.rest.Response; import com.azure.core.test.TestMode; +import com.azure.core.test.utils.TestUtils; import com.azure.core.util.BinaryData; import com.azure.core.util.Context; import com.azure.core.util.CoreUtils; @@ -46,6 +47,7 @@ import com.azure.storage.blob.models.ParallelTransferOptions; import com.azure.storage.blob.options.BlobParallelUploadOptions; import com.azure.storage.blob.specialized.BlobClientBase; +import com.azure.storage.blob.specialized.BlobInputStream; import com.azure.storage.blob.specialized.BlockBlobClient; import com.azure.storage.common.implementation.Constants; import com.azure.storage.common.test.shared.extensions.LiveOnly; @@ -105,6 +107,7 @@ import static com.azure.storage.blob.specialized.cryptography.CryptographyConstants.GCM_ENCRYPTION_REGION_LENGTH; import static com.azure.storage.blob.specialized.cryptography.CryptographyConstants.NONCE_LENGTH; import static com.azure.storage.blob.specialized.cryptography.CryptographyConstants.TAG_LENGTH; +import static com.azure.storage.common.test.shared.StorageCommonTestUtils.convertInputStreamToByteArray; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -1106,6 +1109,45 @@ public void downloadFile(int fileSize, EncryptionVersion version) throws IOExcep Files.deleteIfExists(file.toPath()); } + @LiveOnly + @ParameterizedTest + @MethodSource("downloadFileSupplier") + public void downloadFileAsync(int fileSize, EncryptionVersion version) throws IOException { + File file = getRandomFile(fileSize); + File outFile = new File(testResourceNamer.randomName(prefix, 60) + ".txt"); + + beac = getEncryptionAsyncClient(version); + + StepVerifier.create(beac.uploadFromFile(file.toPath().toString(), true) + .then(beac.downloadToFileWithResponse(outFile.toPath().toString(), null, + new ParallelTransferOptions().setBlockSizeLong((long) (4 * 1024 * 1024)), null, + null, false))) + .assertNext(r -> assertEquals(BlobType.BLOCK_BLOB, r.getValue().getBlobType())) + .verifyComplete(); + + compareFiles(file, outFile, 0, fileSize); + + Files.deleteIfExists(outFile.toPath()); + Files.deleteIfExists(file.toPath()); + } + + @LiveOnly + @ParameterizedTest + @MethodSource("downloadFileSupplier") + public void downloadStream(int fileSize, EncryptionVersion version) throws IOException { + byte[] data = getRandomByteArray(fileSize); + File file = File.createTempFile(CoreUtils.randomUuid().toString(), ".txt"); + Files.write(file.toPath(), data); + + ebc = getEncryptionClient(version); + ebc.uploadFromFile(file.toPath().toString(), true); + + BlobInputStream stream = ebc.openInputStream(); + TestUtils.assertArraysEqual(convertInputStreamToByteArray(stream), data); + + Files.deleteIfExists(file.toPath()); + } + private static Stream downloadFileSupplier() { return Stream.of( Arguments.of(0, EncryptionVersion.V1), // empty file @@ -1115,10 +1157,13 @@ private static Stream downloadFileSupplier() { Arguments.of(50 * Constants.MB, EncryptionVersion.V1), // large file requiring multiple requests Arguments.of(0, EncryptionVersion.V2), // empty file Arguments.of(20, EncryptionVersion.V2), // small file - //Arguments.of(16 * 1024 * 1024, EncryptionVersion.V2), // medium file in several chunks - //todo isbr: uncomment after 4mb boundary bug is fixed + Arguments.of(16 * 1024 * 1024, EncryptionVersion.V2), // medium file in several chunks Arguments.of(8 * 1026 * 1024 + 10, EncryptionVersion.V2), // medium file not aligned to block - Arguments.of(50 * Constants.MB, EncryptionVersion.V2) // large file requiring multiple requests + Arguments.of(50 * Constants.MB, EncryptionVersion.V2), // large file requiring multiple requests + Arguments.of((4 * Constants.MB) + 1, EncryptionVersion.V2), // 4mb file bug + Arguments.of(4 * Constants.MB, EncryptionVersion.V2), // 4mb file bug + Arguments.of((4 * Constants.MB) + 27, EncryptionVersion.V2), // 4mb file bug + Arguments.of(16 * Constants.MB, EncryptionVersion.V2) // 4mb file bug ); } @@ -1773,6 +1818,31 @@ public void uploadAndDownloadToFileDifferentRegionLength(int regionLength, int f Files.deleteIfExists(file.toPath()); } + @ParameterizedTest + @MethodSource("uploadAndDownloadFileDifferentRegionLengthSupplier") + public void uploadAndDownloadToFileDifferentRegionLengthAsync(int regionLength, int fileSize) throws IOException { + File file = getRandomFile(fileSize); + beac = mockAesKey(getEncryptedClientBuilder(fakeKey, null, + ENV.getPrimaryAccount().getCredential(), cc.getBlobContainerUrl(), EncryptionVersion.V2_1) + .blobName(generateBlobName()) + .clientSideEncryptionOptions(new BlobClientSideEncryptionOptions() + .setAuthenticatedRegionDataLengthInBytes(regionLength)) + .buildEncryptedBlobAsyncClient()); + + File outFile = new File(testResourceNamer.randomName(prefix, 60) + ".txt"); + Files.deleteIfExists(outFile.toPath()); + + StepVerifier.create(beac.uploadFromFile(file.toPath().toString(), true) + .then(beac.downloadToFile(outFile.toPath().toString(), true))) + .expectNextCount(1) + .verifyComplete(); + + compareFiles(file, outFile, 0, file.length()); + + file.deleteOnExit(); + outFile.deleteOnExit(); + } + @ParameterizedTest @MethodSource("uploadAndDownloadDifferentRegionLengthSupplier") public void uploadAndDownloadRegionLengthWithDiffBlobClients(int regionLength, int dataSize) { @@ -1892,13 +1962,14 @@ private static Stream uploadAndDownloadDifferentRegionLengthSupplier( private static Stream uploadAndDownloadFileDifferentRegionLengthSupplier() { return Stream.of( -// Arguments.of(4 * Constants.KB, 4 * Constants.MB), // add these back once issue #41709 is resolved -// Arguments.of(Constants.KB, 8 * Constants.MB), // add these back once issue #41709 is resolved + Arguments.of(4 * Constants.KB, 4 * Constants.MB), // tests 4MB V2 download bug + Arguments.of(Constants.KB, 8 * Constants.MB), // tests 4MB V2 download bug Arguments.of(10 * Constants.KB, 4 * Constants.MB), // unaligned Arguments.of(16, Constants.KB), // minimum boundary Arguments.of(25, Constants.KB), // unaligned Arguments.of(6 * Constants.MB, Constants.KB), // testing region smaller than data size - Arguments.of(6 * Constants.MB, 8 * Constants.MB) // testing greater than default 4MB region size + Arguments.of(6 * Constants.MB, 8 * Constants.MB), // testing greater than default 4MB region size + Arguments.of(Constants.KB, 16 * Constants.MB) // 4mb download bug ); } diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/ChunkedDownloadUtils.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/ChunkedDownloadUtils.java index cf6f0add6b4c5..de6f3b6a50a0b 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/ChunkedDownloadUtils.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/ChunkedDownloadUtils.java @@ -4,6 +4,7 @@ package com.azure.storage.blob.implementation.util; import com.azure.core.http.HttpHeaderName; +import com.azure.core.util.Context; import com.azure.core.util.CoreUtils; import com.azure.storage.blob.models.BlobDownloadAsyncResponse; import com.azure.storage.blob.models.BlobErrorCode; @@ -11,11 +12,13 @@ import com.azure.storage.blob.models.BlobRequestConditions; import com.azure.storage.blob.models.BlobStorageException; import com.azure.storage.common.ParallelTransferOptions; +import com.azure.storage.common.implementation.Constants; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.util.function.Tuple3; import reactor.util.function.Tuples; +import java.util.Optional; import java.util.function.BiFunction; import java.util.function.Function; @@ -28,14 +31,24 @@ */ public class ChunkedDownloadUtils { + public static Mono> downloadFirstChunk( + BlobRange range, ParallelTransferOptions parallelTransferOptions, + BlobRequestConditions requestConditions, BiFunction> downloader, boolean eTagLock) { + return downloadFirstChunk(range, parallelTransferOptions, requestConditions, downloader, eTagLock, null); + } + /* + Has a context value for additional download adjustments. + Download the first chunk. Construct a Mono which will emit the total count for calculating the number of chunks, access conditions containing the etag to lock on, and the response from downloading the first chunk. */ + @SuppressWarnings("unchecked") public static Mono> downloadFirstChunk( BlobRange range, ParallelTransferOptions parallelTransferOptions, BlobRequestConditions requestConditions, BiFunction> downloader, boolean eTagLock) { + Mono> downloader, boolean eTagLock, Context context) { // We will scope our initial download to either be one chunk or the total size. long initialChunkSize = range.getCount() != null && range.getCount() < parallelTransferOptions.getBlockSizeLong() @@ -57,6 +70,12 @@ public static Mono contextAdjustment = context.getData(Constants.ADJUSTED_BLOB_LENGTH_KEY); + if (contextAdjustment.isPresent()) { + totalLength = (long) contextAdjustment.get(); + } + } /* If the user either didn't specify a count or they specified a count greater than the size of the remaining data, take the size of the remaining data. This is to prevent the case where the count diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobAsyncClientBase.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobAsyncClientBase.java index a9d489c13feb0..6bd382f5f621c 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobAsyncClientBase.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobAsyncClientBase.java @@ -1574,7 +1574,7 @@ private Mono> downloadToFileImpl(AsynchronousFileChanne rangeGetContentMd5, context); return ChunkedDownloadUtils.downloadFirstChunk(finalRange, finalParallelTransferOptions, requestConditions, - downloadFunc, true) + downloadFunc, true, context) .flatMap(setupTuple3 -> { long newCount = setupTuple3.getT1(); BlobRequestConditions finalConditions = setupTuple3.getT2(); diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobInputStream.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobInputStream.java index 22e6f31f19708..2c9bb3f684190 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobInputStream.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobInputStream.java @@ -8,6 +8,7 @@ import com.azure.storage.blob.models.BlobRequestConditions; import com.azure.storage.blob.models.BlobStorageException; import com.azure.storage.common.StorageInputStream; +import com.azure.storage.common.implementation.Constants; import java.io.IOException; import java.nio.ByteBuffer; @@ -54,7 +55,8 @@ public final class BlobInputStream extends StorageInputStream { BlobInputStream(BlobClientBase blobClient, long blobRangeOffset, Long blobRangeLength, int chunkSize, ByteBuffer initialBuffer, BlobRequestConditions accessCondition, BlobProperties blobProperties, Context context) throws BlobStorageException { - super(blobRangeOffset, blobRangeLength, chunkSize, blobProperties.getBlobSize(), initialBuffer); + + super(blobRangeOffset, blobRangeLength, chunkSize, adjustBlobLength(blobProperties.getBlobSize(), context), initialBuffer); this.blobClient = blobClient; this.accessCondition = accessCondition; @@ -97,4 +99,14 @@ public BlobProperties getProperties() { return this.properties; } + /** + * Allows for encrypted blobs to use BlobInputStream correctly by using the non-encrypted blob length + */ + private static long adjustBlobLength(long initialLength, Context context) { + if (context != null && context.getData(Constants.ADJUSTED_BLOB_LENGTH_KEY).isPresent()) { + return (long) context.getData(Constants.ADJUSTED_BLOB_LENGTH_KEY).get(); + } + return initialLength; + } + } diff --git a/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/Constants.java b/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/Constants.java index 936acae5cd94d..7ad9f93f3df30 100644 --- a/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/Constants.java +++ b/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/Constants.java @@ -91,6 +91,8 @@ public final class Constants { public static final String SAS_SERVICE_VERSION = Configuration.getGlobalConfiguration() .get(PROPERTY_AZURE_STORAGE_SAS_SERVICE_VERSION, "2024-11-04"); + public static final String ADJUSTED_BLOB_LENGTH_KEY = "adjustedBlobLength"; + private Constants() { }