diff --git a/google/cloud/storage/async/options.h b/google/cloud/storage/async/options.h index d979730dbac24..2637be6bab192 100644 --- a/google/cloud/storage/async/options.h +++ b/google/cloud/storage/async/options.h @@ -29,8 +29,13 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN * checksum of an object during uploads and downloads. * * The option has no effect if the service does not return a CRC32C hash. + * + * @deprecated Use `UploadChecksumValidationOption` and + * `DownloadChecksumValidationOption` instead. */ -struct EnableCrc32cValidationOption { +struct [[deprecated( + "Use UploadChecksumValidationOption and DownloadChecksumValidationOption " + "instead")]] EnableCrc32cValidationOption { using Type = bool; }; @@ -54,8 +59,13 @@ struct UseCrc32cValueOption { * * The option has no effect for partial downloads or any other circumstance * where the service does not return a MD5 hash. + * + * @deprecated Use `UploadChecksumValidationOption` and + * `DownloadChecksumValidationOption` instead. */ -struct EnableMD5ValidationOption { +struct [[deprecated( + "Use UploadChecksumValidationOption and DownloadChecksumValidationOption " + "instead")]] EnableMD5ValidationOption { using Type = bool; }; diff --git a/google/cloud/storage/google_cloud_cpp_storage_grpc.bzl b/google/cloud/storage/google_cloud_cpp_storage_grpc.bzl index 4113b1e4cab04..6c1ad25a15247 100644 --- a/google/cloud/storage/google_cloud_cpp_storage_grpc.bzl +++ b/google/cloud/storage/google_cloud_cpp_storage_grpc.bzl @@ -38,6 +38,7 @@ google_cloud_cpp_storage_grpc_hdrs = [ "async/writer_connection.h", "grpc_plugin.h", "internal/async/connection_fwd.h", + "internal/async/checksum_helpers.h", "internal/async/connection_impl.h", "internal/async/connection_tracing.h", "internal/async/default_options.h", diff --git a/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake b/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake index 35f458503ec73..0064cd7e9e78e 100644 --- a/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake +++ b/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake @@ -99,6 +99,7 @@ add_library( async/writer_connection.h grpc_plugin.cc grpc_plugin.h + internal/async/checksum_helpers.h internal/async/connection_fwd.h internal/async/connection_impl.cc internal/async/connection_impl.h diff --git a/google/cloud/storage/hashing_options.h b/google/cloud/storage/hashing_options.h index 3fcac86aea5d4..d9c26edbc6b8f 100644 --- a/google/cloud/storage/hashing_options.h +++ b/google/cloud/storage/hashing_options.h @@ -15,8 +15,10 @@ #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_HASHING_OPTIONS_H #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_HASHING_OPTIONS_H +#include "google/cloud/internal/disable_deprecation_warnings.inc" #include "google/cloud/storage/internal/complex_option.h" #include "google/cloud/storage/version.h" +#include "google/cloud/version.h" #include "absl/strings/string_view.h" #include @@ -70,8 +72,14 @@ inline std::string ComputeMD5Hash(char const* payload) { * 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 DisableMD5Hash : public internal::ComplexOption { +struct [[deprecated( + "Use UploadChecksumValidationOption and DownloadChecksumValidationOption " + "instead")]] DisableMD5Hash + : public internal::ComplexOption { using ComplexOption::ComplexOption; // GCC <= 7.0 does not use the inherited default constructor, redeclare it // explicitly @@ -134,8 +142,13 @@ inline std::string ComputeCrc32cChecksum(char const* payload) { * 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 DisableCrc32cChecksum +struct [[deprecated( + "Use UploadChecksumValidationOption and DownloadChecksumValidationOption " + "instead")]] DisableCrc32cChecksum : public internal::ComplexOption { using ComplexOption::ComplexOption; // GCC <= 7.0 does not use the inherited default constructor, redeclare it @@ -149,4 +162,5 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace cloud } // namespace google +#include "google/cloud/internal/diagnostics_pop.inc" #endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_HASHING_OPTIONS_H diff --git a/google/cloud/storage/hashing_options_test.cc b/google/cloud/storage/hashing_options_test.cc index 047731d655c6b..8bd3ec50dc388 100644 --- a/google/cloud/storage/hashing_options_test.cc +++ b/google/cloud/storage/hashing_options_test.cc @@ -13,6 +13,8 @@ // limitations under the License. #include "google/cloud/storage/hashing_options.h" +#include "google/cloud/storage/options.h" +#include "google/cloud/options.h" #include #include @@ -54,6 +56,23 @@ TEST(ComputeCrc32cChecksumTest, Simple) { EXPECT_EQ("ImIEBA==", actual); } +TEST(ChecksumOptionsTest, SetAndGet) { + Options options; + EXPECT_FALSE(options.has()); + EXPECT_FALSE(options.has()); + + options.set(ChecksumAlgorithm::kMD5); + options.set(ChecksumAlgorithm::kCrc32c); + + EXPECT_TRUE(options.has()); + EXPECT_EQ(ChecksumAlgorithm::kMD5, + options.get()); + + EXPECT_TRUE(options.has()); + EXPECT_EQ(ChecksumAlgorithm::kCrc32c, + options.get()); +} + } // namespace GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace storage diff --git a/google/cloud/storage/internal/async/checksum_helpers.h b/google/cloud/storage/internal/async/checksum_helpers.h new file mode 100644 index 0000000000000..9937c75945236 --- /dev/null +++ b/google/cloud/storage/internal/async/checksum_helpers.h @@ -0,0 +1,59 @@ +// Copyright 2024 Google LLC +// +// 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 +// +// https://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. + +#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ASYNC_CHECKSUM_HELPERS_H +#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ASYNC_CHECKSUM_HELPERS_H + +#include "google/cloud/internal/disable_deprecation_warnings.inc" +#include "google/cloud/storage/async/options.h" +#include "google/cloud/storage/options.h" +#include "google/cloud/options.h" + +namespace google { +namespace cloud { +namespace storage_internal { +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN + +struct ChecksumSettings { + bool enable_crc32c; + bool enable_md5; +}; + +inline ChecksumSettings GetDownloadChecksumSettings(Options const& options) { + if (options.has()) { + auto const algo = options.get(); + return {algo == storage::ChecksumAlgorithm::kCrc32c, + algo == storage::ChecksumAlgorithm::kMD5}; + } + return {options.get(), + options.get()}; +} + +inline ChecksumSettings GetUploadChecksumSettings(Options const& options) { + if (options.has()) { + auto const algo = options.get(); + return {algo == storage::ChecksumAlgorithm::kCrc32c, + algo == storage::ChecksumAlgorithm::kMD5}; + } + return {options.get(), + options.get()}; +} + +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace storage_internal +} // namespace cloud +} // namespace google + +#include "google/cloud/internal/diagnostics_pop.inc" +#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ASYNC_CHECKSUM_HELPERS_H diff --git a/google/cloud/storage/internal/async/connection_impl.cc b/google/cloud/storage/internal/async/connection_impl.cc index 976fea52f2337..2ef264992a79b 100644 --- a/google/cloud/storage/internal/async/connection_impl.cc +++ b/google/cloud/storage/internal/async/connection_impl.cc @@ -19,6 +19,7 @@ #include "google/cloud/storage/async/reader.h" #include "google/cloud/storage/async/resume_policy.h" #include "google/cloud/storage/async/retry_policy.h" +#include "google/cloud/storage/internal/async/checksum_helpers.h" #include "google/cloud/storage/internal/async/default_options.h" #include "google/cloud/storage/internal/async/handle_redirect_error.h" #include "google/cloud/storage/internal/async/insert_object.h" @@ -80,14 +81,18 @@ inline std::unique_ptr idempotency_policy( } std::unique_ptr CreateHashFunction( - Options const& options) { + Options const& options, + storage_internal::ChecksumSettings const& settings) { auto crc32c = std::unique_ptr(); + auto const enable_crc32c = settings.enable_crc32c; + auto const enable_md5 = settings.enable_md5; + if (options.has()) { crc32c = std::make_unique( storage::internal::HashValues{ Crc32cFromProto(options.get()), /*.md5=*/{}}); - } else if (options.get()) { + } else if (enable_crc32c) { crc32c = std::make_unique(); } @@ -97,7 +102,7 @@ std::unique_ptr CreateHashFunction( storage::internal::HashValues{ /*.crc32=*/{}, MD5FromProto(options.get())}); - } else if (options.get()) { + } else if (enable_md5) { md5 = storage::internal::MD5HashFunction::Create(); } @@ -117,9 +122,9 @@ std::unique_ptr CreateHashValidator( request.read_limit() != 0 || request.read_offset() != 0; if (is_ranged_read) return storage::internal::CreateNullHashValidator(); - auto const enable_crc32c = - options.get(); - auto const enable_md5 = options.get(); + auto const settings = GetDownloadChecksumSettings(options); + auto const enable_crc32c = settings.enable_crc32c; + auto const enable_md5 = settings.enable_md5; if (enable_crc32c && enable_md5) { return std::make_unique( @@ -167,7 +172,8 @@ future> AsyncConnectionImpl::InsertObject( options->get(), google::storage::v2::ServiceConstants::MAX_WRITE_CHUNK_BYTES); - auto hash_function = CreateHashFunction(*options); + auto hash_function = + CreateHashFunction(*options, GetUploadChecksumSettings(*options)); ApplyRoutingHeaders(*context, request.write_object_spec()); AddIdempotencyToken(*context, id); auto rpc = stub->AsyncWriteObject(cq, std::move(context), options); @@ -271,9 +277,10 @@ AsyncConnectionImpl::ReadObject(ReadObjectParams p) { // Create the hash function and validator based on the original request. Note // that p.request will be moved-from, so we have to do it relatively early in // this function. + auto const settings = GetDownloadChecksumSettings(*current); auto hash_function = std::make_shared( - CreateHashFunction(*current)); + CreateHashFunction(*current, settings)); auto hash_validator = CreateHashValidator(p.request, *current); absl::optional requested_length; @@ -323,7 +330,7 @@ AsyncConnectionImpl::AppendableObjectUploadImpl(AppendableUploadParams p) { auto current = internal::MakeImmutableOptions(std::move(p.options)); auto request = p.request; std::shared_ptr hash_function = - CreateHashFunction(*current); + CreateHashFunction(*current, GetUploadChecksumSettings(*current)); auto retry = std::shared_ptr(retry_policy(*current)); auto backoff = @@ -703,7 +710,8 @@ AsyncConnectionImpl::StartUnbufferedUploadImpl( StatusOr>( std::move(response).status())); } - auto hash_function = CreateHashFunction(*current); + auto hash_function = + CreateHashFunction(*current, GetUploadChecksumSettings(*current)); auto configure = [current, upload = response->upload_id()](grpc::ClientContext& context) { ApplyResumableUploadRoutingHeader(context, upload); @@ -741,7 +749,8 @@ 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 = + CreateHashFunction(*current, GetUploadChecksumSettings(*current)); } auto configure = [current, upload_id = query.upload_id()](grpc::ClientContext& context) { diff --git a/google/cloud/storage/internal/async/connection_impl_read_hash_test.cc b/google/cloud/storage/internal/async/connection_impl_read_hash_test.cc index 5519f9bdbb2d1..d7e9db7a578ad 100644 --- a/google/cloud/storage/internal/async/connection_impl_read_hash_test.cc +++ b/google/cloud/storage/internal/async/connection_impl_read_hash_test.cc @@ -13,6 +13,17 @@ // limitations under the License. #include "google/cloud/storage/async/options.h" + +// TODO: Remove this when EnableMD5ValidationOption and +// EnableCrc32cValidationOption are removed. +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +#elif defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +#endif + #include "google/cloud/storage/internal/async/connection_impl.h" #include "google/cloud/storage/internal/async/default_options.h" #include "google/cloud/storage/internal/crc32c.h" @@ -75,15 +86,21 @@ auto GeneratedObjectChecksums(HashTestCase const& tc) { std::ostream& operator<<(std::ostream& os, HashTestCase const& rhs) { os << "HashTestCase={options={"; os << "expected_status_code=" << rhs.expected_status_code // - << std::boolalpha // - << ", enable_crc32c_validation=" - << rhs.options.get(); + << std::boolalpha; // + if (rhs.options.has()) { + os << ", download_checksum=" + << static_cast( + rhs.options.get()); + } else { + os << ", enable_crc32c_validation=" + << rhs.options.get(); + os << ", enable_md5_validation=" + << rhs.options.get(); + } if (rhs.options.has()) { os << ", use_crc32_value=" << rhs.options.get(); } - os << ", enable_md5_validation=" - << rhs.options.get(); if (rhs.options.has()) { os << ", use_md5_value=" << rhs.options.get(); } @@ -129,16 +146,14 @@ INSTANTIATE_TEST_SUITE_P( // This is the common case. Only CRC32C is enabled by default. The // service returns both CRC32C and MD5 values. StatusCode::kOk, - Options{} - .set(true) - .set(false), + Options{}.set( + storage::ChecksumAlgorithm::kCrc32c), kQuickFoxCrc32cChecksum, kQuickFoxMD5Hash}, HashTestCase{ // This is also common, the service does not return a MD5 value. StatusCode::kOk, - Options{} - .set(true) - .set(false), + Options{}.set( + storage::ChecksumAlgorithm::kCrc32c), kQuickFoxCrc32cChecksum, ""}, // Make sure things work when both hashes are validated too. HashTestCase{StatusCode::kOk, @@ -149,45 +164,60 @@ INSTANTIATE_TEST_SUITE_P( // In the next three cases we verify that disabling some validation // works. HashTestCase{StatusCode::kOk, - Options{} - .set(true) - .set(false), + Options{}.set( + storage::ChecksumAlgorithm::kCrc32c), kQuickFoxCrc32cChecksum, kQuickFoxMD5HashBad}, HashTestCase{StatusCode::kOk, - Options{} - .set(false) - .set(true), + Options{}.set( + storage::ChecksumAlgorithm::kMD5), kQuickFoxCrc32cChecksumBad, kQuickFoxMD5Hash}, HashTestCase{StatusCode::kOk, - Options{} - .set(false) - .set(false), + Options{}.set( + storage::ChecksumAlgorithm::kNone), + kQuickFoxCrc32cChecksumBad, kQuickFoxMD5HashBad}, + HashTestCase{StatusCode::kOk, + Options{}.set( + storage::ChecksumAlgorithm::kCrc32c), + kQuickFoxCrc32cChecksum, kQuickFoxMD5HashBad}, + HashTestCase{StatusCode::kOk, + Options{}.set( + storage::ChecksumAlgorithm::kMD5), + kQuickFoxCrc32cChecksumBad, kQuickFoxMD5Hash}, + HashTestCase{StatusCode::kOk, + Options{}.set( + storage::ChecksumAlgorithm::kNone), kQuickFoxCrc32cChecksumBad, kQuickFoxMD5HashBad}, // In the next three cases we verify that validation works when the // returned values are not correct. HashTestCase{StatusCode::kInvalidArgument, - Options{} - .set(false) - .set(true), + Options{}.set( + storage::ChecksumAlgorithm::kMD5), kQuickFoxCrc32cChecksumBad, kQuickFoxMD5HashBad}, HashTestCase{StatusCode::kInvalidArgument, - Options{} - .set(true) - .set(false), + Options{}.set( + storage::ChecksumAlgorithm::kCrc32c), kQuickFoxCrc32cChecksumBad, kQuickFoxMD5HashBad}, HashTestCase{StatusCode::kInvalidArgument, Options{} .set(true) .set(true), kQuickFoxCrc32cChecksumBad, kQuickFoxMD5HashBad}, + HashTestCase{StatusCode::kInvalidArgument, + Options{}.set( + storage::ChecksumAlgorithm::kCrc32c), + kQuickFoxCrc32cChecksumBad, kQuickFoxMD5HashBad}, + HashTestCase{StatusCode::kInvalidArgument, + Options{}.set( + storage::ChecksumAlgorithm::kMD5), + kQuickFoxCrc32cChecksumBad, kQuickFoxMD5HashBad}, // The application may know what the values should be. Verify the // validation works correctly when the application provides correct // values. HashTestCase{ StatusCode::kOk, Options{} - .set(true) - .set(true) + .set( + storage::ChecksumAlgorithm::kCrc32c) .set(kQuickFoxCrc32cChecksum) .set(BinaryMD5(kQuickFoxMD5Hash)), kQuickFoxCrc32cChecksum, kQuickFoxMD5Hash}, @@ -195,8 +225,8 @@ INSTANTIATE_TEST_SUITE_P( HashTestCase{ StatusCode::kInvalidArgument, Options{} - .set(true) - .set(true) + .set( + storage::ChecksumAlgorithm::kCrc32c) .set(kQuickFoxCrc32cChecksumBad) .set(BinaryMD5(kQuickFoxMD5Hash)), kQuickFoxCrc32cChecksum, kQuickFoxMD5Hash}, @@ -212,8 +242,8 @@ INSTANTIATE_TEST_SUITE_P( HashTestCase{ StatusCode::kInvalidArgument, Options{} - .set(true) - .set(true) + .set( + storage::ChecksumAlgorithm::kCrc32c) .set(kQuickFoxCrc32cChecksumBad) .set( BinaryMD5(kQuickFoxMD5HashBad)), diff --git a/google/cloud/storage/internal/async/connection_impl_upload_hash_test.cc b/google/cloud/storage/internal/async/connection_impl_upload_hash_test.cc index 9106e0fd3ba1b..1e444cdbb24cd 100644 --- a/google/cloud/storage/internal/async/connection_impl_upload_hash_test.cc +++ b/google/cloud/storage/internal/async/connection_impl_upload_hash_test.cc @@ -13,6 +13,17 @@ // limitations under the License. #include "google/cloud/storage/async/options.h" + +// TODO: Remove this when EnableMD5ValidationOption and +// EnableCrc32cValidationOption are removed. +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +#elif defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +#endif + #include "google/cloud/storage/async/writer_connection.h" #include "google/cloud/storage/internal/async/connection_impl.h" #include "google/cloud/storage/internal/async/default_options.h" @@ -69,15 +80,21 @@ auto ExpectedObjectChecksums(HashTestCase const& tc) { std::ostream& operator<<(std::ostream& os, HashTestCase const& rhs) { os << "HashTestCase={options={"; - os << std::boolalpha // - << "enable_crc32c_validation=" - << rhs.options.get(); + os << std::boolalpha; // + if (rhs.options.has()) { + os << "upload_checksum=" + << static_cast( + rhs.options.get()); + } else { + os << "enable_crc32c_validation=" + << rhs.options.get(); + os << ", enable_md5_validation=" + << rhs.options.get(); + } if (rhs.options.has()) { os << ", use_crc32_value=" << rhs.options.get(); } - os << ", enable_md5_validation=" - << rhs.options.get(); if (rhs.options.has()) { os << ", use_md5_value=" << rhs.options.get(); } @@ -121,17 +138,14 @@ INSTANTIATE_TEST_SUITE_P( .set(true) .set(true), kQuickFoxCrc32cChecksum, kQuickFoxMD5Hash}, - HashTestCase{Options{} - .set(true) - .set(false), + HashTestCase{Options{}.set( + storage::ChecksumAlgorithm::kCrc32c), kQuickFoxCrc32cChecksum, ""}, - HashTestCase{Options{} - .set(false) - .set(true), + HashTestCase{Options{}.set( + storage::ChecksumAlgorithm::kMD5), absl::nullopt, kQuickFoxMD5Hash}, - HashTestCase{Options{} - .set(false) - .set(false), + HashTestCase{Options{}.set( + storage::ChecksumAlgorithm::kNone), absl::nullopt, ""})); INSTANTIATE_TEST_SUITE_P( @@ -139,26 +153,25 @@ INSTANTIATE_TEST_SUITE_P( ::testing::Values( HashTestCase{ Options{} - .set(false) - .set(false) + .set( + storage::ChecksumAlgorithm::kNone) .set(kQuickFoxCrc32cChecksum) .set(BinaryMD5(kQuickFoxMD5Hash)), kQuickFoxCrc32cChecksum, kQuickFoxMD5Hash}, HashTestCase{ Options{} - .set(false) - .set(false) + .set( + storage::ChecksumAlgorithm::kNone) .set(kQuickFoxCrc32cChecksum), kQuickFoxCrc32cChecksum, ""}, HashTestCase{ Options{} - .set(false) - .set(false) + .set( + storage::ChecksumAlgorithm::kNone) .set(BinaryMD5(kQuickFoxMD5Hash)), absl::nullopt, kQuickFoxMD5Hash}, - HashTestCase{Options{} - .set(false) - .set(false), + HashTestCase{Options{}.set( + storage::ChecksumAlgorithm::kNone), absl::nullopt, ""})); TEST_P(AsyncConnectionImplUploadHashTest, StartUnbuffered) { diff --git a/google/cloud/storage/internal/async/default_options_test.cc b/google/cloud/storage/internal/async/default_options_test.cc index dc7e4e8588be8..03ac3f40ca2fd 100644 --- a/google/cloud/storage/internal/async/default_options_test.cc +++ b/google/cloud/storage/internal/async/default_options_test.cc @@ -13,6 +13,17 @@ // limitations under the License. #include "google/cloud/storage/internal/async/default_options.h" + +// TODO: Remove this when EnableMD5ValidationOption and +// EnableCrc32cValidationOption are removed. +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +#elif defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +#endif + #include "google/cloud/storage/async/idempotency_policy.h" #include "google/cloud/storage/async/options.h" #include "google/cloud/storage/async/resume_policy.h" diff --git a/google/cloud/storage/internal/async/object_descriptor_impl.cc b/google/cloud/storage/internal/async/object_descriptor_impl.cc index 4f145986521ad..8d4117cefa0bf 100644 --- a/google/cloud/storage/internal/async/object_descriptor_impl.cc +++ b/google/cloud/storage/internal/async/object_descriptor_impl.cc @@ -14,6 +14,7 @@ #include "google/cloud/storage/internal/async/object_descriptor_impl.h" #include "google/cloud/storage/async/options.h" +#include "google/cloud/storage/internal/async/checksum_helpers.h" #include "google/cloud/storage/internal/async/handle_redirect_error.h" #include "google/cloud/storage/internal/async/multi_stream_manager.h" #include "google/cloud/storage/internal/async/object_descriptor_reader_tracing.h" @@ -23,6 +24,7 @@ #include "google/cloud/storage/internal/hash_validator.h" #include "google/cloud/storage/internal/hash_validator_impl.h" #include "google/cloud/storage/internal/hash_values.h" +#include "google/cloud/storage/options.h" #include "google/cloud/grpc_error_delegate.h" #include "google/cloud/internal/opentelemetry.h" #include "google/rpc/status.pb.h" @@ -221,9 +223,9 @@ std::unique_ptr ObjectDescriptorImpl::Read( std::shared_ptr ObjectDescriptorImpl::CreateHashFunction(bool is_full_read) const { - auto const enable_crc32c = - options_.get(); - auto const enable_md5 = options_.get(); + auto const settings = GetDownloadChecksumSettings(options_); + auto const enable_crc32c = settings.enable_crc32c; + auto const enable_md5 = settings.enable_md5; if (enable_crc32c) { std::unique_ptr child; @@ -255,9 +257,9 @@ ObjectDescriptorImpl::CreateHashValidator(bool is_full_read) const { return storage::internal::CreateNullHashValidator(); } - auto const enable_crc32c = - options_.get(); - auto const enable_md5 = options_.get(); + auto const settings = GetDownloadChecksumSettings(options_); + auto const enable_crc32c = settings.enable_crc32c; + auto const enable_md5 = settings.enable_md5; std::unique_ptr hash_validator; if (enable_crc32c && enable_md5) { diff --git a/google/cloud/storage/internal/async/object_descriptor_impl_test.cc b/google/cloud/storage/internal/async/object_descriptor_impl_test.cc index 5423423b4a08d..821f8745a34f4 100644 --- a/google/cloud/storage/internal/async/object_descriptor_impl_test.cc +++ b/google/cloud/storage/internal/async/object_descriptor_impl_test.cc @@ -13,6 +13,17 @@ // limitations under the License. #include "google/cloud/storage/internal/async/object_descriptor_impl.h" + +// TODO: Remove this when EnableMD5ValidationOption and +// EnableCrc32cValidationOption are removed. +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +#elif defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +#endif + #include "google/cloud/mocks/mock_async_streaming_read_write_rpc.h" #include "google/cloud/storage/async/options.h" #include "google/cloud/storage/async/resume_policy.h" diff --git a/google/cloud/storage/internal/hash_function.h b/google/cloud/storage/internal/hash_function.h index da7699353739f..0e17442acce05 100644 --- a/google/cloud/storage/internal/hash_function.h +++ b/google/cloud/storage/internal/hash_function.h @@ -15,6 +15,7 @@ #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_HASH_FUNCTION_H #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_HASH_FUNCTION_H +#include "google/cloud/internal/disable_deprecation_warnings.inc" #include "google/cloud/storage/hashing_options.h" #include "google/cloud/storage/internal/hash_values.h" #include "google/cloud/storage/version.h" @@ -108,4 +109,5 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace cloud } // namespace google +#include "google/cloud/internal/diagnostics_pop.inc" #endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_HASH_FUNCTION_H diff --git a/google/cloud/storage/internal/object_requests.h b/google/cloud/storage/internal/object_requests.h index 6fd14a86b841b..ee5598d5d49c1 100644 --- a/google/cloud/storage/internal/object_requests.h +++ b/google/cloud/storage/internal/object_requests.h @@ -15,6 +15,7 @@ #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_OBJECT_REQUESTS_H #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_OBJECT_REQUESTS_H +#include "google/cloud/internal/disable_deprecation_warnings.inc" #include "google/cloud/storage/auto_finalize.h" #include "google/cloud/storage/download_options.h" #include "google/cloud/storage/hashing_options.h" @@ -645,4 +646,5 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace cloud } // namespace google +#include "google/cloud/internal/diagnostics_pop.inc" #endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_OBJECT_REQUESTS_H diff --git a/google/cloud/storage/options.h b/google/cloud/storage/options.h index f64f5d1f28e51..649c717e2c45e 100644 --- a/google/cloud/storage/options.h +++ b/google/cloud/storage/options.h @@ -70,6 +70,43 @@ struct CAPathOption { } // namespace internal +/** + * Supported checksum algorithms. + * + * @ingroup storage-options + */ +enum class ChecksumAlgorithm { + kNone, ///< Disable checksum validation + kCrc32c, ///< Use CRC32C for checksum validation + kMD5, ///< Use MD5 for checksum validation +}; + +/** + * Configure the checksum algorithm used for uploads. + * + * If set, the client computes (if necessary) and validates the checksum of + * an object during uploads. Set to `ChecksumAlgorithm::kNone` to disable + * checksum validation. + * + * @ingroup storage-options + */ +struct UploadChecksumValidationOption { + using Type = ChecksumAlgorithm; +}; + +/** + * Configure the checksum algorithm used for downloads. + * + * If set, the client computes (if necessary) and validates the checksum of + * an object during downloads. Set to `ChecksumAlgorithm::kNone` to disable + * checksum validation. + * + * @ingroup storage-options + */ +struct DownloadChecksumValidationOption { + using Type = ChecksumAlgorithm; +}; + /** * Configure the REST endpoint for the GCS client library. * @@ -325,6 +362,7 @@ using ClientOptionList = ::google::cloud::OptionList< MaximumCurlSocketRecvSizeOption, MaximumCurlSocketSendSizeOption, TransferStallTimeoutOption, RetryPolicyOption, BackoffPolicyOption, IdempotencyPolicyOption, CARootsFilePathOption, + UploadChecksumValidationOption, DownloadChecksumValidationOption, storage_experimental::HttpVersionOption>; GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END diff --git a/google/cloud/storage/parallel_upload.h b/google/cloud/storage/parallel_upload.h index 0d0e08db1b0ac..ae06bd1bbb6fe 100644 --- a/google/cloud/storage/parallel_upload.h +++ b/google/cloud/storage/parallel_upload.h @@ -15,6 +15,7 @@ #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_PARALLEL_UPLOAD_H #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_PARALLEL_UPLOAD_H +#include "google/cloud/internal/disable_deprecation_warnings.inc" #include "google/cloud/storage/client.h" #include "google/cloud/storage/internal/tuple_filter.h" #include "google/cloud/storage/object_stream.h" @@ -1217,4 +1218,5 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace cloud } // namespace google +#include "google/cloud/internal/diagnostics_pop.inc" #endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_PARALLEL_UPLOAD_H diff --git a/google/cloud/storage/tests/async_client_integration_test.cc b/google/cloud/storage/tests/async_client_integration_test.cc index 6827ec4ec8f57..71a54f181b229 100644 --- a/google/cloud/storage/tests/async_client_integration_test.cc +++ b/google/cloud/storage/tests/async_client_integration_test.cc @@ -1006,11 +1006,12 @@ TEST_F(AsyncClientIntegrationTest, Open) { TEST_F(AsyncClientIntegrationTest, OpenExceedMaximumRange) { if (!UsingEmulator()) GTEST_SKIP(); - auto async = - AsyncClient(TestOptions() - .set(1024) - .set(false) - .set(false)); + auto async = AsyncClient(TestOptions() + .set(1024) + .set( + storage::ChecksumAlgorithm::kNone) + .set( + storage::ChecksumAlgorithm::kNone)); auto client = MakeIntegrationTestClient(true, TestOptions()); auto object_name = MakeRandomObjectName(); @@ -1065,8 +1066,10 @@ TEST_F(AsyncClientIntegrationTest, OpenExceedMaximumRange) { TEST_F(AsyncClientIntegrationTest, OpenWithChecksumValidation) { if (!UsingEmulator()) GTEST_SKIP(); auto async = AsyncClient(TestOptions() - .set(true) - .set(true)); + .set( + storage::ChecksumAlgorithm::kCrc32c) + .set( + storage::ChecksumAlgorithm::kCrc32c)); auto client = MakeIntegrationTestClient(true, TestOptions()); auto object_name = MakeRandomObjectName();