Bug fix 3.4/temporary directory fixes (#9550)

* added startup error for bad temporary directory setting

if the temporary directory (--temp.path) setting is identical to the database directory (--database.directory)
this can eventually lead to data loss, as temporary files may be created inside the temporary directory, causing
overwrites of existing database files/directories with the same names.
Additionally the temporary directory may be cleaned at some point, and this would lead to an unintended cleanup
of the database files/directories as well.
Now, if the database directory and temporary directory are set to the same path, there will be a startup error
warning about potential data loss (though in ArangoDB 3.4 allowing to continue the startup - in 3.5 and higher
we will abort the startup).

CHANGELOG

@@ -1,7 +1,19 @@
 v3.4.8 (XXXX-XX-XX)
 -------------------
 
-* add gzip and encryption options to arangoimport and arangoexport
+* Added startup error for bad temporary directory setting.
+
+  If the temporary directory (--temp.path) setting is identical to the database 
+  directory (`--database.directory`) this can eventually lead to data loss, as 
+  temporary files may be created inside the temporary directory, causing overwrites of 
+  existing database files/directories with the same names.
+  Additionally the temporary directory may be cleaned at some point, and this would lead 
+  to an unintended cleanup of the database files/directories as well.
+  Now, if the database directory and temporary directory are set to the same path, there 
+  will be a startup warning about potential data loss (though in ArangoDB 3.4 allowing to 
+  continue the startup - in 3.5 and higher we will abort the startup).
+
+* Add gzip and encryption options to arangoimport and arangoexport.
 
 * Fixed a query abort error in smart joins when both collections were
   restricted to a single shard.

arangod/RestServer/DatabasePathFeature.cpp

@@ -23,6 +23,7 @@
 #include "DatabasePathFeature.h"
 
 #include "ApplicationFeatures/ApplicationServer.h"
+#include "ApplicationFeatures/TempFeature.h"
 #include "Basics/ArangoGlobalContext.h"
 #include "Basics/FileUtils.h"
 #include "Basics/StringUtils.h"
@@ -94,14 +95,36 @@ void DatabasePathFeature::validateOptions(std::shared_ptr<ProgramOptions> option
   auto ctx = ArangoGlobalContext::CONTEXT;
 
   if (ctx == nullptr) {
-    LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "failed to get global context.";
+    LOG_TOPIC(FATAL, arangodb::Logger::FIXME) << "failed to get global context.";
     FATAL_ERROR_EXIT();
   }
 
   ctx->normalizePath(_directory, "database.directory", false);
+
 }
 
 void DatabasePathFeature::prepare() {
+  // check if temporary directory and database directory are identical
+  {
+    std::string directoryCopy = _directory;
+    basics::FileUtils::makePathAbsolute(directoryCopy);
+
+    auto* tf = application_features::ApplicationServer::lookupFeature<TempFeature>("Temp");
+    if (tf) {
+      // the feature is not present in unit tests, so make the execution depend
+      // on whether the feature is available
+      std::string tempPathCopy = tf->path();
+      basics::FileUtils::makePathAbsolute(tempPathCopy);
+      tempPathCopy = basics::StringUtils::rTrim(tempPathCopy, TRI_DIR_SEPARATOR_STR);
+
+      if (directoryCopy == tempPathCopy) {
+        LOG_TOPIC(ERR, arangodb::Logger::FIXME) 
+          << "database directory '" << directoryCopy << "' is identical to the temporary directory. "
+          << "This can cause follow-up problems, including data loss. Please review your setup!";
+      }
+    }
+  }
+
   if (_requiredDirectoryState == "any") {
     // database directory can have any state. this is the default
     return;

js/common/modules/@arangodb/foxx/store.js

@@ -91,7 +91,6 @@ var updateFishbowlFromZip = function (filename) {
   var tempPath = fs.getTempPath();
   var toSave = [];
 
-  try {
   fs.makeDirectoryRecursive(tempPath);
   var root = fs.join(tempPath, 'foxx-apps-master/applications');
 
@@ -162,15 +161,6 @@ var updateFishbowlFromZip = function (filename) {
 
     require('console').debug('Updated local Foxx repository with ' + toSave.length + ' service(s)');
   }
-  } catch (err) {
-    if (tempPath !== undefined && tempPath !== '') {
-      try {
-        fs.removeDirectoryRecursive(tempPath);
-      } catch (ignore) {}
-    }
-
-    throw err;
-  }
 };
 
 // //////////////////////////////////////////////////////////////////////////////

lib/ApplicationFeatures/TempFeature.h

@@ -36,6 +36,9 @@ class TempFeature final : public application_features::ApplicationFeature {
   void prepare() override final;
   void start() override final;
   
+  std::string path() const { return _path; }
+
+ private:
   std::string _path;
   std::string _appname;
 };