Skip to content

Commit bf03f40

Browse files
authored
fix json/yaml issues with illegal format (alibaba#1054)
1 parent 39947fd commit bf03f40

2 files changed

Lines changed: 52 additions & 7 deletions

File tree

ecosystem/simple_dom.cpp

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@ struct JHandler : public BaseReaderHandler<UTF8<>, JHandler> {
177177
vector<vector<JNode>> _nodes{1};
178178
str _key;
179179
JNode* _root;
180-
JHandler(const char* text, bool text_ownership) {
180+
JHandler(const char* text) {
181181
assert(_nodes.size() == 1);
182-
_root = new JNode(text, text_ownership);
182+
_root = new JNode(text, false);
183183
}
184184
~JHandler() {
185185
delete _root;
@@ -277,15 +277,26 @@ static bool fix_trail(estring_view text, char header, char trailer) {
277277
}
278278

279279
static NodeImpl* parse_json(char* text, size_t size, int flags) {
280+
// without kParseStopWhenDoneFlag, rapidjson will expect the last character of
281+
// the JSON string to be '\0'.
280282
const auto kFlags = kParseNumbersAsStringsFlag | kParseBoolsAsStringFlag |
281-
kParseInsituFlag | kParseCommentsFlag | kParseTrailingCommasFlag;
283+
kParseInsituFlag | kParseCommentsFlag |
284+
kParseTrailingCommasFlag | kParseStopWhenDoneFlag;
282285
if (!fix_trail({text, size}, '{', '}')) return nullptr;
283-
JHandler h(text, flags & DOC_FREE_TEXT_ON_DESTRUCTION);
286+
JHandler h(text);
284287
using Encoding = UTF8<>;
285288
GenericInsituStringStream<Encoding> s(text);
286289
GenericReader<Encoding, Encoding> reader;
287-
reader.Parse<kFlags>(s, h);
288-
return h.get_root();
290+
auto res = reader.Parse<kFlags>(s, h);
291+
if (!res) {
292+
LOG_ERROR("json parsing failed with error ` at `, origin size `",
293+
res.Code(), res.Offset(), size);
294+
return nullptr;
295+
}
296+
auto root = h.get_root();
297+
if (flags & DOC_FREE_TEXT_ON_DESTRUCTION)
298+
root->_flags |= NodeImpl::FLAG_TEXT_OWNERSHIP;
299+
return root;
289300
}
290301

291302
using namespace rapidxml;
@@ -359,6 +370,12 @@ class YAMLNode : public DocNode<YAMLNode> {
359370
};
360371

361372
static NodeImpl* parse_yaml(char* text, size_t size, int flags) {
373+
ryml::s_default_callbacks.m_error = [](const char* msg, size_t msg_len,
374+
ryml::Location location,
375+
void* user_data) {
376+
LOG_ERROR("yaml parsing failed with `", std::string_view(msg, msg_len));
377+
throw std::runtime_error("yaml parsing failed");
378+
};
362379
auto yaml = ryml::parse_in_place({text, size});
363380
auto root = make_unique<YAMLNode>(text, flags & DOC_FREE_TEXT_ON_DESTRUCTION);
364381
assert(root);
@@ -451,7 +468,7 @@ Node parse(char* text, size_t size, int flags) {
451468
constexpr static Parser parsers[] = {&parse_json, &parse_xml,
452469
&parse_yaml, &parse_ini};
453470
auto i = flags & DOC_TYPE_MASK;
454-
if ((size_t) i > LEN(parsers)) {
471+
if ((size_t) i >= LEN(parsers)) {
455472
if (flags & DOC_FREE_TEXT_IF_PARSING_FAILED) free(text);
456473
LOG_ERROR_RETURN(EINVAL, nullptr, "invalid document type ", HEX(i));
457474
}

ecosystem/test/test_simple_dom.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,34 @@ TEST(simple_dom, json) {
218218
expect_eq_vals(doc["a"], {"1", "2", "3", "4"});
219219
}
220220

221+
TEST(simple_dom, illegal_formats) {
222+
auto ck_parse_copy = [](const std::vector<string>& formats, int type) {
223+
for (const auto& s : formats) {
224+
auto doc = parse_copy((const char*)s.data(), s.size(), type);
225+
EXPECT_FALSE(doc);
226+
}
227+
};
228+
auto ck_parse = [](std::vector<string>& formats, int type) {
229+
for (auto& s : formats) {
230+
auto doc = parse((char*)s.data(), s.size(), type);
231+
EXPECT_FALSE(doc);
232+
}
233+
};
234+
235+
std::vector<std::string> xmls = {"xml1", "<<xml2>>", "4<xml>"};
236+
ck_parse_copy(xmls, DOC_XML);
237+
ck_parse(xmls, DOC_XML);
238+
std::vector<std::string> jsons = {"json1", "33{{json22}}22", "{3{json}}"};
239+
ck_parse_copy(jsons, DOC_JSON);
240+
ck_parse(jsons, DOC_JSON);
241+
std::vector<std::string> yamls = {"[1,2,3,4,5", "{{a:1,b:}", "{yaml3}}"};
242+
ck_parse_copy(yamls, DOC_YAML);
243+
ck_parse(yamls, DOC_YAML);
244+
std::vector<std::string> inis = {"ini1:", ":ini2", "ini3"};
245+
ck_parse_copy(inis, DOC_INI);
246+
ck_parse(inis, DOC_INI);
247+
}
248+
221249
TEST(simple_dom, fix_trail) {
222250
static char s0[] = R"({"a":"b"}})";
223251
static char s1[] = R"({"a":"b")";

0 commit comments

Comments
 (0)