win, fs: detect if symlink target is a directory

On Windows creating a symlink to a directory will not work unless extra 'dir' parameter is passed. This adds a check if link target is a directory, and if so automatically use 'dir' when creating symlink. Ref: #23691
nodejs · bzoz · Oct 18, 2018 · Nov 29, 2018 · Nov 29, 2018 · 7abe6235301da62d9d29d6579d71e08082a1b9b7
commit 7abe6235301da62d9d29d6579d71e08082a1b9b7
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -3028,20 +3028,26 @@ changes:
     description: The `target` and `path` parameters can be WHATWG `URL` objects
                  using `file:` protocol. Support is currently still
                  *experimental*.
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/23724
+    description: If the `type` argument is left undefined, Node will autodetect
+                 `target` type and automatically select `dir` or `file`
 -->
 
 * `target` {string|Buffer|URL}
 * `path` {string|Buffer|URL}
-* `type` {string} **Default:** `'file'`
+* `type` {string}
 * `callback` {Function}
   * `err` {Error}
 
 Asynchronous symlink(2). No arguments other than a possible exception are given
-to the completion callback. The `type` argument can be set to `'dir'`,
-`'file'`, or `'junction'` and is only available on
-Windows (ignored on other platforms). Windows junction points require the
-destination path to be absolute. When using `'junction'`, the `target` argument
-will automatically be normalized to absolute path.
+to the completion callback. The `type` argument is only available on Windows
+and ignored on other platforms. It can be set to `'dir'`, `'file'`, or
+`'junction'`. If the `type` argument is not set, Node will autodetect `target`
+type and use `'file'` or `'dir'`. If the `target` does not exist, `'file'` will
+be used. Windows junction points require the destination path to be absolute.
+When using `'junction'`, the `target` argument will automatically be normalized
+to absolute path.
 
 Here is an example below:
 
@@ -3060,11 +3066,15 @@ changes:
     description: The `target` and `path` parameters can be WHATWG `URL` objects
                  using `file:` protocol. Support is currently still
                  *experimental*.
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/23724
+    description: If the `type` argument is left undefined, Node will autodetect
+                 `target` type and automatically select `dir` or `file`
 -->
 
 * `target` {string|Buffer|URL}
 * `path` {string|Buffer|URL}
-* `type` {string} **Default:** `'file'`
+* `type` {string}
 
 Returns `undefined`.
 

diff --git a/lib/fs.js b/lib/fs.js
@@ -905,16 +905,47 @@ function symlink(target, path, type_, callback_) {
   validatePath(target, 'target');
   validatePath(path);
 
-  const flags = stringToSymlinkType(type);
   const req = new FSReqCallback();
   req.oncomplete = callback;
 
+  if (isWindows && type === null) {
+    let absoluteTarget;
+    try {
+      // Symlinks targets can be relative to the newly created path.
+      // Calculate absolute file name of the symlink target, and check
+      // if it is a directory. Ignore resolve error to keep symlink
+      // errors consistent between platforms if invalid path is
+      // provided.
+      absoluteTarget = pathModule.resolve(path, '..', target);
+    } catch (err) { } // eslint-disable-line no-unused-vars
-    } catch (err) { } // eslint-disable-line no-unused-vars
+    } catch { } 
-    } catch (err) { } // eslint-disable-line no-unused-vars
+    } catch { } 
+    if (absoluteTarget !== undefined) {
+      stat(absoluteTarget, (err, stat) => {
+        const resolvedType = !err && stat.isDirectory() ? 'dir' : 'file';
+        const resolvedFlags = stringToSymlinkType(resolvedType);
+        binding.symlink(preprocessSymlinkDestination(target,
+                                                     resolvedType,
+                                                     path),
+                        pathModule.toNamespacedPath(path), resolvedFlags, req);
+      });
+      return;
+    }
+  }
+
+  const flags = stringToSymlinkType(type);
   binding.symlink(preprocessSymlinkDestination(target, type, path),
                   pathModule.toNamespacedPath(path), flags, req);
 }
 
 function symlinkSync(target, path, type) {
   type = (typeof type === 'string' ? type : null);
+  if (isWindows && type === null) {
+    try {
+      const absoluteTarget = pathModule.resolve(path, '..', target);
+      if (statSync(absoluteTarget).isDirectory()) {
+        type = 'dir';
+      }
+    } catch (err) { } // eslint-disable-line no-unused-vars
-    } catch (err) { } // eslint-disable-line no-unused-vars
+    } catch { }
-    } catch (err) { } // eslint-disable-line no-unused-vars
+    } catch { }
+  }
   target = toPathIfFileURL(target);
   path = toPathIfFileURL(path);
   validatePath(target, 'target');

diff --git a/test/parallel/test-fs-symlink-dir.js b/test/parallel/test-fs-symlink-dir.js
@@ -0,0 +1,46 @@
+'use strict';
+const common = require('../common');
+
+// Test creating a symbolic link pointing to a directory.
+// Ref: https://github.com/nodejs/node/pull/23724
+// Ref: https://github.com/nodejs/node/issues/23596
+
+
+if (!common.canCreateSymLink())
+  common.skip('insufficient privileges');
+
+const assert = require('assert');
+const path = require('path');
+const fs = require('fs');
+
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
+
+const linkTargets = [
+  'relative-target',
+  path.join(tmpdir.path, 'absolute-target')
+];
+const linkPaths = [
+  path.relative(process.cwd(), path.join(tmpdir.path, 'relative-path')),
+  path.join(tmpdir.path, 'absolute-path')
+];
+
+function testSync(target, path) {
+  fs.symlinkSync(target, path);
+  fs.readdirSync(path);
+}
+
+function testAsync(target, path) {
+  fs.symlink(target, path, common.mustCall((err) => {
+    assert.ifError(err);
+    fs.readdirSync(path);
+  }));
+}
+
+for (const linkTarget of linkTargets) {
+  fs.mkdirSync(path.resolve(tmpdir.path, linkTarget));
+  for (const linkPath of linkPaths) {
+    testSync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-sync`);
+    testAsync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-async`);
+  }
+}