From fe0bfe29313331eb1ef9fecf3c35053ce32def4e Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 19 Oct 2017 03:29:50 -0700 Subject: [PATCH 01/13] Rewrite copy to use recursive pattern for dirs, add more tests --- .../copy-prevent-copying-identical.test.js | 192 +++++++++ .../copy-prevent-copying-into-itself.test.js | 372 ++++++++++++++++++ lib/copy/__tests__/copy.test.js | 38 ++ lib/copy/__tests__/ncp/broken-symlink.test.js | 2 +- lib/copy/__tests__/ncp/ncp-error-perm.test.js | 2 +- lib/copy/__tests__/ncp/ncp.test.js | 2 +- lib/copy/__tests__/ncp/symlink.test.js | 2 +- lib/copy/copy.js | 259 ++++++++++-- lib/copy/ncp.js | 234 ----------- lib/move/index.js | 10 +- 10 files changed, 837 insertions(+), 276 deletions(-) create mode 100644 lib/copy/__tests__/copy-prevent-copying-identical.test.js create mode 100644 lib/copy/__tests__/copy-prevent-copying-into-itself.test.js delete mode 100644 lib/copy/ncp.js diff --git a/lib/copy/__tests__/copy-prevent-copying-identical.test.js b/lib/copy/__tests__/copy-prevent-copying-identical.test.js new file mode 100644 index 00000000..64357105 --- /dev/null +++ b/lib/copy/__tests__/copy-prevent-copying-identical.test.js @@ -0,0 +1,192 @@ +'use strict' + +const assert = require('assert') +const os = require('os') +const path = require('path') +const fs = require(process.cwd()) +const klawSync = require('klaw-sync') + +/* global beforeEach, afterEach, describe, it */ + +describe('+ copySync() - prevent copying identical files and dirs', () => { + let TEST_DIR = '' + let src = '' + let dest = '' + + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-prevent-copying-identical') + fs.emptyDir(TEST_DIR, done) + }) + + afterEach(done => fs.remove(TEST_DIR, done)) + + it('should return an error if src and dest are the same', done => { + const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync') + const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync') + + fs.copy(fileSrc, fileDest, err => { + assert.equal(err.message, 'Source and destination must not be the same.') + done() + }) + }) + + // src is directory: + // src is regular, dest is symlink + // src is symlink, dest is regular + // src is symlink, dest is symlink + + describe('> when the source is a directory', () => { + describe(`>> when src is regular and dest is a symlink that points to src`, () => { + it('should not copy and return', done => { + src = path.join(TEST_DIR, 'src') + fs.mkdirsSync(src) + const subdir = path.join(TEST_DIR, 'src', 'subdir') + fs.mkdirsSync(subdir) + fs.writeFileSync(path.join(subdir, 'file.txt'), 'some data') + + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const oldlen = klawSync(src).length + + fs.copy(src, destLink, err => { + assert.ifError(err) + + const newlen = klawSync(src).length + assert.strictEqual(newlen, oldlen) + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + done() + }) + }) + }) + + describe(`>> when src is a symlink that points to a regular dest`, () => { + it('should throw error', done => { + dest = path.join(TEST_DIR, 'dest') + fs.mkdirsSync(dest) + const subdir = path.join(TEST_DIR, 'dest', 'subdir') + fs.mkdirsSync(subdir) + fs.writeFileSync(path.join(subdir, 'file.txt'), 'some data') + + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(dest, srcLink, 'dir') + + const oldlen = klawSync(dest).length + + fs.copy(srcLink, dest, err => { + assert.ok(err) + + // assert nothing copied + const newlen = klawSync(dest).length + assert.strictEqual(newlen, oldlen) + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, dest) + done() + }) + }) + }) + + describe('>> when src and dest are symlinks that point to the exact same path', () => { + it('should not copy and return', done => { + src = path.join(TEST_DIR, 'src') + fs.mkdirsSync(src) + const srcLink = path.join(TEST_DIR, 'src_symlink') + fs.symlinkSync(src, srcLink, 'dir') + const destLink = path.join(TEST_DIR, 'dest_symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(srcLink).length + const destlenBefore = klawSync(destLink).length + + fs.copy(srcLink, destLink, err => { + assert.ifError(err) + + 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) + done() + }) + }) + }) + }) + + // src is file: + // src is regular, dest is symlink + // src is symlink, dest is regular + // src is symlink, dest is symlink + + describe('> when the source is a file', () => { + describe(`>> when src is regular and dest is a symlink that points to src`, () => { + it('should not copy and return', done => { + src = path.join(TEST_DIR, 'src', 'somefile.txt') + fs.ensureFileSync(src) + fs.writeFileSync(src, 'some data') + + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'file') + + fs.copy(src, destLink, err => { + assert.ifError(err) + + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + assert(fs.readFileSync(link, 'utf8'), 'some data') + done() + }) + }) + }) + + 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') + + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(dest, srcLink, 'file') + + fs.copy(srcLink, dest, err => { + assert.ok(err) + + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, dest) + assert(fs.readFileSync(link, 'utf8'), 'some data') + done() + }) + }) + }) + + describe('>> when src and dest are symlinks that point to the exact same path', () => { + it('should not copy and return', done => { + src = path.join(TEST_DIR, 'src', 'srcfile.txt') + fs.ensureFileSync(src) + fs.writeFileSync(src, 'src data') + + const srcLink = path.join(TEST_DIR, 'src_symlink') + fs.symlinkSync(src, srcLink, 'file') + + const destLink = path.join(TEST_DIR, 'dest_symlink') + fs.symlinkSync(src, destLink, 'file') + + fs.copy(srcLink, destLink, err => { + assert.ifError(err) + + const srcln = fs.readlinkSync(srcLink) + assert.strictEqual(srcln, src) + const destln = fs.readlinkSync(destLink) + assert.strictEqual(destln, src) + assert(fs.readFileSync(srcln, 'utf8'), 'src data') + assert(fs.readFileSync(destln, 'utf8'), 'src data') + done() + }) + }) + }) + }) +}) diff --git a/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js b/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js new file mode 100644 index 00000000..d759a1a5 --- /dev/null +++ b/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js @@ -0,0 +1,372 @@ +'use strict' + +const assert = require('assert') +const os = require('os') +const path = require('path') +const fs = require(process.cwd()) +const klawSync = require('klaw-sync') + +/* global beforeEach, afterEach, describe, it */ + +// these files are used for all tests +const FILES = [ + 'file0.txt', + path.join('dir1', 'file1.txt'), + path.join('dir1', 'dir2', 'file2.txt'), + path.join('dir1', 'dir2', 'dir3', 'file3.txt') +] + +const dat0 = 'file0' +const dat1 = 'file1' +const dat2 = 'file2' +const dat3 = 'file3' + +function testSuccess (src, dest, done) { + const srclen = klawSync(src).length + assert(srclen > 2) + fs.copy(src, dest, err => { + assert.ifError(err) + + const destlen = klawSync(dest).length + + assert.strictEqual(destlen, srclen) + + FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied')) + + const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8') + const o1 = fs.readFileSync(path.join(dest, FILES[1]), 'utf8') + const o2 = fs.readFileSync(path.join(dest, FILES[2]), 'utf8') + const o3 = fs.readFileSync(path.join(dest, FILES[3]), 'utf8') + + assert.strictEqual(o0, dat0, 'file contents matched') + assert.strictEqual(o1, dat1, 'file contents matched') + assert.strictEqual(o2, dat2, 'file contents matched') + assert.strictEqual(o3, dat3, 'file contents matched') + done() + }) +} + +function testError (src, dest, done) { + fs.copy(src, dest, err => { + assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + done() + }) +} + +describe('+ copy() - prevent copying into itself', () => { + let TEST_DIR, src + + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-prevent-copying-into-itself-4') + src = path.join(TEST_DIR, 'src') + fs.mkdirpSync(src) + + fs.outputFileSync(path.join(src, FILES[0]), dat0) + fs.outputFileSync(path.join(src, FILES[1]), dat1) + fs.outputFileSync(path.join(src, FILES[2]), dat2) + fs.outputFileSync(path.join(src, FILES[3]), dat3) + done() + }) + + 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 => { + const srcFile = path.join(TEST_DIR, 'src', 'srcfile.txt') + const destFile = path.join(TEST_DIR, 'src', 'dest', 'destfile.txt') + fs.writeFileSync(srcFile, dat0) + + fs.copy(srcFile, destFile, err => { + assert.ifError(err) + + assert(fs.existsSync(destFile, 'file copied')) + const out = fs.readFileSync(destFile, 'utf8') + assert.strictEqual(out, dat0, 'file contents matched') + done() + }) + }) + }) + + // test for these cases: + // - src is directory, dest is directory + // - src is directory, dest is symlink + // - src is symlink, dest is directory + // - src is symlink, dest is symlink + + describe('> when source is a directory', () => { + describe('>> when dest is a directory', () => { + it(`should copy the directory successfully when dest is 'src_dest'`, done => { + const dest = path.join(TEST_DIR, 'src_dest') + return testSuccess(src, dest, done) + }) + it(`should copy the directory successfully when dest is 'src-dest'`, done => { + const dest = path.join(TEST_DIR, 'src-dest') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'dest_src'`, done => { + const dest = path.join(TEST_DIR, 'dest_src') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'src_dest/src'`, done => { + const dest = path.join(TEST_DIR, 'src_dest', 'src') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'src-dest/src'`, done => { + const dest = path.join(TEST_DIR, 'src-dest', 'src') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'dest_src/src'`, done => { + const dest = path.join(TEST_DIR, 'dest_src', 'src') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'src_src/dest'`, done => { + const dest = path.join(TEST_DIR, 'src_src', 'dest') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'src-src/dest'`, done => { + const dest = path.join(TEST_DIR, 'src-src', 'dest') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'srcsrc/dest'`, done => { + const dest = path.join(TEST_DIR, 'srcsrc', 'dest') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'dest/src'`, done => { + const dest = path.join(TEST_DIR, 'dest', 'src') + return testSuccess(src, dest, done) + }) + + it('should copy the directory successfully when dest is very nested that all its parents need to be created', done => { + const dest = path.join(TEST_DIR, 'dest', 'src', 'foo', 'bar', 'baz', 'qux', 'quux', 'waldo', + 'grault', 'garply', 'fred', 'plugh', 'thud', 'some', 'highly', 'deeply', + 'badly', 'nasty', 'crazy', 'mad', 'nested', 'dest') + return testSuccess(src, dest, done) + }) + + it(`should error when dest is 'src/dest'`, done => { + const dest = path.join(TEST_DIR, 'src', 'dest') + return testError(src, dest, done) + }) + + it(`should error when dest is 'src/src_dest'`, done => { + const dest = path.join(TEST_DIR, 'src', 'src_dest') + return testError(src, dest, done) + }) + + it(`should error when dest is 'src/dest_src'`, done => { + const dest = path.join(TEST_DIR, 'src', 'dest_src') + return testError(src, dest, done) + }) + + it(`should error when dest is 'src/dest/src'`, done => { + const dest = path.join(TEST_DIR, 'src', 'dest', 'src') + return testError(src, dest, done) + }) + }) + + describe('>> when dest is a symlink', () => { + it('should not copy and return 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.copy(src, destLink, err => { + assert.ifError(err) + + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + done() + }) + }) + + it('should copy the directory successfully when src is a subdir of resolved dest path', done => { + const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src') + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.copySync(src, srcInDest) // put some stuff in srcInDest + + const dest = path.join(TEST_DIR, 'dest') + fs.symlinkSync(dest, destLink, 'dir') + + const srclen = klawSync(srcInDest).length + const destlenBefore = klawSync(dest).length + + assert(srclen > 2) + fs.copy(srcInDest, destLink, err => { + assert.ifError(err) + + const destlenAfter = klawSync(dest).length + + // assert dest length is oldlen + length of stuff copied from src + assert.strictEqual(destlenAfter, destlenBefore + srclen, 'dest length should be equal to old length + copied legnth') + + FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied')) + + const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8') + const o1 = fs.readFileSync(path.join(dest, FILES[1]), 'utf8') + const o2 = fs.readFileSync(path.join(dest, FILES[2]), 'utf8') + const o3 = fs.readFileSync(path.join(dest, FILES[3]), 'utf8') + + assert.strictEqual(o0, dat0, 'files contents matched') + assert.strictEqual(o1, dat1, 'files contents matched') + assert.strictEqual(o2, dat2, 'files contents matched') + assert.strictEqual(o3, dat3, 'files contents matched') + done() + }) + }) + }) + }) + + describe('> when source is a symlink', () => { + describe('>> when dest is a directory', () => { + it('should error when resolved src path points to dest', done => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + + const dest = path.join(TEST_DIR, 'src') + + fs.copy(srcLink, dest, err => { + assert(err) + // assert source not affected + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, src) + done() + }) + }) + + it('should error when dest is a subdir of resolved src path', done => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + + const dest = path.join(TEST_DIR, 'src', 'some', 'nested', 'dest') + fs.mkdirsSync(dest) + + fs.copy(srcLink, dest, err => { + assert(err) + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, src) + done() + }) + }) + + it('should error when resolved src path is a subdir of dest', done => { + const dest = path.join(TEST_DIR, 'dest') + + const resolvedSrcPath = path.join(dest, 'contains', 'src') + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.copySync(src, resolvedSrcPath) + + // make symlink that points to a subdir in dest + fs.symlinkSync(resolvedSrcPath, srcLink, 'dir') + + fs.copy(srcLink, dest, err => { + assert(err) + done() + }) + }) + + it(`should copy the directory successfully when dest is 'src_src/dest'`, done => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + + const dest = path.join(TEST_DIR, 'src_src', 'dest') + testSuccess(srcLink, dest, () => { + const link = fs.readlinkSync(dest) + assert.strictEqual(link, src) + done() + }) + }) + + it(`should copy the directory successfully when dest is 'srcsrc/dest'`, done => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + + const dest = path.join(TEST_DIR, 'srcsrc', 'dest') + testSuccess(srcLink, dest, () => { + const link = fs.readlinkSync(dest) + assert.strictEqual(link, src) + done() + }) + }) + }) + + describe('>> when dest is a symlink', () => { + it('should silently return 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') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(srcLink).length + const destlenBefore = klawSync(destLink).length + assert(srclenBefore > 2) + assert(destlenBefore > 2) + + fs.copy(srcLink, destLink, err => { + assert.ifError(err) + + 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) + done() + }) + }) + + it('should error when resolved dest path is a subdir of 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') + 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) + const destln = fs.readlinkSync(destLink) + assert.strictEqual(destln, src) + 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.mkdirSync(dest) + + fs.symlinkSync(srcInDest, srcLink, 'dir') + fs.symlinkSync(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 6ed64ff8..4787bfd8 100644 --- a/lib/copy/__tests__/copy.test.js +++ b/lib/copy/__tests__/copy.test.js @@ -30,6 +30,30 @@ describe('fs-extra', () => { }) }) + it('should error when overwrite=false and file exists', done => { + const src = path.join(TEST_DIR, 'src.txt') + const dest = path.join(TEST_DIR, 'dest.txt') + + fse.ensureFileSync(src) + fse.ensureFileSync(dest) + fse.copy(src, dest, {overwrite: false, errorOnExist: true}, err => { + assert(err) + done() + }) + }) + + it('should error when overwrite=false and file exists in a dir', done => { + const src = path.join(TEST_DIR, 'src', 'sfile.txt') + const dest = path.join(TEST_DIR, 'dest', 'dfile.txt') + + fse.ensureFileSync(src) + fse.ensureFileSync(dest) + fse.copy(src, dest, {overwrite: false, errorOnExist: true}, err => { + assert(err) + done() + }) + }) + describe('> when the source is a file', () => { it('should copy the file asynchronously', done => { const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_src') @@ -98,6 +122,20 @@ describe('fs-extra', () => { }) }) + describe('> when dest exists and is a file', () => { + it('should return an error', done => { + const src = path.join(TEST_DIR, 'src') + const dest = path.join(TEST_DIR, 'file.txt') + fs.mkdirSync(src) + fse.ensureFileSync(dest) + + fse.copy(src, dest, err => { + assert.strictEqual(err.message, `Cannot overwrite non-directory '${dest}' with directory '${src}'.`) + done() + }) + }) + }) + it('should copy the directory asynchronously', done => { const FILES = 2 const src = path.join(TEST_DIR, 'src') diff --git a/lib/copy/__tests__/ncp/broken-symlink.test.js b/lib/copy/__tests__/ncp/broken-symlink.test.js index 781ac6d9..278b2c11 100644 --- a/lib/copy/__tests__/ncp/broken-symlink.test.js +++ b/lib/copy/__tests__/ncp/broken-symlink.test.js @@ -3,7 +3,7 @@ const fs = require('fs') const os = require('os') const fse = require(process.cwd()) -const ncp = require('../../ncp') +const ncp = require('../../copy') const path = require('path') const assert = require('assert') diff --git a/lib/copy/__tests__/ncp/ncp-error-perm.test.js b/lib/copy/__tests__/ncp/ncp-error-perm.test.js index 451bbdf7..18876e69 100644 --- a/lib/copy/__tests__/ncp/ncp-error-perm.test.js +++ b/lib/copy/__tests__/ncp/ncp-error-perm.test.js @@ -5,7 +5,7 @@ const fs = require('fs') const os = require('os') const fse = require(process.cwd()) -const ncp = require('../../ncp') +const ncp = require('../../copy') const path = require('path') const assert = require('assert') diff --git a/lib/copy/__tests__/ncp/ncp.test.js b/lib/copy/__tests__/ncp/ncp.test.js index 23ad3fcc..31ae8296 100644 --- a/lib/copy/__tests__/ncp/ncp.test.js +++ b/lib/copy/__tests__/ncp/ncp.test.js @@ -1,7 +1,7 @@ 'use strict' const fs = require('fs') -const ncp = require('../../ncp') +const ncp = require('../../copy') const path = require('path') const rimraf = require('rimraf') const assert = require('assert') diff --git a/lib/copy/__tests__/ncp/symlink.test.js b/lib/copy/__tests__/ncp/symlink.test.js index 1b8816b6..ff481a5b 100644 --- a/lib/copy/__tests__/ncp/symlink.test.js +++ b/lib/copy/__tests__/ncp/symlink.test.js @@ -3,7 +3,7 @@ const fs = require('fs') const os = require('os') const fse = require(process.cwd()) -const ncp = require('../../ncp') +const ncp = require('../../copy') const path = require('path') const assert = require('assert') diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 309a93df..1aa53228 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -2,53 +2,246 @@ const fs = require('graceful-fs') const path = require('path') -const ncp = require('./ncp') -const mkdir = require('../mkdirs') +const mkdirp = require('../mkdirs').mkdirs const pathExists = require('../path-exists').pathExists +const utimes = require('../util/utimes').utimesMillis -function copy (src, dest, options, callback) { - if (typeof options === 'function' && !callback) { - callback = options - options = {} - } else if (typeof options === 'function' || options instanceof RegExp) { - options = {filter: options} +const notExist = Symbol('notExist') +const existsReg = Symbol('existsReg') + +function copy (src, dest, opts, cb) { + if (typeof opts === 'function' && !cb) { + cb = opts + opts = {} + } else if (typeof opts === 'function' || opts instanceof RegExp) { + opts = {filter: opts} } - callback = callback || function () {} - options = options || {} - // Warn about using preserveTimestamps on 32-bit node: - if (options.preserveTimestamps && process.arch === 'ia32') { + cb = cb || function () {} + opts = opts || {} + + opts.clobber = 'clobber' in opts ? !!opts.clobber : true // default to true for now + opts.overwrite = 'overwrite' in opts ? !!opts.overwrite : opts.clobber // overwrite falls back to clobber + + // Warn about using preserveTimestamps on 32-bit node + if (opts.preserveTimestamps && process.arch === 'ia32') { console.warn(`fs-extra: Using the preserveTimestamps option in 32-bit node is not recommended;\n see https://github.com/jprichardson/node-fs-extra/issues/269`) } + src = path.resolve(src) + dest = path.resolve(dest) + // don't allow src and dest to be the same - const basePath = process.cwd() - const currentPath = path.resolve(basePath, src) - const targetPath = path.resolve(basePath, dest) - if (currentPath === targetPath) return callback(new Error('Source and destination must not be the same.')) - - fs.lstat(src, (err, stats) => { - if (err) return callback(err) - - let dir = null - if (stats.isDirectory()) { - const parts = dest.split(path.sep) - parts.pop() - dir = parts.join(path.sep) + if (src === dest) return cb(new Error('Source and destination must not be the same.')) + + const destParent = path.dirname(dest) + pathExists(destParent, (err, dirExists) => { + if (err) return cb(err) + if (dirExists) return startCopy(src, dest, opts, cb) + mkdirp(destParent, err => { + if (err) return cb(err) + return startCopy(src, dest, opts, cb) + }) + }) +} + +function startCopy (src, dest, opts, cb) { + if (opts.filter) { + if (opts.filter instanceof RegExp) { + console.warn('Warning: fs-extra: Passing a RegExp filter is deprecated, use a function') + if (!opts.filter.test(src)) return cb() + } else if (typeof opts.filter === 'function') { + if (!opts.filter(src, dest)) return cb() + } + } + return getStats(src, dest, opts, cb) +} + +function getStats (src, dest, opts, cb) { + const stat = opts.dereference ? fs.stat : fs.lstat + stat(src, (err, st) => { + if (err) return cb(err) + + if (st.isDirectory()) return onDir(st, src, dest, opts, cb) + else if (st.isFile() || + st.isCharacterDevice() || + st.isBlockDevice()) return onFile(st, src, dest, opts, cb) + else if (st.isSymbolicLink()) return onLink(src, dest, opts, cb) + }) +} + +function onFile (srcStat, src, dest, opts, cb) { + checkDest(dest, (err, resolvedPath) => { + if (err) return cb(err) + if (resolvedPath === notExist) { + return cpFile(srcStat, src, dest, opts, cb) + } else if (resolvedPath === existsReg) { + return mayCopyFile(srcStat, src, dest, opts, cb) } else { - dir = path.dirname(dest) + if (src === resolvedPath) return cb() + return mayCopyFile(srcStat, src, dest, opts, cb) } + }) +} + +function mayCopyFile (srcStat, src, dest, opts, cb) { + if (opts.overwrite) { + fs.unlink(dest, err => { + if (err) return cb(err) + return cpFile(srcStat, src, dest, opts, cb) + }) + } else if (opts.errorOnExist) { + return cb(new Error(`'${dest}' already exists`)) + } else return cb() +} + +function cpFile (srcStat, src, dest, opts, cb) { + const rs = fs.createReadStream(src) + const ws = fs.createWriteStream(dest, { mode: srcStat.mode }) - pathExists(dir, (err, dirExists) => { - if (err) return callback(err) - if (dirExists) return ncp(src, dest, options, callback) - mkdir.mkdirs(dir, err => { - if (err) return callback(err) - ncp(src, dest, options, callback) - }) + rs.on('error', err => cb(err)) + ws.on('error', err => cb(err)) + + ws.on('open', () => { + rs.pipe(ws) + }).once('close', () => { + fs.chmod(dest, srcStat.mode, err => { + if (err) return cb(err) + if (opts.preserveTimestamps) { + return utimes(dest, srcStat.atime, srcStat.mtime, cb) + } + return cb() }) }) } +function onDir (srcStat, src, dest, opts, cb) { + checkDest(dest, (err, resolvedPath) => { + if (err) return cb(err) + if (resolvedPath === 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 (resolvedPath === existsReg) { + if (isSrcSubdir(src, dest)) { + return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) + } + return mayCopyDir(src, dest, opts, cb) + } else { + if (src === resolvedPath) return cb() + return cpDir(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 cpDir(src, dest, opts, cb) + }) +} + +function mkDirAndCopy (srcStat, src, dest, opts, cb) { + fs.mkdir(dest, srcStat.mode, err => { + if (err) return cb(err) + fs.chmod(dest, srcStat.mode, err => { + if (err) return cb(err) + return cpDir(src, dest, opts, cb) + }) + }) +} + +function cpDir (src, dest, opts, cb) { + fs.readdir(src, (err, items) => { + if (err) return cb(err) + return cpDirItems(items, src, dest, opts, cb) + }) +} + +function cpDirItems (items, src, dest, opts, cb) { + const item = items.pop() + if (!item) return cb() + startCopy(path.join(src, item), path.join(dest, item), opts, err => { + if (err) return cb(err) + return cpDirItems(items, src, dest, opts, cb) + }) +} + +function onLink (src, dest, opts, cb) { + fs.readlink(src, (err, resolvedSrcPath) => { + if (err) return cb(err) + + if (opts.dereference) { + resolvedSrcPath = path.resolve(process.cwd(), resolvedSrcPath) + } + + checkDest(dest, (err, resolvedDestPath) => { + if (err) return cb(err) + + if (resolvedDestPath === notExist || resolvedDestPath === existsReg) { + // if dest already exists, fs throws error anyway, + // so no need to guard against it here. + return fs.symlink(resolvedSrcPath, dest, cb) + } else { + if (opts.dereference) { + resolvedDestPath = path.resolve(process.cwd(), resolvedDestPath) + } + if (resolvedDestPath === resolvedSrcPath) return cb() + + // prevent copy if src is a subdir of dest since unlinking + // dest in this case results in removing src contents + // and therefore a broken symlink will be created. + fs.stat(dest, (err, st) => { + if (err) return cb(err) + if (st.isDirectory() && isSrcSubdir(resolvedDestPath, resolvedSrcPath)) { + return cb(new Error(`Cannot overwrite '${resolvedDestPath}' with '${resolvedSrcPath}'.`)) + } + return cpLink(resolvedSrcPath, dest, cb) + }) + } + }) + }) +} + +function cpLink (resolvedSrcPath, dest, cb) { + fs.unlink(dest, err => { + if (err) return cb(err) + return fs.symlink(resolvedSrcPath, dest, cb) + }) +} + +// check dest to see if it exists and/or 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 throws 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 + }) +} + +// 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 baseDir = dest.split(path.dirname(src) + path.sep)[1] + if (baseDir) { + const destBasename = baseDir.split(path.sep)[0] + if (destBasename) { + return src !== dest && dest.indexOf(src) > -1 && destBasename === path.basename(src) + } + return false + } + return false +} + module.exports = copy diff --git a/lib/copy/ncp.js b/lib/copy/ncp.js deleted file mode 100644 index 9670ee02..00000000 --- a/lib/copy/ncp.js +++ /dev/null @@ -1,234 +0,0 @@ -// imported from ncp (this is temporary, will rewrite) - -var fs = require('graceful-fs') -var path = require('path') -var utimes = require('../util/utimes') - -function ncp (source, dest, options, callback) { - if (!callback) { - callback = options - options = {} - } - - var basePath = process.cwd() - var currentPath = path.resolve(basePath, source) - var targetPath = path.resolve(basePath, dest) - - var filter = options.filter - var transform = options.transform - var overwrite = options.overwrite - // If overwrite is undefined, use clobber, otherwise default to true: - if (overwrite === undefined) overwrite = options.clobber - if (overwrite === undefined) overwrite = true - var errorOnExist = options.errorOnExist - var dereference = options.dereference - var preserveTimestamps = options.preserveTimestamps === true - - var started = 0 - var finished = 0 - var running = 0 - - var errored = false - - startCopy(currentPath) - - function startCopy (source) { - started++ - if (filter) { - if (filter instanceof RegExp) { - console.warn('Warning: fs-extra: Passing a RegExp filter is deprecated, use a function') - if (!filter.test(source)) { - return doneOne(true) - } - } else if (typeof filter === 'function') { - if (!filter(source, dest)) { - return doneOne(true) - } - } - } - return getStats(source) - } - - function getStats (source) { - var stat = dereference ? fs.stat : fs.lstat - running++ - stat(source, function (err, stats) { - if (err) return onError(err) - - // We need to get the mode from the stats object and preserve it. - var item = { - name: source, - mode: stats.mode, - mtime: stats.mtime, // modified time - atime: stats.atime, // access time - stats: stats // temporary - } - - if (stats.isDirectory()) { - return onDir(item) - } else if (stats.isFile() || stats.isCharacterDevice() || stats.isBlockDevice()) { - return onFile(item) - } else if (stats.isSymbolicLink()) { - // Symlinks don't really need to know about the mode. - return onLink(source) - } - }) - } - - function onFile (file) { - var target = file.name.replace(currentPath, targetPath.replace('$', '$$$$')) // escapes '$' with '$$' - isWritable(target, function (writable) { - if (writable) { - copyFile(file, target) - } else { - if (overwrite) { - rmFile(target, function () { - copyFile(file, target) - }) - } else if (errorOnExist) { - onError(new Error(target + ' already exists')) - } else { - doneOne() - } - } - }) - } - - function copyFile (file, target) { - var readStream = fs.createReadStream(file.name) - var writeStream = fs.createWriteStream(target, { mode: file.mode }) - - readStream.on('error', onError) - writeStream.on('error', onError) - - if (transform) { - transform(readStream, writeStream, file) - } else { - writeStream.on('open', function () { - readStream.pipe(writeStream) - }) - } - - writeStream.once('close', function () { - fs.chmod(target, file.mode, function (err) { - if (err) return onError(err) - if (preserveTimestamps) { - utimes.utimesMillis(target, file.atime, file.mtime, function (err) { - if (err) return onError(err) - return doneOne() - }) - } else { - doneOne() - } - }) - }) - } - - function rmFile (file, done) { - fs.unlink(file, function (err) { - if (err) return onError(err) - return done() - }) - } - - function onDir (dir) { - var target = dir.name.replace(currentPath, targetPath.replace('$', '$$$$')) // escapes '$' with '$$' - isWritable(target, function (writable) { - if (writable) { - return mkDir(dir, target) - } - copyDir(dir.name) - }) - } - - function mkDir (dir, target) { - fs.mkdir(target, dir.mode, function (err) { - if (err) return onError(err) - // despite setting mode in fs.mkdir, doesn't seem to work - // so we set it here. - fs.chmod(target, dir.mode, function (err) { - if (err) return onError(err) - copyDir(dir.name) - }) - }) - } - - function copyDir (dir) { - fs.readdir(dir, function (err, items) { - if (err) return onError(err) - items.forEach(function (item) { - startCopy(path.join(dir, item)) - }) - return doneOne() - }) - } - - function onLink (link) { - var target = link.replace(currentPath, targetPath) - fs.readlink(link, function (err, resolvedPath) { - if (err) return onError(err) - checkLink(resolvedPath, target) - }) - } - - function checkLink (resolvedPath, target) { - if (dereference) { - resolvedPath = path.resolve(basePath, resolvedPath) - } - isWritable(target, function (writable) { - if (writable) { - return makeLink(resolvedPath, target) - } - fs.readlink(target, function (err, targetDest) { - if (err) return onError(err) - - if (dereference) { - targetDest = path.resolve(basePath, targetDest) - } - if (targetDest === resolvedPath) { - return doneOne() - } - return rmFile(target, function () { - makeLink(resolvedPath, target) - }) - }) - }) - } - - function makeLink (linkPath, target) { - fs.symlink(linkPath, target, function (err) { - if (err) return onError(err) - return doneOne() - }) - } - - function isWritable (path, done) { - fs.lstat(path, function (err) { - if (err) { - if (err.code === 'ENOENT') return done(true) - return done(false) - } - return done(false) - }) - } - - function onError (err) { - // ensure callback is defined & called only once: - if (!errored && callback !== undefined) { - errored = true - return callback(err) - } - } - - function doneOne (skipped) { - if (!skipped) running-- - finished++ - if ((started === finished) && (running === 0)) { - if (callback !== undefined) { - return callback(null) - } - } - } -} - -module.exports = ncp diff --git a/lib/move/index.js b/lib/move/index.js index eeeb30fe..a7181351 100644 --- a/lib/move/index.js +++ b/lib/move/index.js @@ -8,7 +8,7 @@ const u = require('universalify').fromCallback const fs = require('graceful-fs') -const ncp = require('../copy/ncp') +const copy = require('../copy/copy') const path = require('path') const remove = require('../remove').remove const mkdirp = require('../mkdirs').mkdirs @@ -133,14 +133,14 @@ function moveDirAcrossDevice (src, dest, overwrite, callback) { if (overwrite) { remove(dest, err => { if (err) return callback(err) - startNcp() + startCopy() }) } else { - startNcp() + startCopy() } - function startNcp () { - ncp(src, dest, options, err => { + function startCopy () { + copy(src, dest, options, err => { if (err) return callback(err) remove(src, callback) }) From 0bd52793a2e15b593ce3f19167d2550e822665a3 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Mon, 30 Oct 2017 23:59:22 -0700 Subject: [PATCH 02/13] Add native fs.copyFile to copy() --- lib/copy/copy.js | 59 ++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 1aa53228..32463b6e 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -75,7 +75,7 @@ function onFile (srcStat, src, dest, opts, cb) { checkDest(dest, (err, resolvedPath) => { if (err) return cb(err) if (resolvedPath === notExist) { - return cpFile(srcStat, src, dest, opts, cb) + return copyFile(srcStat, src, dest, opts, cb) } else if (resolvedPath === existsReg) { return mayCopyFile(srcStat, src, dest, opts, cb) } else { @@ -89,30 +89,41 @@ function mayCopyFile (srcStat, src, dest, opts, cb) { if (opts.overwrite) { fs.unlink(dest, err => { if (err) return cb(err) - return cpFile(srcStat, src, dest, opts, cb) + return copyFile(srcStat, src, dest, opts, cb) }) } else if (opts.errorOnExist) { return cb(new Error(`'${dest}' already exists`)) } else return cb() } -function cpFile (srcStat, src, dest, opts, cb) { +function copyFile (srcStat, src, dest, opts, cb) { + if (typeof fs.copyFile === 'function') { + return fs.copyFile(src, dest, err => { + if (err) return cb(err) + return handleDestModeAndTimestamps(srcStat, dest, opts, cb) + }) + } + return copyFileFallback(srcStat, src, dest, opts, cb) +} + +function copyFileFallback (srcStat, src, dest, opts, cb) { const rs = fs.createReadStream(src) const ws = fs.createWriteStream(dest, { mode: srcStat.mode }) rs.on('error', err => cb(err)) ws.on('error', err => cb(err)) - ws.on('open', () => { - rs.pipe(ws) - }).once('close', () => { - fs.chmod(dest, srcStat.mode, err => { - if (err) return cb(err) - if (opts.preserveTimestamps) { - return utimes(dest, srcStat.atime, srcStat.mtime, cb) - } - return cb() - }) + ws.on('open', () => rs.pipe(ws)) + .once('close', () => handleDestModeAndTimestamps(srcStat, dest, opts, cb)) +} + +function handleDestModeAndTimestamps (srcStat, dest, opts, cb) { + fs.chmod(dest, srcStat.mode, err => { + if (err) return cb(err) + if (opts.preserveTimestamps) { + return utimes(dest, srcStat.atime, srcStat.mtime, cb) + } + return cb() }) } @@ -131,7 +142,7 @@ function onDir (srcStat, src, dest, opts, cb) { return mayCopyDir(src, dest, opts, cb) } else { if (src === resolvedPath) return cb() - return cpDir(src, dest, opts, cb) + return copyDir(src, dest, opts, cb) } }) } @@ -142,7 +153,7 @@ function mayCopyDir (src, dest, opts, cb) { if (!st.isDirectory()) { return cb(new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`)) } - return cpDir(src, dest, opts, cb) + return copyDir(src, dest, opts, cb) }) } @@ -151,24 +162,24 @@ function mkDirAndCopy (srcStat, src, dest, opts, cb) { if (err) return cb(err) fs.chmod(dest, srcStat.mode, err => { if (err) return cb(err) - return cpDir(src, dest, opts, cb) + return copyDir(src, dest, opts, cb) }) }) } -function cpDir (src, dest, opts, cb) { +function copyDir (src, dest, opts, cb) { fs.readdir(src, (err, items) => { if (err) return cb(err) - return cpDirItems(items, src, dest, opts, cb) + return copyDirItems(items, src, dest, opts, cb) }) } -function cpDirItems (items, src, dest, opts, cb) { +function copyDirItems (items, src, dest, opts, cb) { const item = items.pop() if (!item) return cb() startCopy(path.join(src, item), path.join(dest, item), opts, err => { if (err) return cb(err) - return cpDirItems(items, src, dest, opts, cb) + return copyDirItems(items, src, dest, opts, cb) }) } @@ -201,27 +212,27 @@ function onLink (src, dest, opts, cb) { if (st.isDirectory() && isSrcSubdir(resolvedDestPath, resolvedSrcPath)) { return cb(new Error(`Cannot overwrite '${resolvedDestPath}' with '${resolvedSrcPath}'.`)) } - return cpLink(resolvedSrcPath, dest, cb) + return copyLink(resolvedSrcPath, dest, cb) }) } }) }) } -function cpLink (resolvedSrcPath, dest, cb) { +function copyLink (resolvedSrcPath, dest, cb) { fs.unlink(dest, err => { if (err) return cb(err) return fs.symlink(resolvedSrcPath, dest, cb) }) } -// check dest to see if it exists and/or is a symlink +// check if dest exists and/or 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 throws UNKNOWN error. + // 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) From 1abc2a3fb6271e3b33de846a186330fe951accb8 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Wed, 1 Nov 2017 00:20:06 -0700 Subject: [PATCH 03/13] Use native fs.copyFileSync in supported envs, add utimesMillisSync function to util/utimes.js --- .../__tests__/copy-sync-preserve-time.test.js | 6 +++--- lib/copy-sync/copy-file-sync.js | 17 +++++++++++++---- lib/copy/copy.js | 6 +++--- lib/util/utimes.js | 9 ++++++++- 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js b/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js index 6a4ad9f2..3f9f60ce 100644 --- a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js +++ b/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js @@ -21,11 +21,11 @@ describeIf64('copySync', () => { require(process.cwd()).emptyDir(TEST_DIR, done) }) - describe('> modification option', () => { + describe('> preserveTimestamps option', () => { const SRC_FIXTURES_DIR = path.join(__dirname, './fixtures') const FILES = ['a-file', path.join('a-folder', 'another-file'), path.join('a-folder', 'another-folder', 'file3')] - describe('> when modified option is turned off', () => { + describe('> when preserveTimestamps option is false', () => { it('should have different timestamps on copy', () => { const from = path.join(SRC_FIXTURES_DIR) copySync(from, TEST_DIR, {preserveTimestamps: false}) @@ -33,7 +33,7 @@ describeIf64('copySync', () => { }) }) - describe('> when modified option is turned on', () => { + describe('> when preserveTimestamps option is true', () => { it('should have the same timestamps on copy', () => { const from = path.join(SRC_FIXTURES_DIR) copySync(from, TEST_DIR, {preserveTimestamps: true}) diff --git a/lib/copy-sync/copy-file-sync.js b/lib/copy-sync/copy-file-sync.js index 102a6be6..452ca6cc 100644 --- a/lib/copy-sync/copy-file-sync.js +++ b/lib/copy-sync/copy-file-sync.js @@ -1,6 +1,7 @@ 'use strict' const fs = require('graceful-fs') +const utimesSync = require('../util/utimes.js').utimesMillisSync const BUF_LENGTH = 64 * 1024 const _buff = require('../util/buffer')(BUF_LENGTH) @@ -18,6 +19,17 @@ function copyFileSync (srcFile, destFile, options) { } else return } + if (typeof fs.copyFileSync === 'function') { + fs.copyFileSync(srcFile, destFile) + const st = fs.lstatSync(srcFile) + fs.chmodSync(destFile, st.mode) + if (preserveTimestamps) utimesSync(destFile, st.atime, st.mtime) + return undefined + } + return copyFileSyncFallback(srcFile, destFile, preserveTimestamps) +} + +function copyFileSyncFallback (srcFile, destFile, preserveTimestamps) { const fdr = fs.openSync(srcFile, 'r') const stat = fs.fstatSync(fdr) const fdw = fs.openSync(destFile, 'w', stat.mode) @@ -30,10 +42,7 @@ function copyFileSync (srcFile, destFile, options) { pos += bytesRead } - if (preserveTimestamps) { - fs.futimesSync(fdw, stat.atime, stat.mtime) - } - + if (preserveTimestamps) fs.futimesSync(fdw, stat.atime, stat.mtime) fs.closeSync(fdr) fs.closeSync(fdw) } diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 32463b6e..5288dd1c 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -100,7 +100,7 @@ function copyFile (srcStat, src, dest, opts, cb) { if (typeof fs.copyFile === 'function') { return fs.copyFile(src, dest, err => { if (err) return cb(err) - return handleDestModeAndTimestamps(srcStat, dest, opts, cb) + return setDestModeAndTimestamps(srcStat, dest, opts, cb) }) } return copyFileFallback(srcStat, src, dest, opts, cb) @@ -114,10 +114,10 @@ function copyFileFallback (srcStat, src, dest, opts, cb) { ws.on('error', err => cb(err)) ws.on('open', () => rs.pipe(ws)) - .once('close', () => handleDestModeAndTimestamps(srcStat, dest, opts, cb)) + .once('close', () => setDestModeAndTimestamps(srcStat, dest, opts, cb)) } -function handleDestModeAndTimestamps (srcStat, dest, opts, cb) { +function setDestModeAndTimestamps (srcStat, dest, opts, cb) { fs.chmod(dest, srcStat.mode, err => { if (err) return cb(err) if (opts.preserveTimestamps) { diff --git a/lib/util/utimes.js b/lib/util/utimes.js index 4c320993..8916a1b8 100644 --- a/lib/util/utimes.js +++ b/lib/util/utimes.js @@ -64,9 +64,16 @@ function utimesMillis (path, atime, mtime, callback) { }) } +function utimesMillisSync (path, atime, mtime) { + const fd = fs.openSync(path, 'r+') + fs.futimesSync(fd, atime, mtime) + return fs.closeSync(fd) +} + module.exports = { hasMillisRes, hasMillisResSync, timeRemoveMillis, - utimesMillis + utimesMillis, + utimesMillisSync } From 603b8bb661b0bf75e17c2a749b93a14ef4ea4d77 Mon Sep 17 00:00:00 2001 From: Ryan Zimmerman Date: Thu, 9 Nov 2017 09:46:04 -0500 Subject: [PATCH 04/13] BREAKING: Don't allow copy()/copySync()'s filter option to be a Regex (#512) This was deprecated previously, and is now removed. --- docs/copy-sync.md | 2 +- docs/copy.md | 2 +- lib/copy-sync/copy-sync.js | 12 ++++-------- lib/copy/copy.js | 9 +-------- 4 files changed, 7 insertions(+), 18 deletions(-) diff --git a/docs/copy-sync.md b/docs/copy-sync.md index 8e61c2b6..0a33e474 100644 --- a/docs/copy-sync.md +++ b/docs/copy-sync.md @@ -9,7 +9,7 @@ Copy a file or directory. The directory can have contents. Like `cp -r`. - `errorOnExist` ``: when `overwrite` is `false` and the destination exists, throw an error. Default is `false`. - `dereference` ``: dereference symlinks, default is `false`. - `preserveTimestamps` ``: will set last modification and access times to the ones of the original source files, default is `false`. - - `filter` ``: Function to filter copied files. Return `true` to include, `false` to exclude. This can also be a RegExp, however this is deprecated (See [issue #239](https://github.com/jprichardson/node-fs-extra/issues/239) for background). + - `filter` ``: Function to filter copied files. Return `true` to include, `false` to exclude. ## Example: diff --git a/docs/copy.md b/docs/copy.md index 84407261..df834486 100644 --- a/docs/copy.md +++ b/docs/copy.md @@ -9,7 +9,7 @@ Copy a file or directory. The directory can have contents. Like `cp -r`. - `errorOnExist` ``: when `overwrite` is `false` and the destination exists, throw an error. Default is `false`. - `dereference` ``: dereference symlinks, default is `false`. - `preserveTimestamps` ``: will set last modification and access times to the ones of the original source files, default is `false`. - - `filter` ``: Function to filter copied files. Return `true` to include, `false` to exclude. This can also be a RegExp, however this is deprecated (See [issue #239](https://github.com/jprichardson/node-fs-extra/issues/239) for background). + - `filter` ``: Function to filter copied files. Return `true` to include, `false` to exclude. - `callback` `` ## Example: diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 9d5639c3..22f44fb9 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -31,21 +31,17 @@ function copySync (src, dest, options) { const stats = (options.recursive && !options.dereference) ? fs.lstatSync(src) : fs.statSync(src) const destFolder = path.dirname(dest) const destFolderExists = fs.existsSync(destFolder) - let performCopy = false - if (options.filter instanceof RegExp) { - console.warn('Warning: fs-extra: Passing a RegExp filter is deprecated, use a function') - performCopy = options.filter.test(src) - } else if (typeof options.filter === 'function') performCopy = options.filter(src, dest) + if (!options.filter(src, dest)) return - if (stats.isFile() && performCopy) { + if (stats.isFile()) { if (!destFolderExists) mkdir.mkdirsSync(destFolder) copyFileSync(src, dest, { overwrite: options.overwrite, errorOnExist: options.errorOnExist, preserveTimestamps: options.preserveTimestamps }) - } else if (stats.isDirectory() && performCopy) { + } else if (stats.isDirectory()) { if (!fs.existsSync(dest)) mkdir.mkdirsSync(dest) const contents = fs.readdirSync(src) contents.forEach(content => { @@ -53,7 +49,7 @@ function copySync (src, dest, options) { opts.recursive = true copySync(path.join(src, content), path.join(dest, content), opts) }) - } else if (options.recursive && stats.isSymbolicLink() && performCopy) { + } else if (options.recursive && stats.isSymbolicLink()) { const srcPath = fs.readlinkSync(src) fs.symlinkSync(srcPath, dest) } diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 5288dd1c..b0e06cf8 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -47,14 +47,7 @@ function copy (src, dest, opts, cb) { } function startCopy (src, dest, opts, cb) { - if (opts.filter) { - if (opts.filter instanceof RegExp) { - console.warn('Warning: fs-extra: Passing a RegExp filter is deprecated, use a function') - if (!opts.filter.test(src)) return cb() - } else if (typeof opts.filter === 'function') { - if (!opts.filter(src, dest)) return cb() - } - } + if (opts.filter && !opts.filter(src, dest)) return cb() return getStats(src, dest, opts, cb) } From c59e0a6e2e22f7c72ff970858cfe7f5201d24a20 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 9 Nov 2017 11:09:32 -0800 Subject: [PATCH 05/13] Fix streams initialization in copyFileFallback (introduced by @unkelpehr) --- lib/copy/copy.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/copy/copy.js b/lib/copy/copy.js index b0e06cf8..76aaa23c 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -101,13 +101,13 @@ function copyFile (srcStat, src, dest, opts, cb) { function copyFileFallback (srcStat, src, dest, opts, cb) { const rs = fs.createReadStream(src) - const ws = fs.createWriteStream(dest, { mode: srcStat.mode }) - rs.on('error', err => cb(err)) - ws.on('error', err => cb(err)) - - ws.on('open', () => rs.pipe(ws)) - .once('close', () => setDestModeAndTimestamps(srcStat, dest, opts, cb)) + .once('open', () => { + const ws = fs.createWriteStream(dest, { mode: srcStat.mode }) + ws.on('error', err => cb(err)) + .on('open', () => rs.pipe(ws)) + .once('close', () => setDestModeAndTimestamps(srcStat, dest, opts, cb)) + }) } function setDestModeAndTimestamps (srcStat, dest, opts, cb) { @@ -198,8 +198,8 @@ function onLink (src, dest, opts, cb) { if (resolvedDestPath === resolvedSrcPath) return cb() // prevent copy if src is a subdir of dest since unlinking - // dest in this case results in removing src contents - // and therefore a broken symlink will be created. + // 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(resolvedDestPath, resolvedSrcPath)) { From 199ec9f2c28bf14e7da62d131ced80f1255d5f79 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 9 Nov 2017 13:22:59 -0800 Subject: [PATCH 06/13] Apply filter before creating parent dir in copy --- .../copy-prevent-copying-identical.test.js | 8 +-- .../copy-prevent-copying-into-itself.test.js | 66 +++++++++---------- lib/copy/__tests__/copy.test.js | 19 +++++- lib/copy/copy.js | 2 + 4 files changed, 56 insertions(+), 39 deletions(-) diff --git a/lib/copy/__tests__/copy-prevent-copying-identical.test.js b/lib/copy/__tests__/copy-prevent-copying-identical.test.js index 64357105..615645b5 100644 --- a/lib/copy/__tests__/copy-prevent-copying-identical.test.js +++ b/lib/copy/__tests__/copy-prevent-copying-identical.test.js @@ -8,21 +8,21 @@ const klawSync = require('klaw-sync') /* global beforeEach, afterEach, describe, it */ -describe('+ copySync() - prevent copying identical files and dirs', () => { +describe('+ copy() - prevent copying identical files and dirs', () => { let TEST_DIR = '' let src = '' let dest = '' beforeEach(done => { - TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-prevent-copying-identical') + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-prevent-copying-identical') fs.emptyDir(TEST_DIR, done) }) afterEach(done => fs.remove(TEST_DIR, done)) it('should return an error if src and dest are the same', done => { - const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync') - const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync') + const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy') + const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy') fs.copy(fileSrc, fileDest, err => { assert.equal(err.message, 'Source and destination must not be the same.') 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 d759a1a5..9fe0ef0e 100644 --- a/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js +++ b/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js @@ -21,43 +21,11 @@ const dat1 = 'file1' const dat2 = 'file2' const dat3 = 'file3' -function testSuccess (src, dest, done) { - const srclen = klawSync(src).length - assert(srclen > 2) - fs.copy(src, dest, err => { - assert.ifError(err) - - const destlen = klawSync(dest).length - - assert.strictEqual(destlen, srclen) - - FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied')) - - const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8') - const o1 = fs.readFileSync(path.join(dest, FILES[1]), 'utf8') - const o2 = fs.readFileSync(path.join(dest, FILES[2]), 'utf8') - const o3 = fs.readFileSync(path.join(dest, FILES[3]), 'utf8') - - assert.strictEqual(o0, dat0, 'file contents matched') - assert.strictEqual(o1, dat1, 'file contents matched') - assert.strictEqual(o2, dat2, 'file contents matched') - assert.strictEqual(o3, dat3, 'file contents matched') - done() - }) -} - -function testError (src, dest, done) { - fs.copy(src, dest, err => { - assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) - done() - }) -} - describe('+ copy() - prevent copying into itself', () => { let TEST_DIR, src beforeEach(done => { - TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-prevent-copying-into-itself-4') + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-prevent-copying-into-itself') src = path.join(TEST_DIR, 'src') fs.mkdirpSync(src) @@ -370,3 +338,35 @@ describe('+ copy() - prevent copying into itself', () => { }) }) }) + +function testSuccess (src, dest, done) { + const srclen = klawSync(src).length + assert(srclen > 2) + fs.copy(src, dest, err => { + assert.ifError(err) + + const destlen = klawSync(dest).length + + assert.strictEqual(destlen, srclen) + + FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied')) + + const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8') + const o1 = fs.readFileSync(path.join(dest, FILES[1]), 'utf8') + const o2 = fs.readFileSync(path.join(dest, FILES[2]), 'utf8') + const o3 = fs.readFileSync(path.join(dest, FILES[3]), 'utf8') + + assert.strictEqual(o0, dat0, 'file contents matched') + assert.strictEqual(o1, dat1, 'file contents matched') + assert.strictEqual(o2, dat2, 'file contents matched') + assert.strictEqual(o3, dat3, 'file contents matched') + done() + }) +} + +function testError (src, dest, done) { + fs.copy(src, dest, err => { + assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + done() + }) +} diff --git a/lib/copy/__tests__/copy.test.js b/lib/copy/__tests__/copy.test.js index 4787bfd8..d8e38f63 100644 --- a/lib/copy/__tests__/copy.test.js +++ b/lib/copy/__tests__/copy.test.js @@ -208,6 +208,21 @@ describe('fs-extra', () => { }) describe('> when filter is used', () => { + it('should do nothing if filter fails', done => { + const srcDir = path.join(TEST_DIR, 'src') + const srcFile = path.join(srcDir, 'srcfile.css') + fse.outputFileSync(srcFile, 'src contents') + const destDir = path.join(TEST_DIR, 'dest') + const destFile = path.join(destDir, 'destfile.css') + const filter = s => path.extname(s) !== '.css' && !fs.statSync(s).isDirectory() + + fse.copy(srcFile, destFile, filter, err => { + assert.ifError(err) + assert(!fs.existsSync(destDir)) + done() + }) + }) + it('should only copy files allowed by filter fn', done => { const srcFile1 = path.join(TEST_DIR, '1.css') fs.writeFileSync(srcFile1, '') @@ -234,7 +249,7 @@ describe('fs-extra', () => { }) }) - it('should should apply filter recursively', done => { + it('should apply filter recursively', done => { const FILES = 2 // Don't match anything that ends with a digit higher than 0: const filter = s => /(0|\D)$/i.test(s) @@ -280,7 +295,7 @@ describe('fs-extra', () => { }) }) - it('should apply the filter to directory names', done => { + it('should apply filter to directory names', done => { const IGNORE = 'ignore' const filter = p => !~p.indexOf(IGNORE) diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 76aaa23c..f270c1d7 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -35,6 +35,8 @@ function copy (src, dest, opts, cb) { // don't allow src and dest to be the same if (src === dest) return cb(new Error('Source and destination must not be the same.')) + if (opts.filter && !opts.filter(src, dest)) return cb() + const destParent = path.dirname(dest) pathExists(destParent, (err, dirExists) => { if (err) return cb(err) From fda912a38178b0c3c53557b6712d90eea2cabb36 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Fri, 10 Nov 2017 17:56:58 -0800 Subject: [PATCH 07/13] Rewrite copySync, add more tests, remove fixtures folder --- lib/copy-sync/__tests__/copy-sync-dir.test.js | 27 ++ .../__tests__/copy-sync-preserve-time.test.js | 84 ++-- ...opy-sync-prevent-copying-identical.test.js | 181 +++++++++ ...y-sync-prevent-copying-into-itself.test.js | 372 ++++++++++++++++++ lib/copy-sync/__tests__/fixtures/a-file | 1 - .../__tests__/fixtures/a-folder/another-file | 1 - .../fixtures/a-folder/another-folder/file3 | 1 - lib/copy-sync/copy-file-sync.js | 50 --- lib/copy-sync/copy-sync.js | 237 +++++++++-- 9 files changed, 822 insertions(+), 132 deletions(-) create mode 100644 lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js create mode 100644 lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js delete mode 100644 lib/copy-sync/__tests__/fixtures/a-file delete mode 100644 lib/copy-sync/__tests__/fixtures/a-folder/another-file delete mode 100644 lib/copy-sync/__tests__/fixtures/a-folder/another-folder/file3 delete mode 100644 lib/copy-sync/copy-file-sync.js diff --git a/lib/copy-sync/__tests__/copy-sync-dir.test.js b/lib/copy-sync/__tests__/copy-sync-dir.test.js index 82c360e9..e31360b2 100644 --- a/lib/copy-sync/__tests__/copy-sync-dir.test.js +++ b/lib/copy-sync/__tests__/copy-sync-dir.test.js @@ -21,6 +21,21 @@ describe('+ copySync()', () => { }) describe('> when the source is a directory', () => { + describe('> when dest exists and is a file', () => { + it('should throw error', () => { + const src = path.join(TEST_DIR, 'src') + const dest = path.join(TEST_DIR, 'file.txt') + fs.mkdirSync(src) + fs.ensureFileSync(dest) + + try { + fs.copySync(src, dest) + } catch (err) { + assert.strictEqual(err.message, `Cannot overwrite non-directory '${dest}' with directory '${src}'.`) + } + }) + }) + it('should copy the directory synchronously', () => { const FILES = 2 @@ -89,6 +104,18 @@ describe('+ copySync()', () => { }) describe('> when filter is used', () => { + it('should do nothing if filter fails', () => { + const srcDir = path.join(TEST_DIR, 'src') + const srcFile = path.join(srcDir, 'srcfile.css') + fs.outputFileSync(srcFile, 'src contents') + const destDir = path.join(TEST_DIR, 'dest') + const destFile = path.join(destDir, 'destfile.css') + const filter = s => path.extname(s) !== '.css' && !fs.statSync(s).isDirectory() + + fs.copySync(srcFile, destFile, filter) + assert(!fs.existsSync(destDir)) + }) + it('should should apply filter recursively', () => { const FILES = 2 // Don't match anything that ends with a digit higher than 0: diff --git a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js b/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js index 3f9f60ce..c9d21ae4 100644 --- a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js +++ b/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js @@ -1,66 +1,76 @@ 'use strict' -const fs = require('fs') +const fs = require(process.cwd()) const os = require('os') const path = require('path') const utimes = require('../../util/utimes') const assert = require('assert') -const copySync = require('../copy-sync') -/* global beforeEach, describe, it */ +/* global beforeEach, afterEach, describe, it */ if (process.arch === 'ia32') console.warn('32 bit arch; skipping copySync timestamp tests') const describeIf64 = process.arch === 'ia32' ? describe.skip : describe -describeIf64('copySync', () => { - let TEST_DIR +describeIf64('copySync() - preserveTimestamps option', () => { + let TEST_DIR, src, dest beforeEach(done => { TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-preserve-time') - require(process.cwd()).emptyDir(TEST_DIR, done) + fs.emptyDir(TEST_DIR, done) }) - describe('> preserveTimestamps option', () => { - const SRC_FIXTURES_DIR = path.join(__dirname, './fixtures') - const FILES = ['a-file', path.join('a-folder', 'another-file'), path.join('a-folder', 'another-folder', 'file3')] + afterEach(done => fs.remove(TEST_DIR, done)) - describe('> when preserveTimestamps option is false', () => { - it('should have different timestamps on copy', () => { - const from = path.join(SRC_FIXTURES_DIR) - copySync(from, TEST_DIR, {preserveTimestamps: false}) + const FILES = ['a-file', path.join('a-folder', 'another-file'), path.join('a-folder', 'another-folder', 'file3')] + + describe('> when preserveTimestamps option is false', () => { + it('should have different timestamps on copy', done => { + src = path.join(TEST_DIR, 'src') + dest = path.join(TEST_DIR, 'dest') + FILES.forEach(f => fs.ensureFileSync(path.join(src, f))) + + setTimeout(() => { + fs.copySync(src, dest, {preserveTimestamps: false}) FILES.forEach(testFile({preserveTimestamps: false})) - }) + done() + }, 100) }) + }) - describe('> when preserveTimestamps option is true', () => { - it('should have the same timestamps on copy', () => { - const from = path.join(SRC_FIXTURES_DIR) - copySync(from, TEST_DIR, {preserveTimestamps: true}) + describe('> when preserveTimestamps option is true', () => { + it('should have the same timestamps on copy', done => { + src = path.join(TEST_DIR, 'src') + dest = path.join(TEST_DIR, 'dest') + FILES.forEach(f => fs.ensureFileSync(path.join(src, f))) + + setTimeout(() => { + fs.copySync(src, dest, {preserveTimestamps: true}) FILES.forEach(testFile({preserveTimestamps: true})) - }) + done() + }, 100) }) + }) - function testFile (options) { - return function (file) { - const a = path.join(SRC_FIXTURES_DIR, file) - const b = path.join(TEST_DIR, file) - const fromStat = fs.statSync(a) - const toStat = fs.statSync(b) - if (options.preserveTimestamps) { - // https://github.com/nodejs/io.js/issues/2069 - if (process.platform !== 'win32') { - assert.strictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime()) - assert.strictEqual(toStat.atime.getTime(), fromStat.atime.getTime()) - } else { - assert.strictEqual(toStat.mtime.getTime(), utimes.timeRemoveMillis(fromStat.mtime.getTime())) - assert.strictEqual(toStat.atime.getTime(), utimes.timeRemoveMillis(fromStat.atime.getTime())) - } + function testFile (options) { + return function (file) { + const a = path.join(src, file) + const b = path.join(dest, file) + const fromStat = fs.statSync(a) + const toStat = fs.statSync(b) + if (options.preserveTimestamps) { + // https://github.com/nodejs/io.js/issues/2069 + if (process.platform !== 'win32') { + assert.strictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime()) + assert.strictEqual(toStat.atime.getTime(), fromStat.atime.getTime()) } else { - assert.notEqual(toStat.mtime.getTime(), fromStat.mtime.getTime()) - // the access time might actually be the same, so check only modification time + assert.strictEqual(toStat.mtime.getTime(), utimes.timeRemoveMillis(fromStat.mtime.getTime())) + assert.strictEqual(toStat.atime.getTime(), utimes.timeRemoveMillis(fromStat.atime.getTime())) } + } else { + assert.notEqual(toStat.mtime.getTime(), fromStat.mtime.getTime()) + // the access time might actually be the same, so check only modification time } } - }) + } }) 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 new file mode 100644 index 00000000..1e0f5980 --- /dev/null +++ b/lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js @@ -0,0 +1,181 @@ +'use strict' + +const assert = require('assert') +const os = require('os') +const path = require('path') +const fs = require(process.cwd()) +const klawSync = require('klaw-sync') + +/* global beforeEach, afterEach, describe, it */ + +describe('+ copySync() - prevent copying identical files and dirs', () => { + let TEST_DIR = '' + let src = '' + let dest = '' + + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-prevent-copying-identical') + fs.emptyDir(TEST_DIR, done) + }) + + afterEach(done => fs.remove(TEST_DIR, done)) + + 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') + try { + fs.copySync(fileSrc, fileDest) + } catch (err) { + assert.equal(err.message, 'Source and destination must not be the same.') + } + }) + + // src is directory: + // src is regular, dest is symlink + // src is symlink, dest is regular + // src is symlink, dest is symlink + + describe('> when the source is a directory', () => { + describe(`>> when src is regular and dest is a symlink that points to src`, () => { + it('should not copy and return', () => { + src = path.join(TEST_DIR, 'src') + fs.mkdirsSync(src) + const subdir = path.join(TEST_DIR, 'src', 'subdir') + fs.mkdirsSync(subdir) + fs.writeFileSync(path.join(subdir, 'file.txt'), 'some data') + + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const oldlen = klawSync(src).length + + fs.copySync(src, destLink) + + const newlen = klawSync(src).length + assert.strictEqual(newlen, oldlen) + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + }) + }) + + describe(`>> when src is a symlink that points to a regular dest`, () => { + it('should throw error', () => { + dest = path.join(TEST_DIR, 'dest') + fs.mkdirsSync(dest) + const subdir = path.join(TEST_DIR, 'dest', 'subdir') + fs.mkdirsSync(subdir) + fs.writeFileSync(path.join(subdir, 'file.txt'), 'some data') + + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(dest, srcLink, 'dir') + + const oldlen = klawSync(dest).length + + try { + fs.copySync(srcLink, dest) + } catch (err) { + assert(err) + } + + // assert nothing copied + const newlen = klawSync(dest).length + assert.strictEqual(newlen, oldlen) + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, dest) + }) + }) + + describe('>> when src and dest are symlinks that point to the exact same path', () => { + it('should not copy and return', () => { + src = path.join(TEST_DIR, 'src') + fs.mkdirsSync(src) + const srcLink = path.join(TEST_DIR, 'src_symlink') + fs.symlinkSync(src, srcLink, 'dir') + const destLink = path.join(TEST_DIR, 'dest_symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(srcLink).length + const destlenBefore = klawSync(destLink).length + + 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) + }) + }) + }) + + // src is file: + // src is regular, dest is symlink + // src is symlink, dest is regular + // src is symlink, dest is symlink + + describe('> when the source is a file', () => { + describe(`>> when src is regular and dest is a symlink that points to src`, () => { + it('should not copy and return', () => { + src = path.join(TEST_DIR, 'src', 'somefile.txt') + fs.ensureFileSync(src) + fs.writeFileSync(src, 'some data') + + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'file') + + fs.copySync(src, destLink) + + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + assert(fs.readFileSync(link, 'utf8'), 'some data') + }) + }) + + 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') + + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(dest, srcLink, 'file') + + try { + fs.copySync(srcLink, dest) + } catch (err) { + assert.ok(err) + } + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, dest) + assert(fs.readFileSync(link, 'utf8'), 'some data') + }) + }) + + describe('>> when src and dest are symlinks that point to the exact same path', () => { + it('should not copy and return', () => { + src = path.join(TEST_DIR, 'src', 'srcfile.txt') + fs.ensureFileSync(src) + fs.writeFileSync(src, 'src data') + + const srcLink = path.join(TEST_DIR, 'src_symlink') + fs.symlinkSync(src, srcLink, 'file') + + const destLink = path.join(TEST_DIR, 'dest_symlink') + fs.symlinkSync(src, destLink, 'file') + + fs.copySync(srcLink, destLink) + + const srcln = fs.readlinkSync(srcLink) + assert.strictEqual(srcln, src) + const destln = fs.readlinkSync(destLink) + assert.strictEqual(destln, src) + assert(fs.readFileSync(srcln, 'utf8'), 'src data') + assert(fs.readFileSync(destln, 'utf8'), 'src data') + }) + }) + }) +}) 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 new file mode 100644 index 00000000..fdac1892 --- /dev/null +++ b/lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js @@ -0,0 +1,372 @@ +'use strict' + +const assert = require('assert') +const os = require('os') +const path = require('path') +const fs = require(process.cwd()) +const klawSync = require('klaw-sync') + +/* global beforeEach, afterEach, describe, it */ + +// these files are used for all tests +const FILES = [ + 'file0.txt', + path.join('dir1', 'file1.txt'), + path.join('dir1', 'dir2', 'file2.txt'), + path.join('dir1', 'dir2', 'dir3', 'file3.txt') +] + +const dat0 = 'file0' +const dat1 = 'file1' +const dat2 = 'file2' +const dat3 = 'file3' + +describe('+ copySync() - prevent copying into itself', () => { + let TEST_DIR, src + + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-prevent-copying-into-itself') + src = path.join(TEST_DIR, 'src') + fs.mkdirpSync(src) + + fs.outputFileSync(path.join(src, FILES[0]), dat0) + fs.outputFileSync(path.join(src, FILES[1]), dat1) + fs.outputFileSync(path.join(src, FILES[2]), dat2) + fs.outputFileSync(path.join(src, FILES[3]), dat3) + done() + }) + + 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 => { + const srcFile = path.join(TEST_DIR, 'src', 'srcfile.txt') + const destFile = path.join(TEST_DIR, 'src', 'dest', 'destfile.txt') + fs.writeFileSync(srcFile, dat0) + + fs.copy(srcFile, destFile, err => { + assert.ifError(err) + + assert(fs.existsSync(destFile, 'file copied')) + const out = fs.readFileSync(destFile, 'utf8') + assert.strictEqual(out, dat0, 'file contents matched') + done() + }) + }) + }) + + // test for these cases: + // - src is directory, dest is directory + // - src is directory, dest is symlink + // - src is symlink, dest is directory + // - src is symlink, dest is symlink + + describe('> when source is a directory', () => { + describe('>> when dest is a directory', () => { + it(`should copy the directory successfully when dest is 'src_dest'`, done => { + const dest = path.join(TEST_DIR, 'src_dest') + return testSuccess(src, dest, done) + }) + it(`should copy the directory successfully when dest is 'src-dest'`, done => { + const dest = path.join(TEST_DIR, 'src-dest') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'dest_src'`, done => { + const dest = path.join(TEST_DIR, 'dest_src') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'src_dest/src'`, done => { + const dest = path.join(TEST_DIR, 'src_dest', 'src') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'src-dest/src'`, done => { + const dest = path.join(TEST_DIR, 'src-dest', 'src') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'dest_src/src'`, done => { + const dest = path.join(TEST_DIR, 'dest_src', 'src') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'src_src/dest'`, done => { + const dest = path.join(TEST_DIR, 'src_src', 'dest') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'src-src/dest'`, done => { + const dest = path.join(TEST_DIR, 'src-src', 'dest') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'srcsrc/dest'`, done => { + const dest = path.join(TEST_DIR, 'srcsrc', 'dest') + return testSuccess(src, dest, done) + }) + + it(`should copy the directory successfully when dest is 'dest/src'`, done => { + const dest = path.join(TEST_DIR, 'dest', 'src') + return testSuccess(src, dest, done) + }) + + it('should copy the directory successfully when dest is very nested that all its parents need to be created', done => { + const dest = path.join(TEST_DIR, 'dest', 'src', 'foo', 'bar', 'baz', 'qux', 'quux', 'waldo', + 'grault', 'garply', 'fred', 'plugh', 'thud', 'some', 'highly', 'deeply', + 'badly', 'nasty', 'crazy', 'mad', 'nested', 'dest') + return testSuccess(src, dest, done) + }) + + it(`should error when dest is 'src/dest'`, done => { + const dest = path.join(TEST_DIR, 'src', 'dest') + return testError(src, dest, done) + }) + + it(`should error when dest is 'src/src_dest'`, done => { + const dest = path.join(TEST_DIR, 'src', 'src_dest') + return testError(src, dest, done) + }) + + it(`should error when dest is 'src/dest_src'`, done => { + const dest = path.join(TEST_DIR, 'src', 'dest_src') + return testError(src, dest, done) + }) + + it(`should error when dest is 'src/dest/src'`, done => { + const dest = path.join(TEST_DIR, 'src', 'dest', 'src') + return testError(src, dest, done) + }) + }) + + describe('>> when dest is a symlink', () => { + it('should not copy and return 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.copy(src, destLink, err => { + assert.ifError(err) + + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + done() + }) + }) + + it('should copy the directory successfully when src is a subdir of resolved dest path', done => { + const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src') + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.copySync(src, srcInDest) // put some stuff in srcInDest + + const dest = path.join(TEST_DIR, 'dest') + fs.symlinkSync(dest, destLink, 'dir') + + const srclen = klawSync(srcInDest).length + const destlenBefore = klawSync(dest).length + + assert(srclen > 2) + fs.copy(srcInDest, destLink, err => { + assert.ifError(err) + + const destlenAfter = klawSync(dest).length + + // assert dest length is oldlen + length of stuff copied from src + assert.strictEqual(destlenAfter, destlenBefore + srclen, 'dest length should be equal to old length + copied legnth') + + FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied')) + + const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8') + const o1 = fs.readFileSync(path.join(dest, FILES[1]), 'utf8') + const o2 = fs.readFileSync(path.join(dest, FILES[2]), 'utf8') + const o3 = fs.readFileSync(path.join(dest, FILES[3]), 'utf8') + + assert.strictEqual(o0, dat0, 'files contents matched') + assert.strictEqual(o1, dat1, 'files contents matched') + assert.strictEqual(o2, dat2, 'files contents matched') + assert.strictEqual(o3, dat3, 'files contents matched') + done() + }) + }) + }) + }) + + describe('> when source is a symlink', () => { + describe('>> when dest is a directory', () => { + it('should error when resolved src path points to dest', done => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + + const dest = path.join(TEST_DIR, 'src') + + fs.copy(srcLink, dest, err => { + assert(err) + // assert source not affected + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, src) + done() + }) + }) + + it('should error when dest is a subdir of resolved src path', done => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + + const dest = path.join(TEST_DIR, 'src', 'some', 'nested', 'dest') + fs.mkdirsSync(dest) + + fs.copy(srcLink, dest, err => { + assert(err) + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, src) + done() + }) + }) + + it('should error when resolved src path is a subdir of dest', done => { + const dest = path.join(TEST_DIR, 'dest') + + const resolvedSrcPath = path.join(dest, 'contains', 'src') + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.copySync(src, resolvedSrcPath) + + // make symlink that points to a subdir in dest + fs.symlinkSync(resolvedSrcPath, srcLink, 'dir') + + fs.copy(srcLink, dest, err => { + assert(err) + done() + }) + }) + + it(`should copy the directory successfully when dest is 'src_src/dest'`, done => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + + const dest = path.join(TEST_DIR, 'src_src', 'dest') + testSuccess(srcLink, dest, () => { + const link = fs.readlinkSync(dest) + assert.strictEqual(link, src) + done() + }) + }) + + it(`should copy the directory successfully when dest is 'srcsrc/dest'`, done => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + + const dest = path.join(TEST_DIR, 'srcsrc', 'dest') + testSuccess(srcLink, dest, () => { + const link = fs.readlinkSync(dest) + assert.strictEqual(link, src) + done() + }) + }) + }) + + describe('>> when dest is a symlink', () => { + it('should silently return 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') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(srcLink).length + const destlenBefore = klawSync(destLink).length + assert(srclenBefore > 2) + assert(destlenBefore > 2) + + fs.copy(srcLink, destLink, err => { + assert.ifError(err) + + 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) + done() + }) + }) + + it('should error when resolved dest path is a subdir of 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') + 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) + const destln = fs.readlinkSync(destLink) + assert.strictEqual(destln, src) + 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.mkdirSync(dest) + + fs.symlinkSync(srcInDest, srcLink, 'dir') + fs.symlinkSync(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() + }) + }) + }) + }) +}) + +function testSuccess (src, dest, done) { + const srclen = klawSync(src).length + assert(srclen > 2) + fs.copy(src, dest, err => { + assert.ifError(err) + + const destlen = klawSync(dest).length + + assert.strictEqual(destlen, srclen) + + FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied')) + + const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8') + const o1 = fs.readFileSync(path.join(dest, FILES[1]), 'utf8') + const o2 = fs.readFileSync(path.join(dest, FILES[2]), 'utf8') + const o3 = fs.readFileSync(path.join(dest, FILES[3]), 'utf8') + + assert.strictEqual(o0, dat0, 'file contents matched') + assert.strictEqual(o1, dat1, 'file contents matched') + assert.strictEqual(o2, dat2, 'file contents matched') + assert.strictEqual(o3, dat3, 'file contents matched') + done() + }) +} + +function testError (src, dest, done) { + fs.copy(src, dest, err => { + assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + done() + }) +} diff --git a/lib/copy-sync/__tests__/fixtures/a-file b/lib/copy-sync/__tests__/fixtures/a-file deleted file mode 100644 index 94a709d0..00000000 --- a/lib/copy-sync/__tests__/fixtures/a-file +++ /dev/null @@ -1 +0,0 @@ -sonic the hedgehog diff --git a/lib/copy-sync/__tests__/fixtures/a-folder/another-file b/lib/copy-sync/__tests__/fixtures/a-folder/another-file deleted file mode 100644 index 31340c77..00000000 --- a/lib/copy-sync/__tests__/fixtures/a-folder/another-file +++ /dev/null @@ -1 +0,0 @@ -tails diff --git a/lib/copy-sync/__tests__/fixtures/a-folder/another-folder/file3 b/lib/copy-sync/__tests__/fixtures/a-folder/another-folder/file3 deleted file mode 100644 index 73a394da..00000000 --- a/lib/copy-sync/__tests__/fixtures/a-folder/another-folder/file3 +++ /dev/null @@ -1 +0,0 @@ -knuckles diff --git a/lib/copy-sync/copy-file-sync.js b/lib/copy-sync/copy-file-sync.js deleted file mode 100644 index 452ca6cc..00000000 --- a/lib/copy-sync/copy-file-sync.js +++ /dev/null @@ -1,50 +0,0 @@ -'use strict' - -const fs = require('graceful-fs') -const utimesSync = require('../util/utimes.js').utimesMillisSync - -const BUF_LENGTH = 64 * 1024 -const _buff = require('../util/buffer')(BUF_LENGTH) - -function copyFileSync (srcFile, destFile, options) { - const overwrite = options.overwrite - const errorOnExist = options.errorOnExist - const preserveTimestamps = options.preserveTimestamps - - if (fs.existsSync(destFile)) { - if (overwrite) { - fs.unlinkSync(destFile) - } else if (errorOnExist) { - throw new Error(`${destFile} already exists`) - } else return - } - - if (typeof fs.copyFileSync === 'function') { - fs.copyFileSync(srcFile, destFile) - const st = fs.lstatSync(srcFile) - fs.chmodSync(destFile, st.mode) - if (preserveTimestamps) utimesSync(destFile, st.atime, st.mtime) - return undefined - } - return copyFileSyncFallback(srcFile, destFile, preserveTimestamps) -} - -function copyFileSyncFallback (srcFile, destFile, preserveTimestamps) { - const fdr = fs.openSync(srcFile, 'r') - const stat = fs.fstatSync(fdr) - const fdw = fs.openSync(destFile, 'w', stat.mode) - let bytesRead = 1 - let pos = 0 - - while (bytesRead > 0) { - bytesRead = fs.readSync(fdr, _buff, 0, BUF_LENGTH, pos) - fs.writeSync(fdw, _buff, 0, bytesRead) - pos += bytesRead - } - - if (preserveTimestamps) fs.futimesSync(fdw, stat.atime, stat.mtime) - fs.closeSync(fdr) - fs.closeSync(fdw) -} - -module.exports = copyFileSync diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 22f44fb9..81adc397 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -2,57 +2,210 @@ const fs = require('graceful-fs') const path = require('path') -const copyFileSync = require('./copy-file-sync') -const mkdir = require('../mkdirs') +const mkdirpSync = require('../mkdirs').mkdirsSync +const utimesSync = require('../util/utimes.js').utimesMillisSync -function copySync (src, dest, options) { - if (typeof options === 'function' || options instanceof RegExp) { - options = {filter: options} - } - - options = options || {} - options.recursive = !!options.recursive +const notExist = Symbol('notExist') +const existsReg = Symbol('existsReg') - // default to true for now - options.clobber = 'clobber' in options ? !!options.clobber : true - // overwrite falls back to clobber - options.overwrite = 'overwrite' in options ? !!options.overwrite : options.clobber - options.dereference = 'dereference' in options ? !!options.dereference : false - options.preserveTimestamps = 'preserveTimestamps' in options ? !!options.preserveTimestamps : false +function copySync (src, dest, opts) { + if (typeof opts === 'function') { + opts = {filter: opts} + } - options.filter = options.filter || function () { return true } + opts = opts || {} + opts.clobber = 'clobber' in opts ? !!opts.clobber : true // default to true for now + opts.overwrite = 'overwrite' in opts ? !!opts.overwrite : opts.clobber // overwrite falls back to clobber - // Warn about using preserveTimestamps on 32-bit node: - if (options.preserveTimestamps && process.arch === 'ia32') { + // Warn about using preserveTimestamps on 32-bit node + if (opts.preserveTimestamps && process.arch === 'ia32') { console.warn(`fs-extra: Using the preserveTimestamps option in 32-bit node is not recommended;\n see https://github.com/jprichardson/node-fs-extra/issues/269`) } - const stats = (options.recursive && !options.dereference) ? fs.lstatSync(src) : fs.statSync(src) - const destFolder = path.dirname(dest) - const destFolderExists = fs.existsSync(destFolder) - - if (!options.filter(src, dest)) return - - if (stats.isFile()) { - if (!destFolderExists) mkdir.mkdirsSync(destFolder) - copyFileSync(src, dest, { - overwrite: options.overwrite, - errorOnExist: options.errorOnExist, - preserveTimestamps: options.preserveTimestamps - }) - } else if (stats.isDirectory()) { - if (!fs.existsSync(dest)) mkdir.mkdirsSync(dest) - const contents = fs.readdirSync(src) - contents.forEach(content => { - const opts = options - opts.recursive = true - copySync(path.join(src, content), path.join(dest, content), opts) - }) - } else if (options.recursive && stats.isSymbolicLink()) { - const srcPath = fs.readlinkSync(src) - fs.symlinkSync(srcPath, dest) + src = path.resolve(src) + dest = path.resolve(dest) + + // don't allow src and dest to be the same + if (src === dest) throw new Error('Source and destination must not be the same.') + + if (opts.filter && !opts.filter(src, dest)) return + + const destParent = path.dirname(dest) + if (!fs.existsSync(destParent)) mkdirpSync(destParent) + return startCopy(src, dest, opts) +} + +function startCopy (src, dest, opts) { + if (opts.filter && !opts.filter(src, dest)) return + return getStats(src, dest, opts) +} + +function getStats (src, dest, opts) { + const statSync = opts.dereference ? fs.statSync : fs.lstatSync + const st = statSync(src) + + if (st.isDirectory()) return onDir(st, src, dest, opts) + else if (st.isFile() || + st.isCharacterDevice() || + st.isBlockDevice()) return onFile(st, src, dest, opts) + else if (st.isSymbolicLink()) return onLink(src, dest, opts) +} + +function onFile (srcStat, src, dest, opts) { + const resolvedPath = checkDest(dest) + if (resolvedPath === notExist) { + return copyFile(srcStat, src, dest, opts) + } else if (resolvedPath === existsReg) { + return mayCopyFile(srcStat, src, dest, opts) + } else { + if (src === resolvedPath) return + return mayCopyFile(srcStat, src, dest, opts) + } +} + +function mayCopyFile (srcStat, src, dest, opts) { + if (opts.overwrite) { + fs.unlinkSync(dest) + return copyFile(srcStat, src, dest, opts) + } else if (opts.errorOnExist) { + throw new Error(`'${dest}' already exists`) + } +} + +function copyFile (srcStat, src, dest, opts) { + if (typeof fs.copyFile === 'function') { + fs.copyFileSync(src, dest) + fs.chmodSync(dest, srcStat.mode) + if (opts.preserveTimestamps) { + return utimesSync(dest, srcStat.atime, srcStat.mtime) + } + return + } + return copyFileFallback(srcStat, src, dest, opts) +} + +function copyFileFallback (srcStat, src, dest, opts) { + const BUF_LENGTH = 64 * 1024 + const _buff = require('../util/buffer')(BUF_LENGTH) + + const fdr = fs.openSync(src, 'r') + const stat = fs.fstatSync(fdr) + const fdw = fs.openSync(dest, 'w', stat.mode) + let bytesRead = 1 + let pos = 0 + + while (bytesRead > 0) { + bytesRead = fs.readSync(fdr, _buff, 0, BUF_LENGTH, pos) + fs.writeSync(fdw, _buff, 0, bytesRead) + pos += bytesRead + } + + fs.fchmodSync(fdw, srcStat.mode) + if (opts.preserveTimestamps) fs.futimesSync(fdw, srcStat.atime, srcStat.mtime) + + fs.closeSync(fdr) + fs.closeSync(fdw) +} + +function onDir (srcStat, src, dest, opts) { + const resolvedPath = checkDest(dest) + if (resolvedPath === 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 (resolvedPath === existsReg) { + if (isSrcSubdir(src, dest)) { + throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + } + return mayCopyDir(src, dest, opts) + } else { + if (src === resolvedPath) return + return copyDir(src, dest, opts) + } +} + +function mayCopyDir (src, dest, opts) { + if (!fs.statSync(dest).isDirectory()) { + throw new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`) + } + return copyDir(src, dest, opts) +} + +function mkDirAndCopy (srcStat, src, dest, opts) { + fs.mkdirSync(dest, srcStat.mode) + fs.chmodSync(dest, srcStat.mode) + return copyDir(src, dest, opts) +} + +function copyDir (src, dest, opts) { + fs.readdirSync(src).forEach(item => { + startCopy(path.join(src, item), path.join(dest, item), opts) + }) +} + +function onLink (src, dest, opts) { + let resolvedSrcPath = fs.readlinkSync(src) + + if (opts.dereference) { + resolvedSrcPath = path.resolve(process.cwd(), resolvedSrcPath) + } + + let resolvedDestPath = checkDest(dest) + if (resolvedDestPath === notExist || resolvedDestPath === existsReg) { + // if dest already exists, fs throws error anyway, + // so no need to guard against it here. + return fs.symlinkSync(resolvedSrcPath, dest) + } else { + if (opts.dereference) { + resolvedDestPath = path.resolve(process.cwd(), resolvedDestPath) + } + if (resolvedDestPath === resolvedSrcPath) return + + // 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. + if (fs.statSync(dest).isDirectory() && isSrcSubdir(resolvedDestPath, resolvedSrcPath)) { + throw new Error(`Cannot overwrite '${resolvedDestPath}' with '${resolvedSrcPath}'.`) + } + return copyLink(resolvedSrcPath, dest) + } +} + +function copyLink (resolvedSrcPath, dest) { + fs.unlinkSync(dest) + return fs.symlinkSync(resolvedSrcPath, dest) +} + +// check if dest exists and/or is a symlink +function checkDest (dest) { + let resolvedPath + try { + resolvedPath = fs.readlinkSync(dest) + } catch (err) { + if (err.code === 'ENOENT') return notExist + + // dest exists and is a regular file or directory, Windows may throw UNKNOWN error + if (err.code === 'EINVAL' || err.code === 'UNKNOWN') return existsReg + + throw err + } + return resolvedPath // dest exists and is a symlink +} + +// 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 baseDir = dest.split(path.dirname(src) + path.sep)[1] + if (baseDir) { + const destBasename = baseDir.split(path.sep)[0] + if (destBasename) { + return src !== dest && dest.indexOf(src) > -1 && destBasename === path.basename(src) + } + return false } + return false } module.exports = copySync From 546216ccdd3410f1f75910a93593e2bd6ef8350f Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Fri, 10 Nov 2017 18:20:40 -0800 Subject: [PATCH 08/13] Remove setTimeout when preserveTimestamp is true --- lib/copy-sync/__tests__/copy-sync-preserve-time.test.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js b/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js index c9d21ae4..41544866 100644 --- a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js +++ b/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js @@ -39,16 +39,13 @@ describeIf64('copySync() - preserveTimestamps option', () => { }) describe('> when preserveTimestamps option is true', () => { - it('should have the same timestamps on copy', done => { + it('should have the same timestamps on copy', () => { src = path.join(TEST_DIR, 'src') dest = path.join(TEST_DIR, 'dest') FILES.forEach(f => fs.ensureFileSync(path.join(src, f))) - setTimeout(() => { - fs.copySync(src, dest, {preserveTimestamps: true}) - FILES.forEach(testFile({preserveTimestamps: true})) - done() - }, 100) + fs.copySync(src, dest, {preserveTimestamps: true}) + FILES.forEach(testFile({preserveTimestamps: true})) }) }) From d6f6da1e7172cc06b974e5a6f3e202f5d1bdc6ae Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sat, 11 Nov 2017 01:01:40 -0800 Subject: [PATCH 09/13] Use srcStat param in copyFileFallback --- lib/copy-sync/copy-sync.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 81adc397..35387224 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -90,8 +90,7 @@ function copyFileFallback (srcStat, src, dest, opts) { const _buff = require('../util/buffer')(BUF_LENGTH) const fdr = fs.openSync(src, 'r') - const stat = fs.fstatSync(fdr) - const fdw = fs.openSync(dest, 'w', stat.mode) + const fdw = fs.openSync(dest, 'w', srcStat.mode) let bytesRead = 1 let pos = 0 @@ -101,11 +100,9 @@ function copyFileFallback (srcStat, src, dest, opts) { pos += bytesRead } - fs.fchmodSync(fdw, srcStat.mode) - if (opts.preserveTimestamps) fs.futimesSync(fdw, srcStat.atime, srcStat.mtime) - fs.closeSync(fdr) fs.closeSync(fdw) + if (opts.preserveTimestamps) return utimesSync(dest, srcStat.atime, srcStat.mtime) } function onDir (srcStat, src, dest, opts) { From 5d1f2d3c2b66164ec0d91cd061d35834025b156e Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sat, 11 Nov 2017 01:48:37 -0800 Subject: [PATCH 10/13] Use fs.chmodSync() before preserveTimestamp --- lib/copy-sync/copy-sync.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 35387224..edcc0dd7 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -102,7 +102,10 @@ function copyFileFallback (srcStat, src, dest, opts) { fs.closeSync(fdr) fs.closeSync(fdw) - if (opts.preserveTimestamps) return utimesSync(dest, srcStat.atime, srcStat.mtime) + fs.chmodSync(dest, srcStat.mode) + if (opts.preserveTimestamps) { + return utimesSync(dest, srcStat.atime, srcStat.mtime) + } } function onDir (srcStat, src, dest, opts) { From 9732252b9473bc5e631e34de76a9b344319f87d2 Mon Sep 17 00:00:00 2001 From: RyanZim Date: Thu, 9 Nov 2017 17:02:31 -0500 Subject: [PATCH 11/13] Allow copy's filter to return a Promise --- docs/copy.md | 2 +- lib/copy/__tests__/copy.test.js | 13 +++++++++++++ lib/copy/copy.js | 11 ++++++++--- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/docs/copy.md b/docs/copy.md index df834486..f7909d0f 100644 --- a/docs/copy.md +++ b/docs/copy.md @@ -9,7 +9,7 @@ Copy a file or directory. The directory can have contents. Like `cp -r`. - `errorOnExist` ``: when `overwrite` is `false` and the destination exists, throw an error. Default is `false`. - `dereference` ``: dereference symlinks, default is `false`. - `preserveTimestamps` ``: will set last modification and access times to the ones of the original source files, default is `false`. - - `filter` ``: Function to filter copied files. Return `true` to include, `false` to exclude. + - `filter` ``: Function to filter copied files. Return `true` to include, `false` to exclude. Can also return a `Promise` that resolves to `true` or `false` (or pass in an `async` function). - `callback` `` ## Example: diff --git a/lib/copy/__tests__/copy.test.js b/lib/copy/__tests__/copy.test.js index d8e38f63..98692b40 100644 --- a/lib/copy/__tests__/copy.test.js +++ b/lib/copy/__tests__/copy.test.js @@ -249,6 +249,19 @@ describe('fs-extra', () => { }) }) + it('allows filter fn to return a promise', done => { + const srcFile1 = path.join(TEST_DIR, '1.css') + fs.writeFileSync(srcFile1, '') + const destFile1 = path.join(TEST_DIR, 'dest1.css') + const filter = s => Promise.resolve(s.split('.').pop() !== 'css') + + fse.copy(srcFile1, destFile1, filter, err => { + assert(!err) + assert(!fs.existsSync(destFile1)) + done() + }) + }) + it('should apply filter recursively', done => { const FILES = 2 // Don't match anything that ends with a digit higher than 0: diff --git a/lib/copy/copy.js b/lib/copy/copy.js index f270c1d7..421436a5 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -13,7 +13,7 @@ function copy (src, dest, opts, cb) { if (typeof opts === 'function' && !cb) { cb = opts opts = {} - } else if (typeof opts === 'function' || opts instanceof RegExp) { + } else if (typeof opts === 'function') { opts = {filter: opts} } @@ -49,8 +49,13 @@ function copy (src, dest, opts, cb) { } function startCopy (src, dest, opts, cb) { - if (opts.filter && !opts.filter(src, dest)) return cb() - return getStats(src, dest, opts, cb) + if (opts.filter) { + Promise.resolve(opts.filter(src, dest)) + .then(include => { + if (include) getStats(src, dest, opts, cb) + else cb() + }, error => cb(error)) + } else getStats(src, dest, opts, cb) } function getStats (src, dest, opts, cb) { From 5bdbdc4fc87d781b0ae5f41b2bb564c0b89d0362 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Fri, 17 Nov 2017 16:43:54 -0800 Subject: [PATCH 12/13] Skip copySync preserveTimestamp tests on older node versions --- lib/copy-sync/__tests__/copy-sync-preserve-time.test.js | 7 +++++-- lib/copy-sync/copy-sync.js | 8 +++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js b/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js index 41544866..6775a4dd 100644 --- a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js +++ b/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js @@ -5,14 +5,17 @@ const os = require('os') const path = require('path') const utimes = require('../../util/utimes') const assert = require('assert') +const nodeVersion = process.versions.node +const nodeVersionMajor = parseInt(nodeVersion.split('.')[0], 10) /* global beforeEach, afterEach, describe, it */ if (process.arch === 'ia32') console.warn('32 bit arch; skipping copySync timestamp tests') +if (nodeVersionMajor < 8) console.warn(`old node version (v${nodeVersion}); skipping copySync timestamp tests`) -const describeIf64 = process.arch === 'ia32' ? describe.skip : describe +const describeIfPractical = (process.arch === 'ia32' || nodeVersionMajor < 8) ? describe.skip : describe -describeIf64('copySync() - preserveTimestamps option', () => { +describeIfPractical('copySync() - preserveTimestamps option', () => { let TEST_DIR, src, dest beforeEach(done => { diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index edcc0dd7..c4742db8 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -74,7 +74,7 @@ function mayCopyFile (srcStat, src, dest, opts) { } function copyFile (srcStat, src, dest, opts) { - if (typeof fs.copyFile === 'function') { + if (typeof fs.copyFileSync === 'function') { fs.copyFileSync(src, dest) fs.chmodSync(dest, srcStat.mode) if (opts.preserveTimestamps) { @@ -100,12 +100,10 @@ function copyFileFallback (srcStat, src, dest, opts) { pos += bytesRead } + if (opts.preserveTimestamps) fs.futimesSync(fdw, srcStat.atime, srcStat.mtime) + fs.closeSync(fdr) fs.closeSync(fdw) - fs.chmodSync(dest, srcStat.mode) - if (opts.preserveTimestamps) { - return utimesSync(dest, srcStat.atime, srcStat.mtime) - } } function onDir (srcStat, src, dest, opts) { From a06f88b1099e04ebe26aa9e47dbf4a356a7deeb7 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sun, 19 Nov 2017 00:34:48 -0800 Subject: [PATCH 13/13] Apply the async filter to initial filter checking in copy --- lib/copy/copy.js | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 421436a5..fd9687fb 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -35,8 +35,11 @@ function copy (src, dest, opts, cb) { // don't allow src and dest to be the same if (src === dest) return cb(new Error('Source and destination must not be the same.')) - if (opts.filter && !opts.filter(src, dest)) return cb() + if (opts.filter) return handleFilter(checkParentDir, src, dest, opts, cb) + return checkParentDir(src, dest, opts, cb) +} +function checkParentDir (src, dest, opts, cb) { const destParent = path.dirname(dest) pathExists(destParent, (err, dirExists) => { if (err) return cb(err) @@ -49,13 +52,16 @@ function copy (src, dest, opts, cb) { } function startCopy (src, dest, opts, cb) { - if (opts.filter) { - Promise.resolve(opts.filter(src, dest)) - .then(include => { - if (include) getStats(src, dest, opts, cb) - else cb() - }, error => cb(error)) - } else getStats(src, dest, opts, cb) + if (opts.filter) return handleFilter(getStats, src, dest, opts, cb) + return getStats(src, dest, opts, cb) +} + +function handleFilter (onInclude, src, dest, opts, cb) { + Promise.resolve(opts.filter(src, dest)) + .then(include => { + if (include) return onInclude(src, dest, opts, cb) + return cb() + }, error => cb(error)) } function getStats (src, dest, opts, cb) {