[BREAKING] copy*(): use ino to check identical paths (#582)

lib/copy-sync/__tests__/broken-symlink.test.js

@@ -23,9 +23,8 @@ describe('copy-sync / broken symlink', () => {
 
   afterEach(done => fse.remove(TEST_DIR, done))
 
-  it('should copy broken symlinks by default', () => {
-    assert.doesNotThrow(() => copySync(src, out))
-    assert.strictEqual(fs.readlinkSync(path.join(out, 'broken-symlink')), path.join(src, 'does-not-exist'))
+  it('should error if symlink is broken', () => {
+    assert.throws(() => copySync(src, out))
   })
 
   it('should throw an error when dereference=true', () => {

lib/copy-sync/__tests__/copy-sync-dir.test.js

@@ -6,9 +6,9 @@ const path = require('path')
 const assert = require('assert')
 const crypto = require('crypto')
 
-/* global beforeEach, describe, it */
+/* global beforeEach, afterEach, describe, it */
 
-describe('+ copySync()', () => {
+describe('+ copySync() / dir', () => {
   const SIZE = 16 * 64 * 1024 + 7
   let TEST_DIR
   let src, dest
@@ -20,6 +20,8 @@ describe('+ copySync()', () => {
     fs.emptyDir(TEST_DIR, done)
   })
 
+  afterEach(done => fs.remove(TEST_DIR, done))
+
   describe('> when src is a directory', () => {
     describe('> when dest exists and is a file', () => {
       it('should throw error', () => {
@@ -70,13 +72,15 @@ describe('+ copySync()', () => {
     })
 
     it('should preserve symbolic links', () => {
+      const srcTarget = path.join(TEST_DIR, 'destination')
       fs.mkdirSync(src)
-      fs.symlinkSync('destination', path.join(src, 'symlink'))
+      fs.mkdirSync(srcTarget)
+      fs.symlinkSync(srcTarget, path.join(src, 'symlink'))
 
       fs.copySync(src, dest)
 
       const link = fs.readlinkSync(path.join(dest, 'symlink'))
-      assert.strictEqual(link, 'destination')
+      assert.strictEqual(link, srcTarget)
     })
 
     describe('> when the destination dir does not exist', () => {
@@ -183,14 +187,9 @@ describe('+ copySync()', () => {
       const dest = path.join(TEST_DIR, 'dest')
 
       setTimeout(() => {
-        fs.mkdirSync(src)
-        fs.writeFileSync(path.join(src, 'somefile.html'), 'some data')
+        fs.outputFileSync(path.join(src, 'somefile.html'), 'some data')
         fs.mkdirSync(dest)
-        try {
-          fs.copySync(src, dest, filter)
-        } catch (err) {
-          assert.ifError(err)
-        }
+        fs.copySync(src, dest, filter)
         assert(!fs.existsSync(path.join(dest, 'somefile.html')))
         done()
       }, 1000)

lib/copy-sync/__tests__/copy-sync-file.test.js

@@ -10,11 +10,11 @@ const crypto = require('crypto')
 
 const SIZE = 16 * 64 * 1024 + 7
 
-describe('+ copySync()', () => {
+describe('+ copySync() / file', () => {
   let TEST_DIR
 
   beforeEach(done => {
-    TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync')
+    TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-file')
     fs.emptyDir(TEST_DIR, done)
   })
 

lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js

@@ -23,6 +23,8 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
   it('should return an error if src and dest are the same', () => {
     const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync')
     const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync')
+    fs.ensureFileSync(fileSrc)
+
     try {
       fs.copySync(fileSrc, fileDest)
     } catch (err) {
@@ -30,6 +32,80 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
     }
   })
 
+  describe('dest with parent symlink', () => {
+    describe('first parent is symlink', () => {
+      it('should error when src is file', () => {
+        const src = path.join(TEST_DIR, 'a', 'file.txt')
+        const dest = path.join(TEST_DIR, 'b', 'file.txt')
+        const srcParent = path.join(TEST_DIR, 'a')
+        const destParent = path.join(TEST_DIR, 'b')
+        fs.ensureFileSync(src)
+        fs.ensureSymlinkSync(srcParent, destParent, 'dir')
+
+        try {
+          fs.copySync(src, dest)
+        } catch (err) {
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
+        } finally {
+          assert(fs.existsSync(src))
+        }
+      })
+
+      it('should error when src is directory', () => {
+        const src = path.join(TEST_DIR, 'a', 'foo')
+        const dest = path.join(TEST_DIR, 'b', 'foo')
+        const srcParent = path.join(TEST_DIR, 'a')
+        const destParent = path.join(TEST_DIR, 'b')
+        fs.ensureDirSync(src)
+        fs.ensureSymlinkSync(srcParent, destParent, 'dir')
+
+        try {
+          fs.copySync(src, dest)
+        } catch (err) {
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
+        } finally {
+          assert(fs.existsSync(src))
+        }
+      })
+    })
+
+    describe('nested dest', () => {
+      it('should error when src is file', () => {
+        const src = path.join(TEST_DIR, 'a', 'dir', 'file.txt')
+        const dest = path.join(TEST_DIR, 'b', 'dir', 'file.txt')
+        const srcParent = path.join(TEST_DIR, 'a')
+        const destParent = path.join(TEST_DIR, 'b')
+        fs.ensureFileSync(src)
+        fs.ensureSymlinkSync(srcParent, destParent, 'dir')
+
+        try {
+          fs.copySync(src, dest)
+        } catch (err) {
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
+        } finally {
+          assert(fs.existsSync(src))
+        }
+      })
+
+      it('should error when src is directory', () => {
+        const src = path.join(TEST_DIR, 'a', 'dir', 'foo')
+        const dest = path.join(TEST_DIR, 'b', 'dir', 'foo')
+        const srcParent = path.join(TEST_DIR, 'a')
+        const destParent = path.join(TEST_DIR, 'b')
+        fs.ensureDirSync(src)
+        fs.ensureSymlinkSync(srcParent, destParent, 'dir')
+
+        try {
+          fs.copySync(src, dest)
+        } catch (err) {
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
+        } finally {
+          assert(fs.existsSync(src))
+        }
+      })
+    })
+  })
+
   // src is directory:
   //  src is regular, dest is symlink
   //  src is symlink, dest is regular
@@ -90,7 +166,7 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
     })
 
     describe('>> when src and dest are symlinks that point to the exact same path', () => {
-      it('should not copy and return', () => {
+      it('should error src and dest are the same', () => {
         src = path.join(TEST_DIR, 'src')
         fs.mkdirsSync(src)
         const srcLink = path.join(TEST_DIR, 'src_symlink')
@@ -101,7 +177,11 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
         const srclenBefore = klawSync(srcLink).length
         const destlenBefore = klawSync(destLink).length
 
-        fs.copySync(srcLink, destLink)
+        try {
+          fs.copySync(srcLink, destLink)
+        } catch (err) {
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
+        }
 
         const srclenAfter = klawSync(srcLink).length
         assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change')
@@ -146,8 +226,7 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
     describe(`>> when src is a symlink that points to a regular dest`, () => {
       it('should throw error', () => {
         dest = path.join(TEST_DIR, 'dest', 'somefile.txt')
-        fs.ensureFileSync(dest)
-        fs.writeFileSync(dest, 'some data')
+        fs.outputFileSync(dest, 'some data')
 
         const srcLink = path.join(TEST_DIR, 'src-symlink')
         fs.symlinkSync(dest, srcLink, 'file')
@@ -164,10 +243,9 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
     })
 
     describe('>> when src and dest are symlinks that point to the exact same path', () => {
-      it('should not copy and return', () => {
+      it('should error src and dest are the same', () => {
         src = path.join(TEST_DIR, 'src', 'srcfile.txt')
-        fs.ensureFileSync(src)
-        fs.writeFileSync(src, 'src data')
+        fs.outputFileSync(src, 'src data')
 
         const srcLink = path.join(TEST_DIR, 'src_symlink')
         fs.symlinkSync(src, srcLink, 'file')
@@ -175,7 +253,11 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
         const destLink = path.join(TEST_DIR, 'dest_symlink')
         fs.symlinkSync(src, destLink, 'file')
 
-        fs.copySync(srcLink, destLink)
+        try {
+          fs.copySync(srcLink, destLink)
+        } catch (err) {
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
+        }
 
         const srcln = fs.readlinkSync(srcLink)
         assert.strictEqual(srcln, src)

lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js

@@ -39,7 +39,7 @@ describe('+ copySync() - prevent copying into itself', () => {
   afterEach(done => fs.remove(TEST_DIR, done))
 
   describe('> when source is a file', () => {
-    it('should copy the file successfully even when dest parent is a subdir of src', () => {
+    it('should copy the file successfully even if dest parent is a subdir of src', () => {
       const srcFile = path.join(TEST_DIR, 'src', 'srcfile.txt')
       const destFile = path.join(TEST_DIR, 'src', 'dest', 'destfile.txt')
       fs.writeFileSync(srcFile, dat0)
@@ -273,7 +273,7 @@ describe('+ copySync() - prevent copying into itself', () => {
     })
 
     describe('>> when dest is a symlink', () => {
-      it('should silently return when resolved dest path is exactly the same as resolved src path', () => {
+      it('should error when resolved dest path is exactly the same as resolved src path', () => {
         const srcLink = path.join(TEST_DIR, 'src-symlink')
         fs.symlinkSync(src, srcLink, 'dir')
         const destLink = path.join(TEST_DIR, 'dest-symlink')
@@ -284,16 +284,21 @@ describe('+ copySync() - prevent copying into itself', () => {
         assert(srclenBefore > 2)
         assert(destlenBefore > 2)
 
-        fs.copySync(srcLink, destLink)
-        const srclenAfter = klawSync(srcLink).length
-        assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change')
-        const destlenAfter = klawSync(destLink).length
-        assert.strictEqual(destlenAfter, destlenBefore, 'dest length should not change')
-
-        const srcln = fs.readlinkSync(srcLink)
-        assert.strictEqual(srcln, src)
-        const destln = fs.readlinkSync(destLink)
-        assert.strictEqual(destln, src)
+        try {
+          fs.copySync(srcLink, destLink)
+        } catch (err) {
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
+        } finally {
+          const srclenAfter = klawSync(srcLink).length
+          assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change')
+          const destlenAfter = klawSync(destLink).length
+          assert.strictEqual(destlenAfter, destlenBefore, 'dest length should not change')
+
+          const srcln = fs.readlinkSync(srcLink)
+          assert.strictEqual(srcln, src)
+          const destln = fs.readlinkSync(destLink)
+          assert.strictEqual(destln, src)
+        }
       })
 
       it('should error when resolved dest path is a subdir of resolved src path', () => {
@@ -309,10 +314,11 @@ describe('+ copySync() - prevent copying into itself', () => {
         try {
           fs.copySync(srcLink, destLink)
         } catch (err) {
-          assert(err)
+          assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${resolvedDestPath}'.`)
+        } finally {
+          const destln = fs.readlinkSync(destLink)
+          assert.strictEqual(destln, resolvedDestPath)
         }
-        const destln = fs.readlinkSync(destLink)
-        assert.strictEqual(destln, src)
       })
 
       it('should error when resolved src path is a subdir of resolved dest path', () => {
@@ -322,18 +328,18 @@ describe('+ copySync() - prevent copying into itself', () => {
 
         const dest = path.join(TEST_DIR, 'dest')
 
-        fs.mkdirSync(dest)
-
-        fs.symlinkSync(srcInDest, srcLink, 'dir')
-        fs.symlinkSync(dest, destLink, 'dir')
+        fs.ensureDirSync(srcInDest)
+        fs.ensureSymlinkSync(srcInDest, srcLink, 'dir')
+        fs.ensureSymlinkSync(dest, destLink, 'dir')
 
         try {
           fs.copySync(srcLink, destLink)
         } catch (err) {
           assert.strictEqual(err.message, `Cannot overwrite '${dest}' with '${srcInDest}'.`)
+        } finally {
+          const destln = fs.readlinkSync(destLink)
+          assert.strictEqual(destln, dest)
         }
-        const destln = fs.readlinkSync(destLink)
-        assert.strictEqual(destln, dest)
       })
     })
   })

lib/copy-sync/copy-sync.js

@@ -6,7 +6,6 @@ const mkdirpSync = require('../mkdirs').mkdirsSync
 const utimesSync = require('../util/utimes.js').utimesMillisSync
 
 const notExist = Symbol('notExist')
-const existsReg = Symbol('existsReg')
 
 function copySync (src, dest, opts) {
   if (typeof opts === 'function') {
@@ -23,34 +22,33 @@ function copySync (src, dest, opts) {
     see https://github.com/jprichardson/node-fs-extra/issues/269`)
   }
 
-  const resolvedDest = checkPaths(src, dest)
+  const destStat = checkPaths(src, dest)
 
   if (opts.filter && !opts.filter(src, dest)) return
 
   const destParent = path.dirname(dest)
   if (!fs.existsSync(destParent)) mkdirpSync(destParent)
-  return startCopy(resolvedDest, src, dest, opts)
+  return startCopy(destStat, src, dest, opts)
 }
 
-function startCopy (resolvedDest, src, dest, opts) {
+function startCopy (destStat, src, dest, opts) {
   if (opts.filter && !opts.filter(src, dest)) return
-  return getStats(resolvedDest, src, dest, opts)
+  return getStats(destStat, src, dest, opts)
 }
 
-function getStats (resolvedDest, src, dest, opts) {
+function getStats (destStat, src, dest, opts) {
   const statSync = opts.dereference ? fs.statSync : fs.lstatSync
-  const st = statSync(src)
+  const srcStat = statSync(src)
 
-  if (st.isDirectory()) return onDir(st, resolvedDest, src, dest, opts)
-  else if (st.isFile() ||
-           st.isCharacterDevice() ||
-           st.isBlockDevice()) return onFile(st, resolvedDest, src, dest, opts)
-  else if (st.isSymbolicLink()) return onLink(resolvedDest, src, dest, opts)
+  if (srcStat.isDirectory()) return onDir(srcStat, destStat, src, dest, opts)
+  else if (srcStat.isFile() ||
+           srcStat.isCharacterDevice() ||
+           srcStat.isBlockDevice()) return onFile(srcStat, destStat, src, dest, opts)
+  else if (srcStat.isSymbolicLink()) return onLink(destStat, src, dest, opts)
 }
 
-function onFile (srcStat, resolvedDest, src, dest, opts) {
-  if (resolvedDest === notExist) return copyFile(srcStat, src, dest, opts)
-  else if (resolvedDest === existsReg) return mayCopyFile(srcStat, src, dest, opts)
+function onFile (srcStat, destStat, src, dest, opts) {
+  if (destStat === notExist) return copyFile(srcStat, src, dest, opts)
   return mayCopyFile(srcStat, src, dest, opts)
 }
 
@@ -95,23 +93,9 @@ function copyFileFallback (srcStat, src, dest, opts) {
   fs.closeSync(fdw)
 }
 
-function onDir (srcStat, resolvedDest, src, dest, opts) {
-  if (resolvedDest === notExist) {
-    if (isSrcSubdir(src, dest)) {
-      throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)
-    }
-    return mkDirAndCopy(srcStat, src, dest, opts)
-  } else if (resolvedDest === existsReg) {
-    if (isSrcSubdir(src, dest)) {
-      throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)
-    }
-    return mayCopyDir(src, dest, opts)
-  }
-  return copyDir(src, dest, opts)
-}
-
-function mayCopyDir (src, dest, opts) {
-  if (!fs.statSync(dest).isDirectory()) {
+function onDir (srcStat, destStat, src, dest, opts) {
+  if (destStat === notExist) return mkDirAndCopy(srcStat, src, dest, opts)
+  if (destStat && !destStat.isDirectory()) {
     throw new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`)
   }
   return copyDir(src, dest, opts)
@@ -130,26 +114,36 @@ function copyDir (src, dest, opts) {
 function copyDirItem (item, src, dest, opts) {
   const srcItem = path.join(src, item)
   const destItem = path.join(dest, item)
-  const resolvedDest = checkPaths(srcItem, destItem)
-  return startCopy(resolvedDest, srcItem, destItem, opts)
+  const destStat = checkPaths(srcItem, destItem)
+  return startCopy(destStat, srcItem, destItem, opts)
 }
 
-function onLink (resolvedDest, src, dest, opts) {
+function onLink (destStat, src, dest, opts) {
   let resolvedSrc = fs.readlinkSync(src)
 
   if (opts.dereference) {
     resolvedSrc = path.resolve(process.cwd(), resolvedSrc)
   }
 
-  if (resolvedDest === notExist || resolvedDest === existsReg) {
-    // if dest already exists, fs throws error anyway,
-    // so no need to guard against it here.
+  if (destStat === notExist) {
     return fs.symlinkSync(resolvedSrc, dest)
   } else {
+    let resolvedDest
+    try {
+      resolvedDest = fs.readlinkSync(dest)
+    } catch (err) {
+      // 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 fs.symlinkSync(resolvedSrc, dest)
+      throw err
+    }
     if (opts.dereference) {
       resolvedDest = path.resolve(process.cwd(), resolvedDest)
     }
-    if (pathsAreIdentical(resolvedSrc, resolvedDest)) return
+    if (isSrcSubdir(resolvedSrc, resolvedDest)) {
+      throw new Error(`Cannot copy '${resolvedSrc}' to a subdirectory of itself, '${resolvedDest}'.`)
+    }
 
     // prevent copy if src is a subdir of dest since unlinking
     // dest in this case would result in removing src contents
@@ -167,53 +161,33 @@ function copyLink (resolvedSrc, 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.
 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) => {
-    return acc && destArray[i] === current
-  }, true)
+  return srcArray.reduce((acc, current, i) => acc && destArray[i] === current, true)
 }
 
-// check if dest exists and is a symlink.
-function checkDest (dest) {
-  let resolvedPath
+function checkStats (src, dest) {
+  const srcStat = fs.statSync(src)
+  let destStat
   try {
-    resolvedPath = fs.readlinkSync(dest)
+    destStat = fs.statSync(dest)
   } catch (err) {
-    if (err.code === 'ENOENT') return notExist
-
-    // dest exists and is a regular file or directory, Windows may throw UNKNOWN error.
-    if (err.code === 'EINVAL' || err.code === 'UNKNOWN') return existsReg
-
+    if (err.code === 'ENOENT') return {srcStat, destStat: notExist}
     throw err
   }
-  return resolvedPath // dest exists and is a symlink
-}
-
-function pathsAreIdentical (src, dest) {
-  const os = process.platform
-  const resolvedSrc = path.resolve(src)
-  const resolvedDest = path.resolve(dest)
-  // case-insensitive paths
-  if (os === 'darwin' || os === 'win32') {
-    return resolvedSrc.toLowerCase() === resolvedDest.toLowerCase()
-  }
-  return resolvedSrc === resolvedDest
+  return {srcStat, destStat}
 }
 
 function checkPaths (src, dest) {
-  const resolvedDest = checkDest(dest)
-  if (resolvedDest === notExist || resolvedDest === existsReg) {
-    if (pathsAreIdentical(src, dest)) throw new Error('Source and destination must not be the same.')
-    return resolvedDest
-  } else {
-    // check resolved dest path if dest is a symlink
-    if (pathsAreIdentical(src, resolvedDest)) throw new Error('Source and destination must not be the same.')
-    return resolvedDest
+  const {srcStat, destStat} = checkStats(src, dest)
+  if (destStat.ino && destStat.ino === srcStat.ino) {
+    throw new Error('Source and destination must not be the same.')
+  }
+  if (srcStat.isDirectory() && isSrcSubdir(src, dest)) {
+    throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)
   }
+  return destStat
 }
 
 module.exports = copySync

lib/copy/__tests__/copy-prevent-copying-identical.test.js

@@ -23,6 +23,7 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
   it('should return an error if src and dest are the same', done => {
     const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy')
     const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy')
+    fs.ensureFileSync(fileSrc)
 
     fs.copy(fileSrc, fileDest, err => {
       assert.strictEqual(err.message, 'Source and destination must not be the same.')
@@ -30,6 +31,72 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
     })
   })
 
+  describe('dest with parent symlink', () => {
+    describe('first parent is symlink', () => {
+      it('should error when src is file', done => {
+        const src = path.join(TEST_DIR, 'a', 'file.txt')
+        const dest = path.join(TEST_DIR, 'b', 'file.txt')
+        const srcParent = path.join(TEST_DIR, 'a')
+        const destParent = path.join(TEST_DIR, 'b')
+        fs.ensureFileSync(src)
+        fs.ensureSymlinkSync(srcParent, destParent, 'dir')
+
+        fs.copy(src, dest, err => {
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
+          assert(fs.existsSync(src))
+          done()
+        })
+      })
+
+      it('should error when src is directory', done => {
+        const src = path.join(TEST_DIR, 'a', 'foo')
+        const dest = path.join(TEST_DIR, 'b', 'foo')
+        const srcParent = path.join(TEST_DIR, 'a')
+        const destParent = path.join(TEST_DIR, 'b')
+        fs.ensureDirSync(src)
+        fs.ensureSymlinkSync(srcParent, destParent, 'dir')
+
+        fs.copy(src, dest, err => {
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
+          assert(fs.existsSync(src))
+          done()
+        })
+      })
+    })
+
+    describe('nested dest', () => {
+      it('should error when src is file', done => {
+        const src = path.join(TEST_DIR, 'a', 'dir', 'file.txt')
+        const dest = path.join(TEST_DIR, 'b', 'dir', 'file.txt')
+        const srcParent = path.join(TEST_DIR, 'a')
+        const destParent = path.join(TEST_DIR, 'b')
+        fs.ensureFileSync(src)
+        fs.ensureSymlinkSync(srcParent, destParent, 'dir')
+
+        fs.copy(src, dest, err => {
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
+          assert(fs.existsSync(src))
+          done()
+        })
+      })
+
+      it('should error when src is directory', done => {
+        const src = path.join(TEST_DIR, 'a', 'dir', 'foo')
+        const dest = path.join(TEST_DIR, 'b', 'dir', 'foo')
+        const srcParent = path.join(TEST_DIR, 'a')
+        const destParent = path.join(TEST_DIR, 'b')
+        fs.ensureDirSync(src)
+        fs.ensureSymlinkSync(srcParent, destParent, 'dir')
+
+        fs.copy(src, dest, err => {
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
+          assert(fs.existsSync(src))
+          done()
+        })
+      })
+    })
+  })
+
   // src is directory:
   //  src is regular, dest is symlink
   //  src is symlink, dest is regular
@@ -88,7 +155,7 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
     })
 
     describe('>> when src and dest are symlinks that point to the exact same path', () => {
-      it('should not copy and return', done => {
+      it('should error src and dest are the same', done => {
         src = path.join(TEST_DIR, 'src')
         fs.mkdirsSync(src)
         const srcLink = path.join(TEST_DIR, 'src_symlink')
@@ -100,7 +167,7 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
         const destlenBefore = klawSync(destLink).length
 
         fs.copy(srcLink, destLink, err => {
-          assert.ifError(err)
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const srclenAfter = klawSync(srcLink).length
           assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change')
@@ -125,19 +192,14 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
   describe('> when src is a file', () => {
     describe(`>> when src is regular and dest is a symlink that points to src`, () => {
       it('should error', done => {
-        src = path.join(TEST_DIR, 'src', 'somefile.txt')
-        fs.ensureFileSync(src)
-        fs.writeFileSync(src, 'some data')
+        src = path.join(TEST_DIR, 'src.txt')
+        fs.outputFileSync(src, 'some data')
 
         const destLink = path.join(TEST_DIR, 'dest-symlink')
         fs.symlinkSync(src, destLink, 'file')
 
         fs.copy(src, destLink, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
-
-          const link = fs.readlinkSync(destLink)
-          assert.strictEqual(link, src)
-          assert(fs.readFileSync(link, 'utf8'), 'some data')
           done()
         })
       })
@@ -146,14 +208,13 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
     describe(`>> when src is a symlink that points to a regular dest`, () => {
       it('should throw error', done => {
         dest = path.join(TEST_DIR, 'dest', 'somefile.txt')
-        fs.ensureFileSync(dest)
-        fs.writeFileSync(dest, 'some data')
+        fs.outputFileSync(dest, 'some data')
 
         const srcLink = path.join(TEST_DIR, 'src-symlink')
         fs.symlinkSync(dest, srcLink, 'file')
 
         fs.copy(srcLink, dest, err => {
-          assert.ok(err)
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const link = fs.readlinkSync(srcLink)
           assert.strictEqual(link, dest)
@@ -164,10 +225,9 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
     })
 
     describe('>> when src and dest are symlinks that point to the exact same path', () => {
-      it('should not copy and return', done => {
+      it('should error src and dest are the same', done => {
         src = path.join(TEST_DIR, 'src', 'srcfile.txt')
-        fs.ensureFileSync(src)
-        fs.writeFileSync(src, 'src data')
+        fs.outputFileSync(src, 'src data')
 
         const srcLink = path.join(TEST_DIR, 'src_symlink')
         fs.symlinkSync(src, srcLink, 'file')
@@ -176,7 +236,7 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
         fs.symlinkSync(src, destLink, 'file')
 
         fs.copy(srcLink, destLink, err => {
-          assert.ifError(err)
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const srcln = fs.readlinkSync(srcLink)
           assert.strictEqual(srcln, src)

lib/copy/__tests__/copy-prevent-copying-into-itself.test.js

@@ -39,7 +39,7 @@ describe('+ copy() - prevent copying into itself', () => {
   afterEach(done => fs.remove(TEST_DIR, done))
 
   describe('> when source is a file', () => {
-    it('should copy the file successfully even when dest parent is a subdir of src', done => {
+    it('should copy the file successfully even if dest parent is a subdir of src', done => {
       const srcFile = path.join(TEST_DIR, 'src', 'srcfile.txt')
       const destFile = path.join(TEST_DIR, 'src', 'dest', 'destfile.txt')
       fs.writeFileSync(srcFile, dat0)
@@ -279,7 +279,7 @@ describe('+ copy() - prevent copying into itself', () => {
     })
 
     describe('>> when dest is a symlink', () => {
-      it('should silently return when resolved dest path is exactly the same as resolved src path', done => {
+      it('should error when resolved dest path is exactly the same as resolved src path', done => {
         const srcLink = path.join(TEST_DIR, 'src-symlink')
         fs.symlinkSync(src, srcLink, 'dir')
         const destLink = path.join(TEST_DIR, 'dest-symlink')
@@ -291,7 +291,7 @@ describe('+ copy() - prevent copying into itself', () => {
         assert(destlenBefore > 2)
 
         fs.copy(srcLink, destLink, err => {
-          assert.ifError(err)
+          assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const srclenAfter = klawSync(srcLink).length
           assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change')
@@ -313,13 +313,12 @@ describe('+ copy() - prevent copying into itself', () => {
         const destLink = path.join(TEST_DIR, 'dest-symlink')
         const resolvedDestPath = path.join(TEST_DIR, 'src', 'some', 'nested', 'dest')
         fs.ensureFileSync(path.join(resolvedDestPath, 'subdir', 'somefile.txt'))
-
         fs.symlinkSync(resolvedDestPath, destLink, 'dir')
 
         fs.copy(srcLink, destLink, err => {
-          assert.ifError(err)
+          assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${resolvedDestPath}'.`)
           const destln = fs.readlinkSync(destLink)
-          assert.strictEqual(destln, src)
+          assert.strictEqual(destln, resolvedDestPath)
           done()
         })
       })
@@ -328,13 +327,11 @@ describe('+ copy() - prevent copying into itself', () => {
         const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src')
         const srcLink = path.join(TEST_DIR, 'src-symlink')
         const destLink = path.join(TEST_DIR, 'dest-symlink')
-
         const dest = path.join(TEST_DIR, 'dest')
 
-        fs.mkdirSync(dest)
-
-        fs.symlinkSync(srcInDest, srcLink, 'dir')
-        fs.symlinkSync(dest, destLink, 'dir')
+        fs.ensureDirSync(srcInDest)
+        fs.ensureSymlinkSync(srcInDest, srcLink, 'dir')
+        fs.ensureSymlinkSync(dest, destLink, 'dir')
 
         fs.copy(srcLink, destLink, err => {
           assert.strictEqual(err.message, `Cannot overwrite '${dest}' with '${srcInDest}'.`)

lib/copy/__tests__/copy.test.js

@@ -24,6 +24,8 @@ describe('fs-extra', () => {
     it('should return an error if src and dest are the same', done => {
       const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy')
       const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy')
+      fse.ensureFileSync(fileSrc)
+
       fse.copy(fileSrc, fileDest, err => {
         assert.strictEqual(err.message, 'Source and destination must not be the same.')
         done()
@@ -136,6 +138,22 @@ describe('fs-extra', () => {
         })
       })
 
+      it('should preserve symbolic links', done => {
+        const src = path.join(TEST_DIR, 'src')
+        const dest = path.join(TEST_DIR, 'dest')
+        const srcTarget = path.join(TEST_DIR, 'destination')
+        fse.mkdirSync(src)
+        fse.mkdirSync(srcTarget)
+        fse.symlinkSync(srcTarget, path.join(src, 'symlink'))
+
+        fse.copy(src, dest, err => {
+          assert.ifError(err)
+          const link = fs.readlinkSync(path.join(dest, 'symlink'))
+          assert.strictEqual(link, srcTarget)
+          done()
+        })
+      })
+
       it('should copy the directory asynchronously', done => {
         const FILES = 2
         const src = path.join(TEST_DIR, 'src')

lib/copy/__tests__/ncp/broken-symlink.test.js

@@ -23,10 +23,9 @@ describe('ncp broken symlink', () => {
 
   afterEach(done => fse.remove(TEST_DIR, done))
 
-  it('should copy broken symlinks by default', done => {
+  it('should error if symlink is broken', done => {
     ncp(src, out, err => {
-      if (err) return done(err)
-      assert.strictEqual(fs.readlinkSync(path.join(out, 'broken-symlink')), path.join(src, 'does-not-exist'))
+      assert(err)
       done()
     })
   })

lib/copy/copy.js

@@ -7,7 +7,6 @@ const pathExists = require('../path-exists').pathExists
 const utimes = require('../util/utimes').utimesMillis
 
 const notExist = Symbol('notExist')
-const existsReg = Symbol('existsReg')
 
 function copy (src, dest, opts, cb) {
   if (typeof opts === 'function' && !cb) {
@@ -29,56 +28,55 @@ function copy (src, dest, opts, cb) {
     see https://github.com/jprichardson/node-fs-extra/issues/269`)
   }
 
-  checkPaths(src, dest, (err, resolvedDest) => {
+  checkPaths(src, dest, (err, destStat) => {
     if (err) return cb(err)
-    if (opts.filter) return handleFilter(checkParentDir, resolvedDest, src, dest, opts, cb)
-    return checkParentDir(resolvedDest, src, dest, opts, cb)
+    if (opts.filter) return handleFilter(checkParentDir, destStat, src, dest, opts, cb)
+    return checkParentDir(destStat, src, dest, opts, cb)
   })
 }
 
-function checkParentDir (resolvedDest, src, dest, opts, cb) {
+function checkParentDir (destStat, src, dest, opts, cb) {
   const destParent = path.dirname(dest)
   pathExists(destParent, (err, dirExists) => {
     if (err) return cb(err)
-    if (dirExists) return startCopy(resolvedDest, src, dest, opts, cb)
+    if (dirExists) return startCopy(destStat, src, dest, opts, cb)
     mkdirp(destParent, err => {
       if (err) return cb(err)
-      return startCopy(resolvedDest, src, dest, opts, cb)
+      return startCopy(destStat, src, dest, opts, cb)
     })
   })
 }
 
-function startCopy (resolvedDest, src, dest, opts, cb) {
-  if (opts.filter) return handleFilter(getStats, resolvedDest, src, dest, opts, cb)
-  return getStats(resolvedDest, src, dest, opts, cb)
-}
-
-function handleFilter (onInclude, resolvedDest, src, dest, opts, cb) {
+function handleFilter (onInclude, destStat, src, dest, opts, cb) {
   Promise.resolve(opts.filter(src, dest)).then(include => {
     if (include) {
-      if (resolvedDest) return onInclude(resolvedDest, src, dest, opts, cb)
+      if (destStat) return onInclude(destStat, src, dest, opts, cb)
       return onInclude(src, dest, opts, cb)
     }
     return cb()
   }, error => cb(error))
 }
 
-function getStats (resolvedDest, src, dest, opts, cb) {
+function startCopy (destStat, src, dest, opts, cb) {
+  if (opts.filter) return handleFilter(getStats, destStat, src, dest, opts, cb)
+  return getStats(destStat, src, dest, opts, cb)
+}
+
+function getStats (destStat, src, dest, opts, cb) {
   const stat = opts.dereference ? fs.stat : fs.lstat
-  stat(src, (err, st) => {
+  stat(src, (err, srcStat) => {
     if (err) return cb(err)
 
-    if (st.isDirectory()) return onDir(st, resolvedDest, src, dest, opts, cb)
-    else if (st.isFile() ||
-             st.isCharacterDevice() ||
-             st.isBlockDevice()) return onFile(st, resolvedDest, src, dest, opts, cb)
-    else if (st.isSymbolicLink()) return onLink(resolvedDest, src, dest, opts, cb)
+    if (srcStat.isDirectory()) return onDir(srcStat, destStat, src, dest, opts, cb)
+    else if (srcStat.isFile() ||
+             srcStat.isCharacterDevice() ||
+             srcStat.isBlockDevice()) return onFile(srcStat, destStat, src, dest, opts, cb)
+    else if (srcStat.isSymbolicLink()) return onLink(destStat, src, dest, opts, cb)
   })
 }
 
-function onFile (srcStat, resolvedDest, src, dest, opts, cb) {
-  if (resolvedDest === notExist) return copyFile(srcStat, src, dest, opts, cb)
-  else if (resolvedDest === existsReg) return mayCopyFile(srcStat, src, dest, opts, cb)
+function onFile (srcStat, destStat, src, dest, opts, cb) {
+  if (destStat === notExist) return copyFile(srcStat, src, dest, opts, cb)
   return mayCopyFile(srcStat, src, dest, opts, cb)
 }
 
@@ -123,31 +121,14 @@ function setDestModeAndTimestamps (srcStat, dest, opts, cb) {
   })
 }
 
-function onDir (srcStat, resolvedDest, src, dest, opts, cb) {
-  if (resolvedDest === notExist) {
-    if (isSrcSubdir(src, dest)) {
-      return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`))
-    }
-    return mkDirAndCopy(srcStat, src, dest, opts, cb)
-  } else if (resolvedDest === existsReg) {
-    if (isSrcSubdir(src, dest)) {
-      return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`))
-    }
-    return mayCopyDir(src, dest, opts, cb)
+function onDir (srcStat, destStat, src, dest, opts, cb) {
+  if (destStat === notExist) return mkDirAndCopy(srcStat, src, dest, opts, cb)
+  if (destStat && !destStat.isDirectory()) {
+    return cb(new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`))
   }
   return copyDir(src, dest, opts, cb)
 }
 
-function mayCopyDir (src, dest, opts, cb) {
-  fs.stat(dest, (err, st) => {
-    if (err) return cb(err)
-    if (!st.isDirectory()) {
-      return cb(new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`))
-    }
-    return copyDir(src, dest, opts, cb)
-  })
-}
-
 function mkDirAndCopy (srcStat, src, dest, opts, cb) {
   fs.mkdir(dest, srcStat.mode, err => {
     if (err) return cb(err)
@@ -174,16 +155,16 @@ 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, resolvedDest) => {
+  checkPaths(srcItem, destItem, (err, destStat) => {
     if (err) return cb(err)
-    startCopy(resolvedDest, srcItem, destItem, opts, err => {
+    startCopy(destStat, srcItem, destItem, opts, err => {
       if (err) return cb(err)
       return copyDirItems(items, src, dest, opts, cb)
     })
   })
 }
 
-function onLink (resolvedDest, src, dest, opts, cb) {
+function onLink (destStat, src, dest, opts, cb) {
   fs.readlink(src, (err, resolvedSrc) => {
     if (err) return cb(err)
 
@@ -191,22 +172,28 @@ function onLink (resolvedDest, src, dest, opts, cb) {
       resolvedSrc = path.resolve(process.cwd(), resolvedSrc)
     }
 
-    if (resolvedDest === notExist || resolvedDest === existsReg) {
-      // if dest already exists, fs throws error anyway,
-      // so no need to guard against it here.
+    if (destStat === notExist) {
       return fs.symlink(resolvedSrc, dest, cb)
     } else {
-      if (opts.dereference) {
-        resolvedDest = path.resolve(process.cwd(), resolvedDest)
-      }
-      if (pathsAreIdentical(resolvedSrc, resolvedDest)) return cb()
+      fs.readlink(dest, (err, resolvedDest) => {
+        if (err) {
+          // 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 fs.symlink(resolvedSrc, dest, cb)
+          return cb(err)
+        }
+        if (opts.dereference) {
+          resolvedDest = path.resolve(process.cwd(), resolvedDest)
+        }
+        if (isSrcSubdir(resolvedSrc, resolvedDest)) {
+          return cb(new Error(`Cannot copy '${resolvedSrc}' to a subdirectory of itself, '${resolvedDest}'.`))
+        }
 
-      // 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.
-      fs.stat(dest, (err, st) => {
-        if (err) return cb(err)
-        if (st.isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc)) {
+        // do not 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.
+        if (destStat.isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc)) {
           return cb(new Error(`Cannot overwrite '${resolvedDest}' with '${resolvedSrc}'.`))
         }
         return copyLink(resolvedSrc, dest, cb)
@@ -223,53 +210,36 @@ function copyLink (resolvedSrc, 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.
 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) => {
-    return acc && destArray[i] === current
-  }, true)
+  return srcArray.reduce((acc, current, i) => acc && destArray[i] === current, true)
 }
 
-// check if dest exists and is a symlink.
-function checkDest (dest, cb) {
-  fs.readlink(dest, (err, resolvedPath) => {
-    if (err) {
-      if (err.code === 'ENOENT') return cb(null, notExist)
-
-      // dest exists and is a regular file or directory, Windows may throw UNKNOWN error.
-      if (err.code === 'EINVAL' || err.code === 'UNKNOWN') return cb(null, existsReg)
-
-      return cb(err)
-    }
-    return cb(null, resolvedPath) // dest exists and is a symlink
+function checkStats (src, dest, cb) {
+  fs.stat(src, (err, srcStat) => {
+    if (err) return cb(err)
+    fs.stat(dest, (err, destStat) => {
+      if (err) {
+        if (err.code === 'ENOENT') return cb(null, {srcStat, destStat: notExist})
+        return cb(err)
+      }
+      return cb(null, {srcStat, destStat})
+    })
   })
 }
 
-function pathsAreIdentical (src, dest) {
-  const os = process.platform
-  const resolvedSrc = path.resolve(src)
-  const resolvedDest = path.resolve(dest)
-  // case-insensitive paths
-  if (os === 'darwin' || os === 'win32') {
-    return resolvedSrc.toLowerCase() === resolvedDest.toLowerCase()
-  }
-  return resolvedSrc === resolvedDest
-}
-
 function checkPaths (src, dest, cb) {
-  checkDest(dest, (err, resolvedDest) => {
+  checkStats(src, dest, (err, stats) => {
     if (err) return cb(err)
-    if (resolvedDest === notExist || resolvedDest === existsReg) {
-      if (pathsAreIdentical(src, dest)) return cb(new Error('Source and destination must not be the same.'))
-      return cb(null, resolvedDest)
-    } else {
-      // check resolved dest path if dest is a symlink
-      if (pathsAreIdentical(src, resolvedDest)) return cb(new Error('Source and destination must not be the same.'))
-      return cb(null, resolvedDest)
+    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)
   })
 }
 

lib/json/__tests__/output-json-sync.test.js

@@ -13,7 +13,7 @@ describe('json', () => {
   let TEST_DIR
 
   beforeEach(done => {
-    TEST_DIR = path.join(os.tmpdir(), 'fs-extra')
+    TEST_DIR = path.join(os.tmpdir(), 'fs-extra-output-json-sync')
     fse.emptyDir(TEST_DIR, done)
   })
 

lib/json/__tests__/output-json.test.js

@@ -13,7 +13,7 @@ describe('json', () => {
   let TEST_DIR
 
   beforeEach(done => {
-    TEST_DIR = path.join(os.tmpdir(), 'fs-extra')
+    TEST_DIR = path.join(os.tmpdir(), 'fs-extra-output-json')
     fse.emptyDir(TEST_DIR, done)
   })