Fix crash when loading an invalid plugin

Fixes loot/loot#787.

src/api/game/game.cpp

@@ -150,7 +150,11 @@ void Game::LoadPlugins(const std::vector<std::string>& plugins, bool loadHeaders
       for (auto pluginName : pluginGroup) {
         BOOST_LOG_TRIVIAL(trace) << "Loading " << pluginName;
         const bool loadHeader = boost::iequals(pluginName, masterFile_) || loadHeadersOnly;
-        cache_->AddPlugin(Plugin(Type(), DataPath(), loadOrderHandler_, pluginName, loadHeader));
+        try {
+          cache_->AddPlugin(Plugin(Type(), DataPath(), loadOrderHandler_, pluginName, loadHeader));
+        } catch(std::exception& e) {
+          BOOST_LOG_TRIVIAL(error) << "Caught exception while trying to add " << pluginName << " to the cache: " << e.what();
+        }
       }
     }));
   }

src/tests/api/interface/game_interface_test.h

@@ -35,7 +35,6 @@ class GameInterfaceTest : public ApiGameOperationsTest {
 protected:
   GameInterfaceTest() :
     emptyFile("EmptyFile.esm"),
-    nonPluginFile("NotAPlugin.esm"),
     pluginsToLoad({
       masterFile,
       blankEsm,
@@ -54,11 +53,9 @@ class GameInterfaceTest : public ApiGameOperationsTest {
     ApiGameOperationsTest::TearDown();
 
     boost::filesystem::remove(dataPath / emptyFile);
-    boost::filesystem::remove(dataPath / nonPluginFile);
   }
 
   const std::string emptyFile;
-  const std::string nonPluginFile;
   const std::vector<std::string> pluginsToLoad;
 };
 
@@ -79,12 +76,6 @@ TEST_P(GameInterfaceTest, isValidPluginShouldReturnTrueForAValidPlugin) {
 }
 
 TEST_P(GameInterfaceTest, isValidPluginShouldReturnFalseForANonPluginFile) {
-  // Write out an non-empty, non-plugin file.
-  boost::filesystem::ofstream out(dataPath / nonPluginFile);
-  out << "This isn't a valid plugin file.";
-  out.close();
-  ASSERT_TRUE(boost::filesystem::exists(dataPath / nonPluginFile));
-
   EXPECT_FALSE(handle_->IsValidPlugin(nonPluginFile));
 }
 

src/tests/api/internals/game/game_test.h

@@ -105,6 +105,29 @@ TEST_P(GameTest, loadPluginsWithHeadersOnlyTrueShouldLoadTheHeadersOfAllInstalle
   EXPECT_EQ(0, plugin->GetCRC());
 }
 
+TEST_P(GameTest, loadPluginsWithANonPluginShouldNotAddItToTheLoadedPlugins) {
+  Game game = Game(GetParam(), dataPath.parent_path(), localPath);
+
+  ASSERT_THROW(game.LoadPlugins({ nonPluginFile }, false), std::invalid_argument);
+
+  ASSERT_TRUE(game.GetLoadedPlugins().empty());
+}
+
+TEST_P(GameTest, loadPluginsWithAnInvalidPluginShouldNotAddItToTheLoadedPlugins) {
+  ASSERT_FALSE(boost::filesystem::exists(dataPath / invalidPlugin));
+  ASSERT_NO_THROW(boost::filesystem::copy_file(dataPath / blankEsm, dataPath / invalidPlugin));
+  ASSERT_TRUE(boost::filesystem::exists(dataPath / invalidPlugin));
+  boost::filesystem::ofstream out(dataPath / invalidPlugin, std::fstream::app);
+  out << "GRUP0";
+  out.close();
+
+  Game game = Game(GetParam(), dataPath.parent_path(), localPath);
+
+  ASSERT_NO_THROW(game.LoadPlugins({ invalidPlugin }, false));
+
+  ASSERT_TRUE(game.GetLoadedPlugins().empty());
+}
+
 TEST_P(GameTest, loadPluginsWithHeadersOnlyFalseShouldFullyLoadAllInstalledPlugins) {
   Game game = Game(GetParam(), dataPath.parent_path(), localPath);
 

src/tests/api/internals/plugin/plugin_test.h

@@ -36,7 +36,6 @@ class PluginTest : public CommonGameTestFixture {
 protected:
   PluginTest() :
     emptyFile("EmptyFile.esm"),
-    nonPluginFile("NotAPlugin.esm"),
     lowercaseBlankEsp("blank.esp"),
     game_(GetParam(), dataPath.parent_path(), localPath),
     blankArchive("Blank" + GetArchiveFileExtension(game_.Type())),
@@ -50,12 +49,6 @@ class PluginTest : public CommonGameTestFixture {
     out.close();
     ASSERT_TRUE(boost::filesystem::exists(dataPath / emptyFile));
 
-    // Write out an non-empty, non-plugin file.
-    out.open(dataPath / nonPluginFile);
-    out << "This isn't a valid plugin file.";
-    out.close();
-    ASSERT_TRUE(boost::filesystem::exists(dataPath / nonPluginFile));
-
 #ifndef _WIN32
     ASSERT_NO_THROW(boost::filesystem::copy(dataPath / blankEsp, dataPath / lowercaseBlankEsp));
 #endif
@@ -71,7 +64,6 @@ class PluginTest : public CommonGameTestFixture {
     CommonGameTestFixture::TearDown();
 
     boost::filesystem::remove(dataPath / emptyFile);
-    boost::filesystem::remove(dataPath / nonPluginFile);
 #ifndef _WIN32
     boost::filesystem::remove(dataPath / lowercaseBlankEsp);
 #endif
@@ -82,7 +74,6 @@ class PluginTest : public CommonGameTestFixture {
   Game game_;
 
   const std::string emptyFile;
-  const std::string nonPluginFile;
   const std::string lowercaseBlankEsp;
   const std::string blankArchive;
   const std::string blankSuffixArchive;

src/tests/common_game_test_fixture.h

@@ -48,6 +48,8 @@ class CommonGameTestFixture : public ::testing::TestWithParam<GameType> {
     lootDataPath("./local/LOOT"),
     masterFile(getMasterFile()),
     missingEsp("Blank.missing.esp"),
+    nonPluginFile("NotAPlugin.esm"),
+    invalidPlugin("Invalid.esm"),
     blankEsm("Blank.esm"),
     blankDifferentEsm("Blank - Different.esm"),
     blankMasterDependentEsm("Blank - Master Dependent.esm"),
@@ -93,6 +95,12 @@ class CommonGameTestFixture : public ::testing::TestWithParam<GameType> {
     ASSERT_FALSE(boost::filesystem::exists(dataPath / (blankMasterDependentEsm + ".ghost")));
     ASSERT_NO_THROW(boost::filesystem::rename(dataPath / blankMasterDependentEsm, dataPath / (blankMasterDependentEsm + ".ghost")));
     ASSERT_TRUE(boost::filesystem::exists(dataPath / (blankMasterDependentEsm + ".ghost")));
+
+    // Write out an non-empty, non-plugin file.
+    boost::filesystem::ofstream out(dataPath / nonPluginFile);
+    out << "This isn't a valid plugin file.";
+    out.close();
+    ASSERT_TRUE(boost::filesystem::exists(dataPath / nonPluginFile));
   }
 
   void TearDown() {
@@ -105,6 +113,9 @@ class CommonGameTestFixture : public ::testing::TestWithParam<GameType> {
     ASSERT_TRUE(boost::filesystem::exists(dataPath / (blankMasterDependentEsm + ".ghost")));
     ASSERT_NO_THROW(boost::filesystem::rename(dataPath / (blankMasterDependentEsm + ".ghost"), dataPath / blankMasterDependentEsm));
     ASSERT_FALSE(boost::filesystem::exists(dataPath / (blankMasterDependentEsm + ".ghost")));
+
+    ASSERT_NO_THROW(boost::filesystem::remove(dataPath / nonPluginFile));
+    ASSERT_NO_THROW(boost::filesystem::remove(dataPath / invalidPlugin));
   }
 
   std::vector<std::string> readFileLines(const boost::filesystem::path& file) {
@@ -130,6 +141,8 @@ class CommonGameTestFixture : public ::testing::TestWithParam<GameType> {
       for (boost::filesystem::directory_iterator it(dataPath); it != boost::filesystem::directory_iterator(); ++it) {
         if (boost::filesystem::is_regular_file(it->status())) {
           std::string filename = it->path().filename().string();
+          if (filename == nonPluginFile)
+            continue;
           if (boost::ends_with(filename, ".ghost"))
             filename = it->path().stem().string();
           if (boost::ends_with(filename, ".esp") || boost::ends_with(filename, ".esm"))
@@ -185,6 +198,8 @@ class CommonGameTestFixture : public ::testing::TestWithParam<GameType> {
 
   const std::string masterFile;
   const std::string missingEsp;
+  const std::string nonPluginFile;
+  const std::string invalidPlugin;
   const std::string blankEsm;
   const std::string blankDifferentEsm;
   const std::string blankMasterDependentEsm;