From 8e6dd37832420122610b6c1e109afc9a0031caa7 Mon Sep 17 00:00:00 2001 From: JinmingHu Date: Fri, 2 Jul 2021 10:48:45 +0800 Subject: [PATCH] fix bug: transactional MD5 hash was treated as blob MD5 hash when downloading a range of blob (#2517) * fix bug: transactional MD5 hash was treated as blob MD5 hash when downloading a range of blob * CL * fix build error * add test case for 200 download * f --- sdk/storage/azure-storage-blobs/CHANGELOG.md | 3 + .../blobs/protocol/blob_rest_client.hpp | 11 ++- .../test/ut/block_blob_client_test.cpp | 94 +++++++++++++++---- .../azure-storage-files-datalake/CHANGELOG.md | 3 + 4 files changed, 87 insertions(+), 24 deletions(-) diff --git a/sdk/storage/azure-storage-blobs/CHANGELOG.md b/sdk/storage/azure-storage-blobs/CHANGELOG.md index a8543691d..03f7cf29a 100644 --- a/sdk/storage/azure-storage-blobs/CHANGELOG.md +++ b/sdk/storage/azure-storage-blobs/CHANGELOG.md @@ -2,6 +2,9 @@ ## 12.1.0-beta.1 (Unreleased) +### Bug Fixes + +- Fixed a bug where transactional MD5 hash was treated as blob MD5 hash when downloading partial blob. ## 12.0.0 (2021-06-08) diff --git a/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp b/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp index 9762a18b4..4beb40225 100644 --- a/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp +++ b/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp @@ -6424,11 +6424,14 @@ namespace Azure { namespace Storage { namespace Blobs { { response.Details.HttpHeaders.CacheControl = cache_control__iterator->second; } - auto content_md5__iterator = httpResponse.GetHeaders().find("content-md5"); - if (content_md5__iterator != httpResponse.GetHeaders().end()) + if (http_status_code == 200) { - response.Details.HttpHeaders.ContentHash.Value - = Azure::Core::Convert::Base64Decode(content_md5__iterator->second); + auto content_md5__iterator = httpResponse.GetHeaders().find("content-md5"); + if (content_md5__iterator != httpResponse.GetHeaders().end()) + { + response.Details.HttpHeaders.ContentHash.Value + = Azure::Core::Convert::Base64Decode(content_md5__iterator->second); + } } auto x_ms_blob_content_md5__iterator = httpResponse.GetHeaders().find("x-ms-blob-content-md5"); diff --git a/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp b/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp index 36f4ffbde..f03996c8f 100644 --- a/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp +++ b/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp @@ -110,30 +110,84 @@ namespace Azure { namespace Storage { namespace Test { TEST_F(BlockBlobClientTest, DownloadTransactionalHash) { - const int64_t downloadLength = 1024; - Blobs::DownloadBlobOptions options; - options.Range = Azure::Core::Http::HttpRange(); - options.Range.Value().Offset = 0; - options.Range.Value().Length = downloadLength; - options.RangeHashAlgorithm = HashAlgorithm::Md5; - auto res = m_blockBlobClient->Download(options); - ASSERT_TRUE(res.Value.TransactionalContentHash.HasValue()); - EXPECT_EQ(res.Value.TransactionalContentHash.Value().Algorithm, HashAlgorithm::Md5); + const std::vector dataPart1(static_cast(4_MB + 1), 'a'); + const std::vector dataPart2(static_cast(4_MB + 1), 'b'); + + const std::string blockId1 = Base64EncodeText("0"); + const std::string blockId2 = Base64EncodeText("1"); + + auto blobClient = m_blobContainerClient->GetBlockBlobClient(RandomString()); + auto blockContent = Azure::Core::IO::MemoryBodyStream(dataPart1.data(), dataPart1.size()); + blobClient.StageBlock(blockId1, blockContent); + blockContent = Azure::Core::IO::MemoryBodyStream(dataPart2.data(), dataPart2.size()); + blobClient.StageBlock(blockId2, blockContent); + blobClient.CommitBlockList({blockId1, blockId2}); + + std::vector blobMd5; { Azure::Core::Cryptography::Md5Hash instance; - EXPECT_EQ( - res.Value.TransactionalContentHash.Value().Value, - instance.Final(m_blobContent.data(), downloadLength)); + instance.Append(dataPart1.data(), dataPart1.size()); + blobMd5 = instance.Final(dataPart2.data(), dataPart2.size()); } - options.RangeHashAlgorithm = HashAlgorithm::Crc64; - res = m_blockBlobClient->Download(options); - ASSERT_TRUE(res.Value.TransactionalContentHash.HasValue()); - EXPECT_EQ(res.Value.TransactionalContentHash.Value().Algorithm, HashAlgorithm::Crc64); + + for (bool blobHasMd5 : {true, false}) { - Crc64Hash instance; - EXPECT_EQ( - res.Value.TransactionalContentHash.Value().Value, - instance.Final(m_blobContent.data(), downloadLength)); + if (blobHasMd5) + { + Blobs::Models::BlobHttpHeaders headers; + headers.ContentHash.Algorithm = HashAlgorithm::Md5; + headers.ContentHash.Value = blobMd5; + blobClient.SetHttpHeaders(headers); + ASSERT_FALSE(blobClient.GetProperties().Value.HttpHeaders.ContentHash.Value.empty()); + EXPECT_EQ(blobClient.Download().Value.Details.HttpHeaders.ContentHash.Value, blobMd5); + } + else + { + blobClient.SetHttpHeaders(Blobs::Models::BlobHttpHeaders()); + ASSERT_TRUE(blobClient.GetProperties().Value.HttpHeaders.ContentHash.Value.empty()); + ASSERT_TRUE(blobClient.Download().Value.Details.HttpHeaders.ContentHash.Value.empty()); + } + const int64_t downloadLength = 1; + Blobs::DownloadBlobOptions options; + options.Range = Azure::Core::Http::HttpRange(); + options.Range.Value().Offset = 0; + options.Range.Value().Length = downloadLength; + options.RangeHashAlgorithm = HashAlgorithm::Md5; + auto res = blobClient.Download(options); + if (blobHasMd5) + { + EXPECT_EQ(res.Value.Details.HttpHeaders.ContentHash.Value, blobMd5); + } + else + { + EXPECT_TRUE(res.Value.Details.HttpHeaders.ContentHash.Value.empty()); + } + ASSERT_TRUE(res.Value.TransactionalContentHash.HasValue()); + EXPECT_EQ(res.Value.TransactionalContentHash.Value().Algorithm, HashAlgorithm::Md5); + { + Azure::Core::Cryptography::Md5Hash instance; + EXPECT_EQ( + res.Value.TransactionalContentHash.Value().Value, + instance.Final(dataPart1.data(), downloadLength)); + } + options.RangeHashAlgorithm = HashAlgorithm::Crc64; + res = blobClient.Download(options); + if (blobHasMd5) + { + EXPECT_EQ(res.Value.Details.HttpHeaders.ContentHash.Value, blobMd5); + } + else + { + EXPECT_TRUE(res.Value.Details.HttpHeaders.ContentHash.Value.empty()); + } + ASSERT_TRUE(res.Value.TransactionalContentHash.HasValue()); + EXPECT_EQ(res.Value.TransactionalContentHash.Value().Algorithm, HashAlgorithm::Crc64); + { + Crc64Hash instance; + EXPECT_EQ( + res.Value.TransactionalContentHash.Value().Value, + instance.Final(dataPart1.data(), downloadLength)); + } } } diff --git a/sdk/storage/azure-storage-files-datalake/CHANGELOG.md b/sdk/storage/azure-storage-files-datalake/CHANGELOG.md index e4b181fe1..36f9b7667 100644 --- a/sdk/storage/azure-storage-files-datalake/CHANGELOG.md +++ b/sdk/storage/azure-storage-files-datalake/CHANGELOG.md @@ -2,6 +2,9 @@ ## 12.1.0-beta.1 (Unreleased) +### Bug Fixes + +- Fixed a bug where transactional MD5 hash was treated as blob MD5 hash when downloading partial blob. ## 12.0.0 (2021-06-08)