From 842dcf3264fcd9da37e6336dd98c650e3f555d80 Mon Sep 17 00:00:00 2001
From: Rongjian Zhang <pd4d10@gmail.com>
Date: Fri, 22 Nov 2019 14:01:21 +0800
Subject: [PATCH 1/2] fs: fix existsSync for invalid symlink at win32

Fixes: https://github.com/nodejs/node/issues/30538
---
 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 a686b01fff072d..f6324a131fd5e4 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -228,7 +228,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);
+
+  // If it is an invalid symlink, existsSync should return false
+  // binding.access checking is not enough for win32 (return true in this case)
+  // So we need to call binding.stat for double checking
+  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',

From 521f4331cacf54c1a6a69a1e866718ab4822f89c Mon Sep 17 00:00:00 2001
From: Rongjian Zhang <pd4d10@gmail.com>
Date: Sat, 23 Nov 2019 01:43:20 +0800
Subject: [PATCH 2/2] fixup

---
 lib/fs.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/fs.js b/lib/fs.js
index f6324a131fd5e4..2f246515404168 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -231,9 +231,9 @@ function existsSync(path) {
   const nPath = pathModule.toNamespacedPath(path);
   binding.access(nPath, F_OK, undefined, ctx);
 
-  // If it is an invalid symlink, existsSync should return false
-  // binding.access checking is not enough for win32 (return true in this case)
-  // So we need to call binding.stat for double checking
+  // 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);
   }