diff --git a/google/cloud/storage/internal/grpc/object_request_parser.cc b/google/cloud/storage/internal/grpc/object_request_parser.cc index 1ee1d75d8a454..b2bd6db03df79 100644 --- a/google/cloud/storage/internal/grpc/object_request_parser.cc +++ b/google/cloud/storage/internal/grpc/object_request_parser.cc @@ -19,6 +19,7 @@ #include "google/cloud/storage/internal/grpc/object_metadata_parser.h" #include "google/cloud/storage/internal/object_access_control_parser.h" #include "google/cloud/storage/internal/patch_builder_details.h" +#include "google/cloud/internal/absl_str_cat_quiet.h" #include "google/cloud/internal/invoke_result.h" #include "google/cloud/internal/make_status.h" #include "google/cloud/internal/time_utils.h" @@ -377,24 +378,29 @@ StatusOr ToProto( if (request.HasOption()) { r.set_generation(request.GetOption().value()); } - if (request.HasOption()) { + auto const offset = request.GetOption().value_or(0); + if (!request.HasOption()) { + r.set_read_offset(offset); + } else { auto const range = request.GetOption().value(); - r.set_read_offset(range.begin); - r.set_read_limit(range.end - range.begin); + auto const merged_offset = (std::max)(range.begin, offset); + auto const merged_limit = range.end - merged_offset; + if (merged_limit <= 0) { + return internal::InvalidArgumentError( + absl::StrCat("Invalid gRPC range offset=", merged_offset, + ", limit=", merged_limit, ", from offset=", offset, + " range=[", range.begin, ",", range.end, "]"), + GCP_ERROR_INFO()); + } + + r.set_read_offset(merged_offset); + r.set_read_limit(merged_limit); } + if (request.HasOption()) { auto const offset = request.GetOption().value(); r.set_read_offset(-offset); } - if (request.HasOption()) { - auto const offset = request.GetOption().value(); - if (offset > r.read_offset()) { - if (r.read_limit() > 0) { - r.set_read_limit(offset - r.read_offset()); - } - r.set_read_offset(offset); - } - } SetGenerationConditions(r, request); SetMetagenerationConditions(r, request); diff --git a/google/cloud/storage/internal/grpc/object_request_parser_test.cc b/google/cloud/storage/internal/grpc/object_request_parser_test.cc index 4734d8f6e864b..3fc22c8ddc73f 100644 --- a/google/cloud/storage/internal/grpc/object_request_parser_test.cc +++ b/google/cloud/storage/internal/grpc/object_request_parser_test.cc @@ -367,6 +367,45 @@ TEST(GrpcObjectRequestParser, EXPECT_THAT(actual, StatusIs(StatusCode::kInvalidArgument)); } +TEST(GrpcObjectRequestParser, ReadObjectRangeMergeWithOffset) { + auto expected = google::storage::v2::ReadObjectRequest{}; + EXPECT_TRUE(TextFormat::ParseFromString( + R"pb( + bucket: "projects/_/buckets/test-bucket" + object: "test-object" + read_offset: 2000 + read_limit: 1000 + )pb", + &expected)); + auto request = + storage::internal::ReadObjectRangeRequest("test-bucket", "test-object") + .set_multiple_options(storage::ReadRange(2000, 3000)); + EXPECT_THAT(ToProto(request), IsOkAndHolds(IsProtoEqual(expected))); + + request.set_multiple_options(storage::ReadFromOffset(1900)); + EXPECT_THAT(ToProto(request), IsOkAndHolds(IsProtoEqual(expected))); + + request.set_multiple_options(storage::ReadFromOffset(2100)); + expected.set_read_offset(2100); + expected.set_read_limit(900); + EXPECT_THAT(ToProto(request), IsOkAndHolds(IsProtoEqual(expected))); + + request.set_multiple_options(storage::ReadFromOffset(2300)); + expected.set_read_offset(2300); + expected.set_read_limit(700); + EXPECT_THAT(ToProto(request), IsOkAndHolds(IsProtoEqual(expected))); +} + +TEST(GrpcObjectRequestParser, ReadOffsetEmptyRange) { + auto request = + storage::internal::ReadObjectRangeRequest("test-bucket", "test-object") + .set_multiple_options(storage::ReadFromOffset(3000), + storage::ReadRange(2000, 3000)); + + auto const actual = ToProto(request); + EXPECT_THAT(actual, StatusIs(StatusCode::kInvalidArgument)); +} + TEST(GrpcObjectRequestParser, PatchObjectRequestAllOptions) { auto constexpr kTextProto = R"pb( predefined_acl: "projectPrivate"