Skip to content

Commit 9585efd

Browse files
trflynn89trflynn89
committed
Enforce more strict JSON string conversion rules
Rather than allowing conversion of any JSON type to a string by way of Json::serialize(), only allow creating a string from string and numeric types. Coversting e.g. a JSON object to a string should explicitly use Json::serialize().
1 parent 1c34fb0 commit 9585efd

File tree

3 files changed

+49
-51
lines changed

3 files changed

+49
-51
lines changed

‎fly/types/json/json.hpp

+37-17
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,7 @@ namespace fly {
137137
* JSON strings may be converted to numeric values if the string represents a number. For
138138
* example, the string "12389" may be converted to an integer. The string "abc" may not.
139139
*
140-
* All JSON types may be converted to a string type. Non-string JSON values will be
141-
* serialized to a string.
140+
* Numeric JSON types may be converted to a string type.
142141
*
143142
* All JSON types may be converted to a boolean. String, object, and array JSON values will
144143
* convert based on whether the value is empty. JSON numbers will convert based on whether
@@ -395,17 +394,21 @@ class Json
395394

396395
/**
397396
* String conversion operator. Converts the Json instance to a string. The SFINAE declaration
398-
* allows conversion to any string type (e.g. std::string, std::u8string).
397+
* allows conversion to any string type (e.g. std::string, std::u8string). Also allows for
398+
* converting from a numeric type (e.g. 12389) to a string type.
399399
*
400400
* Note that although a Json instance can be constructed from a character array, it is not
401401
* allowed to directly convert a Json instance into a character array.
402402
*
403403
* @tparam T The string type.
404404
*
405405
* @return The Json instance as a string.
406+
*
407+
* @throws JsonException If the Json instance is not a string, or the stored value could not be
408+
* converted to the target string type.
406409
*/
407410
template <typename T, enable_if<JsonTraits::is_string<T>> = 0>
408-
explicit operator T() const noexcept;
411+
explicit operator T() const noexcept(false);
409412

410413
/**
411414
* Object conversion operator. Converts the Json instance to an object. The SFINAE declaration
@@ -471,7 +474,8 @@ class Json
471474
* Numeric conversion operator. Converts the Json instance to a numeric type. The SFINAE
472475
* declaration allows conversion to any numeric type type (e.g. char, uint64_t, float) from
473476
* the Json instance. Allows for converting between signed integers, unsigned integers, and
474-
* floats. Also allows for converting from a numeric-like string (e.g. "123") to a numeric type.
477+
* floats. Also allows for converting from a numeric-like string (e.g. "12389") to a numeric
478+
* type.
475479
*
476480
* @tparam T The numeric type.
477481
*
@@ -1594,21 +1598,37 @@ Json::Json(T value) noexcept : m_value(static_cast<JsonTraits::float_type>(value
15941598

15951599
//==================================================================================================
15961600
template <typename T, enable_if<JsonTraits::is_string<T>>>
1597-
Json::operator T() const noexcept
1601+
Json::operator T() const noexcept(false)
15981602
{
1599-
JsonTraits::string_type value;
1600-
1601-
if (is_string())
1602-
{
1603-
value = std::get<JsonTraits::string_type>(m_value);
1604-
}
1605-
else
1603+
auto visitor = [this](const auto &storage) -> T
16061604
{
1607-
value = serialize();
1608-
}
1605+
using S = std::decay_t<decltype(storage)>;
1606+
1607+
if constexpr (JsonTraits::is_string_v<S>)
1608+
{
1609+
if constexpr (std::is_same_v<T, JsonTraits::string_type>)
1610+
{
1611+
return storage;
1612+
}
1613+
else
1614+
{
1615+
// The JSON string will have been validated for Unicode compliance during
1616+
// construction.
1617+
return *(JsonTraits::StringType::convert<T>(storage));
1618+
}
1619+
}
1620+
else if constexpr (JsonTraits::is_number_v<S>)
1621+
{
1622+
using char_type = typename T::value_type;
1623+
return BasicString<char_type>::format(FLY_ARR(char_type, "{}"), storage);
1624+
}
1625+
else
1626+
{
1627+
throw JsonException(*this, "JSON type is not a string");
1628+
}
1629+
};
16091630

1610-
// The JSON string will have been validated for Unicode compliance during construction.
1611-
return *(JsonTraits::StringType::convert<T>(value));
1631+
return std::visit(std::move(visitor), m_value);
16121632
}
16131633

16141634
//==================================================================================================

‎test/config/config_manager.cpp

+7-13
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,8 @@ CATCH_TEST_CASE("ConfigManager", "[config]")
142142

143143
const fly::Json json {
144144
{fly::test::TestConfig::identifier, {{"name", "John Doe"}, {"address", "MA"}}}};
145-
const std::string contents(json);
146145

147-
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, contents));
146+
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, json.serialize()));
148147
task_runner->wait_for_task_to_complete(s_config_manager_file);
149148
task_runner->wait_for_task_to_complete(s_config_manager_file);
150149

@@ -156,7 +155,7 @@ CATCH_TEST_CASE("ConfigManager", "[config]")
156155
CATCH_CHECK(config->get_value<std::string>("address", "") == "MA");
157156
}
158157

159-
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, contents + "\n"));
158+
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, json.serialize() + "\n"));
160159
task_runner->wait_for_task_to_complete(s_config_manager_file);
161160

162161
CATCH_CHECK(config_manager->prune() == initial_size);
@@ -166,9 +165,8 @@ CATCH_TEST_CASE("ConfigManager", "[config]")
166165
{
167166
const fly::Json json {
168167
{fly::test::TestConfig::identifier, {{"name", "John Doe"}, {"address", "MA"}}}};
169-
const std::string contents(json);
170168

171-
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, contents));
169+
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, json.serialize()));
172170
task_runner->wait_for_task_to_complete(s_config_manager_file);
173171
task_runner->wait_for_task_to_complete(s_config_manager_file);
174172

@@ -184,9 +182,8 @@ CATCH_TEST_CASE("ConfigManager", "[config]")
184182

185183
const fly::Json json {
186184
{fly::test::TestConfig::identifier, {{"name", "John Doe"}, {"address", "MA"}}}};
187-
const std::string contents(json);
188185

189-
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, contents));
186+
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, json.serialize()));
190187
task_runner->wait_for_task_to_complete(s_config_manager_file);
191188
task_runner->wait_for_task_to_complete(s_config_manager_file);
192189

@@ -200,9 +197,8 @@ CATCH_TEST_CASE("ConfigManager", "[config]")
200197

201198
const fly::Json json1 {
202199
{fly::test::TestConfig::identifier, {{"name", "John Doe"}, {"address", "MA"}}}};
203-
const std::string contents1(json1);
204200

205-
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, contents1));
201+
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, json1.serialize()));
206202
task_runner->wait_for_task_to_complete(s_config_manager_file);
207203
task_runner->wait_for_task_to_complete(s_config_manager_file);
208204

@@ -212,9 +208,8 @@ CATCH_TEST_CASE("ConfigManager", "[config]")
212208

213209
const fly::Json json2 {
214210
{fly::test::TestConfig::identifier, {{"name", "Jane Doe"}, {"age", 27}}}};
215-
const std::string contents2(json2);
216211

217-
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, contents2));
212+
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, json2.serialize()));
218213
task_runner->wait_for_task_to_complete(s_config_manager_file);
219214

220215
// Multiple fly::PathEvent::Changed events may be triggered even though the above write
@@ -235,9 +230,8 @@ CATCH_TEST_CASE("ConfigManager", "[config]")
235230

236231
const fly::Json json {
237232
{fly::test::TestConfig::identifier, {{"name", "John Doe"}, {"address", "MA"}}}};
238-
const std::string contents(json);
239233

240-
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, contents));
234+
CATCH_REQUIRE(fly::test::PathUtil::write_file(config_file, json.serialize()));
241235
task_runner->wait_for_task_to_complete(s_config_manager_file);
242236
task_runner->wait_for_task_to_complete(s_config_manager_file);
243237

‎test/types/json/json_conversion.cpp

+5-21
Original file line numberDiff line numberDiff line change
@@ -19,35 +19,19 @@ CATCH_JSON_STRING_TEST_CASE("JsonConversion")
1919

2020
CATCH_SECTION("Convert a JSON instance to string-like types")
2121
{
22-
if constexpr (std::is_same_v<json_type, fly::JsonTraits::null_type>)
23-
{
24-
CATCH_CHECK(string_type(json) == J_STR("null"));
25-
CATCH_CHECK(string_type(empty) == J_STR("null"));
26-
}
27-
else if constexpr (std::is_same_v<json_type, fly::JsonTraits::string_type>)
22+
if constexpr (std::is_same_v<json_type, fly::JsonTraits::string_type>)
2823
{
2924
CATCH_CHECK(string_type(json) == J_STR("abcdef"));
3025
CATCH_CHECK(string_type(empty) == J_STR(""));
3126
}
32-
else if constexpr (std::is_same_v<json_type, fly::JsonTraits::object_type>)
27+
else if constexpr (fly::JsonTraits::is_number_v<json_type>)
3328
{
34-
CATCH_CHECK(string_type(json) == J_STR("{\"a\":1,\"b\":2}"));
35-
CATCH_CHECK(string_type(empty) == J_STR("{}"));
36-
}
37-
else if constexpr (std::is_same_v<json_type, fly::JsonTraits::array_type>)
38-
{
39-
CATCH_CHECK(string_type(json) == J_STR("[55,8,9,10]"));
40-
CATCH_CHECK(string_type(empty) == J_STR("[]"));
41-
}
42-
else if constexpr (std::is_same_v<json_type, fly::JsonTraits::boolean_type>)
43-
{
44-
CATCH_CHECK(string_type(json) == J_STR("true"));
45-
CATCH_CHECK(string_type(empty) == J_STR("false"));
29+
CATCH_CHECK(string_type(json) == J_STR("1"));
30+
CATCH_CHECK(string_type(empty) == J_STR("0"));
4631
}
4732
else
4833
{
49-
CATCH_CHECK(string_type(json) == J_STR("1"));
50-
CATCH_CHECK(string_type(empty) == J_STR("0"));
34+
CATCH_CHECK_THROWS_JSON(string_type(json), "JSON type is not a string: ({})", json);
5135
}
5236
}
5337

0 commit comments

Comments
 (0)