From a0bb74f4ce61e2563f72563818cc0fdc5545d452 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Mon, 6 Aug 2018 05:28:14 -0700 Subject: [PATCH 01/19] Remove secure-random from dev-deps (#610) --- lib/remove/__tests__/remove.test.js | 4 ++-- package.json | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/remove/__tests__/remove.test.js b/lib/remove/__tests__/remove.test.js index 92979d57..5add5997 100644 --- a/lib/remove/__tests__/remove.test.js +++ b/lib/remove/__tests__/remove.test.js @@ -4,7 +4,7 @@ const assert = require('assert') const fs = require('fs') const os = require('os') const path = require('path') -const sr = require('secure-random') +const randomBytes = require('crypto').randomBytes const fse = require(process.cwd()) /* global afterEach, beforeEach, describe, it */ @@ -12,7 +12,7 @@ const fse = require(process.cwd()) let TEST_DIR function buildFixtureDir () { - const buf = sr.randomBuffer(5) + const buf = randomBytes(5) const baseDir = path.join(TEST_DIR, `TEST_fs-extra_remove-${Date.now()}`) fs.mkdirSync(baseDir) diff --git a/package.json b/package.json index 9d992b88..1b73c6bf 100644 --- a/package.json +++ b/package.json @@ -50,7 +50,6 @@ "proxyquire": "^2.0.1", "read-dir-files": "^0.1.1", "rimraf": "^2.2.8", - "secure-random": "^1.1.1", "semver": "^5.3.0", "standard": "^11.0.1", "standard-markdown": "^4.0.1" From 7701ef2406b9ec5162a4d72ccca407fd8d873419 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Tue, 7 Aug 2018 04:01:51 -0700 Subject: [PATCH 02/19] fix ensureDir() doc --- docs/copy-sync.md | 2 +- docs/copy.md | 2 +- docs/emptyDir.md | 2 +- docs/ensureFile.md | 2 +- docs/ensureLink.md | 2 +- docs/ensureSymlink-sync.md | 2 +- docs/ensureSymlink.md | 2 +- docs/move-sync.md | 2 +- docs/move.md | 2 +- docs/outputFile-sync.md | 2 +- docs/outputFile.md | 2 +- docs/outputJson-sync.md | 2 +- docs/outputJson.md | 2 +- docs/readJson-sync.md | 2 +- docs/readJson.md | 2 +- docs/remove.md | 2 +- docs/writeJson-sync.md | 2 +- docs/writeJson.md | 2 +- .../copy-without-checking-paths.test.js | 295 ++++++++++++++++++ 19 files changed, 313 insertions(+), 18 deletions(-) create mode 100644 lib/copy/__tests__/copy-without-checking-paths.test.js diff --git a/docs/copy-sync.md b/docs/copy-sync.md index 76bfd477..3fca1330 100644 --- a/docs/copy-sync.md +++ b/docs/copy-sync.md @@ -1,4 +1,4 @@ -# copySync(src, dest, [options]) +# copySync(src, dest[, options]) Copy a file or directory. The directory can have contents. Like `cp -r`. diff --git a/docs/copy.md b/docs/copy.md index e5f8582b..4add5684 100644 --- a/docs/copy.md +++ b/docs/copy.md @@ -1,4 +1,4 @@ -# copy(src, dest, [options, callback]) +# copy(src, dest[, options][, callback]) Copy a file or directory. The directory can have contents. Like `cp -r`. diff --git a/docs/emptyDir.md b/docs/emptyDir.md index 3d129d42..8f26dc10 100644 --- a/docs/emptyDir.md +++ b/docs/emptyDir.md @@ -1,4 +1,4 @@ -# emptyDir(dir, [callback]) +# emptyDir(dir[, callback]) Ensures that a directory is empty. Deletes directory contents if the directory is not empty. If the directory does not exist, it is created. The directory itself is not deleted. diff --git a/docs/ensureFile.md b/docs/ensureFile.md index aa8e82ac..a560e966 100644 --- a/docs/ensureFile.md +++ b/docs/ensureFile.md @@ -1,4 +1,4 @@ -# ensureFile(file, [callback]) +# ensureFile(file[, callback]) Ensures that the file exists. If the file that is requested to be created is in directories that do not exist, these directories are created. If the file already exists, it is **NOT MODIFIED**. diff --git a/docs/ensureLink.md b/docs/ensureLink.md index 3f39f07b..7bdddebf 100644 --- a/docs/ensureLink.md +++ b/docs/ensureLink.md @@ -1,4 +1,4 @@ -# ensureLink(srcpath, dstpath, [callback]) +# ensureLink(srcpath, dstpath[, callback]) Ensures that the link exists. If the directory structure does not exist, it is created. diff --git a/docs/ensureSymlink-sync.md b/docs/ensureSymlink-sync.md index 328d4c45..72b39c29 100644 --- a/docs/ensureSymlink-sync.md +++ b/docs/ensureSymlink-sync.md @@ -1,4 +1,4 @@ -# ensureSymlinkSync(srcpath, dstpath, [type]) +# ensureSymlinkSync(srcpath, dstpath[, type]) Ensures that the symlink exists. If the directory structure does not exist, it is created. diff --git a/docs/ensureSymlink.md b/docs/ensureSymlink.md index 39c09568..383c49f0 100644 --- a/docs/ensureSymlink.md +++ b/docs/ensureSymlink.md @@ -1,4 +1,4 @@ -# ensureSymlink(srcpath, dstpath, [type, callback]) +# ensureSymlink(srcpath, dstpath[, type][, callback]) Ensures that the symlink exists. If the directory structure does not exist, it is created. diff --git a/docs/move-sync.md b/docs/move-sync.md index cd701fef..d5153b58 100644 --- a/docs/move-sync.md +++ b/docs/move-sync.md @@ -1,4 +1,4 @@ -# moveSync(src, dest, [options]) +# moveSync(src, dest[, options]) Moves a file or directory, even across devices. diff --git a/docs/move.md b/docs/move.md index 343d10be..5f70853e 100644 --- a/docs/move.md +++ b/docs/move.md @@ -1,4 +1,4 @@ -# move(src, dest, [options, callback]) +# move(src, dest[, options][, callback]) Moves a file or directory, even across devices. diff --git a/docs/outputFile-sync.md b/docs/outputFile-sync.md index 38eee8b6..fc7abdf9 100644 --- a/docs/outputFile-sync.md +++ b/docs/outputFile-sync.md @@ -1,4 +1,4 @@ -# outputFileSync(file, data, [options]) +# outputFileSync(file, data[, options]) Almost the same as `writeFileSync` (i.e. it [overwrites](http://pages.citebite.com/v2o5n8l2f5reb)), except that if the parent directory does not exist, it's created. `file` must be a file path (a buffer or a file descriptor is not allowed). `options` are what you'd pass to [`fs.writeFileSync()`](https://nodejs.org/api/fs.html#fs_fs_writefilesync_file_data_options). diff --git a/docs/outputFile.md b/docs/outputFile.md index 34efdc94..ed9c19d6 100644 --- a/docs/outputFile.md +++ b/docs/outputFile.md @@ -1,4 +1,4 @@ -# outputFile(file, data, [options, callback]) +# outputFile(file, data[, options][, callback]) Almost the same as `writeFile` (i.e. it [overwrites](http://pages.citebite.com/v2o5n8l2f5reb)), except that if the parent directory does not exist, it's created. `file` must be a file path (a buffer or a file descriptor is not allowed). `options` are what you'd pass to [`fs.writeFile()`](https://nodejs.org/api/fs.html#fs_fs_writefile_file_data_options_callback). diff --git a/docs/outputJson-sync.md b/docs/outputJson-sync.md index ef78f802..c42497cf 100644 --- a/docs/outputJson-sync.md +++ b/docs/outputJson-sync.md @@ -1,4 +1,4 @@ -# outputJsonSync(file, object, [options]) +# outputJsonSync(file, object[, options]) Almost the same as [`writeJsonSync`](writeJson-sync.md), except that if the directory does not exist, it's created. diff --git a/docs/outputJson.md b/docs/outputJson.md index aa265aee..d5a6d7c7 100644 --- a/docs/outputJson.md +++ b/docs/outputJson.md @@ -1,4 +1,4 @@ -# outputJson(file, object, [options, callback]) +# outputJson(file, object[, options][, callback]) Almost the same as [`writeJson`](writeJson.md), except that if the directory does not exist, it's created. diff --git a/docs/readJson-sync.md b/docs/readJson-sync.md index a1356379..1fc6519d 100644 --- a/docs/readJson-sync.md +++ b/docs/readJson-sync.md @@ -1,4 +1,4 @@ -# readJsonSync(file, [options]) +# readJsonSync(file[, options]) Reads a JSON file and then parses it into an object. `options` are the same that you'd pass to [`jsonFile.readFileSync`](https://github.com/jprichardson/node-jsonfile#readfilesyncfilename-options). diff --git a/docs/readJson.md b/docs/readJson.md index baf80ac8..881e7972 100644 --- a/docs/readJson.md +++ b/docs/readJson.md @@ -1,4 +1,4 @@ -# readJson(file, [options, callback]) +# readJson(file[, options][, callback]) Reads a JSON file and then parses it into an object. `options` are the same that you'd pass to [`jsonFile.readFile`](https://github.com/jprichardson/node-jsonfile#readfilefilename-options-callback). diff --git a/docs/remove.md b/docs/remove.md index bcc49e39..0cdcf7ac 100644 --- a/docs/remove.md +++ b/docs/remove.md @@ -1,4 +1,4 @@ -# remove(path, [callback]) +# remove(path[, callback]) Removes a file or directory. The directory can have contents. Like `rm -rf`. diff --git a/docs/writeJson-sync.md b/docs/writeJson-sync.md index c22459db..b98e0c7d 100644 --- a/docs/writeJson-sync.md +++ b/docs/writeJson-sync.md @@ -1,4 +1,4 @@ -# writeJsonSync(file, object, [options]) +# writeJsonSync(file, object[, options]) Writes an object to a JSON file. diff --git a/docs/writeJson.md b/docs/writeJson.md index 56278029..216d1348 100644 --- a/docs/writeJson.md +++ b/docs/writeJson.md @@ -1,4 +1,4 @@ -# writeJson(file, object, [options, callback]) +# writeJson(file, object[, options][, callback]) Writes an object to a JSON file. diff --git a/lib/copy/__tests__/copy-without-checking-paths.test.js b/lib/copy/__tests__/copy-without-checking-paths.test.js new file mode 100644 index 00000000..68c394c3 --- /dev/null +++ b/lib/copy/__tests__/copy-without-checking-paths.test.js @@ -0,0 +1,295 @@ +'use strict' + +const assert = require('assert') +const os = require('os') +const path = require('path') +const fs = require('../../') +const klawSync = require('klaw-sync') + +/* global beforeEach, afterEach, describe, it */ + +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('+ copy() - without checking paths', () => { + let TEST_DIR, src + + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-without-checking-paths') + 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 if dest parent is a subdir of src', done => { + const srcFile = path.join(TEST_DIR, 'src', 'srcfile.txt') + const destFile = path.join(TEST_DIR, 'src', 'dest', 'destfile.txt') + fs.writeFileSync(srcFile, dat0) + + fs.copy(srcFile, destFile, { checkPathsBeforeCopying: false }, err => { + assert.ifError(err) + + assert(fs.existsSync(destFile)) + const out = fs.readFileSync(destFile, 'utf8') + assert.strictEqual(out, dat0) + 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(`of not itself`, done => { + const dest = path.join(TEST_DIR, src.replace(/^\w:/, '')) + 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 '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) + }) + }) + + describe('>> when dest is a symlink', () => { + 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, { checkPathsBeforeCopying: false }, 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)))) + + 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) + assert.strictEqual(o1, dat1) + assert.strictEqual(o2, dat2) + assert.strictEqual(o3, dat3) + 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, { checkPathsBeforeCopying: false }, 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, { checkPathsBeforeCopying: false }, 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, { checkPathsBeforeCopying: false }, 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 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, { checkPathsBeforeCopying: false }, err => { + assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${resolvedDestPath}'.`) + const destln = fs.readlinkSync(destLink) + assert.strictEqual(destln, resolvedDestPath) + done() + }) + }) + + it('should error when resolved src path is a subdir of resolved dest path', done => { + const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src') + const srcLink = path.join(TEST_DIR, 'src-symlink') + const destLink = path.join(TEST_DIR, 'dest-symlink') + const dest = path.join(TEST_DIR, 'dest') + + fs.ensureDirSync(srcInDest) + fs.ensureSymlinkSync(srcInDest, srcLink, 'dir') + fs.ensureSymlinkSync(dest, destLink, 'dir') + + fs.copy(srcLink, destLink, { checkPathsBeforeCopying: false }, 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, { checkPathsBeforeCopying: false }, err => { + assert.ifError(err) + + FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)))) + + 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) + assert.strictEqual(o1, dat1) + assert.strictEqual(o2, dat2) + assert.strictEqual(o3, dat3) + done() + }) +} From e85799d0955f5c65ce1bb0ed28042ece54f945b5 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sat, 4 Aug 2018 14:08:03 -0700 Subject: [PATCH 03/19] moveSync: refactor to use renameSync --- ...ve-sync-prevent-moving-into-itself.test.js | 2 +- lib/move-sync/__tests__/move-sync.test.js | 94 +++----------- lib/move-sync/index.js | 119 +++++------------- package.json | 1 - 4 files changed, 49 insertions(+), 167 deletions(-) diff --git a/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js b/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js index 3685faaf..1bf32c05 100644 --- a/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js +++ b/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js @@ -182,7 +182,7 @@ function testError (src, dest) { try { fs.moveSync(src, dest) } catch (err) { - assert.strictEqual(err.message, `Cannot move '${src}' into itself '${dest}'.`) + assert.strictEqual(err.message, `Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) assert(fs.existsSync(src)) assert(!fs.existsSync(dest)) } diff --git a/lib/move-sync/__tests__/move-sync.test.js b/lib/move-sync/__tests__/move-sync.test.js index d2c226b3..2139b8ce 100644 --- a/lib/move-sync/__tests__/move-sync.test.js +++ b/lib/move-sync/__tests__/move-sync.test.js @@ -5,7 +5,6 @@ const os = require('os') const fse = require(process.cwd()) const path = require('path') const assert = require('assert') -const rimraf = require('rimraf') /* global afterEach, beforeEach, describe, it */ @@ -19,16 +18,13 @@ function createSyncErrFn (errCode) { } const originalRenameSync = fs.renameSync -const originalLinkSync = fs.linkSync function setUpMockFs (errCode) { fs.renameSync = createSyncErrFn(errCode) - fs.linkSync = createSyncErrFn(errCode) } function tearDownMockFs () { fs.renameSync = originalRenameSync - fs.linkSync = originalLinkSync } describe('moveSync()', () => { @@ -36,18 +32,15 @@ describe('moveSync()', () => { beforeEach(() => { TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'move-sync') - fse.emptyDirSync(TEST_DIR) - // Create fixtures: - fs.writeFileSync(path.join(TEST_DIR, 'a-file'), 'sonic the hedgehog\n') - fs.mkdirSync(path.join(TEST_DIR, 'a-folder')) - fs.writeFileSync(path.join(TEST_DIR, 'a-folder/another-file'), 'tails\n') - fs.mkdirSync(path.join(TEST_DIR, 'a-folder/another-folder')) - fs.writeFileSync(path.join(TEST_DIR, 'a-folder/another-folder/file3'), 'knuckles\n') + // Create fixtures + fse.outputFileSync(path.join(TEST_DIR, 'a-file'), 'sonic the hedgehog\n') + fse.outputFileSync(path.join(TEST_DIR, 'a-folder/another-file'), 'tails\n') + fse.outputFileSync(path.join(TEST_DIR, 'a-folder/another-folder/file3'), 'knuckles\n') }) - afterEach(done => rimraf(TEST_DIR, done)) + afterEach(() => fse.removeSync(TEST_DIR)) it('should not move if src and dest are the same', () => { const src = `${TEST_DIR}/a-file` @@ -58,13 +51,12 @@ describe('moveSync()', () => { // assert src not affected const contents = fs.readFileSync(src, 'utf8') const expected = /^sonic the hedgehog\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert(contents.match(expected)) }) it('should error if src and dest are the same and src does not exist', () => { const src = `${TEST_DIR}/non-existent` const dest = src - assert.throws(() => fse.moveSync(src, dest)) }) @@ -76,7 +68,7 @@ describe('moveSync()', () => { const contents = fs.readFileSync(dest, 'utf8') const expected = /^sonic the hedgehog\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert(contents.match(expected)) }) it('should not overwrite the destination by default', () => { @@ -89,7 +81,7 @@ describe('moveSync()', () => { try { fse.moveSync(src, dest) } catch (err) { - assert.ok(err && err.code === 'EEXIST', 'throw EEXIST') + assert.strictEqual(err.message, 'dest already exists.') } }) @@ -103,7 +95,7 @@ describe('moveSync()', () => { try { fse.moveSync(src, dest, {overwrite: false}) } catch (err) { - assert.ok(err && err.code === 'EEXIST', 'throw EEXIST') + assert.strictEqual(err.message, 'dest already exists.') } }) @@ -121,7 +113,7 @@ describe('moveSync()', () => { assert.ok(contents.match(expected), `${contents} match ${expected}`) }) - it('should overwrite the destination directory if overwrite = true', done => { + it('should overwrite the destination directory if overwrite = true', () => { // Create src const src = path.join(TEST_DIR, 'src') fse.ensureDirSync(src) @@ -145,7 +137,6 @@ describe('moveSync()', () => { // verify dest has new stuff assert(pathsAfter.indexOf('some-file') >= 0) assert(pathsAfter.indexOf('some-folder') >= 0) - done() }) it('should create directory structure by default', () => { @@ -159,7 +150,7 @@ describe('moveSync()', () => { const contents = fs.readFileSync(dest, 'utf8') const expected = /^sonic the hedgehog\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert(contents.match(expected)) }) it('should work across devices', () => { @@ -172,7 +163,7 @@ describe('moveSync()', () => { const contents = fs.readFileSync(dest, 'utf8') const expected = /^sonic the hedgehog\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert(contents.match(expected)) tearDownMockFs() }) @@ -187,27 +178,12 @@ describe('moveSync()', () => { const contents = fs.readFileSync(dest + '/another-file', 'utf8') const expected = /^tails\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) - }) - - it('should move folders across devices with EISDIR error', () => { - const src = `${TEST_DIR}/a-folder` - const dest = `${TEST_DIR}/a-folder-dest` - - setUpMockFs('EISDIR') - - fse.moveSync(src, dest) - - const contents = fs.readFileSync(dest + '/another-folder/file3', 'utf8') - const expected = /^knuckles\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) - tearDownMockFs() + assert(contents.match(expected)) }) it('should overwrite folders across devices', () => { const src = `${TEST_DIR}/a-folder` const dest = `${TEST_DIR}/a-folder-dest` - fs.mkdirSync(dest) setUpMockFs('EXDEV') @@ -216,7 +192,7 @@ describe('moveSync()', () => { const contents = fs.readFileSync(dest + '/another-folder/file3', 'utf8') const expected = /^knuckles\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert(contents.match(expected)) tearDownMockFs() }) @@ -230,35 +206,7 @@ describe('moveSync()', () => { const contents = fs.readFileSync(dest + '/another-folder/file3', 'utf8') const expected = /^knuckles\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) - tearDownMockFs() - }) - - it('should move folders across devices with EPERM error', () => { - const src = `${TEST_DIR}/a-folder` - const dest = `${TEST_DIR}/a-folder-dest` - - setUpMockFs('EPERM') - - fse.moveSync(src, dest) - - const contents = fs.readFileSync(dest + '/another-folder/file3', 'utf8') - const expected = /^knuckles\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) - tearDownMockFs() - }) - - it('should move folders across devices with ENOTSUP error', () => { - const src = `${TEST_DIR}/a-folder` - const dest = `${TEST_DIR}/a-folder-dest` - - setUpMockFs('ENOTSUP') - - fse.moveSync(src, dest) - - const contents = fs.readFileSync(dest + '/another-folder/file3', 'utf8') - const expected = /^knuckles\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert(contents.match(expected)) tearDownMockFs() }) @@ -274,7 +222,7 @@ describe('moveSync()', () => { const contents = fs.readFileSync(dest, 'utf8') const expected = /^sonic the hedgehog\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert(contents.match(expected)) }) }) @@ -306,7 +254,7 @@ describe('moveSync()', () => { const contents = fs.readFileSync(dest, 'utf8') const expected = /^sonic the hedgehog\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert(contents.match(expected)) }) }) @@ -324,7 +272,7 @@ describe('moveSync()', () => { try { fs.writeFileSync(path.join(differentDevice, 'file'), 'hi') } catch (err) { - console.log("Can't write to device. Skipping moveSync test.") + console.log(`Can't write to device. Skipping moveSync test.`) __skipTests = true } @@ -335,12 +283,8 @@ describe('moveSync()', () => { const src = '/mnt/some/weird/dir-really-weird' const dest = path.join(TEST_DIR, 'device-weird') - if (!fs.existsSync(src)) { - fse.mkdirpSync(src) - } - + if (!fs.existsSync(src)) fse.mkdirpSync(src) assert(!fs.existsSync(dest)) - assert(fs.lstatSync(src).isDirectory()) fse.moveSync(src, dest) diff --git a/lib/move-sync/index.js b/lib/move-sync/index.js index 6d4f56fa..8bfdbc14 100644 --- a/lib/move-sync/index.js +++ b/lib/move-sync/index.js @@ -4,114 +4,53 @@ const fs = require('graceful-fs') const path = require('path') const copySync = require('../copy-sync').copySync const removeSync = require('../remove').removeSync -const mkdirpSync = require('../mkdirs').mkdirsSync -const buffer = require('../util/buffer') - -function moveSync (src, dest, options) { - options = options || {} - const overwrite = options.overwrite || options.clobber || false +const mkdirpSync = require('../mkdirs').mkdirpSync +function moveSync (src, dest, opts) { + opts = opts || {} + const overwrite = opts.overwrite || opts.clobber || false src = path.resolve(src) dest = path.resolve(dest) - if (src === dest) return fs.accessSync(src) - if (isSrcSubdir(src, dest)) throw new Error(`Cannot move '${src}' into itself '${dest}'.`) - + const st = fs.statSync(src) + if (st.isDirectory() && isSrcSubdir(src, dest)) throw new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) mkdirpSync(path.dirname(dest)) - tryRenameSync() - - function tryRenameSync () { - if (overwrite) { - try { - return fs.renameSync(src, dest) - } catch (err) { - if (err.code === 'ENOTEMPTY' || err.code === 'EEXIST' || err.code === 'EPERM') { - removeSync(dest) - options.overwrite = false // just overwriteed it, no need to do it again - return moveSync(src, dest, options) - } - - if (err.code !== 'EXDEV') throw err - return moveSyncAcrossDevice(src, dest, overwrite) - } - } else { - try { - fs.linkSync(src, dest) - return fs.unlinkSync(src) - } catch (err) { - if (err.code === 'EXDEV' || err.code === 'EISDIR' || err.code === 'EPERM' || err.code === 'ENOTSUP') { - return moveSyncAcrossDevice(src, dest, overwrite) - } - throw err - } - } - } + return doRename(src, dest, overwrite) } -function moveSyncAcrossDevice (src, dest, overwrite) { - const stat = fs.statSync(src) - - if (stat.isDirectory()) { - return moveDirSyncAcrossDevice(src, dest, overwrite) - } else { - return moveFileSyncAcrossDevice(src, dest, overwrite) +function doRename (src, dest, overwrite) { + if (overwrite) { + removeSync(dest) + return rename(src, dest, overwrite) } + if (fs.existsSync(dest)) throw new Error('dest already exists.') + return rename(src, dest, overwrite) } -function moveFileSyncAcrossDevice (src, dest, overwrite) { - const BUF_LENGTH = 64 * 1024 - const _buff = buffer(BUF_LENGTH) - - const flags = overwrite ? 'w' : 'wx' - - const fdr = fs.openSync(src, 'r') - const stat = fs.fstatSync(fdr) - const fdw = fs.openSync(dest, flags, stat.mode) - let pos = 0 - - while (pos < stat.size) { - const bytesRead = fs.readSync(fdr, _buff, 0, BUF_LENGTH, pos) - fs.writeSync(fdw, _buff, 0, bytesRead) - pos += bytesRead +function rename (src, dest, overwrite) { + try { + fs.renameSync(src, dest) + } catch (err) { + if (err.code !== 'EXDEV') throw err + return moveAcrossDevice(src, dest, overwrite) } - - fs.closeSync(fdr) - fs.closeSync(fdw) - return fs.unlinkSync(src) } -function moveDirSyncAcrossDevice (src, dest, overwrite) { - const options = { - overwrite: false +function moveAcrossDevice (src, dest, overwrite) { + const opts = { + overwrite, + errorOnExist: true } - if (overwrite) { - removeSync(dest) - tryCopySync() - } else { - tryCopySync() - } - - function tryCopySync () { - copySync(src, dest, options) - return removeSync(src) - } + copySync(src, dest, opts) + return removeSync(src) } -// 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) { - try { - return fs.statSync(src).isDirectory() && - src !== dest && - dest.indexOf(src) > -1 && - dest.split(path.dirname(src) + path.sep)[1].split(path.sep)[0] === path.basename(src) - } catch (e) { - return false - } + const srcArr = src.split(path.sep) + const destArr = dest.split(path.sep) + return srcArr.reduce((acc, curr, i) => acc && destArr[i] === curr, true) } -module.exports = { - moveSync -} +module.exports = { moveSync } diff --git a/package.json b/package.json index 1b73c6bf..e27acdb1 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,6 @@ "mocha": "^5.0.5", "proxyquire": "^2.0.1", "read-dir-files": "^0.1.1", - "rimraf": "^2.2.8", "semver": "^5.3.0", "standard": "^11.0.1", "standard-markdown": "^4.0.1" From 5116248b6ac1314c98edba03b40417b7a5ecbdf1 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sun, 2 Sep 2018 11:33:05 -0700 Subject: [PATCH 04/19] copy*(): fix copying bind-mounted directories (#618) * copy*(): fix copying bind-mounted dirs * copy*(): fix case-insensitive-paths tests * copy*(): refactor to check paths more efficiently * destructure stats object after checking err --- .../__tests__/broken-symlink.test.js | 2 +- .../copy-sync-case-insensitive-paths.test.js | 53 ++++++++---------- lib/copy-sync/__tests__/copy-sync-dir.test.js | 4 +- .../__tests__/copy-sync-file.test.js | 2 +- .../__tests__/copy-sync-preserve-time.test.js | 2 +- ...opy-sync-prevent-copying-identical.test.js | 2 +- ...y-sync-prevent-copying-into-itself.test.js | 56 +++++++++++++++++-- .../__tests__/copy-sync-readonly-dir.test.js | 9 ++- lib/copy-sync/__tests__/symlink.test.js | 7 +-- lib/copy-sync/copy-sync.js | 33 +++++++++-- .../copy-case-insensitive-paths.test.js | 45 +++++++-------- lib/copy/__tests__/copy-gh-89.test.js | 2 +- lib/copy/__tests__/copy-preserve-time.test.js | 2 +- .../copy-prevent-copying-identical.test.js | 2 +- .../copy-prevent-copying-into-itself.test.js | 48 ++++++++++++++-- lib/copy/__tests__/copy-readonly-dir.test.js | 2 +- lib/copy/__tests__/copy.test.js | 15 +++-- lib/copy/copy.js | 40 ++++++++++--- 18 files changed, 222 insertions(+), 104 deletions(-) diff --git a/lib/copy-sync/__tests__/broken-symlink.test.js b/lib/copy-sync/__tests__/broken-symlink.test.js index 2744c64e..a5e6d57f 100644 --- a/lib/copy-sync/__tests__/broken-symlink.test.js +++ b/lib/copy-sync/__tests__/broken-symlink.test.js @@ -2,7 +2,7 @@ const fs = require('fs') const os = require('os') -const fse = require(process.cwd()) +const fse = require('../../') const path = require('path') const assert = require('assert') const copySync = require('../copy-sync') diff --git a/lib/copy-sync/__tests__/copy-sync-case-insensitive-paths.test.js b/lib/copy-sync/__tests__/copy-sync-case-insensitive-paths.test.js index 216c58b2..fcffa53e 100644 --- a/lib/copy-sync/__tests__/copy-sync-case-insensitive-paths.test.js +++ b/lib/copy-sync/__tests__/copy-sync-case-insensitive-paths.test.js @@ -3,7 +3,8 @@ const assert = require('assert') const os = require('os') const path = require('path') -const fs = require(process.cwd()) +const fs = require('../../') +const platform = os.platform() /* global beforeEach, afterEach, describe, it */ @@ -17,7 +18,7 @@ describe('+ copySync() - case insensitive paths', () => { fs.emptyDir(TEST_DIR, done) }) - afterEach(done => fs.remove(TEST_DIR, done)) + afterEach(() => fs.removeSync(TEST_DIR)) describe('> when src is a directory', () => { it('should behave correctly based on the OS', () => { @@ -29,15 +30,13 @@ describe('+ copySync() - case insensitive paths', () => { try { fs.copySync(src, dest) } catch (err) { - if (os === 'darwin' || os === 'win32') { + if (platform === 'darwin' || platform === 'win32') { assert.strictEqual(err.message, 'Source and destination must not be the same.') - assert(fs.existsSync(src)) - assert(!fs.existsSync(dest)) errThrown = true } } - if (os === 'darwin' || os === 'win32') assert(errThrown) - if (os === 'linux') { + if (platform === 'darwin' || platform === 'win32') assert(errThrown) + if (platform === 'linux') { assert(fs.existsSync(dest)) assert.strictEqual(fs.readFileSync(path.join(dest, 'subdir', 'file.txt'), 'utf8'), 'some data') assert(!errThrown) @@ -55,15 +54,13 @@ describe('+ copySync() - case insensitive paths', () => { try { fs.copySync(src, dest) } catch (err) { - if (os === 'darwin' || os === 'win32') { + if (platform === 'darwin' || platform === 'win32') { assert.strictEqual(err.message, 'Source and destination must not be the same.') - assert(fs.existsSync(src)) - assert(!fs.existsSync(dest)) errThrown = true } } - if (os === 'darwin' || os === 'win32') assert(errThrown) - if (os === 'linux') { + if (platform === 'darwin' || platform === 'win32') assert(errThrown) + if (platform === 'linux') { assert(fs.existsSync(dest)) assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'some data') assert(!errThrown) @@ -77,25 +74,23 @@ describe('+ copySync() - case insensitive paths', () => { fs.outputFileSync(path.join(src, 'subdir', 'file.txt'), 'some data') const srcLink = path.join(TEST_DIR, 'src-symlink') fs.symlinkSync(src, srcLink, 'dir') - dest = path.join(TEST_DIR, 'srcDir') + dest = path.join(TEST_DIR, 'src-Symlink') let errThrown = false try { - fs.copySync(src, dest) + fs.copySync(srcLink, dest) } catch (err) { - if (os === 'darwin' || os === 'win32') { + if (platform === 'darwin' || platform === 'win32') { assert.strictEqual(err.message, 'Source and destination must not be the same.') - assert(fs.existsSync(src)) - assert(!fs.existsSync(dest)) errThrown = true } } - if (os === 'darwin' || os === 'win32') assert(errThrown) - if (os === 'linux') { + if (platform === 'darwin' || platform === 'win32') assert(errThrown) + if (platform === 'linux') { assert(fs.existsSync(dest)) assert.strictEqual(fs.readFileSync(path.join(dest, 'subdir', 'file.txt'), 'utf8'), 'some data') - const link = fs.readlinkSync(srcLink) - assert.strictEqual(link, dest) + const destLink = fs.readlinkSync(dest) + assert.strictEqual(destLink, src) assert(!errThrown) } }) @@ -105,25 +100,23 @@ describe('+ copySync() - case insensitive paths', () => { fs.outputFileSync(src, 'some data') const srcLink = path.join(TEST_DIR, 'src-symlink') fs.symlinkSync(src, srcLink, 'file') - dest = path.join(TEST_DIR, 'srcFile') + dest = path.join(TEST_DIR, 'src-Symlink') let errThrown = false try { - fs.copySync(src, dest) + fs.copySync(srcLink, dest) } catch (err) { - if (os === 'darwin' || os === 'win32') { + if (platform === 'darwin' || platform === 'win32') { assert.strictEqual(err.message, 'Source and destination must not be the same.') - assert(fs.existsSync(src)) - assert(!fs.existsSync(dest)) errThrown = true } } - if (os === 'darwin' || os === 'win32') assert(errThrown) - if (os === 'linux') { + if (platform === 'darwin' || platform === 'win32') assert(errThrown) + if (platform === 'linux') { assert(fs.existsSync(dest)) assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'some data') - const link = fs.readlinkSync(srcLink) - assert.strictEqual(link, dest) + const destLink = fs.readlinkSync(dest) + assert.strictEqual(destLink, src) assert(!errThrown) } }) diff --git a/lib/copy-sync/__tests__/copy-sync-dir.test.js b/lib/copy-sync/__tests__/copy-sync-dir.test.js index 7ce6e981..7a5ecf51 100644 --- a/lib/copy-sync/__tests__/copy-sync-dir.test.js +++ b/lib/copy-sync/__tests__/copy-sync-dir.test.js @@ -1,6 +1,6 @@ 'use strict' -const fs = require(process.cwd()) +const fs = require('../../') const os = require('os') const path = require('path') const assert = require('assert') @@ -75,7 +75,7 @@ describe('+ copySync() / dir', () => { const srcTarget = path.join(TEST_DIR, 'destination') fs.mkdirSync(src) fs.mkdirSync(srcTarget) - fs.symlinkSync(srcTarget, path.join(src, 'symlink')) + fs.symlinkSync(srcTarget, path.join(src, 'symlink'), 'dir') fs.copySync(src, dest) diff --git a/lib/copy-sync/__tests__/copy-sync-file.test.js b/lib/copy-sync/__tests__/copy-sync-file.test.js index 45ef334d..b1122a23 100644 --- a/lib/copy-sync/__tests__/copy-sync-file.test.js +++ b/lib/copy-sync/__tests__/copy-sync-file.test.js @@ -1,6 +1,6 @@ 'use strict' -const fs = require(process.cwd()) +const fs = require('../../') const os = require('os') const path = require('path') const assert = require('assert') 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 c190da70..76dbfe09 100644 --- a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js +++ b/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js @@ -1,6 +1,6 @@ 'use strict' -const fs = require(process.cwd()) +const fs = require('../../') const os = require('os') const path = require('path') const utimes = require('../../util/utimes') diff --git a/lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js b/lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js index d9dca5e0..c1f3091d 100644 --- a/lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js +++ b/lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js @@ -3,7 +3,7 @@ const assert = require('assert') const os = require('os') const path = require('path') -const fs = require(process.cwd()) +const fs = require('../../') const klawSync = require('klaw-sync') /* global beforeEach, afterEach, describe, it */ diff --git a/lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js b/lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js index 487c997a..51f864ac 100644 --- a/lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js +++ b/lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js @@ -3,7 +3,7 @@ const assert = require('assert') const os = require('os') const path = require('path') -const fs = require(process.cwd()) +const fs = require('../../') const klawSync = require('klaw-sync') /* global beforeEach, afterEach, describe, it */ @@ -166,6 +166,56 @@ describe('+ copySync() - prevent copying into itself', () => { assert.strictEqual(link, src) }) + it('should error when dest is a subdirectory of src (bind-mounted directory with subdirectory)', () => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) + + const dest = path.join(destLink, 'dir1') + assert(fs.existsSync(dest)) + let errThrown = false + try { + fs.copySync(src, dest) + } catch (err) { + assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + errThrown = true + } finally { + assert(errThrown) + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + } + }) + + it('should error when dest is a subdirectory of src (more than one level depth)', () => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) + + const dest = path.join(destLink, 'dir1', 'dir2') + assert(fs.existsSync(dest)) + let errThrown = false + try { + fs.copySync(src, dest) + } catch (err) { + assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${path.join(destLink, 'dir1')}'.`) + errThrown = true + } finally { + assert(errThrown) + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + } + }) + it('should copy the directory successfully when src is a subdir of resolved dest path', () => { const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src') const destLink = path.join(TEST_DIR, 'dest-symlink') @@ -351,10 +401,6 @@ function testSuccess (src, dest) { fs.copySync(src, dest) - 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') diff --git a/lib/copy-sync/__tests__/copy-sync-readonly-dir.test.js b/lib/copy-sync/__tests__/copy-sync-readonly-dir.test.js index b5648043..30ace539 100644 --- a/lib/copy-sync/__tests__/copy-sync-readonly-dir.test.js +++ b/lib/copy-sync/__tests__/copy-sync-readonly-dir.test.js @@ -2,9 +2,8 @@ // relevant: https://github.com/jprichardson/node-fs-extra/issues/599 -const fs = require(process.cwd()) const os = require('os') -const fse = require('../../') +const fs = require('../../') const path = require('path') const assert = require('assert') const klawSync = require('klaw-sync') @@ -22,12 +21,12 @@ const FILES = [ describe('+ copySync() - copy a readonly directory with content', () => { beforeEach(done => { TEST_DIR = path.join(os.tmpdir(), 'test', 'fs-extra', 'copy-readonly-dir') - fse.emptyDir(TEST_DIR, done) + fs.emptyDir(TEST_DIR, done) }) afterEach(done => { klawSync(TEST_DIR).forEach(data => fs.chmodSync(data.path, 0o777)) - fse.remove(TEST_DIR, done) + fs.remove(TEST_DIR, done) }) describe('> when src is readonly directory with content', () => { @@ -40,7 +39,7 @@ describe('+ copySync() - copy a readonly directory with content', () => { sourceHierarchy.forEach(source => fs.chmodSync(source.path, source.stats.isDirectory() ? 0o555 : 0o444)) const targetDir = path.join(TEST_DIR, 'target') - fse.copySync(sourceDir, targetDir) + fs.copySync(sourceDir, targetDir) // Make sure copy was made and mode was preserved assert(fs.existsSync(targetDir)) diff --git a/lib/copy-sync/__tests__/symlink.test.js b/lib/copy-sync/__tests__/symlink.test.js index 062ea0ef..1825eece 100644 --- a/lib/copy-sync/__tests__/symlink.test.js +++ b/lib/copy-sync/__tests__/symlink.test.js @@ -1,8 +1,7 @@ 'use strict' -const fs = require('fs') const os = require('os') -const fse = require(process.cwd()) +const fs = require('../../') const path = require('path') const assert = require('assert') const copySync = require('../copy-sync') @@ -15,14 +14,14 @@ describe('copy-sync / symlink', () => { const out = path.join(TEST_DIR, 'out') beforeEach(done => { - fse.emptyDir(TEST_DIR, err => { + fs.emptyDir(TEST_DIR, err => { assert.ifError(err) createFixtures(src, done) }) }) afterEach(done => { - fse.remove(TEST_DIR, done) + fs.remove(TEST_DIR, done) }) it('copies symlinks by default', () => { diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 14ad9939..fd86a67f 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -22,7 +22,8 @@ function copySync (src, dest, opts) { see https://github.com/jprichardson/node-fs-extra/issues/269`) } - const destStat = checkPaths(src, dest) + const {srcStat, destStat} = checkPaths(src, dest) + checkParentStats(src, srcStat, dest) if (opts.filter && !opts.filter(src, dest)) return @@ -114,7 +115,7 @@ function copyDir (src, dest, opts) { function copyDirItem (item, src, dest, opts) { const srcItem = path.join(src, item) const destItem = path.join(dest, item) - const destStat = checkPaths(srcItem, destItem) + const {destStat} = checkPaths(srcItem, destItem) return startCopy(destStat, srcItem, destItem, opts) } @@ -162,9 +163,9 @@ function copyLink (resolvedSrc, dest) { // return true if dest is a subdir of src, otherwise false. function isSrcSubdir (src, dest) { - const srcArray = path.resolve(src).split(path.sep) - const destArray = path.resolve(dest).split(path.sep) - return srcArray.reduce((acc, current, i) => acc && destArray[i] === current, true) + const srcArr = path.resolve(src).split(path.sep).filter(i => i) + const destArr = path.resolve(dest).split(path.sep).filter(i => i) + return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) } function checkStats (src, dest) { @@ -179,6 +180,26 @@ function checkStats (src, dest) { return {srcStat, destStat} } +// recursively check if dest parent is a subdirectory of src. +// It works for all file types including symlinks since it +// checks the src and dest inodes. It starts from the deepest +// parent and stops once it reaches the src parent or the root path. +function checkParentStats (src, srcStat, dest) { + const destParent = path.dirname(dest) + if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return + let destStat + try { + destStat = fs.statSync(destParent) + } catch (err) { + if (err.code === 'ENOENT') return + throw err + } + if (destStat.ino && destStat.ino === srcStat.ino) { + throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + } + return checkParentStats(src, srcStat, destParent) +} + function checkPaths (src, dest) { const {srcStat, destStat} = checkStats(src, dest) if (destStat.ino && destStat.ino === srcStat.ino) { @@ -187,7 +208,7 @@ function checkPaths (src, dest) { if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) } - return destStat + return {srcStat, destStat} } module.exports = copySync diff --git a/lib/copy/__tests__/copy-case-insensitive-paths.test.js b/lib/copy/__tests__/copy-case-insensitive-paths.test.js index 9c581324..502616e2 100644 --- a/lib/copy/__tests__/copy-case-insensitive-paths.test.js +++ b/lib/copy/__tests__/copy-case-insensitive-paths.test.js @@ -3,7 +3,10 @@ const assert = require('assert') const os = require('os') const path = require('path') -const fs = require(process.cwd()) +const fs = require('../../') +const platform = os.platform() + +console.log('platform: ' + platform) /* global beforeEach, afterEach, describe, it */ @@ -26,15 +29,13 @@ describe('+ copy() - case insensitive paths', () => { dest = path.join(TEST_DIR, 'srcDir') fs.copy(src, dest, err => { - if (os === 'linux') { + if (platform === 'linux') { assert.ifError(err) assert(fs.existsSync(dest)) assert.strictEqual(fs.readFileSync(path.join(dest, 'subdir', 'file.txt'), 'utf8'), 'some data') } - if (os === 'darwin' || os === 'win32') { + if (platform === 'darwin' || platform === 'win32') { assert.strictEqual(err.message, 'Source and destination must not be the same.') - assert(fs.existsSync(src)) - assert(!fs.existsSync(dest)) } done() }) @@ -48,15 +49,13 @@ describe('+ copy() - case insensitive paths', () => { dest = path.join(TEST_DIR, 'srcFile') fs.copy(src, dest, err => { - if (os === 'linux') { + if (platform === 'linux') { assert.ifError(err) assert(fs.existsSync(dest)) assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'some data') } - if (os === 'darwin' || os === 'win32') { + if (platform === 'darwin' || platform === 'win32') { assert.strictEqual(err.message, 'Source and destination must not be the same.') - assert(fs.existsSync(src)) - assert(!fs.existsSync(dest)) } done() }) @@ -69,20 +68,18 @@ describe('+ copy() - case insensitive paths', () => { fs.outputFileSync(path.join(src, 'subdir', 'file.txt'), 'some data') const srcLink = path.join(TEST_DIR, 'src-symlink') fs.symlinkSync(src, srcLink, 'dir') - dest = path.join(TEST_DIR, 'srcDir') + dest = path.join(TEST_DIR, 'src-Symlink') - fs.copy(src, dest, err => { - if (os === 'linux') { + fs.copy(srcLink, dest, err => { + if (platform === 'linux') { assert.ifError(err) assert(fs.existsSync(dest)) assert.strictEqual(fs.readFileSync(path.join(dest, 'subdir', 'file.txt'), 'utf8'), 'some data') - const link = fs.readlinkSync(srcLink) - assert.strictEqual(link, dest) + const destLink = fs.readlinkSync(dest) + assert.strictEqual(destLink, src) } - if (os === 'darwin' || os === 'win32') { + if (platform === 'darwin' || platform === 'win32') { assert.strictEqual(err.message, 'Source and destination must not be the same.') - assert(fs.existsSync(src)) - assert(!fs.existsSync(dest)) } done() }) @@ -93,20 +90,18 @@ describe('+ copy() - case insensitive paths', () => { fs.outputFileSync(src, 'some data') const srcLink = path.join(TEST_DIR, 'src-symlink') fs.symlinkSync(src, srcLink, 'file') - dest = path.join(TEST_DIR, 'srcFile') + dest = path.join(TEST_DIR, 'src-Symlink') - fs.copy(src, dest, err => { - if (os === 'linux') { + fs.copy(srcLink, dest, err => { + if (platform === 'linux') { assert.ifError(err) assert(fs.existsSync(dest)) assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'some data') - const link = fs.readlinkSync(srcLink) - assert.strictEqual(link, dest) + const destLink = fs.readlinkSync(dest) + assert.strictEqual(destLink, src) } - if (os === 'darwin' || os === 'win32') { + if (platform === 'darwin' || platform === 'win32') { assert.strictEqual(err.message, 'Source and destination must not be the same.') - assert(fs.existsSync(src)) - assert(!fs.existsSync(dest)) } done() }) diff --git a/lib/copy/__tests__/copy-gh-89.test.js b/lib/copy/__tests__/copy-gh-89.test.js index 339fc567..537dae8f 100644 --- a/lib/copy/__tests__/copy-gh-89.test.js +++ b/lib/copy/__tests__/copy-gh-89.test.js @@ -5,7 +5,7 @@ const fs = require('fs') const os = require('os') -const fse = require(process.cwd()) +const fse = require('../../') const path = require('path') const assert = require('assert') diff --git a/lib/copy/__tests__/copy-preserve-time.test.js b/lib/copy/__tests__/copy-preserve-time.test.js index ae83ab4c..8c10692c 100644 --- a/lib/copy/__tests__/copy-preserve-time.test.js +++ b/lib/copy/__tests__/copy-preserve-time.test.js @@ -1,6 +1,6 @@ 'use strict' -const fs = require(process.cwd()) +const fs = require('../../') const os = require('os') const path = require('path') const copy = require('../copy') diff --git a/lib/copy/__tests__/copy-prevent-copying-identical.test.js b/lib/copy/__tests__/copy-prevent-copying-identical.test.js index b2aa211b..d30ec91b 100644 --- a/lib/copy/__tests__/copy-prevent-copying-identical.test.js +++ b/lib/copy/__tests__/copy-prevent-copying-identical.test.js @@ -3,7 +3,7 @@ const assert = require('assert') const os = require('os') const path = require('path') -const fs = require(process.cwd()) +const fs = require('../../') const klawSync = require('klaw-sync') /* global beforeEach, afterEach, describe, it */ 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 a3b944f7..552cbded 100644 --- a/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js +++ b/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js @@ -3,7 +3,7 @@ const assert = require('assert') const os = require('os') const path = require('path') -const fs = require(process.cwd()) +const fs = require('../../') const klawSync = require('klaw-sync') /* global beforeEach, afterEach, describe, it */ @@ -168,6 +168,48 @@ describe('+ copy() - prevent copying into itself', () => { }) }) + it('should error when dest is a subdirectory of src (bind-mounted directory with subdirectory)', done => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) + + const dest = path.join(destLink, 'dir1') + assert(fs.existsSync(dest)) + fs.copy(src, dest, err => { + assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + done() + }) + }) + + it('should error when dest is a subdirectory of src (more than one level depth)', done => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) + + const dest = path.join(destLink, 'dir1', 'dir2') + assert(fs.existsSync(dest)) + fs.copy(src, dest, err => { + assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${path.join(destLink, 'dir1')}'.`) + + 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') @@ -350,10 +392,6 @@ function testSuccess (src, dest, done) { 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') diff --git a/lib/copy/__tests__/copy-readonly-dir.test.js b/lib/copy/__tests__/copy-readonly-dir.test.js index 99a3d9af..7a21f4aa 100644 --- a/lib/copy/__tests__/copy-readonly-dir.test.js +++ b/lib/copy/__tests__/copy-readonly-dir.test.js @@ -2,7 +2,7 @@ // relevant: https://github.com/jprichardson/node-fs-extra/issues/599 -const fs = require(process.cwd()) +const fs = require('../../') const os = require('os') const fse = require('../../') const path = require('path') diff --git a/lib/copy/__tests__/copy.test.js b/lib/copy/__tests__/copy.test.js index 23d80863..9c1ce7f8 100644 --- a/lib/copy/__tests__/copy.test.js +++ b/lib/copy/__tests__/copy.test.js @@ -144,7 +144,9 @@ describe('fs-extra', () => { const srcTarget = path.join(TEST_DIR, 'destination') fse.mkdirSync(src) fse.mkdirSync(srcTarget) - fse.symlinkSync(srcTarget, path.join(src, 'symlink')) + // symlink type is only used for Windows and the default is 'file'. + // https://nodejs.org/api/fs.html#fs_fs_symlink_target_path_type_callback + fse.symlinkSync(srcTarget, path.join(src, 'symlink'), 'dir') fse.copy(src, dest, err => { assert.ifError(err) @@ -340,11 +342,12 @@ describe('fs-extra', () => { const dest = path.join(TEST_DIR, 'dest') - fse.copySync(src, dest, filter) - - assert(!fs.existsSync(path.join(dest, IGNORE)), 'directory was not ignored') - assert(!fs.existsSync(path.join(dest, IGNORE, 'file')), 'file was not ignored') - done() + fse.copy(src, dest, filter, err => { + assert.ifError(err) + assert(!fs.existsSync(path.join(dest, IGNORE)), 'directory was not ignored') + assert(!fs.existsSync(path.join(dest, IGNORE, 'file')), 'file was not ignored') + done() + }) }) it('should apply filter when it is applied only to dest', done => { diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 3dfbc540..5890f7ce 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -28,10 +28,14 @@ function copy (src, dest, opts, cb) { see https://github.com/jprichardson/node-fs-extra/issues/269`) } - checkPaths(src, dest, (err, destStat) => { + checkPaths(src, dest, (err, stats) => { if (err) return cb(err) - if (opts.filter) return handleFilter(checkParentDir, destStat, src, dest, opts, cb) - return checkParentDir(destStat, src, dest, opts, cb) + const {srcStat, destStat} = stats + checkParentStats(src, srcStat, dest, err => { + if (err) return cb(err) + if (opts.filter) return handleFilter(checkParentDir, destStat, src, dest, opts, cb) + return checkParentDir(destStat, src, dest, opts, cb) + }) }) } @@ -155,8 +159,9 @@ function copyDirItems (items, src, dest, opts, cb) { function copyDirItem (items, item, src, dest, opts, cb) { const srcItem = path.join(src, item) const destItem = path.join(dest, item) - checkPaths(srcItem, destItem, (err, destStat) => { + checkPaths(srcItem, destItem, (err, stats) => { if (err) return cb(err) + const {destStat} = stats startCopy(destStat, srcItem, destItem, opts, err => { if (err) return cb(err) return copyDirItems(items, src, dest, opts, cb) @@ -211,9 +216,9 @@ function copyLink (resolvedSrc, dest, cb) { // return true if dest is a subdir of src, otherwise false. function isSrcSubdir (src, dest) { - const srcArray = path.resolve(src).split(path.sep) - const destArray = path.resolve(dest).split(path.sep) - return srcArray.reduce((acc, current, i) => acc && destArray[i] === current, true) + const srcArr = path.resolve(src).split(path.sep).filter(i => i) + const destArr = path.resolve(dest).split(path.sep).filter(i => i) + return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) } function checkStats (src, dest, cb) { @@ -229,6 +234,25 @@ function checkStats (src, dest, cb) { }) } +// recursively check if dest parent is a subdirectory of src. +// It works for all file types including symlinks since it +// checks the src and dest inodes. It starts from the deepest +// parent and stops once it reaches the src parent or the root path. +function checkParentStats (src, srcStat, dest, cb) { + const destParent = path.dirname(dest) + if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return cb() + fs.stat(destParent, (err, destStat) => { + if (err) { + if (err.code === 'ENOENT') return cb() + return cb(err) + } + if (destStat.ino && destStat.ino === srcStat.ino) { + return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) + } + return checkParentStats(src, srcStat, destParent, cb) + }) +} + function checkPaths (src, dest, cb) { checkStats(src, dest, (err, stats) => { if (err) return cb(err) @@ -239,7 +263,7 @@ function checkPaths (src, dest, cb) { if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) } - return cb(null, destStat) + return cb(null, {srcStat, destStat}) }) } From f9e25195f302e00499490ef7b3a99d44b5962abb Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sun, 2 Sep 2018 12:06:35 -0700 Subject: [PATCH 05/19] move*(): check paths before moving --- lib/copy-sync/copy-sync.js | 1 + lib/copy/copy.js | 1 + ...ve-sync-prevent-moving-into-itself.test.js | 391 ++++++++++++++---- lib/move-sync/__tests__/move-sync.test.js | 10 +- lib/move-sync/index.js | 59 ++- .../move-prevent-moving-into-itself.test.js | 319 +++++++++++--- lib/move/__tests__/move.test.js | 6 +- lib/move/index.js | 75 +++- 8 files changed, 678 insertions(+), 184 deletions(-) diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index fd86a67f..49e3a532 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -162,6 +162,7 @@ function copyLink (resolvedSrc, dest) { } // return true if dest is a subdir of src, otherwise false. +// It only checks the path strings. function isSrcSubdir (src, dest) { const srcArr = path.resolve(src).split(path.sep).filter(i => i) const destArr = path.resolve(dest).split(path.sep).filter(i => i) diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 5890f7ce..a6ddcf5c 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -215,6 +215,7 @@ function copyLink (resolvedSrc, dest, cb) { } // return true if dest is a subdir of src, otherwise false. +// It only checks the path strings. function isSrcSubdir (src, dest) { const srcArr = path.resolve(src).split(path.sep).filter(i => i) const destArr = path.resolve(dest).split(path.sep).filter(i => i) diff --git a/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js b/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js index 1bf32c05..2cdfadfd 100644 --- a/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js +++ b/lib/move-sync/__tests__/move-sync-prevent-moving-into-itself.test.js @@ -3,11 +3,12 @@ const assert = require('assert') const os = require('os') const path = require('path') -const fs = require(process.cwd()) +const fs = require('../../') 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'), @@ -21,20 +22,35 @@ const dat2 = 'file2' const dat3 = 'file3' describe('+ moveSync() - prevent moving into itself', () => { - let TEST_DIR, src, dest + let TEST_DIR, src - beforeEach(() => { + beforeEach(done => { TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'move-sync-prevent-moving-into-itself') src = path.join(TEST_DIR, 'src') - fs.mkdirsSync(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(() => fs.removeSync(TEST_DIR)) + afterEach(done => fs.remove(TEST_DIR, done)) + + describe('> when source is a file', () => { + it('should move the file successfully even if dest parent is a subdir of src', () => { + const srcFile = path.join(TEST_DIR, 'src', 'srcfile.txt') + const destFile = path.join(TEST_DIR, 'src', 'dest', 'destfile.txt') + fs.writeFileSync(srcFile, dat0) + + fs.moveSync(srcFile, destFile) + + assert(fs.existsSync(destFile)) + const out = fs.readFileSync(destFile, 'utf8') + assert.strictEqual(out, dat0, 'file contents matched') + }) + }) describe('> when source is a file', () => { it(`should move the file successfully even when dest parent is 'src/dest'`, () => { @@ -64,82 +80,276 @@ describe('+ moveSync() - prevent moving into itself', () => { }) describe('> when source is a directory', () => { - it(`should move the directory successfully when dest is 'src_dest'`, () => { - dest = path.join(TEST_DIR, 'src_dest') - return testSuccessDir(src, dest) - }) - - it(`should move the directory successfully when dest is 'src-dest'`, () => { - dest = path.join(TEST_DIR, 'src-dest') - return testSuccessDir(src, dest) - }) - - it(`should move the directory successfully when dest is 'dest_src'`, () => { - dest = path.join(TEST_DIR, 'dest_src') - return testSuccessDir(src, dest) - }) - - it(`should move the directory successfully when dest is 'src_dest/src'`, () => { - dest = path.join(TEST_DIR, 'src_dest', 'src') - return testSuccessDir(src, dest) - }) - - it(`should move the directory successfully when dest is 'src-dest/src'`, () => { - dest = path.join(TEST_DIR, 'src-dest', 'src') - return testSuccessDir(src, dest) - }) - - it(`should move the directory successfully when dest is 'dest_src/src'`, () => { - dest = path.join(TEST_DIR, 'dest_src', 'src') - return testSuccessDir(src, dest) + describe('>> when dest is a directory', () => { + it(`of not itself`, () => { + const dest = path.join(TEST_DIR, src.replace(/^\w:/, '')) + return testSuccessDir(src, dest) + }) + it(`of itself`, () => { + const dest = path.join(src, 'dest') + return testError(src, dest) + }) + it(`should move the directory successfully when dest is 'src_dest'`, () => { + const dest = path.join(TEST_DIR, 'src_dest') + return testSuccessDir(src, dest) + }) + it(`should move the directory successfully when dest is 'src-dest'`, () => { + const dest = path.join(TEST_DIR, 'src-dest') + return testSuccessDir(src, dest) + }) + + it(`should move the directory successfully when dest is 'dest_src'`, () => { + const dest = path.join(TEST_DIR, 'dest_src') + return testSuccessDir(src, dest) + }) + + it(`should move the directory successfully when dest is 'src_dest/src'`, () => { + const dest = path.join(TEST_DIR, 'src_dest', 'src') + return testSuccessDir(src, dest) + }) + + it(`should move the directory successfully when dest is 'src-dest/src'`, () => { + const dest = path.join(TEST_DIR, 'src-dest', 'src') + return testSuccessDir(src, dest) + }) + + it(`should move the directory successfully when dest is 'dest_src/src'`, () => { + const dest = path.join(TEST_DIR, 'dest_src', 'src') + return testSuccessDir(src, dest) + }) + + it(`should move the directory successfully when dest is 'src_src/dest'`, () => { + const dest = path.join(TEST_DIR, 'src_src', 'dest') + return testSuccessDir(src, dest) + }) + + it(`should move the directory successfully when dest is 'src-src/dest'`, () => { + const dest = path.join(TEST_DIR, 'src-src', 'dest') + return testSuccessDir(src, dest) + }) + + it(`should move the directory successfully when dest is 'srcsrc/dest'`, () => { + const dest = path.join(TEST_DIR, 'srcsrc', 'dest') + return testSuccessDir(src, dest) + }) + + it(`should move the directory successfully when dest is 'dest/src'`, () => { + const dest = path.join(TEST_DIR, 'dest', 'src') + return testSuccessDir(src, dest) + }) + + it('should move the directory successfully when dest is very nested that all its parents need to be created', () => { + 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 testSuccessDir(src, dest) + }) + + it(`should error when dest is 'src/dest'`, () => { + const dest = path.join(TEST_DIR, 'src', 'dest') + return testError(src, dest) + }) + + it(`should error when dest is 'src/src_dest'`, () => { + const dest = path.join(TEST_DIR, 'src', 'src_dest') + return testError(src, dest) + }) + + it(`should error when dest is 'src/dest_src'`, () => { + const dest = path.join(TEST_DIR, 'src', 'dest_src') + return testError(src, dest) + }) + + it(`should error when dest is 'src/dest/src'`, () => { + const dest = path.join(TEST_DIR, 'src', 'dest', 'src') + return testError(src, dest) + }) }) - it(`should move the directory successfully when dest is 'src_src/dest'`, () => { - dest = path.join(TEST_DIR, 'src_src', 'dest') - return testSuccessDir(src, dest) - }) - - it(`should move the directory successfully when dest is 'src-src/dest'`, () => { - dest = path.join(TEST_DIR, 'src-src', 'dest') - return testSuccessDir(src, dest) - }) - - it(`should move the directory successfully when dest is 'srcsrc/dest'`, () => { - dest = path.join(TEST_DIR, 'srcsrc', 'dest') - return testSuccessDir(src, dest) - }) - - it(`should move the directory successfully when dest is 'dest/src'`, () => { - dest = path.join(TEST_DIR, 'dest', 'src') - return testSuccessDir(src, dest) - }) - - it('should move the directory successfully when dest is very nested that all its parents need to be created', () => { - 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') - assert(!fs.existsSync(dest)) - return testSuccessDir(src, dest) - }) - - it(`should throw error when dest is 'src/dest'`, () => { - dest = path.join(TEST_DIR, 'src', 'dest') - return testError(src, dest) - }) - - it(`should throw error when dest is 'src/src_dest'`, () => { - dest = path.join(TEST_DIR, 'src', 'src_dest') - return testError(src, dest) + describe('>> when dest is a symlink', () => { + it('should error when dest points exactly to src', () => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) + let errThrown = false + try { + fs.moveSync(src, destLink) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + errThrown = true + } finally { + assert(errThrown) + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + } + }) + + it('should error when dest is a subdirectory of src (bind-mounted directory with subdirectory)', () => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) + + const dest = path.join(destLink, 'dir1') + assert(fs.existsSync(dest)) + let errThrown = false + try { + fs.moveSync(src, dest) + } catch (err) { + assert.strictEqual(err.message, `Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) + errThrown = true + } finally { + assert(errThrown) + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + } + }) + + it('should error when dest is a subdirectory of src (more than one level depth)', () => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) + + const dest = path.join(destLink, 'dir1', 'dir2') + assert(fs.existsSync(dest)) + let errThrown = false + try { + fs.moveSync(src, dest) + } catch (err) { + assert.strictEqual(err.message, `Cannot move '${src}' to a subdirectory of itself, '${path.join(destLink, 'dir1')}'.`) + errThrown = true + } finally { + assert(errThrown) + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + } + }) }) + }) - it(`should throw error when dest is 'src/dest_src'`, () => { - dest = path.join(TEST_DIR, 'src', 'dest_src') - return testError(src, dest) + describe('> when source is a symlink', () => { + describe('>> when dest is a directory', () => { + it('should error when resolved src path points to dest', () => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + + const dest = path.join(TEST_DIR, 'src') + let errThrown = false + try { + fs.moveSync(srcLink, dest) + } catch (err) { + assert(err) + errThrown = true + } finally { + assert(errThrown) + // assert source not affected + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, src) + } + }) + + it('should error when dest is a subdir of resolved src path', () => { + 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) + let errThrown = false + try { + fs.moveSync(srcLink, dest) + } catch (err) { + assert(err) + errThrown = true + } finally { + assert(errThrown) + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, src) + } + }) + + it('should error when resolved src path is a subdir of dest', () => { + 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') + + let errThrown = false + try { + fs.moveSync(srcLink, dest) + } catch (err) { + assert(err) + errThrown = true + } finally { + assert(errThrown) + } + }) + + it(`should move the directory successfully when dest is 'src_src/dest'`, () => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + + const dest = path.join(TEST_DIR, 'src_src', 'dest') + testSuccessDir(srcLink, dest) + const link = fs.readlinkSync(dest) + assert.strictEqual(link, src) + }) + + it(`should move the directory successfully when dest is 'srcsrc/dest'`, () => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + + const dest = path.join(TEST_DIR, 'srcsrc', 'dest') + testSuccessDir(srcLink, dest) + const link = fs.readlinkSync(dest) + assert.strictEqual(link, src) + }) }) - it(`should throw error when dest is 'src/dest/src'`, () => { - dest = path.join(TEST_DIR, 'src', 'dest', 'src') - return testError(src, dest) + describe('>> when dest is a symlink', () => { + it('should error when resolved dest path is exactly the same as resolved src path', () => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(srcLink).length + const destlenBefore = klawSync(destLink).length + assert(srclenBefore > 2) + assert(destlenBefore > 2) + let errThrown = false + try { + fs.moveSync(srcLink, destLink) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + errThrown = true + } finally { + assert(errThrown) + 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) + } + }) }) }) }) @@ -148,42 +358,43 @@ function testSuccessFile (src, destFile) { const srcFile = path.join(src, FILES[0]) fs.moveSync(srcFile, destFile) - - const o0 = fs.readFileSync(destFile, 'utf8') - assert.strictEqual(o0, dat0, 'file contents matched') + const f0 = fs.readFileSync(destFile, 'utf8') + assert.strictEqual(f0, dat0, 'file contents matched') assert(!fs.existsSync(srcFile)) } function testSuccessDir (src, dest) { const srclen = klawSync(src).length - // assert src has contents - assert(srclen > 2) - fs.moveSync(src, dest) + assert(srclen > 2) // assert src has contents + fs.moveSync(src, dest) const destlen = klawSync(dest).length - // assert src and dest length are the same assert.strictEqual(destlen, srclen, 'src and dest length should be equal') - 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') + const f0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8') + const f1 = fs.readFileSync(path.join(dest, FILES[1]), 'utf8') + const f2 = fs.readFileSync(path.join(dest, FILES[2]), 'utf8') + const f3 = 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') + assert.strictEqual(f0, dat0, 'file contents matched') + assert.strictEqual(f1, dat1, 'file contents matched') + assert.strictEqual(f2, dat2, 'file contents matched') + assert.strictEqual(f3, dat3, 'file contents matched') assert(!fs.existsSync(src)) } function testError (src, dest) { + let errThrown = false try { fs.moveSync(src, dest) } catch (err) { assert.strictEqual(err.message, `Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) assert(fs.existsSync(src)) assert(!fs.existsSync(dest)) + errThrown = true + } finally { + assert(errThrown) } } diff --git a/lib/move-sync/__tests__/move-sync.test.js b/lib/move-sync/__tests__/move-sync.test.js index 2139b8ce..486e61d5 100644 --- a/lib/move-sync/__tests__/move-sync.test.js +++ b/lib/move-sync/__tests__/move-sync.test.js @@ -46,7 +46,15 @@ describe('moveSync()', () => { const src = `${TEST_DIR}/a-file` const dest = `${TEST_DIR}/a-file` - fse.moveSync(src, dest) + let errThrown = false + try { + fse.moveSync(src, dest) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + errThrown = true + } finally { + assert(errThrown) + } // assert src not affected const contents = fs.readFileSync(src, 'utf8') diff --git a/lib/move-sync/index.js b/lib/move-sync/index.js index 8bfdbc14..12bef7bb 100644 --- a/lib/move-sync/index.js +++ b/lib/move-sync/index.js @@ -9,12 +9,10 @@ const mkdirpSync = require('../mkdirs').mkdirpSync function moveSync (src, dest, opts) { opts = opts || {} const overwrite = opts.overwrite || opts.clobber || false - src = path.resolve(src) - dest = path.resolve(dest) - if (src === dest) return fs.accessSync(src) - const st = fs.statSync(src) - if (st.isDirectory() && isSrcSubdir(src, dest)) throw new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) + const srcStat = checkPaths(src, dest) + checkParentStats(src, srcStat, dest) + mkdirpSync(path.dirname(dest)) return doRename(src, dest, overwrite) } @@ -47,10 +45,55 @@ function moveAcrossDevice (src, dest, overwrite) { return removeSync(src) } +// return true if dest is a subdir of src, otherwise false. +// It only checks the path strings. function isSrcSubdir (src, dest) { - const srcArr = src.split(path.sep) - const destArr = dest.split(path.sep) - return srcArr.reduce((acc, curr, i) => acc && destArr[i] === curr, true) + const srcArr = path.resolve(src).split(path.sep).filter(i => i) + const destArr = path.resolve(dest).split(path.sep).filter(i => i) + return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) +} + +function checkStats (src, dest) { + const srcStat = fs.statSync(src) + let destStat + try { + destStat = fs.statSync(dest) + } catch (err) { + if (err.code === 'ENOENT') return {srcStat} + throw err + } + return {srcStat, destStat} +} + +// recursively check if dest parent is a subdirectory of src. +// It works for all file types including symlinks since it +// checks the src and dest inodes. It starts from the deepest +// parent and stops once it reaches the src parent or the root path. +function checkParentStats (src, srcStat, dest) { + const destParent = path.dirname(dest) + if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return + let destStat + try { + destStat = fs.statSync(destParent) + } catch (err) { + if (err.code === 'ENOENT') return + throw err + } + if (destStat.ino && destStat.ino === srcStat.ino) { + throw new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) + } + return checkParentStats(src, srcStat, destParent) +} + +function checkPaths (src, dest) { + const {srcStat, destStat} = checkStats(src, dest) + if (destStat && destStat.ino && destStat.ino === srcStat.ino) { + throw new Error('Source and destination must not be the same.') + } + if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { + throw new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) + } + return srcStat } module.exports = { moveSync } diff --git a/lib/move/__tests__/move-prevent-moving-into-itself.test.js b/lib/move/__tests__/move-prevent-moving-into-itself.test.js index 4b98fc6f..38ac398a 100644 --- a/lib/move/__tests__/move-prevent-moving-into-itself.test.js +++ b/lib/move/__tests__/move-prevent-moving-into-itself.test.js @@ -3,11 +3,12 @@ const assert = require('assert') const os = require('os') const path = require('path') -const fs = require(process.cwd()) +const fs = require('../../') 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'), @@ -21,9 +22,9 @@ const dat2 = 'file2' const dat3 = 'file3' describe('+ move() - prevent moving into itself', () => { - let TEST_DIR, src, dest + let TEST_DIR, src - beforeEach(() => { + beforeEach(done => { TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'move-prevent-moving-into-itself') src = path.join(TEST_DIR, 'src') fs.mkdirpSync(src) @@ -32,9 +33,27 @@ describe('+ move() - prevent moving into itself', () => { 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(() => fs.removeSync(TEST_DIR)) + afterEach(done => fs.remove(TEST_DIR, done)) + + describe('> when source is a file', () => { + it('should move the file successfully even if dest parent is a subdir of src', done => { + const srcFile = path.join(TEST_DIR, 'src', 'srcfile.txt') + const destFile = path.join(TEST_DIR, 'src', 'dest', 'destfile.txt') + fs.writeFileSync(srcFile, dat0) + + fs.move(srcFile, destFile, err => { + assert.ifError(err) + + assert(fs.existsSync(destFile)) + const out = fs.readFileSync(destFile, 'utf8') + assert.strictEqual(out, dat0, 'file contents matched') + done() + }) + }) + }) describe('> when source is a file', () => { it(`should move the file successfully even when dest parent is 'src/dest'`, done => { @@ -64,82 +83,256 @@ describe('+ move() - prevent moving into itself', () => { }) describe('> when source is a directory', () => { - it(`should move the directory successfully when dest is 'src_dest'`, done => { - dest = path.join(TEST_DIR, 'src_dest') - return testSuccessDir(src, dest, done) + describe('>> when dest is a directory', () => { + it(`of not itself`, done => { + const dest = path.join(TEST_DIR, src.replace(/^\w:/, '')) + return testSuccessDir(src, dest, done) + }) + it(`of itself`, done => { + const dest = path.join(src, 'dest') + return testError(src, dest, done) + }) + it(`should move the directory successfully when dest is 'src_dest'`, done => { + const dest = path.join(TEST_DIR, 'src_dest') + return testSuccessDir(src, dest, done) + }) + it(`should move the directory successfully when dest is 'src-dest'`, done => { + const dest = path.join(TEST_DIR, 'src-dest') + return testSuccessDir(src, dest, done) + }) + + it(`should move the directory successfully when dest is 'dest_src'`, done => { + const dest = path.join(TEST_DIR, 'dest_src') + return testSuccessDir(src, dest, done) + }) + + it(`should move the directory successfully when dest is 'src_dest/src'`, done => { + const dest = path.join(TEST_DIR, 'src_dest', 'src') + return testSuccessDir(src, dest, done) + }) + + it(`should move the directory successfully when dest is 'src-dest/src'`, done => { + const dest = path.join(TEST_DIR, 'src-dest', 'src') + return testSuccessDir(src, dest, done) + }) + + it(`should move the directory successfully when dest is 'dest_src/src'`, done => { + const dest = path.join(TEST_DIR, 'dest_src', 'src') + return testSuccessDir(src, dest, done) + }) + + it(`should move the directory successfully when dest is 'src_src/dest'`, done => { + const dest = path.join(TEST_DIR, 'src_src', 'dest') + return testSuccessDir(src, dest, done) + }) + + it(`should move the directory successfully when dest is 'src-src/dest'`, done => { + const dest = path.join(TEST_DIR, 'src-src', 'dest') + return testSuccessDir(src, dest, done) + }) + + it(`should move the directory successfully when dest is 'srcsrc/dest'`, done => { + const dest = path.join(TEST_DIR, 'srcsrc', 'dest') + return testSuccessDir(src, dest, done) + }) + + it(`should move the directory successfully when dest is 'dest/src'`, done => { + const dest = path.join(TEST_DIR, 'dest', 'src') + return testSuccessDir(src, dest, done) + }) + + it('should move 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 testSuccessDir(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) + }) }) - it(`should move the directory successfully when dest is 'src-dest'`, done => { - dest = path.join(TEST_DIR, 'src-dest') - return testSuccessDir(src, dest, done) - }) + describe('>> when dest is a symlink', () => { + it('should error when dest points exactly to src', done => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') - it(`should move the directory successfully when dest is 'dest_src'`, done => { - dest = path.join(TEST_DIR, 'dest_src') - return testSuccessDir(src, dest, done) - }) + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) - it(`should move the directory successfully when dest is 'src_dest/src'`, done => { - dest = path.join(TEST_DIR, 'src_dest', 'src') - return testSuccessDir(src, dest, done) - }) + fs.move(src, destLink, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') - it(`should move the directory successfully when dest is 'src-dest/src'`, done => { - dest = path.join(TEST_DIR, 'src-dest', 'src') - return testSuccessDir(src, dest, done) - }) + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') - it(`should move the directory successfully when dest is 'dest_src/src'`, done => { - dest = path.join(TEST_DIR, 'dest_src', 'src') - return testSuccessDir(src, dest, done) - }) + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + done() + }) + }) - it(`should move the directory successfully when dest is 'src_src/dest'`, done => { - dest = path.join(TEST_DIR, 'src_src', 'dest') - return testSuccessDir(src, dest, done) - }) + it('should error when dest is a subdirectory of src (bind-mounted directory with subdirectory)', done => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') - it(`should move the directory successfully when dest is 'src-src/dest'`, done => { - dest = path.join(TEST_DIR, 'src-src', 'dest') - return testSuccessDir(src, dest, done) - }) + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) - it(`should move the directory successfully when dest is 'srcsrc/dest'`, done => { - dest = path.join(TEST_DIR, 'srcsrc', 'dest') - return testSuccessDir(src, dest, done) - }) + const dest = path.join(destLink, 'dir1') + assert(fs.existsSync(dest)) + fs.move(src, dest, err => { + assert.strictEqual(err.message, `Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) - it(`should move the directory successfully when dest is 'dest/src'`, done => { - dest = path.join(TEST_DIR, 'dest', 'src') - return testSuccessDir(src, dest, done) - }) + const srclenAfter = klawSync(src).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') - it('should move the directory successfully when dest is very nested that all its parents need to be created', done => { - 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') - assert(!fs.existsSync(dest)) - return testSuccessDir(src, dest, done) - }) + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + done() + }) + }) - it(`should return error when dest is 'src/dest'`, done => { - dest = path.join(TEST_DIR, 'src', 'dest') - return testError(src, dest, done) - }) + it('should error when dest is a subdirectory of src (more than one level depth)', done => { + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(src).length + assert(srclenBefore > 2) + + const dest = path.join(destLink, 'dir1', 'dir2') + assert(fs.existsSync(dest)) + fs.move(src, dest, err => { + assert.strictEqual(err.message, `Cannot move '${src}' to a subdirectory of itself, '${path.join(destLink, 'dir1')}'.`) - it(`should return error when dest is 'src/src_dest'`, done => { - dest = path.join(TEST_DIR, 'src', 'src_dest') - return testError(src, dest, done) + 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 return error when dest is 'src/dest_src'`, done => { - dest = path.join(TEST_DIR, 'src', 'dest_src') - return testError(src, dest, 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.move(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.move(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.move(srcLink, dest, err => { + assert(err) + done() + }) + }) + + it(`should move 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') + testSuccessDir(srcLink, dest, () => { + const link = fs.readlinkSync(dest) + assert.strictEqual(link, src) + done() + }) + }) + + it(`should move 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') + testSuccessDir(srcLink, dest, () => { + const link = fs.readlinkSync(dest) + assert.strictEqual(link, src) + done() + }) + }) }) - it(`should return error when dest is 'src/dest/src'`, done => { - dest = path.join(TEST_DIR, 'src', 'dest', 'src') - return testError(src, dest, done) + describe('>> when dest is a symlink', () => { + it('should error when resolved dest path is exactly the same as resolved src path', done => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'dir') + + const srclenBefore = klawSync(srcLink).length + const destlenBefore = klawSync(destLink).length + assert(srclenBefore > 2) + assert(destlenBefore > 2) + + fs.move(srcLink, destLink, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + + const srclenAfter = klawSync(srcLink).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + 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() + }) + }) }) }) }) diff --git a/lib/move/__tests__/move.test.js b/lib/move/__tests__/move.test.js index ef37efc3..23e8dfce 100644 --- a/lib/move/__tests__/move.test.js +++ b/lib/move/__tests__/move.test.js @@ -2,7 +2,7 @@ const fs = require('graceful-fs') const os = require('os') -const fse = require(process.cwd()) +const fse = require('../../') const path = require('path') const assert = require('assert') @@ -143,7 +143,7 @@ describe('+ move()', () => { const dest = src fse.move(src, dest, err => { - assert.ifError(err) + assert.strictEqual(err.message, 'Source and destination must not be the same.') done() }) }) @@ -163,7 +163,7 @@ describe('+ move()', () => { const dest = src fse.move(src, dest, err => { - assert.ifError(err) + assert.strictEqual(err.message, 'Source and destination must not be the same.') done() }) }) diff --git a/lib/move/index.js b/lib/move/index.js index 68947f05..04c7c68a 100644 --- a/lib/move/index.js +++ b/lib/move/index.js @@ -16,20 +16,14 @@ function move (src, dest, opts, cb) { const overwrite = opts.overwrite || opts.clobber || false - src = path.resolve(src) - dest = path.resolve(dest) - - if (src === dest) return fs.access(src, cb) - - fs.stat(src, (err, st) => { + checkPaths(src, dest, (err, srcStat) => { if (err) return cb(err) - - if (st.isDirectory() && isSrcSubdir(src, dest)) { - return cb(new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`)) - } - mkdirp(path.dirname(dest), err => { + checkParentStats(src, srcStat, dest, err => { if (err) return cb(err) - return doRename(src, dest, overwrite, cb) + mkdirp(path.dirname(dest), err => { + if (err) return cb(err) + return doRename(src, dest, overwrite, cb) + }) }) }) } @@ -68,15 +62,58 @@ function moveAcrossDevice (src, dest, overwrite, cb) { }) } +// return true if dest is a subdir of src, otherwise false. +// It only checks the path strings. function isSrcSubdir (src, dest) { - const srcArray = src.split(path.sep) - const destArray = dest.split(path.sep) + const srcArr = path.resolve(src).split(path.sep).filter(i => i) + const destArr = path.resolve(dest).split(path.sep).filter(i => i) + return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) +} - return srcArray.reduce((acc, current, i) => { - return acc && destArray[i] === current - }, true) +function checkStats (src, dest, cb) { + fs.stat(src, (err, srcStat) => { + if (err) return cb(err) + fs.stat(dest, (err, destStat) => { + if (err) { + if (err.code === 'ENOENT') return cb(null, {srcStat}) + return cb(err) + } + return cb(null, {srcStat, destStat}) + }) + }) } -module.exports = { - move: u(move) +// recursively check if dest parent is a subdirectory of src. +// It works for all file types including symlinks since it +// checks the src and dest inodes. It starts from the deepest +// parent and stops once it reaches the src parent or the root path. +function checkParentStats (src, srcStat, dest, cb) { + const destParent = path.dirname(dest) + if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return cb() + fs.stat(destParent, (err, destStat) => { + if (err) { + if (err.code === 'ENOENT') return cb() + return cb(err) + } + if (destStat.ino && destStat.ino === srcStat.ino) { + return cb(new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`)) + } + return checkParentStats(src, srcStat, destParent, cb) + }) +} + +function checkPaths (src, dest, cb) { + checkStats(src, dest, (err, stats) => { + if (err) return cb(err) + const {srcStat, destStat} = stats + if (destStat && destStat.ino && destStat.ino === srcStat.ino) { + return cb(new Error('Source and destination must not be the same.')) + } + if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { + return cb(new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`)) + } + return cb(null, srcStat) + }) } + +module.exports = { move: u(move) } From 5ea17f2336fc762603477d84aabbba353c808428 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sun, 2 Sep 2018 12:32:37 -0700 Subject: [PATCH 06/19] move*(): add case-insensitive paths test --- .../move-sync-case-insensitive-paths.test.js | 124 ++++++++++++++++++ .../move-case-insensitive-paths.test.js | 108 +++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 lib/move-sync/__tests__/move-sync-case-insensitive-paths.test.js create mode 100644 lib/move/__tests__/move-case-insensitive-paths.test.js diff --git a/lib/move-sync/__tests__/move-sync-case-insensitive-paths.test.js b/lib/move-sync/__tests__/move-sync-case-insensitive-paths.test.js new file mode 100644 index 00000000..32600b88 --- /dev/null +++ b/lib/move-sync/__tests__/move-sync-case-insensitive-paths.test.js @@ -0,0 +1,124 @@ +'use strict' + +const assert = require('assert') +const os = require('os') +const path = require('path') +const fs = require('../../') +const platform = os.platform() + +/* global beforeEach, afterEach, describe, it */ + +describe('+ moveSync() - case insensitive paths', () => { + let TEST_DIR = '' + let src = '' + let dest = '' + + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'move-sync-case-insensitive-paths') + fs.emptyDir(TEST_DIR, done) + }) + + afterEach(() => fs.removeSync(TEST_DIR)) + + describe('> when src is a directory', () => { + it('should behave correctly based on the OS', () => { + src = path.join(TEST_DIR, 'srcdir') + fs.outputFileSync(path.join(src, 'subdir', 'file.txt'), 'some data') + dest = path.join(TEST_DIR, 'srcDir') + let errThrown = false + + try { + fs.moveSync(src, dest) + } catch (err) { + if (platform === 'darwin' || platform === 'win32') { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + errThrown = true + } + } + if (platform === 'darwin' || platform === 'win32') assert(errThrown) + if (platform === 'linux') { + assert(fs.existsSync(dest)) + assert.strictEqual(fs.readFileSync(path.join(dest, 'subdir', 'file.txt'), 'utf8'), 'some data') + assert(!errThrown) + } + }) + }) + + describe('> when src is a file', () => { + it('should behave correctly based on the OS', () => { + src = path.join(TEST_DIR, 'srcfile') + fs.outputFileSync(src, 'some data') + dest = path.join(TEST_DIR, 'srcFile') + let errThrown = false + + try { + fs.moveSync(src, dest) + } catch (err) { + if (platform === 'darwin' || platform === 'win32') { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + errThrown = true + } + } + if (platform === 'darwin' || platform === 'win32') assert(errThrown) + if (platform === 'linux') { + assert(fs.existsSync(dest)) + assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'some data') + assert(!errThrown) + } + }) + }) + + describe('> when src is a symlink', () => { + it('should behave correctly based on the OS, symlink dir', () => { + src = path.join(TEST_DIR, 'srcdir') + fs.outputFileSync(path.join(src, 'subdir', 'file.txt'), 'some data') + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + dest = path.join(TEST_DIR, 'src-Symlink') + let errThrown = false + + try { + fs.moveSync(srcLink, dest) + } catch (err) { + if (platform === 'darwin' || platform === 'win32') { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + errThrown = true + } + } + if (platform === 'darwin' || platform === 'win32') assert(errThrown) + if (platform === 'linux') { + assert(fs.existsSync(dest)) + assert.strictEqual(fs.readFileSync(path.join(dest, 'subdir', 'file.txt'), 'utf8'), 'some data') + const destLink = fs.readlinkSync(dest) + assert.strictEqual(destLink, src) + assert(!errThrown) + } + }) + + it('should behave correctly based on the OS, symlink file', () => { + src = path.join(TEST_DIR, 'srcfile') + fs.outputFileSync(src, 'some data') + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'file') + dest = path.join(TEST_DIR, 'src-Symlink') + let errThrown = false + + try { + fs.moveSync(srcLink, dest) + } catch (err) { + if (platform === 'darwin' || platform === 'win32') { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + errThrown = true + } + } + if (platform === 'darwin' || platform === 'win32') assert(errThrown) + if (platform === 'linux') { + assert(fs.existsSync(dest)) + assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'some data') + const destLink = fs.readlinkSync(dest) + assert.strictEqual(destLink, src) + assert(!errThrown) + } + }) + }) +}) diff --git a/lib/move/__tests__/move-case-insensitive-paths.test.js b/lib/move/__tests__/move-case-insensitive-paths.test.js new file mode 100644 index 00000000..30763249 --- /dev/null +++ b/lib/move/__tests__/move-case-insensitive-paths.test.js @@ -0,0 +1,108 @@ +'use strict' + +const assert = require('assert') +const os = require('os') +const path = require('path') +const fs = require('../../') +const platform = os.platform() + +/* global beforeEach, afterEach, describe, it */ + +describe('+ move() - case insensitive paths', () => { + let TEST_DIR = '' + let src = '' + let dest = '' + + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'move-case-insensitive-paths') + fs.emptyDir(TEST_DIR, done) + }) + + afterEach(done => fs.remove(TEST_DIR, done)) + + describe('> when src is a directory', () => { + it('should behave correctly based on the OS', done => { + src = path.join(TEST_DIR, 'srcdir') + fs.outputFileSync(path.join(src, 'subdir', 'file.txt'), 'some data') + dest = path.join(TEST_DIR, 'srcDir') + + fs.move(src, dest, err => { + if (platform === 'linux') { + assert.ifError(err) + assert(fs.existsSync(dest)) + assert.strictEqual(fs.readFileSync(path.join(dest, 'subdir', 'file.txt'), 'utf8'), 'some data') + } + if (platform === 'darwin' || platform === 'win32') { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } + done() + }) + }) + }) + + describe('> when src is a file', () => { + it('should behave correctly based on the OS', done => { + src = path.join(TEST_DIR, 'srcfile') + fs.outputFileSync(src, 'some data') + dest = path.join(TEST_DIR, 'srcFile') + + fs.move(src, dest, err => { + if (platform === 'linux') { + assert.ifError(err) + assert(fs.existsSync(dest)) + assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'some data') + } + if (platform === 'darwin' || platform === 'win32') { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } + done() + }) + }) + }) + + describe('> when src is a symlink', () => { + it('should behave correctly based on the OS, symlink dir', done => { + src = path.join(TEST_DIR, 'srcdir') + fs.outputFileSync(path.join(src, 'subdir', 'file.txt'), 'some data') + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + dest = path.join(TEST_DIR, 'src-Symlink') + + fs.move(srcLink, dest, err => { + if (platform === 'linux') { + assert.ifError(err) + assert(fs.existsSync(dest)) + assert.strictEqual(fs.readFileSync(path.join(dest, 'subdir', 'file.txt'), 'utf8'), 'some data') + const destLink = fs.readlinkSync(dest) + assert.strictEqual(destLink, src) + } + if (platform === 'darwin' || platform === 'win32') { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } + done() + }) + }) + + it('should behave correctly based on the OS, symlink file', done => { + src = path.join(TEST_DIR, 'srcfile') + fs.outputFileSync(src, 'some data') + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'file') + dest = path.join(TEST_DIR, 'src-Symlink') + + fs.move(srcLink, dest, err => { + if (platform === 'linux') { + assert.ifError(err) + assert(fs.existsSync(dest)) + assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'some data') + const destLink = fs.readlinkSync(dest) + assert.strictEqual(destLink, src) + } + if (platform === 'darwin' || platform === 'win32') { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } + done() + }) + }) + }) +}) From 7513e2671c9a66240e1ec36caf47ae21feb87a66 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sun, 2 Sep 2018 23:27:40 -0700 Subject: [PATCH 07/19] remove unnecessary done callback from test --- lib/move/__tests__/move-prevent-moving-into-itself.test.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/move/__tests__/move-prevent-moving-into-itself.test.js b/lib/move/__tests__/move-prevent-moving-into-itself.test.js index 38ac398a..fe13d095 100644 --- a/lib/move/__tests__/move-prevent-moving-into-itself.test.js +++ b/lib/move/__tests__/move-prevent-moving-into-itself.test.js @@ -24,7 +24,7 @@ const dat3 = 'file3' describe('+ move() - prevent moving into itself', () => { let TEST_DIR, src - beforeEach(done => { + beforeEach(() => { TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'move-prevent-moving-into-itself') src = path.join(TEST_DIR, 'src') fs.mkdirpSync(src) @@ -33,10 +33,9 @@ describe('+ move() - prevent moving into itself', () => { 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)) + afterEach(() => fs.removeSync(TEST_DIR)) describe('> when source is a file', () => { it('should move the file successfully even if dest parent is a subdir of src', done => { From 0c30f8c1e23a52af5be38d5a21bd788987b4b420 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sun, 31 Mar 2019 23:28:09 -0700 Subject: [PATCH 08/19] copy*(): add new option checkPathsBeforeCopying --- .../copy-sync-without-checking-paths.test.js | 296 ++++++++++++++++++ lib/copy-sync/copy-sync.js | 19 +- .../copy-case-insensitive-paths.test.js | 2 - lib/copy/copy.js | 28 +- package.json | 3 +- 5 files changed, 329 insertions(+), 19 deletions(-) create mode 100644 lib/copy-sync/__tests__/copy-sync-without-checking-paths.test.js diff --git a/lib/copy-sync/__tests__/copy-sync-without-checking-paths.test.js b/lib/copy-sync/__tests__/copy-sync-without-checking-paths.test.js new file mode 100644 index 00000000..459e5f92 --- /dev/null +++ b/lib/copy-sync/__tests__/copy-sync-without-checking-paths.test.js @@ -0,0 +1,296 @@ +'use strict' + +const assert = require('assert') +const os = require('os') +const path = require('path') +const fs = require('../../') +const klawSync = require('klaw-sync') + +/* global beforeEach, afterEach, describe, it */ + +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() - without checking paths', () => { + let TEST_DIR, src + + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-without-checking-paths') + 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 if dest parent is a subdir of src', () => { + const srcFile = path.join(TEST_DIR, 'src', 'srcfile.txt') + const destFile = path.join(TEST_DIR, 'src', 'dest', 'destfile.txt') + fs.writeFileSync(srcFile, dat0) + + fs.copySync(srcFile, destFile, { checkPathsBeforeCopying: false }) + + assert(fs.existsSync(destFile)) + const out = fs.readFileSync(destFile, 'utf8') + assert.strictEqual(out, dat0) + }) + }) + + // 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(`of not itself`, () => { + const dest = path.join(TEST_DIR, src.replace(/^\w:/, '')) + return testSuccess(src, dest) + }) + it(`should copy the directory successfully when dest is 'src_dest'`, () => { + const dest = path.join(TEST_DIR, 'src_dest') + return testSuccess(src, dest) + }) + it(`should copy the directory successfully when dest is 'src-dest'`, () => { + const dest = path.join(TEST_DIR, 'src-dest') + return testSuccess(src, dest) + }) + + it(`should copy the directory successfully when dest is 'dest_src'`, () => { + const dest = path.join(TEST_DIR, 'dest_src') + return testSuccess(src, dest) + }) + + it(`should copy the directory successfully when dest is 'src_dest/src'`, () => { + const dest = path.join(TEST_DIR, 'src_dest', 'src') + return testSuccess(src, dest) + }) + + it(`should copy the directory successfully when dest is 'src-dest/src'`, () => { + const dest = path.join(TEST_DIR, 'src-dest', 'src') + return testSuccess(src, dest) + }) + + it(`should copy the directory successfully when dest is 'dest_src/src'`, () => { + const dest = path.join(TEST_DIR, 'dest_src', 'src') + return testSuccess(src, dest) + }) + + it(`should copy the directory successfully when dest is 'src_src/dest'`, () => { + const dest = path.join(TEST_DIR, 'src_src', 'dest') + return testSuccess(src, dest) + }) + + it(`should copy the directory successfully when dest is 'src-src/dest'`, () => { + const dest = path.join(TEST_DIR, 'src-src', 'dest') + return testSuccess(src, dest) + }) + + it(`should copy the directory successfully when dest is 'srcsrc/dest'`, () => { + const dest = path.join(TEST_DIR, 'srcsrc', 'dest') + return testSuccess(src, dest) + }) + + it(`should copy the directory successfully when dest is 'dest/src'`, () => { + const dest = path.join(TEST_DIR, 'dest', 'src') + return testSuccess(src, dest) + }) + + it('should copy the directory successfully when dest is very nested that all its parents need to be created', () => { + 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) + }) + }) + + describe('>> when dest is a symlink', () => { + it('should copy the directory successfully when src is a subdir of resolved dest path', () => { + 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.copySync(srcInDest, destLink, { checkPathsBeforeCopying: false }) + + 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)))) + + 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) + assert.strictEqual(o1, dat1) + assert.strictEqual(o2, dat2) + assert.strictEqual(o3, dat3) + }) + }) + }) + + describe('> when source is a symlink', () => { + describe('>> when dest is a directory', () => { + it('should error when resolved src path points to dest', () => { + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(src, srcLink, 'dir') + const dest = path.join(TEST_DIR, 'src') + let errThrown + try { + fs.copySync(srcLink, dest, { checkPathsBeforeCopying: false }) + } catch (err) { + errThrown = err + } + assert(errThrown) + + // assert source not affected + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, src) + }) + + it('should error when dest is a subdir of resolved src path', () => { + 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) + + let errThrown + try { + fs.copySync(srcLink, dest, { checkPathsBeforeCopying: false }) + } catch (err) { + errThrown = err + } + assert(errThrown) + const link = fs.readlinkSync(srcLink) + assert.strictEqual(link, src) + }) + + it('should error when resolved src path is a subdir of dest', () => { + 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') + + let errThrown + try { + fs.copySync(srcLink, dest, { checkPathsBeforeCopying: false }) + } catch (err) { + errThrown = err + } + assert(errThrown) + }) + + it(`should copy the directory successfully when dest is 'src_src/dest'`, () => { + 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) + }) + + it(`should copy the directory successfully when dest is 'srcsrc/dest'`, () => { + 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) + }) + }) + + describe('>> when dest is a symlink', () => { + it('should error when resolved dest path is a subdir of resolved src path', () => { + 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') + + let errThrown + try { + fs.copySync(srcLink, destLink, { checkPathsBeforeCopying: false }) + } catch (err) { + errThrown = err + } + assert.strictEqual(errThrown.message, `Cannot copy '${src}' to a subdirectory of itself, '${resolvedDestPath}'.`) + const destln = fs.readlinkSync(destLink) + assert.strictEqual(destln, resolvedDestPath) + }) + + it('should error when resolved src path is a subdir of resolved dest path', () => { + const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src') + const srcLink = path.join(TEST_DIR, 'src-symlink') + const destLink = path.join(TEST_DIR, 'dest-symlink') + const dest = path.join(TEST_DIR, 'dest') + + fs.ensureDirSync(srcInDest) + fs.ensureSymlinkSync(srcInDest, srcLink, 'dir') + fs.ensureSymlinkSync(dest, destLink, 'dir') + + let errThrown + try { + fs.copySync(srcLink, destLink, { checkPathsBeforeCopying: false }) + } catch (err) { + errThrown = err + } + assert.strictEqual(errThrown.message, `Cannot overwrite '${dest}' with '${srcInDest}'.`) + const destln = fs.readlinkSync(destLink) + assert.strictEqual(destln, dest) + }) + }) + }) +}) + +function testSuccess (src, dest) { + const srclen = klawSync(src).length + assert(srclen > 2) + fs.copySync(src, dest, { checkPathsBeforeCopying: false }) + + FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)))) + + 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) + assert.strictEqual(o1, dat1) + assert.strictEqual(o2, dat2) + assert.strictEqual(o3, dat3) +} diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 49e3a532..d97438d3 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -22,11 +22,17 @@ function copySync (src, dest, opts) { see https://github.com/jprichardson/node-fs-extra/issues/269`) } - const {srcStat, destStat} = checkPaths(src, dest) - checkParentStats(src, srcStat, dest) + if (opts.checkPathsBeforeCopying === false) { + const { destStat } = checkStats(src, dest) + return handleFilterAndCopy(destStat, src, dest, opts) + } + const { srcStat, destStat } = checkPaths(src, dest) + checkParentPaths(src, srcStat, dest) + return handleFilterAndCopy(destStat, src, dest, opts) +} +function handleFilterAndCopy (destStat, src, dest, opts) { if (opts.filter && !opts.filter(src, dest)) return - const destParent = path.dirname(dest) if (!fs.existsSync(destParent)) mkdirpSync(destParent) return startCopy(destStat, src, dest, opts) @@ -115,7 +121,8 @@ function copyDir (src, dest, opts) { function copyDirItem (item, src, dest, opts) { const srcItem = path.join(src, item) const destItem = path.join(dest, item) - const {destStat} = checkPaths(srcItem, destItem) + const check = opts.checkPathsBeforeCopying === false ? checkStats : checkPaths + const { destStat } = check(srcItem, destItem) return startCopy(destStat, srcItem, destItem, opts) } @@ -185,7 +192,7 @@ function checkStats (src, dest) { // It works for all file types including symlinks since it // checks the src and dest inodes. It starts from the deepest // parent and stops once it reaches the src parent or the root path. -function checkParentStats (src, srcStat, dest) { +function checkParentPaths (src, srcStat, dest) { const destParent = path.dirname(dest) if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return let destStat @@ -198,7 +205,7 @@ function checkParentStats (src, srcStat, dest) { if (destStat.ino && destStat.ino === srcStat.ino) { throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) } - return checkParentStats(src, srcStat, destParent) + return checkParentPaths(src, srcStat, destParent) } function checkPaths (src, dest) { diff --git a/lib/copy/__tests__/copy-case-insensitive-paths.test.js b/lib/copy/__tests__/copy-case-insensitive-paths.test.js index 502616e2..7618510e 100644 --- a/lib/copy/__tests__/copy-case-insensitive-paths.test.js +++ b/lib/copy/__tests__/copy-case-insensitive-paths.test.js @@ -6,8 +6,6 @@ const path = require('path') const fs = require('../../') const platform = os.platform() -console.log('platform: ' + platform) - /* global beforeEach, afterEach, describe, it */ describe('+ copy() - case insensitive paths', () => { diff --git a/lib/copy/copy.js b/lib/copy/copy.js index a6ddcf5c..608bf496 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -28,15 +28,24 @@ function copy (src, dest, opts, cb) { see https://github.com/jprichardson/node-fs-extra/issues/269`) } - checkPaths(src, dest, (err, stats) => { - if (err) return cb(err) - const {srcStat, destStat} = stats - checkParentStats(src, srcStat, dest, err => { + if (opts.checkPathsBeforeCopying === false) { + checkStats(src, dest, (err, stats) => { if (err) return cb(err) + const { destStat } = stats if (opts.filter) return handleFilter(checkParentDir, destStat, src, dest, opts, cb) return checkParentDir(destStat, src, dest, opts, cb) }) - }) + } else { + checkPaths(src, dest, (err, stats) => { + if (err) return cb(err) + const {srcStat, destStat} = stats + checkParentPaths(src, srcStat, dest, err => { + if (err) return cb(err) + if (opts.filter) return handleFilter(checkParentDir, destStat, src, dest, opts, cb) + return checkParentDir(destStat, src, dest, opts, cb) + }) + }) + } } function checkParentDir (destStat, src, dest, opts, cb) { @@ -159,9 +168,10 @@ function copyDirItems (items, src, dest, opts, cb) { function copyDirItem (items, item, src, dest, opts, cb) { const srcItem = path.join(src, item) const destItem = path.join(dest, item) - checkPaths(srcItem, destItem, (err, stats) => { + const check = opts.checkPathsBeforeCopying === false ? checkStats : checkPaths + check(srcItem, destItem, (err, stats) => { if (err) return cb(err) - const {destStat} = stats + const { destStat } = stats startCopy(destStat, srcItem, destItem, opts, err => { if (err) return cb(err) return copyDirItems(items, src, dest, opts, cb) @@ -239,7 +249,7 @@ function checkStats (src, dest, cb) { // It works for all file types including symlinks since it // checks the src and dest inodes. It starts from the deepest // parent and stops once it reaches the src parent or the root path. -function checkParentStats (src, srcStat, dest, cb) { +function checkParentPaths (src, srcStat, dest, cb) { const destParent = path.dirname(dest) if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return cb() fs.stat(destParent, (err, destStat) => { @@ -250,7 +260,7 @@ function checkParentStats (src, srcStat, dest, cb) { if (destStat.ino && destStat.ino === srcStat.ino) { return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) } - return checkParentStats(src, srcStat, destParent, cb) + return checkParentPaths(src, srcStat, destParent, cb) }) } diff --git a/package.json b/package.json index e27acdb1..c584ce30 100644 --- a/package.json +++ b/package.json @@ -50,8 +50,7 @@ "proxyquire": "^2.0.1", "read-dir-files": "^0.1.1", "semver": "^5.3.0", - "standard": "^11.0.1", - "standard-markdown": "^4.0.1" + "standard": "^11.0.1" }, "main": "./lib/index.js", "scripts": { From 033d7cf8ec1ab67ee083323bb8b6fd82e3db785c Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Mon, 1 Apr 2019 00:31:31 -0700 Subject: [PATCH 09/19] update copy*() docs to include checkPathsBeforeCopying --- docs/copy-sync.md | 1 + docs/copy.md | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/copy-sync.md b/docs/copy-sync.md index 3fca1330..e7cc8b1e 100644 --- a/docs/copy-sync.md +++ b/docs/copy-sync.md @@ -7,6 +7,7 @@ Copy a file or directory. The directory can have contents. Like `cp -r`. - `options` `` - `overwrite` ``: overwrite existing file or directory, default is `true`. _Note that the copy operation will silently fail if you set this to `false` and the destination exists._ Use the `errorOnExist` option to change this behavior. - `errorOnExist` ``: when `overwrite` is `false` and the destination exists, throw an error. Default is `false`. + - `checkPathsBeforeCopying` ``: check `src` and `dest` paths before copying. Default is `true`. - `dereference` ``: dereference symlinks, default is `false`. - `preserveTimestamps` ``: When true, will set last modification and access times to the ones of the original source files. When false, timestamp behavior is OS-dependent. Default is `false`. - `filter` ``: Function to filter copied files. Return `true` to include, `false` to exclude. diff --git a/docs/copy.md b/docs/copy.md index 4add5684..a5ce8958 100644 --- a/docs/copy.md +++ b/docs/copy.md @@ -7,6 +7,7 @@ Copy a file or directory. The directory can have contents. Like `cp -r`. - `options` `` - `overwrite` ``: overwrite existing file or directory, default is `true`. _Note that the copy operation will silently fail if you set this to `false` and the destination exists._ Use the `errorOnExist` option to change this behavior. - `errorOnExist` ``: when `overwrite` is `false` and the destination exists, throw an error. Default is `false`. + - `checkPathsBeforeCopying` ``: check `src` and `dest` paths before copying. Default is `true`. - `dereference` ``: dereference symlinks, default is `false`. - `preserveTimestamps` ``: When true, will set last modification and access times to the ones of the original source files. When false, timestamp behavior is OS-dependent. Default is `false`. - `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). From bb30e7f1758c0388a8e4cddb48752e83bcb85e8b Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 4 Apr 2019 00:32:15 -0700 Subject: [PATCH 10/19] some reformatting --- lib/copy-sync/copy-sync.js | 8 ++++---- lib/copy/copy.js | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index d97438d3..001fd577 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -182,10 +182,10 @@ function checkStats (src, dest) { try { destStat = fs.statSync(dest) } catch (err) { - if (err.code === 'ENOENT') return {srcStat, destStat: notExist} + if (err.code === 'ENOENT') return { srcStat, destStat: notExist } throw err } - return {srcStat, destStat} + return { srcStat, destStat } } // recursively check if dest parent is a subdirectory of src. @@ -209,14 +209,14 @@ function checkParentPaths (src, srcStat, dest) { } function checkPaths (src, dest) { - const {srcStat, destStat} = checkStats(src, dest) + const { srcStat, destStat } = checkStats(src, dest) if (destStat.ino && destStat.ino === srcStat.ino) { throw new Error('Source and destination must not be the same.') } if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) } - return {srcStat, destStat} + return { srcStat, destStat } } module.exports = copySync diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 608bf496..a38e5f46 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -38,7 +38,7 @@ function copy (src, dest, opts, cb) { } else { checkPaths(src, dest, (err, stats) => { if (err) return cb(err) - const {srcStat, destStat} = stats + const { srcStat, destStat } = stats checkParentPaths(src, srcStat, dest, err => { if (err) return cb(err) if (opts.filter) return handleFilter(checkParentDir, destStat, src, dest, opts, cb) @@ -240,7 +240,7 @@ function checkStats (src, dest, cb) { if (err.code === 'ENOENT') return cb(null, {srcStat, destStat: notExist}) return cb(err) } - return cb(null, {srcStat, destStat}) + return cb(null, { srcStat, destStat }) }) }) } @@ -267,14 +267,14 @@ function checkParentPaths (src, srcStat, dest, cb) { function checkPaths (src, dest, cb) { checkStats(src, dest, (err, stats) => { if (err) return cb(err) - const {srcStat, destStat} = stats + const { srcStat, destStat } = stats if (destStat.ino && destStat.ino === srcStat.ino) { return cb(new Error('Source and destination must not be the same.')) } if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) } - return cb(null, {srcStat, destStat}) + return cb(null, { srcStat, destStat }) }) } From 19eabe1b638e9b719d679d728f8333168e04fe15 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 9 May 2019 01:56:28 -0700 Subject: [PATCH 11/19] copy*(): use fs.stat with bigint option --- .../copy-sync-without-checking-paths.test.js | 296 ------------------ lib/copy-sync/copy-sync.js | 76 +---- .../copy-without-checking-paths.test.js | 295 ----------------- lib/copy/copy.js | 95 +----- lib/move-sync/index.js | 98 +----- lib/move-sync/move-sync.js | 99 ++++++ lib/move/index.js | 117 +------ lib/move/move.js | 118 +++++++ lib/util/stat.js | 151 +++++++++ 9 files changed, 394 insertions(+), 951 deletions(-) delete mode 100644 lib/copy-sync/__tests__/copy-sync-without-checking-paths.test.js delete mode 100644 lib/copy/__tests__/copy-without-checking-paths.test.js create mode 100644 lib/move-sync/move-sync.js create mode 100644 lib/move/move.js create mode 100644 lib/util/stat.js diff --git a/lib/copy-sync/__tests__/copy-sync-without-checking-paths.test.js b/lib/copy-sync/__tests__/copy-sync-without-checking-paths.test.js deleted file mode 100644 index 459e5f92..00000000 --- a/lib/copy-sync/__tests__/copy-sync-without-checking-paths.test.js +++ /dev/null @@ -1,296 +0,0 @@ -'use strict' - -const assert = require('assert') -const os = require('os') -const path = require('path') -const fs = require('../../') -const klawSync = require('klaw-sync') - -/* global beforeEach, afterEach, describe, it */ - -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() - without checking paths', () => { - let TEST_DIR, src - - beforeEach(done => { - TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-without-checking-paths') - 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 if dest parent is a subdir of src', () => { - const srcFile = path.join(TEST_DIR, 'src', 'srcfile.txt') - const destFile = path.join(TEST_DIR, 'src', 'dest', 'destfile.txt') - fs.writeFileSync(srcFile, dat0) - - fs.copySync(srcFile, destFile, { checkPathsBeforeCopying: false }) - - assert(fs.existsSync(destFile)) - const out = fs.readFileSync(destFile, 'utf8') - assert.strictEqual(out, dat0) - }) - }) - - // 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(`of not itself`, () => { - const dest = path.join(TEST_DIR, src.replace(/^\w:/, '')) - return testSuccess(src, dest) - }) - it(`should copy the directory successfully when dest is 'src_dest'`, () => { - const dest = path.join(TEST_DIR, 'src_dest') - return testSuccess(src, dest) - }) - it(`should copy the directory successfully when dest is 'src-dest'`, () => { - const dest = path.join(TEST_DIR, 'src-dest') - return testSuccess(src, dest) - }) - - it(`should copy the directory successfully when dest is 'dest_src'`, () => { - const dest = path.join(TEST_DIR, 'dest_src') - return testSuccess(src, dest) - }) - - it(`should copy the directory successfully when dest is 'src_dest/src'`, () => { - const dest = path.join(TEST_DIR, 'src_dest', 'src') - return testSuccess(src, dest) - }) - - it(`should copy the directory successfully when dest is 'src-dest/src'`, () => { - const dest = path.join(TEST_DIR, 'src-dest', 'src') - return testSuccess(src, dest) - }) - - it(`should copy the directory successfully when dest is 'dest_src/src'`, () => { - const dest = path.join(TEST_DIR, 'dest_src', 'src') - return testSuccess(src, dest) - }) - - it(`should copy the directory successfully when dest is 'src_src/dest'`, () => { - const dest = path.join(TEST_DIR, 'src_src', 'dest') - return testSuccess(src, dest) - }) - - it(`should copy the directory successfully when dest is 'src-src/dest'`, () => { - const dest = path.join(TEST_DIR, 'src-src', 'dest') - return testSuccess(src, dest) - }) - - it(`should copy the directory successfully when dest is 'srcsrc/dest'`, () => { - const dest = path.join(TEST_DIR, 'srcsrc', 'dest') - return testSuccess(src, dest) - }) - - it(`should copy the directory successfully when dest is 'dest/src'`, () => { - const dest = path.join(TEST_DIR, 'dest', 'src') - return testSuccess(src, dest) - }) - - it('should copy the directory successfully when dest is very nested that all its parents need to be created', () => { - 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) - }) - }) - - describe('>> when dest is a symlink', () => { - it('should copy the directory successfully when src is a subdir of resolved dest path', () => { - 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.copySync(srcInDest, destLink, { checkPathsBeforeCopying: false }) - - 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)))) - - 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) - assert.strictEqual(o1, dat1) - assert.strictEqual(o2, dat2) - assert.strictEqual(o3, dat3) - }) - }) - }) - - describe('> when source is a symlink', () => { - describe('>> when dest is a directory', () => { - it('should error when resolved src path points to dest', () => { - const srcLink = path.join(TEST_DIR, 'src-symlink') - fs.symlinkSync(src, srcLink, 'dir') - const dest = path.join(TEST_DIR, 'src') - let errThrown - try { - fs.copySync(srcLink, dest, { checkPathsBeforeCopying: false }) - } catch (err) { - errThrown = err - } - assert(errThrown) - - // assert source not affected - const link = fs.readlinkSync(srcLink) - assert.strictEqual(link, src) - }) - - it('should error when dest is a subdir of resolved src path', () => { - 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) - - let errThrown - try { - fs.copySync(srcLink, dest, { checkPathsBeforeCopying: false }) - } catch (err) { - errThrown = err - } - assert(errThrown) - const link = fs.readlinkSync(srcLink) - assert.strictEqual(link, src) - }) - - it('should error when resolved src path is a subdir of dest', () => { - 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') - - let errThrown - try { - fs.copySync(srcLink, dest, { checkPathsBeforeCopying: false }) - } catch (err) { - errThrown = err - } - assert(errThrown) - }) - - it(`should copy the directory successfully when dest is 'src_src/dest'`, () => { - 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) - }) - - it(`should copy the directory successfully when dest is 'srcsrc/dest'`, () => { - 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) - }) - }) - - describe('>> when dest is a symlink', () => { - it('should error when resolved dest path is a subdir of resolved src path', () => { - 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') - - let errThrown - try { - fs.copySync(srcLink, destLink, { checkPathsBeforeCopying: false }) - } catch (err) { - errThrown = err - } - assert.strictEqual(errThrown.message, `Cannot copy '${src}' to a subdirectory of itself, '${resolvedDestPath}'.`) - const destln = fs.readlinkSync(destLink) - assert.strictEqual(destln, resolvedDestPath) - }) - - it('should error when resolved src path is a subdir of resolved dest path', () => { - const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src') - const srcLink = path.join(TEST_DIR, 'src-symlink') - const destLink = path.join(TEST_DIR, 'dest-symlink') - const dest = path.join(TEST_DIR, 'dest') - - fs.ensureDirSync(srcInDest) - fs.ensureSymlinkSync(srcInDest, srcLink, 'dir') - fs.ensureSymlinkSync(dest, destLink, 'dir') - - let errThrown - try { - fs.copySync(srcLink, destLink, { checkPathsBeforeCopying: false }) - } catch (err) { - errThrown = err - } - assert.strictEqual(errThrown.message, `Cannot overwrite '${dest}' with '${srcInDest}'.`) - const destln = fs.readlinkSync(destLink) - assert.strictEqual(destln, dest) - }) - }) - }) -}) - -function testSuccess (src, dest) { - const srclen = klawSync(src).length - assert(srclen > 2) - fs.copySync(src, dest, { checkPathsBeforeCopying: false }) - - FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)))) - - 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) - assert.strictEqual(o1, dat1) - assert.strictEqual(o2, dat2) - assert.strictEqual(o3, dat3) -} diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 001fd577..3865a3ad 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -4,8 +4,7 @@ const fs = require('graceful-fs') const path = require('path') const mkdirpSync = require('../mkdirs').mkdirsSync const utimesSync = require('../util/utimes.js').utimesMillisSync - -const notExist = Symbol('notExist') +const stat = require('../util/stat') function copySync (src, dest, opts) { if (typeof opts === 'function') { @@ -22,12 +21,8 @@ function copySync (src, dest, opts) { see https://github.com/jprichardson/node-fs-extra/issues/269`) } - if (opts.checkPathsBeforeCopying === false) { - const { destStat } = checkStats(src, dest) - return handleFilterAndCopy(destStat, src, dest, opts) - } - const { srcStat, destStat } = checkPaths(src, dest) - checkParentPaths(src, srcStat, dest) + const { srcStat, destStat } = stat.checkPathsSync(src, dest) + stat.checkParentPathsSync(src, srcStat, dest) return handleFilterAndCopy(destStat, src, dest, opts) } @@ -55,7 +50,7 @@ function getStats (destStat, src, dest, opts) { } function onFile (srcStat, destStat, src, dest, opts) { - if (destStat === notExist) return copyFile(srcStat, src, dest, opts) + if (!destStat) return copyFile(srcStat, src, dest, opts) return mayCopyFile(srcStat, src, dest, opts) } @@ -101,7 +96,7 @@ function copyFileFallback (srcStat, src, dest, opts) { } function onDir (srcStat, destStat, src, dest, opts) { - if (destStat === notExist) return mkDirAndCopy(srcStat, src, dest, opts) + if (!destStat) return mkDirAndCopy(srcStat, src, dest, opts) if (destStat && !destStat.isDirectory()) { throw new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`) } @@ -121,19 +116,17 @@ function copyDir (src, dest, opts) { function copyDirItem (item, src, dest, opts) { const srcItem = path.join(src, item) const destItem = path.join(dest, item) - const check = opts.checkPathsBeforeCopying === false ? checkStats : checkPaths - const { destStat } = check(srcItem, destItem) + const { destStat } = stat.checkPathsSync(srcItem, destItem) return startCopy(destStat, srcItem, destItem, opts) } function onLink (destStat, src, dest, opts) { let resolvedSrc = fs.readlinkSync(src) - if (opts.dereference) { resolvedSrc = path.resolve(process.cwd(), resolvedSrc) } - if (destStat === notExist) { + if (!destStat) { return fs.symlinkSync(resolvedSrc, dest) } else { let resolvedDest @@ -149,14 +142,14 @@ function onLink (destStat, src, dest, opts) { if (opts.dereference) { resolvedDest = path.resolve(process.cwd(), resolvedDest) } - if (isSrcSubdir(resolvedSrc, resolvedDest)) { + if (stat.isSrcSubdir(resolvedSrc, resolvedDest)) { throw new Error(`Cannot copy '${resolvedSrc}' to a subdirectory of itself, '${resolvedDest}'.`) } // prevent copy if src is a subdir of dest since unlinking // dest in this case would result in removing src contents // and therefore a broken symlink would be created. - if (fs.statSync(dest).isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc)) { + if (fs.statSync(dest).isDirectory() && stat.isSrcSubdir(resolvedDest, resolvedSrc)) { throw new Error(`Cannot overwrite '${resolvedDest}' with '${resolvedSrc}'.`) } return copyLink(resolvedSrc, dest) @@ -168,55 +161,4 @@ function copyLink (resolvedSrc, dest) { return fs.symlinkSync(resolvedSrc, dest) } -// return true if dest is a subdir of src, otherwise false. -// It only checks the path strings. -function isSrcSubdir (src, dest) { - const srcArr = path.resolve(src).split(path.sep).filter(i => i) - const destArr = path.resolve(dest).split(path.sep).filter(i => i) - return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) -} - -function checkStats (src, dest) { - const srcStat = fs.statSync(src) - let destStat - try { - destStat = fs.statSync(dest) - } catch (err) { - if (err.code === 'ENOENT') return { srcStat, destStat: notExist } - throw err - } - return { srcStat, destStat } -} - -// recursively check if dest parent is a subdirectory of src. -// It works for all file types including symlinks since it -// checks the src and dest inodes. It starts from the deepest -// parent and stops once it reaches the src parent or the root path. -function checkParentPaths (src, srcStat, dest) { - const destParent = path.dirname(dest) - if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return - let destStat - try { - destStat = fs.statSync(destParent) - } catch (err) { - if (err.code === 'ENOENT') return - throw err - } - if (destStat.ino && destStat.ino === srcStat.ino) { - throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) - } - return checkParentPaths(src, srcStat, destParent) -} - -function checkPaths (src, dest) { - const { srcStat, destStat } = checkStats(src, dest) - if (destStat.ino && destStat.ino === srcStat.ino) { - throw new Error('Source and destination must not be the same.') - } - if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { - throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) - } - return { srcStat, destStat } -} - module.exports = copySync diff --git a/lib/copy/__tests__/copy-without-checking-paths.test.js b/lib/copy/__tests__/copy-without-checking-paths.test.js deleted file mode 100644 index 68c394c3..00000000 --- a/lib/copy/__tests__/copy-without-checking-paths.test.js +++ /dev/null @@ -1,295 +0,0 @@ -'use strict' - -const assert = require('assert') -const os = require('os') -const path = require('path') -const fs = require('../../') -const klawSync = require('klaw-sync') - -/* global beforeEach, afterEach, describe, it */ - -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('+ copy() - without checking paths', () => { - let TEST_DIR, src - - beforeEach(done => { - TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-without-checking-paths') - 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 if dest parent is a subdir of src', done => { - const srcFile = path.join(TEST_DIR, 'src', 'srcfile.txt') - const destFile = path.join(TEST_DIR, 'src', 'dest', 'destfile.txt') - fs.writeFileSync(srcFile, dat0) - - fs.copy(srcFile, destFile, { checkPathsBeforeCopying: false }, err => { - assert.ifError(err) - - assert(fs.existsSync(destFile)) - const out = fs.readFileSync(destFile, 'utf8') - assert.strictEqual(out, dat0) - 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(`of not itself`, done => { - const dest = path.join(TEST_DIR, src.replace(/^\w:/, '')) - 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 '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) - }) - }) - - describe('>> when dest is a symlink', () => { - 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, { checkPathsBeforeCopying: false }, 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)))) - - 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) - assert.strictEqual(o1, dat1) - assert.strictEqual(o2, dat2) - assert.strictEqual(o3, dat3) - 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, { checkPathsBeforeCopying: false }, 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, { checkPathsBeforeCopying: false }, 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, { checkPathsBeforeCopying: false }, 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 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, { checkPathsBeforeCopying: false }, err => { - assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${resolvedDestPath}'.`) - const destln = fs.readlinkSync(destLink) - assert.strictEqual(destln, resolvedDestPath) - done() - }) - }) - - it('should error when resolved src path is a subdir of resolved dest path', done => { - const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src') - const srcLink = path.join(TEST_DIR, 'src-symlink') - const destLink = path.join(TEST_DIR, 'dest-symlink') - const dest = path.join(TEST_DIR, 'dest') - - fs.ensureDirSync(srcInDest) - fs.ensureSymlinkSync(srcInDest, srcLink, 'dir') - fs.ensureSymlinkSync(dest, destLink, 'dir') - - fs.copy(srcLink, destLink, { checkPathsBeforeCopying: false }, 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, { checkPathsBeforeCopying: false }, err => { - assert.ifError(err) - - FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)))) - - 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) - assert.strictEqual(o1, dat1) - assert.strictEqual(o2, dat2) - assert.strictEqual(o3, dat3) - done() - }) -} diff --git a/lib/copy/copy.js b/lib/copy/copy.js index a38e5f46..fb2b6e74 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -5,8 +5,7 @@ const path = require('path') const mkdirp = require('../mkdirs').mkdirs const pathExists = require('../path-exists').pathExists const utimes = require('../util/utimes').utimesMillis - -const notExist = Symbol('notExist') +const stat = require('../util/stat') function copy (src, dest, opts, cb) { if (typeof opts === 'function' && !cb) { @@ -28,24 +27,15 @@ function copy (src, dest, opts, cb) { see https://github.com/jprichardson/node-fs-extra/issues/269`) } - if (opts.checkPathsBeforeCopying === false) { - checkStats(src, dest, (err, stats) => { + stat.checkPaths(src, dest, (err, stats) => { + if (err) return cb(err) + const { srcStat, destStat } = stats + stat.checkParentPaths(src, srcStat, dest, err => { if (err) return cb(err) - const { destStat } = stats if (opts.filter) return handleFilter(checkParentDir, destStat, src, dest, opts, cb) return checkParentDir(destStat, src, dest, opts, cb) }) - } else { - checkPaths(src, dest, (err, stats) => { - if (err) return cb(err) - const { srcStat, destStat } = stats - checkParentPaths(src, srcStat, dest, err => { - if (err) return cb(err) - if (opts.filter) return handleFilter(checkParentDir, destStat, src, dest, opts, cb) - return checkParentDir(destStat, src, dest, opts, cb) - }) - }) - } + }) } function checkParentDir (destStat, src, dest, opts, cb) { @@ -62,10 +52,7 @@ function checkParentDir (destStat, src, dest, opts, cb) { function handleFilter (onInclude, destStat, src, dest, opts, cb) { Promise.resolve(opts.filter(src, dest)).then(include => { - if (include) { - if (destStat) return onInclude(destStat, src, dest, opts, cb) - return onInclude(src, dest, opts, cb) - } + if (include) return onInclude(destStat, src, dest, opts, cb) return cb() }, error => cb(error)) } @@ -89,7 +76,7 @@ function getStats (destStat, src, dest, opts, cb) { } function onFile (srcStat, destStat, src, dest, opts, cb) { - if (destStat === notExist) return copyFile(srcStat, src, dest, opts, cb) + if (!destStat) return copyFile(srcStat, src, dest, opts, cb) return mayCopyFile(srcStat, src, dest, opts, cb) } @@ -135,7 +122,7 @@ function setDestModeAndTimestamps (srcStat, dest, opts, cb) { } function onDir (srcStat, destStat, src, dest, opts, cb) { - if (destStat === notExist) return mkDirAndCopy(srcStat, src, dest, opts, cb) + if (!destStat) return mkDirAndCopy(srcStat, src, dest, opts, cb) if (destStat && !destStat.isDirectory()) { return cb(new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`)) } @@ -168,8 +155,7 @@ function copyDirItems (items, src, dest, opts, cb) { function copyDirItem (items, item, src, dest, opts, cb) { const srcItem = path.join(src, item) const destItem = path.join(dest, item) - const check = opts.checkPathsBeforeCopying === false ? checkStats : checkPaths - check(srcItem, destItem, (err, stats) => { + stat.checkPaths(srcItem, destItem, (err, stats) => { if (err) return cb(err) const { destStat } = stats startCopy(destStat, srcItem, destItem, opts, err => { @@ -182,12 +168,11 @@ function copyDirItem (items, item, src, dest, opts, cb) { function onLink (destStat, src, dest, opts, cb) { fs.readlink(src, (err, resolvedSrc) => { if (err) return cb(err) - if (opts.dereference) { resolvedSrc = path.resolve(process.cwd(), resolvedSrc) } - if (destStat === notExist) { + if (!destStat) { return fs.symlink(resolvedSrc, dest, cb) } else { fs.readlink(dest, (err, resolvedDest) => { @@ -201,14 +186,14 @@ function onLink (destStat, src, dest, opts, cb) { if (opts.dereference) { resolvedDest = path.resolve(process.cwd(), resolvedDest) } - if (isSrcSubdir(resolvedSrc, resolvedDest)) { + if (stat.isSrcSubdir(resolvedSrc, resolvedDest)) { return cb(new Error(`Cannot copy '${resolvedSrc}' to a subdirectory of itself, '${resolvedDest}'.`)) } // do not copy if src is a subdir of dest since unlinking // dest in this case would result in removing src contents // and therefore a broken symlink would be created. - if (destStat.isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc)) { + if (destStat.isDirectory() && stat.isSrcSubdir(resolvedDest, resolvedSrc)) { return cb(new Error(`Cannot overwrite '${resolvedDest}' with '${resolvedSrc}'.`)) } return copyLink(resolvedSrc, dest, cb) @@ -224,58 +209,4 @@ function copyLink (resolvedSrc, dest, cb) { }) } -// return true if dest is a subdir of src, otherwise false. -// It only checks the path strings. -function isSrcSubdir (src, dest) { - const srcArr = path.resolve(src).split(path.sep).filter(i => i) - const destArr = path.resolve(dest).split(path.sep).filter(i => i) - return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) -} - -function checkStats (src, dest, cb) { - fs.stat(src, (err, srcStat) => { - if (err) return cb(err) - fs.stat(dest, (err, destStat) => { - if (err) { - if (err.code === 'ENOENT') return cb(null, {srcStat, destStat: notExist}) - return cb(err) - } - return cb(null, { srcStat, destStat }) - }) - }) -} - -// recursively check if dest parent is a subdirectory of src. -// It works for all file types including symlinks since it -// checks the src and dest inodes. It starts from the deepest -// parent and stops once it reaches the src parent or the root path. -function checkParentPaths (src, srcStat, dest, cb) { - const destParent = path.dirname(dest) - if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return cb() - fs.stat(destParent, (err, destStat) => { - if (err) { - if (err.code === 'ENOENT') return cb() - return cb(err) - } - if (destStat.ino && destStat.ino === srcStat.ino) { - return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) - } - return checkParentPaths(src, srcStat, destParent, cb) - }) -} - -function checkPaths (src, dest, cb) { - checkStats(src, dest, (err, stats) => { - if (err) return cb(err) - const { srcStat, destStat } = stats - if (destStat.ino && destStat.ino === srcStat.ino) { - return cb(new Error('Source and destination must not be the same.')) - } - if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { - return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) - } - return cb(null, { srcStat, destStat }) - }) -} - module.exports = copy diff --git a/lib/move-sync/index.js b/lib/move-sync/index.js index 12bef7bb..af90b06b 100644 --- a/lib/move-sync/index.js +++ b/lib/move-sync/index.js @@ -1,99 +1,5 @@ 'use strict' -const fs = require('graceful-fs') -const path = require('path') -const copySync = require('../copy-sync').copySync -const removeSync = require('../remove').removeSync -const mkdirpSync = require('../mkdirs').mkdirpSync - -function moveSync (src, dest, opts) { - opts = opts || {} - const overwrite = opts.overwrite || opts.clobber || false - - const srcStat = checkPaths(src, dest) - checkParentStats(src, srcStat, dest) - - mkdirpSync(path.dirname(dest)) - return doRename(src, dest, overwrite) -} - -function doRename (src, dest, overwrite) { - if (overwrite) { - removeSync(dest) - return rename(src, dest, overwrite) - } - if (fs.existsSync(dest)) throw new Error('dest already exists.') - return rename(src, dest, overwrite) -} - -function rename (src, dest, overwrite) { - try { - fs.renameSync(src, dest) - } catch (err) { - if (err.code !== 'EXDEV') throw err - return moveAcrossDevice(src, dest, overwrite) - } -} - -function moveAcrossDevice (src, dest, overwrite) { - const opts = { - overwrite, - errorOnExist: true - } - - copySync(src, dest, opts) - return removeSync(src) +module.exports = { + moveSync: require('./move-sync') } - -// return true if dest is a subdir of src, otherwise false. -// It only checks the path strings. -function isSrcSubdir (src, dest) { - const srcArr = path.resolve(src).split(path.sep).filter(i => i) - const destArr = path.resolve(dest).split(path.sep).filter(i => i) - return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) -} - -function checkStats (src, dest) { - const srcStat = fs.statSync(src) - let destStat - try { - destStat = fs.statSync(dest) - } catch (err) { - if (err.code === 'ENOENT') return {srcStat} - throw err - } - return {srcStat, destStat} -} - -// recursively check if dest parent is a subdirectory of src. -// It works for all file types including symlinks since it -// checks the src and dest inodes. It starts from the deepest -// parent and stops once it reaches the src parent or the root path. -function checkParentStats (src, srcStat, dest) { - const destParent = path.dirname(dest) - if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return - let destStat - try { - destStat = fs.statSync(destParent) - } catch (err) { - if (err.code === 'ENOENT') return - throw err - } - if (destStat.ino && destStat.ino === srcStat.ino) { - throw new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) - } - return checkParentStats(src, srcStat, destParent) -} - -function checkPaths (src, dest) { - const {srcStat, destStat} = checkStats(src, dest) - if (destStat && destStat.ino && destStat.ino === srcStat.ino) { - throw new Error('Source and destination must not be the same.') - } - if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { - throw new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) - } - return srcStat -} - -module.exports = { moveSync } diff --git a/lib/move-sync/move-sync.js b/lib/move-sync/move-sync.js new file mode 100644 index 00000000..599b846b --- /dev/null +++ b/lib/move-sync/move-sync.js @@ -0,0 +1,99 @@ +'use strict' + +const fs = require('graceful-fs') +const path = require('path') +const copySync = require('../copy-sync').copySync +const removeSync = require('../remove').removeSync +const mkdirpSync = require('../mkdirs').mkdirpSync + +function moveSync (src, dest, opts) { + opts = opts || {} + const overwrite = opts.overwrite || opts.clobber || false + + const srcStat = checkPaths(src, dest) + checkParentStats(src, srcStat, dest) + + mkdirpSync(path.dirname(dest)) + return doRename(src, dest, overwrite) +} + +function doRename (src, dest, overwrite) { + if (overwrite) { + removeSync(dest) + return rename(src, dest, overwrite) + } + if (fs.existsSync(dest)) throw new Error('dest already exists.') + return rename(src, dest, overwrite) +} + +function rename (src, dest, overwrite) { + try { + fs.renameSync(src, dest) + } catch (err) { + if (err.code !== 'EXDEV') throw err + return moveAcrossDevice(src, dest, overwrite) + } +} + +function moveAcrossDevice (src, dest, overwrite) { + const opts = { + overwrite, + errorOnExist: true + } + + copySync(src, dest, opts) + return removeSync(src) +} + +// return true if dest is a subdir of src, otherwise false. +// It only checks the path strings. +function isSrcSubdir (src, dest) { + const srcArr = path.resolve(src).split(path.sep).filter(i => i) + const destArr = path.resolve(dest).split(path.sep).filter(i => i) + return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) +} + +function checkStats (src, dest) { + const srcStat = fs.statSync(src) + let destStat + try { + destStat = fs.statSync(dest) + } catch (err) { + if (err.code === 'ENOENT') return {srcStat} + throw err + } + return {srcStat, destStat} +} + +// recursively check if dest parent is a subdirectory of src. +// It works for all file types including symlinks since it +// checks the src and dest inodes. It starts from the deepest +// parent and stops once it reaches the src parent or the root path. +function checkParentStats (src, srcStat, dest) { + const destParent = path.dirname(dest) + if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return + let destStat + try { + destStat = fs.statSync(destParent) + } catch (err) { + if (err.code === 'ENOENT') return + throw err + } + if (destStat.ino && destStat.ino === srcStat.ino) { + throw new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) + } + return checkParentStats(src, srcStat, destParent) +} + +function checkPaths (src, dest) { + const {srcStat, destStat} = checkStats(src, dest) + if (destStat && destStat.ino && destStat.ino === srcStat.ino) { + throw new Error('Source and destination must not be the same.') + } + if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { + throw new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) + } + return srcStat +} + +module.exports = moveSync diff --git a/lib/move/index.js b/lib/move/index.js index 04c7c68a..3785345b 100644 --- a/lib/move/index.js +++ b/lib/move/index.js @@ -1,119 +1,6 @@ 'use strict' const u = require('universalify').fromCallback -const fs = require('graceful-fs') -const path = require('path') -const copy = require('../copy').copy -const remove = require('../remove').remove -const mkdirp = require('../mkdirs').mkdirp -const pathExists = require('../path-exists').pathExists - -function move (src, dest, opts, cb) { - if (typeof opts === 'function') { - cb = opts - opts = {} - } - - const overwrite = opts.overwrite || opts.clobber || false - - checkPaths(src, dest, (err, srcStat) => { - if (err) return cb(err) - checkParentStats(src, srcStat, dest, err => { - if (err) return cb(err) - mkdirp(path.dirname(dest), err => { - if (err) return cb(err) - return doRename(src, dest, overwrite, cb) - }) - }) - }) -} - -function doRename (src, dest, overwrite, cb) { - if (overwrite) { - return remove(dest, err => { - if (err) return cb(err) - return rename(src, dest, overwrite, cb) - }) - } - pathExists(dest, (err, destExists) => { - if (err) return cb(err) - if (destExists) return cb(new Error('dest already exists.')) - return rename(src, dest, overwrite, cb) - }) -} - -function rename (src, dest, overwrite, cb) { - fs.rename(src, dest, err => { - if (!err) return cb() - if (err.code !== 'EXDEV') return cb(err) - return moveAcrossDevice(src, dest, overwrite, cb) - }) -} - -function moveAcrossDevice (src, dest, overwrite, cb) { - const opts = { - overwrite, - errorOnExist: true - } - - copy(src, dest, opts, err => { - if (err) return cb(err) - return remove(src, cb) - }) +module.exports = { + move: u(require('./move')) } - -// return true if dest is a subdir of src, otherwise false. -// It only checks the path strings. -function isSrcSubdir (src, dest) { - const srcArr = path.resolve(src).split(path.sep).filter(i => i) - const destArr = path.resolve(dest).split(path.sep).filter(i => i) - return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) -} - -function checkStats (src, dest, cb) { - fs.stat(src, (err, srcStat) => { - if (err) return cb(err) - fs.stat(dest, (err, destStat) => { - if (err) { - if (err.code === 'ENOENT') return cb(null, {srcStat}) - return cb(err) - } - return cb(null, {srcStat, destStat}) - }) - }) -} - -// recursively check if dest parent is a subdirectory of src. -// It works for all file types including symlinks since it -// checks the src and dest inodes. It starts from the deepest -// parent and stops once it reaches the src parent or the root path. -function checkParentStats (src, srcStat, dest, cb) { - const destParent = path.dirname(dest) - if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return cb() - fs.stat(destParent, (err, destStat) => { - if (err) { - if (err.code === 'ENOENT') return cb() - return cb(err) - } - if (destStat.ino && destStat.ino === srcStat.ino) { - return cb(new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`)) - } - return checkParentStats(src, srcStat, destParent, cb) - }) -} - -function checkPaths (src, dest, cb) { - checkStats(src, dest, (err, stats) => { - if (err) return cb(err) - const {srcStat, destStat} = stats - if (destStat && destStat.ino && destStat.ino === srcStat.ino) { - return cb(new Error('Source and destination must not be the same.')) - } - if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { - return cb(new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`)) - } - return cb(null, srcStat) - }) -} - -module.exports = { move: u(move) } diff --git a/lib/move/move.js b/lib/move/move.js new file mode 100644 index 00000000..2ed51feb --- /dev/null +++ b/lib/move/move.js @@ -0,0 +1,118 @@ +'use strict' + +const fs = require('graceful-fs') +const path = require('path') +const copy = require('../copy').copy +const remove = require('../remove').remove +const mkdirp = require('../mkdirs').mkdirp +const pathExists = require('../path-exists').pathExists + +function move (src, dest, opts, cb) { + if (typeof opts === 'function') { + cb = opts + opts = {} + } + + const overwrite = opts.overwrite || opts.clobber || false + + checkPaths(src, dest, (err, srcStat) => { + if (err) return cb(err) + checkParentStats(src, srcStat, dest, err => { + if (err) return cb(err) + mkdirp(path.dirname(dest), err => { + if (err) return cb(err) + return doRename(src, dest, overwrite, cb) + }) + }) + }) +} + +function doRename (src, dest, overwrite, cb) { + if (overwrite) { + return remove(dest, err => { + if (err) return cb(err) + return rename(src, dest, overwrite, cb) + }) + } + pathExists(dest, (err, destExists) => { + if (err) return cb(err) + if (destExists) return cb(new Error('dest already exists.')) + return rename(src, dest, overwrite, cb) + }) +} + +function rename (src, dest, overwrite, cb) { + fs.rename(src, dest, err => { + if (!err) return cb() + if (err.code !== 'EXDEV') return cb(err) + return moveAcrossDevice(src, dest, overwrite, cb) + }) +} + +function moveAcrossDevice (src, dest, overwrite, cb) { + const opts = { + overwrite, + errorOnExist: true + } + + copy(src, dest, opts, err => { + if (err) return cb(err) + return remove(src, cb) + }) +} + +// return true if dest is a subdir of src, otherwise false. +// It only checks the path strings. +function isSrcSubdir (src, dest) { + const srcArr = path.resolve(src).split(path.sep).filter(i => i) + const destArr = path.resolve(dest).split(path.sep).filter(i => i) + return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) +} + +function checkStats (src, dest, cb) { + fs.stat(src, (err, srcStat) => { + if (err) return cb(err) + fs.stat(dest, (err, destStat) => { + if (err) { + if (err.code === 'ENOENT') return cb(null, {srcStat}) + return cb(err) + } + return cb(null, {srcStat, destStat}) + }) + }) +} + +// recursively check if dest parent is a subdirectory of src. +// It works for all file types including symlinks since it +// checks the src and dest inodes. It starts from the deepest +// parent and stops once it reaches the src parent or the root path. +function checkParentStats (src, srcStat, dest, cb) { + const destParent = path.dirname(dest) + if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return cb() + fs.stat(destParent, (err, destStat) => { + if (err) { + if (err.code === 'ENOENT') return cb() + return cb(err) + } + if (destStat.ino && destStat.ino === srcStat.ino) { + return cb(new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`)) + } + return checkParentStats(src, srcStat, destParent, cb) + }) +} + +function checkPaths (src, dest, cb) { + checkStats(src, dest, (err, stats) => { + if (err) return cb(err) + const {srcStat, destStat} = stats + if (destStat && destStat.ino && destStat.ino === srcStat.ino) { + return cb(new Error('Source and destination must not be the same.')) + } + if (srcStat.isDirectory() && stat.isSrcSubdir(src, dest)) { + return cb(new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`)) + } + return cb(null, srcStat) + }) +} + +module.exports = move diff --git a/lib/util/stat.js b/lib/util/stat.js new file mode 100644 index 00000000..e452dd59 --- /dev/null +++ b/lib/util/stat.js @@ -0,0 +1,151 @@ +'use strict' + +// TODO: enable this once graceful-fs supports bigint option. +// const fs = require('graceful-fs') +const fs = require('fs') +const path = require('path') + +const NODE_VERSION_WITH_BIGINT = 1050 +const nodeVersion = Number.parseInt(process.versions.node.split('.').join(''), 10) + +function getStats (src, dest, cb) { + if (nodeVersion >= NODE_VERSION_WITH_BIGINT) { + fs.stat(src, { bigint: true }, (err, srcStat) => { + if (err) return cb(err) + fs.stat(dest, { bigint: true }, (err, destStat) => { + if (err) { + if (err.code === 'ENOENT') return cb(null, { srcStat, destStat: null }) + return cb(err) + } + return cb(null, { srcStat, destStat }) + }) + }) + } else { + fs.stat(src, (err, srcStat) => { + if (err) return cb(err) + fs.stat(dest, (err, destStat) => { + if (err) { + if (err.code === 'ENOENT') return cb(null, { srcStat, destStat: null }) + return cb(err) + } + return cb(null, { srcStat, destStat }) + }) + }) + } +} + +function getStatsSync (src, dest) { + let srcStat, destStat + if (nodeVersion >= NODE_VERSION_WITH_BIGINT) { + srcStat = fs.statSync(src, { bigint: true }) + } else { + srcStat = fs.statSync(src) + } + try { + if (nodeVersion >= NODE_VERSION_WITH_BIGINT) { + destStat = fs.statSync(dest, { bigint: true }) + } else { + destStat = fs.statSync(dest) + } + } catch (err) { + if (err.code === 'ENOENT') return { srcStat, destStat: null } + throw err + } + return { srcStat, destStat } +} + +function checkPaths (src, dest, cb) { + getStats(src, dest, (err, stats) => { + if (err) return cb(err) + const { srcStat, destStat } = stats + if (destStat && destStat.ino && destStat.dev && destStat.ino === srcStat.ino && destStat.dev === srcStat.dev) { + return cb(new Error('Source and destination must not be the same.')) + } + if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { + return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) + } + return cb(null, { srcStat, destStat }) + }) +} + +function checkPathsSync (src, dest) { + const { srcStat, destStat } = getStatsSync(src, dest) + if (destStat && destStat.ino && destStat.dev && destStat.ino === srcStat.ino && destStat.dev === srcStat.dev) { + throw new Error('Source and destination must not be the same.') + } + if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { + throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + } + return { srcStat, destStat } +} + +// recursively check if dest parent is a subdirectory of src. +// It works for all file types including symlinks since it +// checks the src and dest inodes. It starts from the deepest +// parent and stops once it reaches the src parent or the root path. +function checkParentPaths (src, srcStat, dest, cb) { + const destParent = path.dirname(dest) + if (destParent && + (destParent === path.dirname(src) || + destParent === path.parse(destParent).root) + ) return cb() + if (nodeVersion >= NODE_VERSION_WITH_BIGINT) { + fs.stat(destParent, { bigint: true }, (err, destStat) => { + if (err) { + if (err.code === 'ENOENT') return cb() + return cb(err) + } + if (destStat.ino && destStat.dev && destStat.ino === srcStat.ino && destStat.dev === srcStat.dev) { + return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) + } + return checkParentPaths(src, srcStat, destParent, cb) + }) + } else { + fs.stat(destParent, (err, destStat) => { + if (err) { + if (err.code === 'ENOENT') return cb() + return cb(err) + } + if (destStat.ino && destStat.dev && destStat.ino === srcStat.ino && destStat.dev === srcStat.dev) { + return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) + } + return checkParentPaths(src, srcStat, destParent, cb) + }) + } +} + +function checkParentPathsSync (src, srcStat, dest) { + const destParent = path.dirname(dest) + if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return + let destStat + try { + if (nodeVersion >= NODE_VERSION_WITH_BIGINT) { + destStat = fs.statSync(destParent, { bigint: true }) + } else { + destStat = fs.statSync(destParent) + } + } catch (err) { + if (err.code === 'ENOENT') return + throw err + } + if (destStat.ino && destStat.dev && destStat.ino === srcStat.ino && destStat.dev === srcStat.dev) { + throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + } + return checkParentPathsSync(src, srcStat, destParent) +} + +// return true if dest is a subdir of src, otherwise false. +// It only checks the path strings. +function isSrcSubdir (src, dest) { + const srcArr = path.resolve(src).split(path.sep).filter(i => i) + const destArr = path.resolve(dest).split(path.sep).filter(i => i) + return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) +} + +module.exports = { + checkPaths, + checkPathsSync, + checkParentPaths, + checkParentPathsSync, + isSrcSubdir +} From ac57df7e4e47adc8178c120970dfad3b08233255 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 9 May 2019 02:27:23 -0700 Subject: [PATCH 12/19] move*(): refactor to use the internal stat functions --- lib/copy-sync/copy-sync.js | 6 ++-- lib/copy/copy.js | 6 ++-- lib/move-sync/move-sync.js | 58 ++---------------------------------- lib/move/move.js | 61 +++----------------------------------- lib/util/stat.js | 28 +++++++++-------- 5 files changed, 29 insertions(+), 130 deletions(-) diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 3865a3ad..70f026a4 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -21,8 +21,8 @@ function copySync (src, dest, opts) { see https://github.com/jprichardson/node-fs-extra/issues/269`) } - const { srcStat, destStat } = stat.checkPathsSync(src, dest) - stat.checkParentPathsSync(src, srcStat, dest) + const { srcStat, destStat } = stat.checkPathsSync(src, dest, 'copy') + stat.checkParentPathsSync(src, srcStat, dest, 'copy') return handleFilterAndCopy(destStat, src, dest, opts) } @@ -116,7 +116,7 @@ function copyDir (src, dest, opts) { function copyDirItem (item, src, dest, opts) { const srcItem = path.join(src, item) const destItem = path.join(dest, item) - const { destStat } = stat.checkPathsSync(srcItem, destItem) + const { destStat } = stat.checkPathsSync(srcItem, destItem, 'copy') return startCopy(destStat, srcItem, destItem, opts) } diff --git a/lib/copy/copy.js b/lib/copy/copy.js index fb2b6e74..76c4bdb8 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -27,10 +27,10 @@ function copy (src, dest, opts, cb) { see https://github.com/jprichardson/node-fs-extra/issues/269`) } - stat.checkPaths(src, dest, (err, stats) => { + stat.checkPaths(src, dest, 'copy', (err, stats) => { if (err) return cb(err) const { srcStat, destStat } = stats - stat.checkParentPaths(src, srcStat, dest, err => { + stat.checkParentPaths(src, srcStat, dest, 'copy', err => { if (err) return cb(err) if (opts.filter) return handleFilter(checkParentDir, destStat, src, dest, opts, cb) return checkParentDir(destStat, src, dest, opts, cb) @@ -155,7 +155,7 @@ function copyDirItems (items, src, dest, opts, cb) { function copyDirItem (items, item, src, dest, opts, cb) { const srcItem = path.join(src, item) const destItem = path.join(dest, item) - stat.checkPaths(srcItem, destItem, (err, stats) => { + stat.checkPaths(srcItem, destItem, 'copy', (err, stats) => { if (err) return cb(err) const { destStat } = stats startCopy(destStat, srcItem, destItem, opts, err => { diff --git a/lib/move-sync/move-sync.js b/lib/move-sync/move-sync.js index 599b846b..20f910cc 100644 --- a/lib/move-sync/move-sync.js +++ b/lib/move-sync/move-sync.js @@ -5,14 +5,14 @@ const path = require('path') const copySync = require('../copy-sync').copySync const removeSync = require('../remove').removeSync const mkdirpSync = require('../mkdirs').mkdirpSync +const stat = require('../util/stat') function moveSync (src, dest, opts) { opts = opts || {} const overwrite = opts.overwrite || opts.clobber || false - const srcStat = checkPaths(src, dest) - checkParentStats(src, srcStat, dest) - + const { srcStat } = stat.checkPathsSync(src, dest, 'move') + stat.checkParentPathsSync(src, srcStat, dest, 'move') mkdirpSync(path.dirname(dest)) return doRename(src, dest, overwrite) } @@ -40,60 +40,8 @@ function moveAcrossDevice (src, dest, overwrite) { overwrite, errorOnExist: true } - copySync(src, dest, opts) return removeSync(src) } -// return true if dest is a subdir of src, otherwise false. -// It only checks the path strings. -function isSrcSubdir (src, dest) { - const srcArr = path.resolve(src).split(path.sep).filter(i => i) - const destArr = path.resolve(dest).split(path.sep).filter(i => i) - return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) -} - -function checkStats (src, dest) { - const srcStat = fs.statSync(src) - let destStat - try { - destStat = fs.statSync(dest) - } catch (err) { - if (err.code === 'ENOENT') return {srcStat} - throw err - } - return {srcStat, destStat} -} - -// recursively check if dest parent is a subdirectory of src. -// It works for all file types including symlinks since it -// checks the src and dest inodes. It starts from the deepest -// parent and stops once it reaches the src parent or the root path. -function checkParentStats (src, srcStat, dest) { - const destParent = path.dirname(dest) - if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return - let destStat - try { - destStat = fs.statSync(destParent) - } catch (err) { - if (err.code === 'ENOENT') return - throw err - } - if (destStat.ino && destStat.ino === srcStat.ino) { - throw new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) - } - return checkParentStats(src, srcStat, destParent) -} - -function checkPaths (src, dest) { - const {srcStat, destStat} = checkStats(src, dest) - if (destStat && destStat.ino && destStat.ino === srcStat.ino) { - throw new Error('Source and destination must not be the same.') - } - if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { - throw new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`) - } - return srcStat -} - module.exports = moveSync diff --git a/lib/move/move.js b/lib/move/move.js index 2ed51feb..fa3ea618 100644 --- a/lib/move/move.js +++ b/lib/move/move.js @@ -6,6 +6,7 @@ const copy = require('../copy').copy const remove = require('../remove').remove const mkdirp = require('../mkdirs').mkdirp const pathExists = require('../path-exists').pathExists +const stat = require('../util/stat') function move (src, dest, opts, cb) { if (typeof opts === 'function') { @@ -15,9 +16,10 @@ function move (src, dest, opts, cb) { const overwrite = opts.overwrite || opts.clobber || false - checkPaths(src, dest, (err, srcStat) => { + stat.checkPaths(src, dest, 'move', (err, stats) => { if (err) return cb(err) - checkParentStats(src, srcStat, dest, err => { + const { srcStat } = stats + stat.checkParentPaths(src, srcStat, dest, 'move', err => { if (err) return cb(err) mkdirp(path.dirname(dest), err => { if (err) return cb(err) @@ -54,65 +56,10 @@ function moveAcrossDevice (src, dest, overwrite, cb) { overwrite, errorOnExist: true } - copy(src, dest, opts, err => { if (err) return cb(err) return remove(src, cb) }) } -// return true if dest is a subdir of src, otherwise false. -// It only checks the path strings. -function isSrcSubdir (src, dest) { - const srcArr = path.resolve(src).split(path.sep).filter(i => i) - const destArr = path.resolve(dest).split(path.sep).filter(i => i) - return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) -} - -function checkStats (src, dest, cb) { - fs.stat(src, (err, srcStat) => { - if (err) return cb(err) - fs.stat(dest, (err, destStat) => { - if (err) { - if (err.code === 'ENOENT') return cb(null, {srcStat}) - return cb(err) - } - return cb(null, {srcStat, destStat}) - }) - }) -} - -// recursively check if dest parent is a subdirectory of src. -// It works for all file types including symlinks since it -// checks the src and dest inodes. It starts from the deepest -// parent and stops once it reaches the src parent or the root path. -function checkParentStats (src, srcStat, dest, cb) { - const destParent = path.dirname(dest) - if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return cb() - fs.stat(destParent, (err, destStat) => { - if (err) { - if (err.code === 'ENOENT') return cb() - return cb(err) - } - if (destStat.ino && destStat.ino === srcStat.ino) { - return cb(new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`)) - } - return checkParentStats(src, srcStat, destParent, cb) - }) -} - -function checkPaths (src, dest, cb) { - checkStats(src, dest, (err, stats) => { - if (err) return cb(err) - const {srcStat, destStat} = stats - if (destStat && destStat.ino && destStat.ino === srcStat.ino) { - return cb(new Error('Source and destination must not be the same.')) - } - if (srcStat.isDirectory() && stat.isSrcSubdir(src, dest)) { - return cb(new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`)) - } - return cb(null, srcStat) - }) -} - module.exports = move diff --git a/lib/util/stat.js b/lib/util/stat.js index e452dd59..a409055d 100644 --- a/lib/util/stat.js +++ b/lib/util/stat.js @@ -54,7 +54,7 @@ function getStatsSync (src, dest) { return { srcStat, destStat } } -function checkPaths (src, dest, cb) { +function checkPaths (src, dest, funcName, cb) { getStats(src, dest, (err, stats) => { if (err) return cb(err) const { srcStat, destStat } = stats @@ -62,19 +62,19 @@ function checkPaths (src, dest, cb) { return cb(new Error('Source and destination must not be the same.')) } if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { - return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) + return cb(new Error(errMsg(src, dest, funcName))) } return cb(null, { srcStat, destStat }) }) } -function checkPathsSync (src, dest) { +function checkPathsSync (src, dest, funcName) { const { srcStat, destStat } = getStatsSync(src, dest) if (destStat && destStat.ino && destStat.dev && destStat.ino === srcStat.ino && destStat.dev === srcStat.dev) { throw new Error('Source and destination must not be the same.') } if (srcStat.isDirectory() && isSrcSubdir(src, dest)) { - throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + throw new Error(errMsg(src, dest, funcName)) } return { srcStat, destStat } } @@ -83,7 +83,7 @@ function checkPathsSync (src, dest) { // It works for all file types including symlinks since it // checks the src and dest inodes. It starts from the deepest // parent and stops once it reaches the src parent or the root path. -function checkParentPaths (src, srcStat, dest, cb) { +function checkParentPaths (src, srcStat, dest, funcName, cb) { const destParent = path.dirname(dest) if (destParent && (destParent === path.dirname(src) || @@ -96,9 +96,9 @@ function checkParentPaths (src, srcStat, dest, cb) { return cb(err) } if (destStat.ino && destStat.dev && destStat.ino === srcStat.ino && destStat.dev === srcStat.dev) { - return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) + return cb(new Error(errMsg(src, dest, funcName))) } - return checkParentPaths(src, srcStat, destParent, cb) + return checkParentPaths(src, srcStat, destParent, funcName, cb) }) } else { fs.stat(destParent, (err, destStat) => { @@ -107,14 +107,14 @@ function checkParentPaths (src, srcStat, dest, cb) { return cb(err) } if (destStat.ino && destStat.dev && destStat.ino === srcStat.ino && destStat.dev === srcStat.dev) { - return cb(new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)) + return cb(new Error(errMsg(src, dest, funcName))) } - return checkParentPaths(src, srcStat, destParent, cb) + return checkParentPaths(src, srcStat, destParent, funcName, cb) }) } } -function checkParentPathsSync (src, srcStat, dest) { +function checkParentPathsSync (src, srcStat, dest, funcName) { const destParent = path.dirname(dest) if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return let destStat @@ -129,9 +129,9 @@ function checkParentPathsSync (src, srcStat, dest) { throw err } if (destStat.ino && destStat.dev && destStat.ino === srcStat.ino && destStat.dev === srcStat.dev) { - throw new Error(`Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + throw new Error(errMsg(src, dest, funcName)) } - return checkParentPathsSync(src, srcStat, destParent) + return checkParentPathsSync(src, srcStat, destParent, funcName) } // return true if dest is a subdir of src, otherwise false. @@ -142,6 +142,10 @@ function isSrcSubdir (src, dest) { return srcArr.reduce((acc, cur, i) => acc && destArr[i] === cur, true) } +function errMsg (src, dest, funcName) { + return `Cannot ${funcName} '${src}' to a subdirectory of itself, '${dest}'.` +} + module.exports = { checkPaths, checkPathsSync, From ffe29995eebd7bc305c7d096880c4b9c1b0f9e19 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 9 May 2019 02:48:55 -0700 Subject: [PATCH 13/19] move*(): add test for prevent moving identical --- ...move-sync-prevent-moving-identical.test.js | 271 ++++++++++++++++++ .../move-prevent-moving-identical.test.js | 252 ++++++++++++++++ 2 files changed, 523 insertions(+) create mode 100644 lib/move-sync/__tests__/move-sync-prevent-moving-identical.test.js create mode 100644 lib/move/__tests__/move-prevent-moving-identical.test.js diff --git a/lib/move-sync/__tests__/move-sync-prevent-moving-identical.test.js b/lib/move-sync/__tests__/move-sync-prevent-moving-identical.test.js new file mode 100644 index 00000000..238cb2a3 --- /dev/null +++ b/lib/move-sync/__tests__/move-sync-prevent-moving-identical.test.js @@ -0,0 +1,271 @@ +'use strict' + +const assert = require('assert') +const os = require('os') +const path = require('path') +const fs = require('../../') +const klawSync = require('klaw-sync') + +/* global beforeEach, afterEach, describe, it */ + +describe('+ moveSync() - prevent moving identical files and dirs', () => { + let TEST_DIR = '' + let src = '' + let dest = '' + + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'move-sync-prevent-moving-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_move_sync') + const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_move_sync') + fs.ensureFileSync(fileSrc) + + try { + fs.moveSync(fileSrc, fileDest) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } + }) + + describe('dest with parent symlink', () => { + describe('first parent is symlink', () => { + it('should error when src is file', () => { + const src = path.join(TEST_DIR, 'a', 'file.txt') + const dest = path.join(TEST_DIR, 'b', 'file.txt') + const srcParent = path.join(TEST_DIR, 'a') + const destParent = path.join(TEST_DIR, 'b') + fs.ensureFileSync(src) + fs.ensureSymlinkSync(srcParent, destParent, 'dir') + + try { + fs.moveSync(src, dest) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } finally { + assert(fs.existsSync(src)) + } + }) + + it('should error when src is directory', () => { + const src = path.join(TEST_DIR, 'a', 'foo') + const dest = path.join(TEST_DIR, 'b', 'foo') + const srcParent = path.join(TEST_DIR, 'a') + const destParent = path.join(TEST_DIR, 'b') + fs.ensureDirSync(src) + fs.ensureSymlinkSync(srcParent, destParent, 'dir') + + try { + fs.moveSync(src, dest) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } finally { + assert(fs.existsSync(src)) + } + }) + }) + + describe('nested dest', () => { + it('should error when src is file', () => { + const src = path.join(TEST_DIR, 'a', 'dir', 'file.txt') + const dest = path.join(TEST_DIR, 'b', 'dir', 'file.txt') + const srcParent = path.join(TEST_DIR, 'a') + const destParent = path.join(TEST_DIR, 'b') + fs.ensureFileSync(src) + fs.ensureSymlinkSync(srcParent, destParent, 'dir') + + try { + fs.moveSync(src, dest) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } finally { + assert(fs.existsSync(src)) + } + }) + + it('should error when src is directory', () => { + const src = path.join(TEST_DIR, 'a', 'dir', 'foo') + const dest = path.join(TEST_DIR, 'b', 'dir', 'foo') + const srcParent = path.join(TEST_DIR, 'a') + const destParent = path.join(TEST_DIR, 'b') + fs.ensureDirSync(src) + fs.ensureSymlinkSync(srcParent, destParent, 'dir') + + try { + fs.moveSync(src, dest) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } finally { + assert(fs.existsSync(src)) + } + }) + }) + }) + + // src is directory: + // src is regular, dest is symlink + // src is symlink, dest is regular + // src is symlink, dest is symlink + + describe('> when src is a directory', () => { + describe(`>> when src is regular and dest is a symlink that points to src`, () => { + it('should error', () => { + 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 + + try { + fs.moveSync(src, destLink) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } + + 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.moveSync(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 error src and dest are the same', () => { + 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 + + try { + fs.moveSync(srcLink, destLink) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } + + const srclenAfter = klawSync(srcLink).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + 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 src is a file', () => { + describe(`>> when src is regular and dest is a symlink that points to src`, () => { + it('should error', () => { + 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') + + try { + fs.moveSync(src, destLink) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } + + const link = fs.readlinkSync(destLink) + assert.strictEqual(link, src) + assert(fs.readFileSync(link, 'utf8'), 'some data') + }) + }) + + 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.outputFileSync(dest, 'some data') + + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(dest, srcLink, 'file') + + try { + fs.moveSync(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 error src and dest are the same', () => { + src = path.join(TEST_DIR, 'src', 'srcfile.txt') + fs.outputFileSync(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') + + try { + fs.moveSync(srcLink, destLink) + } catch (err) { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + } + + const srcln = fs.readlinkSync(srcLink) + assert.strictEqual(srcln, src) + 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/move/__tests__/move-prevent-moving-identical.test.js b/lib/move/__tests__/move-prevent-moving-identical.test.js new file mode 100644 index 00000000..096a9d45 --- /dev/null +++ b/lib/move/__tests__/move-prevent-moving-identical.test.js @@ -0,0 +1,252 @@ +'use strict' + +const assert = require('assert') +const os = require('os') +const path = require('path') +const fs = require('../../') +const klawSync = require('klaw-sync') + +/* global beforeEach, afterEach, describe, it */ + +describe('+ move() - prevent moving identical files and dirs', () => { + let TEST_DIR = '' + let src = '' + let dest = '' + + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'move-prevent-moving-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_move') + const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_move') + fs.ensureFileSync(fileSrc) + + fs.move(fileSrc, fileDest, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + done() + }) + }) + + describe('dest with parent symlink', () => { + describe('first parent is symlink', () => { + it('should error when src is file', done => { + const src = path.join(TEST_DIR, 'a', 'file.txt') + const dest = path.join(TEST_DIR, 'b', 'file.txt') + const srcParent = path.join(TEST_DIR, 'a') + const destParent = path.join(TEST_DIR, 'b') + fs.ensureFileSync(src) + fs.ensureSymlinkSync(srcParent, destParent, 'dir') + + fs.move(src, dest, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + assert(fs.existsSync(src)) + done() + }) + }) + + it('should error when src is directory', done => { + const src = path.join(TEST_DIR, 'a', 'foo') + const dest = path.join(TEST_DIR, 'b', 'foo') + const srcParent = path.join(TEST_DIR, 'a') + const destParent = path.join(TEST_DIR, 'b') + fs.ensureDirSync(src) + fs.ensureSymlinkSync(srcParent, destParent, 'dir') + + fs.move(src, dest, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + assert(fs.existsSync(src)) + done() + }) + }) + }) + + describe('nested dest', () => { + it('should error when src is file', done => { + const src = path.join(TEST_DIR, 'a', 'dir', 'file.txt') + const dest = path.join(TEST_DIR, 'b', 'dir', 'file.txt') + const srcParent = path.join(TEST_DIR, 'a') + const destParent = path.join(TEST_DIR, 'b') + fs.ensureFileSync(src) + fs.ensureSymlinkSync(srcParent, destParent, 'dir') + + fs.move(src, dest, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + assert(fs.existsSync(src)) + done() + }) + }) + + it('should error when src is directory', done => { + const src = path.join(TEST_DIR, 'a', 'dir', 'foo') + const dest = path.join(TEST_DIR, 'b', 'dir', 'foo') + const srcParent = path.join(TEST_DIR, 'a') + const destParent = path.join(TEST_DIR, 'b') + fs.ensureDirSync(src) + fs.ensureSymlinkSync(srcParent, destParent, 'dir') + + fs.move(src, dest, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + assert(fs.existsSync(src)) + done() + }) + }) + }) + }) + + // src is directory: + // src is regular, dest is symlink + // src is symlink, dest is regular + // src is symlink, dest is symlink + + describe('> when src is a directory', () => { + describe(`>> when src is regular and dest is a symlink that points to src`, () => { + it('should error', 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.move(src, destLink, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + + 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.move(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 error src and dest are the same', done => { + src = path.join(TEST_DIR, 'src') + fs.mkdirsSync(src) + const srcLink = path.join(TEST_DIR, 'src_symlink') + 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.move(srcLink, destLink, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + + const srclenAfter = klawSync(srcLink).length + assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change') + 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 src is a file', () => { + describe(`>> when src is regular and dest is a symlink that points to src`, () => { + it('should error', done => { + src = path.join(TEST_DIR, 'src.txt') + fs.outputFileSync(src, 'some data') + + const destLink = path.join(TEST_DIR, 'dest-symlink') + fs.symlinkSync(src, destLink, 'file') + + fs.move(src, destLink, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + done() + }) + }) + }) + + describe(`>> when src is a symlink that points to a regular dest`, () => { + it('should throw error', done => { + dest = path.join(TEST_DIR, 'dest', 'somefile.txt') + fs.outputFileSync(dest, 'some data') + + const srcLink = path.join(TEST_DIR, 'src-symlink') + fs.symlinkSync(dest, srcLink, 'file') + + fs.move(srcLink, dest, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + + 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 error src and dest are the same', done => { + src = path.join(TEST_DIR, 'src', 'srcfile.txt') + fs.outputFileSync(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.move(srcLink, destLink, err => { + assert.strictEqual(err.message, 'Source and destination must not be the same.') + + 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() + }) + }) + }) + }) +}) From c8c5002e78f94a5a1fc0cd4cc3939e0a002dd3f3 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 9 May 2019 05:50:14 -0700 Subject: [PATCH 14/19] disable graceful-fs in copy and move tests --- lib/copy-sync/copy-sync.js | 4 +++- lib/copy/copy.js | 4 +++- lib/move-sync/__tests__/move-sync.test.js | 4 +++- lib/move-sync/move-sync.js | 4 +++- lib/move/__tests__/move.test.js | 4 +++- lib/move/move.js | 4 +++- 6 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 70f026a4..0ca35c85 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -1,6 +1,8 @@ 'use strict' -const fs = require('graceful-fs') +// TODO: enable this once graceful-fs supports bigint option. +// const fs = require('graceful-fs') +const fs = require('fs') const path = require('path') const mkdirpSync = require('../mkdirs').mkdirsSync const utimesSync = require('../util/utimes.js').utimesMillisSync diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 76c4bdb8..b5738b72 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -1,6 +1,8 @@ 'use strict' -const fs = require('graceful-fs') +// TODO: enable this once graceful-fs supports bigint option. +// const fs = require('graceful-fs') +const fs = require('fs') const path = require('path') const mkdirp = require('../mkdirs').mkdirs const pathExists = require('../path-exists').pathExists diff --git a/lib/move-sync/__tests__/move-sync.test.js b/lib/move-sync/__tests__/move-sync.test.js index 486e61d5..e4b97ddb 100644 --- a/lib/move-sync/__tests__/move-sync.test.js +++ b/lib/move-sync/__tests__/move-sync.test.js @@ -1,6 +1,8 @@ 'use strict' -const fs = require('graceful-fs') +// TODO: enable this once graceful-fs supports bigint option. +// const fs = require('graceful-fs') +const fs = require('fs') const os = require('os') const fse = require(process.cwd()) const path = require('path') diff --git a/lib/move-sync/move-sync.js b/lib/move-sync/move-sync.js index 20f910cc..62f8e1a6 100644 --- a/lib/move-sync/move-sync.js +++ b/lib/move-sync/move-sync.js @@ -1,6 +1,8 @@ 'use strict' -const fs = require('graceful-fs') +// TODO: enable this once graceful-fs supports bigint option. +// const fs = require('graceful-fs') +const fs = require('fs') const path = require('path') const copySync = require('../copy-sync').copySync const removeSync = require('../remove').removeSync diff --git a/lib/move/__tests__/move.test.js b/lib/move/__tests__/move.test.js index 23e8dfce..7ee2ac02 100644 --- a/lib/move/__tests__/move.test.js +++ b/lib/move/__tests__/move.test.js @@ -1,6 +1,8 @@ 'use strict' -const fs = require('graceful-fs') +// TODO: enable this once graceful-fs supports bigint option. +// const fs = require('graceful-fs') +const fs = require('fs') const os = require('os') const fse = require('../../') const path = require('path') diff --git a/lib/move/move.js b/lib/move/move.js index fa3ea618..94dc7949 100644 --- a/lib/move/move.js +++ b/lib/move/move.js @@ -1,6 +1,8 @@ 'use strict' -const fs = require('graceful-fs') +// TODO: enable this once graceful-fs supports bigint option. +// const fs = require('graceful-fs') +const fs = require('fs') const path = require('path') const copy = require('../copy').copy const remove = require('../remove').remove From ec208f47fd84a5915924d3cd2fa2e650a6bc9d81 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 9 May 2019 06:07:26 -0700 Subject: [PATCH 15/19] fix parsing node version --- lib/util/stat.js | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/util/stat.js b/lib/util/stat.js index a409055d..1f81afb4 100644 --- a/lib/util/stat.js +++ b/lib/util/stat.js @@ -5,11 +5,22 @@ const fs = require('fs') const path = require('path') -const NODE_VERSION_WITH_BIGINT = 1050 -const nodeVersion = Number.parseInt(process.versions.node.split('.').join(''), 10) +const NODE_VERSION_WITH_BIGINT_MAJOR = 10 +const NODE_VERSION_WITH_BIGINT_MINOR = 5 +const NODE_VERSION_WITH_BIGINT_PATCH = 0 +const nodeVersion = process.versions.node +const nodeVersionMajor = Number.parseInt(nodeVersion.split('.')[0], 10) +const nodeVersionMinor = Number.parseInt(nodeVersion.split('.')[1], 10) +const nodeVersionPatch = Number.parseInt(nodeVersion.split('.')[2], 10) + +function nodeSupportsBigInt () { + return nodeVersionMajor >= NODE_VERSION_WITH_BIGINT_MAJOR && + nodeVersionMinor >= NODE_VERSION_WITH_BIGINT_MINOR && + nodeVersionPatch >= NODE_VERSION_WITH_BIGINT_PATCH +} function getStats (src, dest, cb) { - if (nodeVersion >= NODE_VERSION_WITH_BIGINT) { + if (nodeSupportsBigInt()) { fs.stat(src, { bigint: true }, (err, srcStat) => { if (err) return cb(err) fs.stat(dest, { bigint: true }, (err, destStat) => { @@ -36,13 +47,13 @@ function getStats (src, dest, cb) { function getStatsSync (src, dest) { let srcStat, destStat - if (nodeVersion >= NODE_VERSION_WITH_BIGINT) { + if (nodeSupportsBigInt()) { srcStat = fs.statSync(src, { bigint: true }) } else { srcStat = fs.statSync(src) } try { - if (nodeVersion >= NODE_VERSION_WITH_BIGINT) { + if (nodeSupportsBigInt()) { destStat = fs.statSync(dest, { bigint: true }) } else { destStat = fs.statSync(dest) @@ -89,7 +100,7 @@ function checkParentPaths (src, srcStat, dest, funcName, cb) { (destParent === path.dirname(src) || destParent === path.parse(destParent).root) ) return cb() - if (nodeVersion >= NODE_VERSION_WITH_BIGINT) { + if (nodeSupportsBigInt()) { fs.stat(destParent, { bigint: true }, (err, destStat) => { if (err) { if (err.code === 'ENOENT') return cb() @@ -119,7 +130,7 @@ function checkParentPathsSync (src, srcStat, dest, funcName) { if (destParent && (destParent === path.dirname(src) || destParent === path.parse(destParent).root)) return let destStat try { - if (nodeVersion >= NODE_VERSION_WITH_BIGINT) { + if (nodeSupportsBigInt()) { destStat = fs.statSync(destParent, { bigint: true }) } else { destStat = fs.statSync(destParent) From 04be6bc3266b5edb290a9a9107c2c4b5e7456f50 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 9 May 2019 06:20:07 -0700 Subject: [PATCH 16/19] tiny reformat --- lib/util/stat.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/util/stat.js b/lib/util/stat.js index 1f81afb4..b098eaac 100644 --- a/lib/util/stat.js +++ b/lib/util/stat.js @@ -8,10 +8,10 @@ const path = require('path') const NODE_VERSION_WITH_BIGINT_MAJOR = 10 const NODE_VERSION_WITH_BIGINT_MINOR = 5 const NODE_VERSION_WITH_BIGINT_PATCH = 0 -const nodeVersion = process.versions.node -const nodeVersionMajor = Number.parseInt(nodeVersion.split('.')[0], 10) -const nodeVersionMinor = Number.parseInt(nodeVersion.split('.')[1], 10) -const nodeVersionPatch = Number.parseInt(nodeVersion.split('.')[2], 10) +const nodeVersion = process.versions.node.split('.') +const nodeVersionMajor = Number.parseInt(nodeVersion[0], 10) +const nodeVersionMinor = Number.parseInt(nodeVersion[1], 10) +const nodeVersionPatch = Number.parseInt(nodeVersion[2], 10) function nodeSupportsBigInt () { return nodeVersionMajor >= NODE_VERSION_WITH_BIGINT_MAJOR && From 867596c0e6bbc7bf03e24e7d6d6cff32bf130831 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 9 May 2019 06:29:48 -0700 Subject: [PATCH 17/19] update copy*() docs --- docs/copy-sync.md | 1 - docs/copy.md | 1 - 2 files changed, 2 deletions(-) diff --git a/docs/copy-sync.md b/docs/copy-sync.md index e7cc8b1e..3fca1330 100644 --- a/docs/copy-sync.md +++ b/docs/copy-sync.md @@ -7,7 +7,6 @@ Copy a file or directory. The directory can have contents. Like `cp -r`. - `options` `` - `overwrite` ``: overwrite existing file or directory, default is `true`. _Note that the copy operation will silently fail if you set this to `false` and the destination exists._ Use the `errorOnExist` option to change this behavior. - `errorOnExist` ``: when `overwrite` is `false` and the destination exists, throw an error. Default is `false`. - - `checkPathsBeforeCopying` ``: check `src` and `dest` paths before copying. Default is `true`. - `dereference` ``: dereference symlinks, default is `false`. - `preserveTimestamps` ``: When true, will set last modification and access times to the ones of the original source files. When false, timestamp behavior is OS-dependent. Default is `false`. - `filter` ``: Function to filter copied files. Return `true` to include, `false` to exclude. diff --git a/docs/copy.md b/docs/copy.md index a5ce8958..4add5684 100644 --- a/docs/copy.md +++ b/docs/copy.md @@ -7,7 +7,6 @@ Copy a file or directory. The directory can have contents. Like `cp -r`. - `options` `` - `overwrite` ``: overwrite existing file or directory, default is `true`. _Note that the copy operation will silently fail if you set this to `false` and the destination exists._ Use the `errorOnExist` option to change this behavior. - `errorOnExist` ``: when `overwrite` is `false` and the destination exists, throw an error. Default is `false`. - - `checkPathsBeforeCopying` ``: check `src` and `dest` paths before copying. Default is `true`. - `dereference` ``: dereference symlinks, default is `false`. - `preserveTimestamps` ``: When true, will set last modification and access times to the ones of the original source files. When false, timestamp behavior is OS-dependent. Default is `false`. - `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). From 6866691ead6c1d9505df36a9a3442107488a4c41 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 9 May 2019 22:59:23 -0700 Subject: [PATCH 18/19] refactor parsing node version --- lib/util/stat.js | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/util/stat.js b/lib/util/stat.js index b098eaac..5896a2bf 100644 --- a/lib/util/stat.js +++ b/lib/util/stat.js @@ -5,18 +5,27 @@ const fs = require('fs') const path = require('path') -const NODE_VERSION_WITH_BIGINT_MAJOR = 10 -const NODE_VERSION_WITH_BIGINT_MINOR = 5 -const NODE_VERSION_WITH_BIGINT_PATCH = 0 +const NODE_VERSION_MAJOR_WITH_BIGINT = 10 +const NODE_VERSION_MINOR_WITH_BIGINT = 5 +const NODE_VERSION_PATCH_WITH_BIGINT = 0 const nodeVersion = process.versions.node.split('.') const nodeVersionMajor = Number.parseInt(nodeVersion[0], 10) const nodeVersionMinor = Number.parseInt(nodeVersion[1], 10) const nodeVersionPatch = Number.parseInt(nodeVersion[2], 10) function nodeSupportsBigInt () { - return nodeVersionMajor >= NODE_VERSION_WITH_BIGINT_MAJOR && - nodeVersionMinor >= NODE_VERSION_WITH_BIGINT_MINOR && - nodeVersionPatch >= NODE_VERSION_WITH_BIGINT_PATCH + if (nodeVersionMajor > NODE_VERSION_MAJOR_WITH_BIGINT) { + return true + } else if (nodeVersionMajor === NODE_VERSION_MAJOR_WITH_BIGINT) { + if (nodeVersionMinor > NODE_VERSION_MINOR_WITH_BIGINT) { + return true + } else if (nodeVersionMinor === NODE_VERSION_MINOR_WITH_BIGINT) { + if (nodeVersionPatch >= NODE_VERSION_PATCH_WITH_BIGINT) { + return true + } + } + } + return false } function getStats (src, dest, cb) { From c80c36739ec426ca9bcc3529b7300d0f965c3dae Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Fri, 10 May 2019 00:38:27 -0700 Subject: [PATCH 19/19] use semver to parse node version in tests --- ...rve-time.test.js => copy-sync-preserve-timestamp.test.js} | 5 +++-- ...preserve-time.test.js => copy-preserve-timestamp.test.js} | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) rename lib/copy-sync/__tests__/{copy-sync-preserve-time.test.js => copy-sync-preserve-timestamp.test.js} (94%) rename lib/copy/__tests__/{copy-preserve-time.test.js => copy-preserve-timestamp.test.js} (95%) diff --git a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js b/lib/copy-sync/__tests__/copy-sync-preserve-timestamp.test.js similarity index 94% rename from lib/copy-sync/__tests__/copy-sync-preserve-time.test.js rename to lib/copy-sync/__tests__/copy-sync-preserve-timestamp.test.js index 76dbfe09..aa8318d0 100644 --- a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js +++ b/lib/copy-sync/__tests__/copy-sync-preserve-timestamp.test.js @@ -5,8 +5,9 @@ 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) +const semver = require('semver') +const nodeVersion = process.version +const nodeVersionMajor = semver.major(nodeVersion) /* global beforeEach, afterEach, describe, it */ diff --git a/lib/copy/__tests__/copy-preserve-time.test.js b/lib/copy/__tests__/copy-preserve-timestamp.test.js similarity index 95% rename from lib/copy/__tests__/copy-preserve-time.test.js rename to lib/copy/__tests__/copy-preserve-timestamp.test.js index 8c10692c..3e1e5584 100644 --- a/lib/copy/__tests__/copy-preserve-time.test.js +++ b/lib/copy/__tests__/copy-preserve-timestamp.test.js @@ -6,8 +6,9 @@ const path = require('path') const copy = require('../copy') const utimes = require('../../util/utimes') const assert = require('assert') -const nodeVersion = process.versions.node -const nodeVersionMajor = parseInt(nodeVersion.split('.')[0], 10) +const semver = require('semver') +const nodeVersion = process.version +const nodeVersionMajor = semver.major(nodeVersion) /* global beforeEach, afterEach, describe, it */