From fda912a38178b0c3c53557b6712d90eea2cabb36 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Fri, 10 Nov 2017 17:56:58 -0800 Subject: [PATCH 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5bdbdc4fc87d781b0ae5f41b2bb564c0b89d0362 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Fri, 17 Nov 2017 16:43:54 -0800 Subject: [PATCH 5/5] 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) {