diff --git a/google/cloud/storage/internal/object_read_streambuf.cc b/google/cloud/storage/internal/object_read_streambuf.cc index 7bc8ae3e27d3b..343dd9689f0ae 100644 --- a/google/cloud/storage/internal/object_read_streambuf.cc +++ b/google/cloud/storage/internal/object_read_streambuf.cc @@ -16,6 +16,7 @@ #include "google/cloud/storage/hash_mismatch_error.h" #include "google/cloud/storage/internal/hash_function.h" #include "google/cloud/internal/make_status.h" +#include "google/cloud/log.h" #include #include #include @@ -37,6 +38,18 @@ std::streamoff InitialOffset(ReadObjectRangeRequest const& request) { return request.StartingByte(); } +absl::optional ExpectedBytes( + ReadObjectRangeRequest const& request) { + if (request.HasOption()) { + auto const& range = request.GetOption().value(); + return range.end - range.begin; + } + if (request.HasOption()) { + return request.GetOption().value(); + } + return absl::nullopt; +} + } // namespace ObjectReadStreambuf::ObjectReadStreambuf( @@ -45,14 +58,20 @@ ObjectReadStreambuf::ObjectReadStreambuf( : source_(std::move(source)), source_pos_(InitialOffset(request)), hash_function_(CreateHashFunction(request)), - hash_validator_(CreateHashValidator(request)) {} + hash_validator_(CreateHashValidator(request)), + bucket_name_(request.bucket_name()), + object_name_(request.object_name()), + expected_bytes_(ExpectedBytes(request)) {} -ObjectReadStreambuf::ObjectReadStreambuf(ReadObjectRangeRequest const&, +ObjectReadStreambuf::ObjectReadStreambuf(ReadObjectRangeRequest const& request, Status status) : source_(new ObjectReadErrorSource(status)), source_pos_(-1), hash_validator_(CreateNullHashValidator()), - status_(std::move(status)) {} + status_(std::move(status)), + bucket_name_(request.bucket_name()), + object_name_(request.object_name()), + expected_bytes_(ExpectedBytes(request)) {} ObjectReadStreambuf::pos_type ObjectReadStreambuf::seekpos( pos_type /*pos*/, std::ios_base::openmode /*which*/) { @@ -77,6 +96,16 @@ ObjectReadStreambuf::pos_type ObjectReadStreambuf::seekoff( bool ObjectReadStreambuf::IsOpen() const { return source_->IsOpen(); } void ObjectReadStreambuf::Close() { + if (expected_bytes_.has_value() && total_bytes_received_ > *expected_bytes_) { + if (!transformation().has_value()) { + GCP_LOG(WARNING) << "storage: received " + << (total_bytes_received_ - *expected_bytes_) + << " more bytes than requested from GCS for bucket " + << bucket_name_ << ", object " << object_name_; + } + expected_bytes_ = absl::nullopt; + } + auto response = source_->Close(); if (!response.ok()) { ReportError(std::move(response).status()); @@ -224,6 +253,8 @@ std::streamsize ObjectReadStreambuf::xsgetn(char* s, std::streamsize count) { if (!size_) size_ = std::move(read->size); if (!transformation_) transformation_ = std::move(read->transformation); + total_bytes_received_ += read->bytes_received; + if (source_pos_ >= 0) { source_pos_ += static_cast(read->bytes_received); } else if (size_) { diff --git a/google/cloud/storage/internal/object_read_streambuf.h b/google/cloud/storage/internal/object_read_streambuf.h index cfdb1fe5b44f9..eb307b282bed7 100644 --- a/google/cloud/storage/internal/object_read_streambuf.h +++ b/google/cloud/storage/internal/object_read_streambuf.h @@ -106,6 +106,10 @@ class ObjectReadStreambuf : public std::basic_streambuf { absl::optional storage_class_; absl::optional size_; absl::optional transformation_; + std::string bucket_name_; + std::string object_name_; + absl::optional expected_bytes_; + std::int64_t total_bytes_received_ = 0; }; } // namespace internal diff --git a/google/cloud/storage/internal/object_read_streambuf_test.cc b/google/cloud/storage/internal/object_read_streambuf_test.cc index 47f2fc144364d..cd3585204d7ec 100644 --- a/google/cloud/storage/internal/object_read_streambuf_test.cc +++ b/google/cloud/storage/internal/object_read_streambuf_test.cc @@ -14,8 +14,8 @@ #include "google/cloud/storage/internal/object_read_streambuf.h" #include "google/cloud/storage/testing/mock_client.h" +#include "google/cloud/testing_util/scoped_log.h" #include -#include #include #include @@ -173,6 +173,84 @@ TEST(ObjectReadStreambufTest, WrongSeek) { EXPECT_TRUE(stream.fail()); } +TEST(ObjectReadStreambufTest, LogsWarningOnExcessBytes) { + testing_util::ScopedLog log; + auto read_source = std::make_unique(); + EXPECT_CALL(*read_source, IsOpen()).WillRepeatedly(Return(true)); + EXPECT_CALL(*read_source, Read).WillOnce(Return(ReadSourceResult{15, {}})); + EXPECT_CALL(*read_source, Close()) + .WillOnce(Return(HttpResponse{200, "OK", {}})); + + ObjectReadStreambuf buf( + ReadObjectRangeRequest("test-bucket", "test-object") + .set_option(ReadRange(0, 10)), + std::move(read_source)); + + std::istream stream(&buf); + std::vector v(1024); + stream.read(v.data(), 15); + buf.Close(); + + auto const log_lines = log.ExtractLines(); + EXPECT_THAT( + log_lines, + testing::Contains(testing::HasSubstr( + "storage: received 5 more bytes than requested from GCS for bucket " + "test-bucket, object test-object"))); +} + +TEST(ObjectReadStreambufTest, NoWarningWhenExactBytesReceived) { + testing_util::ScopedLog log; + auto read_source = std::make_unique(); + EXPECT_CALL(*read_source, IsOpen()).WillRepeatedly(Return(true)); + EXPECT_CALL(*read_source, Read).WillOnce(Return(ReadSourceResult{10, {}})); + EXPECT_CALL(*read_source, Close()) + .WillOnce(Return(HttpResponse{200, "OK", {}})); + + ObjectReadStreambuf buf( + ReadObjectRangeRequest("test-bucket", "test-object") + .set_option(ReadRange(0, 10)), + std::move(read_source)); + + std::istream stream(&buf); + std::vector v(1024); + stream.read(v.data(), 10); + buf.Close(); + + auto const log_lines = log.ExtractLines(); + EXPECT_THAT( + log_lines, + testing::Not(testing::Contains(testing::HasSubstr( + "more bytes than requested from GCS for bucket")))); +} + +TEST(ObjectReadStreambufTest, NoWarningWhenDecompressed) { + testing_util::ScopedLog log; + auto read_source = std::make_unique(); + EXPECT_CALL(*read_source, IsOpen()).WillRepeatedly(Return(true)); + auto response = ReadSourceResult{15, {}}; + response.transformation = "gunzipped"; + EXPECT_CALL(*read_source, Read).WillOnce(Return(response)); + EXPECT_CALL(*read_source, Close()) + .WillOnce(Return(HttpResponse{200, "OK", {}})); + + ObjectReadStreambuf buf( + ReadObjectRangeRequest("test-bucket", "test-object") + .set_option(ReadRange(0, 10)), + std::move(read_source)); + + std::istream stream(&buf); + std::vector v(1024); + stream.read(v.data(), 15); + buf.Close(); + + auto const log_lines = log.ExtractLines(); + EXPECT_THAT( + log_lines, + testing::Not(testing::Contains(testing::HasSubstr( + "more bytes than requested from GCS for bucket")))); +} + } // namespace } // namespace internal GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END