BREAKING: copy*(): allow copying broken symlinks (#779)

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

@@ -2,17 +2,17 @@
 
 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')
 
 /* 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')
+    })
   })
 })
 

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')

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.')
         }

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

@@ -146,15 +146,15 @@ 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')
 
         const srclenBefore = klawSync(src).length
         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 {

lib/copy-sync/__tests__/symlink.test.js → 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')

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)
 }
 

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)
+  })
+}

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,22 +198,22 @@ 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()
         })
       })
     })
 
     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)