Parse conditions and regex during YAML decoding.

To prevent syntax errors sneaking in and later causing problems. Closes
#473.

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) {

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<std::string> 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<boost::filesystem::path, std::regex>(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<boost::filesystem::path, std::regex> 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<boost::filesystem::path, std::regex> 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<std::string, uint32_t>::iterator it = _game->crcCache.find(boost::locale::to_lower(file));
 

src/backend/metadata/file.h

@@ -83,6 +83,14 @@ namespace YAML {
             else
                 rhs = loot::File(node.as<std::string>());
 
+            // 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;
         }
     };

src/backend/metadata/message.h

@@ -144,6 +144,15 @@ namespace YAML {
                 condition = node["condition"].as<std::string>();
 
             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;
         }
     };

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();
     }

src/backend/metadata/plugin_metadata.h

@@ -37,6 +37,7 @@
 #include <vector>
 #include <list>
 #include <set>
+#include <regex>
 
 #include <boost/locale.hpp>
 
@@ -89,7 +90,6 @@ namespace loot {
         void Locations(const std::set<Location>& 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<std::string>());
 
+            // 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<bool>());
 

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;
         }
     };

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());
 

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<File>();
     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<File>();
@@ -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<File>();
     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<File>(), YAML::RepresentationException);
 
     node = YAML::Load("[0, 1, 2]");
     EXPECT_ANY_THROW(node.as<File>());

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<loot::Message>();
     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<loot::Message>(), YAML::RepresentationException);
+
     node = YAML::Load("scalar");
     EXPECT_THROW(node.as<loot::Message>(), YAML::RepresentationException);
 

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<loot::PluginMetadata>());
 
-    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<loot::File>({
@@ -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<loot::PluginMetadata>());
-
-    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<loot::PluginMetadata>());
-
     node = YAML::Load("name: Blank.esp");
     pm = node.as<loot::PluginMetadata>();
     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<loot::PluginMetadata>());
+    EXPECT_THROW(node.as<loot::PluginMetadata>(), 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<loot::PluginMetadata>(), 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<loot::PluginMetadata>(), YAML::RepresentationException);
 
     node = YAML::Load("scalar");
-    EXPECT_ANY_THROW(node.as<loot::PluginMetadata>());
+    EXPECT_THROW(node.as<loot::PluginMetadata>(), YAML::RepresentationException);
 
     node = YAML::Load("[0, 1, 2]");
-    EXPECT_ANY_THROW(node.as<loot::PluginMetadata>());
+    EXPECT_THROW(node.as<loot::PluginMetadata>(), YAML::RepresentationException);
 }
 
 #endif

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<Tag>();
     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<Tag>(), YAML::RepresentationException);
 
     node = YAML::Load("[0, 1, 2]");
-    EXPECT_ANY_THROW(node.as<Tag>());
+    EXPECT_THROW(node.as<Tag>(), YAML::RepresentationException);
 }
 
 #endif

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;