V2 Encryption Bug 4MB file downloads #41813

ibrandes · 2024-09-11T19:20:14Z

Addresses #41709

…wnloads

azure-sdk · 2024-09-11T19:30:26Z

API change check

API changes are not detected in this pull request.

ibrandes · 2024-09-12T03:05:26Z

/azp run java - storage - tests

azure-pipelines · 2024-09-12T03:05:42Z

Azure Pipelines successfully started running 1 pipeline(s).

…to encryptionBug

kyleknap

It is looking good. I like how we are able to minimize the invasiveness of the change with the context approach. I just had some general questions and suggestions related to refactoring some of this.

.../src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobAsyncClient.java

...raphy/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobClient.java

...rage-blob/src/main/java/com/azure/storage/blob/implementation/util/ChunkedDownloadUtils.java

...rage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobClientBase.java

...rc/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlockBlobApiTests.java

...e-blob-changefeed/src/main/java/com/azure/storage/blob/changefeed/BlobChunkedDownloader.java

kyleknap

It's looking good. Just had some smaller comments.

.../src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobAsyncClient.java

...raphy/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobClient.java

...raphy/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLength.java

...rc/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlockBlobApiTests.java

...rage-blob/src/main/java/com/azure/storage/blob/implementation/util/ChunkedDownloadUtils.java

...raphy/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLength.java

…-for-java into encryptionBug

kyleknap

Looks good. I just had a couple more small questions/comments.

.../src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLengthTests.java

...raphy/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLength.java

kyleknap

Looks good! 🚢

ibrandes · 2024-10-03T23:52:52Z

/azp run java - storage - tests

azure-pipelines · 2024-10-03T23:53:08Z

Azure Pipelines successfully started running 1 pipeline(s).

…d support for unencrypted blob length for blobinputstream

kyleknap

Looks good. I just had a few more small comments. We should also probably add to our changelog that we fixed the same issue in the openInputStream API.

...graphy/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobRange.java

...rc/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlockBlobApiTests.java

...age/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobInputStream.java

…per method to blobinputstream

ibrandes · 2024-10-07T20:40:18Z

/azp run java - storage - tests

azure-pipelines · 2024-10-07T20:40:34Z

Azure Pipelines successfully started running 1 pipeline(s).

...age/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobInputStream.java

kyleknap

Looks good. Good catch on finding those other decryption issues 🚢

* 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

* 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 * update versions * update readmes * update changelogs --------- Co-authored-by: Isabelle <[email protected]>

ibrandes added 6 commits September 5, 2024 10:57

testing, need to get this branch on my other workstation

032a442

adding broken test

9277aac

updating tests

669d6c4

tests with logging

2c8b0b1

updating test and adding fix

1c91fba

making test paramaterized and adding change for only encryption v2 do…

fa17600

…wnloads

github-actions bot added the Storage Storage Service (Queues, Blobs, Files) label Sep 11, 2024

fixing imports

53c830a

ibrandes added 5 commits September 17, 2024 09:45

Merge branch 'main' of https://github.com/Azure/azure-sdk-for-java in…

797baad

…to encryptionBug

wip context addition

918aad8

context implementation and sync / async tests

68007ca

adding back unpulled tests

2479526

consolidating one of the async util methods

27bef9e

kyleknap reviewed Sep 24, 2024

View reviewed changes

ibrandes added 4 commits September 27, 2024 14:04

addressing all comments besides 1

58334be

removing leftover changes and adding back test for V2

f6c2f9c

kyle test

d3b005c

adjusting unencrypted blob length calculation

057c192

ibrandes requested a review from kyleknap October 2, 2024 18:33

recording tests and fixing style

0aeb4ee

ibrandes marked this pull request as ready for review October 2, 2024 20:36

ibrandes requested review from ibrahimrabab, seanmcc-msft and alzimmermsft as code owners October 2, 2024 20:36

kyleknap reviewed Oct 2, 2024

View reviewed changes

ibrandes added 2 commits October 3, 2024 12:00

addressing comments

611923e

changelog update

ac0e725

ibrandes and others added 3 commits October 3, 2024 12:10

Merge branch 'main' into encryptionBug

1a9dc2e

fixing messed up recorded test

d42eac2

Merge branch 'encryptionBug' of https://github.com/ibrandes/azure-sdk…

7ba3ae2

…-for-java into encryptionBug

kyleknap reviewed Oct 3, 2024

View reviewed changes

.../src/test/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLengthTests.java Show resolved Hide resolved

...raphy/src/main/java/com/azure/storage/blob/specialized/cryptography/EncryptedBlobLength.java Outdated Show resolved Hide resolved

adding more test cases and updating adjustment method signature

ba7c75b

kyleknap approved these changes Oct 3, 2024

View reviewed changes

adding missed recordings and fixing style

c151e22

changing offsetAdjustment to a long to prevent int overflow, and adde…

6c7d1d5

…d support for unencrypted blob length for blobinputstream

kyleknap reviewed Oct 7, 2024

View reviewed changes

ibrandes added 3 commits October 7, 2024 12:42

adding test for integer overflow, removing redundant test, adding hel…

a82ef2a

…per method to blobinputstream

adjusting changelong

2654857

removing unused imports

e915796

kyleknap reviewed Oct 7, 2024

View reviewed changes

...age/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobInputStream.java Outdated Show resolved Hide resolved

changing blobinputstream util method to use long instead of Long

198ff81

kyleknap approved these changes Oct 7, 2024

View reviewed changes

ibrandes merged commit 50e825d into Azure:main Oct 8, 2024
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

V2 Encryption Bug 4MB file downloads #41813

V2 Encryption Bug 4MB file downloads #41813

ibrandes commented Sep 11, 2024 •

edited

Loading

azure-sdk commented Sep 11, 2024

ibrandes commented Sep 12, 2024

azure-pipelines bot commented Sep 12, 2024

kyleknap left a comment

kyleknap left a comment

kyleknap left a comment

kyleknap left a comment

ibrandes commented Oct 3, 2024

azure-pipelines bot commented Oct 3, 2024

kyleknap left a comment

ibrandes commented Oct 7, 2024

azure-pipelines bot commented Oct 7, 2024

kyleknap left a comment

V2 Encryption Bug 4MB file downloads #41813

V2 Encryption Bug 4MB file downloads #41813

Conversation

ibrandes commented Sep 11, 2024 • edited Loading

azure-sdk commented Sep 11, 2024

ibrandes commented Sep 12, 2024

azure-pipelines bot commented Sep 12, 2024

kyleknap left a comment

Choose a reason for hiding this comment

kyleknap left a comment

Choose a reason for hiding this comment

kyleknap left a comment

Choose a reason for hiding this comment

kyleknap left a comment

Choose a reason for hiding this comment

ibrandes commented Oct 3, 2024

azure-pipelines bot commented Oct 3, 2024

kyleknap left a comment

Choose a reason for hiding this comment

ibrandes commented Oct 7, 2024

azure-pipelines bot commented Oct 7, 2024

kyleknap left a comment

Choose a reason for hiding this comment

ibrandes commented Sep 11, 2024 •

edited

Loading