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

bzoz · 2018-10-18T00:46:01Z

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 the symlink.

Ref: #23691

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

nodejs-github-bot · 2018-10-18T00:46:02Z

@bzoz build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1278/pipeline

bzoz · 2018-10-18T00:46:27Z

CI: https://ci.nodejs.org/job/node-test-pull-request/17954/

refack

Looks good, but I left some comments

lib/fs.js

test/parallel/test-fs-symlink-dir.js

lib/fs.js

test/parallel/test-fs-symlink-dir.js

lib/fs.js

test/parallel/test-fs-symlink-dir.js

refack · 2018-10-18T20:45:51Z

P.S. I'm not sure this is semver-major if we only change a case that failed into a case that works.

bzoz · 2018-10-23T16:06:15Z

Updated, PTAL

refack

All changes LGTM.
But I forget to mentions that async calls need common.mustCall

refack · 2018-10-23T16:18:29Z

test/parallel/test-fs-symlink-dir.js

+}
+
+function async(target, path) {
+  fs.symlink(target, path, (err) => {


Suggested change

fs.symlink(target, path, (err) => {

fs.symlink(target, path, common.mustCall((err) => {

changes LGTM

bzoz · 2018-10-24T09:44:38Z

Updated with mustCall, CI: https://ci.nodejs.org/job/node-test-pull-request/18101/

refack · 2018-10-24T14:26:45Z

lint:

05:47:27 not ok 15 - /usr/home/iojs/build/workspace/node-test-linter/test/parallel/test-fs-symlink-dir.js
05:47:27   ---
05:47:27   message: '''linkTarget'' is never reassigned. Use ''const'' instead.'
05:47:27   severity: error
05:47:27   data:
05:47:27     line: 37
05:47:27     column: 10
05:47:27     ruleId: prefer-const
05:47:27   messages:
05:47:27     - message: '''linkPath'' is never reassigned. Use ''const'' instead.'
05:47:27       severity: error
05:47:27       data:
05:47:27         line: 39
05:47:27         column: 12
05:47:27         ruleId: prefer-const
05:47:27   ...

Also test fails when run in worker.

refack

LGTM with lint removed

test/parallel/test-fs-symlink-dir.js

refack · 2018-10-24T14:30:47Z

/CC @nodejs/release is this semver-major or minor?

bzoz · 2018-10-25T11:05:09Z

Updated, linter should pass, and the test does not use chdir any more: https://ci.nodejs.org/job/node-test-pull-request/18136/

refack

Also a good solution

refack · 2018-10-25T13:04:30Z

Fail on windows:

07:57:48 not ok 529 parallel/test-worker-uncaught-exception
07:57:48     timeout
07:57:48     foo

Seems unrelated -> #23873
Resume: https://ci.nodejs.org/job/node-test-pull-request/18146/

bzoz · 2018-11-01T09:39:11Z

Resumed CI is green

bzoz · 2018-11-08T09:51:13Z

@refack: If a user creates a symlink to a folder, removes the folder and creates a file with the same name as the folder, currently the symlink will work, and the user will be able to access the file. With this change, the symlink will be broken. It's an obscure corner case, but still, this is why I think its semver-major.

refack · 2018-11-08T15:41:50Z

@nodejs/TSC PTAL (bug fix with some semver-major edge cases)

bzoz · 2018-11-20T14:58:41Z

@nodejs/tsc PTAL

bzoz · 2018-11-22T10:36:58Z

The git node thingy complains that I need 2 TSC approvals for semver-major change.. so, ping?

rvagg · 2018-11-22T10:52:57Z

OK, so a few questions:

What happens if you supply type='file' for a directory and vice versa on Windows? (background I don't understand)
With this change, what's the point of the type argument now? Is that just useful for junction? Or does the answer to the question above suggest other novel uses?
Should the docs be updated to reflect this new behaviour? They currently say the default is 'file' which it clearly isn't after this change and may not even be before this change (although again, I'm missing background on how Windows does symlinks).

bzoz · 2018-11-22T11:23:43Z

type='file' for a target that is a directory:
- fs.readdir or cd/dir in the shell will not work
- if there is a subfolder in the driectory both fs.readdir('link/subdir') and cd link\subdir/dir link/subdir in the shell will work
type='dir' to a target that is a file: nothing works
junction and to override the detection. If the target does not exists, 'file' will be used by default, if one knows that the link will point to a directory, the type argument can be used
Yeah, it should. I'm on it.

rvagg · 2018-11-22T11:30:54Z

OK, that blows my mind a little bit. But I think the take-away is that the flexibility might be handy for obscure cases but doing best-guess for the user is going to be optimal. Sounds good then.

bzoz · 2018-11-22T11:36:52Z

Updated with doc update, PTAL

rvagg · 2018-11-22T11:41:03Z

doc/api/fs.md

-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`


comma between "set" and "Node"

rvagg · 2018-11-22T11:41:16Z

doc/api/fs.md

+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 exits, `'file'` will


"exits" -> "exist"

mcollina

LGTM with a a passing CITGM

mcollina · 2018-11-23T11:07:11Z

test/parallel/test-fs-symlink-dir.js

+  fs.readdirSync(path);
+}
+
+function async(target, path) {


nit: I would prefer this function not to be named as language keyword.

bzoz · 2018-11-29T07:56:44Z

Updated,
CI: https://ci.nodejs.org/job/node-test-pull-request/19030/

bzoz · 2018-11-29T10:14:37Z

Linter fluke? Resumed CI: https://ci.nodejs.org/job/node-test-pull-request/19038/

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: nodejs#23691

Trott · 2018-11-29T14:04:26Z

lib/fs.js

+      // errors consistent between platforms if invalid path is
+      // provided.
+      absoluteTarget = pathModule.resolve(path, '..', target);
+    } catch (err) { } // eslint-disable-line no-unused-vars


Any reason not to use this instead?:

Suggested change

} catch (err) { } // eslint-disable-line no-unused-vars

} catch { }

Then you don't need to disable the ESLint rule. The usual reason to avoid that catch syntax would be to simplify backporting to 8.x and 6.x but this is a semver-major so that's not an issue here.

Trott · 2018-11-29T14:05:16Z

lib/fs.js

+      if (statSync(absoluteTarget).isDirectory()) {
+        type = 'dir';
+      }
+    } catch (err) { } // eslint-disable-line no-unused-vars


Suggested change

} catch (err) { } // eslint-disable-line no-unused-vars

} catch { }

Trott · 2018-11-29T19:29:16Z

Landed in cda6b20

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. PR-URL: nodejs#23724 Refs: nodejs#23691 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Notable changes: * assert: * improve performance to instantiate errors (Ruben Bridgewater) [#26738](#26738) * validate required arguments (Ruben Bridgewater) [#26641](#26641) * adjust loose assertions (Ruben Bridgewater) [#25008](#25008) * async_hooks: * remove deprecated emitBefore and emitAfter (Matteo Collina) [#26530](#26530) * remove promise object from resource (Andreas Madsen) [#23443](#23443) * bootstrap * make Buffer and process non-enumerable (Ruben Bridgewater) [#24874](#24874) * buffer: * use stricter range checks (Ruben Bridgewater) [#27045](#27045) * harden SlowBuffer creation (ZYSzys) [#26272](#26272) * harden validation of buffer allocation size (ZYSzys) [#26162](#26162) * do proper error propagation in addon methods (Anna Henningsen) [#23939](#23939) * child_process: * change the defaults maxBuffer size (kohta ito) [#27179](#27179) * harden fork arguments validation (ZYSzys) [#27039](#27039) * use non-infinite maxBuffer defaults (kohta ito) [#23027](#23027) * console: * don't use ANSI escape codes when TERM=dumb (Vladislav Kaminsky) [#26261](#26261) * crypto: * remove legacy native handles (Tobias Nießen) [#27011](#27011) * decode missing passphrase errors (Tobias Nießen) [#25208](#25208) * move DEP0113 to End-of-Life (Tobias Nießen) [#26249](#26249) * remove deprecated crypto.\_toBuf (Tobias Nießen) [#25338](#25338) * set `DEFAULT\_ENCODING` property to non-enumerable (Antoine du Hamel) [#23222](#23222) * deps: * silence irrelevant V8 warning (Michaël Zasso) [#26685](#26685) * update postmortem metadata generation script (cjihrig) [#26685](#26685) * V8: un-cherry-pick bd019bd (Refael Ackermann) [#26685](#26685) * V8: cherry-pick 6 commits (Michaël Zasso) [#26685](#26685) * V8: cherry-pick d82c9af (Anna Henningsen) [#26685](#26685) * V8: cherry-pick e5f01ba (Anna Henningsen) [#26685](#26685) * V8: cherry-pick d5f08e4 (Anna Henningsen) [#26685](#26685) * V8: cherry-pick 6b09d21 (Anna Henningsen) [#26685](#26685) * V8: cherry-pick f0bb5d2 (Anna Henningsen) [#26685](#26685) * V8: cherry-pick 5b0510d (Anna Henningsen) [#26685](#26685) * V8: cherry-pick 91f0cd0 (Anna Henningsen) [#26685](#26685) * V8: cherry-pick 392316d (Anna Henningsen) [#26685](#26685) * V8: cherry-pick 2f79d68 (Anna Henningsen) [#26685](#26685) * sync V8 gypfiles with 7.4 (Ujjwal Sharma) [#26685](#26685) * update V8 to 7.4.288.13 (Ujjwal Sharma) [#26685](#26685) * bump minimum icu version to 63 (Ujjwal Sharma) [#25852](#25852) * silence irrelevant V8 warnings (Michaël Zasso) [#25852](#25852) * V8: cherry-pick 7803fa6 (Jon Kunkee) [#25852](#25852) * V8: cherry-pick 58cefed (Jon Kunkee) [#25852](#25852) * V8: cherry-pick d3308d0 (Michaël Zasso) [#25852](#25852) * V8: cherry-pick 74571c8 (Michaël Zasso) [#25852](#25852) * cherry-pick fc0ddf5 from upstream V8 (Anna Henningsen) [#25852](#25852) * sync V8 gypfiles with 7.3 (Ujjwal Sharma) [#25852](#25852) * sync V8 gypfiles with 7.2 (Michaël Zasso) [#25852](#25852) * update V8 to 7.3.492.25 (Michaël Zasso) [#25852](#25852) * add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) [#19794](#19794) * sync V8 gypfiles with 7.1 (Refael Ackermann) [#23423](#23423) * update V8 to 7.1.302.28 (Michaël Zasso) [#23423](#23423) * doc: * update behaviour of fs.writeFile (Sakthipriyan Vairamani (thefourtheye)) [#25080](#25080) * add internal functionality details of util.inherits (Ruben Bridgewater) [#24755](#24755) * errors: * update error name (Ruben Bridgewater) [#26738](#26738) * fs: * use proper .destroy() implementation for SyncWriteStream (Matteo Collina) [#26690](#26690) * improve mode validation (Ruben Bridgewater) [#26575](#26575) * harden validation of start option in createWriteStream (ZYSzys) [#25579](#25579) * make writeFile consistent with readFile wrt fd (Sakthipriyan Vairamani (thefourtheye)) [#23709](#23709) * http: * validate timeout in ClientRequest() (cjihrig) [#26214](#26214) * return HTTP 431 on HPE\_HEADER\_OVERFLOW error (Albert Still) [#25605](#25605) * switch default parser to llhttp (Anna Henningsen) [#24870](#24870) * change DEP0066 to a runtime deprecation (Morgan Roderick) [#24167](#24167) * else case is not reachable (szabolcsit) [#24176](#24176) * lib: * move DEP0021 to end of life (cjihrig) [#27127](#27127) * remove Atomics.wake (Gus Caplan) [#27033](#27033) * validate Error.captureStackTrace() calls (Ruben Bridgewater) [#26738](#26738) * refactor Error.captureStackTrace() usage (Ruben Bridgewater) [#26738](#26738) * move DTRACE\_\* probes out of global scope (James M Snell) [#26541](#26541) * deprecate \_stream\_wrap (Sam Roberts) [#26245] (#26245) * don't use `util.inspect()` internals (Ruben Bridgewater) [#24971](#24971) * improve error message for MODULE\_NOT\_FOUND (Ali Ijaz Sheikh) [#25690](#25690) * requireStack property for MODULE\_NOT\_FOUND (Ali Ijaz Sheikh) [#25690](#25690) * move DEP0029 to end of life (cjihrig) [#25377](#25377) * move DEP0028 to end of life (cjihrig) [#25377](#25377) * move DEP0027 to end of life (cjihrig) [#25377](#25377) * move DEP0026 to end of life (cjihrig) [#25377](#25377) * move DEP0023 to end of life (cjihrig) [#25280](#25280) * move DEP0006 to end of life (cjihrig) [#25279](#25279) * remove unintended access to deps/ (Anna Henningsen) [#25138](#25138) * move DEP0120 to end of life (cjihrig) [#24862](#24862) * use ES6 class inheritance style (Ruben Bridgewater) [#24755](#24755) * remove `inherits()` usage (Ruben Bridgewater) [#24755](#24755) * module: * remove dead code (Ruben Bridgewater) [#26983](#26983) * mark DEP0019 as End-of-Life (Ruben Bridgewater) [#26973](#26973) * throw an error for invalid package.json main entries (Ruben Bridgewater) [#26823](#26823) * don't search in require.resolve.paths (cjihrig) [#23683](#23683) * n-api: * remove code from error name (Ruben Bridgewater) [#26738](#26738) * net: * do not manipulate potential user code (Ruben Bridgewater) [#26751](#26751) * emit "write after end" errors in the next tick (Ouyang Yadong) [#24457](#24457) * deprecate \_setSimultaneousAccepts() undocumented function (James M Snell) [#23760](#23760) * net,http2: * merge setTimeout code (ZYSzys) [#25084](#25084) * os: * implement os.type() using uv\_os\_uname() (cjihrig) [#25659](#25659) * process: * global.process, global.Buffer getters (Guy Bedford) [#26882](#26882) * move DEP0062 (node --debug) to end-of-life (Joyee Cheung) [#25828](#25828) * exit on --debug and --debug-brk after option parsing (Joyee Cheung) [#25828](#25828) * improve `--redirect-warnings` handling (Ruben Bridgewater) [#24965](#24965) * readline: * support TERM=dumb (Vladislav Kaminsky) [#26261](#26261) * repl: * add welcome message (gengjiawen) [#25947](#25947) * fix terminal default setting (Ruben Bridgewater) [#26518](#26518) * check colors with .getColorDepth() (Vladislav Kaminsky) [#26261](#26261) * deprecate REPLServer.rli (Ruben Bridgewater) [#26260](#26260) * src: * remove unused INT\_MAX constant (Sam Roberts) [#27078](#27078) * update NODE\_MODULE\_VERSION to 72 (Ujjwal Sharma) [#26685](#26685) * remove `AddPromiseHook()` (Anna Henningsen) [#26574](#26574) * update NODE\_MODULE\_VERSION to 71 (Michaël Zasso) [#25852](#25852) * clean up MultiIsolatePlatform interface (Anna Henningsen) [#26384](#26384) * properly configure default heap limits (Ali Ijaz Sheikh) [#25576](#25576) * remove icuDataDir from node config (GauthamBanasandra) [#24780](#24780) * explicitly allow JS in ReadHostObject (Yang Guo) [#23423](#23423) * update postmortem constant (cjihrig) [#23423](#23423) * update NODE\_MODULE\_VERSION to 68 (Michaël Zasso) [#23423](#23423) * tls: * support TLSv1.3 (Sam Roberts) [#26209](#26209) * return correct version from getCipher() (Sam Roberts) [#26625](#26625) * check arg types of renegotiate() (Sam Roberts) [#25876](#25876) * add code for ERR\_TLS\_INVALID\_PROTOCOL\_METHOD (Sam Roberts) [#24729](#24729) * emit a warning when servername is an IP address (Rodger Combs) [#23329](#23329) * disable TLS v1.0 and v1.1 by default (Ben Noordhuis) [#23814](#23814) * remove unused arg to createSecureContext() (Sam Roberts) [#24241](#24241) * deprecate Server.prototype.setOptions() (cjihrig)[ #23820](#23820) * load NODE\_EXTRA\_CA\_CERTS at startup (Ouyang Yadong) [#23354](#23354) * util: * change inspect compact and breakLength default (Ruben Bridgewater) [#27109](#27109) * improve inspect edge cases (Ruben Bridgewater) [#27109](#27109) * only the first line of the error message (Simon Zünd) [#26685](#26685) * don't set the prototype of callbackified functions (Ruben Bridgewater) [#26893](#26893) * rename callbackified function (Ruben Bridgewater) [#26893](#26893) * increase function length when using `callbackify()` (Ruben Bridgewater) [#26893](#26893) * prevent tampering with internals in `inspect()` (Ruben Bridgewater) [#26577](#26577) * fix proxy inspection (Ruben Bridgewater) [#26241](#26241) * prevent leaking internal properties (Ruben Bridgewater) [#24971](#24971) * protect against monkeypatched Object prototype for inspect() (Rich Trott) [#25953](#25953) * treat format arguments equally (Roman Reiss) [#23162](#23162) * win, fs: * detect if symlink target is a directory (Bartosz Sosnowski) [#23724](#23724) * zlib: * throw TypeError if callback is missing (Anna Henningsen)[ #24929](#24929) * make “bare” constants un-enumerable (Anna Henningsen) [#24824](#24824) PR-URL: #26930

Notable changes: * assert: * validate required arguments (Ruben Bridgewater) [#26641](#26641) * adjust loose assertions (Ruben Bridgewater) [#25008](#25008) * async_hooks: * remove deprecated `emitBefore` and `emitAfter` (Matteo Collina) [#26530](#26530) * remove promise object from resource (Andreas Madsen) [#23443](#23443) * bootstrap: make Buffer and process non-enumerable (Ruben Bridgewater) [#24874](#24874) * buffer: * use stricter range checks (Ruben Bridgewater) [#27045](#27045) * harden `SlowBuffer` creation (ZYSzys) [#26272](#26272) * harden validation of buffer allocation size (ZYSzys) [#26162](#26162) * do proper error propagation in addon methods (Anna Henningsen) [#23939](#23939) * child_process: * remove `options.customFds` (cjihrig) [#25279](#25279) * harden fork arguments validation (ZYSzys) [#27039](#27039) * use non-infinite `maxBuffer` defaults (kohta ito) [#23027](#23027) * console: * don't use ANSI escape codes when `TERM=dumb` (Vladislav Kaminsky) [#26261](#26261) * crypto: * remove legacy native handles (Tobias Nießen) [#27011](#27011) * decode missing passphrase errors (Tobias Nießen) [#25208](#25208) * remove `Cipher.setAuthTag()` and `Decipher.getAuthTag()` (Tobias Nießen) [#26249](#26249) * remove deprecated `crypto._toBuf()` (Tobias Nießen) [#25338](#25338) * set `DEFAULT\_ENCODING` property to non-enumerable (Antoine du Hamel) [#23222](#23222) * deps: * update V8 to 7.4.288.13 (Michaël Zasso, cjihrig, Refael Ackermann) (Anna Henningsen, Ujjwal Sharma) [#26685](#26685) * bump minimum icu version to 63 (Ujjwal Sharma) [#25852](#25852) * update OpenSSL to 1.1.1b (Sam Roberts, Shigeki Ohtsu) [#26327](#26327) * errors: * update error name (Ruben Bridgewater) [#26738](#26738) * fs: * use proper .destroy() implementation for SyncWriteStream (Matteo Collina) [#26690](#26690) * improve mode validation (Ruben Bridgewater) [#26575](#26575) * harden validation of start option in `createWriteStream()` (ZYSzys) [#25579](#25579) * make writeFile consistent with readFile wrt fd (Sakthipriyan Vairamani (thefourtheye)) [#23709](#23709) * http: * validate timeout in `ClientRequest()` (cjihrig) [#26214](#26214) * return HTTP 431 on `HPE_HEADER_OVERFLOW` error (Albert Still) [#25605](#25605) * switch default parser to llhttp (Anna Henningsen) [#24870](#24870) * Runtime-deprecate `outgoingMessage._headers` and `outgoingMessage._headerNames` (Morgan Roderick) [#24167](#24167) * lib: * remove `Atomics.wake()` (Gus Caplan) [#27033](#27033) * move DTRACE\_\* probes out of global scope (James M Snell) [#26541](#26541) * deprecate `_stream_wrap` (Sam Roberts) [#26245](#26245) * use ES6 class inheritance style (Ruben Bridgewater) [#24755](#24755) * module: * remove unintended access to deps/ (Anna Henningsen) [#25138](#25138) * improve error message for MODULE\_NOT\_FOUND (Ali Ijaz Sheikh) [#25690](#25690) * requireStack property for MODULE\_NOT\_FOUND (Ali Ijaz Sheikh) [#25690](#25690) * remove dead code (Ruben Bridgewater) [#26983](#26983) * make `require('.')` never resolve outside the current directory (Ruben Bridgewater) [#26973](#26973) * throw an error for invalid package.json main entries (Ruben Bridgewater) [#26823](#26823) * don't search in `require.resolve.paths` (cjihrig) [#23683](#23683) * net: * remove `Server.listenFD()` (cjihrig) [#27127](#27127) * do not add `.host` and `.port` properties to DNS error (Ruben Bridgewater) [#26751](#26751) * emit "write after end" errors in the next tick (Ouyang Yadong) [#24457](#24457) * deprecate `_setSimultaneousAccepts()` undocumented function (James M Snell) [#23760](#23760) * os: * implement `os.type()` using `uv_os_uname()` (cjihrig) [#25659](#25659) * remove `os.getNetworkInterfaces()` (cjihrig) [#25280](#25280) * process: * make global.process, global.Buffer getters (Guy Bedford) [#26882](#26882) * move DEP0062 (node --debug) to end-of-life (Joyee Cheung) [#25828](#25828) * exit on --debug and --debug-brk after option parsing (Joyee Cheung) [#25828](#25828) * improve `--redirect-warnings` handling (Ruben Bridgewater) [#24965](#24965) * readline: * support TERM=dumb (Vladislav Kaminsky) [#26261](#26261) * repl: * add welcome message (gengjiawen) [#25947](#25947) * fix terminal default setting (Ruben Bridgewater) [#26518](#26518) * check colors with `.getColorDepth()` (Vladislav Kaminsky) [#26261](#26261) * deprecate REPLServer.rli (Ruben Bridgewater) [#26260](#26260) * src: * remove unused `INT_MAX` constant (Sam Roberts) [#27078](#27078) * update `NODE_MODULE_VERSION` to 72 (Ujjwal Sharma) [#26685](#26685) * remove `AddPromiseHook()` (Anna Henningsen) [#26574](#26574) * clean up `MultiIsolatePlatform` interface (Anna Henningsen) [#26384](#26384) * properly configure default heap limits (Ali Ijaz Sheikh) [#25576](#25576) * remove `icuDataDir` from node config (GauthamBanasandra) [#24780](#24780) * tls: * support TLSv1.3 (Sam Roberts) [#26209](#26209) * return correct version from `getCipher()` (Sam Roberts) [#26625](#26625) * check arg types of renegotiate() (Sam Roberts) [#25876](#25876) * add code for `ERR_TLS_INVALID_PROTOCOL_METHOD` (Sam Roberts) [#24729](#24729) * emit a warning when servername is an IP address (Rodger Combs) [#23329](#23329) * disable TLS v1.0 and v1.1 by default (Ben Noordhuis) [#23814](#23814) * remove unused arg to createSecureContext() (Sam Roberts) [#24241](#24241) * deprecate `Server.prototype.setOptions()` (cjihrig) [#23820](#23820) * load `NODE_EXTRA_CA_CERTS` at startup (Ouyang Yadong) [#23354](#23354) * util: * remove `util.print()`, `util.puts()`, `util.debug()` and `util.error()` (cjihrig) [#25377](#25377) * change inspect compact and breakLength default (Ruben Bridgewater) [#27109](#27109) * improve inspect edge cases (Ruben Bridgewater) [#27109](#27109) * only the first line of the error message (Simon Zünd) [#26685](#26685) * don't set the prototype of callbackified functions (Ruben Bridgewater) [#26893](#26893) * rename callbackified function (Ruben Bridgewater) [#26893](#26893) * increase function length when using `callbackify()` (Ruben Bridgewater) [#26893](#26893) * prevent tampering with internals in `inspect()` (Ruben Bridgewater) [#26577](#26577) * prevent Proxy traps being triggered by `.inspect()` (Ruben Bridgewater) [#26241](#26241) * prevent leaking internal properties (Ruben Bridgewater) [#24971](#24971) * protect against monkeypatched Object prototype for inspect() (Rich Trott) [#25953](#25953) * treat format arguments equally (Roman Reiss) [#23162](#23162) * win, fs: * detect if symlink target is a directory (Bartosz Sosnowski) [#23724](#23724) * zlib: * throw TypeError if callback is missing (Anna Henningsen) [#24929](#24929) * make “bare” constants un-enumerable (Anna Henningsen) [#24824](#24824) PR-URL: #26930

bzoz added fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform. semver-major PRs that contain breaking changes and should be released in the next major version. labels Oct 18, 2018

refack previously requested changes Oct 18, 2018

View reviewed changes

refack reviewed Oct 23, 2018

View reviewed changes

refack approved these changes Oct 24, 2018

View reviewed changes

test/parallel/test-fs-symlink-dir.js Outdated Show resolved Hide resolved

test/parallel/test-fs-symlink-dir.js Outdated Show resolved Hide resolved

test/parallel/test-fs-symlink-dir.js Show resolved Hide resolved

refack approved these changes Oct 25, 2018

View reviewed changes

refack added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Oct 25, 2018

jasnell approved these changes Nov 21, 2018

View reviewed changes

rvagg approved these changes Nov 22, 2018

View reviewed changes

mcollina approved these changes Nov 23, 2018

View reviewed changes

win, fs: detect if symlink target is a directory

7abe623

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: nodejs#23691

bzoz force-pushed the bartek-win-dir-symlink branch from 4ae122e to 7abe623 Compare November 29, 2018 11:29

Trott reviewed Nov 29, 2018

View reviewed changes

fixup

e8646bf

Trott closed this Nov 29, 2018

joaocgreis mentioned this pull request Dec 12, 2018

windows: have a machine run tests in an elevated env nodejs/build#1546

Closed

BethGriggs mentioned this pull request Mar 26, 2019

v12.0.0 proposal #26930

Merged

Hakerh400 mentioned this pull request Jan 7, 2020

rmdirsync doesn't throw error when symlinked path is passed as argument in windows node 12 #31231

Open

LiviaMedeiros mentioned this pull request Apr 28, 2022

fs: add directory autodetection to fsPromises.symlink() #42894

Merged

LiviaMedeiros mentioned this pull request Sep 20, 2023

fs: adjust typecheck for type in fs.symlink() #49741

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

bzoz commented Oct 18, 2018

nodejs-github-bot commented Oct 18, 2018

bzoz commented Oct 18, 2018

refack left a comment

refack commented Oct 18, 2018

bzoz commented Oct 23, 2018

refack left a comment

refack Oct 23, 2018

bzoz commented Oct 24, 2018

refack commented Oct 24, 2018

refack left a comment

refack commented Oct 24, 2018

bzoz commented Oct 25, 2018

refack left a comment

refack commented Oct 25, 2018

bzoz commented Nov 1, 2018

bzoz commented Nov 8, 2018

refack commented Nov 8, 2018

bzoz commented Nov 20, 2018

bzoz commented Nov 22, 2018

rvagg commented Nov 22, 2018

bzoz commented Nov 22, 2018

rvagg commented Nov 22, 2018

bzoz commented Nov 22, 2018

rvagg Nov 22, 2018

rvagg Nov 22, 2018

mcollina left a comment

mcollina Nov 23, 2018

bzoz commented Nov 29, 2018

bzoz commented Nov 29, 2018

Trott Nov 29, 2018 •

edited

Trott Nov 29, 2018

Trott commented Nov 29, 2018

	fs.symlink(target, path, (err) => {
	fs.symlink(target, path, common.mustCall((err) => {

	} catch (err) { } // eslint-disable-line no-unused-vars
	} catch { }

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

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

Conversation

bzoz commented Oct 18, 2018

Checklist

nodejs-github-bot commented Oct 18, 2018

bzoz commented Oct 18, 2018

refack left a comment

Choose a reason for hiding this comment

refack commented Oct 18, 2018

bzoz commented Oct 23, 2018

refack left a comment

Choose a reason for hiding this comment

refack Oct 23, 2018

Choose a reason for hiding this comment

bzoz commented Oct 24, 2018

refack commented Oct 24, 2018

refack left a comment

Choose a reason for hiding this comment

refack commented Oct 24, 2018

bzoz commented Oct 25, 2018

refack left a comment

Choose a reason for hiding this comment

refack commented Oct 25, 2018

bzoz commented Nov 1, 2018

bzoz commented Nov 8, 2018

refack commented Nov 8, 2018

bzoz commented Nov 20, 2018

bzoz commented Nov 22, 2018

rvagg commented Nov 22, 2018

bzoz commented Nov 22, 2018

rvagg commented Nov 22, 2018

bzoz commented Nov 22, 2018

rvagg Nov 22, 2018

Choose a reason for hiding this comment

rvagg Nov 22, 2018

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

mcollina Nov 23, 2018

Choose a reason for hiding this comment

bzoz commented Nov 29, 2018

bzoz commented Nov 29, 2018

Trott Nov 29, 2018 • edited

Choose a reason for hiding this comment

Trott Nov 29, 2018

Choose a reason for hiding this comment

Trott commented Nov 29, 2018

Trott Nov 29, 2018 •

edited