From f21048b21d3806775560e4b112cddc54a1cb29d8 Mon Sep 17 00:00:00 2001
From: Mani Maghsoudlou <manidlou@gmail.com>
Date: Thu, 26 Mar 2020 01:10:07 -0700
Subject: [PATCH] BREAKING: copy*(): allow copying broken symlinks (#779)

---
 ...st.js => copy-sync-broken-symlink.test.js} | 24 ++++---
 .../__tests__/copy-sync-file.test.js          | 15 +++++
 ...opy-sync-prevent-copying-identical.test.js | 16 ++---
 ...y-sync-prevent-copying-into-itself.test.js | 14 ++---
 ...link.test.js => copy-sync-symlink.test.js} |  2 +-
 lib/copy-sync/copy-sync.js                    | 13 ++--
 .../__tests__/copy-broken-symlink.test.js     | 62 +++++++++++++++++++
 .../copy-prevent-copying-identical.test.js    | 18 +++---
 .../copy-prevent-copying-into-itself.test.js  | 30 ++-------
 lib/copy/__tests__/copy.test.js               | 14 +++++
 lib/copy/__tests__/ncp/broken-symlink.test.js |  6 +-
 lib/copy/copy.js                              | 13 ++--
 ...move-sync-prevent-moving-identical.test.js | 16 ++---
 ...ve-sync-prevent-moving-into-itself.test.js |  8 +--
 lib/move-sync/move-sync.js                    |  2 +-
 .../move-prevent-moving-identical.test.js     | 20 +++---
 .../move-prevent-moving-into-itself.test.js   | 26 +-------
 lib/move/move.js                              |  2 +-
 lib/util/__tests__/stat.test.js               |  4 +-
 lib/util/stat.js                              | 52 +++++++++++-----
 20 files changed, 215 insertions(+), 142 deletions(-)
 rename lib/copy-sync/__tests__/{broken-symlink.test.js => copy-sync-broken-symlink.test.js} (67%)
 rename lib/copy-sync/__tests__/{symlink.test.js => copy-sync-symlink.test.js} (98%)
 create mode 100644 lib/copy/__tests__/copy-broken-symlink.test.js

diff --git a/lib/copy-sync/__tests__/broken-symlink.test.js b/lib/copy-sync/__tests__/copy-sync-broken-symlink.test.js
similarity index 67%
rename from lib/copy-sync/__tests__/broken-symlink.test.js
rename to lib/copy-sync/__tests__/copy-sync-broken-symlink.test.js
index d6d5b57b0..33ba910d8 100644
--- a/lib/copy-sync/__tests__/broken-symlink.test.js
+++ b/lib/copy-sync/__tests__/copy-sync-broken-symlink.test.js
@@ -2,7 +2,7 @@
 
 const fs = require('fs')
 const os = require('os')
-const fse = require('../../')
+const fse = require('../..')
 const path = require('path')
 const assert = require('assert')
 const copySync = require('../copy-sync')
@@ -10,9 +10,9 @@ const copySync = require('../copy-sync')
 /* global afterEach, beforeEach, describe, it */
 
 describe('copy-sync / broken symlink', () => {
-  const TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-broken-symlinks')
+  const TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-broken-symlink')
   const src = path.join(TEST_DIR, 'src')
-  const out = path.join(TEST_DIR, 'out')
+  const dest = path.join(TEST_DIR, 'dest')
 
   beforeEach(done => {
     fse.emptyDir(TEST_DIR, err => {
@@ -23,12 +23,20 @@ describe('copy-sync / broken symlink', () => {
 
   afterEach(done => fse.remove(TEST_DIR, done))
 
-  it('should error if symlink is broken', () => {
-    assert.throws(() => copySync(src, out))
-  })
+  describe('when symlink is broken', () => {
+    it('should not throw error if dereference is false', () => {
+      let err = null
+      try {
+        copySync(src, dest)
+      } catch (e) {
+        err = e
+      }
+      assert.strictEqual(err, null)
+    })
 
-  it('should throw an error when dereference=true', () => {
-    assert.throws(() => copySync(src, out, { dereference: true }), err => err.code === 'ENOENT')
+    it('should throw error if dereference is true', () => {
+      assert.throws(() => copySync(src, dest, { dereference: true }), err => err.code === 'ENOENT')
+    })
   })
 })
 
diff --git a/lib/copy-sync/__tests__/copy-sync-file.test.js b/lib/copy-sync/__tests__/copy-sync-file.test.js
index 1218fce82..6ffff60da 100644
--- a/lib/copy-sync/__tests__/copy-sync-file.test.js
+++ b/lib/copy-sync/__tests__/copy-sync-file.test.js
@@ -21,6 +21,21 @@ describe('+ copySync() / file', () => {
   afterEach(done => fs.remove(TEST_DIR, done))
 
   describe('> when src is a file', () => {
+    describe('> when dest exists and is a directory', () => {
+      it('should throw error', () => {
+        const src = path.join(TEST_DIR, 'file.txt')
+        const dest = path.join(TEST_DIR, 'dir')
+        fs.ensureFileSync(src)
+        fs.ensureDirSync(dest)
+
+        try {
+          fs.copySync(src, dest)
+        } catch (err) {
+          assert.strictEqual(err.message, `Cannot overwrite directory '${dest}' with non-directory '${src}'.`)
+        }
+      })
+    })
+
     it('should copy the file synchronously', () => {
       const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_src')
       const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy')
diff --git a/lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js b/lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js
index 87135a965..c34590966 100644
--- a/lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js
+++ b/lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js
@@ -113,7 +113,7 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
 
   describe('> when src is a directory', () => {
     describe('>> when src is regular and dest is a symlink that points to src', () => {
-      it('should error', () => {
+      it('should error if dereference is true', () => {
         src = path.join(TEST_DIR, 'src')
         fs.mkdirsSync(src)
         const subdir = path.join(TEST_DIR, 'src', 'subdir')
@@ -126,7 +126,7 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
         const oldlen = klawSync(src).length
 
         try {
-          fs.copySync(src, destLink)
+          fs.copySync(src, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
         }
@@ -166,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 error src and dest are the same', () => {
+      it('should error if src and dest are the same and dereferene is true', () => {
         src = path.join(TEST_DIR, 'src')
         fs.mkdirsSync(src)
         const srcLink = path.join(TEST_DIR, 'src_symlink')
@@ -178,7 +178,7 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
         const destlenBefore = klawSync(destLink).length
 
         try {
-          fs.copySync(srcLink, destLink)
+          fs.copySync(srcLink, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
         }
@@ -203,7 +203,7 @@ describe('+ copySync() - 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', () => {
+      it('should error if dereference is true', () => {
         src = path.join(TEST_DIR, 'src', 'somefile.txt')
         fs.ensureFileSync(src)
         fs.writeFileSync(src, 'some data')
@@ -212,7 +212,7 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
         fs.symlinkSync(src, destLink, 'file')
 
         try {
-          fs.copySync(src, destLink)
+          fs.copySync(src, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
         }
@@ -243,7 +243,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 error src and dest are the same', () => {
+      it('should error if src and dest are the same and dereference is true', () => {
         src = path.join(TEST_DIR, 'src', 'srcfile.txt')
         fs.outputFileSync(src, 'src data')
 
@@ -254,7 +254,7 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
         fs.symlinkSync(src, destLink, 'file')
 
         try {
-          fs.copySync(srcLink, destLink)
+          fs.copySync(srcLink, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
         }
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 f9418cb18..24bdd0695 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
@@ -146,7 +146,7 @@ describe('+ copySync() - prevent copying into itself', () => {
     })
 
     describe('>> when dest is a symlink', () => {
-      it('should error when dest points exactly to src', () => {
+      it('should error when dest points exactly to src and dereference is true', () => {
         const destLink = path.join(TEST_DIR, 'dest-symlink')
         fs.symlinkSync(src, destLink, 'dir')
 
@@ -154,7 +154,7 @@ describe('+ copySync() - prevent copying into itself', () => {
         assert(srclenBefore > 2)
 
         try {
-          fs.copySync(src, destLink)
+          fs.copySync(src, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
         }
@@ -216,7 +216,7 @@ describe('+ copySync() - prevent copying into itself', () => {
         }
       })
 
-      it('should copy the directory successfully when src is a subdir of resolved dest path', () => {
+      it('should copy the directory successfully when src is a subdir of resolved dest path and dereferene is true', () => {
         const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src')
         const destLink = path.join(TEST_DIR, 'dest-symlink')
         fs.copySync(src, srcInDest) // put some stuff in srcInDest
@@ -226,9 +226,9 @@ describe('+ copySync() - prevent copying into itself', () => {
 
         const srclen = klawSync(srcInDest).length
         const destlenBefore = klawSync(dest).length
-
         assert(srclen > 2)
-        fs.copySync(srcInDest, destLink)
+
+        fs.copySync(srcInDest, destLink, { dereference: true })
 
         const destlenAfter = klawSync(dest).length
 
@@ -323,7 +323,7 @@ describe('+ copySync() - prevent copying into itself', () => {
     })
 
     describe('>> when dest is a symlink', () => {
-      it('should error 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 and dereference is true', () => {
         const srcLink = path.join(TEST_DIR, 'src-symlink')
         fs.symlinkSync(src, srcLink, 'dir')
         const destLink = path.join(TEST_DIR, 'dest-symlink')
@@ -335,7 +335,7 @@ describe('+ copySync() - prevent copying into itself', () => {
         assert(destlenBefore > 2)
 
         try {
-          fs.copySync(srcLink, destLink)
+          fs.copySync(srcLink, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
         } finally {
diff --git a/lib/copy-sync/__tests__/symlink.test.js b/lib/copy-sync/__tests__/copy-sync-symlink.test.js
similarity index 98%
rename from lib/copy-sync/__tests__/symlink.test.js
rename to lib/copy-sync/__tests__/copy-sync-symlink.test.js
index c6e550809..3f7cfe1c6 100644
--- a/lib/copy-sync/__tests__/symlink.test.js
+++ b/lib/copy-sync/__tests__/copy-sync-symlink.test.js
@@ -1,7 +1,7 @@
 'use strict'
 
 const os = require('os')
-const fs = require('../../')
+const fs = require('../..')
 const path = require('path')
 const assert = require('assert')
 const copySync = require('../copy-sync')
diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js
index 72174de06..d837780c3 100644
--- a/lib/copy-sync/copy-sync.js
+++ b/lib/copy-sync/copy-sync.js
@@ -21,7 +21,7 @@ function copySync (src, dest, opts) {
     see https://github.com/jprichardson/node-fs-extra/issues/269`)
   }
 
-  const { srcStat, destStat } = stat.checkPathsSync(src, dest, 'copy')
+  const { srcStat, destStat } = stat.checkPathsSync(src, dest, 'copy', opts)
   stat.checkParentPathsSync(src, srcStat, dest, 'copy')
   return handleFilterAndCopy(destStat, src, dest, opts)
 }
@@ -98,13 +98,8 @@ function setDestTimestamps (src, dest) {
 }
 
 function onDir (srcStat, destStat, src, dest, opts) {
-  if (destStat) {
-    if (destStat.isDirectory()) {
-      return copyDir(src, dest, opts)
-    }
-    throw new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`)
-  }
-  return mkDirAndCopy(srcStat.mode, src, dest, opts)
+  if (!destStat) return mkDirAndCopy(srcStat.mode, src, dest, opts)
+  return copyDir(src, dest, opts)
 }
 
 function mkDirAndCopy (srcMode, src, dest, opts) {
@@ -120,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 } = stat.checkPathsSync(srcItem, destItem, 'copy')
+  const { destStat } = stat.checkPathsSync(srcItem, destItem, 'copy', opts)
   return startCopy(destStat, srcItem, destItem, opts)
 }
 
diff --git a/lib/copy/__tests__/copy-broken-symlink.test.js b/lib/copy/__tests__/copy-broken-symlink.test.js
new file mode 100644
index 000000000..81e46dea5
--- /dev/null
+++ b/lib/copy/__tests__/copy-broken-symlink.test.js
@@ -0,0 +1,62 @@
+'use strict'
+
+const fs = require('fs')
+const os = require('os')
+const fse = require('../..')
+const path = require('path')
+const assert = require('assert')
+const copy = require('../copy')
+
+/* global afterEach, beforeEach, describe, it */
+
+describe('copy / broken symlink', () => {
+  const TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-broken-symlink')
+  const src = path.join(TEST_DIR, 'src')
+  const dest = path.join(TEST_DIR, 'dest')
+
+  beforeEach(done => {
+    fse.emptyDir(TEST_DIR, err => {
+      assert.ifError(err)
+      createFixtures(src, done)
+    })
+  })
+
+  afterEach(done => fse.remove(TEST_DIR, done))
+
+  describe('when symlink is broken', () => {
+    it('should not throw error if dereference is false', done => {
+      copy(src, dest, err => {
+        assert.strictEqual(err, null)
+        done()
+      })
+    })
+
+    it('should throw error if dereference is true', done => {
+      copy(src, dest, { dereference: true }, err => {
+        assert.strictEqual(err.code, 'ENOENT')
+        done()
+      })
+    })
+  })
+})
+
+function createFixtures (srcDir, callback) {
+  fs.mkdir(srcDir, err => {
+    let brokenFile
+    let brokenFileLink
+
+    if (err) return callback(err)
+
+    try {
+      brokenFile = path.join(srcDir, 'does-not-exist')
+      brokenFileLink = path.join(srcDir, 'broken-symlink')
+      fs.writeFileSync(brokenFile, 'does not matter')
+      fs.symlinkSync(brokenFile, brokenFileLink, 'file')
+    } catch (err) {
+      callback(err)
+    }
+
+    // break the symlink now
+    fse.remove(brokenFile, callback)
+  })
+}
diff --git a/lib/copy/__tests__/copy-prevent-copying-identical.test.js b/lib/copy/__tests__/copy-prevent-copying-identical.test.js
index b79674aee..4c6ebe940 100644
--- a/lib/copy/__tests__/copy-prevent-copying-identical.test.js
+++ b/lib/copy/__tests__/copy-prevent-copying-identical.test.js
@@ -104,7 +104,7 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
 
   describe('> when src is a directory', () => {
     describe('>> when src is regular and dest is a symlink that points to src', () => {
-      it('should error', done => {
+      it('should error if dereference is true', done => {
         src = path.join(TEST_DIR, 'src')
         fs.mkdirsSync(src)
         const subdir = path.join(TEST_DIR, 'src', 'subdir')
@@ -116,7 +116,7 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
 
         const oldlen = klawSync(src).length
 
-        fs.copy(src, destLink, err => {
+        fs.copy(src, destLink, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const newlen = klawSync(src).length
@@ -155,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 error src and dest are the same', done => {
+      it('should error src and dest are the same and dereference is true', done => {
         src = path.join(TEST_DIR, 'src')
         fs.mkdirsSync(src)
         const srcLink = path.join(TEST_DIR, 'src_symlink')
@@ -166,7 +166,7 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
         const srclenBefore = klawSync(srcLink).length
         const destlenBefore = klawSync(destLink).length
 
-        fs.copy(srcLink, destLink, err => {
+        fs.copy(srcLink, destLink, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const srclenAfter = klawSync(srcLink).length
@@ -198,7 +198,7 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
         const destLink = path.join(TEST_DIR, 'dest-symlink')
         fs.symlinkSync(src, destLink, 'file')
 
-        fs.copy(src, destLink, err => {
+        fs.copy(src, destLink, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
           done()
         })
@@ -206,14 +206,14 @@ 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 => {
+      it('should throw error if dereference is true', done => {
         dest = path.join(TEST_DIR, 'dest', 'somefile.txt')
         fs.outputFileSync(dest, 'some data')
 
         const srcLink = path.join(TEST_DIR, 'src-symlink')
         fs.symlinkSync(dest, srcLink, 'file')
 
-        fs.copy(srcLink, dest, err => {
+        fs.copy(srcLink, dest, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const link = fs.readlinkSync(srcLink)
@@ -225,7 +225,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 error src and dest are the same', done => {
+      it('should error src and dest are the same and dereferene is true', done => {
         src = path.join(TEST_DIR, 'src', 'srcfile.txt')
         fs.outputFileSync(src, 'src data')
 
@@ -235,7 +235,7 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
         const destLink = path.join(TEST_DIR, 'dest_symlink')
         fs.symlinkSync(src, destLink, 'file')
 
-        fs.copy(srcLink, destLink, err => {
+        fs.copy(srcLink, destLink, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const srcln = fs.readlinkSync(srcLink)
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 7c629234d..4e0fd4ae8 100644
--- a/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js
+++ b/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js
@@ -149,14 +149,14 @@ describe('+ copy() - prevent copying into itself', () => {
     })
 
     describe('>> when dest is a symlink', () => {
-      it('should error when dest points exactly to src', done => {
+      it('should error when dest points exactly to src and dereference is true', done => {
         const destLink = path.join(TEST_DIR, 'dest-symlink')
         fs.symlinkSync(src, destLink, 'dir')
 
         const srclenBefore = klawSync(src).length
         assert(srclenBefore > 2)
 
-        fs.copy(src, destLink, err => {
+        fs.copy(src, destLink, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const srclenAfter = klawSync(src).length
@@ -210,7 +210,7 @@ describe('+ copy() - prevent copying into itself', () => {
         })
       })
 
-      it('should copy the directory successfully when src is a subdir of resolved dest path', done => {
+      it('should copy the directory successfully when src is a subdir of resolved dest path and dereference is true', done => {
         const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src')
         const destLink = path.join(TEST_DIR, 'dest-symlink')
         fs.copySync(src, srcInDest) // put some stuff in srcInDest
@@ -222,7 +222,7 @@ describe('+ copy() - prevent copying into itself', () => {
         const destlenBefore = klawSync(dest).length
 
         assert(srclen > 2)
-        fs.copy(srcInDest, destLink, err => {
+        fs.copy(srcInDest, destLink, { dereference: true }, err => {
           assert.ifError(err)
 
           const destlenAfter = klawSync(dest).length
@@ -321,7 +321,7 @@ describe('+ copy() - prevent copying into itself', () => {
     })
 
     describe('>> when dest is a symlink', () => {
-      it('should error 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 and dereference is true', done => {
         const srcLink = path.join(TEST_DIR, 'src-symlink')
         fs.symlinkSync(src, srcLink, 'dir')
         const destLink = path.join(TEST_DIR, 'dest-symlink')
@@ -332,7 +332,7 @@ describe('+ copy() - prevent copying into itself', () => {
         assert(srclenBefore > 2)
         assert(destlenBefore > 2)
 
-        fs.copy(srcLink, destLink, err => {
+        fs.copy(srcLink, destLink, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const srclenAfter = klawSync(srcLink).length
@@ -364,24 +364,6 @@ describe('+ copy() - prevent copying into itself', () => {
           done()
         })
       })
-
-      it('should error when resolved src path is a subdir of resolved dest path', done => {
-        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.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}'.`)
-          const destln = fs.readlinkSync(destLink)
-          assert.strictEqual(destln, dest)
-          done()
-        })
-      })
     })
   })
 })
diff --git a/lib/copy/__tests__/copy.test.js b/lib/copy/__tests__/copy.test.js
index 5cc07e32c..e7eed97f8 100644
--- a/lib/copy/__tests__/copy.test.js
+++ b/lib/copy/__tests__/copy.test.js
@@ -123,6 +123,20 @@ describe('fs-extra', () => {
           })
         })
       })
+
+      describe('> when dest exists and is a directory', () => {
+        it('should return an error', done => {
+          const src = path.join(TEST_DIR, 'file.txt')
+          const dest = path.join(TEST_DIR, 'dir')
+          fse.ensureFileSync(src)
+          fse.ensureDirSync(dest)
+
+          fse.copy(src, dest, err => {
+            assert.strictEqual(err.message, `Cannot overwrite directory '${dest}' with non-directory '${src}'.`)
+            done()
+          })
+        })
+      })
     })
 
     describe('> when src is a directory', () => {
diff --git a/lib/copy/__tests__/ncp/broken-symlink.test.js b/lib/copy/__tests__/ncp/broken-symlink.test.js
index 572e821df..ba398084b 100644
--- a/lib/copy/__tests__/ncp/broken-symlink.test.js
+++ b/lib/copy/__tests__/ncp/broken-symlink.test.js
@@ -23,14 +23,14 @@ describe('ncp broken symlink', () => {
 
   afterEach(done => fse.remove(TEST_DIR, done))
 
-  it('should error if symlink is broken', done => {
+  it('should not error if symlink is broken', done => {
     ncp(src, out, err => {
-      assert(err)
+      assert.strictEqual(err, null)
       done()
     })
   })
 
-  it('should return an error when dereference=true', done => {
+  it('should return an error if symlink is broken and dereference=true', done => {
     ncp(src, out, { dereference: true }, err => {
       assert.strictEqual(err.code, 'ENOENT')
       done()
diff --git a/lib/copy/copy.js b/lib/copy/copy.js
index 480dfbbe3..14aab7d5c 100644
--- a/lib/copy/copy.js
+++ b/lib/copy/copy.js
@@ -27,7 +27,7 @@ function copy (src, dest, opts, cb) {
     see https://github.com/jprichardson/node-fs-extra/issues/269`)
   }
 
-  stat.checkPaths(src, dest, 'copy', (err, stats) => {
+  stat.checkPaths(src, dest, 'copy', opts, (err, stats) => {
     if (err) return cb(err)
     const { srcStat, destStat } = stats
     stat.checkParentPaths(src, srcStat, dest, 'copy', err => {
@@ -142,13 +142,8 @@ function setDestTimestamps (src, dest, cb) {
 }
 
 function onDir (srcStat, destStat, src, dest, opts, cb) {
-  if (destStat) {
-    if (destStat.isDirectory()) {
-      return copyDir(src, dest, opts, cb)
-    }
-    return cb(new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`))
-  }
-  return mkDirAndCopy(srcStat.mode, src, dest, opts, cb)
+  if (!destStat) return mkDirAndCopy(srcStat.mode, src, dest, opts, cb)
+  return copyDir(src, dest, opts, cb)
 }
 
 function mkDirAndCopy (srcMode, src, dest, opts, cb) {
@@ -177,7 +172,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)
-  stat.checkPaths(srcItem, destItem, 'copy', (err, stats) => {
+  stat.checkPaths(srcItem, destItem, 'copy', opts, (err, stats) => {
     if (err) return cb(err)
     const { destStat } = stats
     startCopy(destStat, srcItem, destItem, opts, err => {
diff --git a/lib/move-sync/__tests__/move-sync-prevent-moving-identical.test.js b/lib/move-sync/__tests__/move-sync-prevent-moving-identical.test.js
index aed8acddd..343586a36 100644
--- a/lib/move-sync/__tests__/move-sync-prevent-moving-identical.test.js
+++ b/lib/move-sync/__tests__/move-sync-prevent-moving-identical.test.js
@@ -113,7 +113,7 @@ describe('+ moveSync() - prevent moving identical files and dirs', () => {
 
   describe('> when src is a directory', () => {
     describe('>> when src is regular and dest is a symlink that points to src', () => {
-      it('should error', () => {
+      it('should error if dereferene is true', () => {
         src = path.join(TEST_DIR, 'src')
         fs.mkdirsSync(src)
         const subdir = path.join(TEST_DIR, 'src', 'subdir')
@@ -126,7 +126,7 @@ describe('+ moveSync() - prevent moving identical files and dirs', () => {
         const oldlen = klawSync(src).length
 
         try {
-          fs.moveSync(src, destLink)
+          fs.moveSync(src, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
         }
@@ -166,7 +166,7 @@ describe('+ moveSync() - prevent moving identical files and dirs', () => {
     })
 
     describe('>> when src and dest are symlinks that point to the exact same path', () => {
-      it('should error src and dest are the same', () => {
+      it('should error if src and dest are the same and dereference is true', () => {
         src = path.join(TEST_DIR, 'src')
         fs.mkdirsSync(src)
         const srcLink = path.join(TEST_DIR, 'src_symlink')
@@ -178,7 +178,7 @@ describe('+ moveSync() - prevent moving identical files and dirs', () => {
         const destlenBefore = klawSync(destLink).length
 
         try {
-          fs.moveSync(srcLink, destLink)
+          fs.moveSync(srcLink, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
         }
@@ -203,7 +203,7 @@ describe('+ moveSync() - prevent moving 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', () => {
+      it('should error if dereference is true', () => {
         src = path.join(TEST_DIR, 'src', 'somefile.txt')
         fs.ensureFileSync(src)
         fs.writeFileSync(src, 'some data')
@@ -212,7 +212,7 @@ describe('+ moveSync() - prevent moving identical files and dirs', () => {
         fs.symlinkSync(src, destLink, 'file')
 
         try {
-          fs.moveSync(src, destLink)
+          fs.moveSync(src, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
         }
@@ -243,7 +243,7 @@ describe('+ moveSync() - prevent moving identical files and dirs', () => {
     })
 
     describe('>> when src and dest are symlinks that point to the exact same path', () => {
-      it('should error src and dest are the same', () => {
+      it('should error if src and dest are the same and dereference is true', () => {
         src = path.join(TEST_DIR, 'src', 'srcfile.txt')
         fs.outputFileSync(src, 'src data')
 
@@ -254,7 +254,7 @@ describe('+ moveSync() - prevent moving identical files and dirs', () => {
         fs.symlinkSync(src, destLink, 'file')
 
         try {
-          fs.moveSync(srcLink, destLink)
+          fs.moveSync(srcLink, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
         }
diff --git a/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js b/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js
index dc2c75970..ba31f92e1 100644
--- a/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js
+++ b/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js
@@ -167,7 +167,7 @@ describe('+ moveSync() - prevent moving into itself', () => {
     })
 
     describe('>> when dest is a symlink', () => {
-      it('should error when dest points exactly to src', () => {
+      it('should error when dest points exactly to src and dereference is true', () => {
         const destLink = path.join(TEST_DIR, 'dest-symlink')
         fs.symlinkSync(src, destLink, 'dir')
 
@@ -175,7 +175,7 @@ describe('+ moveSync() - prevent moving into itself', () => {
         assert(srclenBefore > 2)
         let errThrown = false
         try {
-          fs.moveSync(src, destLink)
+          fs.moveSync(src, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
           errThrown = true
@@ -321,7 +321,7 @@ describe('+ moveSync() - prevent moving into itself', () => {
     })
 
     describe('>> when dest is a symlink', () => {
-      it('should error 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 and dereferene is true', () => {
         const srcLink = path.join(TEST_DIR, 'src-symlink')
         fs.symlinkSync(src, srcLink, 'dir')
         const destLink = path.join(TEST_DIR, 'dest-symlink')
@@ -333,7 +333,7 @@ describe('+ moveSync() - prevent moving into itself', () => {
         assert(destlenBefore > 2)
         let errThrown = false
         try {
-          fs.moveSync(srcLink, destLink)
+          fs.moveSync(srcLink, destLink, { dereference: true })
         } catch (err) {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
           errThrown = true
diff --git a/lib/move-sync/move-sync.js b/lib/move-sync/move-sync.js
index 20f910cc2..40244dcf9 100644
--- a/lib/move-sync/move-sync.js
+++ b/lib/move-sync/move-sync.js
@@ -11,7 +11,7 @@ function moveSync (src, dest, opts) {
   opts = opts || {}
   const overwrite = opts.overwrite || opts.clobber || false
 
-  const { srcStat } = stat.checkPathsSync(src, dest, 'move')
+  const { srcStat } = stat.checkPathsSync(src, dest, 'move', opts)
   stat.checkParentPathsSync(src, srcStat, dest, 'move')
   mkdirpSync(path.dirname(dest))
   return doRename(src, dest, overwrite)
diff --git a/lib/move/__tests__/move-prevent-moving-identical.test.js b/lib/move/__tests__/move-prevent-moving-identical.test.js
index 840ab004e..c2d4a1654 100644
--- a/lib/move/__tests__/move-prevent-moving-identical.test.js
+++ b/lib/move/__tests__/move-prevent-moving-identical.test.js
@@ -104,7 +104,7 @@ describe('+ move() - prevent moving identical files and dirs', () => {
 
   describe('> when src is a directory', () => {
     describe('>> when src is regular and dest is a symlink that points to src', () => {
-      it('should error', done => {
+      it('should error if dereference is true', done => {
         src = path.join(TEST_DIR, 'src')
         fs.mkdirsSync(src)
         const subdir = path.join(TEST_DIR, 'src', 'subdir')
@@ -116,7 +116,7 @@ describe('+ move() - prevent moving identical files and dirs', () => {
 
         const oldlen = klawSync(src).length
 
-        fs.move(src, destLink, err => {
+        fs.move(src, destLink, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const newlen = klawSync(src).length
@@ -155,7 +155,7 @@ describe('+ move() - prevent moving identical files and dirs', () => {
     })
 
     describe('>> when src and dest are symlinks that point to the exact same path', () => {
-      it('should error src and dest are the same', done => {
+      it('should error src and dest are the same and dereference is true', done => {
         src = path.join(TEST_DIR, 'src')
         fs.mkdirsSync(src)
         const srcLink = path.join(TEST_DIR, 'src_symlink')
@@ -166,7 +166,7 @@ describe('+ move() - prevent moving identical files and dirs', () => {
         const srclenBefore = klawSync(srcLink).length
         const destlenBefore = klawSync(destLink).length
 
-        fs.move(srcLink, destLink, err => {
+        fs.move(srcLink, destLink, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const srclenAfter = klawSync(srcLink).length
@@ -191,14 +191,14 @@ describe('+ move() - prevent moving 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 => {
+      it('should error if dereference is true', done => {
         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.move(src, destLink, err => {
+        fs.move(src, destLink, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
           done()
         })
@@ -206,14 +206,14 @@ describe('+ move() - prevent moving identical files and dirs', () => {
     })
 
     describe('>> when src is a symlink that points to a regular dest', () => {
-      it('should throw error', done => {
+      it('should throw error if dereferene is true', done => {
         dest = path.join(TEST_DIR, 'dest', 'somefile.txt')
         fs.outputFileSync(dest, 'some data')
 
         const srcLink = path.join(TEST_DIR, 'src-symlink')
         fs.symlinkSync(dest, srcLink, 'file')
 
-        fs.move(srcLink, dest, err => {
+        fs.move(srcLink, dest, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const link = fs.readlinkSync(srcLink)
@@ -225,7 +225,7 @@ describe('+ move() - prevent moving identical files and dirs', () => {
     })
 
     describe('>> when src and dest are symlinks that point to the exact same path', () => {
-      it('should error src and dest are the same', done => {
+      it('should error src and dest are the same and dereferene is true', done => {
         src = path.join(TEST_DIR, 'src', 'srcfile.txt')
         fs.outputFileSync(src, 'src data')
 
@@ -235,7 +235,7 @@ describe('+ move() - prevent moving identical files and dirs', () => {
         const destLink = path.join(TEST_DIR, 'dest_symlink')
         fs.symlinkSync(src, destLink, 'file')
 
-        fs.move(srcLink, destLink, err => {
+        fs.move(srcLink, destLink, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const srcln = fs.readlinkSync(srcLink)
diff --git a/lib/move/__tests__/move-prevent-moving-into-itself.test.js b/lib/move/__tests__/move-prevent-moving-into-itself.test.js
index d24aed668..f40eba304 100644
--- a/lib/move/__tests__/move-prevent-moving-into-itself.test.js
+++ b/lib/move/__tests__/move-prevent-moving-into-itself.test.js
@@ -52,9 +52,6 @@ describe('+ move() - prevent moving into itself', () => {
         done()
       })
     })
-  })
-
-  describe('> when source is a file', () => {
     it("should move the file successfully even when dest parent is 'src/dest'", done => {
       const destFile = path.join(TEST_DIR, 'src', 'dest', 'destfile.txt')
       return testSuccessFile(src, destFile, done)
@@ -169,25 +166,6 @@ describe('+ move() - prevent moving into itself', () => {
     })
 
     describe('>> when dest is a symlink', () => {
-      it('should error when dest points exactly to src', done => {
-        const destLink = path.join(TEST_DIR, 'dest-symlink')
-        fs.symlinkSync(src, destLink, 'dir')
-
-        const srclenBefore = klawSync(src).length
-        assert(srclenBefore > 2)
-
-        fs.move(src, destLink, err => {
-          assert.strictEqual(err.message, 'Source and destination must not be the same.')
-
-          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 error when dest is a subdirectory of src (bind-mounted directory with subdirectory)', done => {
         const destLink = path.join(TEST_DIR, 'dest-symlink')
         fs.symlinkSync(src, destLink, 'dir')
@@ -306,7 +284,7 @@ describe('+ move() - prevent moving into itself', () => {
     })
 
     describe('>> when dest is a symlink', () => {
-      it('should error 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 and dereferene is true', done => {
         const srcLink = path.join(TEST_DIR, 'src-symlink')
         fs.symlinkSync(src, srcLink, 'dir')
         const destLink = path.join(TEST_DIR, 'dest-symlink')
@@ -317,7 +295,7 @@ describe('+ move() - prevent moving into itself', () => {
         assert(srclenBefore > 2)
         assert(destlenBefore > 2)
 
-        fs.move(srcLink, destLink, err => {
+        fs.move(srcLink, destLink, { dereference: true }, err => {
           assert.strictEqual(err.message, 'Source and destination must not be the same.')
 
           const srclenAfter = klawSync(srcLink).length
diff --git a/lib/move/move.js b/lib/move/move.js
index fa3ea618a..4687c6ab7 100644
--- a/lib/move/move.js
+++ b/lib/move/move.js
@@ -16,7 +16,7 @@ function move (src, dest, opts, cb) {
 
   const overwrite = opts.overwrite || opts.clobber || false
 
-  stat.checkPaths(src, dest, 'move', (err, stats) => {
+  stat.checkPaths(src, dest, 'move', opts, (err, stats) => {
     if (err) return cb(err)
     const { srcStat } = stats
     stat.checkParentPaths(src, srcStat, dest, 'move', err => {
diff --git a/lib/util/__tests__/stat.test.js b/lib/util/__tests__/stat.test.js
index 8d0e85de7..2b7de648d 100644
--- a/lib/util/__tests__/stat.test.js
+++ b/lib/util/__tests__/stat.test.js
@@ -27,7 +27,7 @@ describe('util/stat', () => {
       const dest = path.join(TEST_DIR, 'dest')
       fs.ensureFileSync(src)
       fs.ensureFileSync(dest)
-      stat.checkPaths(src, dest, 'copy', (err, stats) => {
+      stat.checkPaths(src, dest, 'copy', {}, (err, stats) => {
         assert.ifError(err)
         const { srcStat } = stats
         if (atLeastNode(NODE_VERSION_WITH_BIGINT)) {
@@ -43,7 +43,7 @@ describe('util/stat', () => {
       const dest = path.join(TEST_DIR, 'dest')
       fs.ensureFileSync(src)
       fs.ensureFileSync(dest)
-      const { srcStat } = stat.checkPathsSync(src, dest, 'copy')
+      const { srcStat } = stat.checkPathsSync(src, dest, 'copy', {})
       if (atLeastNode(NODE_VERSION_WITH_BIGINT)) {
         assert.strictEqual(typeof srcStat.ino, 'bigint')
       } else {
diff --git a/lib/util/stat.js b/lib/util/stat.js
index 0b1c1b096..7927900c7 100644
--- a/lib/util/stat.js
+++ b/lib/util/stat.js
@@ -7,23 +7,27 @@ const atLeastNode = require('at-least-node')
 
 const nodeSupportsBigInt = atLeastNode('10.5.0')
 const stat = (file) => nodeSupportsBigInt ? fs.stat(file, { bigint: true }) : fs.stat(file)
+const lstat = (file) => nodeSupportsBigInt ? fs.lstat(file, { bigint: true }) : fs.lstat(file)
 const statSync = (file) => nodeSupportsBigInt ? fs.statSync(file, { bigint: true }) : fs.statSync(file)
+const lstatSync = (file) => nodeSupportsBigInt ? fs.lstatSync(file, { bigint: true }) : fs.lstatSync(file)
 
-function getStats (src, dest) {
+function getStats (src, dest, opts) {
+  const statFunc = opts.dereference ? stat : lstat
   return Promise.all([
-    stat(src),
-    stat(dest).catch(err => {
+    statFunc(src),
+    statFunc(dest).catch(err => {
       if (err.code === 'ENOENT') return null
       throw err
     })
   ]).then(([srcStat, destStat]) => ({ srcStat, destStat }))
 }
 
-function getStatsSync (src, dest) {
+function getStatsSync (src, dest, opts) {
   let destStat
-  const srcStat = statSync(src)
+  const statFunc = opts.dereference ? statSync : lstatSync
+  const srcStat = statFunc(src)
   try {
-    destStat = statSync(dest)
+    destStat = statFunc(dest)
   } catch (err) {
     if (err.code === 'ENOENT') return { srcStat, destStat: null }
     throw err
@@ -31,13 +35,23 @@ function getStatsSync (src, dest) {
   return { srcStat, destStat }
 }
 
-function checkPaths (src, dest, funcName, cb) {
-  util.callbackify(getStats)(src, dest, (err, stats) => {
+function checkPaths (src, dest, funcName, opts, cb) {
+  util.callbackify(getStats)(src, dest, opts, (err, stats) => {
     if (err) return cb(err)
     const { srcStat, destStat } = stats
-    if (destStat && areIdentical(srcStat, destStat)) {
-      return cb(new Error('Source and destination must not be the same.'))
+
+    if (destStat) {
+      if (areIdentical(srcStat, destStat)) {
+        return cb(new Error('Source and destination must not be the same.'))
+      }
+      if (srcStat.isDirectory() && !destStat.isDirectory()) {
+        return cb(new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`))
+      }
+      if (!srcStat.isDirectory() && destStat.isDirectory()) {
+        return cb(new Error(`Cannot overwrite directory '${dest}' with non-directory '${src}'.`))
+      }
     }
+
     if (srcStat.isDirectory() && isSrcSubdir(src, dest)) {
       return cb(new Error(errMsg(src, dest, funcName)))
     }
@@ -45,11 +59,21 @@ function checkPaths (src, dest, funcName, cb) {
   })
 }
 
-function checkPathsSync (src, dest, funcName) {
-  const { srcStat, destStat } = getStatsSync(src, dest)
-  if (destStat && areIdentical(srcStat, destStat)) {
-    throw new Error('Source and destination must not be the same.')
+function checkPathsSync (src, dest, funcName, opts) {
+  const { srcStat, destStat } = getStatsSync(src, dest, opts)
+
+  if (destStat) {
+    if (areIdentical(srcStat, destStat)) {
+      throw new Error('Source and destination must not be the same.')
+    }
+    if (srcStat.isDirectory() && !destStat.isDirectory()) {
+      throw new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`)
+    }
+    if (!srcStat.isDirectory() && destStat.isDirectory()) {
+      throw new Error(`Cannot overwrite directory '${dest}' with non-directory '${src}'.`)
+    }
   }
+
   if (srcStat.isDirectory() && isSrcSubdir(src, dest)) {
     throw new Error(errMsg(src, dest, funcName))
   }