Skip to content

Commit a4d1ac4

Browse files
jviottijviotti
authored
Improve from_json to safely parse input (#467)
Signed-off-by: Juan Cruz Viotti <[email protected]>
1 parent cab2775 commit a4d1ac4

File tree

16 files changed

+530
-207
lines changed

16 files changed

+530
-207
lines changed

‎DEPENDENCIES‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
vendorpull https://github.com/sourcemeta/vendorpull dea311b5bfb53b6926a4140267959ae334d3ecf4
2-
core https://github.com/sourcemeta/core d7199d56450c95c5190c9e002f572199e02234ae
2+
core https://github.com/sourcemeta/core fb91cc0f4bf496f6f56b1c9ae2bb4763e10adc59
33
jsonschema-test-suite https://github.com/json-schema-org/JSON-Schema-Test-Suite 48461fc3568972801b40eaccc428a31bce338f6e

‎src/compiler/compile_json.cc‎

Lines changed: 113 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -5,38 +5,42 @@
55

66
namespace {
77
auto value_from_json(const sourcemeta::core::JSON &wrapper)
8-
-> sourcemeta::blaze::Value {
9-
assert(wrapper.is_object());
10-
assert(wrapper.defines("t"));
11-
assert(wrapper.defines("v"));
12-
const auto &type{wrapper.at("t")};
13-
const auto &value{wrapper.at("v")};
14-
assert(type.is_integer() && type.is_positive());
8+
-> std::optional<sourcemeta::blaze::Value> {
9+
if (!wrapper.is_object()) {
10+
return std::nullopt;
11+
}
12+
13+
const auto type{wrapper.try_at("t")};
14+
const auto value{wrapper.try_at("v")};
15+
if (!type || !value) {
16+
return std::nullopt;
17+
}
18+
1519
using namespace sourcemeta::blaze;
16-
switch (type.to_integer()) {
20+
switch (type->to_integer()) {
1721
// clang-format off
18-
case 0: return sourcemeta::core::from_json<ValueNone>(value);
19-
case 1: return sourcemeta::core::from_json<ValueJSON>(value);
20-
case 2: return sourcemeta::core::from_json<ValueSet>(value);
21-
case 3: return sourcemeta::core::from_json<ValueString>(value);
22-
case 4: return sourcemeta::core::from_json<ValueProperty>(value);
23-
case 5: return sourcemeta::core::from_json<ValueStrings>(value);
24-
case 6: return sourcemeta::core::from_json<ValueStringSet>(value);
25-
case 7: return sourcemeta::core::from_json<ValueTypes>(value);
26-
case 8: return sourcemeta::core::from_json<ValueType>(value);
27-
case 9: return sourcemeta::core::from_json<ValueRegex>(value);
28-
case 10: return sourcemeta::core::from_json<ValueUnsignedInteger>(value);
29-
case 11: return sourcemeta::core::from_json<ValueRange>(value);
30-
case 12: return sourcemeta::core::from_json<ValueBoolean>(value);
31-
case 13: return sourcemeta::core::from_json<ValueNamedIndexes>(value);
32-
case 14: return sourcemeta::core::from_json<ValueStringType>(value);
33-
case 15: return sourcemeta::core::from_json<ValueStringMap>(value);
34-
case 16: return sourcemeta::core::from_json<ValuePropertyFilter>(value);
35-
case 17: return sourcemeta::core::from_json<ValueIndexPair>(value);
36-
case 18: return sourcemeta::core::from_json<ValuePointer>(value);
37-
case 19: return sourcemeta::core::from_json<ValueTypedProperties>(value);
38-
case 20: return sourcemeta::core::from_json<ValueStringHashes>(value);
39-
case 21: return sourcemeta::core::from_json<ValueTypedHashes>(value);
22+
case 0: return sourcemeta::core::from_json<ValueNone>(*value);
23+
case 1: return sourcemeta::core::from_json<ValueJSON>(*value);
24+
case 2: return sourcemeta::core::from_json<ValueSet>(*value);
25+
case 3: return sourcemeta::core::from_json<ValueString>(*value);
26+
case 4: return sourcemeta::core::from_json<ValueProperty>(*value);
27+
case 5: return sourcemeta::core::from_json<ValueStrings>(*value);
28+
case 6: return sourcemeta::core::from_json<ValueStringSet>(*value);
29+
case 7: return sourcemeta::core::from_json<ValueTypes>(*value);
30+
case 8: return sourcemeta::core::from_json<ValueType>(*value);
31+
case 9: return sourcemeta::core::from_json<ValueRegex>(*value);
32+
case 10: return sourcemeta::core::from_json<ValueUnsignedInteger>(*value);
33+
case 11: return sourcemeta::core::from_json<ValueRange>(*value);
34+
case 12: return sourcemeta::core::from_json<ValueBoolean>(*value);
35+
case 13: return sourcemeta::core::from_json<ValueNamedIndexes>(*value);
36+
case 14: return sourcemeta::core::from_json<ValueStringType>(*value);
37+
case 15: return sourcemeta::core::from_json<ValueStringMap>(*value);
38+
case 16: return sourcemeta::core::from_json<ValuePropertyFilter>(*value);
39+
case 17: return sourcemeta::core::from_json<ValueIndexPair>(*value);
40+
case 18: return sourcemeta::core::from_json<ValuePointer>(*value);
41+
case 19: return sourcemeta::core::from_json<ValueTypedProperties>(*value);
42+
case 20: return sourcemeta::core::from_json<ValueStringHashes>(*value);
43+
case 21: return sourcemeta::core::from_json<ValueTypedHashes>(*value);
4044
// clang-format on
4145
default:
4246
assert(false);
@@ -45,44 +49,70 @@ auto value_from_json(const sourcemeta::core::JSON &wrapper)
4549
}
4650

4751
auto instructions_from_json(const sourcemeta::core::JSON &instructions)
48-
-> sourcemeta::blaze::Instructions {
49-
assert(instructions.is_array());
52+
-> std::optional<sourcemeta::blaze::Instructions> {
53+
if (!instructions.is_array()) {
54+
return std::nullopt;
55+
}
56+
5057
sourcemeta::blaze::Instructions result;
5158
result.reserve(instructions.size());
5259
for (const auto &instruction : instructions.as_array()) {
53-
assert(instruction.is_object());
54-
assert(instruction.defines("t"));
55-
assert(instruction.defines("s"));
56-
assert(instruction.defines("i"));
57-
assert(instruction.defines("k"));
58-
assert(instruction.defines("r"));
59-
assert(instruction.defines("v"));
60-
assert(instruction.defines("c"));
61-
const auto &type{instruction.at("t")};
62-
const auto &relative_schema_location{instruction.at("s")};
63-
const auto &relative_instance_location{instruction.at("i")};
64-
const auto &keyword_location{instruction.at("k")};
65-
const auto &schema_resource{instruction.at("r")};
66-
const auto &value{instruction.at("v")};
67-
const auto &children{instruction.at("c")};
68-
assert(type.is_integer() && type.is_positive());
69-
assert(relative_schema_location.is_string());
70-
assert(relative_instance_location.is_string());
71-
assert(keyword_location.is_string());
72-
assert(schema_resource.is_integer() && schema_resource.is_positive());
73-
assert(value.is_object());
74-
assert(children.is_array());
60+
if (!instruction.is_object()) {
61+
return std::nullopt;
62+
}
63+
64+
const auto type{instruction.try_at("t")};
65+
const auto relative_schema_location{instruction.try_at("s")};
66+
const auto relative_instance_location{instruction.try_at("i")};
67+
const auto keyword_location{instruction.try_at("k")};
68+
const auto schema_resource{instruction.try_at("r")};
69+
const auto value{instruction.try_at("v")};
70+
const auto children{instruction.try_at("c")};
71+
72+
if (!type || !relative_schema_location || !relative_instance_location ||
73+
!keyword_location || !schema_resource || !value || !children) {
74+
return std::nullopt;
75+
} else if (!type->is_positive() || !relative_schema_location->is_string() ||
76+
!relative_instance_location->is_string() ||
77+
!keyword_location->is_string() ||
78+
!schema_resource->is_positive() || !value->is_object() ||
79+
!children->is_array()) {
80+
return std::nullopt;
81+
}
82+
83+
auto type_result{
84+
sourcemeta::core::from_json<sourcemeta::blaze::InstructionIndex>(
85+
*type)};
86+
auto relative_schema_location_result{
87+
sourcemeta::core::from_json<sourcemeta::core::Pointer>(
88+
*relative_schema_location)};
89+
auto relative_instance_location_result{
90+
sourcemeta::core::from_json<sourcemeta::core::Pointer>(
91+
*relative_instance_location)};
92+
auto keyword_location_result{
93+
sourcemeta::core::from_json<std::string>(*keyword_location)};
94+
auto schema_resource_result{
95+
sourcemeta::core::from_json<std::size_t>(*schema_resource)};
96+
auto value_result{value_from_json(*value)};
97+
auto children_result{instructions_from_json(*children)};
98+
99+
if (!type_result.has_value() ||
100+
!relative_schema_location_result.has_value() ||
101+
!relative_instance_location_result.has_value() ||
102+
!keyword_location_result.has_value() ||
103+
!schema_resource_result.has_value() || !value_result.has_value() ||
104+
!children_result.has_value()) {
105+
return std::nullopt;
106+
}
75107

76108
// TODO: Maybe we should emplace here?
77-
result.push_back(
78-
{sourcemeta::core::from_json<sourcemeta::blaze::InstructionIndex>(type),
79-
sourcemeta::core::from_json<sourcemeta::core::Pointer>(
80-
relative_schema_location),
81-
sourcemeta::core::from_json<sourcemeta::core::Pointer>(
82-
relative_instance_location),
83-
sourcemeta::core::from_json<std::string>(keyword_location),
84-
sourcemeta::core::from_json<std::size_t>(schema_resource),
85-
value_from_json(value), instructions_from_json(children)});
109+
result.push_back({std::move(type_result).value(),
110+
std::move(relative_schema_location_result).value(),
111+
std::move(relative_instance_location_result).value(),
112+
std::move(keyword_location_result).value(),
113+
std::move(schema_resource_result).value(),
114+
std::move(value_result).value(),
115+
std::move(children_result).value()});
86116
}
87117

88118
return result;
@@ -139,19 +169,26 @@ auto to_json(const Template &schema_template) -> sourcemeta::core::JSON {
139169
return result;
140170
}
141171

142-
auto from_json(const sourcemeta::core::JSON &json) -> Template {
143-
assert(json.is_object());
144-
assert(json.defines("instructions"));
145-
assert(json.defines("dynamic"));
146-
assert(json.defines("track"));
147-
const auto &instructions{json.at("instructions")};
148-
const auto &dynamic{json.at("dynamic")};
149-
const auto &track{json.at("track")};
150-
assert(instructions.is_array());
151-
assert(dynamic.is_boolean());
152-
assert(track.is_boolean());
153-
return {instructions_from_json(instructions), dynamic.to_boolean(),
154-
track.to_boolean()};
172+
auto from_json(const sourcemeta::core::JSON &json) -> std::optional<Template> {
173+
if (!json.is_object()) {
174+
return std::nullopt;
175+
}
176+
177+
const auto instructions{json.try_at("instructions")};
178+
const auto dynamic{json.try_at("dynamic")};
179+
const auto track{json.try_at("track")};
180+
if (!instructions || !dynamic || !track) {
181+
return std::nullopt;
182+
}
183+
184+
auto instructions_result{instructions_from_json(*instructions)};
185+
if (!instructions_result.has_value() || !dynamic->is_boolean() ||
186+
!track->is_boolean()) {
187+
return std::nullopt;
188+
}
189+
190+
return Template{std::move(instructions_result).value(), dynamic->to_boolean(),
191+
track->to_boolean()};
155192
}
156193

157194
} // namespace sourcemeta::blaze

‎src/compiler/include/sourcemeta/blaze/compiler.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,9 @@ auto SOURCEMETA_BLAZE_COMPILER_EXPORT to_json(const Template &schema_template)
191191
-> sourcemeta::core::JSON;
192192

193193
/// @ingroup compiler
194-
/// Parse a template from JSON. Note that this function assumes that the JSON
195-
/// document represents a valid template and minimal error checking is performed
194+
/// Parse a template from JSON
196195
auto SOURCEMETA_BLAZE_COMPILER_EXPORT
197-
from_json(const sourcemeta::core::JSON &json) -> Template;
196+
from_json(const sourcemeta::core::JSON &json) -> std::optional<Template>;
198197

199198
} // namespace sourcemeta::blaze
200199

‎src/evaluator/include/sourcemeta/blaze/evaluator_string_set.h‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@
77

88
#include <sourcemeta/core/json.h>
99

10-
#include <utility> // std::pair
11-
#include <vector> // std::vector
10+
#include <optional> // std::optional
11+
#include <utility> // std::pair
12+
#include <vector> // std::vector
1213

1314
namespace sourcemeta::blaze {
1415

@@ -51,13 +52,21 @@ class SOURCEMETA_BLAZE_EVALUATOR_EXPORT StringSet {
5152
});
5253
}
5354

54-
static auto from_json(const sourcemeta::core::JSON &value) -> StringSet {
55-
assert(value.is_array());
55+
static auto from_json(const sourcemeta::core::JSON &value)
56+
-> std::optional<StringSet> {
57+
if (!value.is_array()) {
58+
return std::nullopt;
59+
}
60+
5661
StringSet result;
5762
for (const auto &item : value.as_array()) {
58-
assert(item.is_string());
59-
result.insert(
60-
sourcemeta::core::from_json<sourcemeta::core::JSON::String>(item));
63+
auto subvalue{
64+
sourcemeta::core::from_json<sourcemeta::core::JSON::String>(item)};
65+
if (!subvalue.has_value()) {
66+
return std::nullopt;
67+
}
68+
69+
result.insert(std::move(subvalue).value());
6170
}
6271

6372
return result;

‎src/evaluator/include/sourcemeta/blaze/evaluator_value.h‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ struct ValueNone {
2424
return sourcemeta::core::JSON{nullptr};
2525
}
2626

27-
static auto from_json(const sourcemeta::core::JSON &) -> ValueNone {
28-
return {};
27+
static auto from_json(const sourcemeta::core::JSON &)
28+
-> std::optional<ValueNone> {
29+
return ValueNone{};
2930
}
3031
};
3132

@@ -80,12 +81,19 @@ struct ValueRegex {
8081
return sourcemeta::core::to_json(this->second);
8182
}
8283

83-
static auto from_json(const sourcemeta::core::JSON &value) -> ValueRegex {
84-
assert(value.is_string());
84+
static auto from_json(const sourcemeta::core::JSON &value)
85+
-> std::optional<ValueRegex> {
86+
if (!value.is_string()) {
87+
return std::nullopt;
88+
}
89+
8590
auto string{value.to_string()};
8691
auto regex{sourcemeta::core::to_regex(string)};
87-
assert(regex.has_value());
88-
return {std::move(regex).value(), std::move(string)};
92+
if (!regex.has_value()) {
93+
return std::nullopt;
94+
}
95+
96+
return ValueRegex{std::move(regex).value(), std::move(string)};
8997
}
9098
};
9199

‎test/compiler/compiler_json_test.cc‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
{ \
77
const auto result{sourcemeta::blaze::to_json(schema_template)}; \
88
EXPECT_EQ(result, (expected)); \
9-
EXPECT_EQ( \
10-
sourcemeta::blaze::to_json(sourcemeta::blaze::from_json(result)), \
11-
(expected)); \
9+
const auto template_back{sourcemeta::blaze::from_json(result)}; \
10+
EXPECT_TRUE(template_back.has_value()); \
11+
EXPECT_EQ(sourcemeta::blaze::to_json(template_back.value()), (expected)); \
1212
}
1313

1414
TEST(Compiler_JSON, example_1) {
@@ -137,3 +137,16 @@ TEST(Compiler_JSON, example_3) {
137137

138138
EXPECT_BIDIRECTIONAL_JSON(schema_template, expected);
139139
}
140+
141+
TEST(Compiler_JSON, invalid_1) {
142+
const auto input{sourcemeta::core::parse_json(R"JSON({
143+
"dynamic": false,
144+
"track": false,
145+
"instructions": [
146+
{ "t": 99 }
147+
]
148+
})JSON")};
149+
150+
const auto result{sourcemeta::blaze::from_json(input)};
151+
EXPECT_FALSE(result.has_value());
152+
}

‎test/evaluator/evaluator_utils.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,10 @@ inline auto FIRST_PROPERTY_IS(const sourcemeta::core::JSON &document,
4444
{ \
4545
const auto template_json{sourcemeta::blaze::to_json(compiled_schema)}; \
4646
EXPECT_TRUE(template_json.is_object()); \
47-
const auto template_json_back{sourcemeta::blaze::to_json( \
48-
sourcemeta::blaze::from_json(template_json))}; \
47+
const auto template_back{sourcemeta::blaze::from_json(template_json)}; \
48+
EXPECT_TRUE(template_back.has_value()); \
49+
const auto template_json_back{ \
50+
sourcemeta::blaze::to_json(template_back.value())}; \
4951
EXPECT_EQ(template_json, template_json_back); \
5052
}
5153

0 commit comments

Comments
 (0)