From 42c3aebda9cb7c5898379538c8dbb1744394db92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Fri, 5 Jan 2024 15:01:02 +0100 Subject: [PATCH] AddonInfo: const-ified loading and improved errorhandling (#5834) --- lib/addoninfo.cpp | 100 ++++++++++++++++++++++++++--------------- test/cli/test-other.py | 78 +++++++++++++++++++++++--------- 2 files changed, 120 insertions(+), 58 deletions(-) diff --git a/lib/addoninfo.cpp b/lib/addoninfo.cpp index c96543f75..7930f2e59 100644 --- a/lib/addoninfo.cpp +++ b/lib/addoninfo.cpp @@ -52,47 +52,73 @@ static std::string parseAddonInfo(AddonInfo& addoninfo, const picojson::value &j return "Loading " + fileName + " failed. " + json_error; } if (!json.is()) - return "Loading " + fileName + " failed. Bad json."; - picojson::object obj = json.get(); - if (obj.count("args")) { - if (!obj["args"].is()) - return "Loading " + fileName + " failed. args must be array."; - for (const picojson::value &v : obj["args"].get()) - addoninfo.args += " " + v.get(); - } + return "Loading " + fileName + " failed. JSON is not an object."; - if (obj.count("ctu")) { - // ctu is specified in the config file - if (!obj["ctu"].is()) - return "Loading " + fileName + " failed. ctu must be boolean."; - addoninfo.ctu = obj["ctu"].get(); - } else { - addoninfo.ctu = false; - } - - if (obj.count("python")) { - // Python was defined in the config file - if (obj["python"].is()) { - return "Loading " + fileName +" failed. python must not be an array."; - } - addoninfo.python = obj["python"].get(); - } else { - addoninfo.python = ""; - } - - if (obj.count("executable")) { - if (!obj["executable"].is()) - return "Loading " + fileName + " failed. executable must be a string."; - addoninfo.executable = getFullPath(obj["executable"].get(), fileName); - return ""; - } - - if (!obj.count("script") || !obj["script"].is()) + // TODO: remove/complete default value handling for missing fields + const picojson::object& obj = json.get(); { - return "Loading " + fileName + " failed. script must be set to a string value."; + const auto it = obj.find("args"); + if (it != obj.cend()) { + const auto& val = it->second; + if (!val.is()) + return "Loading " + fileName + " failed. 'args' must be an array."; + for (const picojson::value &v : val.get()) { + if (!v.is()) + return "Loading " + fileName + " failed. 'args' entry is not a string."; + addoninfo.args += " " + v.get(); + } + } } - return addoninfo.getAddonInfo(obj["script"].get(), exename); + { + const auto it = obj.find("ctu"); + if (it != obj.cend()) { + const auto& val = it->second; + // ctu is specified in the config file + if (!val.is()) + return "Loading " + fileName + " failed. 'ctu' must be a boolean."; + addoninfo.ctu = val.get(); + } + else { + addoninfo.ctu = false; + } + } + + { + const auto it = obj.find("python"); + if (it != obj.cend()) { + const auto& val = it->second; + // Python was defined in the config file + if (!val.is()) { + return "Loading " + fileName +" failed. 'python' must be a string."; + } + addoninfo.python = val.get(); + } + else { + addoninfo.python = ""; + } + } + + { + const auto it = obj.find("executable"); + if (it != obj.cend()) { + const auto& val = it->second; + if (!val.is()) + return "Loading " + fileName + " failed. 'executable' must be a string."; + addoninfo.executable = getFullPath(val.get(), fileName); + return ""; // TODO: why bail out? + } + } + + const auto it = obj.find("script"); + if (it == obj.cend()) + return "Loading " + fileName + " failed. 'script' is missing."; + + const auto& val = it->second; + if (!val.is()) + return "Loading " + fileName + " failed. 'script' must be a string."; + + return addoninfo.getAddonInfo(val.get(), exename); } std::string AddonInfo::getAddonInfo(const std::string &fileName, const std::string &exename) { diff --git a/test/cli/test-other.py b/test/cli/test-other.py index fdb67b992..5ed9c4353 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -4,6 +4,7 @@ import os import sys import pytest +import json from testutils import cppcheck, assert_cppcheck @@ -278,22 +279,6 @@ extern const char* f() assert stderr == '{}:4:12: warning: strerror is MT-unsafe [threadsafety-unsafe-call]\n'.format(test_file) -def test_addon_invalidjson(tmpdir): - addon_file = os.path.join(tmpdir, 'invalid.json') - with open(addon_file, 'wt') as f: - f.write(""" -{ - "Script": "addons/something.py" -} - """) - - args = ['--addon={}'.format(addon_file), '--enable=all', 'nonexistent.cpp'] - - exitcode, stdout, stderr = cppcheck(args) - assert exitcode != 0 - assert stdout == 'Loading {} failed. script must be set to a string value.\n'.format(addon_file) - - def test_addon_naming(tmpdir): # the addon does nothing without a config addon_file = os.path.join(tmpdir, 'naming1.json') @@ -644,12 +629,10 @@ def test_invalid_addon_json(tmpdir): """) test_file = os.path.join(tmpdir, 'file.cpp') - with open(test_file, 'wt') as f: - f.write(""" -typedef int MISRA_5_6_VIOLATION; - """) + with open(test_file, 'wt'): + pass - args = ['--addon={}'.format(addon_file), '--enable=all', test_file] + args = ['--addon={}'.format(addon_file), test_file] exitcode, stdout, stderr = cppcheck(args) assert exitcode == 1 @@ -1124,3 +1107,56 @@ def test_build_dir_j_memleak(tmpdir): #12111 ] assert_cppcheck(args, ec_exp=0, err_exp=[], out_exp=out_lines) + + +def __test_addon_json_invalid(tmpdir, addon_json, expected): + addon_file = os.path.join(tmpdir, 'invalid.json') + with open(addon_file, 'wt') as f: + f.write(addon_json) + + test_file = os.path.join(tmpdir, 'file.cpp') + with open(test_file, 'wt'): + pass + + args = ['--addon={}'.format(addon_file), test_file] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 1 + lines = stdout.splitlines() + assert len(lines) == 1 + assert lines == [ + 'Loading {} failed. {}'.format(addon_file, expected) + ] + assert stderr == '' + + +def test_addon_json_invalid_no_obj(tmpdir): + __test_addon_json_invalid(tmpdir, json.dumps([]), "JSON is not an object.") + + +def test_addon_json_invalid_args_1(tmpdir): + __test_addon_json_invalid(tmpdir, json.dumps({'args':0}), "'args' must be an array.") + + +def test_addon_json_invalid_args_2(tmpdir): + __test_addon_json_invalid(tmpdir, json.dumps({'args':[0]}), "'args' entry is not a string.") + + +def test_addon_json_invalid_ctu(tmpdir): + __test_addon_json_invalid(tmpdir, json.dumps({'ctu':0}), "'ctu' must be a boolean.") + + +def test_addon_json_invalid_python(tmpdir): + __test_addon_json_invalid(tmpdir, json.dumps({'python':0}), "'python' must be a string.") + + +def test_addon_json_invalid_executable(tmpdir): + __test_addon_json_invalid(tmpdir, json.dumps({'executable':0}), "'executable' must be a string.") + + +def test_addon_json_invalid_script_1(tmpdir): + __test_addon_json_invalid(tmpdir, json.dumps({'Script':''}), "'script' is missing.") + + +def test_addon_json_invalid_script_2(tmpdir): + __test_addon_json_invalid(tmpdir, json.dumps({'script':0}), "'script' must be a string.") \ No newline at end of file