Skip to content

Commit e4dc3a1

Browse files
committed
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. VFS dispatch stays in JS and still runs before validation. The copy itself uses uv_fs_copyfile, so flags, mode/timestamp preservation, and UV error shapes stay the same. Also fix FileURLToPath aborting on file URLs with a hostname because the ERR_INVALID_FILE_URL_HOST format string lacked %s. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
1 parent e2b33e2 commit e4dc3a1

5 files changed

Lines changed: 240 additions & 6 deletions

File tree

lib/fs.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3682,11 +3682,7 @@ function copyFileSync(src, dest, mode) {
36823682
const result = h.copyFileSync(src, dest, mode);
36833683
if (result !== undefined) return;
36843684
}
3685-
binding.copyFile(
3686-
getValidatedPath(src, 'src'),
3687-
getValidatedPath(dest, 'dest'),
3688-
mode,
3689-
);
3685+
binding.copyFileSync(src, dest, mode);
36903686
}
36913687

36923688
/**

src/node_file.cc

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
#include <errno.h>
4747
#include <cerrno>
4848
#include <cstdio>
49+
#include <cstring>
4950
#include <filesystem>
5051

5152
#if defined(__MINGW32__) || defined(_MSC_VER)
@@ -77,6 +78,7 @@ using v8::Local;
7778
using v8::LocalVector;
7879
using v8::Maybe;
7980
using v8::MaybeLocal;
81+
using v8::NewStringType;
8082
using v8::Nothing;
8183
using v8::Number;
8284
using v8::Object;
@@ -2416,6 +2418,184 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {
24162418
}
24172419
}
24182420

2421+
// Matches lib/internal/url.js isURL(): duck-type WHATWG URL objects vs
2422+
// legacy url.parse() results (which have `auth` and `path` properties).
2423+
static bool IsUrlLike(Environment* env, Local<Value> value) {
2424+
if (!value->IsObject() || value->IsUint8Array()) {
2425+
return false;
2426+
}
2427+
2428+
Isolate* isolate = env->isolate();
2429+
Local<Context> context = env->context();
2430+
Local<Object> obj = value.As<Object>();
2431+
2432+
Local<Value> href;
2433+
if (!obj->Get(context, env->href_string()).ToLocal(&href)) {
2434+
return false;
2435+
}
2436+
if (!href->BooleanValue(isolate)) {
2437+
return false;
2438+
}
2439+
2440+
Local<Value> protocol;
2441+
if (!obj->Get(context, env->protocol_string()).ToLocal(&protocol)) {
2442+
return false;
2443+
}
2444+
if (!protocol->BooleanValue(isolate)) {
2445+
return false;
2446+
}
2447+
2448+
Local<Value> auth;
2449+
if (!obj->Get(context, FIXED_ONE_BYTE_STRING(isolate, "auth"))
2450+
.ToLocal(&auth)) {
2451+
return false;
2452+
}
2453+
2454+
Local<Value> path;
2455+
if (!obj->Get(context, env->path_string()).ToLocal(&path)) {
2456+
return false;
2457+
}
2458+
2459+
return auth->IsUndefined() && path->IsUndefined();
2460+
}
2461+
2462+
static bool ContainsNul(const BufferValue& path) {
2463+
return path.length() > 0 &&
2464+
std::memchr(path.out(), '\0', path.length()) != nullptr;
2465+
}
2466+
2467+
// C++ equivalent of getValidatedPath(value, propName): accepts string,
2468+
// Uint8Array/Buffer, or WHATWG URL, rejects embedded NUL bytes, and converts
2469+
// file: URLs to paths. Returns an empty MaybeLocal and throws on failure.
2470+
static MaybeLocal<Value> GetValidatedPath(Environment* env,
2471+
Local<Value> input,
2472+
const char* prop_name) {
2473+
Isolate* isolate = env->isolate();
2474+
2475+
if (input->IsString() || input->IsUint8Array()) {
2476+
return input;
2477+
}
2478+
2479+
if (IsUrlLike(env, input)) {
2480+
Local<Context> context = env->context();
2481+
Local<Value> href;
2482+
if (!input.As<Object>()->Get(context, env->href_string()).ToLocal(&href)) {
2483+
return MaybeLocal<Value>();
2484+
}
2485+
2486+
Utf8Value href_utf8(isolate, href);
2487+
auto parsed = ada::parse<ada::url_aggregator>(href_utf8.ToStringView());
2488+
if (!parsed) {
2489+
url::ThrowInvalidURL(env, href_utf8.ToStringView(), std::nullopt);
2490+
return MaybeLocal<Value>();
2491+
}
2492+
2493+
// Match lib/internal/url.js fileURLToPath(): non-file schemes throw
2494+
// ERR_INVALID_URL_SCHEME with the same message as JS (`file`, not `file:`).
2495+
if (parsed->type != ada::scheme::FILE) {
2496+
THROW_ERR_INVALID_URL_SCHEME(isolate, "The URL must be of scheme file");
2497+
return MaybeLocal<Value>();
2498+
}
2499+
2500+
std::optional<std::string> file_path = url::FileURLToPath(env, *parsed);
2501+
if (!file_path.has_value()) {
2502+
return MaybeLocal<Value>();
2503+
}
2504+
2505+
if (file_path->find('\0') != std::string::npos) {
2506+
THROW_ERR_INVALID_ARG_VALUE(
2507+
env,
2508+
"The argument '%s' must be a string, Uint8Array, or URL "
2509+
"without null bytes. Received %s",
2510+
prop_name,
2511+
DetermineSpecificErrorType(env, input));
2512+
return MaybeLocal<Value>();
2513+
}
2514+
2515+
Local<String> path_string;
2516+
if (!String::NewFromUtf8(isolate,
2517+
file_path->data(),
2518+
NewStringType::kNormal,
2519+
static_cast<int>(file_path->size()))
2520+
.ToLocal(&path_string)) {
2521+
return MaybeLocal<Value>();
2522+
}
2523+
return path_string;
2524+
}
2525+
2526+
if (isolate->HasPendingException()) {
2527+
return MaybeLocal<Value>();
2528+
}
2529+
2530+
THROW_ERR_INVALID_ARG_TYPE(
2531+
env,
2532+
"The \"%s\" argument must be of type string or an instance of "
2533+
"Buffer or URL. Received %s",
2534+
prop_name,
2535+
DetermineSpecificErrorType(env, input));
2536+
return MaybeLocal<Value>();
2537+
}
2538+
2539+
// Full C++ implementation of fs.copyFileSync(): path validation, mode
2540+
// validation, permission checks, and the copy itself.
2541+
static void CopyFileSync(const FunctionCallbackInfo<Value>& args) {
2542+
Environment* env = Environment::GetCurrent(args);
2543+
Isolate* isolate = env->isolate();
2544+
2545+
CHECK_GE(args.Length(), 2); // src, dest[, mode]
2546+
2547+
Local<Value> src_val;
2548+
if (!GetValidatedPath(env, args[0], "src").ToLocal(&src_val)) {
2549+
return;
2550+
}
2551+
Local<Value> dest_val;
2552+
if (!GetValidatedPath(env, args[1], "dest").ToLocal(&dest_val)) {
2553+
return;
2554+
}
2555+
2556+
BufferValue src(isolate, src_val);
2557+
CHECK_NOT_NULL(*src);
2558+
if (ContainsNul(src)) {
2559+
THROW_ERR_INVALID_ARG_VALUE(
2560+
env,
2561+
"The argument 'src' must be a string, Uint8Array, or URL "
2562+
"without null bytes. Received %s",
2563+
DetermineSpecificErrorType(env, args[0]));
2564+
return;
2565+
}
2566+
2567+
BufferValue dest(isolate, dest_val);
2568+
CHECK_NOT_NULL(*dest);
2569+
if (ContainsNul(dest)) {
2570+
THROW_ERR_INVALID_ARG_VALUE(
2571+
env,
2572+
"The argument 'dest' must be a string, Uint8Array, or URL "
2573+
"without null bytes. Received %s",
2574+
DetermineSpecificErrorType(env, args[1]));
2575+
return;
2576+
}
2577+
2578+
Local<Value> mode = args.Length() > 2 ? args[2] : Undefined(isolate);
2579+
int flags;
2580+
if (!GetValidFileMode(env, mode, UV_FS_COPYFILE).To(&flags)) {
2581+
return;
2582+
}
2583+
2584+
ToNamespacedPath(env, &src);
2585+
ToNamespacedPath(env, &dest);
2586+
2587+
THROW_IF_INSUFFICIENT_PERMISSIONS(
2588+
env, permission::PermissionScope::kFileSystemRead, src.ToStringView());
2589+
THROW_IF_INSUFFICIENT_PERMISSIONS(
2590+
env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView());
2591+
2592+
FSReqWrapSync req_wrap_sync("copyfile", *src, *dest);
2593+
FS_SYNC_TRACE_BEGIN(copyfile);
2594+
SyncCallAndThrowOnError(
2595+
env, &req_wrap_sync, uv_fs_copyfile, *src, *dest, flags);
2596+
FS_SYNC_TRACE_END(copyfile);
2597+
}
2598+
24192599
static void CopyFile(const FunctionCallbackInfo<Value>& args) {
24202600
Environment* env = Environment::GetCurrent(args);
24212601
Isolate* isolate = env->isolate();
@@ -4229,6 +4409,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
42294409
SetMethod(isolate, target, "writeFileUtf8", WriteFileUtf8);
42304410
SetMethod(isolate, target, "realpath", RealPath);
42314411
SetMethod(isolate, target, "copyFile", CopyFile);
4412+
SetMethod(isolate, target, "copyFileSync", CopyFileSync);
42324413

42334414
SetMethod(isolate, target, "chmod", Chmod);
42344415
SetMethod(isolate, target, "fchmod", FChmod);
@@ -4357,6 +4538,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
43574538
registry->Register(WriteFileUtf8);
43584539
registry->Register(RealPath);
43594540
registry->Register(CopyFile);
4541+
registry->Register(CopyFileSync);
43604542

43614543
registry->Register(CpSyncCheckPaths);
43624544
registry->Register(CpSyncOverrideFile);

src/node_url.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,7 @@ std::optional<std::string> FileURLToPath(Environment* env,
688688
if (hostname.size() > 0) {
689689
THROW_ERR_INVALID_FILE_URL_HOST(
690690
env->isolate(),
691-
"File URL host must be \"localhost\" or empty on ",
691+
"File URL host must be \"localhost\" or empty on %s",
692692
std::string(per_process::metadata.platform));
693693
return std::nullopt;
694694
}

test/parallel/test-fs-copyfile.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const {
1010
UV_ENOENT,
1111
UV_EEXIST
1212
} = internalBinding('uv');
13+
const { pathToFileURL } = require('url');
1314
const src = fixtures.path('a.js');
1415
const dest = tmpdir.resolve('copyfile.out');
1516
const {
@@ -55,6 +56,22 @@ verify(src, dest);
5556
fs.copyFileSync(src, dest, 0);
5657
verify(src, dest);
5758

59+
// Verify Buffer and file: URL paths.
60+
{
61+
const destBuf = tmpdir.resolve('copyfile.buffer');
62+
fs.copyFileSync(Buffer.from(src), Buffer.from(destBuf));
63+
verify(src, destBuf);
64+
65+
const destUrl = tmpdir.resolve('copyfile.url');
66+
fs.copyFileSync(pathToFileURL(src), pathToFileURL(destUrl));
67+
verify(src, destUrl);
68+
69+
const destU8 = tmpdir.resolve('copyfile.uint8');
70+
fs.copyFileSync(new Uint8Array(Buffer.from(src)),
71+
new Uint8Array(Buffer.from(destU8)));
72+
verify(src, destU8);
73+
}
74+
5875
// Verify that UV_FS_COPYFILE_FICLONE can be used.
5976
fs.unlinkSync(dest);
6077
fs.copyFileSync(src, dest, UV_FS_COPYFILE_FICLONE);
@@ -142,6 +159,43 @@ assert.throws(() => {
142159
);
143160
});
144161

162+
assert.throws(() => {
163+
fs.copyFileSync(new URL('http://example.com/a'), dest);
164+
}, {
165+
code: 'ERR_INVALID_URL_SCHEME',
166+
name: 'TypeError',
167+
message: 'The URL must be of scheme file',
168+
});
169+
170+
if (common.isWindows) {
171+
['%2f', '%2F', '%5c', '%5C'].forEach((i) => {
172+
assert.throws(
173+
() => fs.copyFileSync(new URL(`file:///c:/tmp/${i}`), dest),
174+
{
175+
code: 'ERR_INVALID_FILE_URL_PATH',
176+
name: 'TypeError',
177+
}
178+
);
179+
});
180+
} else {
181+
['%2f', '%2F'].forEach((i) => {
182+
assert.throws(
183+
() => fs.copyFileSync(new URL(`file:///c:/tmp/${i}`), dest),
184+
{
185+
code: 'ERR_INVALID_FILE_URL_PATH',
186+
name: 'TypeError',
187+
}
188+
);
189+
});
190+
assert.throws(
191+
() => fs.copyFileSync(new URL('file://hostname/a/b/c'), dest),
192+
{
193+
code: 'ERR_INVALID_FILE_URL_HOST',
194+
name: 'TypeError',
195+
}
196+
);
197+
}
198+
145199
assert.throws(() => {
146200
fs.copyFileSync(src, dest, 'r');
147201
}, {

typings/internalBinding/fs.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ declare namespace InternalFSBinding {
7575
function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: FSReqCallback): void;
7676
function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: undefined, ctx: FSSyncContext): void;
7777
function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, usePromises: typeof kUsePromises): Promise<void>;
78+
function copyFileSync(src: unknown, dest: unknown, mode?: unknown): void;
7879

7980
function cpSyncCheckPaths(src: StringOrBuffer, dest: StringOrBuffer, dereference: boolean, recursive: boolean): void;
8081
function cpSyncOverrideFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, preserveTimestamps: boolean): void;
@@ -261,6 +262,7 @@ export interface FsBinding {
261262
chown: typeof InternalFSBinding.chown;
262263
close: typeof InternalFSBinding.close;
263264
copyFile: typeof InternalFSBinding.copyFile;
265+
copyFileSync: typeof InternalFSBinding.copyFileSync;
264266
cpSyncCheckPaths: typeof InternalFSBinding.cpSyncCheckPaths;
265267
cpSyncOverrideFile: typeof InternalFSBinding.cpSyncOverrideFile;
266268
cpSyncCopyDir: typeof InternalFSBinding.cpSyncCopyDir;

0 commit comments

Comments
 (0)