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.

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

Already on GitHub? Sign in to your account

win, fs: detect if symlink target is a directory #23724

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand All @@ -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`.

Expand Down
33 changes: 32 additions & 1 deletion lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
refack marked this conversation as resolved.
Show resolved Hide resolved
} 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 { }
}
target = toPathIfFileURL(target);
path = toPathIfFileURL(path);
validatePath(target, 'target');
Expand Down
46 changes: 46 additions & 0 deletions test/parallel/test-fs-symlink-dir.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';
const common = require('../common');

refack marked this conversation as resolved.
Show resolved Hide resolved
// 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


refack marked this conversation as resolved.
Show resolved Hide resolved
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();

refack marked this conversation as resolved.
Show resolved Hide resolved
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`);
}
}