diff --git a/google/cloud/storage/internal/metadata_parser.cc b/google/cloud/storage/internal/metadata_parser.cc index 1c8a7607b7ba5..d5852c47e08c6 100644 --- a/google/cloud/storage/internal/metadata_parser.cc +++ b/google/cloud/storage/internal/metadata_parser.cc @@ -121,6 +121,17 @@ StatusOr ParseTimestampField( return google::cloud::internal::InvalidArgumentError(std::move(os).str(), GCP_ERROR_INFO()); } +StatusOr ParseStringField(nlohmann::json const& json, + char const* field_name) { + auto it = json.find(field_name); + if (it == json.end() || it->is_null()) return std::string{}; + if (it->is_string()) return it->get(); + + std::ostringstream os; + os << "Error parsing field <" << field_name << "> as a string, json=" << json; + return google::cloud::internal::InvalidArgumentError(std::move(os).str(), + GCP_ERROR_INFO()); +} Status NotJsonObject(nlohmann::json const& j, google::cloud::internal::ErrorInfoBuilder eib) { diff --git a/google/cloud/storage/internal/metadata_parser.h b/google/cloud/storage/internal/metadata_parser.h index 9b27181387635..7c00a847c39e2 100644 --- a/google/cloud/storage/internal/metadata_parser.h +++ b/google/cloud/storage/internal/metadata_parser.h @@ -80,6 +80,15 @@ StatusOr ParseUnsignedLongField(nlohmann::json const& json, StatusOr ParseTimestampField( nlohmann::json const& json, char const* field_name); +/** + * Parses a string field. + * + * @return the value of @p field_name in @p json, or an empty string if the + * field is not present. + */ +StatusOr ParseStringField(nlohmann::json const& json, + char const* field_name); + /// Return an error indicating that `j` is not a JSON object (it may be a /// string, or other valid JSON) Status NotJsonObject(nlohmann::json const& j, diff --git a/google/cloud/storage/internal/metadata_parser_test.cc b/google/cloud/storage/internal/metadata_parser_test.cc index a8845463e533a..0c85d6e89ec54 100644 --- a/google/cloud/storage/internal/metadata_parser_test.cc +++ b/google/cloud/storage/internal/metadata_parser_test.cc @@ -235,6 +235,52 @@ TEST(MetadataParserTest, ParseIntegralFieldInvalidFieldType) { CheckParseInvalidFieldType(&ParseUnsignedLongField); } +/// @test Verify that we parse string values in JSON objects. +TEST(MetadataParserTest, ParseStringField) { + std::string text = R"""({ + "field1": "value1", + "field2": "" + })"""; + auto json_object = nlohmann::json::parse(text); + EXPECT_EQ("value1", ParseStringField(json_object, "field1").value()); + EXPECT_EQ("", ParseStringField(json_object, "field2").value()); +} + +/// @test Verify that we parse missing string values in JSON objects. +TEST(MetadataParserTest, ParseMissingStringField) { + std::string text = R"""({ + "some-field": "some-value" + })"""; + auto json_object = nlohmann::json::parse(text); + auto actual = ParseStringField(json_object, "missing-field").value(); + EXPECT_EQ("", actual); +} + +/// @test Verify that we return an error for invalid string field types. +TEST(MetadataParserTest, ParseInvalidStringFieldType) { + std::string text = R"""({ + "field": 12345 + })"""; + auto json_object = nlohmann::json::parse(text); + + auto actual = ParseStringField(json_object, "field"); + EXPECT_THAT(actual, StatusIs(StatusCode::kInvalidArgument)); + EXPECT_THAT(actual.status().message(), + ::testing::HasSubstr("Error parsing field as a string")); +} + +/// @test Verify that we return an error for complex types where a string is +/// expected. +TEST(MetadataParserTest, ParseInvalidStringFieldTypeComplex) { + std::string text = R"""({ + "field": [1, 2, 3] + })"""; + auto json_object = nlohmann::json::parse(text); + + EXPECT_THAT(ParseStringField(json_object, "field"), + StatusIs(StatusCode::kInvalidArgument)); +} + TEST(MetadataParserTest, NotJsonObject) { EXPECT_THAT(NotJsonObject(nlohmann::json{}, GCP_ERROR_INFO()), StatusIs(StatusCode::kInvalidArgument)); diff --git a/google/cloud/storage/internal/object_metadata_parser.cc b/google/cloud/storage/internal/object_metadata_parser.cc index 0e97c194e8fb8..4f5e7efa20795 100644 --- a/google/cloud/storage/internal/object_metadata_parser.cc +++ b/google/cloud/storage/internal/object_metadata_parser.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -109,7 +110,19 @@ Status ParseMetadata(ObjectMetadata& meta, nlohmann::json const& json) { if (f == json.end()) return Status{}; std::map metadata; for (auto const& kv : f->items()) { - metadata.emplace(kv.key(), kv.value().get()); + if (!kv.value().is_string()) { + if (kv.value().is_null()) { + metadata.emplace(kv.key(), std::string{}); + } else { + std::ostringstream os; + os << "Error parsing field as a string, json=" << kv.value(); + return google::cloud::internal::InvalidArgumentError( + std::move(os).str(), GCP_ERROR_INFO()); + } + } else { + metadata.emplace(kv.key(), kv.value().get()); + } } meta.mutable_metadata() = std::move(metadata); return Status{}; @@ -204,6 +217,14 @@ Status ParseHardDeleteTime(ObjectMetadata& meta, nlohmann::json const& json) { return Status{}; } +Status SetStringField(ObjectMetadata& meta, nlohmann::json const& json, + char const* key, + ObjectMetadata& (ObjectMetadata::*setter)(std::string)) { + auto v = internal::ParseStringField(json, key); + if (!v) return std::move(v).status(); + (meta.*setter)(*std::move(v)); + return Status{}; +} } // namespace StatusOr ObjectMetadataParser::FromJson( @@ -214,78 +235,74 @@ StatusOr ObjectMetadataParser::FromJson( Parser parsers[] = { ParseAcl, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_bucket(json.value("bucket", "")); - return Status{}; + return SetStringField(meta, json, "bucket", + &ObjectMetadata::set_bucket); }, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_cache_control(json.value("cacheControl", "")); - return Status{}; + return SetStringField(meta, json, "cacheControl", + &ObjectMetadata::set_cache_control); }, ParseComponentCount, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_content_disposition(json.value("contentDisposition", "")); - return Status{}; + return SetStringField(meta, json, "contentDisposition", + &ObjectMetadata::set_content_disposition); }, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_content_encoding(json.value("contentEncoding", "")); - return Status{}; + return SetStringField(meta, json, "contentEncoding", + &ObjectMetadata::set_content_encoding); }, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_content_language(json.value("contentLanguage", "")); - return Status{}; + return SetStringField(meta, json, "contentLanguage", + &ObjectMetadata::set_content_language); }, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_content_type(json.value("contentType", "")); - return Status{}; + return SetStringField(meta, json, "contentType", + &ObjectMetadata::set_content_type); }, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_crc32c(json.value("crc32c", "")); - return Status{}; + return SetStringField(meta, json, "crc32c", + &ObjectMetadata::set_crc32c); }, ParseCustomTime, ParseCustomerEncryption, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_etag(json.value("etag", "")); - return Status{}; + return SetStringField(meta, json, "etag", &ObjectMetadata::set_etag); }, ParseEventBasedHold, ParseGeneration, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_id(json.value("id", "")); - return Status{}; + return SetStringField(meta, json, "id", &ObjectMetadata::set_id); }, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_kind(json.value("kind", "")); - return Status{}; + return SetStringField(meta, json, "kind", &ObjectMetadata::set_kind); }, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_kms_key_name(json.value("kmsKeyName", "")); - return Status{}; + return SetStringField(meta, json, "kmsKeyName", + &ObjectMetadata::set_kms_key_name); }, ParseMetageneration, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_md5_hash(json.value("md5Hash", "")); - return Status{}; + return SetStringField(meta, json, "md5Hash", + &ObjectMetadata::set_md5_hash); }, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_media_link(json.value("mediaLink", "")); - return Status{}; + return SetStringField(meta, json, "mediaLink", + &ObjectMetadata::set_media_link); }, ParseMetadata, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_name(json.value("name", "")); - return Status{}; + return SetStringField(meta, json, "name", &ObjectMetadata::set_name); }, ParseOwner, ParseRetentionExpirationTime, ParseRetention, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_self_link(json.value("selfLink", "")); - return Status{}; + return SetStringField(meta, json, "selfLink", + &ObjectMetadata::set_self_link); }, [](ObjectMetadata& meta, nlohmann::json const& json) { - meta.set_storage_class(json.value("storageClass", "")); - return Status{}; + return SetStringField(meta, json, "storageClass", + &ObjectMetadata::set_storage_class); }, ParseSize, ParseTemporaryHold,