From 4566d841425da460df97412bbfc583c66a0d98ca Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Tue, 18 Aug 2026 00:35:25 +0000 Subject: [PATCH 1/3] fs: implement copyFileSync in C++ Move path validation, file URL conversion, NUL checks, mode validation, permission checks, and the copy into a dedicated C++ binding so fs.copyFileSync() no longer goes through getValidatedPath() in JavaScript. Signed-off-by: Yagiz Nizipli Co-authored-by: Yagiz Nizipli Signed-off-by: Yagiz Nizipli --- lib/fs.js | 6 +- src/node_file.cc | 179 ++++++++++++++++++++++++++++++++ typings/internalBinding/fs.d.ts | 2 + 3 files changed, 182 insertions(+), 5 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 81c4d2b9c884..10fa76d51001 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -3682,11 +3682,7 @@ function copyFileSync(src, dest, mode) { const result = h.copyFileSync(src, dest, mode); if (result !== undefined) return; } - binding.copyFile( - getValidatedPath(src, 'src'), - getValidatedPath(dest, 'dest'), - mode, - ); + binding.copyFileSync(src, dest, mode); } /** diff --git a/src/node_file.cc b/src/node_file.cc index ae0d9f34f8e1..b7dfcbcb5f19 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -46,6 +46,7 @@ #include #include #include +#include #include #if defined(__MINGW32__) || defined(_MSC_VER) @@ -78,6 +79,7 @@ using v8::LocalVector; using v8::Maybe; using v8::MaybeLocal; using v8::Nothing; +using v8::NewStringType; using v8::Number; using v8::Object; using v8::ObjectTemplate; @@ -2416,6 +2418,181 @@ static void OpenFileHandle(const FunctionCallbackInfo& args) { } } +// Matches lib/internal/url.js isURL(): duck-type WHATWG URL objects vs +// legacy url.parse() results (which have `auth` and `path` properties). +static bool IsUrlLike(Environment* env, Local value) { + if (!value->IsObject() || value->IsUint8Array()) { + return false; + } + + Isolate* isolate = env->isolate(); + Local context = env->context(); + Local obj = value.As(); + + Local href; + if (!obj->Get(context, env->href_string()).ToLocal(&href)) { + return false; + } + if (!href->BooleanValue(isolate)) { + return false; + } + + Local protocol; + if (!obj->Get(context, env->protocol_string()).ToLocal(&protocol)) { + return false; + } + if (!protocol->BooleanValue(isolate)) { + return false; + } + + Local auth; + if (!obj->Get(context, FIXED_ONE_BYTE_STRING(isolate, "auth")) + .ToLocal(&auth)) { + return false; + } + + Local path; + if (!obj->Get(context, env->path_string()).ToLocal(&path)) { + return false; + } + + return auth->IsUndefined() && path->IsUndefined(); +} + +static bool ContainsNul(const BufferValue& path) { + return path.length() > 0 && + std::memchr(path.out(), '\0', path.length()) != nullptr; +} + +// C++ equivalent of getValidatedPath(value, propName): accepts string, +// Uint8Array/Buffer, or WHATWG URL, rejects embedded NUL bytes, and converts +// file: URLs to paths. Returns an empty MaybeLocal and throws on failure. +static MaybeLocal GetValidatedPath(Environment* env, + Local input, + const char* prop_name) { + Isolate* isolate = env->isolate(); + + if (input->IsString() || input->IsUint8Array()) { + return input; + } + + if (IsUrlLike(env, input)) { + Local context = env->context(); + Local href; + if (!input.As() + ->Get(context, env->href_string()) + .ToLocal(&href)) { + return MaybeLocal(); + } + + Utf8Value href_utf8(isolate, href); + auto parsed = ada::parse(href_utf8.ToStringView()); + if (!parsed) { + url::ThrowInvalidURL(env, href_utf8.ToStringView(), std::nullopt); + return MaybeLocal(); + } + + std::optional file_path = url::FileURLToPath(env, *parsed); + if (!file_path.has_value()) { + return MaybeLocal(); + } + + if (file_path->find('\0') != std::string::npos) { + THROW_ERR_INVALID_ARG_VALUE( + env, + "The argument '%s' must be a string, Uint8Array, or URL " + "without null bytes. Received %s", + prop_name, + DetermineSpecificErrorType(env, input)); + return MaybeLocal(); + } + + Local path_string; + if (!String::NewFromUtf8(isolate, + file_path->data(), + NewStringType::kNormal, + static_cast(file_path->size())) + .ToLocal(&path_string)) { + return MaybeLocal(); + } + return path_string; + } + + if (isolate->HasPendingException()) { + return MaybeLocal(); + } + + THROW_ERR_INVALID_ARG_TYPE( + env, + "The \"%s\" argument must be of type string or an instance of " + "Buffer or URL. Received %s", + prop_name, + DetermineSpecificErrorType(env, input)); + return MaybeLocal(); +} + +// Full C++ implementation of fs.copyFileSync(): path validation, mode +// validation, permission checks, and the copy itself. +static void CopyFileSync(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + + CHECK_GE(args.Length(), 2); // src, dest[, mode] + + Local src_val; + if (!GetValidatedPath(env, args[0], "src").ToLocal(&src_val)) { + return; + } + Local dest_val; + if (!GetValidatedPath(env, args[1], "dest").ToLocal(&dest_val)) { + return; + } + + BufferValue src(isolate, src_val); + CHECK_NOT_NULL(*src); + if (ContainsNul(src)) { + THROW_ERR_INVALID_ARG_VALUE( + env, + "The argument 'src' must be a string, Uint8Array, or URL " + "without null bytes. Received %s", + DetermineSpecificErrorType(env, args[0])); + return; + } + + BufferValue dest(isolate, dest_val); + CHECK_NOT_NULL(*dest); + if (ContainsNul(dest)) { + THROW_ERR_INVALID_ARG_VALUE( + env, + "The argument 'dest' must be a string, Uint8Array, or URL " + "without null bytes. Received %s", + DetermineSpecificErrorType(env, args[1])); + return; + } + + Local mode = args.Length() > 2 ? args[2] : Undefined(isolate); + int flags; + if (!GetValidFileMode(env, mode, UV_FS_COPYFILE).To(&flags)) { + return; + } + + ToNamespacedPath(env, &src); + ToNamespacedPath(env, &dest); + + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemRead, src.ToStringView()); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + permission::PermissionScope::kFileSystemWrite, + dest.ToStringView()); + + FSReqWrapSync req_wrap_sync("copyfile", *src, *dest); + FS_SYNC_TRACE_BEGIN(copyfile); + SyncCallAndThrowOnError( + env, &req_wrap_sync, uv_fs_copyfile, *src, *dest, flags); + FS_SYNC_TRACE_END(copyfile); +} + static void CopyFile(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); @@ -4229,6 +4406,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data, SetMethod(isolate, target, "writeFileUtf8", WriteFileUtf8); SetMethod(isolate, target, "realpath", RealPath); SetMethod(isolate, target, "copyFile", CopyFile); + SetMethod(isolate, target, "copyFileSync", CopyFileSync); SetMethod(isolate, target, "chmod", Chmod); SetMethod(isolate, target, "fchmod", FChmod); @@ -4357,6 +4535,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(WriteFileUtf8); registry->Register(RealPath); registry->Register(CopyFile); + registry->Register(CopyFileSync); registry->Register(CpSyncCheckPaths); registry->Register(CpSyncOverrideFile); diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts index 6e1702996dfc..907a48c07bc4 100644 --- a/typings/internalBinding/fs.d.ts +++ b/typings/internalBinding/fs.d.ts @@ -75,6 +75,7 @@ declare namespace InternalFSBinding { function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: FSReqCallback): void; function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: undefined, ctx: FSSyncContext): void; function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, usePromises: typeof kUsePromises): Promise; + function copyFileSync(src: unknown, dest: unknown, mode?: unknown): void; function cpSyncCheckPaths(src: StringOrBuffer, dest: StringOrBuffer, dereference: boolean, recursive: boolean): void; function cpSyncOverrideFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, preserveTimestamps: boolean): void; @@ -261,6 +262,7 @@ export interface FsBinding { chown: typeof InternalFSBinding.chown; close: typeof InternalFSBinding.close; copyFile: typeof InternalFSBinding.copyFile; + copyFileSync: typeof InternalFSBinding.copyFileSync; cpSyncCheckPaths: typeof InternalFSBinding.cpSyncCheckPaths; cpSyncOverrideFile: typeof InternalFSBinding.cpSyncOverrideFile; cpSyncCopyDir: typeof InternalFSBinding.cpSyncCopyDir; From c9909b1f3b3a71acb94650561e24e7b32951e963 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Tue, 18 Aug 2026 00:36:43 +0000 Subject: [PATCH 2/3] test: cover Buffer and file URL copyFileSync paths Exercise the C++ copyFileSync validator with Buffer paths, file: URLs, and a non-file URL scheme. Signed-off-by: Yagiz Nizipli Co-authored-by: Yagiz Nizipli --- test/parallel/test-fs-copyfile.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/parallel/test-fs-copyfile.js b/test/parallel/test-fs-copyfile.js index 51d7153de025..e6144d05dc35 100644 --- a/test/parallel/test-fs-copyfile.js +++ b/test/parallel/test-fs-copyfile.js @@ -10,6 +10,7 @@ const { UV_ENOENT, UV_EEXIST } = internalBinding('uv'); +const { pathToFileURL } = require('url'); const src = fixtures.path('a.js'); const dest = tmpdir.resolve('copyfile.out'); const { @@ -55,6 +56,17 @@ verify(src, dest); fs.copyFileSync(src, dest, 0); verify(src, dest); +// Verify Buffer and file: URL paths. +{ + const destBuf = tmpdir.resolve('copyfile.buffer'); + fs.copyFileSync(Buffer.from(src), Buffer.from(destBuf)); + verify(src, destBuf); + + const destUrl = tmpdir.resolve('copyfile.url'); + fs.copyFileSync(pathToFileURL(src), pathToFileURL(destUrl)); + verify(src, destUrl); +} + // Verify that UV_FS_COPYFILE_FICLONE can be used. fs.unlinkSync(dest); fs.copyFileSync(src, dest, UV_FS_COPYFILE_FICLONE); @@ -142,6 +154,13 @@ assert.throws(() => { ); }); +assert.throws(() => { + fs.copyFileSync(new URL('http://example.com/a'), dest); +}, { + code: 'ERR_INVALID_URL_SCHEME', + name: 'TypeError', +}); + assert.throws(() => { fs.copyFileSync(src, dest, 'r'); }, { From 8a97605b4c3ef8d9253b35f7b745cf3e84d668cf Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Tue, 18 Aug 2026 02:29:41 +0000 Subject: [PATCH 3/3] fs: fix FileURLToPath host error used by copyFileSync The C++ FileURLToPath helper aborted on file URLs with a hostname because the ERR_INVALID_FILE_URL_HOST format string was missing %s. copyFileSync now converts URLs in C++, so that path was user-visible. Also match JS fileURLToPath's ERR_INVALID_URL_SCHEME message and cover Uint8Array paths, encoded slashes, and file URL hosts. Co-authored-by: Yagiz Nizipli --- src/node_file.cc | 7 +++++++ src/node_url.cc | 2 +- test/parallel/test-fs-copyfile.js | 35 +++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/node_file.cc b/src/node_file.cc index b7dfcbcb5f19..4314e991add6 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2492,6 +2492,13 @@ static MaybeLocal GetValidatedPath(Environment* env, return MaybeLocal(); } + // Match lib/internal/url.js fileURLToPath(): non-file schemes throw + // ERR_INVALID_URL_SCHEME with the same message as JS (`file`, not `file:`). + if (parsed->type != ada::scheme::FILE) { + THROW_ERR_INVALID_URL_SCHEME(isolate, "The URL must be of scheme file"); + return MaybeLocal(); + } + std::optional file_path = url::FileURLToPath(env, *parsed); if (!file_path.has_value()) { return MaybeLocal(); diff --git a/src/node_url.cc b/src/node_url.cc index 38aa7fc48eb1..03f66d68c476 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -688,7 +688,7 @@ std::optional FileURLToPath(Environment* env, if (hostname.size() > 0) { THROW_ERR_INVALID_FILE_URL_HOST( env->isolate(), - "File URL host must be \"localhost\" or empty on ", + "File URL host must be \"localhost\" or empty on %s", std::string(per_process::metadata.platform)); return std::nullopt; } diff --git a/test/parallel/test-fs-copyfile.js b/test/parallel/test-fs-copyfile.js index e6144d05dc35..edae50fb6d84 100644 --- a/test/parallel/test-fs-copyfile.js +++ b/test/parallel/test-fs-copyfile.js @@ -65,6 +65,11 @@ verify(src, dest); const destUrl = tmpdir.resolve('copyfile.url'); fs.copyFileSync(pathToFileURL(src), pathToFileURL(destUrl)); verify(src, destUrl); + + const destU8 = tmpdir.resolve('copyfile.uint8'); + fs.copyFileSync(new Uint8Array(Buffer.from(src)), + new Uint8Array(Buffer.from(destU8))); + verify(src, destU8); } // Verify that UV_FS_COPYFILE_FICLONE can be used. @@ -159,8 +164,38 @@ assert.throws(() => { }, { code: 'ERR_INVALID_URL_SCHEME', name: 'TypeError', + message: 'The URL must be of scheme file', }); +if (common.isWindows) { + ['%2f', '%2F', '%5c', '%5C'].forEach((i) => { + assert.throws( + () => fs.copyFileSync(new URL(`file:///c:/tmp/${i}`), dest), + { + code: 'ERR_INVALID_FILE_URL_PATH', + name: 'TypeError', + } + ); + }); +} else { + ['%2f', '%2F'].forEach((i) => { + assert.throws( + () => fs.copyFileSync(new URL(`file:///c:/tmp/${i}`), dest), + { + code: 'ERR_INVALID_FILE_URL_PATH', + name: 'TypeError', + } + ); + }); + assert.throws( + () => fs.copyFileSync(new URL('file://hostname/a/b/c'), dest), + { + code: 'ERR_INVALID_FILE_URL_HOST', + name: 'TypeError', + } + ); +} + assert.throws(() => { fs.copyFileSync(src, dest, 'r'); }, {