Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d7bf416
revert: revert previous appendable upload checksum validation impleme…
v-pratap Jun 29, 2026
fb0c064
feat(storage): implement full object checksum validation for appendab…
v-pratap Jun 29, 2026
dc1168a
style(storage): format code with clang-format
v-pratap Jun 30, 2026
36ca641
fix(storage): fix data races when accessing impl_ in AsyncWriterConne…
v-pratap Jun 30, 2026
612bae7
fix(storage): resolve hash function lifecycle and rollback corruption…
v-pratap Jun 30, 2026
3fc0179
fix(storage): preserve ABI compatibility by adding explicit Finalize …
v-pratap Jun 30, 2026
861f0e9
fix(storage): resolve missing FormatCrc32c declaration and redundant …
v-pratap Jun 30, 2026
2baac36
fix(storage): prevent unsigned integer underflow during CRC32C rollba…
v-pratap Jun 30, 2026
5be9817
fix(storage): always send full-object checksum on finalize for append…
v-pratap Jul 1, 2026
5571438
refactor(storage): address reviewer comments on appendable upload che…
v-pratap Jul 2, 2026
f18ffb7
refactor(storage): separate EnsureCrc32cHistory from RestoreChecksumS…
v-pratap Jul 3, 2026
ee69899
fix(storage): only rewind the active hash function and purge future c…
v-pratap Jul 6, 2026
0775138
chore(storage): add comment explaining why MD5 won't crash on stream …
v-pratap Jul 6, 2026
bbdf129
refactor(storage): bypass checksum verification during Flush() and Cl…
v-pratap Jul 8, 2026
1772900
refactor(storage): completely remove incremental CRC hashing for appe…
v-pratap Jul 8, 2026
3599ff0
chore: remove client-side full object checksum tracking and validation
v-pratap Jul 8, 2026
b8eafc4
fix: address PR review comments for appendable upload checksum valida…
v-pratap Jul 9, 2026
40f5fed
fix(storage): split writer and reader hash functions
v-pratap Jul 9, 2026
33c8d5f
chore(storage): remove trailing blank lines in upload test
v-pratap Jul 9, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions google/cloud/storage/async/writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,20 +71,31 @@ future<StatusOr<AsyncToken>> AsyncWriter::Write(AsyncToken token,

future<StatusOr<google::storage::v2::Object>> AsyncWriter::Finalize(
AsyncToken token, WritePayload payload) {
return Finalize(std::move(token), std::move(payload), absl::nullopt);
}

future<StatusOr<google::storage::v2::Object>> AsyncWriter::Finalize(
AsyncToken token, WritePayload payload,
absl::optional<Crc32cChecksumValue> const& expected_checksum) {
if (!impl_) return StreamError<google::storage::v2::Object>(GCP_ERROR_INFO());
auto t = storage_internal::MakeAsyncToken(impl_.get());
if (token != t) {
return TokenError<google::storage::v2::Object>(GCP_ERROR_INFO());
}

return impl_->Finalize(std::move(payload)).then([impl = impl_](auto f) {
return f.get();
});
return impl_->Finalize(std::move(payload), expected_checksum)
.then([impl = impl_](auto f) { return f.get(); });
}

future<StatusOr<google::storage::v2::Object>> AsyncWriter::Finalize(
AsyncToken token) {
return Finalize(std::move(token), WritePayload{});
return Finalize(std::move(token), WritePayload{}, absl::nullopt);
}

future<StatusOr<google::storage::v2::Object>> AsyncWriter::Finalize(
AsyncToken token,
absl::optional<Crc32cChecksumValue> const& expected_checksum) {
return Finalize(std::move(token), WritePayload{}, expected_checksum);
}

future<Status> AsyncWriter::Flush() {
Expand Down
21 changes: 20 additions & 1 deletion google/cloud/storage/async/writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "google/cloud/storage/async/token.h"
#include "google/cloud/storage/async/write_payload.h"
#include "google/cloud/storage/async/writer_connection.h"
#include "google/cloud/storage/hashing_options.h"
#include "google/cloud/future.h"
#include "google/cloud/status_or.h"
#include "google/cloud/version.h"
Expand Down Expand Up @@ -114,14 +115,32 @@ class AsyncWriter {
/// Upload @p payload returning a new token to continue the upload.
future<StatusOr<AsyncToken>> Write(AsyncToken token, WritePayload payload);

/// Finalize the upload with the existing data.
/**
* Finalize the upload with the existing data.
*
* @warning For appendable uploads, the SDK does not incrementally compute
* full-object hashes. You *must* calculate and inject your own expected
* checksum here to get end-to-end data integrity. If you omit it, the object
* will finalize without full-object verification.
*/
future<StatusOr<google::storage::v2::Object>> Finalize(AsyncToken token);
future<StatusOr<google::storage::v2::Object>> Finalize(
AsyncToken token,
absl::optional<Crc32cChecksumValue> const& expected_checksum);

/**
* Upload @p payload and then finalize the upload.
*
* @warning For appendable uploads, the SDK does not incrementally compute
* full-object hashes. You *must* calculate and inject your own expected
* checksum here to get end-to-end data integrity. If you omit it, the object
* will finalize without full-object verification.
*/
future<StatusOr<google::storage::v2::Object>> Finalize(AsyncToken token,
WritePayload payload);
future<StatusOr<google::storage::v2::Object>> Finalize(
AsyncToken token, WritePayload payload,
absl::optional<Crc32cChecksumValue> const& expected_checksum);

/**
* Flush any buffered data to the service.
Expand Down
6 changes: 6 additions & 0 deletions google/cloud/storage/async/writer_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_ASYNC_WRITER_CONNECTION_H

#include "google/cloud/storage/async/write_payload.h"
#include "google/cloud/storage/hashing_options.h"
#include "google/cloud/storage/object_metadata.h"
#include "google/cloud/future.h"
#include "google/cloud/rpc_metadata.h"
Expand Down Expand Up @@ -112,6 +113,11 @@ class AsyncWriterConnection {
/// Finalizes an upload.
virtual future<StatusOr<google::storage::v2::Object>> Finalize(
WritePayload) = 0;
virtual future<StatusOr<google::storage::v2::Object>> Finalize(
WritePayload p,
absl::optional<Crc32cChecksumValue> const& /*expected_checksum*/) {
return Finalize(std::move(p));
}

/// Uploads some data to the service and flushes the value.
virtual future<Status> Flush(WritePayload payload) = 0;
Expand Down
48 changes: 34 additions & 14 deletions google/cloud/storage/internal/async/connection_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,34 @@ inline std::unique_ptr<storage::AsyncIdempotencyPolicy> idempotency_policy(
return options.get<storage::AsyncIdempotencyPolicyOption>()();
}

std::unique_ptr<storage::internal::HashFunction> CreateHashFunction(
std::unique_ptr<storage::internal::HashFunction> CreateWriterHashFunction(
Options const& options) {
auto crc32c = std::unique_ptr<storage::internal::HashFunction>();
if (options.has<storage::UseCrc32cValueOption>()) {
crc32c = std::make_unique<storage::internal::PrecomputedHashFunction>(
storage::internal::HashValues{
Crc32cFromProto(options.get<storage::UseCrc32cValueOption>()),
/*.md5=*/{}});
}

auto md5 = std::unique_ptr<storage::internal::HashFunction>();
if (options.has<storage::UseMD5ValueOption>()) {
md5 = std::make_unique<storage::internal::PrecomputedHashFunction>(
storage::internal::HashValues{
/*.crc32=*/{},
MD5FromProto(options.get<storage::UseMD5ValueOption>())});
}

if (crc32c && md5) {
return std::make_unique<storage::internal::CompositeFunction>(
std::move(crc32c), std::move(md5));
}
if (crc32c) return crc32c;
if (md5) return md5;
return storage::internal::CreateNullHashFunction();
}

std::unique_ptr<storage::internal::HashFunction> CreateReaderHashFunction(
Options const& options) {
auto crc32c = std::unique_ptr<storage::internal::HashFunction>();
if (options.has<storage::UseCrc32cValueOption>()) {
Expand Down Expand Up @@ -125,19 +152,12 @@ StatusOr<std::unique_ptr<storage::AsyncWriterConnection>> MakeAppendableWriter(

if (rpc->first_response.has_resource()) {
auto const& resource = rpc->first_response.resource();
if (current->get<storage::EnableCrc32cValidationOption>() &&
resource.has_checksums() && resource.checksums().has_crc32c()) {
hash = std::make_shared<
::google::cloud::storage::internal::Crc32cHashFunction>(
resource.checksums().crc32c(), resource.size());
} else {
hash = CreateHashFunction(*current);
}
hash = CreateWriterHashFunction(*current);
impl = std::make_unique<AsyncWriterConnectionImpl>(
current, request, std::move(rpc->stream), hash, resource, false);
} else {
persisted_size = rpc->first_response.persisted_size();
hash = CreateHashFunction(*current);
hash = CreateWriterHashFunction(*current);
auto checksums = rpc->first_response.has_persisted_data_checksums()
? absl::make_optional(
rpc->first_response.persisted_data_checksums())
Expand Down Expand Up @@ -208,7 +228,7 @@ future<StatusOr<google::storage::v2::Object>> AsyncConnectionImpl::InsertObject(
options->get<storage::TransferStallMinimumRateOption>(),
google::storage::v2::ServiceConstants::MAX_WRITE_CHUNK_BYTES);

auto hash_function = CreateHashFunction(*options);
auto hash_function = CreateWriterHashFunction(*options);
ApplyRoutingHeaders(*context, request.write_object_spec());
AddIdempotencyToken(*context, id);
auto rpc = stub->AsyncWriteObject(cq, std::move(context), options);
Expand Down Expand Up @@ -314,7 +334,7 @@ AsyncConnectionImpl::ReadObject(ReadObjectParams p) {
// this function.
auto hash_function =
std::make_shared<storage::internal::Crc32cMessageHashFunction>(
CreateHashFunction(*current));
CreateReaderHashFunction(*current));
auto hash_validator = CreateHashValidator(p.request, *current);

absl::optional<std::int64_t> requested_length;
Expand Down Expand Up @@ -730,7 +750,7 @@ AsyncConnectionImpl::StartUnbufferedUploadImpl(
StatusOr<std::unique_ptr<storage::AsyncWriterConnection>>(
std::move(response).status()));
}
auto hash_function = CreateHashFunction(*current);
auto hash_function = CreateWriterHashFunction(*current);
auto configure =
[current, upload = response->upload_id()](grpc::ClientContext& context) {
ApplyResumableUploadRoutingHeader(context, upload);
Expand Down Expand Up @@ -768,7 +788,7 @@ AsyncConnectionImpl::ResumeUnbufferedUploadImpl(
// the upload resumes from the beginning of the file.
auto hash_function = storage::internal::CreateNullHashFunction();
if (response->persisted_size() == 0) {
hash_function = CreateHashFunction(*current);
hash_function = CreateWriterHashFunction(*current);
}
auto configure =
[current, upload_id = query.upload_id()](grpc::ClientContext& context) {
Expand Down
Loading
Loading