From 9e26450d734e718520846b93da75b836301db4d4 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Wed, 22 Jul 2015 17:52:07 +0100 Subject: [PATCH 1/2] Parse conditions and regex during YAML decoding. To prevent syntax errors sneaking in and later causing problems. Closes #473. --- src/backend/masterlist.cpp | 10 --- src/backend/metadata/condition_grammar.h | 26 ++++--- src/backend/metadata/file.h | 8 +++ src/backend/metadata/message.h | 9 +++ src/backend/metadata/plugin_metadata.cpp | 18 ----- src/backend/metadata/plugin_metadata.h | 12 +++- src/backend/metadata/tag.h | 8 +++ .../metadata/test_conditional_metadata.h | 4 ++ src/tests/backend/metadata/test_file.h | 11 +-- src/tests/backend/metadata/test_message.h | 9 ++- .../backend/metadata/test_plugin_metadata.h | 70 +++++++++---------- src/tests/backend/metadata/test_tag.h | 9 ++- src/validator/main.cpp | 8 --- 13 files changed, 112 insertions(+), 90 deletions(-) diff --git a/src/backend/masterlist.cpp b/src/backend/masterlist.cpp index 6c64574a..238f8d64 100644 --- a/src/backend/masterlist.cpp +++ b/src/backend/masterlist.cpp @@ -368,16 +368,6 @@ namespace loot { try { this->Load(path); - for (auto &plugin : plugins) { - plugin.ParseAllConditions(); - } - for (auto &plugin : regexPlugins) { - plugin.ParseAllConditions(); - } - for (auto &message : messages) { - message.ParseCondition(); - } - parsingFailed = false; } catch (std::exception& e) { diff --git a/src/backend/metadata/condition_grammar.h b/src/backend/metadata/condition_grammar.h index c26a659f..a15f7f2b 100644 --- a/src/backend/metadata/condition_grammar.h +++ b/src/backend/metadata/condition_grammar.h @@ -184,6 +184,13 @@ namespace loot { In C++ string literals, the backslash must be escaped once more to give "\\\\". Split the regex with another regex! */ + try { + std::regex(regex, std::regex::ECMAScript | std::regex::icase); + } + catch (std::regex_error& e) { + throw loot::error(loot::error::invalid_args, (boost::format(boost::locale::translate("Invalid regex string \"%1%\": %2%")) % regex % e.what()).str()); + } + std::regex sepReg("/|(\\\\\\\\)", std::regex::ECMAScript | std::regex::icase); std::vector components; @@ -213,23 +220,24 @@ namespace loot { try { reg = std::regex(filename, std::regex::ECMAScript | std::regex::icase); } - catch (std::exception& /*e*/) { + catch (std::regex_error& e) { BOOST_LOG_TRIVIAL(error) << "Invalid regex string:" << filename; - throw loot::error(loot::error::invalid_args, boost::locale::translate("Invalid regex string:").str() + " " + filename); + throw loot::error(loot::error::invalid_args, (boost::format(boost::locale::translate("Invalid regex string \"%1%\": %2%")) % filename % e.what()).str()); } return std::pair(parent, reg); } void CheckRegex(bool& result, const std::string& regexStr) const { - if (_parseOnly) - return; result = false; BOOST_LOG_TRIVIAL(trace) << "Checking to see if any files matching the regex \"" << regexStr << "\" exist."; std::pair pathRegex = SplitRegex(regexStr); + if (_parseOnly) + return; + //Now we have a valid parent path and a regex filename. Check that //the parent path exists and is a directory. @@ -249,14 +257,15 @@ namespace loot { } void CheckMany(bool& result, const std::string& regexStr) const { - if (_parseOnly) - return; result = false; BOOST_LOG_TRIVIAL(trace) << "Checking to see if more than one file matching the regex \"" << regexStr << "\" exist."; std::pair pathRegex = SplitRegex(regexStr); + if (_parseOnly) + return; + //Now we have a valid parent path and a regex filename. Check that //the parent path exists and is a directory. @@ -278,8 +287,6 @@ namespace loot { } void CheckSum(bool& result, const std::string& file, const uint32_t checksum) { - if (_parseOnly) - return; BOOST_LOG_TRIVIAL(trace) << "Checking the CRC of the file \"" << file << "\"."; @@ -288,6 +295,9 @@ namespace loot { throw loot::error(loot::error::invalid_args, boost::locale::translate("Invalid file path:").str() + " " + file); } + if (_parseOnly) + return; + uint32_t crc; std::unordered_map::iterator it = _game->crcCache.find(boost::locale::to_lower(file)); diff --git a/src/backend/metadata/file.h b/src/backend/metadata/file.h index e6ed2a8e..6c22d132 100644 --- a/src/backend/metadata/file.h +++ b/src/backend/metadata/file.h @@ -83,6 +83,14 @@ namespace YAML { else rhs = loot::File(node.as()); + // Test condition syntax. + try { + rhs.ParseCondition(); + } + catch (std::exception& e) { + throw RepresentationException(node.Mark(), std::string("bad conversion: invalid condition syntax: ") + e.what()); + } + return true; } }; diff --git a/src/backend/metadata/message.h b/src/backend/metadata/message.h index 4e85ed46..ef21335c 100644 --- a/src/backend/metadata/message.h +++ b/src/backend/metadata/message.h @@ -144,6 +144,15 @@ namespace YAML { condition = node["condition"].as(); rhs = loot::Message(typeNo, content, condition); + + // Test condition syntax. + try { + rhs.ParseCondition(); + } + catch (std::exception& e) { + throw RepresentationException(node.Mark(), std::string("bad conversion: invalid condition syntax: ") + e.what()); + } + return true; } }; diff --git a/src/backend/metadata/plugin_metadata.cpp b/src/backend/metadata/plugin_metadata.cpp index 281a082e..4ef39356 100644 --- a/src/backend/metadata/plugin_metadata.cpp +++ b/src/backend/metadata/plugin_metadata.cpp @@ -332,24 +332,6 @@ namespace loot { return *this; } - void PluginMetadata::ParseAllConditions() const { - for (const File& file : loadAfter) { - file.ParseCondition(); - } - for (const File& file : requirements) { - file.ParseCondition(); - } - for (const File& file : incompatibilities) { - file.ParseCondition(); - } - for (const Message& message : messages) { - message.ParseCondition(); - } - for (const Tag& tag : tags) { - tag.ParseCondition(); - } - } - bool PluginMetadata::HasNameOnly() const { return !IsPriorityExplicit() && loadAfter.empty() && requirements.empty() && incompatibilities.empty() && messages.empty() && tags.empty() && _dirtyInfo.empty() && _locations.empty(); } diff --git a/src/backend/metadata/plugin_metadata.h b/src/backend/metadata/plugin_metadata.h index 95da590c..b9f69e94 100644 --- a/src/backend/metadata/plugin_metadata.h +++ b/src/backend/metadata/plugin_metadata.h @@ -37,6 +37,7 @@ #include #include #include +#include #include @@ -89,7 +90,6 @@ namespace loot { void Locations(const std::set& locations); PluginMetadata& EvalAllConditions(Game& game, const unsigned int language); - void ParseAllConditions() const; bool HasNameOnly() const; bool IsRegexPlugin() const; bool IsPriorityExplicit() const; @@ -160,6 +160,16 @@ namespace YAML { rhs = loot::PluginMetadata(node["name"].as()); + // Test for valid regex. + if (rhs.IsRegexPlugin()) { + try { + std::regex(rhs.Name(), std::regex::ECMAScript | std::regex::icase); + } + catch (std::regex_error& e) { + throw RepresentationException(node.Mark(), std::string("bad conversion: invalid regex in 'name' key: ") + e.what()); + } + } + if (node["enabled"]) rhs.Enabled(node["enabled"].as()); diff --git a/src/backend/metadata/tag.h b/src/backend/metadata/tag.h index 6676cad4..8188d2a0 100644 --- a/src/backend/metadata/tag.h +++ b/src/backend/metadata/tag.h @@ -82,6 +82,14 @@ namespace YAML { else rhs = loot::Tag(tag, true, condition); + // Test condition syntax. + try { + rhs.ParseCondition(); + } + catch (std::exception& e) { + throw RepresentationException(node.Mark(), std::string("bad conversion: invalid condition syntax: ") + e.what()); + } + return true; } }; diff --git a/src/tests/backend/metadata/test_conditional_metadata.h b/src/tests/backend/metadata/test_conditional_metadata.h index b1afe8a8..25de042e 100644 --- a/src/tests/backend/metadata/test_conditional_metadata.h +++ b/src/tests/backend/metadata/test_conditional_metadata.h @@ -72,6 +72,10 @@ TEST_F(ConditionalMetadata, ParseCondition) { cm = loot::ConditionalMetadata("condition"); EXPECT_THROW(cm.ParseCondition(), loot::error); + // Check that invalid regex also throws. + cm = loot::ConditionalMetadata("regex(\"RagnvaldBook(Farengar(+Ragnvald)?)?\\.esp\")"); + EXPECT_THROW(cm.ParseCondition(), loot::error); + cm = loot::ConditionalMetadata("file(\"Blank.esm\")"); EXPECT_NO_THROW(cm.ParseCondition()); diff --git a/src/tests/backend/metadata/test_file.h b/src/tests/backend/metadata/test_file.h index 6977cbd7..1c81ea1e 100644 --- a/src/tests/backend/metadata/test_file.h +++ b/src/tests/backend/metadata/test_file.h @@ -144,11 +144,11 @@ TEST(File, YamlEncode) { } TEST(File, YamlDecode) { - YAML::Node node = YAML::Load("{name: name1, display: display1, condition: condition1}"); + YAML::Node node = YAML::Load("{name: name1, display: display1, condition: 'file(\"Foo.esp\")'}"); File file = node.as(); EXPECT_EQ("name1", file.Name()); EXPECT_EQ("display1", file.DisplayName()); - EXPECT_EQ("condition1", file.Condition()); + EXPECT_EQ("file(\"Foo.esp\")", file.Condition()); node = YAML::Load("name1"); file = node.as(); @@ -162,11 +162,14 @@ TEST(File, YamlDecode) { EXPECT_EQ("display1", file.DisplayName()); EXPECT_EQ("", file.Condition()); - node = YAML::Load("{name: name1, condition: condition1}"); + node = YAML::Load("{name: name1, condition: 'file(\"Foo.esp\")'}"); file = node.as(); EXPECT_EQ("name1", file.Name()); EXPECT_EQ("name1", file.DisplayName()); - EXPECT_EQ("condition1", file.Condition()); + EXPECT_EQ("file(\"Foo.esp\")", file.Condition()); + + node = YAML::Load("{name: name1, condition: invalid}"); + EXPECT_THROW(node.as(), YAML::RepresentationException); node = YAML::Load("[0, 1, 2]"); EXPECT_ANY_THROW(node.as()); diff --git a/src/tests/backend/metadata/test_message.h b/src/tests/backend/metadata/test_message.h index c9d517e6..7b483df9 100644 --- a/src/tests/backend/metadata/test_message.h +++ b/src/tests/backend/metadata/test_message.h @@ -326,11 +326,11 @@ TEST_F(Message, YamlDecode) { node = YAML::Load("type: say\n" "content: content1\n" - "condition: condition1"); + "condition: 'file(\"Foo.esp\")'"); message = node.as(); EXPECT_EQ(loot::Message::say, message.Type()); EXPECT_EQ(MessageContents({MessageContent("content1", Language::english)}), message.Content()); - EXPECT_EQ("condition1", message.Condition()); + EXPECT_EQ("file(\"Foo.esp\")", message.Condition()); node = YAML::Load("type: say\n" "content:\n" @@ -405,6 +405,11 @@ TEST_F(Message, YamlDecode) { EXPECT_EQ(MessageContents({MessageContent("con%1%tent1", Language::english)}), message.Content()); EXPECT_EQ("", message.Condition()); + node = YAML::Load("type: say\n" + "content: content1\n" + "condition: invalid"); + EXPECT_THROW(node.as(), YAML::RepresentationException); + node = YAML::Load("scalar"); EXPECT_THROW(node.as(), YAML::RepresentationException); diff --git a/src/tests/backend/metadata/test_plugin_metadata.h b/src/tests/backend/metadata/test_plugin_metadata.h index b92afa85..6a1a0ed2 100644 --- a/src/tests/backend/metadata/test_plugin_metadata.h +++ b/src/tests/backend/metadata/test_plugin_metadata.h @@ -502,7 +502,6 @@ TEST_F(PluginMetadata, EvalAllConditions) { "msg:\n" " - type: say\n" " content: 'content'\n" - " condition: 'condition'\n" "tag:\n" " - name: Relev\n" " condition: 'file(\"Blank.missing.esm\")'\n" @@ -517,8 +516,6 @@ TEST_F(PluginMetadata, EvalAllConditions) { " nav: 2" ).as()); - EXPECT_ANY_THROW(pm.EvalAllConditions(game, loot::Language::english)); - pm.Messages({loot::Message(loot::Message::say, "content")}); EXPECT_NO_THROW(pm.EvalAllConditions(game, loot::Language::english)); EXPECT_EQ(std::set({ @@ -537,33 +534,6 @@ TEST_F(PluginMetadata, EvalAllConditions) { }), pm.DirtyInfo()); } -TEST_F(PluginMetadata, ParseAllConditions) { - loot::PluginMetadata pm(YAML::Load( - "name: 'Blank.esp'\n" - "after:\n" - " - name: 'Blank.esm'\n" - " condition: 'file(\"Blank.esm\")'\n" - "req:\n" - " - name: 'Blank.esm'\n" - " condition: 'file(\"Blank.missing.esm\")'\n" - "inc:\n" - " - name: 'Blank.esm'\n" - " condition: 'file(\"Blank.esm\")'\n" - "msg:\n" - " - type: say\n" - " content: 'content'\n" - " condition: 'condition'\n" - "tag:\n" - " - name: Relev\n" - " condition: 'file(\"Blank.missing.esm\")'" - ).as()); - - EXPECT_ANY_THROW(pm.ParseAllConditions()); - - pm.Messages({loot::Message(loot::Message::say, "content")}); - EXPECT_NO_THROW(pm.ParseAllConditions()); -} - TEST_F(PluginMetadata, HasNameOnly) { loot::PluginMetadata pm; EXPECT_TRUE(pm.HasNameOnly()); @@ -823,9 +793,6 @@ TEST_F(PluginMetadata, YamlDecode) { YAML::Node node; loot::PluginMetadata pm; - node = YAML::Load("Blank.esp"); - EXPECT_ANY_THROW(node.as()); - node = YAML::Load("name: Blank.esp"); pm = node.as(); EXPECT_EQ("Blank.esp", pm.Name()); @@ -887,13 +854,44 @@ TEST_F(PluginMetadata, YamlDecode) { " util: 'utility'\n" " udr: 1\n" " nav: 2"); - EXPECT_ANY_THROW(node.as()); + EXPECT_THROW(node.as(), YAML::RepresentationException); + + // Don't allow invalid regex. + node = YAML::Load("name: 'RagnvaldBook(Farengar(+Ragnvald)?)?\\.esp'\n" + "dirty:\n" + " - crc: 0x5\n" + " util: 'utility'\n" + " udr: 1\n" + " nav: 2"); + EXPECT_THROW(node.as(), YAML::RepresentationException); + + // Catch condition syntax errors. + node = YAML::Load( + "name: 'Blank.esp'\n" + "after:\n" + " - name: 'Blank.esm'\n" + " condition: 'file(\"Blank.esm\")'\n" + "req:\n" + " - name: 'Blank.esm'\n" + " condition: 'file(\"Blank.missing.esm\")'\n" + "inc:\n" + " - name: 'Blank.esm'\n" + " condition: 'file(\"Blank.esm\")'\n" + "msg:\n" + " - type: say\n" + " content: 'content'\n" + " condition: 'condition'\n" + "tag:\n" + " - name: Relev\n" + " condition: 'file(\"Blank.missing.esm\")'" + ); + EXPECT_THROW(node.as(), YAML::RepresentationException); node = YAML::Load("scalar"); - EXPECT_ANY_THROW(node.as()); + EXPECT_THROW(node.as(), YAML::RepresentationException); node = YAML::Load("[0, 1, 2]"); - EXPECT_ANY_THROW(node.as()); + EXPECT_THROW(node.as(), YAML::RepresentationException); } #endif diff --git a/src/tests/backend/metadata/test_tag.h b/src/tests/backend/metadata/test_tag.h index 697e90fe..245371fa 100644 --- a/src/tests/backend/metadata/test_tag.h +++ b/src/tests/backend/metadata/test_tag.h @@ -164,14 +164,17 @@ TEST(Tag, YamlDecode) { EXPECT_FALSE(tag.IsAddition()); EXPECT_EQ("", tag.Condition()); - node = YAML::Load("{name: name1, condition: condition1}"); + node = YAML::Load("{name: name1, condition: 'file(\"Foo.esp\")'}"); tag = node.as(); EXPECT_EQ("name1", tag.Name()); EXPECT_TRUE(tag.IsAddition()); - EXPECT_EQ("condition1", tag.Condition()); + EXPECT_EQ("file(\"Foo.esp\")", tag.Condition()); + + node = YAML::Load("{name: name1, condition: invalid}"); + EXPECT_THROW(node.as(), YAML::RepresentationException); node = YAML::Load("[0, 1, 2]"); - EXPECT_ANY_THROW(node.as()); + EXPECT_THROW(node.as(), YAML::RepresentationException); } #endif diff --git a/src/validator/main.cpp b/src/validator/main.cpp index 6c4d5f20..492dfd22 100644 --- a/src/validator/main.cpp +++ b/src/validator/main.cpp @@ -65,14 +65,6 @@ int main(int argc, char **argv) { // Test YAML parsing. loot::MetadataList metadata; metadata.Load(argv[1]); - - // Test condition parsing. - for (auto &plugin : metadata.Plugins()) { - plugin.ParseAllConditions(); - } - for (auto &message : metadata.messages) { - message.ParseCondition(); - } } catch (std::exception& e) { std::cout << "ERROR: " << e.what() << std::endl << std::endl; From ff2b62fab28ef4a1e04dd73d8fa85c07c2e0b1ec Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Wed, 22 Jul 2015 20:07:48 +0100 Subject: [PATCH 2/2] Condition grammar improvements. Check for safe file paths when parsing, simplify grammar constructor to avoid invalid arguments, simplify some code. --- src/backend/metadata/condition_grammar.h | 70 +++++++-------- src/backend/metadata/conditional_metadata.cpp | 4 +- .../backend/metadata/test_condition_grammar.h | 90 +++++++++++-------- src/tests/fixtures.h | 18 +++- 4 files changed, 103 insertions(+), 79 deletions(-) diff --git a/src/backend/metadata/condition_grammar.h b/src/backend/metadata/condition_grammar.h index a15f7f2b..3d12aead 100644 --- a/src/backend/metadata/condition_grammar.h +++ b/src/backend/metadata/condition_grammar.h @@ -64,10 +64,8 @@ namespace loot { template class ConditionGrammar : public qi::grammar < Iterator, bool(), Skipper > { public: - ConditionGrammar(Game * game, bool parseOnly) : ConditionGrammar::base_type(expression, "condition grammar"), _game(game), _parseOnly(parseOnly) { - if (!_parseOnly && _game == nullptr) - throw error(error::invalid_args, "A valid game pointer was not passed during a condition evaluation."); - + ConditionGrammar() : ConditionGrammar(nullptr) {} + ConditionGrammar(Game * game) : ConditionGrammar::base_type(expression, "condition grammar"), _game(game) { expression = qi::eps > compound[qi::labels::_val = qi::labels::_1] @@ -142,13 +140,9 @@ namespace loot { qi::rule invalidPathChars; Game * _game; - bool _parseOnly; //Eval's exact paths. Check for files and ghosted plugins. void CheckFile(bool& result, const std::string& file) const { - if (_parseOnly) - return; - BOOST_LOG_TRIVIAL(trace) << "Checking to see if the file \"" << file << "\" exists."; if (file == "LOOT") { @@ -161,6 +155,9 @@ namespace loot { throw loot::error(loot::error::invalid_args, boost::locale::translate("Invalid file path:").str() + " " + file); } + if (_game == nullptr) + return; + if (boost::iends_with(file, ".esp") || boost::iends_with(file, ".esm")) result = boost::filesystem::exists(_game->DataPath() / file) || boost::filesystem::exists(_game->DataPath() / (file + ".ghost")); else @@ -191,27 +188,20 @@ namespace loot { throw loot::error(loot::error::invalid_args, (boost::format(boost::locale::translate("Invalid regex string \"%1%\": %2%")) % regex % e.what()).str()); } - std::regex sepReg("/|(\\\\\\\\)", std::regex::ECMAScript | std::regex::icase); + std::regex sepReg("/|(\\\\\\\\)", std::regex::ECMAScript); - std::vector components; std::sregex_token_iterator it(regex.begin(), regex.end(), sepReg, -1); - std::sregex_token_iterator itend; - for (; it != itend; ++it) { - components.push_back(*it); - } + std::vector components(it, std::sregex_token_iterator()); std::string filename = components.back(); components.pop_back(); boost::filesystem::path parent; - for (std::vector::const_iterator it = components.begin(), endIt = components.end()--; it != endIt; ++it) { - if (*it == ".") - continue; - - parent += *it; + for (const auto& component : components) { + parent /= component; } - if (!IsSafePath(parent.string())) { + if (!IsSafePath(parent)) { BOOST_LOG_TRIVIAL(error) << "Invalid folder path: " << parent; throw loot::error(loot::error::invalid_args, boost::locale::translate("Invalid folder path:").str() + " " + parent.string()); } @@ -235,7 +225,7 @@ namespace loot { std::pair pathRegex = SplitRegex(regexStr); - if (_parseOnly) + if (_game == nullptr) return; //Now we have a valid parent path and a regex filename. Check that @@ -263,7 +253,7 @@ namespace loot { std::pair pathRegex = SplitRegex(regexStr); - if (_parseOnly) + if (_game == nullptr) return; //Now we have a valid parent path and a regex filename. Check that @@ -287,7 +277,6 @@ namespace loot { } void CheckSum(bool& result, const std::string& file, const uint32_t checksum) { - BOOST_LOG_TRIVIAL(trace) << "Checking the CRC of the file \"" << file << "\"."; if (!IsSafePath(file)) { @@ -295,7 +284,7 @@ namespace loot { throw loot::error(loot::error::invalid_args, boost::locale::translate("Invalid file path:").str() + " " + file); } - if (_parseOnly) + if (_game == nullptr) return; uint32_t crc; @@ -322,12 +311,13 @@ namespace loot { } void CheckVersion(bool& result, const std::string& file, const std::string& version, const std::string& comparator) const { - if (_parseOnly) - return; - BOOST_LOG_TRIVIAL(trace) << "Checking version of file \"" << file << "\"."; CheckFile(result, file); + + if (_game == nullptr) + return; + if (!result) { if (comparator == "!=" || comparator == "<" || comparator == "<=") result = true; @@ -360,14 +350,14 @@ namespace loot { } void CheckActive(bool& result, const std::string& file) const { - if (_parseOnly) - return; - if (!IsSafePath(file)) { BOOST_LOG_TRIVIAL(error) << "Invalid file path: " << file; throw loot::error(loot::error::invalid_args, boost::locale::translate("Invalid file path:").str() + " " + file); } + if (_game == nullptr) + return; + if (file == "LOOT") result = false; else @@ -386,19 +376,21 @@ namespace loot { } //Checks that the path (not regex) doesn't go outside any game folders. - bool IsSafePath(const std::string& path) const { + bool IsSafePath(const boost::filesystem::path& path) const { BOOST_LOG_TRIVIAL(trace) << "Checking to see if the path \"" << path << "\" is safe."; - std::vector components; - boost::split(components, path, boost::is_any_of("/\\")); - components.pop_back(); - std::string parent_path; - for (auto it = components.cbegin(), endIt = components.cend()--; it != endIt; ++it) { - if (*it == ".") + boost::filesystem::path temp; + for (const auto& component : path) { + if (component == ".") continue; - parent_path += *it + '/'; + + if (component == ".." && temp.filename() == "..") + return false; + + temp /= component; } - return !boost::contains(parent_path, "../../"); + + return true; } }; } diff --git a/src/backend/metadata/conditional_metadata.cpp b/src/backend/metadata/conditional_metadata.cpp index da7fdd71..078877aa 100644 --- a/src/backend/metadata/conditional_metadata.cpp +++ b/src/backend/metadata/conditional_metadata.cpp @@ -55,7 +55,7 @@ namespace loot { if (it != game.conditionCache.end()) return it->second; - ConditionGrammar grammar(&game, false); + ConditionGrammar grammar(&game); boost::spirit::qi::space_type skipper; std::string::const_iterator begin, end; bool eval; @@ -88,7 +88,7 @@ namespace loot { BOOST_LOG_TRIVIAL(trace) << "Testing condition syntax: " << _condition; - ConditionGrammar grammar(nullptr, true); + ConditionGrammar grammar(nullptr); boost::spirit::qi::space_type skipper; std::string::const_iterator begin, end; diff --git a/src/tests/backend/metadata/test_condition_grammar.h b/src/tests/backend/metadata/test_condition_grammar.h index 07a373fc..ef68b746 100644 --- a/src/tests/backend/metadata/test_condition_grammar.h +++ b/src/tests/backend/metadata/test_condition_grammar.h @@ -34,14 +34,12 @@ class ConditionGrammar : public SkyrimTest {}; typedef loot::ConditionGrammar Grammar; TEST_F(ConditionGrammar, Constructor) { - EXPECT_NO_THROW(Grammar cg(nullptr, true)); - EXPECT_THROW(Grammar cg(nullptr, false), loot::error); + EXPECT_NO_THROW(Grammar cg(nullptr)); loot::Game game(loot::Game::tes5); game.SetGamePath(dataPath.parent_path()); - EXPECT_NO_THROW(Grammar cg(&game, true)); - EXPECT_NO_THROW(Grammar cg(&game, false)); + EXPECT_NO_THROW(Grammar cg(&game)); } TEST_F(ConditionGrammar, InvalidSyntax) { @@ -51,7 +49,7 @@ TEST_F(ConditionGrammar, InvalidSyntax) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("file(foo)"); std::string::const_iterator begin = condition.begin(); @@ -67,7 +65,7 @@ TEST_F(ConditionGrammar, EmptyCondition) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition(""); std::string::const_iterator begin = condition.begin(); @@ -83,13 +81,13 @@ TEST_F(ConditionGrammar, FileConditionTrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("file(\"Blank.esm\")"); std::string::const_iterator begin = condition.begin(); std::string::const_iterator end = condition.end(); - EXPECT_NO_THROW(r = boost::spirit::qi::phrase_parse(begin, end, cg, skipper, eval)); + r = boost::spirit::qi::phrase_parse(begin, end, cg, skipper, eval); EXPECT_TRUE(r); EXPECT_TRUE(eval); } @@ -101,7 +99,7 @@ TEST_F(ConditionGrammar, FileConditionFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("file(\"Blank.missing.esm\")"); std::string::const_iterator begin = condition.begin(); @@ -119,7 +117,7 @@ TEST_F(ConditionGrammar, UnsafePath) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("file(\"../../Blank.esm\")"); std::string::const_iterator begin = condition.begin(); @@ -135,7 +133,7 @@ TEST_F(ConditionGrammar, RegexConditionTrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("regex(\"Blank.+\\.esm\")"); std::string::const_iterator begin = condition.begin(); @@ -153,7 +151,7 @@ TEST_F(ConditionGrammar, RegexConditionFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("regex(\"Blank\\.m.+\\.esm\")"); std::string::const_iterator begin = condition.begin(); @@ -164,6 +162,24 @@ TEST_F(ConditionGrammar, RegexConditionFalse) { EXPECT_FALSE(eval); } +TEST_F(ConditionGrammar, RegexCondition_Subfolder) { + loot::Game game(loot::Game::tes5); + game.SetGamePath(dataPath.parent_path()); + + boost::spirit::qi::space_type skipper; + bool eval = true; + bool r = false; + Grammar cg(&game); + + std::string condition("regex(\"resource\\\\detail\\\\resource\\.txt\")"); + std::string::const_iterator begin = condition.begin(); + std::string::const_iterator end = condition.end(); + + EXPECT_NO_THROW(r = boost::spirit::qi::phrase_parse(begin, end, cg, skipper, eval)); + EXPECT_TRUE(r); + EXPECT_TRUE(eval); +} + TEST_F(ConditionGrammar, ManyConditionTrue) { loot::Game game(loot::Game::tes5); game.SetGamePath(dataPath.parent_path()); @@ -171,7 +187,7 @@ TEST_F(ConditionGrammar, ManyConditionTrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("many(\"Blank.+\\.esm\")"); std::string::const_iterator begin = condition.begin(); @@ -189,7 +205,7 @@ TEST_F(ConditionGrammar, ManyConditionFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("many(\"Blank\\.esm\")"); std::string::const_iterator begin = condition.begin(); @@ -208,7 +224,7 @@ TEST_F(ConditionGrammar, ChecksumConditionTrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("checksum(\"Blank.esp\", E12EFAAA)"); std::string::const_iterator begin = condition.begin(); @@ -227,7 +243,7 @@ TEST_F(ConditionGrammar, ChecksumConditionFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("checksum(\"Blank.esp\", DEADBEEF)"); std::string::const_iterator begin = condition.begin(); @@ -247,7 +263,7 @@ TEST_F(ConditionGrammar, VersionConditionEqualTrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"5.0\", ==)"); std::string::const_iterator begin = condition.begin(); @@ -267,7 +283,7 @@ TEST_F(ConditionGrammar, VersionConditionEqualFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"6.0\", ==)"); std::string::const_iterator begin = condition.begin(); @@ -287,7 +303,7 @@ TEST_F(ConditionGrammar, VersionConditionNotEqualTrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"6.0\", !=)"); std::string::const_iterator begin = condition.begin(); @@ -307,7 +323,7 @@ TEST_F(ConditionGrammar, VersionConditionNotEqualFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"5.0\", !=)"); std::string::const_iterator begin = condition.begin(); @@ -327,7 +343,7 @@ TEST_F(ConditionGrammar, VersionConditionLessThanTrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"6.0\", <)"); std::string::const_iterator begin = condition.begin(); @@ -347,7 +363,7 @@ TEST_F(ConditionGrammar, VersionConditionLessThanFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"5.0\", <)"); std::string::const_iterator begin = condition.begin(); @@ -367,7 +383,7 @@ TEST_F(ConditionGrammar, VersionConditionGreaterThanTrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"4.0\", >)"); std::string::const_iterator begin = condition.begin(); @@ -387,7 +403,7 @@ TEST_F(ConditionGrammar, VersionConditionGreaterThanFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"5.0\", >)"); std::string::const_iterator begin = condition.begin(); @@ -407,7 +423,7 @@ TEST_F(ConditionGrammar, VersionConditionLETrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"5.0\", <=)"); std::string::const_iterator begin = condition.begin(); @@ -427,7 +443,7 @@ TEST_F(ConditionGrammar, VersionConditionLEFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"4.0\", <=)"); std::string::const_iterator begin = condition.begin(); @@ -447,7 +463,7 @@ TEST_F(ConditionGrammar, VersionConditionGETrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"5.0\", >=)"); std::string::const_iterator begin = condition.begin(); @@ -466,7 +482,7 @@ TEST_F(ConditionGrammar, VersionConditionGEFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("version(\"Blank.esm\", \"6.0\", >=)"); std::string::const_iterator begin = condition.begin(); @@ -485,7 +501,7 @@ TEST_F(ConditionGrammar, ActiveConditionTrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("active(\"Blank.esm\")"); std::string::const_iterator begin = condition.begin(); @@ -504,7 +520,7 @@ TEST_F(ConditionGrammar, ActiveConditionFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("active(\"Blank.esp\")"); std::string::const_iterator begin = condition.begin(); @@ -522,7 +538,7 @@ TEST_F(ConditionGrammar, NegatorTrue) { boost::spirit::qi::space_type skipper; bool eval = false; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("not file(\"Blank.missing.esm\")"); std::string::const_iterator begin = condition.begin(); @@ -540,7 +556,7 @@ TEST_F(ConditionGrammar, NegatorFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("not file(\"Blank.esm\")"); std::string::const_iterator begin = condition.begin(); @@ -558,7 +574,7 @@ TEST_F(ConditionGrammar, CompoundAndTrue) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("file(\"Blank.esm\") and file(\"Blank.esp\")"); std::string::const_iterator begin = condition.begin(); @@ -576,7 +592,7 @@ TEST_F(ConditionGrammar, CompoundAndFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("file(\"Blank.esm\") and file(\"Blank.missing.esp\")"); std::string::const_iterator begin = condition.begin(); @@ -594,7 +610,7 @@ TEST_F(ConditionGrammar, CompoundOrTrue) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("file(\"Blank.missing.esm\") or file(\"Blank.esp\")"); std::string::const_iterator begin = condition.begin(); @@ -612,7 +628,7 @@ TEST_F(ConditionGrammar, CompoundOrFalse) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("file(\"Blank.missing.esm\") or file(\"Blank.missing.esp\")"); std::string::const_iterator begin = condition.begin(); @@ -630,7 +646,7 @@ TEST_F(ConditionGrammar, OrderOfEvaluation) { boost::spirit::qi::space_type skipper; bool eval = true; bool r = false; - Grammar cg(&game, false); + Grammar cg(&game); std::string condition("file(\"Blank.esm\") and ( not file(\"Blank.esm\") or file(\"Blank.esp\") ) or file(\"Blank.missing.esp\")"); std::string::const_iterator begin = condition.begin(); diff --git a/src/tests/fixtures.h b/src/tests/fixtures.h index 1213e191..3dc8476b 100644 --- a/src/tests/fixtures.h +++ b/src/tests/fixtures.h @@ -35,7 +35,13 @@ along with LOOT. If not, see class GameTest : public ::testing::Test { protected: GameTest(const boost::filesystem::path& gameDataPath, const boost::filesystem::path& gameLocalPath) - : dataPath(gameDataPath), localPath(gameLocalPath), missingPath("./missing"), masterlistPath(localPath / "masterlist.yaml"), userlistPath(localPath / "userlist.yaml"), db(nullptr) {} + : dataPath(gameDataPath), + localPath(gameLocalPath), + missingPath("./missing"), + masterlistPath(localPath / "masterlist.yaml"), + userlistPath(localPath / "userlist.yaml"), + resourcePath(dataPath / "resource" / "detail" / "resource.txt"), + db(nullptr) {} inline virtual void SetUp() { ASSERT_NO_THROW(boost::filesystem::create_directories(localPath)); @@ -70,6 +76,12 @@ protected: out.close(); ASSERT_TRUE(boost::filesystem::exists(dataPath / "EmptyFile.esm")); + // Write out an empty resource file. + ASSERT_NO_THROW(boost::filesystem::create_directories(resourcePath.parent_path())); + out.open(resourcePath); + out.close(); + ASSERT_TRUE(boost::filesystem::exists(resourcePath)); + // Write out an non-empty, non-plugin file. out.open(dataPath / "NotAPlugin.esm"); out << "This isn't a valid plugin file."; @@ -85,8 +97,10 @@ protected: // Delete generated files. ASSERT_NO_THROW(boost::filesystem::remove(dataPath / "EmptyFile.esm")); + ASSERT_NO_THROW(boost::filesystem::remove(resourcePath)); ASSERT_NO_THROW(boost::filesystem::remove(dataPath / "NotAPlugin.esm")); ASSERT_FALSE(boost::filesystem::exists(dataPath / "EmptyFile.esm")); + ASSERT_FALSE(boost::filesystem::exists(resourcePath)); ASSERT_FALSE(boost::filesystem::exists(dataPath / "NotAPlugin.esm")); // Masterlist & userlist may have been created during test, so delete them. @@ -106,6 +120,8 @@ protected: const boost::filesystem::path masterlistPath; const boost::filesystem::path userlistPath; + const boost::filesystem::path resourcePath; + loot_db db; };