Skip to content

Remove deprecated legacy checksum options completely #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
v-pratap wants to merge 2 commits into
base: unified-checksum-options-pr5
Choose a base branch
from
Open

Remove deprecated legacy checksum options completely #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3282cbe
Completely remove deprecated legacy checksum options
v-pratap Jul 24, 2026
90bce30
Fix integration tests overlapping options, fix ReadObject using Uploa…
v-pratap Jul 25, 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
58 changes: 0 additions & 58 deletions google/cloud/storage/client_object_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,64 +234,6 @@ TEST_F(ObjectTest, ReadObject) {
EXPECT_EQ(actual.gcount(), 1024);
}

TEST_F(ObjectTest, ReadObjectChecksumPrecedence) {
EXPECT_CALL(*mock_, ReadObject)
.WillOnce([](internal::ReadObjectRangeRequest const& r) {
EXPECT_TRUE(r.HasOption<DisableMD5Hash>());
EXPECT_FALSE(r.GetOption<DisableMD5Hash>().value());

auto settings =
internal::GetDownloadChecksumSettings(r, CurrentOptions());
// Verify MD5 is enabled (disable_md5 = false) and CRC32C is disabled
// (disable_crc32c = true)
EXPECT_FALSE(settings.md5);
EXPECT_TRUE(settings.crc32c);

auto read_source = std::make_unique<testing::MockObjectReadSource>();
EXPECT_CALL(*read_source, IsOpen()).WillRepeatedly(Return(true));
EXPECT_CALL(*read_source, Read)
.WillOnce(Return(internal::ReadSourceResult{1024, {}}));
EXPECT_CALL(*read_source, Close).Times(1);
return StatusOr<std::unique_ptr<internal::ObjectReadSource>>(
std::move(read_source));
});
auto client = ClientForMock();
auto actual = client.ReadObject(
"test-bucket-name", "test-object-name", DisableMD5Hash(false),
Options{}.set<DownloadChecksumValidationOption>(
ChecksumAlgorithm::kNone));
ASSERT_STATUS_OK(actual.status());
std::vector<char> v(1024);
actual.read(v.data(), v.size());
EXPECT_EQ(actual.gcount(), 1024);
}

TEST_F(ObjectTest, InsertObjectChecksumPrecedence) {
EXPECT_CALL(*mock_, InsertObjectMedia)
.WillOnce([](internal::InsertObjectMediaRequest const& r) {
EXPECT_TRUE(r.HasOption<DisableCrc32cChecksum>());
EXPECT_TRUE(r.GetOption<DisableCrc32cChecksum>().value());

auto settings =
internal::GetUploadChecksumSettings(r, CurrentOptions());
// Verify CRC32C is disabled (disable_crc32c = true) and MD5 remains
// enabled (disable_md5 = false)
EXPECT_TRUE(settings.crc32c);
EXPECT_FALSE(settings.md5);

return make_status_or(
storage::internal::ObjectMetadataParser::FromString(
R"({"name": "test-object-name"})")
.value());
});
auto client = ClientForMock();
auto actual =
client.InsertObject("test-bucket-name", "test-object-name", "payload",
DisableCrc32cChecksum(true),
Options{}.set<UploadChecksumValidationOption>(
ChecksumAlgorithm::kCrc32cAndMD5));
ASSERT_STATUS_OK(actual);
}

TEST_F(ObjectTest, WriteObject) {
EXPECT_CALL(*mock_, CreateResumableUpload)
Expand Down
60 changes: 0 additions & 60 deletions google/cloud/storage/hashing_options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,40 +61,6 @@ inline std::string ComputeMD5Hash(char const* payload) {
: absl::string_view{payload});
}

/**
* Disable or enable MD5 Hashing computations.
*
* By default MD5 hashes are disabled. To enable them use the
* `EnableMD5Hash()` helper function.
*
* @warning MD5 hashes are disabled by default, as they are computationally
* expensive, and CRC32C checksums provide enough data integrity protection
* for most applications. Disabling CRC32C checksums while MD5 hashes remain
* disabled exposes your application to data corruption. We recommend that all
* uploads to GCS and downloads from GCS use CRC32C checksums.
*
* @deprecated Use `UploadChecksumValidationOption` and
* `DownloadChecksumValidationOption` instead.
*/
struct [[deprecated(
"Use UploadChecksumValidationOption and DownloadChecksumValidationOption "
"instead")]] DisableMD5Hash
: public internal::ComplexOption<DisableMD5Hash, bool> {
using ComplexOption<DisableMD5Hash, bool>::ComplexOption;
// GCC <= 7.0 does not use the inherited default constructor, redeclare it
// explicitly
DisableMD5Hash() : DisableMD5Hash(true) {}
static char const* name() { return "disable-md5-hash"; }
};

/**
* Enable MD5 hashes in upload and download operations.
*
* Use this function where the option `DisableMD5Hash` is expected to enable MD5
* hashes.
*/
inline DisableMD5Hash EnableMD5Hash() { return DisableMD5Hash(false); }

/**
* Provide a pre-computed CRC32C checksum value.
*
Expand Down Expand Up @@ -130,32 +96,6 @@ inline std::string ComputeCrc32cChecksum(char const* payload) {
: absl::string_view{payload});
}

/**
* Disable CRC32C checksum computations.
*
* By default the GCS client library computes CRC32C checksums in all upload and
* download operations. The application can use this option to disable the
* checksum computation.
*
* @warning MD5 hashes are disabled by default, as they are computationally
* expensive, and CRC32C checksums provide enough data integrity protection
* for most applications. Disabling CRC32C checksums while MD5 hashes remain
* disabled exposes your application to data corruption. We recommend that all
* uploads to GCS and downloads from GCS use CRC32C checksums.
*
* @deprecated Use `UploadChecksumValidationOption` and
* `DownloadChecksumValidationOption` instead.
*/
struct [[deprecated(
"Use UploadChecksumValidationOption and DownloadChecksumValidationOption "
"instead")]] DisableCrc32cChecksum
: public internal::ComplexOption<DisableCrc32cChecksum, bool> {
using ComplexOption<DisableCrc32cChecksum, bool>::ComplexOption;
// GCC <= 7.0 does not use the inherited default constructor, redeclare it
// explicitly
DisableCrc32cChecksum() = default;
static char const* name() { return "disable-crc32c-checksum"; }
};

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
} // namespace storage
Expand Down
46 changes: 4 additions & 42 deletions google/cloud/storage/internal/checksum_helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,69 +31,31 @@ struct HashDisabled {
bool crc32c;
};

template <typename Request>
HashDisabled GetDownloadChecksumSettings(Request const& request,
Options const& options) {
bool disable_md5 = false;
inline HashDisabled GetDownloadChecksumSettings(Options const& options) {
bool disable_md5 = true;
bool disable_crc32c = false;
bool use_new_algo = false;
if (options.has<DownloadChecksumValidationOption>()) {
auto const algo = options.get<DownloadChecksumValidationOption>();
disable_md5 = (algo != ChecksumAlgorithm::kMD5 &&
algo != ChecksumAlgorithm::kCrc32cAndMD5);
disable_crc32c = (algo != ChecksumAlgorithm::kCrc32c &&
algo != ChecksumAlgorithm::kCrc32cAndMD5);
use_new_algo = true;
}

auto const md5 = request.template GetOption<DisableMD5Hash>();
if (md5.has_value()) {
// DisableMD5Hash defaults to true. We only override the new option if the
// legacy option was explicitly set to false, or if the new option was not
// provided at all.
if (!use_new_algo || md5.value() != true) {
disable_md5 = md5.value();
}
}
auto const crc32c = request.template GetOption<DisableCrc32cChecksum>();
if (crc32c.has_value()) {
// DisableCrc32cChecksum defaults to std::nullopt, so if it has a value, it
// was explicitly set.
disable_crc32c = crc32c.value();
}
return {disable_md5, disable_crc32c};
}

template <typename Request>
HashDisabled GetUploadChecksumSettings(Request const& request,
Options const& options) {
bool disable_md5 = false;
inline HashDisabled GetUploadChecksumSettings(Options const& options) {
bool disable_md5 = true;
bool disable_crc32c = false;
bool use_new_algo = false;
if (options.has<UploadChecksumValidationOption>()) {
auto const algo = options.get<UploadChecksumValidationOption>();
disable_md5 = (algo != ChecksumAlgorithm::kMD5 &&
algo != ChecksumAlgorithm::kCrc32cAndMD5);
disable_crc32c = (algo != ChecksumAlgorithm::kCrc32c &&
algo != ChecksumAlgorithm::kCrc32cAndMD5);
use_new_algo = true;
}

auto const md5 = request.template GetOption<DisableMD5Hash>();
if (md5.has_value()) {
// DisableMD5Hash defaults to true. We only override the new option if the
// legacy option was explicitly set to false, or if the new option was not
// provided at all.
if (!use_new_algo || md5.value() != true) {
disable_md5 = md5.value();
}
}
auto const crc32c = request.template GetOption<DisableCrc32cChecksum>();
if (crc32c.has_value()) {
// DisableCrc32cChecksum defaults to std::nullopt, so if it has a value, it
// was explicitly set.
disable_crc32c = crc32c.value();
}
return {disable_md5, disable_crc32c};
}

Expand Down
2 changes: 1 addition & 1 deletion google/cloud/storage/internal/connection_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ This is often a problem because:
preserve data integrity.

Consider using UploadLimit option or Client::WriteObject(). You may also need to disable data
integrity checks using the DisableMD5Hash() and DisableCrc32cChecksum() options.
integrity checks using the UploadChecksumValidationOption() option.
)""";
} else {
std::error_code size_err;
Expand Down
8 changes: 3 additions & 5 deletions google/cloud/storage/internal/grpc/stub.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,12 +349,10 @@ StatusOr<storage::ObjectMetadata> GrpcStub::InsertObjectMedia(
auto stream = stub_->WriteObject(std::move(ctx), options);

auto const settings =
storage::internal::GetUploadChecksumSettings(request, options);
auto disable_md5 = storage::DisableMD5Hash(settings.md5);
auto disable_crc32c = storage::DisableCrc32cChecksum(settings.crc32c);
storage::internal::GetUploadChecksumSettings(options);
auto hash_function = storage::internal::CreateHashFunction(
request.GetOption<storage::Crc32cChecksumValue>(), disable_crc32c,
request.GetOption<storage::MD5HashValue>(), disable_md5);
request.GetOption<storage::Crc32cChecksumValue>(), settings.crc32c,
request.GetOption<storage::MD5HashValue>(), settings.md5);

auto splitter = SplitObjectWriteData<ContentType>(request.payload());
std::int64_t offset = 0;
Expand Down
21 changes: 8 additions & 13 deletions google/cloud/storage/internal/hash_function.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,14 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
namespace internal {

std::unique_ptr<HashFunction> CreateHashFunction(
Crc32cChecksumValue const& crc32c_value,
DisableCrc32cChecksum const& crc32c_disabled, MD5HashValue const& md5_value,
DisableMD5Hash const& md5_disabled) {
Crc32cChecksumValue const& crc32c_value, bool disable_crc32c,
MD5HashValue const& md5_value, bool disable_md5) {
auto crc32c = std::unique_ptr<HashFunction>();
auto crc32c_v = crc32c_value.value_or("");
if (!crc32c_v.empty()) {
crc32c = std::make_unique<PrecomputedHashFunction>(
HashValues{/*.crc32c=*/std::move(crc32c_v), /*md5=*/{}});
} else if (!crc32c_disabled.value_or(false)) {
} else if (!disable_crc32c) {
crc32c = std::make_unique<Crc32cHashFunction>();
}

Expand All @@ -46,7 +45,7 @@ std::unique_ptr<HashFunction> CreateHashFunction(
if (!md5_v.empty()) {
md5 = std::make_unique<PrecomputedHashFunction>(
HashValues{/*.crc32c=*/{}, /*.md5=*/std::move(md5_v)});
} else if (!md5_disabled.value_or(false)) {
} else if (!disable_md5) {
md5 = MD5HashFunction::Create();
}

Expand All @@ -65,8 +64,7 @@ std::unique_ptr<HashFunction> CreateHashFunction(
ReadObjectRangeRequest const& request) {
if (request.RequiresRangeHeader()) return CreateNullHashFunction();

auto const settings = GetDownloadChecksumSettings(
request, google::cloud::internal::CurrentOptions());
auto const settings = GetDownloadChecksumSettings(google::cloud::internal::CurrentOptions());
auto const disable_md5 = settings.md5;
auto const disable_crc32c = settings.crc32c;
if (disable_md5 && disable_crc32c) {
Expand All @@ -86,13 +84,10 @@ std::unique_ptr<HashFunction> CreateHashFunction(
return CreateNullHashFunction();
}

auto const settings = GetUploadChecksumSettings(
request, google::cloud::internal::CurrentOptions());
auto disable_md5 = DisableMD5Hash(settings.md5);
auto disable_crc32c = DisableCrc32cChecksum(settings.crc32c);
auto const settings = GetUploadChecksumSettings(google::cloud::internal::CurrentOptions());
return CreateHashFunction(request.GetOption<Crc32cChecksumValue>(),
disable_crc32c, request.GetOption<MD5HashValue>(),
disable_md5);
settings.crc32c, request.GetOption<MD5HashValue>(),
settings.md5);
}

} // namespace internal
Expand Down
5 changes: 2 additions & 3 deletions google/cloud/storage/internal/hash_function.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,8 @@ class HashFunction {

/// Create a hash function configured by several options.
std::unique_ptr<HashFunction> CreateHashFunction(
Crc32cChecksumValue const& crc32c_value,
DisableCrc32cChecksum const& crc32c_disabled, MD5HashValue const& md5_value,
DisableMD5Hash const& md5_disabled);
Crc32cChecksumValue const& crc32c_value, bool disable_crc32c,
MD5HashValue const& md5_value, bool disable_md5);

/// Create a no-op hash function
std::unique_ptr<HashFunction> CreateNullHashFunction();
Expand Down
37 changes: 17 additions & 20 deletions google/cloud/storage/internal/hash_function_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,22 +378,19 @@ TEST(HashFunctionImplTest, CreateHashFunctionRead) {
struct Test {
std::string crc32c_expected;
std::string md5_expected;
DisableCrc32cChecksum crc32_disabled;
DisableMD5Hash md5_disabled;
ChecksumAlgorithm validation_algo;
} cases[]{
{"", "", DisableCrc32cChecksum(true), DisableMD5Hash(true)},
{"", kQuickFoxMD5Hash, DisableCrc32cChecksum(true),
DisableMD5Hash(false)},
{kQuickFoxCrc32cChecksum, "", DisableCrc32cChecksum(false),
DisableMD5Hash(true)},
{kQuickFoxCrc32cChecksum, kQuickFoxMD5Hash, DisableCrc32cChecksum(false),
DisableMD5Hash(false)},
{"", "", ChecksumAlgorithm::kNone},
{"", kQuickFoxMD5Hash, ChecksumAlgorithm::kMD5},
{kQuickFoxCrc32cChecksum, "", ChecksumAlgorithm::kCrc32c},
{kQuickFoxCrc32cChecksum, kQuickFoxMD5Hash, ChecksumAlgorithm::kCrc32cAndMD5},
};

for (auto const& test : cases) {
google::cloud::internal::OptionsSpan span(
Options{}.set<DownloadChecksumValidationOption>(test.validation_algo));
auto function = CreateHashFunction(
ReadObjectRangeRequest("test-bucket", "test-object")
.set_multiple_options(test.crc32_disabled, test.md5_disabled));
ReadObjectRangeRequest("test-bucket", "test-object"));
function->Update(kQuickFox);
auto const actual = std::move(*function).Finish();
EXPECT_EQ(test.crc32c_expected, actual.crc32c);
Expand All @@ -404,20 +401,21 @@ TEST(HashFunctionImplTest, CreateHashFunctionRead) {
struct UploadTest {
std::string crc32c_expected;
std::string md5_expected;
DisableCrc32cChecksum crc32_disabled;
bool disable_crc32c;
Crc32cChecksumValue crc32_value;
DisableMD5Hash md5_disabled;
bool disable_md5;
MD5HashValue md5_value;
};

TEST(HashFunctionImplTest, CreateHashFunctionUpload) {
auto const upload_cases = testing::UploadHashCases();

for (auto const& test : upload_cases) {
google::cloud::internal::OptionsSpan span(
Options{}.set<UploadChecksumValidationOption>(test.validation_algo));
auto function = CreateHashFunction(
ResumableUploadRequest("test-bucket", "test-object")
.set_multiple_options(test.crc32_disabled, test.crc32_value,
test.md5_disabled, test.md5_value));
.set_multiple_options(test.crc32_value, test.md5_value));
function->Update(kQuickFox);
auto const actual = std::move(*function).Finish();
EXPECT_EQ(test.crc32c_expected, actual.crc32c);
Expand All @@ -428,9 +426,7 @@ TEST(HashFunctionImplTest, CreateHashFunctionUpload) {
TEST(HashFunctionImplTest, CreateHashFunctionUploadResumedSession) {
auto function = CreateHashFunction(
ResumableUploadRequest("test-bucket", "test-object")
.set_multiple_options(UseResumableUploadSession("test-session-id"),
DisableCrc32cChecksum(false),
DisableMD5Hash(false)));
.set_multiple_options(UseResumableUploadSession("test-session-id")));
function->Update(kQuickFox);
auto const actual = std::move(*function).Finish();
EXPECT_THAT(actual.crc32c, IsEmpty());
Expand All @@ -441,8 +437,9 @@ TEST(HashFunctionImplTest, CreateHashFunctionInsertObjectMedia) {
auto const upload_cases = testing::UploadHashCases();

for (auto const& test : upload_cases) {
auto function = CreateHashFunction(test.crc32_value, test.crc32_disabled,
test.md5_value, test.md5_disabled);
bool disable_crc32c = (test.validation_algo == ChecksumAlgorithm::kNone || test.validation_algo == ChecksumAlgorithm::kMD5);
bool disable_md5 = (test.validation_algo == ChecksumAlgorithm::kNone || test.validation_algo == ChecksumAlgorithm::kCrc32c);
auto function = CreateHashFunction(test.crc32_value, disable_crc32c, test.md5_value, disable_md5);
ASSERT_STATUS_OK(function->Update(/*offset=*/0, kQuickFox));
auto const actual = function->Finish();
EXPECT_EQ(test.crc32c_expected, actual.crc32c);
Expand Down
Loading
Loading