Validate INFO filters when loading the record

apps/openmw/mwdialogue/filter.cpp

@@ -30,7 +30,7 @@ namespace
 {
     bool matchesStaticFilters(const MWDialogue::SelectWrapper& select, const MWWorld::Ptr& actor)
     {
-        const ESM::RefId selectId = ESM::RefId::stringRefId(select.getName());
+        const ESM::RefId selectId = select.getId();
         if (select.getFunction() == MWDialogue::SelectWrapper::Function_NotId)
             return actor.getCellRef().getRefId() != selectId;
         if (actor.getClass().isNpc())
@@ -356,19 +356,18 @@ int MWDialogue::Filter::getSelectStructInteger(const SelectWrapper& select) cons
     {
         case SelectWrapper::Function_Journal:
 
-            return MWBase::Environment::get().getJournal()->getJournalIndex(ESM::RefId::stringRefId(select.getName()));
+            return MWBase::Environment::get().getJournal()->getJournalIndex(select.getId());
 
         case SelectWrapper::Function_Item:
         {
             MWWorld::ContainerStore& store = player.getClass().getContainerStore(player);
 
-            return store.count(ESM::RefId::stringRefId(select.getName()));
+            return store.count(select.getId());
         }
 
         case SelectWrapper::Function_Dead:
 
-            return MWBase::Environment::get().getMechanicsManager()->countDeaths(
+            return MWBase::Environment::get().getMechanicsManager()->countDeaths(select.getId());
-                ESM::RefId::stringRefId(select.getName()));
 
         case SelectWrapper::Function_Choice:
 
@@ -546,24 +545,24 @@ bool MWDialogue::Filter::getSelectStructBoolean(const SelectWrapper& select) con
 
         case SelectWrapper::Function_NotId:
 
-            return !(mActor.getCellRef().getRefId() == ESM::RefId::stringRefId(select.getName()));
+            return mActor.getCellRef().getRefId() != select.getId();
 
         case SelectWrapper::Function_NotFaction:
 
-            return !(mActor.getClass().getPrimaryFaction(mActor) == ESM::RefId::stringRefId(select.getName()));
+            return mActor.getClass().getPrimaryFaction(mActor) != select.getId();
 
         case SelectWrapper::Function_NotClass:
 
-            return !(mActor.get<ESM::NPC>()->mBase->mClass == ESM::RefId::stringRefId(select.getName()));
+            return mActor.get<ESM::NPC>()->mBase->mClass != select.getId();
 
         case SelectWrapper::Function_NotRace:
 
-            return !(mActor.get<ESM::NPC>()->mBase->mRace == ESM::RefId::stringRefId(select.getName()));
+            return mActor.get<ESM::NPC>()->mBase->mRace != select.getId();
 
         case SelectWrapper::Function_NotCell:
         {
             std::string_view actorCell = MWBase::Environment::get().getWorld()->getCellName(mActor.getCell());
-            return !Misc::StringUtils::ciStartsWith(actorCell, select.getName());
+            return !Misc::StringUtils::ciStartsWith(actorCell, select.getCellName());
         }
         case SelectWrapper::Function_SameGender:
 

apps/openmw/mwdialogue/selectwrapper.cpp

@@ -454,5 +454,15 @@ bool MWDialogue::SelectWrapper::selectCompare(bool value) const
 
 std::string MWDialogue::SelectWrapper::getName() const
 {
-    return Misc::StringUtils::lowerCase(std::string_view(mSelect.mSelectRule).substr(5));
+    return Misc::StringUtils::lowerCase(getCellName());
+}
+
+std::string_view MWDialogue::SelectWrapper::getCellName() const
+{
+    return std::string_view(mSelect.mSelectRule).substr(5);
+}
+
+ESM::RefId MWDialogue::SelectWrapper::getId() const
+{
+    return ESM::RefId::stringRefId(getCellName());
 }

apps/openmw/mwdialogue/selectwrapper.hpp

@@ -95,6 +95,10 @@ namespace MWDialogue
 
         std::string getName() const;
         ///< Return case-smashed name.
+
+        std::string_view getCellName() const;
+
+        ESM::RefId getId() const;
     };
 }
 

components/esm3/loadinfo.cpp

@@ -3,7 +3,65 @@
 #include "esmreader.hpp"
 #include "esmwriter.hpp"
 
+#include <components/debug/debuglog.hpp>
 #include <components/misc/concepts.hpp>
+#include <components/misc/strings/conversion.hpp>
+
+namespace
+{
+    enum class SelectRuleStatus
+    {
+        Valid,
+        Invalid,
+        Ignorable
+    };
+
+    SelectRuleStatus isValidSelectRule(std::string_view rule)
+    {
+        if (rule.size() < 5)
+            return SelectRuleStatus::Invalid;
+        if (rule[4] < '0' || rule[4] > '5') // Comparison operators
+            return SelectRuleStatus::Invalid;
+        if (rule[1] == '1') // Function
+        {
+            int function = Misc::StringUtils::toNumeric<int>(rule.substr(2, 2), -1);
+            if (function >= 0 && function <= 73)
+                return SelectRuleStatus::Valid;
+            return SelectRuleStatus::Invalid;
+        }
+        if (rule.size() == 5) // Missing ID
+            return SelectRuleStatus::Invalid;
+        if (rule[3] != 'X')
+            return SelectRuleStatus::Ignorable;
+        constexpr auto ignorable
+            = [](bool valid) { return valid ? SelectRuleStatus::Valid : SelectRuleStatus::Ignorable; };
+        switch (rule[1])
+        {
+            case '2':
+            case '3':
+            case 'C':
+                return ignorable(rule[2] == 's' || rule[2] == 'l' || rule[2] == 'f');
+            case '4':
+                return ignorable(rule[2] == 'J');
+            case '5':
+                return ignorable(rule[2] == 'I');
+            case '6':
+                return ignorable(rule[2] == 'D');
+            case '7':
+                return ignorable(rule[2] == 'X');
+            case '8':
+                return ignorable(rule[2] == 'F');
+            case '9':
+                return ignorable(rule[2] == 'C');
+            case 'A':
+                return ignorable(rule[2] == 'R');
+            case 'B':
+                return ignorable(rule[2] == 'L');
+            default:
+                return SelectRuleStatus::Invalid;
+        }
+    }
+}
 
 namespace ESM
 {
@@ -69,7 +127,18 @@ namespace ESM
                     SelectStruct ss;
                     ss.mSelectRule = esm.getHString();
                     ss.mValue.read(esm, Variant::Format_Info);
-                    mSelects.push_back(ss);
+                    auto valid = isValidSelectRule(ss.mSelectRule);
+                    if (ss.mValue.getType() != VT_Int && ss.mValue.getType() != VT_Float)
+                        valid = SelectRuleStatus::Invalid;
+                    if (valid == SelectRuleStatus::Invalid)
+                        Log(Debug::Warning) << "Skipping invalid SCVR for INFO " << mId;
+                    else
+                    {
+                        mSelects.push_back(ss);
+                        if (valid == SelectRuleStatus::Ignorable)
+                            Log(Debug::Info)
+                                << "Found malformed SCVR for INFO " << mId << " at index " << ss.mSelectRule[0];
+                    }
                     break;
                 }
                 case fourCC("BNAM"):