Skip to content

Commit

Permalink
fix: Make all JSON parsing case insensitive
Browse files Browse the repository at this point in the history
Although the JSON standard defines the keys as case sensitive, the
golang standard library implements it as case insensitive. Therefore to
avoid regressions we need to make all our JSON parsing case insensitive.

To implement it, we define a custom `nlohmann::basic_json` with a custom
`std::map` that compares the keys as lowercase. See:
* https://json.nlohmann.me/api/basic_json/basic_json/
* https://json.nlohmann.me/api/basic_json/object_t/

Ticket: MEN-6834
Changelog: None

Signed-off-by: Lluis Campos <lluis.campos@northern.tech>
  • Loading branch information
lluiscampos committed Nov 30, 2023
1 parent 20e1290 commit cb118d4
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ add_library(common_json STATIC json/json.cpp json/platform/${json_sources})
target_compile_options(common_json PRIVATE ${PLATFORM_SPECIFIC_COMPILE_OPTIONS})
if(${json_sources} MATCHES ".*nlohmann.*")
target_include_directories(common_json PUBLIC ${CMAKE_SOURCE_DIR}/vendor/json/include/)
target_link_libraries(common_json PUBLIC common_io common_error)
target_link_libraries(common_json PUBLIC common common_io common_error)
endif()

add_library(common_testing EXCLUDE_FROM_ALL STATIC testing.cpp)
Expand Down
7 changes: 7 additions & 0 deletions src/common/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ mender::common::expected::ExpectedLongLong StringToLongLong(const string &str, i
return num;
}

string StringToLower(const string &str) {
string lower_str(str.length(), ' ');
std::transform(
str.begin(), str.end(), lower_str.begin(), [](unsigned char c) { return std::tolower(c); });
return lower_str;
}

vector<string> SplitString(const string &str, const string &delim) {
vector<string> ret;
for (size_t begin = 0, end = str.find(delim);;) {
Expand Down
2 changes: 2 additions & 0 deletions src/common/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ inline static string StringFromByteVector(const vector<uint8_t> &vec) {

mender::common::expected::ExpectedLongLong StringToLongLong(const string &str, int base = 10);

string StringToLower(const string &str);

vector<string> SplitString(const string &str, const string &delim);
string JoinStrings(const vector<string> &str, const string &delim);
vector<string> JoinStringsMaxWidth(
Expand Down
42 changes: 42 additions & 0 deletions src/common/config_parser_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,3 +581,45 @@ TEST_F(ConfigParserTests, ValidateServerConfig) {
EXPECT_THAT(ret.error().String(), testing::HasSubstr("ServerURL"));
EXPECT_THAT(ret.error().String(), testing::HasSubstr("Servers"));
}

TEST_F(ConfigParserTests, CaseInsensitiveParsing) {
ofstream os(test_config_fname);
os << R"({
"artifactverifykey": "ArtifactVerifyKey_value",
"deviceTypeFile": "DeviceTypeFile_value",
"SERVERURL": "ServerURL_value"
})";
os.close();

config_parser::MenderConfigFromFile mc;
config_parser::ExpectedBool ret = mc.LoadFile(test_config_fname);
ASSERT_TRUE(ret);
ASSERT_TRUE(ret.value());

ASSERT_EQ(mc.artifact_verify_keys.size(), 1);
EXPECT_EQ(mc.artifact_verify_keys[0], "ArtifactVerifyKey_value");

EXPECT_EQ(mc.device_type_file, "DeviceTypeFile_value");

ASSERT_EQ(mc.servers.size(), 1);
EXPECT_EQ(mc.servers[0], "ServerURL_value");
}

TEST_F(ConfigParserTests, CaseInsensitiveCollision) {
ofstream os(test_config_fname);
os << R"({
"ServerUrl": "ServerURL_value_1",
"ServerUrl": "ServerURL_value_2",
"serverurl": "ServerURL_value_3",
"SERVERURL": "ServerURL_value_4"
})";
os.close();

config_parser::MenderConfigFromFile mc;
config_parser::ExpectedBool ret = mc.LoadFile(test_config_fname);
ASSERT_TRUE(ret);
ASSERT_TRUE(ret.value());

ASSERT_EQ(mc.servers.size(), 1);
EXPECT_EQ(mc.servers[0], "ServerURL_value_4");
}
5 changes: 1 addition & 4 deletions src/common/http/http.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,7 @@ string JoinOneUrl(const string &prefix, const string &suffix) {
}

size_t CaseInsensitiveHasher::operator()(const string &str) const {
string lower_str(str.length(), ' ');
transform(
str.begin(), str.end(), lower_str.begin(), [](unsigned char c) { return std::tolower(c); });
return hash<string>()(lower_str);
return hash<string>()(common::StringToLower(str));
}

bool CaseInsensitiveComparator::operator()(const string &str1, const string &str2) const {
Expand Down
38 changes: 35 additions & 3 deletions src/common/json.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <map>
#include <unordered_map>

#include <common/common.hpp>
#include <common/error.hpp>
#include <common/expected.hpp>
#include <common/io.hpp>
Expand All @@ -37,6 +38,7 @@ using namespace std;

namespace error = mender::common::error;
namespace io = mender::common::io;
namespace common = mender::common;

enum JsonErrorCode {
NoError = 0,
Expand All @@ -46,7 +48,37 @@ enum JsonErrorCode {
TypeError,
};

class JsonErrorCategoryClass : public std::error_category {
class CaseInsensitiveLess {
public:
bool operator()(const string &lhs, const string &rhs) const {
return common::StringToLower(lhs) < common::StringToLower(rhs);
}
};

template <class Key, class T, class IgnoredLess, class Allocator = allocator<pair<const Key, T>>>
class CaseInsensitiveMap : public map<const Key, T, CaseInsensitiveLess, Allocator> {
public:
using CaseInsensitiveMapType = map<const Key, T, CaseInsensitiveLess, Allocator>;

CaseInsensitiveMap() :
CaseInsensitiveMapType() {
}

template <class InputIterator>
CaseInsensitiveMap(
InputIterator first,
InputIterator last,
const CaseInsensitiveLess &comp = CaseInsensitiveLess(),
const Allocator &alloc = Allocator()) :
CaseInsensitiveMapType(first, last, comp, alloc) {
}
};

#ifdef MENDER_USE_NLOHMANN_JSON
using insensitive_json = nlohmann::basic_json<CaseInsensitiveMap>;
#endif

class JsonErrorCategoryClass : public error_category {
public:
const char *name() const noexcept override;
string message(int code) const override;
Expand Down Expand Up @@ -114,8 +146,8 @@ class Json {

private:
#ifdef MENDER_USE_NLOHMANN_JSON
nlohmann::json n_json;
Json(nlohmann::json n_json) :
insensitive_json n_json;
Json(insensitive_json n_json) :
n_json(n_json) {};
#endif
};
Expand Down
19 changes: 9 additions & 10 deletions src/common/json/platform/nlohmann/nlohmann_json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

#include <common/io.hpp>

using njson = nlohmann::json;
using namespace std;
namespace expected = mender::common::expected;
namespace error = mender::common::error;
Expand All @@ -38,9 +37,9 @@ static error::Error GetErrorFromException(exception &e, const string &context_me
// (Lippincott Function). The `e` argument is not actually needed, but included for
// clarity.
throw;
} catch (njson::parse_error &e) {
} catch (insensitive_json::parse_error &e) {
return MakeError(JsonErrorCode::ParseError, context_message + ": " + e.what());
} catch (njson::type_error &e) {
} catch (insensitive_json::type_error &e) {
return MakeError(JsonErrorCode::TypeError, context_message + ": " + e.what());
} catch (system_error &e) {
return error::Error(e.code().default_error_condition(), context_message + ": " + e.what());
Expand All @@ -62,7 +61,7 @@ ExpectedJson LoadFromFile(string file_path) {
}

try {
njson parsed = njson::parse(f);
insensitive_json parsed = insensitive_json::parse(f);
Json j = Json(parsed);
return ExpectedJson(j);
} catch (exception &e) {
Expand All @@ -73,7 +72,7 @@ ExpectedJson LoadFromFile(string file_path) {

ExpectedJson Load(string json_str) {
try {
njson parsed = njson::parse(json_str);
insensitive_json parsed = insensitive_json::parse(json_str);
Json j = Json(parsed);
return ExpectedJson(j);
} catch (exception &e) {
Expand All @@ -83,7 +82,7 @@ ExpectedJson Load(string json_str) {

ExpectedJson Load(istream &str) {
try {
njson parsed = njson::parse(str);
insensitive_json parsed = insensitive_json::parse(str);
Json j = Json(parsed);
return ExpectedJson(j);
} catch (exception &e) {
Expand Down Expand Up @@ -118,7 +117,7 @@ ExpectedJson Json::Get(const char *child_key) const {
return expected::unexpected(err);
}

njson n_json = this->n_json[child_key];
insensitive_json n_json = this->n_json[child_key];
Json j = Json(n_json);
return j;
}
Expand All @@ -137,7 +136,7 @@ ExpectedJson Json::Get(const size_t idx) const {
return expected::unexpected(err);
}

njson n_json = this->n_json[idx];
insensitive_json n_json = this->n_json[idx];
return Json(n_json);
}

Expand Down Expand Up @@ -233,13 +232,13 @@ ExpectedSize Json::GetArraySize() const {

template <>
ExpectedString Dump(unordered_map<string, vector<string>> std_map) {
njson map_json(std_map);
insensitive_json map_json(std_map);
return map_json.dump();
}

template <>
ExpectedString Dump(unordered_map<string, string> std_map) {
njson map_json(std_map);
insensitive_json map_json(std_map);
return map_json.dump();
}

Expand Down

0 comments on commit cb118d4

Please sign in to comment.