Skip to content

Commit

Permalink
CRC: Always Structured Message (Azure#44955)
Browse files Browse the repository at this point in the history 
* blockblob working

* revert testing change

* page/append

* datalake file

* testfix

* bug fixes | test fixes

* disable new API for presenting CRC from structured message

* fix nunit

* whitespace

* fix/test-proxy

* csproj

* more csproj removeals

This is building fine locally idk what's up

* Trigger Fresh Build

* fileshare testproxy

* fix mock

* Update macos image from 11 to latest (Azure#44607)

* Update macos image from 11 to latest

* Update eng/pipelines/templates/jobs/ci.mgmt.yml

Co-authored-by: Ben Broderick Phillips <ben@benbp.net>

---------

Co-authored-by: Ben Broderick Phillips <ben@benbp.net>

* Revert "Update macos image from 11 to latest (Azure#44607)"

this is causing too many problems. skipping macos tests for now. They'll
run when this feature branch merges into main.

This reverts commit 29e87b4.

---------

Co-authored-by: Wes Haggard <weshaggard@users.noreply.github.com>
Co-authored-by: Ben Broderick Phillips <ben@benbp.net>
  • Loading branch information
@jaschrep-msft @weshaggard @benbp
3 people committed Aug 12, 2024
1 parent b40939c commit 4786071
Show file tree
Hide file tree
Showing 36 changed files with 796 additions and 191 deletions.
3 changes: 2 additions & 1 deletion sdk/storage/Azure.Storage.Blobs.Batch/tests/Azure.Storage.Blobs.Batch.Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
<Compile Remove="$(AzureStorageSharedTestSources)\AzuriteNUnitFixture.cs" />
<Compile Remove="$(AzureStorageSharedTestSources)\TransferValidationTestBase.cs" />
<Compile Remove="$(AzureStorageSharedTestSources)\ClientSideEncryptionTestExtensions.cs" />
<Compile Remove="$(AzureStorageSharedTestSources)\ObserveStructuredMessagePolicy.cs" />
<None Include="$(AzureStorageSharedTestSources)\*.xml">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
Expand All @@ -42,4 +43,4 @@
<Compile Include="$(AzureStorageSharedSources)UriQueryParamsCollection.cs" LinkBase="Shared" />
<Compile Include="$(AzureStorageSharedSources)Argument.cs" LinkBase="Shared" />
</ItemGroup>
</Project>
</Project>
3 changes: 2 additions & 1 deletion sdk/storage/Azure.Storage.Blobs.ChangeFeed/tests/Azure.Storage.Blobs.ChangeFeed.Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<Compile Remove="$(AzureStorageSharedTestSources)\TransferValidationTestBase.cs" />
<Compile Remove="$(AzureStorageSharedTestSources)\ClientSideEncryptionTestExtensions.cs" />
<Compile Remove="$(AzureStorageSharedTestSources)\RepeatingStream.cs" />
<Compile Remove="$(AzureStorageSharedTestSources)\ObserveStructuredMessagePolicy.cs" />
<Compile Remove="$(AzureStorageSharedTestSources)\StorageTestBase.SasVersion.cs" />
<Compile Remove="$(AzureStorageSharedTestSources)\Sas\*.cs" />
<None Include="$(AzureStorageSharedTestSources)\*.xml">
Expand All @@ -28,4 +29,4 @@
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
</ItemGroup>
</Project>
</Project>
2 changes: 1 addition & 1 deletion sdk/storage/Azure.Storage.Blobs/assets.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"AssetsRepo": "Azure/azure-sdk-assets",
"AssetsRepoPrefixPath": "net",
"TagPrefix": "net/storage/Azure.Storage.Blobs",
"Tag": "net/storage/Azure.Storage.Blobs_dcc7be748a"
"Tag": "net/storage/Azure.Storage.Blobs_8b3f7ac2a4"
}
13 changes: 8 additions & 5 deletions sdk/storage/Azure.Storage.Blobs/src/AppendBlobClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1248,18 +1248,21 @@ internal async Task<Response<BlobAppendInfo>> AppendBlockInternal(
string structuredBodyType = null;
if (validationOptions != null &&
validationOptions.ChecksumAlgorithm.ResolveAuto() == StorageChecksumAlgorithm.StorageCrc64 &&
validationOptions.PrecalculatedChecksum.IsEmpty &&
ClientSideEncryption == null) // don't allow feature combination
{
// report progress in terms of caller bytes, not encoded bytes
structuredContentLength = contentLength;
contentLength = (content?.Length - content?.Position) ?? 0;
structuredBodyType = Constants.StructuredMessage.CrcStructuredMessage;
content = content.WithNoDispose().WithProgress(progressHandler);
content = new StructuredMessageEncodingStream(
content,
Constants.StructuredMessage.DefaultSegmentContentLength,
StructuredMessage.Flags.StorageCrc64);
content = validationOptions.PrecalculatedChecksum.IsEmpty
? new StructuredMessageEncodingStream(
content,
Constants.StructuredMessage.DefaultSegmentContentLength,
StructuredMessage.Flags.StorageCrc64)
: new StructuredMessagePrecalculatedCrcWrapperStream(
content,
validationOptions.PrecalculatedChecksum.Span);
contentLength = (content?.Length - content?.Position) ?? 0;
}
else
Expand Down
1 change: 1 addition & 0 deletions sdk/storage/Azure.Storage.Blobs/src/Azure.Storage.Blobs.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
<Compile Include="$(AzureStorageSharedSources)StructuredMessageDecodingRetriableStream.cs" LinkBase="Shared" />
<Compile Include="$(AzureStorageSharedSources)StructuredMessageDecodingStream.cs" LinkBase="Shared" />
<Compile Include="$(AzureStorageSharedSources)StructuredMessageEncodingStream.cs" LinkBase="Shared" />
<Compile Include="$(AzureStorageSharedSources)StructuredMessagePrecalculatedCrcWrapperStream.cs" LinkBase="Shared" />
<Compile Include="$(AzureStorageSharedSources)UriExtensions.cs" LinkBase="Shared" />
<Compile Include="$(AzureStorageSharedSources)UriQueryParamsCollection.cs" LinkBase="Shared" />
<Compile Include="$(AzureStorageSharedSources)UserDelegationKeyProperties.cs" LinkBase="Shared" />
Expand Down
10 changes: 2 additions & 8 deletions sdk/storage/Azure.Storage.Blobs/src/BlobBaseClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1574,6 +1574,7 @@ ValueTask<Response<BlobDownloadStreamingResult>> Factory(long offset, bool force
.EnsureCompleted(),
async startOffset => await StructuredMessageFactory(startOffset, async: true, cancellationToken)
.ConfigureAwait(false),
default, //decodedData => response.Value.Details.ContentCrc = decodedData.TotalCrc.ToArray(),
ClientConfiguration.Pipeline.ResponseClassifier,
Constants.MaxReliabilityRetries);
}
Expand Down Expand Up @@ -1723,14 +1724,7 @@ private async ValueTask<Response<BlobDownloadStreamingResult>> StartDownloadAsyn
rangeGetContentMD5 = true;
break;
case StorageChecksumAlgorithm.StorageCrc64:
if (!forceStructuredMessage && pageRange?.Length <= Constants.StructuredMessage.MaxDownloadCrcWithHeader)
{
rangeGetContentCRC64 = true;
}
else
{
structuredBodyType = Constants.StructuredMessage.CrcStructuredMessage;
}
structuredBodyType = Constants.StructuredMessage.CrcStructuredMessage;
break;
default:
break;
Expand Down
13 changes: 8 additions & 5 deletions sdk/storage/Azure.Storage.Blobs/src/BlockBlobClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1337,18 +1337,21 @@ internal virtual async Task<Response<BlockInfo>> StageBlockInternal(
string structuredBodyType = null;
if (validationOptions != null &&
validationOptions.ChecksumAlgorithm.ResolveAuto() == StorageChecksumAlgorithm.StorageCrc64 &&
validationOptions.PrecalculatedChecksum.IsEmpty &&
ClientSideEncryption == null) // don't allow feature combination
{
// report progress in terms of caller bytes, not encoded bytes
structuredContentLength = contentLength;
contentLength = (content?.Length - content?.Position) ?? 0;
structuredBodyType = Constants.StructuredMessage.CrcStructuredMessage;
content = content.WithNoDispose().WithProgress(progressHandler);
content = new StructuredMessageEncodingStream(
content,
Constants.StructuredMessage.DefaultSegmentContentLength,
StructuredMessage.Flags.StorageCrc64);
content = validationOptions.PrecalculatedChecksum.IsEmpty
? new StructuredMessageEncodingStream(
content,
Constants.StructuredMessage.DefaultSegmentContentLength,
StructuredMessage.Flags.StorageCrc64)
: new StructuredMessagePrecalculatedCrcWrapperStream(
content,
validationOptions.PrecalculatedChecksum.Span);
contentLength = (content?.Length - content?.Position) ?? 0;
}
else
Expand Down
9 changes: 9 additions & 0 deletions sdk/storage/Azure.Storage.Blobs/src/Models/BlobDownloadDetails.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ public class BlobDownloadDetails
public byte[] ContentHash { get; internal set; }
#pragma warning restore CA1819 // Properties should not return arrays

// TODO enable in following PR
///// <summary>
///// When requested using <see cref="DownloadTransferValidationOptions"/>, this value contains the CRC for the download blob range.
///// This value may only become populated once the network stream is fully consumed. If this instance is accessed through
///// <see cref="BlobDownloadResult"/>, the network stream has already been consumed. Otherwise, consume the content stream before
///// checking this value.
///// </summary>
//public byte[] ContentCrc { get; internal set; }

/// <summary>
/// Returns the date and time the container was last modified. Any operation that modifies the blob, including an update of the blob's metadata or properties, changes the last-modified time of the blob.
/// </summary>
Expand Down
11 changes: 11 additions & 0 deletions sdk/storage/Azure.Storage.Blobs/src/Models/BlobDownloadInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System;
using System.ComponentModel;
using System.IO;
using System.Threading.Tasks;
using Azure.Core;
using Azure.Storage.Shared;

namespace Azure.Storage.Blobs.Models
Expand Down Expand Up @@ -49,6 +51,15 @@ public class BlobDownloadInfo : IDisposable, IDownloadedContent
/// </summary>
public BlobDownloadDetails Details { get; internal set; }

// TODO enable in following PR
///// <summary>
///// Indicates some contents of <see cref="Details"/> are mixed into the response stream.
///// They will not be set until <see cref="Content"/> has been fully consumed. These details
///// will be extracted from the content stream by the library before the calling code can
///// encounter them.
///// </summary>
//public bool ExpectTrailingDetails { get; internal set; }

/// <summary>
/// Constructor.
/// </summary>
Expand Down
13 changes: 8 additions & 5 deletions sdk/storage/Azure.Storage.Blobs/src/PageBlobClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1370,7 +1370,6 @@ internal async Task<Response<PageInfo>> UploadPagesInternal(
HttpRange range;
if (validationOptions != null &&
validationOptions.ChecksumAlgorithm.ResolveAuto() == StorageChecksumAlgorithm.StorageCrc64 &&
validationOptions.PrecalculatedChecksum.IsEmpty &&
ClientSideEncryption == null) // don't allow feature combination
{
// report progress in terms of caller bytes, not encoded bytes
Expand All @@ -1379,10 +1378,14 @@ internal async Task<Response<PageInfo>> UploadPagesInternal(
range = new HttpRange(offset, (content?.Length - content?.Position) ?? null);
structuredBodyType = Constants.StructuredMessage.CrcStructuredMessage;
content = content?.WithNoDispose().WithProgress(progressHandler);
content = new StructuredMessageEncodingStream(
content,
Constants.StructuredMessage.DefaultSegmentContentLength,
StructuredMessage.Flags.StorageCrc64);
content = validationOptions.PrecalculatedChecksum.IsEmpty
? new StructuredMessageEncodingStream(
content,
Constants.StructuredMessage.DefaultSegmentContentLength,
StructuredMessage.Flags.StorageCrc64)
: new StructuredMessagePrecalculatedCrcWrapperStream(
content,
validationOptions.PrecalculatedChecksum.Span);
contentLength = (content?.Length - content?.Position) ?? 0;
}
else
Expand Down
50 changes: 26 additions & 24 deletions sdk/storage/Azure.Storage.Blobs/src/PartitionedDownloader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ internal class PartitionedDownloader
/// </summary>
private readonly StorageChecksumAlgorithm _validationAlgorithm;
private readonly int _checksumSize;
private bool UseMasterCrc => _validationAlgorithm.ResolveAuto() == StorageChecksumAlgorithm.StorageCrc64;
// TODO disabling master crc temporarily. segment CRCs still handled.
private bool UseMasterCrc => false; // _validationAlgorithm.ResolveAuto() == StorageChecksumAlgorithm.StorageCrc64;
private StorageCrc64HashAlgorithm _masterCrcCalculator = null;

/// <summary>
Expand Down Expand Up @@ -212,8 +213,20 @@ public async Task<Response> DownloadToInternal(

// If the first segment was the entire blob, we'll copy that to
// the output stream and finish now
long initialLength = initialResponse.Value.Details.ContentLength;
long totalLength = ParseRangeTotalLength(initialResponse.Value.Details.ContentRange);
long initialLength;
long totalLength;
// Get blob content length downloaded from content range when available to handle transit encoding
if (string.IsNullOrWhiteSpace(initialResponse.Value.Details.ContentRange))
{
initialLength = initialResponse.Value.Details.ContentLength;
totalLength = 0;
}
else
{
ContentRange recievedRange = ContentRange.Parse(initialResponse.Value.Details.ContentRange);
initialLength = recievedRange.End.Value - recievedRange.Start.Value + 1;
totalLength = recievedRange.Size.Value;
}
if (initialLength == totalLength)
{
await HandleOneShotDownload(initialResponse, destination, async, cancellationToken)
Expand Down Expand Up @@ -395,20 +408,6 @@ private async Task FinalizeDownloadInternal(
}
}

private static long ParseRangeTotalLength(string range)
{
if (range == null)
{
return 0;
}
int lengthSeparator = range.IndexOf("/", StringComparison.InvariantCultureIgnoreCase);
if (lengthSeparator == -1)
{
throw BlobErrors.ParsingFullHttpRangeFailed(range);
}
return long.Parse(range.Substring(lengthSeparator + 1), CultureInfo.InvariantCulture);
}

private async Task CopyToInternal(
Response<BlobDownloadStreamingResult> response,
Stream destination,
Expand All @@ -417,7 +416,10 @@ private async Task CopyToInternal(
CancellationToken cancellationToken)
{
CancellationHelper.ThrowIfCancellationRequested(cancellationToken);
using IHasher hasher = ContentHasher.GetHasherFromAlgorithmId(_validationAlgorithm);
// if structured message, this crc is validated in the decoding process. don't decode it here.
using IHasher hasher = response.GetRawResponse().Headers.Contains(Constants.StructuredMessage.CrcStructuredMessageHeader)
? null
: ContentHasher.GetHasherFromAlgorithmId(_validationAlgorithm);
using Stream rawSource = response.Value.Content;
using Stream source = hasher != null
? ChecksumCalculatingStream.GetReadStream(rawSource, hasher.AppendHash)
Expand All @@ -432,13 +434,13 @@ await source.CopyToInternal(
if (hasher != null)
{
hasher.GetFinalHash(checksumBuffer.Span);
(ReadOnlyMemory<byte> checksum, StorageChecksumAlgorithm _)
= ContentHasher.GetResponseChecksumOrDefault(response.GetRawResponse());
if (!checksumBuffer.Span.SequenceEqual(checksum.Span))
{
throw Errors.HashMismatchOnStreamedDownload(response.Value.Details.ContentRange);
(ReadOnlyMemory<byte> checksum, StorageChecksumAlgorithm _)
= ContentHasher.GetResponseChecksumOrDefault(response.GetRawResponse());
if (!checksumBuffer.Span.SequenceEqual(checksum.Span))
{
throw Errors.HashMismatchOnStreamedDownload(response.Value.Details.ContentRange);
}
}
}
}

private IEnumerable<HttpRange> GetRanges(long initialLength, long totalLength)
Expand Down
53 changes: 0 additions & 53 deletions sdk/storage/Azure.Storage.Blobs/tests/BlobBaseClientTransferValidationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,59 +95,6 @@ public override void TestAutoResolve()
}

#region Added Tests
[TestCaseSource("GetValidationAlgorithms")]
public async Task ExpectedDownloadStreamingStreamTypeReturned(StorageChecksumAlgorithm algorithm)
{
await using var test = await GetDisposingContainerAsync();

// Arrange
var data = GetRandomBuffer(Constants.KB);
BlobClient blob = InstrumentClient(test.Container.GetBlobClient(GetNewResourceName()));
using (var stream = new MemoryStream(data))
{
await blob.UploadAsync(stream);
}
// don't make options instance at all for no hash request
DownloadTransferValidationOptions transferValidation = algorithm == StorageChecksumAlgorithm.None
? default
: new DownloadTransferValidationOptions { ChecksumAlgorithm = algorithm };

// Act
Response<BlobDownloadStreamingResult> response = await blob.DownloadStreamingAsync(new BlobDownloadOptions
{
TransferValidation = transferValidation,
Range = new HttpRange(length: data.Length)
});

// Assert
// validated stream is buffered
Assert.AreEqual(typeof(MemoryStream), response.Value.Content.GetType());
}

[Test]
public async Task ExpectedDownloadStreamingStreamTypeReturned_None()
{
await using var test = await GetDisposingContainerAsync();

// Arrange
var data = GetRandomBuffer(Constants.KB);
BlobClient blob = InstrumentClient(test.Container.GetBlobClient(GetNewResourceName()));
using (var stream = new MemoryStream(data))
{
await blob.UploadAsync(stream);
}

// Act
Response<BlobDownloadStreamingResult> response = await blob.DownloadStreamingAsync(new BlobDownloadOptions
{
Range = new HttpRange(length: data.Length)
});

// Assert
// unvalidated stream type is private; just check we didn't get back a buffered stream
Assert.AreNotEqual(typeof(MemoryStream), response.Value.Content.GetType());
}

[Test]
public virtual async Task OlderServiceVersionThrowsOnStructuredMessage()
{
Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/Azure.Storage.Blobs/tests/PartitionedDownloaderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ public Response<BlobDownloadStreamingResult> GetStream(HttpRange range, BlobRequ
ContentHash = new byte[] { 1, 2, 3 },
LastModified = DateTimeOffset.Now,
Metadata = new Dictionary<string, string>() { { "meta", "data" } },
ContentRange = $"bytes {range.Offset}-{range.Offset + contentLength}/{_length}",
ContentRange = $"bytes {range.Offset}-{Math.Max(1, range.Offset + contentLength - 1)}/{_length}",
ETag = s_etag,
ContentEncoding = "test",
CacheControl = "test",
Expand Down
Loading

0 comments on commit 4786071

Please sign in to comment.