From b45062b1ba1c926397bfb8c4d3788ea368118705 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 17 Apr 2024 00:36:45 +0200 Subject: [PATCH] fixup! module: implement NODE_COMPILE_CACHE for automatic on-disk code caching --- src/compile_cache.cc | 51 +++++---- src/compile_cache.h | 2 +- src/env.cc | 2 +- .../parallel/test-compile-cache-bad-syntax.js | 54 ++++++++++ .../test-compile-cache-permission-allowed.js | 78 ++++++++++++++ ...est-compile-cache-permission-disallowed.js | 100 ++++++++++++++++++ 6 files changed, 264 insertions(+), 23 deletions(-) create mode 100644 test/parallel/test-compile-cache-bad-syntax.js create mode 100644 test/parallel/test-compile-cache-permission-allowed.js create mode 100644 test/parallel/test-compile-cache-permission-disallowed.js diff --git a/src/compile_cache.cc b/src/compile_cache.cc index 1d6c8c70082211..07969ca754f5bc 100644 --- a/src/compile_cache.cc +++ b/src/compile_cache.cc @@ -4,6 +4,7 @@ #include "node_file.h" #include "node_internals.h" #include "node_version.h" +#include "path.h" #include "zlib.h" namespace node { @@ -27,7 +28,7 @@ uint32_t GetHash(const char* data, size_t size) { } uint32_t GetCacheVersionTag() { - std::string node_version(NODE_VERSION); + std::string_view node_version(NODE_VERSION); uint32_t v8_tag = v8::ScriptCompiler::CachedDataVersionTag(); uLong crc = crc32(0L, Z_NULL, 0); crc = crc32(crc, reinterpret_cast(&v8_tag), sizeof(uint32_t)); @@ -119,7 +120,7 @@ void CompileCacheHandler::ReadCacheFile(CompileCacheEntry* entry) { return; } - // Read the cache, grow the buffer exponentially whenever it ills up. + // Read the cache, grow the buffer exponentially whenever it fills up. size_t offset = headers_buf.len; size_t capacity = 4096; // Initial buffer capacity size_t total_read = 0; @@ -340,10 +341,33 @@ CompileCacheHandler::CompileCacheHandler(Environment* env) // - : a hash of filename + module type // - // - -bool CompileCacheHandler::InitializeDirectory(const std::string& dir) { +bool CompileCacheHandler::InitializeDirectory(Environment* env, + const std::string& dir) { compiler_cache_key_ = GetCacheVersionTag(); - std::string cache_dir = - dir + kPathSeparator + Uint32ToHex(compiler_cache_key_); + std::string compiler_cache_key_string = Uint32ToHex(compiler_cache_key_); + std::vector paths = {dir, compiler_cache_key_string}; + std::string cache_dir = PathResolve(env, paths); + + Debug("[compile cache] resolved path %s + %s -> %s\n", + dir, + compiler_cache_key_string, + cache_dir); + + if (UNLIKELY(!env->permission()->is_granted( + permission::PermissionScope::kFileSystemWrite, cache_dir))) { + Debug("[compile cache] skipping cache because write permission for %s " + "is not granted\n", + cache_dir); + return false; + } + + if (UNLIKELY(!env->permission()->is_granted( + permission::PermissionScope::kFileSystemRead, cache_dir))) { + Debug("[compile cache] skipping cache because read permission for %s " + "is not granted\n", + cache_dir); + return false; + } fs::FSReqWrapSync req_wrap; int err = fs::MKDirpSync(nullptr, &(req_wrap.req), cache_dir, 0777, nullptr); @@ -356,22 +380,7 @@ bool CompileCacheHandler::InitializeDirectory(const std::string& dir) { return false; } - uv_fs_t req; - auto clean = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); }); - err = uv_fs_realpath(nullptr, &req, cache_dir.data(), nullptr); - if (is_debug_) { - Debug("[compile cache] resolving real path %s...%s\n", - cache_dir, - err < 0 ? uv_strerror(err) : "success"); - } - if (err != 0 && err != UV_ENOENT) { - return false; - } - - compile_cache_dir_ = std::string(static_cast(req.ptr)); - Debug("[compile cache] resolved real path %s -> %s\n", - cache_dir, - compile_cache_dir_); + compile_cache_dir_ = cache_dir; return true; } diff --git a/src/compile_cache.h b/src/compile_cache.h index 41d6ed8d72167a..57d30c9dd2dbff 100644 --- a/src/compile_cache.h +++ b/src/compile_cache.h @@ -36,7 +36,7 @@ struct CompileCacheEntry { class CompileCacheHandler { public: explicit CompileCacheHandler(Environment* env); - bool InitializeDirectory(const std::string& dir); + bool InitializeDirectory(Environment* env, const std::string& dir); void Persist(); diff --git a/src/env.cc b/src/env.cc index 53c49852712771..b8574fcfb775b8 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1097,7 +1097,7 @@ void Environment::InitializeCompileCache() { return; } auto handler = std::make_unique(this); - if (handler->InitializeDirectory(dir_from_env)) { + if (handler->InitializeDirectory(this, dir_from_env)) { compile_cache_handler_ = std::move(handler); AtExit( [](void* env) { diff --git a/test/parallel/test-compile-cache-bad-syntax.js b/test/parallel/test-compile-cache-bad-syntax.js new file mode 100644 index 00000000000000..63f6374b2c38cb --- /dev/null +++ b/test/parallel/test-compile-cache-bad-syntax.js @@ -0,0 +1,54 @@ +'use strict'; + +// This tests NODE_COMPILE_CACHE works. + +require('../common'); +const { spawnSyncAndExit } = require('../common/child_process'); +const fixtures = require('../common/fixtures'); +const tmpdir = require('../common/tmpdir'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +{ + // Test that it throws if the script fails to parse, and no cache is created. + tmpdir.refresh(); + const dir = tmpdir.resolve('.compile_cache_dir'); + + spawnSyncAndExit( + process.execPath, + [fixtures.path('syntax', 'bad_syntax.js')], + { + env: { + ...process.env, + NODE_DEBUG_NATIVE: 'COMPILE_CACHE', + NODE_COMPILE_CACHE: dir + }, + cwd: tmpdir.path + }, + { + status: 1, + stderr: /skip .*bad_syntax\.js because the cache was not initialized/, + }); + + const cacheDir = fs.readdirSync(dir); + assert.strictEqual(cacheDir.length, 1); + const entries = fs.readdirSync(path.join(dir, cacheDir[0])); + assert.strictEqual(entries.length, 0); + + spawnSyncAndExit( + process.execPath, + [fixtures.path('syntax', 'bad_syntax.mjs')], + { + env: { + ...process.env, + NODE_DEBUG_NATIVE: 'COMPILE_CACHE', + NODE_COMPILE_CACHE: dir + }, + cwd: tmpdir.path + }, + { + status: 1, + stderr: /skip .*bad_syntax\.mjs because the cache was not initialized/, + }); +} diff --git a/test/parallel/test-compile-cache-permission-allowed.js b/test/parallel/test-compile-cache-permission-allowed.js new file mode 100644 index 00000000000000..76dbfab720d8df --- /dev/null +++ b/test/parallel/test-compile-cache-permission-allowed.js @@ -0,0 +1,78 @@ +'use strict'; + +// This tests NODE_COMPILE_CACHE works in existing directory. + +require('../common'); +const { spawnSyncAndAssert } = require('../common/child_process'); +const assert = require('assert'); +const tmpdir = require('../common/tmpdir'); +const fixtures = require('../common/fixtures'); +const fs = require('fs'); + +function testAllowed(readDir, writeDir, envDir) { + console.log(readDir, writeDir, envDir); // Logging for debugging. + + tmpdir.refresh(); + const dummyDir = tmpdir.resolve('dummy'); + fs.mkdirSync(dummyDir); + const script = tmpdir.resolve(dummyDir, 'empty.js'); + fs.copyFileSync(fixtures.path('empty.js'), script); + // If the directory doesn't exist, permission will just be disallowed. + fs.mkdirSync(tmpdir.resolve(envDir)); + + spawnSyncAndAssert( + process.execPath, + [ + '--experimental-permission', + `--allow-fs-read=${dummyDir}`, + `--allow-fs-read=${readDir}`, + `--allow-fs-write=${writeDir}`, + script, + ], + { + env: { + ...process.env, + NODE_DEBUG_NATIVE: 'COMPILE_CACHE', + NODE_COMPILE_CACHE: `${envDir}` + }, + cwd: tmpdir.path + }, + { + stderr(output) { + assert.match(output, /writing cache for .*empty\.js.*success/); + return true; + } + }); + + spawnSyncAndAssert( + process.execPath, + [ + '--experimental-permission', + `--allow-fs-read=${dummyDir}`, + `--allow-fs-read=${readDir}`, + `--allow-fs-write=${writeDir}`, + script, + ], + { + env: { + ...process.env, + NODE_DEBUG_NATIVE: 'COMPILE_CACHE', + NODE_COMPILE_CACHE: `${envDir}` + }, + cwd: tmpdir.path + }, + { + stderr(output) { + assert.match(output, /cache for .*empty\.js was accepted/); + return true; + } + }); +} + +{ + testAllowed(tmpdir.resolve('.compile_cache'), tmpdir.resolve('.compile_cache'), '.compile_cache'); + testAllowed(tmpdir.resolve('.compile_cache'), tmpdir.resolve('.compile_cache'), tmpdir.resolve('.compile_cache')); + testAllowed('*', '*', '.compile_cache'); + testAllowed('*', tmpdir.resolve('.compile_cache'), '.compile_cache'); + testAllowed(tmpdir.resolve('.compile_cache'), '*', '.compile_cache'); +} diff --git a/test/parallel/test-compile-cache-permission-disallowed.js b/test/parallel/test-compile-cache-permission-disallowed.js new file mode 100644 index 00000000000000..2f00da958217fc --- /dev/null +++ b/test/parallel/test-compile-cache-permission-disallowed.js @@ -0,0 +1,100 @@ +'use strict'; + +// This tests NODE_COMPILE_CACHE works in existing directory. + +require('../common'); +const { spawnSyncAndAssert } = require('../common/child_process'); +const assert = require('assert'); +const tmpdir = require('../common/tmpdir'); +const fixtures = require('../common/fixtures'); +const fs = require('fs'); + +function testDisallowed(dummyDir, cacheDirInPermission, cacheDirInEnv) { + console.log(dummyDir, cacheDirInPermission, cacheDirInEnv); // Logging for debugging. + + tmpdir.refresh(); + const script = tmpdir.resolve(dummyDir, 'empty.js'); + fs.mkdirSync(tmpdir.resolve(dummyDir)); + fs.copyFileSync(fixtures.path('empty.js'), script); + // If the directory doesn't exist, permission will just be disallowed. + if (cacheDirInPermission !== '*') { + fs.mkdirSync(tmpdir.resolve(cacheDirInPermission)); + } + + spawnSyncAndAssert( + process.execPath, + [ + '--experimental-permission', + `--allow-fs-read=${dummyDir}`, // No read or write permission for cache dir. + `--allow-fs-write=${dummyDir}`, + script, + ], + { + env: { + ...process.env, + NODE_DEBUG_NATIVE: 'COMPILE_CACHE', + NODE_COMPILE_CACHE: `${cacheDirInEnv}` + }, + cwd: tmpdir.path + }, + { + stderr(output) { + assert.match(output, /skipping cache because write permission for .* is not granted/); + return true; + } + }); + + spawnSyncAndAssert( + process.execPath, + [ + '--experimental-permission', + `--allow-fs-read=${dummyDir}`, + `--allow-fs-read=${cacheDirInPermission}`, // Read-only + `--allow-fs-write=${dummyDir}`, + script, + ], + { + env: { + ...process.env, + NODE_DEBUG_NATIVE: 'COMPILE_CACHE', + NODE_COMPILE_CACHE: `${cacheDirInEnv}` + }, + cwd: tmpdir.path + }, + { + stderr(output) { + assert.match(output, /skipping cache because write permission for .* is not granted/); + return true; + } + }); + + spawnSyncAndAssert( + process.execPath, + [ + '--experimental-permission', + `--allow-fs-read=${dummyDir}`, + `--allow-fs-write=${cacheDirInPermission}`, // Write-only + script, + ], + { + env: { + ...process.env, + NODE_DEBUG_NATIVE: 'COMPILE_CACHE', + NODE_COMPILE_CACHE: `${cacheDirInEnv}` + }, + cwd: tmpdir.path + }, + { + stderr(output) { + assert.match(output, /skipping cache because read permission for .* is not granted/); + return true; + } + }); +} + +{ + testDisallowed(tmpdir.resolve('dummy'), tmpdir.resolve('.compile_cache') + '/', '.compile_cache'); + testDisallowed(tmpdir.resolve('dummy'), tmpdir.resolve('.compile_cache/') + '/', tmpdir.resolve('.compile_cache')); + testDisallowed(tmpdir.resolve('dummy'), '*', '.compile_cache'); + testDisallowed(tmpdir.resolve('dummy'), '*', tmpdir.resolve('.compile_cache')); +}