diff --git a/CHANGELOG b/CHANGELOG index a8f0b4ae8b13..aa0204077117 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,12 @@ v3.6.16 (XXXX-XX-XX) -------------------- +* Make `--javascript.copy-installation` also copy the `node_modules` + subdirectory. This is required so we have a full copy of the JavaScript + dependencies and not one that excludes some infrequently changed modules. + In addition, file copying now intentionally excludes .map files as they are + not needed. + * Updated JavaScript dependencies, including breaking changes to non-public modules. We recommend always bundling your own copy of third-party modules, even ones listed as public. diff --git a/arangod/V8Server/V8DealerFeature.cpp b/arangod/V8Server/V8DealerFeature.cpp index c63bb252f339..507850ec88ba 100644 --- a/arangod/V8Server/V8DealerFeature.cpp +++ b/arangod/V8Server/V8DealerFeature.cpp @@ -247,33 +247,10 @@ void V8DealerFeature::validateOptions(std::shared_ptr options) { ctx->normalizePath(_moduleDirectories, "javascript.module-directory", false); - // try to append the current version name to the startup directory, - // so instead of "/path/to/js" we will get "/path/to/js/3.4.0" - std::string const versionAppendix = - std::regex_replace(rest::Version::getServerVersion(), std::regex("-.*$"), - ""); - std::string versionedPath = - basics::FileUtils::buildFilename(_startupDirectory, versionAppendix); - - LOG_TOPIC("604da", DEBUG, Logger::V8) - << "checking for existence of version-specific startup-directory '" - << versionedPath << "'"; - if (basics::FileUtils::isDirectory(versionedPath)) { - // version-specific js path exists! - _startupDirectory = versionedPath; - } - - for (auto& it : _moduleDirectories) { - versionedPath = basics::FileUtils::buildFilename(it, versionAppendix); - - LOG_TOPIC("8e21a", DEBUG, Logger::V8) - << "checking for existence of version-specific module-directory '" - << versionedPath << "'"; - if (basics::FileUtils::isDirectory(versionedPath)) { - // version-specific js path exists! - it = versionedPath; + for (auto const& it : _moduleDirectories) { + if (!it.empty()) { + v8security.addToInternalWhitelist(it, FSAccessType::READ); } - v8security.addToInternalWhitelist(it, FSAccessType::READ); } // check whether app-path was specified @@ -309,26 +286,38 @@ void V8DealerFeature::start() { // now check if we have a js directory inside the database directory, and if // it looks good auto& dbPathFeature = server().getFeature(); - const std::string dbJSPath = + std::string const dbJSPath = FileUtils::buildFilename(dbPathFeature.directory(), "js"); - const std::string checksumFile = + std::string const checksumFile = FileUtils::buildFilename(dbJSPath, StaticStrings::checksumFileJs); - const std::string serverPath = FileUtils::buildFilename(dbJSPath, "server"); - const std::string commonPath = FileUtils::buildFilename(dbJSPath, "common"); + std::string const serverPath = FileUtils::buildFilename(dbJSPath, "server"); + std::string const commonPath = FileUtils::buildFilename(dbJSPath, "common"); + std::string const nodeModulesPath = FileUtils::buildFilename(dbJSPath, "node", "node_modules"); if (FileUtils::isDirectory(dbJSPath) && FileUtils::exists(checksumFile) && FileUtils::isDirectory(serverPath) && FileUtils::isDirectory(commonPath)) { - // only load node modules from original startup path - _nodeModulesDirectory = _startupDirectory; // js directory inside database directory looks good. now use it! _startupDirectory = dbJSPath; + // older versions didn't copy node_modules. so check if it exists inside the + // database directory or not. + if (FileUtils::isDirectory(nodeModulesPath)) { + _nodeModulesDirectory = nodeModulesPath; + } else { + _nodeModulesDirectory = _startupDirectory; + } } } + + V8SecurityFeature& v8security = server().getFeature(); + v8security.addToInternalWhitelist(_startupDirectory, FSAccessType::READ); + if (!_nodeModulesDirectory.empty()) { + v8security.addToInternalWhitelist(_nodeModulesDirectory, FSAccessType::READ); + } LOG_TOPIC("77c97", DEBUG, Logger::V8) << "effective startup-directory: " << _startupDirectory << ", effective module-directories: " << _moduleDirectories << ", node-modules-directory: " << _nodeModulesDirectory; - + _startupLoader.setDirectory(_startupDirectory); ServerState::instance()->setJavaScriptPath(_startupDirectory); @@ -458,9 +447,9 @@ void V8DealerFeature::copyInstallationFiles() { _nodeModulesDirectory = _startupDirectory; - const std::string checksumFile = + std::string const checksumFile = FileUtils::buildFilename(_startupDirectory, StaticStrings::checksumFileJs); - const std::string copyChecksumFile = + std::string const copyChecksumFile = FileUtils::buildFilename(copyJSPath, StaticStrings::checksumFileJs); bool overwriteCopy = false; @@ -470,7 +459,7 @@ void V8DealerFeature::copyInstallationFiles() { } else { try { overwriteCopy = - (FileUtils::slurp(copyChecksumFile) != FileUtils::slurp(checksumFile)); + (StringUtils::trim(FileUtils::slurp(copyChecksumFile)) != StringUtils::trim(FileUtils::slurp(checksumFile))); } catch (basics::Exception const& e) { LOG_TOPIC("efa47", ERR, Logger::V8) << "Error reading '" << StaticStrings::checksumFileJs << "' from disk: " << e.what(); @@ -505,40 +494,46 @@ void V8DealerFeature::copyInstallationFiles() { FATAL_ERROR_EXIT(); } - // intentionally do not copy js/node/node_modules... + // intentionally do not copy js/node/node_modules/estlint! // we avoid copying this directory because it contains 5000+ files at the - // moment, and copying them one by one is darn slow at least on Windows... + // moment, and copying them one by one is slow. In addition, eslint is not + // needed in release builds std::string const versionAppendix = std::regex_replace(rest::Version::getServerVersion(), std::regex("-.*$"), ""); - std::string const nodeModulesPath = - FileUtils::buildFilename("js", "node", "node_modules"); - std::string const nodeModulesPathVersioned = - basics::FileUtils::buildFilename("js", versionAppendix, "node", - "node_modules"); + std::string const eslintPath = + FileUtils::buildFilename("js", "node", "node_modules", "eslint"); + + // .bin directories could be harmful, and .map files are large and unnecessary + std::string const binDirectory = std::string(TRI_DIR_SEPARATOR_STR) + ".bin" + TRI_DIR_SEPARATOR_STR; - std::regex const binRegex("[/\\\\]\\.bin[/\\\\]", std::regex::ECMAScript); + size_t copied = 0; - auto filter = [&nodeModulesPath, &nodeModulesPathVersioned, &binRegex](std::string const& filename) -> bool { - if (std::regex_search(filename, binRegex)) { + auto filter = [&eslintPath, &binDirectory, &copied](std::string const& filename) -> bool { + if (filename.size() >= 4 && filename.compare(filename.size() - 4, 4, ".map") == 0) { + // filename ends with ".map". filter it out! + return true; + } + if (filename.find(binDirectory) != std::string::npos) { // don't copy files in .bin return true; } + std::string normalized = filename; FileUtils::normalizePath(normalized); - if ((!nodeModulesPath.empty() && - normalized.size() >= nodeModulesPath.size() && - normalized.substr(normalized.size() - nodeModulesPath.size(), nodeModulesPath.size()) == nodeModulesPath) || - (!nodeModulesPathVersioned.empty() && - normalized.size() >= nodeModulesPathVersioned.size() && - normalized.substr(normalized.size() - nodeModulesPathVersioned.size(), nodeModulesPathVersioned.size()) == nodeModulesPathVersioned)) { + if ((normalized.size() >= eslintPath.size() && + normalized.compare(normalized.size() - eslintPath.size(), eslintPath.size(), eslintPath) == 0)) { // filter it out! return true; } + // let the file/directory pass through + ++copied; return false; }; + double start = TRI_microtime(); + std::string error; if (!FileUtils::copyRecursive(_startupDirectory, copyJSPath, filter, error)) { LOG_TOPIC("45261", FATAL, Logger::V8) << "Error copying JS installation files to '" @@ -561,8 +556,14 @@ void V8DealerFeature::copyInstallationFiles() { << copyJSPath << "': " << error; } } + + LOG_TOPIC("38e1e", INFO, Logger::V8) + << "copying " << copied << " JS installation file(s) took " << Logger::FIXED(TRI_microtime() - start, 6) << "s"; } + + // finally switch over the paths _startupDirectory = copyJSPath; + _nodeModulesDirectory = basics::FileUtils::buildFilename(copyJSPath, "node", "node_modules"); } V8Context* V8DealerFeature::addContext() { diff --git a/tests/js/client/server_permissions/test-copy-installation.js b/tests/js/client/server_permissions/test-copy-installation.js index 2514859d1846..973e0462b2d2 100644 --- a/tests/js/client/server_permissions/test-copy-installation.js +++ b/tests/js/client/server_permissions/test-copy-installation.js @@ -1,5 +1,5 @@ /*jshint globalstrict:false, strict:false */ -/* global getOptions, assertEqual, arango */ +/* global getOptions, assertEqual, assertTrue, assertFalse, arango */ //////////////////////////////////////////////////////////////////////////////// /// @brief test for security-related server options @@ -34,7 +34,10 @@ if (getOptions === true) { 'javascript.allow-admin-execute': 'true', }; } -var jsunity = require('jsunity'); + +const db = require("@arangodb").db; +const fs = require("fs"); +const jsunity = require('jsunity'); function testSuite() { const errors = require('@arangodb').errors; @@ -44,12 +47,17 @@ function testSuite() { tearDown: function() {}, testCanExecuteAction : function() { - // this test just does something, and it doesn't really matter - // the key thing in this test is to get the server started using - // the `--javascript.copy-installation true` setting without failing - let data = "return 'test!'"; - let result = arango.POST("/_admin/execute", data); - assertEqual("test!", result); + // fetch server-side database directory name + let data = "return require('@arangodb').db._path();"; + let dbPath = arango.POST("/_admin/execute", data); + if (db._engine().name === 'rocksdb') { + let jsPath = fs.join(dbPath, "js"); + assertTrue(fs.exists(jsPath)); + assertTrue(fs.exists(fs.join(jsPath, "node"))); + assertTrue(fs.exists(fs.join(jsPath, "node", "node_modules"))); + assertTrue(fs.exists(fs.join(jsPath, "node", "node_modules", "lodash"))); + assertFalse(fs.exists(fs.join(jsPath, "node", "eslint"))); + } }, };