diff --git a/garnet/lib/cmx/cmx_unittest.cc b/garnet/lib/cmx/cmx_unittest.cc index 6b9399abfc29ccafba524867c34d0dc3c51c7433..303a9f53756de38e0ba66225cd81e78547acbf9d 100644 --- a/garnet/lib/cmx/cmx_unittest.cc +++ b/garnet/lib/cmx/cmx_unittest.cc @@ -111,7 +111,7 @@ TEST_F(CmxMetadataTest, ParseWithErrors) { ExpectFailedParse(R"JSON({ "sandbox" : 3})JSON", "'sandbox' is not an object."); ExpectFailedParse(R"JSON({ "sandbox" : {"dev": "notarray"} })JSON", - "'dev' in sandbox is not an array."); + "'dev' is not an array."); ExpectFailedParse(R"JSON({ "runner" : 3 })JSON", "'runner' is not a string."); ExpectFailedParse(R"JSON({ "program" : { "binary": 3 } })JSON", "'binary' in program is not a string."); diff --git a/garnet/lib/cmx/program.cc b/garnet/lib/cmx/program.cc index 29cd89fdac82c1a61bc092c690ec872f64951ae7..25c75f2d4712f4d8296db6f54338c41b11313941 100644 --- a/garnet/lib/cmx/program.cc +++ b/garnet/lib/cmx/program.cc @@ -54,19 +54,8 @@ bool ProgramMetadata::ParseBinary(const rapidjson::Value& program_value, const auto args = program_value.FindMember(kArgs); if (args != program_value.MemberEnd()) { - if (!args->value.IsArray()) { - json_parser->ReportError("'args' in program is not an array."); - } else { - for (const auto& entry : args->value.GetArray()) { - if (!entry.IsString()) { - json_parser->ReportError( - "'args' in program contains an item that's not a string."); - } else { - args_.push_back(entry.GetString()); - } - } - args_null_ = false; - } + json_parser->CopyStringArray("args", args->value, &args_); + args_null_ = false; } return !json_parser->HasError(); diff --git a/garnet/lib/cmx/program_unittest.cc b/garnet/lib/cmx/program_unittest.cc index 737e1f8e13c7be055927e45d803a7e9ea3b857e4..7c4b274f204a3e4a4e82da26175147c10dea757b 100644 --- a/garnet/lib/cmx/program_unittest.cc +++ b/garnet/lib/cmx/program_unittest.cc @@ -77,7 +77,7 @@ TEST_F(ProgramMetadataTest, ParseBinaryArgsWithErrors) { &program, R"JSON({ "binary": "bin/app", "args": [0, 1] })JSON", &error)); EXPECT_THAT(error, ::testing::HasSubstr( - "'args' in program contains an item that's not a string")); + "'args' contains an item that's not a string")); } TEST_F(ProgramMetadataTest, ParseData) { diff --git a/garnet/lib/cmx/sandbox.cc b/garnet/lib/cmx/sandbox.cc index 6882a1bd93009868197d1c2a5beca4d496f7ca15..23205a1fc7322ce788faa75ba388ea79caf82b1a 100644 --- a/garnet/lib/cmx/sandbox.cc +++ b/garnet/lib/cmx/sandbox.cc @@ -12,28 +12,6 @@ #include "src/lib/fxl/strings/substitute.h" namespace component { -namespace { - -template <typename Value> -void CopyArrayToVector(const std::string& name, const Value& value, - json::JSONParser* json_parser, - std::vector<std::string>* vec) { - if (!value.IsArray()) { - json_parser->ReportError( - fxl::Substitute("'$0' in sandbox is not an array.", name)); - return; - } - for (const auto& entry : value.GetArray()) { - if (!entry.IsString()) { - json_parser->ReportError( - fxl::Substitute("Entry for '$0' in sandbox is not a string.", name)); - return; - } - vec->push_back(entry.GetString()); - } -} - -} // namespace constexpr char kDev[] = "dev"; constexpr char kSystem[] = "system"; @@ -63,7 +41,7 @@ bool SandboxMetadata::Parse(const rapidjson::Value& sandbox_value, auto* vec = entry.second; auto member = sandbox_value.FindMember(name); if (member != sandbox_value.MemberEnd()) { - CopyArrayToVector(name, member->value, json_parser, vec); + json_parser->CopyStringArray(name, member->value, vec); } } diff --git a/garnet/lib/cmx/sandbox_unittest.cc b/garnet/lib/cmx/sandbox_unittest.cc index 1bf7c17b1e8e7833c4588fefcd23550564cbfd69..68b41ab503d4257a27856d40c74142be588296ab 100644 --- a/garnet/lib/cmx/sandbox_unittest.cc +++ b/garnet/lib/cmx/sandbox_unittest.cc @@ -6,9 +6,9 @@ #include <string> -#include "lib/json/json_parser.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "lib/json/json_parser.h" #include "rapidjson/document.h" namespace component { @@ -128,7 +128,7 @@ TEST_F(SandboxMetadataTest, ParseWithErrors) { R"JSON({ "dev": [ "class/input", 3 ] })JSON", - "Entry for 'dev' in sandbox is not a string."); + "'dev' contains an item that's not a string"); } } // namespace diff --git a/garnet/public/lib/json/json_parser.cc b/garnet/public/lib/json/json_parser.cc index 526fdc45bd8afde866002c6b6a582b8bbae13c93..333de9fbca8a233549f2b0685e86b4cb1f26814a 100644 --- a/garnet/public/lib/json/json_parser.cc +++ b/garnet/public/lib/json/json_parser.cc @@ -121,6 +121,25 @@ void JSONParser::ParseFromDirectory( } } +void JSONParser::CopyStringArray(const std::string& name, + const rapidjson::Value& value, + std::vector<std::string>* out) { + out->clear(); + if (!value.IsArray()) { + ReportError(fxl::StringPrintf("'%s' is not an array.", name.c_str())); + return; + } + for (const auto& entry : value.GetArray()) { + if (!entry.IsString()) { + ReportError( + fxl::StringPrintf("'%s' contains an item that's not a string", name.c_str())); + out->clear(); + return; + } + out->push_back(entry.GetString()); + } +} + void JSONParser::ReportError(const std::string& error) { ReportErrorInternal(0, error); } diff --git a/garnet/public/lib/json/json_parser.h b/garnet/public/lib/json/json_parser.h index 3b3de0fadc3a7a3c56caa5b43cd05f7fec028a25..68bc341780c7ea0cb69dd96b74f17fd43caa0671 100644 --- a/garnet/public/lib/json/json_parser.h +++ b/garnet/public/lib/json/json_parser.h @@ -54,8 +54,15 @@ class JSONParser { void ParseFromDirectory(const std::string& path, fit::function<void(rapidjson::Document)> cb); + // Copies the string values from a |name|d |value| to the |out| vector. + // Clears |out| and calls ReportError() if |value| does not refer to an array, + // or if the array contains non-string values. + void CopyStringArray(const std::string& name, const rapidjson::Value& value, + std::vector<std::string>* out); + // Returns true if there was an error initializing the document. bool HasError() const; + // If HasError() is true, returns a human-readable string describing the // error(s) initializing the document. std::string error_str() const; diff --git a/garnet/public/lib/json/json_parser_unittest.cc b/garnet/public/lib/json/json_parser_unittest.cc index baff315b71741afe3ce047e0169691abb8f896aa..da85b9751635254306fed8fb6c0c85864609b728 100644 --- a/garnet/public/lib/json/json_parser_unittest.cc +++ b/garnet/public/lib/json/json_parser_unittest.cc @@ -250,5 +250,61 @@ TEST_F(JSONParserTest, ParseFromDirectoryWithErrors) { EXPECT_EQ(1, props_found_); } +TEST_F(JSONParserTest, CopyArrayToVectorNonArray) { + const std::string json = R"JSON({ + "foo": 0 + })JSON"; + JSONParser parser; + auto document = parser.ParseFromString(json, "test_file"); + ASSERT_FALSE(parser.HasError()); + std::vector<std::string> vec = {"foo"}; + parser.CopyStringArray("foo", document["foo"], &vec); + EXPECT_TRUE(parser.HasError()); + EXPECT_EQ(parser.error_str(), "test_file: 'foo' is not an array."); + EXPECT_EQ(vec.size(), 0u); +} + +TEST_F(JSONParserTest, CopyArrayToVectorNonString) { + const std::string json = R"JSON({ + "bar": [ "1", 2, "3" ] + })JSON"; + JSONParser parser; + auto document = parser.ParseFromString(json, "test_file"); + ASSERT_FALSE(parser.HasError()); + std::vector<std::string> vec = {"foo"}; + parser.CopyStringArray("bar", document["bar"], &vec); + EXPECT_TRUE(parser.HasError()); + EXPECT_EQ(parser.error_str(), "test_file: 'bar' contains an item that's not a string"); + EXPECT_EQ(vec.size(), 0u); +} + +TEST_F(JSONParserTest, CopyArrayToVectorEmpty) { + const std::string json = R"JSON({ + "baz": [] + })JSON"; + JSONParser parser; + auto document = parser.ParseFromString(json, "test_file"); + ASSERT_FALSE(parser.HasError()); + std::vector<std::string> vec = {"foo"}; + parser.CopyStringArray("baz", document["baz"], &vec); + EXPECT_FALSE(parser.HasError()); + EXPECT_EQ(vec.size(), 0u); +} + +TEST_F(JSONParserTest, CopyArrayToVector) { + const std::string json = R"JSON({ + "qux": [ "quux", "quuz" ] + })JSON"; + JSONParser parser; + auto document = parser.ParseFromString(json, "test_file"); + ASSERT_FALSE(parser.HasError()); + std::vector<std::string> vec = {"foo"}; + parser.CopyStringArray("qux", document["qux"], &vec); + EXPECT_FALSE(parser.HasError()); + EXPECT_EQ(vec.size(), 2u); + EXPECT_EQ(vec[0], "quux"); + EXPECT_EQ(vec[1], "quuz"); +} + } // namespace } // namespace json