Skip to content
Open
2 changes: 1 addition & 1 deletion ci/cloudbuild/builds/lib/integration.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ source module ci/lib/io.sh
export PATH="${HOME}/.local/bin:${PATH}"
python3 -m pip uninstall -y --quiet googleapis-storage-testbench
python3 -m pip install --upgrade --user --quiet --disable-pip-version-check \
"git+https://github.com/googleapis/storage-testbench@v0.59.0"
"git+https://github.com/googleapis/storage-testbench@v0.60.0"

# Some of the tests will need a valid roots.pem file.
rm -f /dev/shm/roots.pem
Expand Down
16 changes: 14 additions & 2 deletions google/cloud/storage/internal/grpc/object_request_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -851,6 +851,15 @@ Status Finalize(google::storage::v2::WriteObjectRequest& write_request,
Merge(std::move(hashes), hash_function.Finish()));
}

Status Finalize(google::storage::v2::WriteObjectRequest& write_request,
grpc::WriteOptions& options,
storage::internal::HashValues hashes) {
write_request.set_finish_write(true);
options.set_last_message();
return FinalizeChecksums(*write_request.mutable_object_checksums(),
std::move(hashes));
}

Status Finalize(google::storage::v2::BidiWriteObjectRequest& write_request,
grpc::WriteOptions& options,
storage::internal::HashFunction& hash_function,
Expand Down Expand Up @@ -879,8 +888,11 @@ Status MaybeFinalize(google::storage::v2::WriteObjectRequest& write_request,
bool chunk_has_more) {
if (!chunk_has_more) options.set_last_message();
if (!request.last_chunk() || chunk_has_more) return {};
return Finalize(write_request, options, request.hash_function(),
request.known_object_hashes());
if (request.hash_function_ptr()) {
return Finalize(write_request, options, request.hash_function(),
request.known_object_hashes());
}
return Finalize(write_request, options, request.known_object_hashes());
}

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand Down
4 changes: 4 additions & 0 deletions google/cloud/storage/internal/grpc/object_request_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ Status Finalize(google::storage::v2::WriteObjectRequest& write_request,
storage::internal::HashFunction& hash_function,
storage::internal::HashValues = {});

Status Finalize(google::storage::v2::WriteObjectRequest& write_request,
grpc::WriteOptions& options,
storage::internal::HashValues hashes);

Status Finalize(google::storage::v2::BidiWriteObjectRequest& write_request,
grpc::WriteOptions& options,
storage::internal::HashFunction& hash_function,
Expand Down
4 changes: 3 additions & 1 deletion google/cloud/storage/internal/grpc/stub.cc
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,9 @@ StatusOr<storage::internal::QueryResumableUploadResponse> GrpcStub::UploadChunk(
auto& data = *proto_request.mutable_checksummed_data();
SetMutableContent(data, splitter.Next());
data.set_crc32c(Crc32c(GetContent(data)));
request.hash_function().Update(offset, GetContent(data), data.crc32c());
if (request.hash_function_ptr()) {
request.hash_function().Update(offset, GetContent(data), data.crc32c());
}
offset += GetContent(data).size();

auto wopts = grpc::WriteOptions();
Expand Down
5 changes: 4 additions & 1 deletion google/cloud/storage/internal/object_requests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,10 @@ UploadChunkRequest UploadChunkRequest::RemainingChunk(
HashValues FinishHashes(UploadChunkRequest const& request) {
// Prefer the hashes provided via *Value options in the request. If those
// are not set, use the computed hashes from the data.
return Merge(request.known_object_hashes(), request.hash_function().Finish());
if (auto hf = request.hash_function_ptr()) {
return Merge(request.known_object_hashes(), hf->Finish());
}
return request.known_object_hashes();
}

std::ostream& operator<<(std::ostream& os, UploadChunkRequest const& r) {
Expand Down
3 changes: 3 additions & 0 deletions google/cloud/storage/internal/object_requests.h
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,9 @@ class UploadChunkRequest
HashValues const& known_object_hashes() const { return known_object_hashes_; }

HashFunction& hash_function() const { return *hash_function_; }
std::shared_ptr<HashFunction> hash_function_ptr() const {
return hash_function_;
}

bool last_chunk() const { return upload_size_.has_value(); }
std::size_t payload_size() const { return TotalBytes(payload_); }
Expand Down
12 changes: 8 additions & 4 deletions google/cloud/storage/internal/object_write_streambuf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,22 +144,26 @@ void ObjectWriteStreambuf::FlushFinal() {

// Calculate the portion of the buffer that needs to be uploaded, if any.
auto const actual_size = put_area_size();
HashValues final_hashes = known_hashes_;
if (hash_function_) {
hash_function_->Update(committed_size_, {pbase(), actual_size});
final_hashes = hash_function_->Finish();
hash_function_.reset();
}

// After this point the session will be closed, and no more calls to the hash
// function are possible.
auto upload_request = UploadChunkRequest(upload_id_, committed_size_,
{ConstBuffer(pbase(), actual_size)},
hash_function_, known_hashes_);
hash_function_, final_hashes);
request_.ForEachOption(internal::CopyCommonOptions(upload_request));
OptionsSpan const span(span_options_);
auto response = connection_->UploadChunk(upload_request);
if (!response) {
last_status_ = std::move(response).status();
return;
}

auto function = std::move(hash_function_);
hash_values_ = std::move(*function).Finish();
hash_values_ = final_hashes;

committed_size_ = response->committed_size.value_or(0);
metadata_ = std::move(response->payload);
Expand Down
31 changes: 31 additions & 0 deletions google/cloud/storage/internal/object_write_streambuf_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,37 @@ TEST(ObjectWriteStreambufTest, WriteObjectWithCustomHeader) {
EXPECT_STATUS_OK(response);
}

/// @test Verify that hashes are computed and passed in FlushFinal.
TEST(ObjectWriteStreambufTest, FlushFinalWithHashes) {
auto mock = std::make_unique<testing::MockClient>();
auto const quantum = UploadChunkRequest::kChunkSizeQuantum;
std::string const payload = "small test payload";

EXPECT_CALL(*mock, UploadChunk).WillOnce([&](UploadChunkRequest const& r) {
EXPECT_EQ(payload.size(), r.payload_size());
EXPECT_EQ(0, r.offset());
EXPECT_TRUE(r.last_chunk());
EXPECT_EQ(r.hash_function_ptr(), nullptr);
EXPECT_EQ(r.known_object_hashes().crc32c, ComputeCrc32cChecksum(payload));
EXPECT_EQ(r.known_object_hashes().md5, ComputeMD5Hash(payload));
return QueryResumableUploadResponse{payload.size(), ObjectMetadata()};
});

ResumableUploadRequest request;
request.set_option(DisableCrc32cChecksum(false));
request.set_option(DisableMD5Hash(false));
ObjectWriteStreambuf streambuf(
std::move(mock), request, "test-only-upload-id",
/*committed_size=*/0, absl::nullopt, /*max_buffer_size=*/quantum,
CreateHashFunction(Crc32cChecksumValue(), DisableCrc32cChecksum(false),
MD5HashValue(), DisableMD5Hash(false)),
HashValues{}, CreateHashValidator(request), AutoFinalizeConfig::kEnabled);

streambuf.sputn(payload.data(), payload.size());
auto response = streambuf.Close();
EXPECT_STATUS_OK(response);
}

} // namespace
} // namespace internal
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand Down
21 changes: 16 additions & 5 deletions google/cloud/storage/internal/rest/stub.cc
Original file line number Diff line number Diff line change
Expand Up @@ -770,11 +770,22 @@ StatusOr<QueryResumableUploadResponse> RestStub::UploadChunk(
// default (at least in this case), and that wastes bandwidth as the content
// length is known.
builder.AddHeader("Transfer-Encoding", {});
auto offset = request.offset();
for (auto const& b : request.payload()) {
request.hash_function().Update(offset,
absl::string_view{b.data(), b.size()});
offset += b.size();
auto hash_function = request.hash_function_ptr();
if (hash_function) {
auto offset = request.offset();
for (auto const& b : request.payload()) {
hash_function->Update(offset, absl::string_view{b.data(), b.size()});
offset += b.size();
}
}
if (request.last_chunk()) {
auto const& hashes = request.known_object_hashes();
if (!hashes.crc32c.empty()) {
builder.AddHeader("x-goog-hash", "crc32c=" + hashes.crc32c);
}
if (!hashes.md5.empty()) {
builder.AddHeader("x-goog-hash", "md5=" + hashes.md5);
}
}

auto failure_predicate = [](rest::HttpStatusCode code) {
Expand Down
110 changes: 110 additions & 0 deletions google/cloud/storage/internal/rest/stub_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

#include "google/cloud/storage/internal/rest/stub.h"
#include "google/cloud/storage/internal/hash_function.h"
#include "google/cloud/storage/testing/canonical_errors.h"
#include "google/cloud/internal/api_client_header.h"
#include "google/cloud/testing_util/mock_rest_client.h"
Expand Down Expand Up @@ -45,6 +46,29 @@ using ::testing::Pair;
using ::testing::ResultOf;
using ::testing::Return;

class NoOpHashFunction : public HashFunction {
public:
std::string Name() const override { return "NoOp"; }
void Update(absl::string_view b) override { Cormorant(b); }
Status Update(std::int64_t o, absl::string_view b) override {
Cormorant(o, b);
return Status{};
}
Status Update(std::int64_t o, absl::string_view b, std::uint32_t c) override {
Cormorant(o, b, c);
return Status{};
}
Status Update(std::int64_t o, absl::Cord const& b, std::uint32_t c) override {
Cormorant(o, b, c);
return Status{};
}
HashValues Finish() override { return {}; }

private:
template <typename... Args>
void Cormorant(Args const&...) {}
};

TEST(RestStubTest, ResolveStorageAuthorityProdEndpoint) {
auto options =
Options{}.set<RestEndpointOption>("https://storage.googleapis.com");
Expand Down Expand Up @@ -921,6 +945,92 @@ TEST(RestStubTest, DeleteNotification) {
StatusIs(PermanentError().code(), PermanentError().message()));
}

TEST(RestStubTest, UploadChunkLastChunkWithCrc32c) {
auto mock = std::make_shared<MockRestClient>();
EXPECT_CALL(
*mock,
Put(ExpectedContext(),
ResultOf(
"request headers contain x-goog-hash with crc32c",
[](RestRequest const& r) { return r.headers(); },
Contains(Pair("x-goog-hash", ElementsAre("crc32c=test-crc32c")))),
ExpectedPayload()))
.WillOnce(Return(PermanentError()));
auto tested = std::make_unique<RestStub>(Options{}, mock, mock);
auto context = TestContext();
auto status = tested->UploadChunk(
context, TestOptions(),
UploadChunkRequest("test-url", 0, {},
std::make_shared<NoOpHashFunction>(),
{"test-crc32c", ""}));
EXPECT_THAT(status,
StatusIs(PermanentError().code(), PermanentError().message()));
}

TEST(RestStubTest, UploadChunkLastChunkWithMd5) {
auto mock = std::make_shared<MockRestClient>();
EXPECT_CALL(
*mock,
Put(ExpectedContext(),
ResultOf(
"request headers contain x-goog-hash with md5",
[](RestRequest const& r) { return r.headers(); },
Contains(Pair("x-goog-hash", ElementsAre("md5=test-md5")))),
ExpectedPayload()))
.WillOnce(Return(PermanentError()));
auto tested = std::make_unique<RestStub>(Options{}, mock, mock);
auto context = TestContext();
auto status = tested->UploadChunk(
context, TestOptions(),
UploadChunkRequest("test-url", 0, {},
std::make_shared<NoOpHashFunction>(),
{"", "test-md5"}));
EXPECT_THAT(status,
StatusIs(PermanentError().code(), PermanentError().message()));
}

TEST(RestStubTest, UploadChunkLastChunkWithBoth) {
auto mock = std::make_shared<MockRestClient>();
EXPECT_CALL(
*mock,
Put(ExpectedContext(),
ResultOf(
"request headers contain x-goog-hash with crc32c and md5",
[](RestRequest const& r) { return r.headers(); },
Contains(Pair("x-goog-hash", ElementsAre("crc32c=test-crc32c",
"md5=test-md5")))),
ExpectedPayload()))
.WillOnce(Return(PermanentError()));
auto tested = std::make_unique<RestStub>(Options{}, mock, mock);
auto context = TestContext();
auto status = tested->UploadChunk(
context, TestOptions(),
UploadChunkRequest("test-url", 0, {},
std::make_shared<NoOpHashFunction>(),
{"test-crc32c", "test-md5"}));
EXPECT_THAT(status,
StatusIs(PermanentError().code(), PermanentError().message()));
}

TEST(RestStubTest, UploadChunkIntermediate) {
auto mock = std::make_shared<MockRestClient>();
EXPECT_CALL(*mock, Put(ExpectedContext(),
ResultOf(
"request headers do not contain x-goog-hash",
[](RestRequest const& r) { return r.headers(); },
Not(Contains(Pair("x-goog-hash", _)))),
ExpectedPayload()))
.WillOnce(Return(PermanentError()));
auto tested = std::make_unique<RestStub>(Options{}, mock, mock);
auto context = TestContext();
auto status = tested->UploadChunk(
context, TestOptions(),
UploadChunkRequest("test-url", 0, {},
std::make_shared<NoOpHashFunction>()));
EXPECT_THAT(status,
StatusIs(PermanentError().code(), PermanentError().message()));
}

} // namespace
} // namespace internal
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand Down
68 changes: 52 additions & 16 deletions google/cloud/storage/tests/object_checksum_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,8 @@ TEST_F(ObjectChecksumIntegrationTest, WriteObjectDefault) {
EXPECT_THAT(os.computed_hash(),
HasSubstr(ComputeCrc32cChecksum(LoremIpsum())));
if (meta->has_metadata("x_emulator_upload")) {
if (UsingGrpc()) {
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_crc32c", _)));
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_no_md5", _)));
} else {
// Streaming uploads over REST cannot include checksums
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_no_crc32c", _)));
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_no_md5", _)));
}
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_crc32c", _)));
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_no_md5", _)));
}
}

Expand Down Expand Up @@ -200,14 +194,8 @@ TEST_F(ObjectChecksumIntegrationTest, WriteObjectExplicitEnable) {
EXPECT_THAT(os.computed_hash(),
HasSubstr(ComputeCrc32cChecksum(LoremIpsum())));
if (meta->has_metadata("x_emulator_upload")) {
if (UsingGrpc()) {
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_crc32c", _)));
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_no_md5", _)));
} else {
// Streaming uploads over REST cannot include checksums
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_no_crc32c", _)));
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_no_md5", _)));
}
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_crc32c", _)));
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_no_md5", _)));
}
}

Expand Down Expand Up @@ -289,6 +277,54 @@ TEST_F(ObjectChecksumIntegrationTest, WriteObjectUploadBadChecksum) {
ASSERT_THAT(stream.metadata(), Not(IsOk()));
}

/// @test Verify that full object checksums are sent in the final chunk.
TEST_F(ObjectChecksumIntegrationTest, WriteObjectWithFullChecksumValidation) {
auto client = MakeIntegrationTestClient();
auto object_name = MakeRandomObjectName();
auto content = LoremIpsum();
auto expected_crc32c = ComputeCrc32cChecksum(content);

auto os = client.WriteObject(bucket_name_, object_name,
DisableCrc32cChecksum(false),
DisableMD5Hash(true), IfGenerationMatch(0));
os << content;
os.Close();
auto meta = os.metadata();
ASSERT_STATUS_OK(meta);
ScheduleForDelete(*meta);

EXPECT_EQ(os.computed_hash(), expected_crc32c);

if (meta->has_metadata("x_emulator_upload")) {
EXPECT_THAT(meta->metadata(),
Contains(Pair("x_emulator_crc32c", expected_crc32c)));
EXPECT_THAT(meta->metadata(), Contains(Pair("x_emulator_no_md5", "true")));
}
}

/// @test Verify that the upload fails when the provided CRC32C checksum does
/// not match the data.
TEST_F(ObjectChecksumIntegrationTest, WriteObjectWithIncorrectChecksumValue) {
// TODO(#14385) - the emulator does not support this feature for gRPC.
if (UsingEmulator() && UsingGrpc()) GTEST_SKIP();
auto client = MakeIntegrationTestClient();
auto object_name = MakeRandomObjectName();
auto content = LoremIpsum();

auto bad_crc32c =
ComputeCrc32cChecksum("this is not the data being uploaded");

auto os = client.WriteObject(bucket_name_, object_name,
Crc32cChecksumValue(bad_crc32c),
DisableMD5Hash(true), IfGenerationMatch(0));

os << content;
os.Close();
EXPECT_TRUE(os.bad());
auto meta = os.metadata();
EXPECT_THAT(meta, Not(IsOk()));
}

/// @test Verify that CRC32C checksums are computed by default on downloads.
TEST_F(ObjectChecksumIntegrationTest, ReadObjectDefault) {
// TODO(#14385) - the emulator does not support this feature for gRPC.
Expand Down
Loading