Skip to content

fs: support Buffer paths in cp methods #64686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
StealthEyeLLC wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fs: support Buffer paths in cp methods #64686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions doc/api/fs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1367,8 +1367,8 @@ changes:
whether to perform path resolution for symlinks.
-->
* `src` {string|URL} source path to copy.
* `dest` {string|URL} destination path to copy to.
* `src` {string|Buffer|URL} source path to copy.
* `dest` {string|Buffer|URL} destination path to copy to.
* `options` {Object}
* `dereference` {boolean} dereference symlinks. **Default:** `false`.
* `errorOnExist` {boolean} when `force` is `false`, and the destination
Expand All @@ -1377,8 +1377,8 @@ changes:
`true` to copy the item, `false` to ignore it. When ignoring a directory,
all of its contents will be skipped as well. Can also return a `Promise`
that resolves to `true` or `false` **Default:** `undefined`.
* `src` {string} source path to copy.
* `dest` {string} destination path to copy to.
* `src` {string|Buffer} source path to copy.
* `dest` {string|Buffer} destination path to copy to.
* Returns: {boolean|Promise} A value that is coercible to `boolean` or
a `Promise` that fulfils with such value.
* `force` {boolean} overwrite existing file or directory. The copy
Expand Down Expand Up @@ -2892,8 +2892,8 @@ changes:
whether to perform path resolution for symlinks.
-->
* `src` {string|URL} source path to copy.
* `dest` {string|URL} destination path to copy to.
* `src` {string|Buffer|URL} source path to copy.
* `dest` {string|Buffer|URL} destination path to copy to.
* `options` {Object}
* `dereference` {boolean} dereference symlinks. **Default:** `false`.
* `errorOnExist` {boolean} when `force` is `false`, and the destination
Expand All @@ -2902,8 +2902,8 @@ changes:
`true` to copy the item, `false` to ignore it. When ignoring a directory,
all of its contents will be skipped as well. Can also return a `Promise`
that fulfills with `true` or `false`. **Default:** `undefined`.
* `src` {string} source path to copy.
* `dest` {string} destination path to copy to.
* `src` {string|Buffer} source path to copy.
* `dest` {string|Buffer} destination path to copy to.
* Returns: {boolean|Promise} A value that is coercible to `boolean` or
a `Promise` that fulfils with such value.
* `force` {boolean} overwrite existing file or directory. The copy
Expand Down Expand Up @@ -6023,17 +6023,17 @@ changes:
whether to perform path resolution for symlinks.
-->
* `src` {string|URL} source path to copy.
* `dest` {string|URL} destination path to copy to.
* `src` {string|Buffer|URL} source path to copy.
* `dest` {string|Buffer|URL} destination path to copy to.
* `options` {Object}
* `dereference` {boolean} dereference symlinks. **Default:** `false`.
* `errorOnExist` {boolean} when `force` is `false`, and the destination
exists, throw an error. **Default:** `false`.
* `filter` {Function} Function to filter copied files/directories. Return
`true` to copy the item, `false` to ignore it. When ignoring a directory,
all of its contents will be skipped as well. **Default:** `undefined`
* `src` {string} source path to copy.
* `dest` {string} destination path to copy to.
* `src` {string|Buffer} source path to copy.
* `dest` {string|Buffer} destination path to copy to.
* Returns: {boolean} Any non-`Promise` value that is coercible
to `boolean`.
* `force` {boolean} overwrite existing file or directory. The copy
Expand Down
9 changes: 5 additions & 4 deletions lib/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ const {
getDirents,
getOptions,
getValidatedFd,
getValidatedCpPath,
getValidatedPath,
handleErrorFromBinding,
preprocessSymlinkDestination,
Expand Down Expand Up @@ -3694,8 +3695,8 @@ function cp(src, dest, options, callback) {
}
callback = makeCallback(callback);
options = validateCpOptions(options);
src = getValidatedPath(src, 'src');
dest = getValidatedPath(dest, 'dest');
src = getValidatedCpPath(src, 'src');
dest = getValidatedCpPath(dest, 'dest');
lazyLoadCp();
cpFn(src, dest, options, callback);
}
Expand All @@ -3710,8 +3711,8 @@ function cp(src, dest, options, callback) {
*/
function cpSync(src, dest, options) {
options = validateCpOptions(options);
src = getValidatedPath(src, 'src');
dest = getValidatedPath(dest, 'dest');
src = getValidatedCpPath(src, 'src');
dest = getValidatedCpPath(dest, 'dest');
lazyLoadCp();
cpSyncFn(src, dest, options);
}
Expand Down
63 changes: 35 additions & 28 deletions lib/internal/fs/cp/cp-sync.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@
// This file is a modified version of the fs-extra's copySync method.

const fsBinding = internalBinding('fs');
const { isSrcSubdir } = require('internal/fs/cp/cp');
const {
dirnamePath,
isSrcSubdir,
joinPath,
maybeDecodePath,
resolveLinkTarget,
} = require('internal/fs/cp/path');
const { codes: {
ERR_FS_CP_EEXIST,
ERR_FS_CP_EINVAL,
Expand All @@ -30,12 +36,6 @@ const {
unlinkSync,
utimesSync,
} = require('fs');
const {
dirname,
isAbsolute,
join,
resolve,
} = require('path');
const { isPromise } = require('util/types');

function cpSyncFn(src, dest, opts) {
Expand Down Expand Up @@ -138,9 +138,10 @@ function onDir(srcStat, destStat, src, dest, opts) {
}

function copyDir(src, dest, opts, mkDir, srcMode) {
if (!opts.filter) {
// The caller didn't provide a js filter function, in this case
// we can run the whole function faster in C++
if (!opts.filter && typeof src === 'string' && typeof dest === 'string') {
// The caller didn't provide a js filter function and both paths are strings,
// in this case we can run the whole function faster in C++.
// Buffer paths require the byte-preserving JavaScript traversal.
// TODO(dario-piotrowicz): look into making cpSyncCopyDir also accept the potential filter function
return fsBinding.cpSyncCopyDir(src, dest,
opts.force,
Expand All @@ -154,15 +155,15 @@ function copyDir(src, dest, opts, mkDir, srcMode) {
mkdirSync(dest);
}

const dir = opendirSync(src);
const dir = opendirSync(src, { encoding: 'buffer' });

try {
let dirent;

while ((dirent = dir.readSync()) !== null) {
const { name } = dirent;
const srcItem = join(src, name);
const destItem = join(dest, name);
const srcItem = joinPath(src, name);
const destItem = joinPath(dest, name);
let shouldCopy = true;

if (opts.filter) {
Expand All @@ -187,43 +188,49 @@ function copyDir(src, dest, opts, mkDir, srcMode) {

// TODO(@anonrig): Move this function to C++.
function onLink(destStat, src, dest, verbatimSymlinks) {
let resolvedSrc = readlinkSync(src);
if (!verbatimSymlinks && !isAbsolute(resolvedSrc)) {
resolvedSrc = resolve(dirname(src), resolvedSrc);
}
if (!destStat) {
return symlinkSync(resolvedSrc, dest);
let resolvedSrc = maybeDecodePath(
readlinkSync(src, { encoding: 'buffer' }),
typeof src === 'string',
);
if (!verbatimSymlinks) {
resolvedSrc = resolveLinkTarget(dirnamePath(src), resolvedSrc);
}
if (!destStat) return symlinkSync(resolvedSrc, dest);

let resolvedDest;
try {
resolvedDest = readlinkSync(dest);
resolvedDest = maybeDecodePath(
readlinkSync(dest, { encoding: 'buffer' }),
typeof dest === 'string',
);
} catch (err) {
// Dest exists and is a regular file or directory,
// Dest exists and is a regular file or directory.
// Windows may throw UNKNOWN error. If dest already exists,
// fs throws error anyway, so no need to guard against it here.
if (err.code === 'EINVAL' || err.code === 'UNKNOWN') {
return symlinkSync(resolvedSrc, dest);
}
throw err;
}
if (!isAbsolute(resolvedDest)) {
resolvedDest = resolve(dirname(dest), resolvedDest);
if (!verbatimSymlinks) {
resolvedDest = resolveLinkTarget(dirnamePath(dest), resolvedDest);
}
const srcIsDir = fsBinding.internalModuleStat(src) === 1;

const srcIsDir = typeof src === 'string' ?
fsBinding.internalModuleStat(src) === 1 :
statSync(src).isDirectory();
if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) {
throw new ERR_FS_CP_EINVAL({
message: `cannot copy ${resolvedSrc} to a subdirectory of self ` +
`${resolvedDest}`,
`${resolvedDest}`,
path: dest,
syscall: 'cp',
errno: EINVAL,
code: 'EINVAL',
});
}
// Prevent copy if src is a subdir of dest since unlinking
// dest in this case would result in removing src contents
// and therefore a broken symlink would be created.
// Prevent copy if src is a subdir of dest since unlinking dest in this case
// would result in removing src contents and therefore a broken symlink.
if (statSync(dest).isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc)) {
throw new ERR_FS_CP_SYMLINK_TO_SUBDIRECTORY({
message: `cannot overwrite ${resolvedDest} with ${resolvedSrc}`,
Expand Down
Loading
Loading