From 950aefe6b84ede91d8de156f70e6654cf03e1950 Mon Sep 17 00:00:00 2001 From: Jan Date: Wed, 24 Jul 2019 12:46:27 +0200 Subject: [PATCH] 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 | 14 ++- arangod/RestServer/DatabasePathFeature.cpp | 27 +++- js/common/modules/@arangodb/foxx/store.js | 140 ++++++++++----------- lib/ApplicationFeatures/TempFeature.h | 5 +- 4 files changed, 107 insertions(+), 79 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index b635790727..953f952a22 100644 --- a/CHANGELOG +++ b/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. diff --git a/arangod/RestServer/DatabasePathFeature.cpp b/arangod/RestServer/DatabasePathFeature.cpp index 20dc85a3e7..5ad820584c 100644 --- a/arangod/RestServer/DatabasePathFeature.cpp +++ b/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" @@ -90,18 +91,40 @@ void DatabasePathFeature::validateOptions(std::shared_ptr option // strip trailing separators _directory = basics::StringUtils::rTrim(_directory, TRI_DIR_SEPARATOR_STR); - + 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("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; diff --git a/js/common/modules/@arangodb/foxx/store.js b/js/common/modules/@arangodb/foxx/store.js index 0bae44dc4a..603b07e351 100644 --- a/js/common/modules/@arangodb/foxx/store.js +++ b/js/common/modules/@arangodb/foxx/store.js @@ -91,85 +91,75 @@ var updateFishbowlFromZip = function (filename) { var tempPath = fs.getTempPath(); var toSave = []; - try { - fs.makeDirectoryRecursive(tempPath); - var root = fs.join(tempPath, 'foxx-apps-master/applications'); + fs.makeDirectoryRecursive(tempPath); + var root = fs.join(tempPath, 'foxx-apps-master/applications'); - // remove any previous files in the directory - fs.listTree(root).forEach(function (file) { - if (file.match(/\.json$/)) { - try { - fs.remove(fs.join(root, file)); - } catch (ignore) {} + // remove any previous files in the directory + fs.listTree(root).forEach(function (file) { + if (file.match(/\.json$/)) { + try { + fs.remove(fs.join(root, file)); + } catch (ignore) {} + } + }); + + fs.unzipFile(filename, tempPath, false, true); + + if (!fs.exists(root)) { + throw new Error("'applications' directory is missing in foxx-apps-master, giving up"); + } + + var m = fs.listTree(root); + var reSub = /(.*)\.json$/; + var f, match, service, desc; + + for (i = 0; i < m.length; ++i) { + f = m[i]; + match = reSub.exec(f); + if (match === null) { + continue; + } + + service = fs.join(root, f); + + try { + desc = JSON.parse(fs.read(service)); + } catch (err1) { + arangodb.printf("Cannot parse description for service '" + f + "': %s\n", String(err1)); + continue; + } + + desc._key = match[1]; + + if (!desc.hasOwnProperty('name')) { + desc.name = match[1]; + } + + toSave.push(desc); + } + + if (toSave.length > 0) { + var fishbowl = getFishbowlStorage(); + + db._executeTransaction({ + collections: { + exclusive: fishbowl.name() + }, + action: function (params) { + var c = require('internal').db._collection(params.collection); + c.truncate(); + + params.services.forEach(function (service) { + c.save(service); + }); + }, + params: { + services: toSave, + collection: fishbowl.name() } }); - fs.unzipFile(filename, tempPath, false, true); - - if (!fs.exists(root)) { - throw new Error("'applications' directory is missing in foxx-apps-master, giving up"); - } - - var m = fs.listTree(root); - var reSub = /(.*)\.json$/; - var f, match, service, desc; - - for (i = 0; i < m.length; ++i) { - f = m[i]; - match = reSub.exec(f); - if (match === null) { - continue; - } - - service = fs.join(root, f); - - try { - desc = JSON.parse(fs.read(service)); - } catch (err1) { - arangodb.printf("Cannot parse description for service '" + f + "': %s\n", String(err1)); - continue; - } - - desc._key = match[1]; - - if (!desc.hasOwnProperty('name')) { - desc.name = match[1]; - } - - toSave.push(desc); - } - - if (toSave.length > 0) { - var fishbowl = getFishbowlStorage(); - - db._executeTransaction({ - collections: { - exclusive: fishbowl.name() - }, - action: function (params) { - var c = require('internal').db._collection(params.collection); - c.truncate(); - - params.services.forEach(function (service) { - c.save(service); - }); - }, - params: { - services: toSave, - collection: fishbowl.name() - } - }); - - 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; + require('console').debug('Updated local Foxx repository with ' + toSave.length + ' service(s)'); } }; diff --git a/lib/ApplicationFeatures/TempFeature.h b/lib/ApplicationFeatures/TempFeature.h index fd65040599..55708ddf55 100644 --- a/lib/ApplicationFeatures/TempFeature.h +++ b/lib/ApplicationFeatures/TempFeature.h @@ -35,11 +35,14 @@ class TempFeature final : public application_features::ApplicationFeature { void validateOptions(std::shared_ptr) override final; void prepare() override final; void start() override final; + + std::string path() const { return _path; } + private: std::string _path; std::string _appname; }; } // namespace arangodb -#endif \ No newline at end of file +#endif