From 60b1e1ad6197d3f4db94b33a8adb4d6569bc7114 Mon Sep 17 00:00:00 2001 From: Rongjian Zhang Date: Fri, 22 Nov 2019 14:01:21 +0800 Subject: [PATCH] fs: fix existsSync for invalid symlink at win32 Fixes: https://github.com/nodejs/node/issues/30538 PR-URL: https://github.com/nodejs/node/pull/30556 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- lib/fs.js | 11 +++++++++- test/parallel/test-fs-symlink-dir-junction.js | 17 ++++++++++++++ test/parallel/test-fs-symlink-dir.js | 22 +++++++++++++++++++ test/parallel/test-fs-symlink.js | 12 ++++++++++ 4 files changed, 61 insertions(+), 1 deletion(-) diff --git a/lib/fs.js b/lib/fs.js index 6a5ebb8203af15..de37a0c2a0a704 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -234,7 +234,16 @@ function existsSync(path) { return false; } const ctx = { path }; - binding.access(pathModule.toNamespacedPath(path), F_OK, undefined, ctx); + const nPath = pathModule.toNamespacedPath(path); + binding.access(nPath, F_OK, undefined, ctx); + + // In case of an invalid symlink, `binding.access()` on win32 + // will **not** return an error and is therefore not enough. + // Double check with `binding.stat()`. + if (isWindows && ctx.errno === undefined) { + binding.stat(nPath, false, undefined, ctx); + } + return ctx.errno === undefined; } diff --git a/test/parallel/test-fs-symlink-dir-junction.js b/test/parallel/test-fs-symlink-dir-junction.js index fc89ad368401e4..42d6bc1214132b 100644 --- a/test/parallel/test-fs-symlink-dir-junction.js +++ b/test/parallel/test-fs-symlink-dir-junction.js @@ -53,3 +53,20 @@ fs.symlink(linkData, linkPath, 'junction', common.mustCall(function(err) { })); })); })); + +// Test invalid symlink +{ + const linkData = fixtures.path('/not/exists/dir'); + const linkPath = path.join(tmpdir.path, 'invalid_junction_link'); + + fs.symlink(linkData, linkPath, 'junction', common.mustCall(function(err) { + assert.ifError(err); + + assert(!fs.existsSync(linkPath)); + + fs.unlink(linkPath, common.mustCall(function(err) { + assert.ifError(err); + assert(!fs.existsSync(linkPath)); + })); + })); +} diff --git a/test/parallel/test-fs-symlink-dir.js b/test/parallel/test-fs-symlink-dir.js index 1ab1361a43fb9b..707bc5b486d723 100644 --- a/test/parallel/test-fs-symlink-dir.js +++ b/test/parallel/test-fs-symlink-dir.js @@ -44,3 +44,25 @@ for (const linkTarget of linkTargets) { testAsync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-async`); } } + +// Test invalid symlink +{ + function testSync(target, path) { + fs.symlinkSync(target, path); + assert(!fs.existsSync(path)); + } + + function testAsync(target, path) { + fs.symlink(target, path, common.mustCall((err) => { + assert.ifError(err); + assert(!fs.existsSync(path)); + })); + } + + for (const linkTarget of linkTargets.map((p) => p + '-broken')) { + for (const linkPath of linkPaths) { + testSync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-sync`); + testAsync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-async`); + } + } +} diff --git a/test/parallel/test-fs-symlink.js b/test/parallel/test-fs-symlink.js index a6001103190aad..c52ffbc105ffae 100644 --- a/test/parallel/test-fs-symlink.js +++ b/test/parallel/test-fs-symlink.js @@ -58,6 +58,18 @@ fs.symlink(linkData, linkPath, common.mustCall(function(err) { })); })); +// Test invalid symlink +{ + const linkData = fixtures.path('/not/exists/file'); + const linkPath = path.join(tmpdir.path, 'symlink2.js'); + + fs.symlink(linkData, linkPath, common.mustCall(function(err) { + assert.ifError(err); + + assert(!fs.existsSync(linkPath)); + })); +} + [false, 1, {}, [], null, undefined].forEach((input) => { const errObj = { code: 'ERR_INVALID_ARG_TYPE',