feat(storage): Add full object checksum validation at finalization for json path

google/cloud/storage/internal/object_requests.h

@@ -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_); }

google/cloud/storage/internal/object_write_streambuf.cc

@@ -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);

google/cloud/storage/internal/object_write_streambuf_test.cc

@@ -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

google/cloud/storage/internal/rest/stub.cc

@@ -770,12 +770,23 @@ 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) {
     return (code != rest::HttpStatusCode::kResumeIncomplete &&

google/cloud/storage/internal/rest/stub_test.cc

@@ -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"
@@ -45,6 +46,31 @@ 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");
@@ -921,6 +947,90 @@ 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-crc32")))),
+          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