Skip to content
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

Jump to bottom

[BREAKING] copy & copySync: do not overwrite src & dest paths to avoid side effects #554

Merged
merged 1 commit into from Apr 6, 2018
Merged

[BREAKING] copy & copySync: do not overwrite src & dest paths to avoid side effects #554

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 6 additions & 9 deletions lib/copy-sync/copy-sync.js
View file
Open in desktop
Expand Up @@ -23,11 +23,8 @@ function copySync (src, dest, opts) {
see https://github.com/jprichardson/node-fs-extra/issues/269`)
}

src = path.resolve(src)
dest = path.resolve(dest)

// don't allow src and dest to be the same
if (src === dest) throw new Error('Source and destination must not be the same.')
if (path.resolve(src) === path.resolve(dest)) throw new Error('Source and destination must not be the same.')

if (opts.filter && !opts.filter(src, dest)) return

Expand Down Expand Up @@ -176,15 +173,15 @@ function copyLink (resolvedSrcPath, dest) {
return fs.symlinkSync(resolvedSrcPath, dest)
}

// check if dest exists and/or is a symlink
// check if dest exists and is a symlink.
function checkDest (dest) {
let resolvedPath
try {
resolvedPath = fs.readlinkSync(dest)
} catch (err) {
if (err.code === 'ENOENT') return notExist

// dest exists and is a regular file or directory, Windows may throw UNKNOWN error
// dest exists and is a regular file or directory, Windows may throw UNKNOWN error.
if (err.code === 'EINVAL' || err.code === 'UNKNOWN') return existsReg

throw err
Expand All @@ -193,10 +190,10 @@ function checkDest (dest) {
}

// return true if dest is a subdir of src, otherwise false.
// extract dest base dir and check if that is the same as src basename
// extract dest base dir and check if that is the same as src basename.
function isSrcSubdir (src, dest) {
let srcArray = src.split(path.sep)
let destArray = dest.split(path.sep)
let srcArray = path.resolve(src).split(path.sep)
let destArray = path.resolve(dest).split(path.sep)

return srcArray.reduce((acc, current, i) => {
return acc && destArray[i] === current
Expand Down
35 changes: 15 additions & 20 deletions lib/copy/copy.js
View file
Open in desktop
Expand Up @@ -29,11 +29,8 @@ function copy (src, dest, opts, cb) {
see https://github.com/jprichardson/node-fs-extra/issues/269`)
}

src = path.resolve(src)
dest = path.resolve(dest)

// don't allow src and dest to be the same
if (src === dest) return cb(new Error('Source and destination must not be the same.'))
if (path.resolve(src) === path.resolve(dest)) return cb(new Error('Source and destination must not be the same.'))

if (opts.filter) return handleFilter(checkParentDir, src, dest, opts, cb)
return checkParentDir(src, dest, opts, cb)
Expand All @@ -57,11 +54,10 @@ function startCopy (src, dest, opts, cb) {
}

function handleFilter (onInclude, src, dest, opts, cb) {
Promise.resolve(opts.filter(src, dest))
.then(include => {
if (include) return onInclude(src, dest, opts, cb)
return cb()
}, error => cb(error))
Promise.resolve(opts.filter(src, dest)).then(include => {
if (include) return onInclude(src, dest, opts, cb)
return cb()
}, error => cb(error))
}

function getStats (src, dest, opts, cb) {
Expand Down Expand Up @@ -114,13 +110,12 @@ function copyFile (srcStat, src, dest, opts, cb) {

function copyFileFallback (srcStat, src, dest, opts, cb) {
const rs = fs.createReadStream(src)
rs.on('error', err => cb(err))
.once('open', () => {
const ws = fs.createWriteStream(dest, { mode: srcStat.mode })
ws.on('error', err => cb(err))
.on('open', () => rs.pipe(ws))
.once('close', () => setDestModeAndTimestamps(srcStat, dest, opts, cb))
})
rs.on('error', err => cb(err)).once('open', () => {
const ws = fs.createWriteStream(dest, { mode: srcStat.mode })
ws.on('error', err => cb(err))
.on('open', () => rs.pipe(ws))
.once('close', () => setDestModeAndTimestamps(srcStat, dest, opts, cb))
})
}

function setDestModeAndTimestamps (srcStat, dest, opts, cb) {
Expand Down Expand Up @@ -232,7 +227,7 @@ function copyLink (resolvedSrcPath, dest, cb) {
})
}

// check if dest exists and/or is a symlink
// check if dest exists and is a symlink.
function checkDest (dest, cb) {
fs.readlink(dest, (err, resolvedPath) => {
if (err) {
Expand All @@ -248,10 +243,10 @@ function checkDest (dest, cb) {
}

// return true if dest is a subdir of src, otherwise false.
// extract dest base dir and check if that is the same as src basename
// extract dest base dir and check if that is the same as src basename.
function isSrcSubdir (src, dest) {
let srcArray = src.split(path.sep)
let destArray = dest.split(path.sep)
let srcArray = path.resolve(src).split(path.sep)
let destArray = path.resolve(dest).split(path.sep)

return srcArray.reduce((acc, current, i) => {
return acc && destArray[i] === current
Expand Down