Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BREAKING] copy*(): use ino to check identical paths #582

Merged
merged 2 commits into from Jul 17, 2018
Merged

[BREAKING] copy*(): use ino to check identical paths #582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b4f3cf
copy*(): use ino to check identical paths
manidlou May 16, 2018
456241a
refactor a bit
manidlou May 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions lib/copy-sync/__tests__/broken-symlink.test.js
View file
Open in desktop
Expand Up @@ -23,9 +23,8 @@ describe('copy-sync / broken symlink', () => {

afterEach(done => fse.remove(TEST_DIR, done))

it('should copy broken symlinks by default', () => {
assert.doesNotThrow(() => copySync(src, out))
assert.strictEqual(fs.readlinkSync(path.join(out, 'broken-symlink')), path.join(src, 'does-not-exist'))
it('should error if symlink is broken', () => {
assert.throws(() => copySync(src, out))
})

it('should throw an error when dereference=true', () => {
Expand Down
21 changes: 10 additions & 11 deletions lib/copy-sync/__tests__/copy-sync-dir.test.js
View file
Open in desktop
Expand Up @@ -6,9 +6,9 @@ const path = require('path')
const assert = require('assert')
const crypto = require('crypto')

/* global beforeEach, describe, it */
/* global beforeEach, afterEach, describe, it */

describe('+ copySync()', () => {
describe('+ copySync() / dir', () => {
const SIZE = 16 * 64 * 1024 + 7
let TEST_DIR
let src, dest
Expand All @@ -20,6 +20,8 @@ describe('+ copySync()', () => {
fs.emptyDir(TEST_DIR, done)
})

afterEach(done => fs.remove(TEST_DIR, done))

describe('> when src is a directory', () => {
describe('> when dest exists and is a file', () => {
it('should throw error', () => {
Expand Down Expand Up @@ -70,13 +72,15 @@ describe('+ copySync()', () => {
})

it('should preserve symbolic links', () => {
const srcTarget = path.join(TEST_DIR, 'destination')
fs.mkdirSync(src)
fs.symlinkSync('destination', path.join(src, 'symlink'))
fs.mkdirSync(srcTarget)
fs.symlinkSync(srcTarget, path.join(src, 'symlink'))

fs.copySync(src, dest)

const link = fs.readlinkSync(path.join(dest, 'symlink'))
assert.strictEqual(link, 'destination')
assert.strictEqual(link, srcTarget)
})

describe('> when the destination dir does not exist', () => {
Expand Down Expand Up @@ -183,14 +187,9 @@ describe('+ copySync()', () => {
const dest = path.join(TEST_DIR, 'dest')

setTimeout(() => {
fs.mkdirSync(src)
fs.writeFileSync(path.join(src, 'somefile.html'), 'some data')
fs.outputFileSync(path.join(src, 'somefile.html'), 'some data')
fs.mkdirSync(dest)
try {
fs.copySync(src, dest, filter)
} catch (err) {
assert.ifError(err)
}
fs.copySync(src, dest, filter)
assert(!fs.existsSync(path.join(dest, 'somefile.html')))
done()
}, 1000)
Expand Down
4 changes: 2 additions & 2 deletions lib/copy-sync/__tests__/copy-sync-file.test.js
View file
Open in desktop
Expand Up @@ -10,11 +10,11 @@ const crypto = require('crypto')

const SIZE = 16 * 64 * 1024 + 7

describe('+ copySync()', () => {
describe('+ copySync() / file', () => {
let TEST_DIR

beforeEach(done => {
TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync')
TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-file')
fs.emptyDir(TEST_DIR, done)
})

Expand Down
98 changes: 90 additions & 8 deletions lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js
View file
Open in desktop
Expand Up @@ -23,13 +23,89 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
it('should return an error if src and dest are the same', () => {
const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync')
const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync')
fs.ensureFileSync(fileSrc)

try {
fs.copySync(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.copySync(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.copySync(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.copySync(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.copySync(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
Expand Down Expand Up @@ -90,7 +166,7 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
})

describe('>> when src and dest are symlinks that point to the exact same path', () => {
it('should not copy and return', () => {
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')
Expand All @@ -101,7 +177,11 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
const srclenBefore = klawSync(srcLink).length
const destlenBefore = klawSync(destLink).length

fs.copySync(srcLink, destLink)
try {
fs.copySync(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')
Expand Down Expand Up @@ -146,8 +226,7 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
describe(`>> when src is a symlink that points to a regular dest`, () => {
it('should throw error', () => {
dest = path.join(TEST_DIR, 'dest', 'somefile.txt')
fs.ensureFileSync(dest)
fs.writeFileSync(dest, 'some data')
fs.outputFileSync(dest, 'some data')

const srcLink = path.join(TEST_DIR, 'src-symlink')
fs.symlinkSync(dest, srcLink, 'file')
Expand All @@ -164,18 +243,21 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
})

describe('>> when src and dest are symlinks that point to the exact same path', () => {
it('should not copy and return', () => {
it('should error src and dest are the same', () => {
src = path.join(TEST_DIR, 'src', 'srcfile.txt')
fs.ensureFileSync(src)
fs.writeFileSync(src, 'src data')
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.copySync(srcLink, destLink)
try {
fs.copySync(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)
Expand Down
48 changes: 27 additions & 21 deletions lib/copy-sync/__tests__/copy-sync-prevent-copying-into-itself.test.js
View file
Open in desktop
Expand Up @@ -39,7 +39,7 @@ describe('+ copySync() - prevent copying into itself', () => {
afterEach(done => fs.remove(TEST_DIR, done))

describe('> when source is a file', () => {
it('should copy the file successfully even when dest parent is a subdir of src', () => {
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)
Expand Down Expand Up @@ -273,7 +273,7 @@ describe('+ copySync() - prevent copying into itself', () => {
})

describe('>> when dest is a symlink', () => {
it('should silently return when resolved dest path is exactly the same as resolved src path', () => {
it('should error when resolved dest path is exactly the same as resolved src path', () => {
const srcLink = path.join(TEST_DIR, 'src-symlink')
fs.symlinkSync(src, srcLink, 'dir')
const destLink = path.join(TEST_DIR, 'dest-symlink')
Expand All @@ -284,16 +284,21 @@ describe('+ copySync() - prevent copying into itself', () => {
assert(srclenBefore > 2)
assert(destlenBefore > 2)

fs.copySync(srcLink, destLink)
const srclenAfter = klawSync(srcLink).length
assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change')
const destlenAfter = klawSync(destLink).length
assert.strictEqual(destlenAfter, destlenBefore, 'dest length should not change')

const srcln = fs.readlinkSync(srcLink)
assert.strictEqual(srcln, src)
const destln = fs.readlinkSync(destLink)
assert.strictEqual(destln, src)
try {
fs.copySync(srcLink, destLink)
} catch (err) {
assert.strictEqual(err.message, 'Source and destination must not be the same.')
} finally {
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)
}
})

it('should error when resolved dest path is a subdir of resolved src path', () => {
Expand All @@ -309,10 +314,11 @@ describe('+ copySync() - prevent copying into itself', () => {
try {
fs.copySync(srcLink, destLink)
} catch (err) {
assert(err)
assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${resolvedDestPath}'.`)
} finally {
const destln = fs.readlinkSync(destLink)
assert.strictEqual(destln, resolvedDestPath)
}
const destln = fs.readlinkSync(destLink)
assert.strictEqual(destln, src)
})

it('should error when resolved src path is a subdir of resolved dest path', () => {
Expand All @@ -322,18 +328,18 @@ describe('+ copySync() - prevent copying into itself', () => {

const dest = path.join(TEST_DIR, 'dest')

fs.mkdirSync(dest)

fs.symlinkSync(srcInDest, srcLink, 'dir')
fs.symlinkSync(dest, destLink, 'dir')
fs.ensureDirSync(srcInDest)
fs.ensureSymlinkSync(srcInDest, srcLink, 'dir')
fs.ensureSymlinkSync(dest, destLink, 'dir')

try {
fs.copySync(srcLink, destLink)
} catch (err) {
assert.strictEqual(err.message, `Cannot overwrite '${dest}' with '${srcInDest}'.`)
} finally {
const destln = fs.readlinkSync(destLink)
assert.strictEqual(destln, dest)
}
const destln = fs.readlinkSync(destLink)
assert.strictEqual(destln, dest)
})
})
})
Expand Down