From 4b0d135acd283f8ef362b253ac8758d41d1a08e3 Mon Sep 17 00:00:00 2001 From: mahendra-google Date: Thu, 15 Jan 2026 23:15:04 -0800 Subject: [PATCH 1/4] feat(Storage): Enable full object checksum validation for resumable uploads --- .../UploadObjectTest.cs | 219 +++++++++++------- .../UploadObjectOptionsTest.cs | 8 +- .../UploadValidationExceptionTest.cs | 44 ---- .../CustomMediaUpload.cs | 170 +++++++++++++- .../Google.Cloud.Storage.V1.csproj | 1 + .../StorageClientImpl.UploadObject.cs | 52 +---- .../UploadObjectOptions.cs | 2 +- .../UploadValidationException.cs | 68 ------ .../UploadValidationMode.cs | 22 +- 9 files changed, 320 insertions(+), 266 deletions(-) delete mode 100644 apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.Tests/UploadValidationExceptionTest.cs delete mode 100644 apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadValidationException.cs 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 5568ca990378..75f0254081e4 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; @@ -299,7 +300,7 @@ public void UploadObject_InvalidHash_None() } [Fact] - public void UploadObject_InvalidHash_ThrowOnly() + public void UploadObject_InvalidHash_RejectAndThrow() { var client = StorageClient.Create(); var interceptor = new BreakUploadInterceptor(); @@ -307,44 +308,13 @@ public void UploadObject_InvalidHash_ThrowOnly() var stream = GenerateData(50); 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)); - } - - [Fact] - public void UploadObject_InvalidHash_DeleteAndThrow() - { - var client = StorageClient.Create(); - var interceptor = new BreakUploadInterceptor(); - client.Service.HttpClient.MessageHandler.AddExecuteInterceptor(interceptor); - var stream = GenerateData(50); - 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 options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.RejectAndThrow }; + 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() { @@ -362,53 +332,21 @@ public async Task UploadObjectAsync_InvalidHash_None() } [Fact] - public async Task UploadObjectAsync_InvalidHash_ThrowOnly() - { - var client = StorageClient.Create(); - var interceptor = new BreakUploadInterceptor(); - client.Service.HttpClient.MessageHandler.AddExecuteInterceptor(interceptor); - var stream = GenerateData(50); - 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)); - } - - [Fact] - public async Task UploadObjectAsync_InvalidHash_DeleteAndThrow() + public async Task UploadObjectAsync_InvalidHash_RejectAndThrow() { var client = StorageClient.Create(); var interceptor = new BreakUploadInterceptor(); client.Service.HttpClient.MessageHandler.AddExecuteInterceptor(interceptor); - var stream = GenerateData(50); 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 options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.RejectAndThrow }; + 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,19 +426,134 @@ public async Task InterceptAsync(HttpRequestMessage request, CancellationToken c } } - private class BreakDeleteInterceptor : IHttpExecuteInterceptor + [Fact] + public void HashingStream_ShouldHandleRetries_WhenRestartedFromBeginning() { - 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); - } + 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); + + // 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); + + // 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); + } + + [Fact] + public void HashingStream_ShouldDetectGaps_WhenResumingFromIntermediateOffset() + { + 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]; + + // Simulate resuming an upload from a new process starting at intermediate offset 10 + hashingStream.Position = 10; + int bytesRead = hashingStream.Read(buffer, 0, data.Length - 10); + + Assert.Equal(data.Length - 10, bytesRead); + // Because bytes 0-9 were never hashed, IsHashComplete must be false + Assert.False(hashingStream.IsHashComplete); + } + + [Fact] + public void CustomMediaUpload_ShouldThrowArgumentException_WhenResumingFromIntermediateOffset() + { + var data = Encoding.UTF8.GetBytes("The quick brown fox jumps over the lazy dog"); + var baseStream = new MemoryStream(data); + var client = _fixture.Client; + var service = client.Service; + var obj = new Object { Bucket = _fixture.MultiVersionBucket, Name = IdGenerator.FromGuid() }; + var options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.RejectAndThrow }; + + var uploader = new CustomMediaUpload(service, obj, _fixture.MultiVersionBucket, baseStream, "text/plain", options); + var hashingStream = uploader.ContentStream as CustomMediaUpload.HashingStream; + Assert.NotNull(hashingStream); + + // Simulate resuming from offset 10 + hashingStream.Position = 10; + var buffer = new byte[data.Length]; + hashingStream.Read(buffer, 0, data.Length - 10); + + // Verify IsHashComplete is false due to unhashed prefix + Assert.False(hashingStream.IsHashComplete); + + // Simulate final request execution + var request = new HttpRequestMessage(HttpMethod.Put, "https://example.com/upload"); + var eventField = typeof(ResumableUpload).GetField( + "LastRequestExecuting", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + var handler = (Action) eventField.GetValue(uploader); + var exception = Assert.Throws(() => handler?.Invoke(request)); + Assert.Contains("Cannot perform hash validation when resuming", exception.Message); + Assert.Equal("stream", exception.ParamName); + } + + [Fact] + public void CustomMediaUpload_ShouldContainHashHeaderAndCorrectHash_WhenRetriedFromIntermediateOffset() + { + var data = Encoding.UTF8.GetBytes("The quick brown fox jumps over the lazy dog"); + var baseStream = new MemoryStream(data); + var client = _fixture.Client; + var service = client.Service; + var obj = new Object { Bucket = _fixture.MultiVersionBucket, Name = IdGenerator.FromGuid() }; + var options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.RejectAndThrow }; + + var uploader = new CustomMediaUpload(service, obj, _fixture.MultiVersionBucket, baseStream, "text/plain", options); + var hashingStream = uploader.ContentStream as CustomMediaUpload.HashingStream; + Assert.NotNull(hashingStream); + + var buffer = new byte[data.Length]; + hashingStream.Read(buffer, 0, 10); + // 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.True(hashingStream.IsHashComplete); + + // Simulate final request execution + var request = new HttpRequestMessage(HttpMethod.Put, "https://example.com/upload"); + var eventField = typeof(ResumableUpload).GetField( + "LastRequestExecuting", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + var handler = (Action) eventField.GetValue(uploader); + handler?.Invoke(request); + Assert.True(request.Headers.Contains("x-goog-hash")); + Assert.Equal(expectedHash, finalHash); } private Object GetExistingObject() 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.Tests/UploadValidationExceptionTest.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.Tests/UploadValidationExceptionTest.cs deleted file mode 100644 index a3af1d16a424..000000000000 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.Tests/UploadValidationExceptionTest.cs +++ /dev/null @@ -1,44 +0,0 @@ -// 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. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -using System; -using Xunit; -using Object = Google.Apis.Storage.v1.Data.Object; - -namespace Google.Cloud.Storage.V1.Tests -{ - public class UploadValidationExceptionTest - { - [Fact] - public void Construction_NoAdditionalFailure() - { - var ex = new UploadValidationException("hash", new Object(), null); - Assert.Null(ex.AdditionalFailures); - } - - [Fact] - public void Construction_WithAdditionalFailure() - { - var additional = new Exception(); - var ex = new UploadValidationException("hash", new Object(), new AggregateException(additional)); - Assert.Same(additional, ex.AdditionalFailures.InnerExceptions[0]); - } - - [Fact] - 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/CustomMediaUpload.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs index c91a50ef9265..825a050cc913 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,15 @@ // See the License for the specific language governing permissions and // limitations under the License. +using Google.Api.Gax; using Google.Apis.Services; +using Google.Apis.Upload; using System; using System.IO; 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 +29,171 @@ namespace Google.Cloud.Storage.V1 /// internal sealed class CustomMediaUpload : InsertMediaUpload { + private readonly HashingStream _hashingStream; + private const string GoogleHashHeader = "x-goog-hash"; + 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, (options?.UploadValidationMode ?? UploadObjectOptions.DefaultValidationMode) != UploadValidationMode.None ? new HashingStream(stream) : stream, contentType) { + var validationMode = options?.UploadValidationMode ?? UploadObjectOptions.DefaultValidationMode; + GaxPreconditions.CheckEnumValue(validationMode, nameof(UploadObjectOptions.UploadValidationMode)); + if (validationMode != UploadValidationMode.None) + { + _hashingStream = ContentStream as HashingStream; + LastRequestExecuting += (HttpRequestMessage request) => + { + if (_hashingStream != null) + { + if (_hashingStream.HasGaps) + { + throw new ArgumentException( + "Cannot perform hash validation when resuming an upload from a non-zero offset, " + + "as the complete stream contents are required to compute the hash. " + + "To resume this upload, disable validation by setting UploadValidationMode to None.", + nameof(stream)); + } + if (_hashingStream.IsHashComplete) + { + var calculatedHash = _hashingStream.GetBase64Hash(); + bool hasCrc32c = false; + if (request.Headers.TryGetValues(GoogleHashHeader, out var values)) + { + foreach (var value in values) + { + if (value?.IndexOf("crc32c", StringComparison.OrdinalIgnoreCase) >= 0) + { + hasCrc32c = true; + break; + } + } + } + if (!hasCrc32c) + { + request.Headers.TryAddWithoutValidation(GoogleHashHeader, $"crc32c={calculatedHash}"); + } + } + } + }; + } } internal new ResumableUploadOptions Options => base.Options; + + internal sealed class HashingStream : Stream + { + private readonly Stream _stream; + private readonly Crc32c _hasher; + private long _maxPositionHashed = 0; + private long _position = 0; + private bool _hasGaps = false; + private bool _reachedEof = false; + public bool HasGaps => _hasGaps; + + public HashingStream(Stream stream) + { + _stream = stream; + _hasher = new Crc32c(); + } + + public bool IsHashComplete => !_hasGaps && (_stream.CanSeek ? _maxPositionHashed == _stream.Length : _reachedEof); + + public override int Read(byte[] buffer, int offset, int count) + { + long startingPos = _stream.CanSeek ? _stream.Position : _position; + int bytesRead = _stream.Read(buffer, offset, count); + if (count > 0 && bytesRead == 0) + { + _reachedEof = true; + } + ProcessBytes(buffer, offset, bytesRead, startingPos); + if (!_stream.CanSeek) + { + _position += bytesRead; + } + return bytesRead; + } + + public override async Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + long startingPos = _stream.CanSeek ? _stream.Position : _position; + int bytesRead = await _stream.ReadAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false); + if (count > 0 && bytesRead == 0) + { + _reachedEof = true; + } + ProcessBytes(buffer, offset, bytesRead, startingPos); + if (!_stream.CanSeek) + { + _position += bytesRead; + } + return bytesRead; + } + + private void ProcessBytes(byte[] buffer, int offset, int bytesRead, long startingPos) + { + if (bytesRead <= 0) return; + + if (startingPos > _maxPositionHashed) + { + _hasGaps = true; + 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) + { + 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 Position + { + get => _stream.CanSeek ? _stream.Position : _position; + set + { + if (_stream.CanSeek) + { + _stream.Position = value; + } + else + { + throw new NotSupportedException(); + } + } + } + + public override long Seek(long offset, SeekOrigin origin) + { + if (_stream.CanSeek) + { + return _stream.Seek(offset, origin); + } + throw new NotSupportedException(); + } + + 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 void Flush() => _stream.Flush(); + public override void SetLength(long value) => _stream.SetLength(value); + public override void Write(byte[] buffer, int offset, int count) => _stream.Write(buffer, offset, count); + protected override void Dispose(bool disposing) + { + if (disposing) + { + _stream?.Dispose(); + } + base.Dispose(disposing); + } + } } } diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.csproj b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.csproj index 255673446be7..8f96abc839b8 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.csproj +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1.csproj @@ -9,6 +9,7 @@ + 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..270e1a1b681a 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; @@ -143,9 +143,6 @@ private sealed class UploadHelper { private readonly StorageClient _client; private readonly ObjectsResource.InsertMediaUpload _mediaUpload; - private readonly Crc32c _crc; - private readonly Action _validationFailureAction; - private readonly Func _validationFailureAsyncAction; internal UploadHelper( StorageClient client, @@ -160,22 +157,6 @@ internal UploadHelper( { _mediaUpload.ProgressChanged += progress.Report; } - - var validationMode = options?.UploadValidationMode ?? UploadObjectOptions.DefaultValidationMode; - GaxPreconditions.CheckEnumValue(validationMode, nameof(UploadObjectOptions.UploadValidationMode)); - switch (validationMode) - { - case UploadValidationMode.DeleteAndThrow: - _crc = new Crc32c(); - _mediaUpload.UploadStreamInterceptor += _crc.UpdateHash; - _validationFailureAction = obj => client.DeleteObject(obj, new DeleteObjectOptions { Generation = obj.Generation }); - _validationFailureAsyncAction = (obj, token) => client.DeleteObjectAsync(obj, new DeleteObjectOptions { Generation = obj.Generation }, token); - break; - case UploadValidationMode.ThrowOnly: - _crc = new Crc32c(); - _mediaUpload.UploadStreamInterceptor += _crc.UpdateHash; - break; - } } internal Object Execute() @@ -183,20 +164,6 @@ internal Object Execute() _mediaUpload.Upload(); _mediaUpload.GetProgress().ThrowOnFailure(); var result = _mediaUpload.ResponseBody; - var hash = _crc == null ? result.Crc32c : Convert.ToBase64String(_crc.GetHash()); - if (hash != result.Crc32c) - { - AggregateException additionalFailures = null; - try - { - _validationFailureAction?.Invoke(result); - } - catch (Exception e) - { - additionalFailures = new AggregateException(e); - } - throw new UploadValidationException(hash, result, additionalFailures); - } return result; } @@ -205,23 +172,6 @@ internal async Task ExecuteAsync(CancellationToken cancellationToken) await _mediaUpload.UploadAsync(cancellationToken).ConfigureAwait(false); _mediaUpload.GetProgress().ThrowOnFailure(); var result = _mediaUpload.ResponseBody; - var hash = _crc == null ? result.Crc32c : Convert.ToBase64String(_crc.GetHash()); - if (hash != result.Crc32c) - { - AggregateException additionalFailures = null; - try - { - if (_validationFailureAsyncAction != null) - { - await _validationFailureAsyncAction(result, cancellationToken).ConfigureAwait(false); - } - } - catch (Exception e) - { - additionalFailures = new AggregateException(e); - } - throw new UploadValidationException(hash, result, additionalFailures); - } return result; } } diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadObjectOptions.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadObjectOptions.cs index 11992d0fdab6..2405cd25edc7 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadObjectOptions.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadObjectOptions.cs @@ -32,7 +32,7 @@ public sealed class UploadObjectOptions /// public const int MinimumChunkSize = ResumableUpload.MinimumChunkSize; - internal static UploadValidationMode DefaultValidationMode { get; } = V1.UploadValidationMode.DeleteAndThrow; + internal static UploadValidationMode DefaultValidationMode { get; } = V1.UploadValidationMode.RejectAndThrow; /// /// Precondition for upload: the object is only uploaded if the existing object's 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 deleted file mode 100644 index 4f7a9d674b81..000000000000 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadValidationException.cs +++ /dev/null @@ -1,68 +0,0 @@ -// 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. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -using Google.Api.Gax; -using System; -using System.IO; -using Object = Google.Apis.Storage.v1.Data.Object; - -namespace Google.Cloud.Storage.V1 -{ - /// - /// Exception thrown when an upload failed validation. - /// - public sealed class UploadValidationException : IOException - { - /// - /// The hash computed locally, in base64. - /// - public string ClientSideHash { get; } - - /// - /// The uploaded object. - /// - public Object UploadedObject { get; } - - /// - /// A collection of additional failures following on from this one, if any. For - /// example, if the validation mode indicates that on failure the file should be deleted, - /// but the deletion fails, that exception would be present here. This property - /// is either null, or returns an containing one or more - /// exceptions; it will never return an empty . - /// - public AggregateException AdditionalFailures { get; } - - /// - /// Creates a new exception. - /// - /// The hash of the uploaded data, as computed at the client. Must not be null. - /// The object created by Google Cloud Storage. Must not be null. - /// Any additional failures encountered while handling the error. May be null; if non-null, - /// must contain at least one exception. - public UploadValidationException(string clientSideHash, Object uploadedObject, AggregateException additionalFailures) - : base("Upload validation failed") - { - ClientSideHash = GaxPreconditions.CheckNotNull(clientSideHash, nameof(clientSideHash)); - UploadedObject = GaxPreconditions.CheckNotNull(uploadedObject, nameof(uploadedObject)); - AdditionalFailures = additionalFailures; - if (additionalFailures != null) - { - GaxPreconditions.CheckArgument( - additionalFailures.InnerExceptions.Count > 0, - nameof(additionalFailures), - "Additional failures AggregateException cannot be empty"); - } - } - } -} diff --git a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadValidationMode.cs b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadValidationMode.cs index 59b4e68152c7..bb994f658c6c 100644 --- a/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadValidationMode.cs +++ b/apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/UploadValidationMode.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,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +using System; + namespace Google.Cloud.Storage.V1 { /// @@ -25,19 +27,17 @@ public enum UploadValidationMode None = 0, /// - /// The hash of the data is computed while uploading, and - /// if the resulting object has a different hash, an - /// is thrown, but the object remains present in Storage. + /// Obsolete. Use instead. + /// Previously, the object was uploaded and then deleted if the hash mismatched. + /// The server now rejects mismatched objects automatically before creation. /// - ThrowOnly = 1, + [Obsolete("DeleteAndThrow is deprecated. Use RejectAndThrow instead, as the server now rejects the object before creation.")] + DeleteAndThrow = 2, /// - /// The hash of the data is computed while uploading, and - /// if the resulting object has a different hash, an attempt is made to delete the object. - /// Whether the deletion fails or not, an - /// is thrown. If the deletion fails, that failure can be examined via - /// + /// The server validates the object hash during upload. If a hash mismatch is detected, + /// the server rejects the upload entirely, preventing the object from being created. /// - DeleteAndThrow = 2 + RejectAndThrow = 3 } } From fe3538c7e43766a3f1374f16a46be30a3d767330 Mon Sep 17 00:00:00 2001 From: mahendra-google Date: Mon, 24 Aug 2026 06:01:12 -0700 Subject: [PATCH 2/4] refactor(Storage): Address review feedback from the Storage Team --- .../UploadObjectTest.cs | 8 ++++++-- .../Google.Cloud.Storage.V1/CustomMediaUpload.cs | 11 +++++------ 2 files changed, 11 insertions(+), 8 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 75f0254081e4..639fd91848d2 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 @@ -514,8 +514,10 @@ public void CustomMediaUpload_ShouldThrowArgumentException_WhenResumingFromInter var eventField = typeof(ResumableUpload).GetField( "LastRequestExecuting", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + Assert.NotNull(eventField); var handler = (Action) eventField.GetValue(uploader); - var exception = Assert.Throws(() => handler?.Invoke(request)); + Assert.NotNull(handler); + var exception = Assert.Throws(() => handler.Invoke(request)); Assert.Contains("Cannot perform hash validation when resuming", exception.Message); Assert.Equal("stream", exception.ParamName); } @@ -550,8 +552,10 @@ public void CustomMediaUpload_ShouldContainHashHeaderAndCorrectHash_WhenRetriedF var eventField = typeof(ResumableUpload).GetField( "LastRequestExecuting", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + Assert.NotNull(eventField); var handler = (Action) eventField.GetValue(uploader); - handler?.Invoke(request); + Assert.NotNull(handler); + handler.Invoke(request); Assert.True(request.Headers.Contains("x-goog-hash")); Assert.Equal(expectedHash, finalHash); } 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 825a050cc913..fb16dac90993 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 @@ -29,7 +29,6 @@ namespace Google.Cloud.Storage.V1 /// internal sealed class CustomMediaUpload : InsertMediaUpload { - private readonly HashingStream _hashingStream; private const string GoogleHashHeader = "x-goog-hash"; public CustomMediaUpload(IClientService service, Apis.Storage.v1.Data.Object body, string bucket, @@ -40,12 +39,12 @@ public CustomMediaUpload(IClientService service, Apis.Storage.v1.Data.Object bod GaxPreconditions.CheckEnumValue(validationMode, nameof(UploadObjectOptions.UploadValidationMode)); if (validationMode != UploadValidationMode.None) { - _hashingStream = ContentStream as HashingStream; + var hashingStream = ContentStream as HashingStream; LastRequestExecuting += (HttpRequestMessage request) => { - if (_hashingStream != null) + if (hashingStream != null) { - if (_hashingStream.HasGaps) + if (hashingStream.HasGaps) { throw new ArgumentException( "Cannot perform hash validation when resuming an upload from a non-zero offset, " + @@ -53,9 +52,9 @@ public CustomMediaUpload(IClientService service, Apis.Storage.v1.Data.Object bod "To resume this upload, disable validation by setting UploadValidationMode to None.", nameof(stream)); } - if (_hashingStream.IsHashComplete) + if (hashingStream.IsHashComplete) { - var calculatedHash = _hashingStream.GetBase64Hash(); + var calculatedHash = hashingStream.GetBase64Hash(); bool hasCrc32c = false; if (request.Headers.TryGetValues(GoogleHashHeader, out var values)) { From 2c8f1936a6d0066e42fbe741aa2df76e4e43d689 Mon Sep 17 00:00:00 2001 From: mahendra-google Date: Tue, 25 Aug 2026 02:30:22 -0700 Subject: [PATCH 3/4] refactor(Storage): Address review feedback from the Storage Team - Add integration tests to verify ArgumentException propagation through Google.Apis ResumableUpload when resuming an upload from the intermediate offset. --- .../UploadObjectTest.cs | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) 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 639fd91848d2..750a38f75396 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 @@ -487,6 +487,102 @@ public void HashingStream_ShouldDetectGaps_WhenResumingFromIntermediateOffset() Assert.False(hashingStream.IsHashComplete); } + [Fact] + public async Task CustomMediaUpload_ResumeAsync_WithStreamGap_FailsWithArgumentException() + { + var client = _fixture.Client; + var bucket = _fixture.MultiVersionBucket; + var name = IdGenerator.FromGuid(); + + int chunk1Size = 256 * 1024; + int chunk2Size = 100; + int totalSize = chunk1Size + chunk2Size; + + var fullData = GenerateData(totalSize); + byte[] fullBytes = fullData.ToArray(); + + var uploadUri = await client.InitiateUploadSessionAsync(bucket, name, "application/octet-stream", totalSize); + + // 2. Upload the first 256 KiB chunk directly using HTTP PUT so GCS contains bytes 0..262143 + var chunk1Content = new ByteArrayContent(fullBytes, 0, chunk1Size); + chunk1Content.Headers.Add("Content-Range", $"bytes 0-{chunk1Size - 1}/{totalSize}"); + chunk1Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/octet-stream"); + + var chunk1Request = new HttpRequestMessage(HttpMethod.Put, uploadUri) + { + Content = chunk1Content + }; + var chunk1Response = await client.Service.HttpClient.SendAsync(chunk1Request); + Assert.Equal((HttpStatusCode) 308, chunk1Response.StatusCode); + + // Resume the session using a new CustomMediaUpload instance with validation enabled + var fullStream = new MemoryStream(fullBytes); + var destination = new Object { Bucket = bucket, Name = name, ContentType = "application/octet-stream" }; + var options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.RejectAndThrow }; + + var uploader = (CustomMediaUpload) client.CreateObjectUploader(destination, fullStream, options); + + // Execute ResumeAsync via the Google.Apis ResumableUpload + var progress = await uploader.ResumeAsync(uploadUri); + + // Verify failure and exception + Assert.Equal(UploadStatus.Failed, progress.Status); + var exception = Assert.IsType(progress.Exception); + Assert.Contains("Cannot perform hash validation when resuming", exception.Message); + Assert.Equal("stream", exception.ParamName); + + // Verify ThrowOnFailure() unwraps and rethrows the ArgumentException + var thrown = Assert.Throws(() => progress.ThrowOnFailure()); + Assert.Same(exception, thrown); + } + + [Fact] + public async Task CustomMediaUpload_ResumeSync_WithStreamGap_FailsWithArgumentException() + { + var client = _fixture.Client; + var bucket = _fixture.MultiVersionBucket; + var name = IdGenerator.FromGuid(); + + int chunk1Size = 256 * 1024; + int chunk2Size = 100; + int totalSize = chunk1Size + chunk2Size; + + var fullData = GenerateData(totalSize); + byte[] fullBytes = fullData.ToArray(); + + var uploadUri = await client.InitiateUploadSessionAsync(bucket, name, "application/octet-stream", totalSize); + + // Upload first 256 KiB chunk directly via HTTP + var chunk1Content = new ByteArrayContent(fullBytes, 0, chunk1Size); + chunk1Content.Headers.Add("Content-Range", $"bytes 0-{chunk1Size - 1}/{totalSize}"); + chunk1Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/octet-stream"); + + var chunk1Request = new HttpRequestMessage(HttpMethod.Put, uploadUri) + { + Content = chunk1Content + }; + var chunk1Response = await client.Service.HttpClient.SendAsync(chunk1Request); + Assert.Equal((HttpStatusCode) 308, chunk1Response.StatusCode); + + // Resume synchronously with CustomMediaUpload + var fullStream = new MemoryStream(fullBytes); + var destination = new Object { Bucket = bucket, Name = name, ContentType = "application/octet-stream" }; + var options = new UploadObjectOptions { UploadValidationMode = UploadValidationMode.RejectAndThrow }; + + var uploader = (CustomMediaUpload) client.CreateObjectUploader(destination, fullStream, options); + + var progress = uploader.Resume(uploadUri); + + Assert.Equal(UploadStatus.Failed, progress.Status); + var exception = Assert.IsType(progress.Exception); + Assert.Contains("Cannot perform hash validation when resuming", exception.Message); + Assert.Equal("stream", exception.ParamName); + + // Verify ThrowOnFailure() unwraps and rethrows the ArgumentException + var thrown = Assert.Throws(() => progress.ThrowOnFailure()); + Assert.Same(exception, thrown); + } + [Fact] public void CustomMediaUpload_ShouldThrowArgumentException_WhenResumingFromIntermediateOffset() { From b97accadea44e66bb6695f0544199170f4bd1941 Mon Sep 17 00:00:00 2001 From: mahendra-google Date: Tue, 25 Aug 2026 02:35:08 -0700 Subject: [PATCH 4/4] docs(Storage): Remove erroneous character from comment --- .../UploadObjectTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 750a38f75396..c0e5318f4f8e 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 @@ -503,7 +503,7 @@ public async Task CustomMediaUpload_ResumeAsync_WithStreamGap_FailsWithArgumentE var uploadUri = await client.InitiateUploadSessionAsync(bucket, name, "application/octet-stream", totalSize); - // 2. Upload the first 256 KiB chunk directly using HTTP PUT so GCS contains bytes 0..262143 + // Upload the first 256 KiB chunk directly using HTTP PUT so GCS contains bytes 0..262143 var chunk1Content = new ByteArrayContent(fullBytes, 0, chunk1Size); chunk1Content.Headers.Add("Content-Range", $"bytes 0-{chunk1Size - 1}/{totalSize}"); chunk1Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/octet-stream");