diff --git a/src/common/ocispec/imageconfig.cpp b/src/common/ocispec/imageconfig.cpp index 67d3fef8b..2d123a52e 100644 --- a/src/common/ocispec/imageconfig.cpp +++ b/src/common/ocispec/imageconfig.cpp @@ -4,9 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include -#include - #include #include #include @@ -118,14 +115,8 @@ Poco::JSON::Object RootfsToJSON(const aos::oci::Rootfs& rootfs) Error OCISpec::LoadImageConfig(const String& path, aos::oci::ImageConfig& imageConfig) { try { - std::ifstream file(path.CStr()); - - if (!file.is_open()) { - AOS_ERROR_THROW(ErrorEnum::eNotFound, "failed to open file"); - } - - auto [var, err] = utils::ParseJson(file); - AOS_ERROR_CHECK_AND_THROW(err, "failed to parse json"); + auto [var, err] = utils::ReadJsonFromFile(path.CStr()); + AOS_ERROR_CHECK_AND_THROW(err, "failed to load json file"); Poco::JSON::Object::Ptr object = var.extract(); utils::CaseInsensitiveObjectWrapper wrapper(object); diff --git a/src/common/ocispec/imageindex.cpp b/src/common/ocispec/imageindex.cpp index 02d6e92ff..7574054d2 100644 --- a/src/common/ocispec/imageindex.cpp +++ b/src/common/ocispec/imageindex.cpp @@ -4,9 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include -#include - #include #include #include @@ -56,14 +53,8 @@ Poco::JSON::Object IndexContentDescriptorToJSONObject(const aos::oci::IndexConte Error OCISpec::LoadImageIndex(const String& path, aos::oci::ImageIndex& index) { try { - std::ifstream file(path.CStr()); - - if (!file.is_open()) { - AOS_ERROR_THROW(ErrorEnum::eNotFound, "failed to open file"); - } - - auto [var, err] = utils::ParseJson(file); - AOS_ERROR_CHECK_AND_THROW(err, "failed to parse json"); + auto [var, err] = utils::ReadJsonFromFile(path.CStr()); + AOS_ERROR_CHECK_AND_THROW(err, "failed to load json file"); Poco::JSON::Object::Ptr object = var.extract(); utils::CaseInsensitiveObjectWrapper wrapper(object); diff --git a/src/common/ocispec/imagemanifest.cpp b/src/common/ocispec/imagemanifest.cpp index a40fdd258..708560ef3 100644 --- a/src/common/ocispec/imagemanifest.cpp +++ b/src/common/ocispec/imagemanifest.cpp @@ -4,9 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include -#include - #include #include #include @@ -23,14 +20,8 @@ namespace aos::common::oci { Error OCISpec::LoadImageManifest(const String& path, aos::oci::ImageManifest& manifest) { try { - std::ifstream file(path.CStr()); - - if (!file.is_open()) { - AOS_ERROR_THROW(ErrorEnum::eNotFound, "failed to open file"); - } - - auto [var, err] = utils::ParseJson(file); - AOS_ERROR_CHECK_AND_THROW(err, "failed to parse json"); + auto [var, err] = utils::ReadJsonFromFile(path.CStr()); + AOS_ERROR_CHECK_AND_THROW(err, "failed to load json file"); Poco::JSON::Object::Ptr object = var.extract(); utils::CaseInsensitiveObjectWrapper wrapper(object); diff --git a/src/common/ocispec/itemconfig.cpp b/src/common/ocispec/itemconfig.cpp index 5a69bc958..c080cfa80 100644 --- a/src/common/ocispec/itemconfig.cpp +++ b/src/common/ocispec/itemconfig.cpp @@ -4,9 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include -#include - #include #include #include @@ -458,14 +455,8 @@ Poco::JSON::Object AlertRulesToJSON(const AlertRules& rules) Error OCISpec::LoadItemConfig(const String& path, aos::oci::ItemConfig& itemConfig) { try { - std::ifstream file(path.CStr()); - - if (!file.is_open()) { - AOS_ERROR_THROW(ErrorEnum::eNotFound, "failed to open file"); - } - - auto [var, err] = utils::ParseJson(file); - AOS_ERROR_CHECK_AND_THROW(err, "failed to parse json"); + auto [var, err] = utils::ReadJsonFromFile(path.CStr()); + AOS_ERROR_CHECK_AND_THROW(err, "failed to load json file"); Poco::JSON::Object::Ptr object = var.extract(); utils::CaseInsensitiveObjectWrapper wrapper(object); diff --git a/src/common/ocispec/runtimeconfig.cpp b/src/common/ocispec/runtimeconfig.cpp index a545ce406..657bf1ab1 100644 --- a/src/common/ocispec/runtimeconfig.cpp +++ b/src/common/ocispec/runtimeconfig.cpp @@ -4,8 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include - #include #include #include @@ -928,14 +926,8 @@ Poco::JSON::Object VMToJSON(const aos::oci::VM& vm) Error OCISpec::LoadRuntimeConfig(const String& path, aos::oci::RuntimeConfig& runtimeConfig) { try { - std::ifstream file(path.CStr()); - - if (!file.is_open()) { - AOS_ERROR_THROW(ErrorEnum::eNotFound, "failed to open file"); - } - - auto [var, err] = utils::ParseJson(file); - AOS_ERROR_CHECK_AND_THROW(err, "failed to parse json"); + auto [var, err] = utils::ReadJsonFromFile(path.CStr()); + AOS_ERROR_CHECK_AND_THROW(err, "failed to load json file"); Poco::JSON::Object::Ptr object = var.extract(); utils::CaseInsensitiveObjectWrapper wrapper(object); diff --git a/src/common/ocispec/tests/ocispec.cpp b/src/common/ocispec/tests/ocispec.cpp index 1c4a359ec..4f8ec21fd 100644 --- a/src/common/ocispec/tests/ocispec.cpp +++ b/src/common/ocispec/tests/ocispec.cpp @@ -4,6 +4,15 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include +#include + +#include +#include +#include +#include +#include + #include #include @@ -16,6 +25,25 @@ using namespace testing; +namespace { + +std::atomic sReadErrno {0}; + +} // namespace + +// File read is intercepted to inject the read errors of the integrity checked storage, which can't be reproduced with a +// regular file. +extern "C" ssize_t read(int fd, void* buf, size_t count) +{ + if (const auto readErrno = sReadErrno.load(); readErrno != 0) { + errno = readErrno; + + return -1; + } + + return static_cast(syscall(SYS_read, fd, buf, count)); +} + namespace aos::common::jsonprovider { namespace { @@ -318,6 +346,29 @@ std::string MakeItemConfigWithPermissions(const std::string& permissions) )"; } +using LoadFunc = std::function; + +template +LoadFunc MakeLoadFunc(Method method) +{ + return [method](oci::OCISpec& ociSpec, const String& path) { + auto object = std::make_unique(); + + return (ociSpec.*method)(path, *object); + }; +} + +std::vector GetLoadFuncs() +{ + return { + MakeLoadFunc(&oci::OCISpec::LoadImageIndex), + MakeLoadFunc(&oci::OCISpec::LoadImageManifest), + MakeLoadFunc(&oci::OCISpec::LoadImageConfig), + MakeLoadFunc(&oci::OCISpec::LoadItemConfig), + MakeLoadFunc(&oci::OCISpec::LoadRuntimeConfig), + }; +} + class OCISpecTest : public Test { public: void SetUp() override @@ -332,6 +383,8 @@ class OCISpecTest : public Test { fs::WriteStringToFile(cItemConfigPath, cItemConfig, S_IRUSR | S_IWUSR); } + void TearDown() override { sReadErrno = 0; } + oci::OCISpec mOCISpec; }; @@ -582,6 +635,56 @@ TEST_F(OCISpecTest, LoadItemConfigPermissionsExceedLimitsFails) } } +TEST_F(OCISpecTest, LoadReadErrorIsIntegrity) +{ + for (const auto readErrno : {EIO, EBADMSG}) { + for (const auto& load : GetLoadFuncs()) { + sReadErrno = readErrno; + const auto err = load(mOCISpec, cImageIndexPath); + sReadErrno = 0; + + EXPECT_TRUE(err.Is(ErrorEnum::eIntegrity)) + << "errno: " << readErrno << ", error: " << tests::utils::ErrorToStr(err); + } + } +} + +TEST_F(OCISpecTest, LoadOtherReadErrorIsReturnedAsIs) +{ + for (const auto& load : GetLoadFuncs()) { + sReadErrno = EPERM; + const auto err = load(mOCISpec, cImageIndexPath); + sReadErrno = 0; + + EXPECT_EQ(err.Value(), ErrorEnum::eRuntime) << tests::utils::ErrorToStr(err); + EXPECT_EQ(err.Code(), EPERM) << tests::utils::ErrorToStr(err); + } +} + +TEST_F(OCISpecTest, LoadMissingFileIsNotFound) +{ + const auto cMissingPath = fs::JoinPath(cTestBaseDir, "missing.json"); + + for (const auto& load : GetLoadFuncs()) { + const auto err = load(mOCISpec, cMissingPath); + + EXPECT_TRUE(err.Is(ErrorEnum::eNotFound)) << tests::utils::ErrorToStr(err); + } +} + +TEST_F(OCISpecTest, LoadMalformedFileIsNotIntegrity) +{ + const auto cMalformedPath = fs::JoinPath(cTestBaseDir, "malformed.json"); + + ASSERT_TRUE(fs::WriteStringToFile(cMalformedPath, "{\"schemaVersion\": ", S_IRUSR | S_IWUSR).IsNone()); + + for (const auto& load : GetLoadFuncs()) { + const auto err = load(mOCISpec, cMalformedPath); + + EXPECT_TRUE(err.Is(ErrorEnum::eInvalidArgument)) << tests::utils::ErrorToStr(err); + } +} + TEST_F(OCISpecTest, ServiceConfigFromFileRunParams) { const std::vector runParams = { diff --git a/src/common/utils/json.cpp b/src/common/utils/json.cpp index 1cb95921f..b1ca1c639 100644 --- a/src/common/utils/json.cpp +++ b/src/common/utils/json.cpp @@ -6,12 +6,16 @@ */ #include +#include #include #include +#include #include #include +#include + #include "exception.hpp" #include "json.hpp" @@ -133,6 +137,25 @@ Error WriteJsonToFile(const Poco::JSON::Object::Ptr& json, const std::string& pa return ErrorEnum::eNone; } +aos::RetWithError ReadJsonFromFile(const std::string& path) +{ + std::error_code ec; + + const auto size = std::filesystem::file_size(path, ec); + + // The size error is not returned: if the file can't be accessed, ReadFile fails with the proper error. + std::string content(ec ? 0 : size, '\0'); + auto buffer = Array(reinterpret_cast(content.data()), content.size()); + + if (auto err = fs::ReadFile(path.c_str(), buffer); !err.IsNone()) { + return {{}, AOS_ERROR_WRAP(err)}; + } + + content.resize(buffer.Size()); + + return ParseJson(content); +} + Poco::Dynamic::Var FindByPath(const Poco::Dynamic::Var object, const std::vector& keys) { if (keys.empty()) { diff --git a/src/common/utils/json.hpp b/src/common/utils/json.hpp index fd7d81e4a..32f8b2617 100644 --- a/src/common/utils/json.hpp +++ b/src/common/utils/json.hpp @@ -55,6 +55,17 @@ aos::RetWithError ParseJson(std::istream& in) noexcept; */ Error WriteJsonToFile(const Poco::JSON::Object::Ptr& json, const std::string& path); +/** + * Reads and parses json from file. + * + * The errors of the file open and read are returned as follows: EIO and EBADMSG as ErrorEnum::eIntegrity, ENOENT as + * ErrorEnum::eNotFound, any other errno as is. + * + * @param path path to the file. + * @return aos::RetWithError. + */ +aos::RetWithError ReadJsonFromFile(const std::string& path); + /** * Finds value of the json by path * diff --git a/src/common/utils/tests/json.cpp b/src/common/utils/tests/json.cpp index e19bd0ccd..198909ede 100644 --- a/src/common/utils/tests/json.cpp +++ b/src/common/utils/tests/json.cpp @@ -5,6 +5,9 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include + +#include #include #include @@ -284,6 +287,45 @@ TEST_F(JsonTest, WriteJsonToFileFails) EXPECT_EQ(WriteJsonToFile(object, path), aos::ErrorEnum::eFailed); } +TEST_F(JsonTest, ReadJsonFromFileSucceeds) +{ + const std::string path = "read_json_from_file.json"; + + Poco::JSON::Object::Ptr object = new Poco::JSON::Object; + object->set("key", "value"); + + ASSERT_EQ(WriteJsonToFile(object, path), aos::ErrorEnum::eNone); + + auto [var, err] = ReadJsonFromFile(path); + + EXPECT_EQ(err, aos::ErrorEnum::eNone); + EXPECT_EQ(var.extract()->getValue("key"), "value"); + + std::remove(path.c_str()); +} + +TEST_F(JsonTest, ReadJsonFromFileNotFound) +{ + auto [var, err] = ReadJsonFromFile("read_json_from_file_missing.json"); + + EXPECT_EQ(err, aos::ErrorEnum::eNotFound); +} + +TEST_F(JsonTest, ReadJsonFromFileReturnsErrnoAsIs) +{ + const std::string path = "read_json_from_file_link_loop.json"; + + std::remove(path.c_str()); + + ASSERT_EQ(symlink(path.c_str(), path.c_str()), 0); + + auto [var, err] = ReadJsonFromFile(path); + + EXPECT_EQ(err.Code(), ELOOP); + + std::remove(path.c_str()); +} + TEST_F(JsonTest, ToJSONArray) { auto array