From 613e15202da4c357f9cd12ae241d13a65232cbbe Mon Sep 17 00:00:00 2001 From: mahendra-google Date: Thu, 15 Jan 2026 23:15:04 -0800 Subject: [PATCH 1/9] feat(Storage): Enable full object checksum validation for resumable uploads --- .../UploadObjectTest.cs | 65 +------- .../UploadObjectOptionsTest.cs | 8 +- .../CustomMediaUpload.cs | 157 +++++++++++++++++- .../StorageClientImpl.UploadObject.cs | 58 +++++-- 4 files changed, 208 insertions(+), 80 deletions(-) diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.IntegrationTests/UploadObjectTest.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.IntegrationTests/UploadObjectTest.cs index b1cb95bb2fba..a419799a78ea 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.IntegrationTests/UploadObjectTest.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.IntegrationTests/UploadObjectTest.cs @@ -308,9 +308,8 @@ public void UploadObject_InvalidHash_ThrowOnly() var name = IdGenerator.FromGuid(); var bucket = _fixture.MultiVersionBucket; var options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.ThrowOnly }; - Assert.Throws(() => client.UploadObject(bucket, name, null, stream, options)); - // We don't delete the object, so it's still present. - ValidateData(bucket, name, new MemoryStream(interceptor.UploadedBytes)); + var exception = Assert.Throws(() => client.UploadObject(bucket, name, null, stream, options)); + Assert.Equal(HttpStatusCode.BadRequest, exception.HttpStatusCode); } [Fact] @@ -323,28 +322,12 @@ public void UploadObject_InvalidHash_DeleteAndThrow() var name = IdGenerator.FromGuid(); var bucket = _fixture.MultiVersionBucket; var options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.DeleteAndThrow }; - Assert.Throws(() => client.UploadObject(bucket, name, null, stream, options)); + var exception = Assert.Throws(() => client.UploadObject(bucket, name, null, stream, options)); + Assert.Equal(HttpStatusCode.BadRequest, exception.HttpStatusCode); var notFound = Assert.Throws(() => _fixture.Client.GetObject(bucket, name)); Assert.Equal(HttpStatusCode.NotFound, notFound.HttpStatusCode); } - [Fact] - public void UploadObject_InvalidHash_DeleteAndThrow_DeleteFails() - { - var client = StorageClient.Create(); - var interceptor = new BreakUploadInterceptor(); - client.Service.HttpClient.MessageHandler.AddExecuteInterceptor(interceptor); - client.Service.HttpClient.MessageHandler.AddExecuteInterceptor(new BreakDeleteInterceptor()); - var stream = GenerateData(50); - var name = IdGenerator.FromGuid(); - var bucket = _fixture.MultiVersionBucket; - var options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.DeleteAndThrow }; - var ex = Assert.Throws(() => client.UploadObject(bucket, name, null, stream, options)); - Assert.NotNull(ex.AdditionalFailures); - // The deletion failed, so the uploaded object still exists. - ValidateData(bucket, name, new MemoryStream(interceptor.UploadedBytes)); - } - [Fact] public async Task UploadObjectAsync_InvalidHash_None() { @@ -371,9 +354,8 @@ public async Task UploadObjectAsync_InvalidHash_ThrowOnly() var name = IdGenerator.FromGuid(); var bucket = _fixture.MultiVersionBucket; var options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.ThrowOnly }; - await Assert.ThrowsAsync(() => client.UploadObjectAsync(bucket, name, null, stream, options)); - // We don't delete the object, so it's still present. - ValidateData(bucket, name, new MemoryStream(interceptor.UploadedBytes)); + var exception = await Assert.ThrowsAsync(() => client.UploadObjectAsync(bucket, name, null, stream, options)); + Assert.Equal(HttpStatusCode.BadRequest, exception.HttpStatusCode); } [Fact] @@ -387,28 +369,12 @@ public async Task UploadObjectAsync_InvalidHash_DeleteAndThrow() var name = IdGenerator.FromGuid(); var bucket = _fixture.MultiVersionBucket; var options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.DeleteAndThrow }; - await Assert.ThrowsAsync(() => client.UploadObjectAsync(bucket, name, null, stream, options)); + var exception = await Assert.ThrowsAsync(() => client.UploadObjectAsync(bucket, name, null, stream, options)); + Assert.Equal(HttpStatusCode.BadRequest, exception.HttpStatusCode); var notFound = await Assert.ThrowsAsync(() => _fixture.Client.GetObjectAsync(bucket, name)); Assert.Equal(HttpStatusCode.NotFound, notFound.HttpStatusCode); } - [Fact] - public async Task UploadObjectAsync_InvalidHash_DeleteAndThrow_DeleteFails() - { - var client = StorageClient.Create(); - var interceptor = new BreakUploadInterceptor(); - client.Service.HttpClient.MessageHandler.AddExecuteInterceptor(interceptor); - client.Service.HttpClient.MessageHandler.AddExecuteInterceptor(new BreakDeleteInterceptor()); - var stream = GenerateData(50); - var name = IdGenerator.FromGuid(); - var bucket = _fixture.MultiVersionBucket; - var options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.DeleteAndThrow }; - var ex = await Assert.ThrowsAsync(() => client.UploadObjectAsync(bucket, name, null, stream, options)); - Assert.NotNull(ex.AdditionalFailures); - // The deletion failed, so the uploaded object still exists. - ValidateData(bucket, name, new MemoryStream(interceptor.UploadedBytes)); - } - [Fact] public async Task InitiateUploadSessionAsync_NegativeLength() { @@ -488,21 +454,6 @@ public async Task InterceptAsync(HttpRequestMessage request, CancellationToken c } } - private class BreakDeleteInterceptor : IHttpExecuteInterceptor - { - public Task InterceptAsync(HttpRequestMessage request, CancellationToken cancellationToken) - { - // We only care about Delete requests - if (request.Method == HttpMethod.Delete) - { - // Ugly but effective hack: replace the generation URL parameter so that we add a leading 9, - // so the generation we try to delete is the wrong one. - request.RequestUri = new Uri(request.RequestUri.ToString().Replace("generation=", "generation=9")); - } - return Task.FromResult(0); - } - } - private Object GetExistingObject() { var obj = _fixture.Client.UploadObject(_fixture.MultiVersionBucket, IdGenerator.FromGuid(), "application/octet-stream", GenerateData(100)); diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.Tests/UploadObjectOptionsTest.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.Tests/UploadObjectOptionsTest.cs index d0ef1b0d9f93..1838a0aebca1 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.Tests/UploadObjectOptionsTest.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.Tests/UploadObjectOptionsTest.cs @@ -54,7 +54,7 @@ public void InvalidChunkSize(int chunkSize) [Fact] public void ModifyMediaUpload_DefaultOptions() { - var upload = new CustomMediaUpload(new DummyService(), null, "bucket", new MemoryStream(), null); + var upload = new CustomMediaUpload(new DummyService(), null, "bucket", new MemoryStream(), null, null); var options = new UploadObjectOptions(); options.ModifyMediaUpload(upload); Assert.Equal(ResumableUpload.DefaultChunkSize, upload.ChunkSize); @@ -71,7 +71,7 @@ public void ModifyMediaUpload_DefaultOptions() [Fact] public void ModifyMediaUpload_AllOptions_PositiveMatch() { - var upload = new CustomMediaUpload(new DummyService(), null, "bucket", new MemoryStream(), null); + var upload = new CustomMediaUpload(new DummyService(), null, "bucket", new MemoryStream(), null, null); var options = new UploadObjectOptions { ChunkSize = UploadObjectOptions.MinimumChunkSize * 3, @@ -96,7 +96,7 @@ public void ModifyMediaUpload_AllOptions_PositiveMatch() [Fact] public void ModifyMediaUpload_AllOptions_NegativeMatch() { - var upload = new CustomMediaUpload(new DummyService(), null, "bucket", new MemoryStream(), null); + var upload = new CustomMediaUpload(new DummyService(), null, "bucket", new MemoryStream(), null, null); var options = new UploadObjectOptions { ChunkSize = UploadObjectOptions.MinimumChunkSize * 3, @@ -117,7 +117,7 @@ public void ModifyMediaUpload_AllOptions_NegativeMatch() [Fact] public void ModifyMediaUpload_MatchNotMatchConflicts() { - var upload = new CustomMediaUpload(new DummyService(), null, "bucket", new MemoryStream(), null); + var upload = new CustomMediaUpload(new DummyService(), null, "bucket", new MemoryStream(), null, null); Assert.Throws(() => { var options = new UploadObjectOptions { IfGenerationMatch = 1L, IfGenerationNotMatch = 2L }; diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs index c91a50ef9265..f497bc497ea9 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs @@ -1,4 +1,4 @@ -// Copyright 2017 Google Inc. All Rights Reserved. +// Copyright 2017 Google Inc. All Rights Reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -12,12 +12,16 @@ // See the License for the specific language governing permissions and // limitations under the License. +using Google.Apis.Http; using Google.Apis.Services; +using Google.Apis.Upload; using System; using System.IO; +using System.Linq; using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; using static Google.Apis.Storage.v1.ObjectsResource; -using Google.Apis.Upload; namespace Google.Cloud.Storage.V1 { @@ -26,12 +30,157 @@ namespace Google.Cloud.Storage.V1 /// internal sealed class CustomMediaUpload : InsertMediaUpload { + private readonly Stream _stream; + private readonly Crc32cHashInterceptor _interceptor; + private readonly IClientService _service; + private readonly HashingStream _hashingStream; + public CustomMediaUpload(IClientService service, Apis.Storage.v1.Data.Object body, string bucket, - Stream stream, string contentType) - : base(service, body, bucket, stream, contentType) + Stream stream, string contentType, UploadObjectOptions options) + : base(service, body, bucket, new HashingStream(stream), contentType) { + _stream = stream; + _service = service; + var validationMode = options?.UploadValidationMode ?? UploadObjectOptions.DefaultValidationMode; + _hashingStream = (HashingStream) ContentStream; + _interceptor = new Crc32cHashInterceptor(this, _hashingStream, _service, validationMode); + _service?.HttpClient?.MessageHandler?.AddExecuteInterceptor(_interceptor); } internal new ResumableUploadOptions Options => base.Options; + + private sealed class Crc32cHashInterceptor : IHttpExecuteInterceptor + { + private const string GoogleHashHeader = "x-goog-hash"; + private const int ReadBufferSize = 81920; + private readonly IClientService _service; + private readonly CustomMediaUpload _owner; + private Uri _uploadUri; + private readonly UploadValidationMode _validationMode; + private readonly HashingStream _hashingStream; + + public Crc32cHashInterceptor(CustomMediaUpload owner, HashingStream hashingStream, IClientService service, UploadValidationMode validationMode) + { + _hashingStream = hashingStream; + _service = service; + _owner = owner; + _validationMode = validationMode; + _owner.UploadSessionData += OnSessionData; + _owner.ProgressChanged += OnProgressChanged; + } + + public Task InterceptAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + if (_uploadUri != null && !_uploadUri.Equals(request.RequestUri)) + { + return Task.CompletedTask; + } + + if (request.Method == System.Net.Http.HttpMethod.Put && request.Content?.Headers.Contains("Content-Range") is true) + { + var rangeHeader = request.Content.Headers.GetValues("Content-Range").First(); + + if (IsFinalChunk(rangeHeader)) + { + if (_validationMode != UploadValidationMode.None) + { + var calculatedHash = _hashingStream.GetBase64Hash(); + request.Headers.TryAddWithoutValidation(GoogleHashHeader, $"crc32c={calculatedHash}"); + } + } + } + return Task.CompletedTask; + } + + private void OnSessionData(IUploadSessionData data) + { + _uploadUri = data.UploadUri; + _owner.UploadSessionData -= OnSessionData; + } + + private void OnProgressChanged(IUploadProgress progress) + { + if (progress.Status == UploadStatus.Completed || progress.Status == UploadStatus.Failed) + { + // Clean up when upload is finished. + _service?.HttpClient?.MessageHandler?.RemoveExecuteInterceptor(this); + _owner.ProgressChanged -= OnProgressChanged; + } + } + + private bool IsFinalChunk(string rangeHeader) + { + // Expected format: "bytes {start}-{end}/{total}" or "bytes */{total}" for the final request. + // We are interested in the final chunk of a known-size upload. + const string prefix = "bytes "; + if (!rangeHeader.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + ReadOnlySpan span = rangeHeader.AsSpan(prefix.Length); + int slashIndex = span.IndexOf('/'); + if (slashIndex == -1) + { + return false; + } + + var totalSpan = span.Slice(slashIndex + 1); + if (totalSpan.IsEmpty || totalSpan[0] == '*') + { + return false; + } + + if (!long.TryParse(totalSpan.ToString(), System.Globalization.NumberStyles.Integer, System.Globalization.CultureInfo.InvariantCulture, out long totalSize)) + { + return false; + } + + var rangeSpan = span.Slice(0, slashIndex); + int dashIndex = rangeSpan.IndexOf('-'); + if (dashIndex == -1) + { + return false; + } + + var endByteSpan = rangeSpan.Slice(dashIndex + 1); + if (!long.TryParse(endByteSpan.ToString(), System.Globalization.NumberStyles.Integer, System.Globalization.CultureInfo.InvariantCulture, out long endByte)) + { + return false; + } + + // If endByte is the last byte of the file, it's the final chunk. + return (endByte + 1) == totalSize; + } + } + + private sealed class HashingStream : Stream + { + private readonly Stream _inner; + private readonly Crc32c _hasher = new Crc32c(); + + public HashingStream(Stream inner) => _inner = inner; + + public override int Read(byte[] buffer, int offset, int count) + { + int bytesRead = _inner.Read(buffer, offset, count); + if (bytesRead > 0) + { + _hasher.UpdateHash(buffer, offset, bytesRead); + } + return bytesRead; + } + + public string GetBase64Hash() => Convert.ToBase64String(_hasher.GetHash()); + public override bool CanRead => _inner.CanRead; + public override bool CanSeek => _inner.CanSeek; + public override bool CanWrite => _inner.CanWrite; + public override long Length => _inner.Length; + public override long Position { get => _inner.Position; set => _inner.Position = value; } + public override void Flush() => _inner.Flush(); + public override long Seek(long offset, SeekOrigin origin) => _inner.Seek(offset, origin); + public override void SetLength(long value) => _inner.SetLength(value); + public override void Write(byte[] buffer, int offset, int count) => _inner.Write(buffer, offset, count); + } } } diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs index 84e7f457edb6..5b45304839c6 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs @@ -48,7 +48,7 @@ public override ObjectsResource.InsertMediaUpload CreateObjectUploader( { ValidateObject(destination, nameof(destination)); GaxPreconditions.CheckNotNull(source, nameof(source)); - var mediaUpload = new CustomMediaUpload(Service, destination, destination.Bucket, source, destination.ContentType); + var mediaUpload = new CustomMediaUpload(Service, destination, destination.Bucket, source, destination.ContentType, options); options?.ModifyMediaUpload(mediaUpload); ApplyEncryptionKey(options?.EncryptionKey, options?.KmsKeyName, mediaUpload); return mediaUpload; @@ -229,29 +229,57 @@ internal async Task ExecuteAsync(CancellationToken cancellationToken) private sealed class LengthOnlyStream : Stream { private readonly long? _length; + private long _position; internal LengthOnlyStream(long? length) => _length = length; public override long Length => _length ?? throw new NotSupportedException(); public override bool CanSeek => _length.HasValue; - - public override bool CanRead => throw new NotImplementedException(); - public override bool CanWrite => throw new NotImplementedException(); + public override bool CanRead => true; + public override bool CanWrite => false; public override long Position { - get => throw new NotImplementedException(); - set => throw new NotImplementedException(); + get => _position; + set + { + if (!CanSeek) throw new NotSupportedException(); + if (value < 0 || value > Length) throw new ArgumentOutOfRangeException(nameof(value)); + _position = value; + } + } + + public override int Read(byte[] buffer, int offset, int count) + { + if (!_length.HasValue) + { + return 0; + } + long remaining = _length.Value - _position; + if (remaining <= 0) return 0; + + int toRead = (int) Math.Min(count, remaining); + + Array.Clear(buffer, offset, toRead); + + _position += toRead; + return toRead; + } + + public override void Flush() { } + + public override long Seek(long offset, SeekOrigin origin) + { + switch (origin) + { + case SeekOrigin.Begin: Position = offset; break; + case SeekOrigin.Current: Position += offset; break; + case SeekOrigin.End: Position = Length + offset; break; + } + return Position; } - public override void Flush() => throw new NotImplementedException(); - public override int Read(byte[] buffer, int offset, int count) => - throw new NotImplementedException(); - public override long Seek(long offset, SeekOrigin origin) => - throw new NotImplementedException(); - public override void SetLength(long value) => - throw new NotImplementedException(); - public override void Write(byte[] buffer, int offset, int count) => - throw new NotImplementedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); } } } From 827f17b3d8d1586be61a8db594a2a807dc1d0643 Mon Sep 17 00:00:00 2001 From: Mahendra Date: Tue, 31 Mar 2026 15:19:28 +0530 Subject: [PATCH 2/9] refactor(Storage): Remove unused stream field --- .../CustomMediaUpload.cs | 45 +++++++++---------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs index f497bc497ea9..0561b37d98cb 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs @@ -30,7 +30,6 @@ namespace Google.Cloud.Storage.V1 /// internal sealed class CustomMediaUpload : InsertMediaUpload { - private readonly Stream _stream; private readonly Crc32cHashInterceptor _interceptor; private readonly IClientService _service; private readonly HashingStream _hashingStream; @@ -39,7 +38,6 @@ public CustomMediaUpload(IClientService service, Apis.Storage.v1.Data.Object bod Stream stream, string contentType, UploadObjectOptions options) : base(service, body, bucket, new HashingStream(stream), contentType) { - _stream = stream; _service = service; var validationMode = options?.UploadValidationMode ?? UploadObjectOptions.DefaultValidationMode; _hashingStream = (HashingStream) ContentStream; @@ -52,21 +50,20 @@ public CustomMediaUpload(IClientService service, Apis.Storage.v1.Data.Object bod private sealed class Crc32cHashInterceptor : IHttpExecuteInterceptor { private const string GoogleHashHeader = "x-goog-hash"; - private const int ReadBufferSize = 81920; private readonly IClientService _service; - private readonly CustomMediaUpload _owner; + private readonly CustomMediaUpload _mediaUpload; private Uri _uploadUri; private readonly UploadValidationMode _validationMode; private readonly HashingStream _hashingStream; - public Crc32cHashInterceptor(CustomMediaUpload owner, HashingStream hashingStream, IClientService service, UploadValidationMode validationMode) + public Crc32cHashInterceptor(CustomMediaUpload mediaUpload, HashingStream hashingStream, IClientService service, UploadValidationMode validationMode) { _hashingStream = hashingStream; _service = service; - _owner = owner; + _mediaUpload = mediaUpload; _validationMode = validationMode; - _owner.UploadSessionData += OnSessionData; - _owner.ProgressChanged += OnProgressChanged; + _mediaUpload.UploadSessionData += OnSessionData; + _mediaUpload.ProgressChanged += OnProgressChanged; } public Task InterceptAsync(HttpRequestMessage request, CancellationToken cancellationToken) @@ -95,16 +92,16 @@ public Task InterceptAsync(HttpRequestMessage request, CancellationToken cancell private void OnSessionData(IUploadSessionData data) { _uploadUri = data.UploadUri; - _owner.UploadSessionData -= OnSessionData; + _mediaUpload.UploadSessionData -= OnSessionData; } private void OnProgressChanged(IUploadProgress progress) { - if (progress.Status == UploadStatus.Completed || progress.Status == UploadStatus.Failed) + if (progress.Status is UploadStatus.Completed or UploadStatus.Failed) { // Clean up when upload is finished. _service?.HttpClient?.MessageHandler?.RemoveExecuteInterceptor(this); - _owner.ProgressChanged -= OnProgressChanged; + _mediaUpload.ProgressChanged -= OnProgressChanged; } } @@ -156,14 +153,14 @@ private bool IsFinalChunk(string rangeHeader) private sealed class HashingStream : Stream { - private readonly Stream _inner; - private readonly Crc32c _hasher = new Crc32c(); + private readonly Stream _stream; + private readonly Crc32c _hasher = new(); - public HashingStream(Stream inner) => _inner = inner; + public HashingStream(Stream stream) => _stream = stream; public override int Read(byte[] buffer, int offset, int count) { - int bytesRead = _inner.Read(buffer, offset, count); + int bytesRead = _stream.Read(buffer, offset, count); if (bytesRead > 0) { _hasher.UpdateHash(buffer, offset, bytesRead); @@ -172,15 +169,15 @@ public override int Read(byte[] buffer, int offset, int count) } public string GetBase64Hash() => Convert.ToBase64String(_hasher.GetHash()); - public override bool CanRead => _inner.CanRead; - public override bool CanSeek => _inner.CanSeek; - public override bool CanWrite => _inner.CanWrite; - public override long Length => _inner.Length; - public override long Position { get => _inner.Position; set => _inner.Position = value; } - public override void Flush() => _inner.Flush(); - public override long Seek(long offset, SeekOrigin origin) => _inner.Seek(offset, origin); - public override void SetLength(long value) => _inner.SetLength(value); - public override void Write(byte[] buffer, int offset, int count) => _inner.Write(buffer, offset, count); + public override bool CanRead => _stream.CanRead; + public override bool CanSeek => _stream.CanSeek; + public override bool CanWrite => _stream.CanWrite; + public override long Length => _stream.Length; + public override long Position { get => _stream.Position; set => _stream.Position = value; } + public override void Flush() => _stream.Flush(); + public override long Seek(long offset, SeekOrigin origin) => _stream.Seek(offset, origin); + public override void SetLength(long value) => _stream.SetLength(value); + public override void Write(byte[] buffer, int offset, int count) => _stream.Write(buffer, offset, count); } } } From 1f8b6da1790ad711733722d0eeaa15fe9d127241 Mon Sep 17 00:00:00 2001 From: Mahendra Date: Tue, 31 Mar 2026 15:20:52 +0530 Subject: [PATCH 3/9] refactor(Storage): Revert LengthOnlyStream methods to not implement functionality --- .../StorageClientImpl.UploadObject.cs | 56 +++++-------------- 1 file changed, 14 insertions(+), 42 deletions(-) diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs index 5b45304839c6..e4278df853d2 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs @@ -229,57 +229,29 @@ internal async Task ExecuteAsync(CancellationToken cancellationToken) private sealed class LengthOnlyStream : Stream { private readonly long? _length; - private long _position; internal LengthOnlyStream(long? length) => _length = length; public override long Length => _length ?? throw new NotSupportedException(); public override bool CanSeek => _length.HasValue; - public override bool CanRead => true; - public override bool CanWrite => false; - public override long Position - { - get => _position; - set - { - if (!CanSeek) throw new NotSupportedException(); - if (value < 0 || value > Length) throw new ArgumentOutOfRangeException(nameof(value)); - _position = value; - } - } - - public override int Read(byte[] buffer, int offset, int count) - { - if (!_length.HasValue) - { - return 0; - } - long remaining = _length.Value - _position; - if (remaining <= 0) return 0; + public override bool CanRead => throw new NotImplementedException(); + public override bool CanWrite => throw new NotImplementedException(); - int toRead = (int) Math.Min(count, remaining); - - Array.Clear(buffer, offset, toRead); - - _position += toRead; - return toRead; - } - - public override void Flush() { } - - public override long Seek(long offset, SeekOrigin origin) + public override long Position { - switch (origin) - { - case SeekOrigin.Begin: Position = offset; break; - case SeekOrigin.Current: Position += offset; break; - case SeekOrigin.End: Position = Length + offset; break; - } - return Position; + get => throw new NotImplementedException(); + set => throw new NotImplementedException(); } - public override void SetLength(long value) => throw new NotSupportedException(); - public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + public override void Flush() => throw new NotImplementedException(); + public override int Read(byte[] buffer, int offset, int count) => + throw new NotImplementedException(); + public override long Seek(long offset, SeekOrigin origin) => + throw new NotImplementedException(); + public override void SetLength(long value) => + throw new NotImplementedException(); + public override void Write(byte[] buffer, int offset, int count) => + throw new NotImplementedException(); } } } From cf817fd880cc417cfc9aefe25a9c170fbf7c1afd Mon Sep 17 00:00:00 2001 From: mahendra-google Date: Tue, 7 Apr 2026 05:03:58 -0700 Subject: [PATCH 4/9] refactor(Storage): Address code review feedback fix(Storage): Check for disabled checksumming before wrapping stream --- .../CustomMediaUpload.cs | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs index 0561b37d98cb..d73127f63bcb 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs @@ -36,13 +36,16 @@ internal sealed class CustomMediaUpload : InsertMediaUpload public CustomMediaUpload(IClientService service, Apis.Storage.v1.Data.Object body, string bucket, Stream stream, string contentType, UploadObjectOptions options) - : base(service, body, bucket, new HashingStream(stream), contentType) + : base(service, body, bucket, options?.UploadValidationMode != UploadValidationMode.None ? new HashingStream(stream) : stream, contentType) { _service = service; var validationMode = options?.UploadValidationMode ?? UploadObjectOptions.DefaultValidationMode; - _hashingStream = (HashingStream) ContentStream; - _interceptor = new Crc32cHashInterceptor(this, _hashingStream, _service, validationMode); - _service?.HttpClient?.MessageHandler?.AddExecuteInterceptor(_interceptor); + if (validationMode != UploadValidationMode.None) + { + _hashingStream = ContentStream as HashingStream; + _interceptor = new Crc32cHashInterceptor(this, _hashingStream, _service); + _service?.HttpClient?.MessageHandler?.AddExecuteInterceptor(_interceptor); + } } internal new ResumableUploadOptions Options => base.Options; @@ -53,15 +56,13 @@ private sealed class Crc32cHashInterceptor : IHttpExecuteInterceptor private readonly IClientService _service; private readonly CustomMediaUpload _mediaUpload; private Uri _uploadUri; - private readonly UploadValidationMode _validationMode; private readonly HashingStream _hashingStream; - public Crc32cHashInterceptor(CustomMediaUpload mediaUpload, HashingStream hashingStream, IClientService service, UploadValidationMode validationMode) + public Crc32cHashInterceptor(CustomMediaUpload mediaUpload, HashingStream hashingStream, IClientService service) { _hashingStream = hashingStream; _service = service; _mediaUpload = mediaUpload; - _validationMode = validationMode; _mediaUpload.UploadSessionData += OnSessionData; _mediaUpload.ProgressChanged += OnProgressChanged; } @@ -79,11 +80,8 @@ public Task InterceptAsync(HttpRequestMessage request, CancellationToken cancell if (IsFinalChunk(rangeHeader)) { - if (_validationMode != UploadValidationMode.None) - { - var calculatedHash = _hashingStream.GetBase64Hash(); - request.Headers.TryAddWithoutValidation(GoogleHashHeader, $"crc32c={calculatedHash}"); - } + var calculatedHash = _hashingStream.GetBase64Hash(); + request.Headers.TryAddWithoutValidation(GoogleHashHeader, $"crc32c={calculatedHash}"); } } return Task.CompletedTask; @@ -168,6 +166,16 @@ public override int Read(byte[] buffer, int offset, int count) return bytesRead; } + public override async Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + int bytesRead = await _stream.ReadAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false); + if (bytesRead > 0) + { + _hasher.UpdateHash(buffer, offset, bytesRead); + } + return bytesRead; + } + public string GetBase64Hash() => Convert.ToBase64String(_hasher.GetHash()); public override bool CanRead => _stream.CanRead; public override bool CanSeek => _stream.CanSeek; From afce48bf66a694d50b44f4bd10d8ab6a8e098de6 Mon Sep 17 00:00:00 2001 From: mahendra-google Date: Tue, 7 Apr 2026 22:55:21 -0700 Subject: [PATCH 5/9] fix(Storage): Reset CRC32C hasher on stream seek to handle retries --- .../CustomMediaUpload.cs | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs index d73127f63bcb..77654b13f72f 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs @@ -152,9 +152,13 @@ private bool IsFinalChunk(string rangeHeader) private sealed class HashingStream : Stream { private readonly Stream _stream; - private readonly Crc32c _hasher = new(); + private Crc32c _hasher; - public HashingStream(Stream stream) => _stream = stream; + public HashingStream(Stream stream) + { + _stream = stream; + _hasher = new Crc32c(); + } public override int Read(byte[] buffer, int offset, int count) { @@ -176,14 +180,43 @@ public override async Task ReadAsync(byte[] buffer, int offset, int count, return bytesRead; } + public override long Position + { + get => _stream.Position; + set + { + if (value < _stream.Position) + { + _hasher = new Crc32c(); + } + _stream.Position = value; + } + } + + public override long Seek(long offset, SeekOrigin origin) + { + long target; + switch (origin) + { + case SeekOrigin.Begin: target = offset; break; + case SeekOrigin.Current: target = _stream.Position + offset; break; + case SeekOrigin.End: target = _stream.Length + offset; break; + default: target = _stream.Position; break; + } + + if (target < _stream.Position) + { + _hasher = new Crc32c(); + } + return _stream.Seek(offset, origin); + } + public string GetBase64Hash() => Convert.ToBase64String(_hasher.GetHash()); public override bool CanRead => _stream.CanRead; public override bool CanSeek => _stream.CanSeek; public override bool CanWrite => _stream.CanWrite; public override long Length => _stream.Length; - public override long Position { get => _stream.Position; set => _stream.Position = value; } public override void Flush() => _stream.Flush(); - public override long Seek(long offset, SeekOrigin origin) => _stream.Seek(offset, origin); public override void SetLength(long value) => _stream.SetLength(value); public override void Write(byte[] buffer, int offset, int count) => _stream.Write(buffer, offset, count); } From 73133c0424cbefadf986adcec1ad8b1d614c0669 Mon Sep 17 00:00:00 2001 From: mahendra-google Date: Wed, 22 Apr 2026 04:08:28 -0700 Subject: [PATCH 6/9] refactor(Storage): Strengthen URI validation in InterceptAsync --- .../Google.Cloud.Storage.V1/CustomMediaUpload.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs index 77654b13f72f..be0841de233b 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs @@ -69,7 +69,7 @@ public Crc32cHashInterceptor(CustomMediaUpload mediaUpload, HashingStream hashin public Task InterceptAsync(HttpRequestMessage request, CancellationToken cancellationToken) { - if (_uploadUri != null && !_uploadUri.Equals(request.RequestUri)) + if (_uploadUri == null || !_uploadUri.Equals(request.RequestUri)) { return Task.CompletedTask; } @@ -149,7 +149,7 @@ private bool IsFinalChunk(string rangeHeader) } } - private sealed class HashingStream : Stream + internal sealed class HashingStream : Stream { private readonly Stream _stream; private Crc32c _hasher; From 599c2fa4aaa62c2dcbe464b79753fcddbe4be614 Mon Sep 17 00:00:00 2001 From: Mahendra Date: Wed, 22 Apr 2026 17:04:16 +0530 Subject: [PATCH 7/9] refactor(Storage): Change internal class to private in CustomMediaUpload --- .../Google.Cloud.Storage.V1/CustomMediaUpload.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs index be0841de233b..8d905637785f 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs @@ -149,7 +149,7 @@ private bool IsFinalChunk(string rangeHeader) } } - internal sealed class HashingStream : Stream + private sealed class HashingStream : Stream { private readonly Stream _stream; private Crc32c _hasher; From d02012db33a3ae44f0c005a367bdbcdcd5352985 Mon Sep 17 00:00:00 2001 From: mahendra-google Date: Thu, 23 Apr 2026 05:10:59 -0700 Subject: [PATCH 8/9] refactor(Storage): Deprecate UploadValidationException Mark UploadValidationException as [Obsolete] to signal the transition from client-side to server-side upload validation. Server-side validation now returns a GoogleApiException with a 400 (Bad Request) status code instead of throwing this custom exception. This change maintains binary compatibility while providing a migration path for existing catch blocks. --- .../UploadValidationExceptionTest.cs | 5 ++++- .../StorageClientImpl.UploadObject.cs | 2 -- .../Google.Cloud.Storage.V1/UploadValidationException.cs | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.Tests/UploadValidationExceptionTest.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.Tests/UploadValidationExceptionTest.cs index a3af1d16a424..32b7d52cab3e 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.Tests/UploadValidationExceptionTest.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.Tests/UploadValidationExceptionTest.cs @@ -1,4 +1,4 @@ -// Copyright 2017 Google Inc. All Rights Reserved. +// Copyright 2017 Google Inc. All Rights Reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -21,6 +21,7 @@ namespace Google.Cloud.Storage.V1.Tests public class UploadValidationExceptionTest { [Fact] + [Obsolete] public void Construction_NoAdditionalFailure() { var ex = new UploadValidationException("hash", new Object(), null); @@ -28,6 +29,7 @@ public void Construction_NoAdditionalFailure() } [Fact] + [Obsolete] public void Construction_WithAdditionalFailure() { var additional = new Exception(); @@ -36,6 +38,7 @@ public void Construction_WithAdditionalFailure() } [Fact] + [Obsolete] public void Construction_WithAdditionalFailure_Empty() { Assert.Throws(() => new UploadValidationException("hash", new Object(), new AggregateException("No inner exceptions"))); diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs index e4278df853d2..ada419658d39 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/StorageClientImpl.UploadObject.cs @@ -195,7 +195,6 @@ internal Object Execute() { additionalFailures = new AggregateException(e); } - throw new UploadValidationException(hash, result, additionalFailures); } return result; } @@ -220,7 +219,6 @@ internal async Task ExecuteAsync(CancellationToken cancellationToken) { additionalFailures = new AggregateException(e); } - throw new UploadValidationException(hash, result, additionalFailures); } return result; } diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadValidationException.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadValidationException.cs index 4f7a9d674b81..61f04bb8cd35 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadValidationException.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadValidationException.cs @@ -1,4 +1,4 @@ -// Copyright 2017 Google Inc. All Rights Reserved. +// Copyright 2017 Google Inc. All Rights Reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -22,6 +22,8 @@ namespace Google.Cloud.Storage.V1 /// /// Exception thrown when an upload failed validation. /// + [Obsolete("This exception is no longer thrown. Server-side validation is now performed, " + + "resulting in a GoogleApiException with a 400 (Bad Request) status code on failure.")] public sealed class UploadValidationException : IOException { /// From 54d1d1c0ec90cff81882a31821497c2ed4c2381d Mon Sep 17 00:00:00 2001 From: mahendra-google Date: Thu, 23 Apr 2026 06:16:17 -0700 Subject: [PATCH 9/9] refactor(Storage): Modify HashingStream class to accomodate all retry scenarios Add tests for retry scenarios --- .../UploadObjectTest.cs | 47 ++++++++++++++++ .../CustomMediaUpload.cs | 56 ++++++++----------- 2 files changed, 70 insertions(+), 33 deletions(-) diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.IntegrationTests/UploadObjectTest.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.IntegrationTests/UploadObjectTest.cs index a419799a78ea..972848f0ab79 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.IntegrationTests/UploadObjectTest.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.IntegrationTests/UploadObjectTest.cs @@ -24,6 +24,7 @@ using System.Linq; using System.Net; using System.Net.Http; +using System.Text; using System.Threading; using System.Threading.Tasks; using Xunit; @@ -454,6 +455,52 @@ public async Task InterceptAsync(HttpRequestMessage request, CancellationToken c } } + [Fact] + public void HashingStream_ShouldHandleRetries_WhenRestartedFromBeginning() + { + var data = Encoding.UTF8.GetBytes("The quick brown fox jumps over the lazy dog"); + var baseStream = new MemoryStream(data); + var hashingStream = new CustomMediaUpload.HashingStream(baseStream); + var buffer = new byte[data.Length]; + + hashingStream.Read(buffer, 0, 10); + var hashAfterPartial = hashingStream.GetBase64Hash(); + + // Simulate the Retry logic: Seek back to the beginning + hashingStream.Position = 0; + + hashingStream.Read(buffer, 0, data.Length); + var finalHash = hashingStream.GetBase64Hash(); + + var expectedHasher = new Crc32c(); + expectedHasher.UpdateHash(data, 0, data.Length); + var expectedHash = Convert.ToBase64String(expectedHasher.GetHash()); + Assert.Equal(expectedHash, finalHash); + } + + [Fact] + public void HashingStream_ShouldHandleRetries_WhenSeekingBackwardsToIntermediatePoint() + { + var data = Encoding.UTF8.GetBytes("The quick brown fox jumps over the lazy dog"); + var baseStream = new MemoryStream(data); + var hashingStream = new CustomMediaUpload.HashingStream(baseStream); + var buffer = new byte[data.Length]; + + hashingStream.Read(buffer, 0, 10); + var hashAfterPartial = hashingStream.GetBase64Hash(); + + // Simulate the Retry logic: Seek back to the intermediate point. + hashingStream.Position = 5; + + hashingStream.Read(buffer, 0, data.Length); + var finalHash = hashingStream.GetBase64Hash(); + + var expectedHasher = new Crc32c(); + expectedHasher.UpdateHash(data, 0, data.Length); + var expectedHash = Convert.ToBase64String(expectedHasher.GetHash()); + Assert.Equal(expectedHash, finalHash); + } + private Object GetExistingObject() { var obj = _fixture.Client.UploadObject(_fixture.MultiVersionBucket, IdGenerator.FromGuid(), "application/octet-stream", GenerateData(100)); diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs index 8d905637785f..0ede13e023a8 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs @@ -149,10 +149,11 @@ private bool IsFinalChunk(string rangeHeader) } } - private sealed class HashingStream : Stream + internal sealed class HashingStream : Stream { private readonly Stream _stream; - private Crc32c _hasher; + private readonly Crc32c _hasher; + private long _maxPositionHashed = 0; public HashingStream(Stream stream) { @@ -162,55 +163,44 @@ public HashingStream(Stream stream) public override int Read(byte[] buffer, int offset, int count) { + long startingPos = _stream.Position; int bytesRead = _stream.Read(buffer, offset, count); - if (bytesRead > 0) - { - _hasher.UpdateHash(buffer, offset, bytesRead); - } + ProcessBytes(buffer, offset, bytesRead, startingPos); return bytesRead; } public override async Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { + long startingPos = _stream.Position; int bytesRead = await _stream.ReadAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false); - if (bytesRead > 0) - { - _hasher.UpdateHash(buffer, offset, bytesRead); - } + ProcessBytes(buffer, offset, bytesRead, startingPos); return bytesRead; } - public override long Position + private void ProcessBytes(byte[] buffer, int offset, int bytesRead, long startingPos) { - get => _stream.Position; - set + if (bytesRead <= 0) return; + + // Only hash bytes that are beyond the furthest point we've already hashed. + // This handles the rewind and re-read scenario during retries. + if (startingPos + bytesRead > _maxPositionHashed) { - if (value < _stream.Position) - { - _hasher = new Crc32c(); - } - _stream.Position = value; + long newBytesStart = Math.Max(startingPos, _maxPositionHashed); + int actuallyNewCount = (int) ((startingPos + bytesRead) - newBytesStart); + int bufferOffset = offset + (int) (newBytesStart - startingPos); + + _hasher.UpdateHash(buffer, bufferOffset, actuallyNewCount); + _maxPositionHashed = startingPos + bytesRead; } } - public override long Seek(long offset, SeekOrigin origin) + public override long Position { - long target; - switch (origin) - { - case SeekOrigin.Begin: target = offset; break; - case SeekOrigin.Current: target = _stream.Position + offset; break; - case SeekOrigin.End: target = _stream.Length + offset; break; - default: target = _stream.Position; break; - } - - if (target < _stream.Position) - { - _hasher = new Crc32c(); - } - return _stream.Seek(offset, origin); + get => _stream.Position; + set => _stream.Position = value; } + public override long Seek(long offset, SeekOrigin origin) => _stream.Seek(offset, origin); public string GetBase64Hash() => Convert.ToBase64String(_hasher.GetHash()); public override bool CanRead => _stream.CanRead; public override bool CanSeek => _stream.CanSeek;