diff --git a/lib/copy-sync/__tests__/copy-sync-dir.test.js b/lib/copy-sync/__tests__/copy-sync-dir.test.js index 37a5a832..7a5ecf51 100644 --- a/lib/copy-sync/__tests__/copy-sync-dir.test.js +++ b/lib/copy-sync/__tests__/copy-sync-dir.test.js @@ -75,7 +75,7 @@ describe('+ copySync() / dir', () => { const srcTarget = path.join(TEST_DIR, 'destination') fs.mkdirSync(src) fs.mkdirSync(srcTarget) - fs.symlinkSync(srcTarget, path.join(src, 'symlink')) + fs.symlinkSync(srcTarget, path.join(src, 'symlink'), 'dir') fs.copySync(src, dest) diff --git a/lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js b/lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js index 90feba4b..51f864ac 100644 --- a/lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js +++ b/lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js @@ -174,17 +174,46 @@ describe('+ copySync() - prevent copying into itself', () => { assert(srclenBefore > 2) const dest = path.join(destLink, 'dir1') + assert(fs.existsSync(dest)) + let errThrown = false try { fs.copySync(src, dest) } catch (err) { assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + errThrown = true + } finally { + assert(errThrown) + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) } + }) - const srclenAfter = klawSync(src).length - assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + it('should error when dest is a subdirectory of src (more than one level depth)', () => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') - const link = fs.readlinkSync(destLink) - assert.strictEqual(link, src) + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) + + const dest = path.join(destLink, 'dir1', 'dir2') + assert(fs.existsSync(dest)) + let errThrown = false + try { + fs.copySync(src, dest) + } catch (err) { + assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${path.join(destLink, 'dir1')}'.`) + errThrown = true + } finally { + assert(errThrown) + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + } }) it('should copy the directory successfully when src is a subdir of resolved dest path', () => { @@ -372,10 +401,6 @@ function testSuccess (src, dest) { fs.copySync(src, dest) - const destlen = klawSync(dest).length - - assert.strictEqual(destlen, srclen) - FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied')) const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8') diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index f59f0e4c..fd86a67f 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -22,7 +22,8 @@ function copySync (src, dest, opts) { see https://github.com/jprichardson/node-fs-extra/issues/269`) } - const destStat = checkPaths(src, dest) + const {srcStat, destStat} = checkPaths(src, dest) + checkParentStats(src, srcStat, dest) if (opts.filter && !opts.filter(src, dest)) return @@ -114,7 +115,7 @@ function copyDir (src, dest, opts) { function copyDirItem (item, src, dest, opts) { const srcItem = path.join(src, item) const destItem = path.join(dest, item) - const destStat = checkPaths(srcItem, destItem) + const {destStat} = checkPaths(srcItem, destItem) return startCopy(destStat, srcItem, destItem, opts) } @@ -179,15 +180,27 @@ function checkStats (src, dest) { return {srcStat, destStat} } -function checkParentStats (src, dest, cb) { - const {srcStat, destStat} = checkStats(src, path.dirname(dest)) +// recursively check if dest parent is a subdirectory of src. +// It works for all file types including symlinks since it +// checks the src and dest inodes. It starts from the deepest +// parent and stops once it reaches the src parent or the root path. +function checkParentStats (src, srcStat, dest) { + const destParent = path.dirname(dest) + if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return + let destStat + try { + destStat = fs.statSync(destParent) + } catch (err) { + if (err.code === 'ENOENT') return + throw err + } if (destStat.ino && destStat.ino === srcStat.ino) { throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) } + return checkParentStats(src, srcStat, destParent) } function checkPaths (src, dest) { - checkParentStats(src, dest) const {srcStat, destStat} = checkStats(src, dest) if (destStat.ino && destStat.ino === srcStat.ino) { throw new Error('Source and destination must not be the same.') @@ -195,7 +208,7 @@ function checkPaths (src, dest) { if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) } - return destStat + return {srcStat, destStat} } module.exports = copySync diff --git a/lib/copy/__tests__/copy-case-insensitive-paths.test.js b/lib/copy/__tests__/copy-case-insensitive-paths.test.js index 132ead51..502616e2 100644 --- a/lib/copy/__tests__/copy-case-insensitive-paths.test.js +++ b/lib/copy/__tests__/copy-case-insensitive-paths.test.js @@ -5,7 +5,9 @@ const os = require('os') const path = require('path') const fs = require('../../') const platform = os.platform() + console.log('platform: ' + platform) + /* global beforeEach, afterEach, describe, it */ describe('+ copy() - case insensitive paths', () => { diff --git a/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js b/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js index 5500dd83..552cbded 100644 --- a/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js +++ b/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js @@ -176,6 +176,7 @@ describe('+ copy() - prevent copying into itself', () => { assert(srclenBefore > 2) const dest = path.join(destLink, 'dir1') + assert(fs.existsSync(dest)) fs.copy(src, dest, err => { assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) @@ -188,6 +189,27 @@ describe('+ copy() - prevent copying into itself', () => { }) }) + it('should error when dest is a subdirectory of src (more than one level depth)', done => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) + + const dest = path.join(destLink, 'dir1', 'dir2') + assert(fs.existsSync(dest)) + fs.copy(src, dest, err => { + assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${path.join(destLink, 'dir1')}'.`) + + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + done() + }) + }) + it('should copy the directory successfully when src is a subdir of resolved dest path', done => { const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src') const destLink = path.join(TEST_DIR, 'dest-symlink') @@ -370,10 +392,6 @@ function testSuccess (src, dest, done) { fs.copy(src, dest, err => { assert.ifError(err) - const destlen = klawSync(dest).length - - assert.strictEqual(destlen, srclen) - FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied')) const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8') diff --git a/lib/copy/__tests__/copy.test.js b/lib/copy/__tests__/copy.test.js index 23d80863..9c1ce7f8 100644 --- a/lib/copy/__tests__/copy.test.js +++ b/lib/copy/__tests__/copy.test.js @@ -144,7 +144,9 @@ describe('fs-extra', () => { const srcTarget = path.join(TEST_DIR, 'destination') fse.mkdirSync(src) fse.mkdirSync(srcTarget) - fse.symlinkSync(srcTarget, path.join(src, 'symlink')) + // symlink type is only used for Windows and the default is 'file'. + // https://nodejs.org/api/fs.html#fs_fs_symlink_target_path_type_callback + fse.symlinkSync(srcTarget, path.join(src, 'symlink'), 'dir') fse.copy(src, dest, err => { assert.ifError(err) @@ -340,11 +342,12 @@ describe('fs-extra', () => { const dest = path.join(TEST_DIR, 'dest') - fse.copySync(src, dest, filter) - - assert(!fs.existsSync(path.join(dest, IGNORE)), 'directory was not ignored') - assert(!fs.existsSync(path.join(dest, IGNORE, 'file')), 'file was not ignored') - done() + fse.copy(src, dest, filter, err => { + assert.ifError(err) + assert(!fs.existsSync(path.join(dest, IGNORE)), 'directory was not ignored') + assert(!fs.existsSync(path.join(dest, IGNORE, 'file')), 'file was not ignored') + done() + }) }) it('should apply filter when it is applied only to dest', done => { diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 1c4d840e..7483477d 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -28,10 +28,14 @@ function copy (src, dest, opts, cb) { see https://github.com/jprichardson/node-fs-extra/issues/269`) } - checkPaths(src, dest, (err, destStat) => { + checkPaths(src, dest, (err, stats) => { if (err) return cb(err) - if (opts.filter) return handleFilter(checkParentDir, destStat, src, dest, opts, cb) - return checkParentDir(destStat, src, dest, opts, cb) + const {srcStat, destStat} = stats + checkParentStats(src, srcStat, dest, err => { + if (err) return cb(err) + if (opts.filter) return handleFilter(checkParentDir, destStat, src, dest, opts, cb) + return checkParentDir(destStat, src, dest, opts, cb) + }) }) } @@ -155,7 +159,7 @@ function copyDirItems (items, src, dest, opts, cb) { function copyDirItem (items, item, src, dest, opts, cb) { const srcItem = path.join(src, item) const destItem = path.join(dest, item) - checkPaths(srcItem, destItem, (err, destStat) => { + checkPaths(srcItem, destItem, (err, {destStat}) => { if (err) return cb(err) startCopy(destStat, srcItem, destItem, opts, err => { if (err) return cb(err) @@ -211,9 +215,9 @@ function copyLink (resolvedSrc, dest, cb) { // return true if dest is a subdir of src, otherwise false. function isSrcSubdir (src, dest) { - const srcArray = path.resolve(src).split(path.sep) - const destArray = path.resolve(dest).split(path.sep) - return srcArray.reduce((acc, current, i) => acc && destArray[i] === current, true) + const srcArr = path.resolve(src).split(path.sep).filter(i => i) + const destArr = path.resolve(dest).split(path.sep).filter(i => i) + return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) } function checkStats (src, dest, cb) { @@ -229,31 +233,36 @@ function checkStats (src, dest, cb) { }) } -function checkParentStats (src, dest, cb) { - checkStats(src, path.dirname(dest), (err, stats) => { - if (err) return cb(err) - const {srcStat, destStat} = stats +// recursively check if dest parent is a subdirectory of src. +// It works for all file types including symlinks since it +// checks the src and dest inodes. It starts from the deepest +// parent and stops once it reaches the src parent or the root path. +function checkParentStats (src, srcStat, dest, cb) { + const destParent = path.dirname(dest) + if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return cb() + fs.stat(destParent, (err, destStat) => { + if (err) { + if (err.code === 'ENOENT') return cb() + return cb(err) + } if (destStat.ino && destStat.ino === srcStat.ino) { return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) } - return cb() + return checkParentStats(src, srcStat, destParent, cb) }) } function checkPaths (src, dest, cb) { - checkParentStats(src, dest, err => { + checkStats(src, dest, (err, stats) => { if (err) return cb(err) - checkStats(src, dest, (err, stats) => { - if (err) return cb(err) - const {srcStat, destStat} = stats - if (destStat.ino && destStat.ino === srcStat.ino) { - return cb(new Error('Source and destination must not be the same.')) - } - if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { - return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) - } - return cb(null, destStat) - }) + const {srcStat, destStat} = stats + if (destStat.ino && destStat.ino === srcStat.ino) { + return cb(new Error('Source and destination must not be the same.')) + } + if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { + return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) + } + return cb(null, {srcStat, destStat}) }) }