From aab0a156a618303002e1c54cfd343d6e6de41aeb Mon Sep 17 00:00:00 2001 From: Paul Harris Date: Fri, 18 Oct 2024 09:04:28 +0800 Subject: [PATCH 1/3] Mustache: use signed size() ... later use std::ssize() --- include/crow/mustache.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/crow/mustache.h b/include/crow/mustache.h index 50ffda20a..e5b032eac 100644 --- a/include/crow/mustache.h +++ b/include/crow/mustache.h @@ -526,7 +526,7 @@ namespace crow // NOTE: Already documented in "crow/app.h" body_.substr(matched.start, matched.end - matched.start) + ", " + body_.substr(idx, endIdx - idx)); } - matched.pos = actions_.size(); + matched.pos = static_cast(actions_.size()); } actions_.emplace_back(ActionType::CloseBlock, idx, endIdx, blockPositions.back()); blockPositions.pop_back(); @@ -626,7 +626,7 @@ namespace crow // NOTE: Already documented in "crow/app.h" } // removing standalones - for (int i = actions_.size() - 2; i >= 0; i--) + for (int i = static_cast(actions_.size()) - 2; i >= 0; i--) { if (actions_[i].t == ActionType::Tag || actions_[i].t == ActionType::UnescapeTag) continue; From 3ea04db66e5e38f3b0154ea46ff6b8587c13841d Mon Sep 17 00:00:00 2001 From: Paul Harris Date: Fri, 18 Oct 2024 09:11:38 +0800 Subject: [PATCH 2/3] Add test that shows an infinite loop with an invalid mustache template --- tests/template/crow_extra_mustache_tests.json | 15 ++++++++++ tests/template/mustachetest.cpp | 30 +++++++++++-------- tests/template/test.py | 2 +- 3 files changed, 34 insertions(+), 13 deletions(-) create mode 100644 tests/template/crow_extra_mustache_tests.json diff --git a/tests/template/crow_extra_mustache_tests.json b/tests/template/crow_extra_mustache_tests.json new file mode 100644 index 000000000..f81a18913 --- /dev/null +++ b/tests/template/crow_extra_mustache_tests.json @@ -0,0 +1,15 @@ +{ + "overview": "Check Crow's behaviour when given invalid mustache templates", + "tests": [ + { + "name": "Missing end-tags", + "desc": "Missing end-tags should fail to render ... and not enter infinite loops or other undefined behaviour", + "data": { + "boolean": true + }, + "template": "\"{{#boolean}}{{^boolean}}\"", + "expected": "COMPILE EXCEPTION: crow::mustache error: open tag has no matching end tag {{# {{/ pair: boolean" + } + ], + "__ATTN__": "This file was hand-written" +} diff --git a/tests/template/mustachetest.cpp b/tests/template/mustachetest.cpp index 2701be2c9..55f2700f4 100644 --- a/tests/template/mustachetest.cpp +++ b/tests/template/mustachetest.cpp @@ -18,17 +18,23 @@ string read_all(const string& filename) int main() { - auto data = json::load(read_all("data")); - auto templ = compile(read_all("template")); - auto partials = json::load(read_all("partials")); - set_loader([&](std::string name) -> std::string { - if (partials.count(name)) - { - return partials[name].s(); - } - return ""; - }); - context ctx(data); - cout << templ.render_string(ctx); + try { + auto data = json::load(read_all("data")); + auto templ = compile(read_all("template")); + auto partials = json::load(read_all("partials")); + set_loader([&](std::string name) -> std::string { + if (partials.count(name)) + { + return partials[name].s(); + } + return ""; + }); + context ctx(data); + cout << templ.render_string(ctx); + } + // catch and return compile errors as text, for the python test to compare + catch (invalid_template_exception & err) { + cout << "COMPILE EXCEPTION: " << err.what(); + } return 0; } diff --git a/tests/template/test.py b/tests/template/test.py index 4e2a2c6d9..eb9d9bab7 100755 --- a/tests/template/test.py +++ b/tests/template/test.py @@ -30,7 +30,7 @@ print('Data: ', json.dumps(test["data"])) print('Template: ', test["template"]) print('Expected:', repr(test["expected"])) - print('Actual:', repr(ret)) + print('Actual: ', repr(ret)) assert ret == test["expected"] os.unlink('data') From b90a739422118babdd153bdf3112eee26698ef56 Mon Sep 17 00:00:00 2001 From: Paul Harris Date: Fri, 18 Oct 2024 10:14:15 +0800 Subject: [PATCH 3/3] Mustache: Check start tags were matched to an end tag --- include/crow/mustache.h | 75 ++++++++++++++++++++++++++++++++--------- 1 file changed, 59 insertions(+), 16 deletions(-) diff --git a/include/crow/mustache.h b/include/crow/mustache.h index e5b032eac..e0e4889ec 100644 --- a/include/crow/mustache.h +++ b/include/crow/mustache.h @@ -114,14 +114,37 @@ namespace crow // NOTE: Already documented in "crow/app.h" */ struct Action { + bool has_end_match; + char tag_char; int start; int end; int pos; ActionType t; - Action(ActionType t_, size_t start_, size_t end_, size_t pos_ = 0): - start(static_cast(start_)), end(static_cast(end_)), pos(static_cast(pos_)), t(t_) + + Action(char tag_char_, ActionType t_, size_t start_, size_t end_, size_t pos_ = 0): + has_end_match(false), tag_char(tag_char_), start(static_cast(start_)), end(static_cast(end_)), pos(static_cast(pos_)), t(t_) { } + + bool missing_end_pair() const { + switch (t) + { + case ActionType::Ignore: + case ActionType::Tag: + case ActionType::UnescapeTag: + case ActionType::CloseBlock: + case ActionType::Partial: + return false; + + // requires a match + case ActionType::OpenBlock: + case ActionType::ElseBlock: + return !has_end_match; + + default: + throw std::logic_error("invalid type"); + } + } }; /** @@ -483,7 +506,7 @@ namespace crow // NOTE: Already documented in "crow/app.h" if (idx == body_.npos) { fragments_.emplace_back(static_cast(current), static_cast(body_.size())); - actions_.emplace_back(ActionType::Ignore, 0, 0); + actions_.emplace_back('!', ActionType::Ignore, 0, 0); break; } fragments_.emplace_back(static_cast(current), static_cast(idx)); @@ -500,7 +523,8 @@ namespace crow // NOTE: Already documented in "crow/app.h" throw invalid_template_exception("not matched opening tag"); } current = endIdx + tag_close.size(); - switch (body_[idx]) + char tag_char = body_[idx]; + switch (tag_char) { case '#': idx++; @@ -509,7 +533,7 @@ namespace crow // NOTE: Already documented in "crow/app.h" while (body_[endIdx - 1] == ' ') endIdx--; blockPositions.emplace_back(static_cast(actions_.size())); - actions_.emplace_back(ActionType::OpenBlock, idx, endIdx); + actions_.emplace_back(tag_char, ActionType::OpenBlock, idx, endIdx); break; case '/': idx++; @@ -522,13 +546,18 @@ namespace crow // NOTE: Already documented in "crow/app.h" if (body_.compare(idx, endIdx - idx, body_, matched.start, matched.end - matched.start) != 0) { - throw invalid_template_exception("not matched {{# {{/ pair: " + - body_.substr(matched.start, matched.end - matched.start) + ", " + - body_.substr(idx, endIdx - idx)); + throw invalid_template_exception( + std::string("not matched {{") + + matched.tag_char + + "{{/ pair: " + + body_.substr(matched.start, matched.end - matched.start) + ", " + + body_.substr(idx, endIdx - idx) + ); } matched.pos = static_cast(actions_.size()); + matched.has_end_match = true; } - actions_.emplace_back(ActionType::CloseBlock, idx, endIdx, blockPositions.back()); + actions_.emplace_back(tag_char, ActionType::CloseBlock, idx, endIdx, blockPositions.back()); blockPositions.pop_back(); break; case '^': @@ -538,11 +567,11 @@ namespace crow // NOTE: Already documented in "crow/app.h" while (body_[endIdx - 1] == ' ') endIdx--; blockPositions.emplace_back(static_cast(actions_.size())); - actions_.emplace_back(ActionType::ElseBlock, idx, endIdx); + actions_.emplace_back(tag_char, ActionType::ElseBlock, idx, endIdx); break; case '!': // do nothing action - actions_.emplace_back(ActionType::Ignore, idx + 1, endIdx); + actions_.emplace_back(tag_char, ActionType::Ignore, idx + 1, endIdx); break; case '>': // partial idx++; @@ -550,7 +579,7 @@ namespace crow // NOTE: Already documented in "crow/app.h" idx++; while (body_[endIdx - 1] == ' ') endIdx--; - actions_.emplace_back(ActionType::Partial, idx, endIdx); + actions_.emplace_back(tag_char, ActionType::Partial, idx, endIdx); break; case '{': if (tag_open != "{{" || tag_close != "}}") @@ -565,7 +594,7 @@ namespace crow // NOTE: Already documented in "crow/app.h" idx++; while (body_[endIdx - 1] == ' ') endIdx--; - actions_.emplace_back(ActionType::UnescapeTag, idx, endIdx); + actions_.emplace_back(tag_char, ActionType::UnescapeTag, idx, endIdx); current++; break; case '&': @@ -574,12 +603,12 @@ namespace crow // NOTE: Already documented in "crow/app.h" idx++; while (body_[endIdx - 1] == ' ') endIdx--; - actions_.emplace_back(ActionType::UnescapeTag, idx, endIdx); + actions_.emplace_back(tag_char, ActionType::UnescapeTag, idx, endIdx); break; case '=': // tag itself is no-op idx++; - actions_.emplace_back(ActionType::Ignore, idx, endIdx); + actions_.emplace_back(tag_char, ActionType::Ignore, idx, endIdx); endIdx--; if (body_[endIdx] != '=') throw invalid_template_exception("{{=: not matching = tag: " + body_.substr(idx, endIdx - idx)); @@ -620,11 +649,25 @@ namespace crow // NOTE: Already documented in "crow/app.h" idx++; while (body_[endIdx - 1] == ' ') endIdx--; - actions_.emplace_back(ActionType::Tag, idx, endIdx); + actions_.emplace_back(tag_char, ActionType::Tag, idx, endIdx); break; } } + // ensure no unmatched tags + for (int i = 0; i < static_cast(actions_.size()); i++) + { + if (actions_[i].missing_end_pair()) + { + throw invalid_template_exception( + std::string("open tag has no matching end tag {{") + + actions_[i].tag_char + + " {{/ pair: " + + body_.substr(actions_[i].start, actions_[i].end - actions_[i].start) + ); + } + } + // removing standalones for (int i = static_cast(actions_.size()) - 2; i >= 0; i--) {