Crc reporting pt2 (Azure#45447)

* expose crc from structured message * testproxy * undo typo * exportapi
jaschrep-msft · Aug 13, 2024 · 5b2c153 · 5b2c153
1 parent b90be4c
commit 5b2c153
Show file tree

Hide file tree

Showing 19 changed files with 174 additions and 35 deletions.
diff --git a/sdk/storage/Azure.Storage.Blobs/api/Azure.Storage.Blobs.net6.0.cs b/sdk/storage/Azure.Storage.Blobs/api/Azure.Storage.Blobs.net6.0.cs
@@ -516,6 +516,7 @@ public BlobDownloadDetails() { }
         public long BlobSequenceNumber { get { throw null; } }
         public Azure.Storage.Blobs.Models.BlobType BlobType { get { throw null; } }
         public string CacheControl { get { throw null; } }
+        public byte[] ContentCrc { get { throw null; } }
         public string ContentDisposition { get { throw null; } }
         public string ContentEncoding { get { throw null; } }
         public byte[] ContentHash { get { throw null; } }

diff --git a/sdk/storage/Azure.Storage.Blobs/api/Azure.Storage.Blobs.netstandard2.0.cs b/sdk/storage/Azure.Storage.Blobs/api/Azure.Storage.Blobs.netstandard2.0.cs
@@ -516,6 +516,7 @@ public BlobDownloadDetails() { }
         public long BlobSequenceNumber { get { throw null; } }
         public Azure.Storage.Blobs.Models.BlobType BlobType { get { throw null; } }
         public string CacheControl { get { throw null; } }
+        public byte[] ContentCrc { get { throw null; } }
         public string ContentDisposition { get { throw null; } }
         public string ContentEncoding { get { throw null; } }
         public byte[] ContentHash { get { throw null; } }

diff --git a/sdk/storage/Azure.Storage.Blobs/api/Azure.Storage.Blobs.netstandard2.1.cs b/sdk/storage/Azure.Storage.Blobs/api/Azure.Storage.Blobs.netstandard2.1.cs
@@ -516,6 +516,7 @@ public BlobDownloadDetails() { }
         public long BlobSequenceNumber { get { throw null; } }
         public Azure.Storage.Blobs.Models.BlobType BlobType { get { throw null; } }
         public string CacheControl { get { throw null; } }
+        public byte[] ContentCrc { get { throw null; } }
         public string ContentDisposition { get { throw null; } }
         public string ContentEncoding { get { throw null; } }
         public byte[] ContentHash { get { throw null; } }

diff --git a/sdk/storage/Azure.Storage.Blobs/src/Azure.Storage.Blobs.csproj b/sdk/storage/Azure.Storage.Blobs/src/Azure.Storage.Blobs.csproj
@@ -52,6 +52,7 @@
     <Compile Include="$(AzureStorageSharedSources)BufferExtensions.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)ChecksumCalculatingStream.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)ContentRange.cs" LinkBase="Shared" />
+    <Compile Include="$(AzureStorageSharedSources)ChecksumExtensions.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)Constants.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)CompatSwitches.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)ContentHasher.cs" LinkBase="Shared" />

diff --git a/sdk/storage/Azure.Storage.Blobs/src/BlobBaseClient.cs b/sdk/storage/Azure.Storage.Blobs/src/BlobBaseClient.cs
@@ -1575,7 +1575,11 @@ 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(),
+                            decodedData =>
+                            {
+                                response.Value.Details.ContentCrc = new byte[StructuredMessage.Crc64Length];
+                                decodedData.Crc.WriteCrc64(response.Value.Details.ContentCrc);
+                            },
                             ClientConfiguration.Pipeline.ResponseClassifier,
                             Constants.MaxReliabilityRetries);
                     }

diff --git a/sdk/storage/Azure.Storage.Blobs/src/BlobClientOptions.cs b/sdk/storage/Azure.Storage.Blobs/src/BlobClientOptions.cs
@@ -318,6 +318,8 @@ private void AddHeadersAndQueryParameters()
             Diagnostics.LoggedHeaderNames.Add("x-ms-encryption-key-sha256");
             Diagnostics.LoggedHeaderNames.Add("x-ms-copy-source-error-code");
             Diagnostics.LoggedHeaderNames.Add("x-ms-copy-source-status-code");
+            Diagnostics.LoggedHeaderNames.Add("x-ms-structured-body");
+            Diagnostics.LoggedHeaderNames.Add("x-ms-structured-content-length");
 
             Diagnostics.LoggedQueryParameters.Add("comp");
             Diagnostics.LoggedQueryParameters.Add("maxresults");

diff --git a/sdk/storage/Azure.Storage.Blobs/src/Models/BlobDownloadDetails.cs b/sdk/storage/Azure.Storage.Blobs/src/Models/BlobDownloadDetails.cs
@@ -34,14 +34,13 @@ 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>
+        /// 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.

diff --git a/sdk/storage/Azure.Storage.Blobs/tests/BlobBaseClientTransferValidationTests.cs b/sdk/storage/Azure.Storage.Blobs/tests/BlobBaseClientTransferValidationTests.cs
@@ -124,6 +124,68 @@ public virtual async Task OlderServiceVersionThrowsOnStructuredMessage()
                 })).Value.Content.CopyToAsync(Stream.Null);
             Assert.That(operation, Throws.TypeOf<RequestFailedException>());
         }
+
+        [Test]
+        public async Task StructuredMessagePopulatesCrcDownloadStreaming()
+        {
+            await using DisposingContainer disposingContainer = await ClientBuilder.GetTestContainerAsync(
+                publicAccessType: PublicAccessType.None);
+
+            const int dataLength = Constants.KB;
+            byte[] data = GetRandomBuffer(dataLength);
+            byte[] dataCrc = new byte[8];
+            StorageCrc64Calculator.ComputeSlicedSafe(data, 0L).WriteCrc64(dataCrc);
+
+            var blob = disposingContainer.Container.GetBlobClient(GetNewResourceName());
+            await blob.UploadAsync(BinaryData.FromBytes(data));
+
+            Response<BlobDownloadStreamingResult> response = await blob.DownloadStreamingAsync(new()
+            {
+                TransferValidation = new DownloadTransferValidationOptions
+                {
+                    ChecksumAlgorithm = StorageChecksumAlgorithm.StorageCrc64
+                }
+            });
+
+            // crc is not present until response stream is consumed
+            Assert.That(response.Value.Details.ContentCrc, Is.Null);
+
+            byte[] downloadedData;
+            using (MemoryStream ms = new())
+            {
+                await response.Value.Content.CopyToAsync(ms);
+                downloadedData = ms.ToArray();
+            }
+
+            Assert.That(response.Value.Details.ContentCrc, Is.EqualTo(dataCrc));
+            Assert.That(downloadedData, Is.EqualTo(data));
+        }
+
+        [Test]
+        public async Task StructuredMessagePopulatesCrcDownloadContent()
+        {
+            await using DisposingContainer disposingContainer = await ClientBuilder.GetTestContainerAsync(
+                publicAccessType: PublicAccessType.None);
+
+            const int dataLength = Constants.KB;
+            byte[] data = GetRandomBuffer(dataLength);
+            byte[] dataCrc = new byte[8];
+            StorageCrc64Calculator.ComputeSlicedSafe(data, 0L).WriteCrc64(dataCrc);
+
+            var blob = disposingContainer.Container.GetBlobClient(GetNewResourceName());
+            await blob.UploadAsync(BinaryData.FromBytes(data));
+
+            Response<BlobDownloadResult> response = await blob.DownloadContentAsync(new BlobDownloadOptions()
+            {
+                TransferValidation = new DownloadTransferValidationOptions
+                {
+                    ChecksumAlgorithm = StorageChecksumAlgorithm.StorageCrc64
+                }
+            });
+
+            Assert.That(response.Value.Details.ContentCrc, Is.EqualTo(dataCrc));
+            Assert.That(response.Value.Content.ToArray(), Is.EqualTo(data));
+        }
         #endregion
     }
 }
diff --git a/sdk/storage/Azure.Storage.Common/src/Shared/ChecksumExtensions.cs b/sdk/storage/Azure.Storage.Common/src/Shared/ChecksumExtensions.cs
@@ -0,0 +1,22 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Buffers.Binary;
+
+namespace Azure.Storage;
+
+internal static class ChecksumExtensions
+{
+    public static void WriteCrc64(this ulong crc, Span<byte> dest)
+        => BinaryPrimitives.WriteUInt64LittleEndian(dest, crc);
+
+    public static bool TryWriteCrc64(this ulong crc, Span<byte> dest)
+        => BinaryPrimitives.TryWriteUInt64LittleEndian(dest, crc);
+
+    public static ulong ReadCrc64(this ReadOnlySpan<byte> crc)
+        => BinaryPrimitives.ReadUInt64LittleEndian(crc);
+
+    public static bool TryReadCrc64(this ReadOnlySpan<byte> crc, out ulong value)
+        => BinaryPrimitives.TryReadUInt64LittleEndian(crc, out value);
+}
diff --git a/sdk/storage/Azure.Storage.Common/src/Shared/StructuredMessage.cs b/sdk/storage/Azure.Storage.Common/src/Shared/StructuredMessage.cs
@@ -104,7 +104,7 @@ public static void ReadStreamFooter(
             int expectedBufferSize = GetSegmentFooterSize(flags);
             Errors.AssertBufferExactSize(buffer, expectedBufferSize, nameof(buffer));
 
-            crc64 = flags.HasFlag(Flags.StorageCrc64) ? BinaryPrimitives.ReadUInt64LittleEndian(buffer) : default;
+            crc64 = flags.HasFlag(Flags.StorageCrc64) ? buffer.ReadCrc64() : default;
         }
 
         public static int WriteStreamFooter(Span<byte> buffer, ReadOnlySpan<byte> crc64 = default)
@@ -200,7 +200,7 @@ public static void ReadSegmentFooter(
             int expectedBufferSize = GetSegmentFooterSize(flags);
             Errors.AssertBufferExactSize(buffer, expectedBufferSize, nameof(buffer));
 
-            crc64 = flags.HasFlag(Flags.StorageCrc64) ? BinaryPrimitives.ReadUInt64LittleEndian(buffer) : default;
+            crc64 = flags.HasFlag(Flags.StorageCrc64) ? buffer.ReadCrc64() : default;
         }
 
         public static int WriteSegmentFooter(Span<byte> buffer, ReadOnlySpan<byte> crc64 = default)

diff --git a/sdk/storage/Azure.Storage.Common/src/Shared/StructuredMessageDecodingStream.cs b/sdk/storage/Azure.Storage.Common/src/Shared/StructuredMessageDecodingStream.cs
@@ -431,20 +431,11 @@ private int ProcessStreamFooter(ReadOnlySpan<byte> span)
             out ulong reportedCrc);
         if (_decodedData.Flags.Value.HasFlag(StructuredMessage.Flags.StorageCrc64))
         {
-            _decodedData.TotalCrc = reportedCrc;
             if (_validateChecksums)
             {
-                using (ArrayPool<byte>.Shared.RentDisposable(StructuredMessage.Crc64Length * 2, out byte[] buf))
-                {
-                    Span<byte> calculated = new(buf, 0, StructuredMessage.Crc64Length);
-                    _totalContentCrc.GetCurrentHash(calculated);
-                    if (BinaryPrimitives.ReadUInt64LittleEndian(calculated) != reportedCrc)
-                    {
-                        Span<byte> reportedAsBytes = new(buf, calculated.Length, StructuredMessage.Crc64Length);
-                        throw Errors.ChecksumMismatch(calculated, reportedAsBytes);
-                    }
-                }
+                ValidateCrc64(_totalContentCrc, reportedCrc);
             }
+            _decodedData.TotalCrc = reportedCrc;
         }
 
         if (_innerStreamConsumed != _decodedData.InnerStreamLength)
@@ -487,23 +478,27 @@ private int ProcessSegmentFooter(ReadOnlySpan<byte> span)
         {
             if (_validateChecksums)
             {
-                using (ArrayPool<byte>.Shared.RentDisposable(StructuredMessage.Crc64Length * 2, out byte[] buf))
-                {
-                    Span<byte> calculated = new(buf, 0, StructuredMessage.Crc64Length);
-                    _segmentCrc.GetCurrentHash(calculated);
-                    _segmentCrc = StorageCrc64HashAlgorithm.Create();
-                    if (BinaryPrimitives.ReadUInt64LittleEndian(calculated) != reportedCrc)
-                    {
-                        Span<byte> reportedAsBytes = new(buf, calculated.Length, StructuredMessage.Crc64Length);
-                        throw Errors.ChecksumMismatch(calculated, reportedAsBytes);
-                    }
-                }
+                ValidateCrc64(_segmentCrc, reportedCrc);
+                _segmentCrc = StorageCrc64HashAlgorithm.Create();
             }
             _decodedData.SegmentCrcs.Add((reportedCrc, _currentSegmentContentLength));
         }
         _currentRegion = _currentSegmentNum == _decodedData.TotalSegments ? SMRegion.StreamFooter : SMRegion.SegmentHeader;
         return footerLen;
     }
+
+    private static void ValidateCrc64(StorageCrc64HashAlgorithm calculation, ulong reported)
+    {
+        using IDisposable _ = ArrayPool<byte>.Shared.RentDisposable(StructuredMessage.Crc64Length * 2, out byte[] buf);
+        Span<byte> calculatedBytes = new(buf, 0, StructuredMessage.Crc64Length);
+        Span<byte> reportedBytes = new(buf, calculatedBytes.Length, StructuredMessage.Crc64Length);
+        calculation.GetCurrentHash(calculatedBytes);
+        reported.WriteCrc64(reportedBytes);
+        if (!calculatedBytes.SequenceEqual(reportedBytes))
+        {
+            throw Errors.ChecksumMismatch(calculatedBytes, reportedBytes);
+        }
+    }
     #endregion
 
     public override long Seek(long offset, SeekOrigin origin)

diff --git a/sdk/storage/Azure.Storage.Common/tests/Azure.Storage.Common.Tests.csproj b/sdk/storage/Azure.Storage.Common/tests/Azure.Storage.Common.Tests.csproj
@@ -31,6 +31,7 @@
     <Compile Include="$(AzureStorageSharedSources)AggregatingProgressIncrementer.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)BufferExtensions.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)ChecksumCalculatingStream.cs" LinkBase="Shared" />
+    <Compile Include="$(AzureStorageSharedSources)ChecksumExtensions.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)ContentHasher.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)CompatSwitches.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)DisposableBucket.cs" LinkBase="Shared" />

diff --git a/sdk/storage/Azure.Storage.Files.DataLake/src/Azure.Storage.Files.DataLake.csproj b/sdk/storage/Azure.Storage.Files.DataLake/src/Azure.Storage.Files.DataLake.csproj
@@ -42,6 +42,7 @@
     <Compile Include="$(AzureStorageSharedSources)AggregatingProgressIncrementer.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)BufferExtensions.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)ChecksumCalculatingStream.cs" LinkBase="Shared" />
+    <Compile Include="$(AzureStorageSharedSources)ChecksumExtensions.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)Constants.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)CompatSwitches.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)ContentHasher.cs" LinkBase="Shared" />

diff --git a/sdk/storage/Azure.Storage.Files.Shares/api/Azure.Storage.Files.Shares.net6.0.cs b/sdk/storage/Azure.Storage.Files.Shares/api/Azure.Storage.Files.Shares.net6.0.cs
@@ -796,6 +796,7 @@ public partial class ShareFileDownloadInfo : System.IDisposable
     {
         internal ShareFileDownloadInfo() { }
         public System.IO.Stream Content { get { throw null; } }
+        public byte[] ContentCrc { get { throw null; } }
         public byte[] ContentHash { get { throw null; } }
         public long ContentLength { get { throw null; } }
         public string ContentType { get { throw null; } }

diff --git a/sdk/storage/Azure.Storage.Files.Shares/api/Azure.Storage.Files.Shares.netstandard2.0.cs b/sdk/storage/Azure.Storage.Files.Shares/api/Azure.Storage.Files.Shares.netstandard2.0.cs
@@ -796,6 +796,7 @@ public partial class ShareFileDownloadInfo : System.IDisposable
     {
         internal ShareFileDownloadInfo() { }
         public System.IO.Stream Content { get { throw null; } }
+        public byte[] ContentCrc { get { throw null; } }
         public byte[] ContentHash { get { throw null; } }
         public long ContentLength { get { throw null; } }
         public string ContentType { get { throw null; } }

diff --git a/sdk/storage/Azure.Storage.Files.Shares/src/Azure.Storage.Files.Shares.csproj b/sdk/storage/Azure.Storage.Files.Shares/src/Azure.Storage.Files.Shares.csproj
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <TargetFrameworks>$(RequiredTargetFrameworks);net6.0</TargetFrameworks>
   </PropertyGroup>
@@ -42,6 +42,7 @@
     <Compile Include="$(AzureStorageSharedSources)AggregatingProgressIncrementer.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)BufferExtensions.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)ChecksumCalculatingStream.cs" LinkBase="Shared" />
+    <Compile Include="$(AzureStorageSharedSources)ChecksumExtensions.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)Constants.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)CompatSwitches.cs" LinkBase="Shared" />
     <Compile Include="$(AzureStorageSharedSources)ContentHasher.cs" LinkBase="Shared" />

diff --git a/sdk/storage/Azure.Storage.Files.Shares/src/Models/ShareFileDownloadInfo.cs b/sdk/storage/Azure.Storage.Files.Shares/src/Models/ShareFileDownloadInfo.cs
@@ -38,6 +38,12 @@ public partial class ShareFileDownloadInfo : IDisposable, IDownloadedContent
         public byte[] ContentHash { get; internal set; }
 #pragma warning restore CA1819 // Properties should not return arrays
 
+        /// <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.
+        /// </summary>
+        public byte[] ContentCrc { get; internal set; }
+
         /// <summary>
         /// Details returned when downloading a file
         /// </summary>

diff --git a/sdk/storage/Azure.Storage.Files.Shares/src/ShareFileClient.cs b/sdk/storage/Azure.Storage.Files.Shares/src/ShareFileClient.cs
@@ -2421,7 +2421,11 @@ async ValueTask<Response<ShareFileDownloadInfo>> Factory(long offset, bool async
                                 .EnsureCompleted(),
                             async startOffset => await StructuredMessageFactory(startOffset, async: true, cancellationToken)
                                 .ConfigureAwait(false),
-                            default, //decodedData => response.Value.Details.ContentCrc = decodedData.TotalCrc.ToArray(),
+                            decodedData =>
+                            {
+                                initialResponse.Value.ContentCrc = new byte[StructuredMessage.Crc64Length];
+                                decodedData.Crc.WriteCrc64(initialResponse.Value.ContentCrc);
+                            },
                             ClientConfiguration.Pipeline.ResponseClassifier,
                             Constants.MaxReliabilityRetries);
                     }

diff --git a/sdk/storage/Azure.Storage.Files.Shares/tests/ShareFileClientTransferValidationTests.cs b/sdk/storage/Azure.Storage.Files.Shares/tests/ShareFileClientTransferValidationTests.cs
@@ -146,5 +146,41 @@ public override void TestAutoResolve()
                 StorageChecksumAlgorithm.MD5,
                 TransferValidationOptionsExtensions.ResolveAuto(StorageChecksumAlgorithm.Auto));
         }
+
+        [Test]
+        public async Task StructuredMessagePopulatesCrcDownloadStreaming()
+        {
+            await using DisposingShare disposingContainer = await ClientBuilder.GetTestShareAsync();
+
+            const int dataLength = Constants.KB;
+            byte[] data = GetRandomBuffer(dataLength);
+            byte[] dataCrc = new byte[8];
+            StorageCrc64Calculator.ComputeSlicedSafe(data, 0L).WriteCrc64(dataCrc);
+
+            ShareFileClient file = disposingContainer.Container.GetRootDirectoryClient().GetFileClient(GetNewResourceName());
+            await file.CreateAsync(data.Length);
+            await file.UploadAsync(new MemoryStream(data));
+
+            Response<ShareFileDownloadInfo> response = await file.DownloadAsync(new ShareFileDownloadOptions()
+            {
+                TransferValidation = new DownloadTransferValidationOptions
+                {
+                    ChecksumAlgorithm = StorageChecksumAlgorithm.StorageCrc64
+                }
+            });
+
+            // crc is not present until response stream is consumed
+            Assert.That(response.Value.ContentCrc, Is.Null);
+
+            byte[] downloadedData;
+            using (MemoryStream ms = new())
+            {
+                await response.Value.Content.CopyToAsync(ms);
+                downloadedData = ms.ToArray();
+            }
+
+            Assert.That(response.Value.ContentCrc, Is.EqualTo(dataCrc));
+            Assert.That(downloadedData, Is.EqualTo(data));
+        }
     }
 }