diff --git a/lib/ensure/__tests__/symlink.test.js b/lib/ensure/__tests__/symlink.test.js index cec7e9dd..92222738 100644 --- a/lib/ensure/__tests__/symlink.test.js +++ b/lib/ensure/__tests__/symlink.test.js @@ -7,6 +7,8 @@ const os = require('os') const fse = require('../..') const path = require('path') const assert = require('assert') +const proxyquire = require('proxyquire') +const realFs = require('../../fs') const _symlinkPaths = require('../symlink-paths') const symlinkPathsSync = _symlinkPaths.symlinkPathsSync const ensureSymlink = fse.ensureSymlink @@ -399,4 +401,75 @@ describe('fse-ensure-symlink', () => { assert.strictEqual(content, 'content') }) }) + + // https://github.com/jprichardson/node-fs-extra/issues/925 + describe('ensureSymlink() when dest is an existing broken symlink (issue #925)', () => { + it('should fail with EEXIST (not ENOENT) when dest is a broken symlink', async () => { + const srcpath = path.join(TEST_DIR, 'src-925.txt') + const dstpath = path.join(TEST_DIR, 'broken-link-925') + fs.writeFileSync(srcpath, 'src\n') + fs.symlinkSync(path.join(TEST_DIR, 'missing-925.txt'), dstpath) + assert.strictEqual(fs.lstatSync(dstpath).isSymbolicLink(), true) + + await assert.rejects(ensureSymlink(srcpath, dstpath), { code: 'EEXIST' }) + }) + + it('should fail with EEXIST (not ENOENT) when dest is a broken symlink (sync)', () => { + const srcpath = path.join(TEST_DIR, 'src-925-sync.txt') + const dstpath = path.join(TEST_DIR, 'broken-link-925-sync') + fs.writeFileSync(srcpath, 'src\n') + fs.symlinkSync(path.join(TEST_DIR, 'missing-925-sync.txt'), dstpath) + assert.strictEqual(fs.lstatSync(dstpath).isSymbolicLink(), true) + + assert.throws(() => ensureSymlinkSync(srcpath, dstpath), { code: 'EEXIST' }) + }) + }) + + // When checking whether an existing dest symlink points at src, only an + // ENOENT (broken link) should be swallowed. Any other error must propagate. + describe('ensureSymlink() when statting the existing dest fails for a non-ENOENT reason', () => { + it('should rethrow the error instead of swallowing it', async () => { + const srcpath = path.join(TEST_DIR, 'src-eacces.txt') + const dstpath = path.join(TEST_DIR, 'dst-symlink-eacces') + fs.writeFileSync(srcpath, 'src\n') + fs.symlinkSync(path.join(TEST_DIR, 'missing-eacces.txt'), dstpath) + assert.strictEqual(fs.lstatSync(dstpath).isSymbolicLink(), true) + + const fsStub = { + stat (p, ...args) { + if (p === dstpath) { + const err = new Error('EACCES: permission denied, stat') + err.code = 'EACCES' + return Promise.reject(err) + } + return realFs.stat(p, ...args) + } + } + const { createSymlink } = proxyquire('../symlink', { '../fs': fsStub }) + + await assert.rejects(createSymlink(srcpath, dstpath), { code: 'EACCES' }) + }) + + it('should rethrow the error instead of swallowing it (sync)', () => { + const srcpath = path.join(TEST_DIR, 'src-eacces-sync.txt') + const dstpath = path.join(TEST_DIR, 'dst-symlink-eacces-sync') + fs.writeFileSync(srcpath, 'src\n') + fs.symlinkSync(path.join(TEST_DIR, 'missing-eacces-sync.txt'), dstpath) + assert.strictEqual(fs.lstatSync(dstpath).isSymbolicLink(), true) + + const fsStub = { + statSync (p, ...args) { + if (p === dstpath) { + const err = new Error('EACCES: permission denied, stat') + err.code = 'EACCES' + throw err + } + return realFs.statSync(p, ...args) + } + } + const { createSymlinkSync } = proxyquire('../symlink', { '../fs': fsStub }) + + assert.throws(() => createSymlinkSync(srcpath, dstpath), { code: 'EACCES' }) + }) + }) }) diff --git a/lib/ensure/symlink.js b/lib/ensure/symlink.js index a7de8cad..295b236f 100644 --- a/lib/ensure/symlink.js +++ b/lib/ensure/symlink.js @@ -35,8 +35,16 @@ async function createSymlink (srcpath, dstpath, type) { } } - const dstStat = await fs.stat(dstpath, { bigint: true }) - if (areIdentical(srcStat, dstStat)) return + // Following the existing symlink fails with ENOENT if it's broken; in that + // case fall through so fs.symlink reports EEXIST, rather than leaking the + // stat ENOENT. Any other error is unexpected, so rethrow it. + let dstStat + try { + dstStat = await fs.stat(dstpath, { bigint: true }) + } catch (err) { + if (err.code !== 'ENOENT') throw err + } + if (dstStat && areIdentical(srcStat, dstStat)) return } const relative = await symlinkPaths(srcpath, dstpath) @@ -72,8 +80,16 @@ function createSymlinkSync (srcpath, dstpath, type) { } } - const dstStat = fs.statSync(dstpath, { bigint: true }) - if (areIdentical(srcStat, dstStat)) return + // Following the existing symlink fails with ENOENT if it's broken; in that + // case fall through so fs.symlinkSync reports EEXIST, rather than leaking the + // stat ENOENT. Any other error is unexpected, so rethrow it. + let dstStat + try { + dstStat = fs.statSync(dstpath, { bigint: true }) + } catch (err) { + if (err.code !== 'ENOENT') throw err + } + if (dstStat && areIdentical(srcStat, dstStat)) return } const relative = symlinkPathsSync(srcpath, dstpath)