FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: adjust typecheck for `type` in `fs.symlink()` · nodejs/node@f202322 · GitHub

/ node Public

Commit f202322

Browse files
LiviaMedeiros
authored
fs: adjust typecheck for type in fs.symlink()
Throws `TypeError` instead of `Error` Enables autodetection on Windows if `type === undefined` Explicitly disallows unknown strings and non-string values PR-URL: #49741 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
1 parent 7588467 commit f202322

7 files changed

Lines changed: 56 additions & 43 deletions

File tree

‎doc/api/errors.md‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,13 +1377,6 @@ Path is a directory.
13771377
An attempt has been made to read a file whose size is larger than the maximum
13781378
allowed size for a `Buffer`.
13791379

1380-
<a id="ERR_FS_INVALID_SYMLINK_TYPE"></a>
1381-
1382-
### `ERR_FS_INVALID_SYMLINK_TYPE`
1383-
1384-
An invalid symlink type was passed to the [`fs.symlink()`][] or
1385-
[`fs.symlinkSync()`][] methods.
1386-
13871380
<a id="ERR_HTTP_HEADERS_SENT"></a>
13881381

13891382
### `ERR_HTTP_HEADERS_SENT`
@@ -3276,6 +3269,17 @@ The UTF-16 encoding was used with [`hash.digest()`][]. While the
32763269
causing the method to return a string rather than a `Buffer`, the UTF-16
32773270
encoding (e.g. `ucs` or `utf16le`) is not supported.
32783271

3272+
<a id="ERR_FS_INVALID_SYMLINK_TYPE"></a>
3273+
3274+
### `ERR_FS_INVALID_SYMLINK_TYPE`
3275+
3276+
<!-- YAML
3277+
removed: REPLACEME
3278+
-->
3279+
3280+
An invalid symlink type was passed to the [`fs.symlink()`][] or
3281+
[`fs.symlinkSync()`][] methods.
3282+
32793283
<a id="ERR_HTTP2_FRAME_ERROR"></a>
32803284

32813285
### `ERR_HTTP2_FRAME_ERROR`

‎doc/api/fs.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,7 +1693,7 @@ changes:
16931693
Creates a symbolic link.
16941694
16951695
The `type` argument is only used on Windows platforms and can be one of `'dir'`,
1696-
`'file'`, or `'junction'`. If the `type` argument is not a string, Node.js will
1696+
`'file'`, or `'junction'`. If the `type` argument is `null`, Node.js will
16971697
autodetect `target` type and use `'file'` or `'dir'`. If the `target` does not
16981698
exist, `'file'` will be used. Windows junction points require the destination
16991699
path to be absolute. When using `'junction'`, the `target` argument will
@@ -4444,7 +4444,7 @@ See the POSIX symlink(2) documentation for more details.
44444444
44454445
The `type` argument is only available on Windows and ignored on other platforms.
44464446
It can be set to `'dir'`, `'file'`, or `'junction'`. If the `type` argument is
4447-
not a string, Node.js will autodetect `target` type and use `'file'` or `'dir'`.
4447+
`null`, Node.js will autodetect `target` type and use `'file'` or `'dir'`.
44484448
If the `target` does not exist, `'file'` will be used. Windows junction points
44494449
require the destination path to be absolute. When using `'junction'`, the
44504450
`target` argument will automatically be normalized to absolute path. Junction

‎lib/fs.js‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ const {
142142
validateFunction,
143143
validateInteger,
144144
validateObject,
145+
validateOneOf,
145146
validateString,
146147
kValidateObjectAllowNullable,
147148
} = require('internal/validators');
@@ -1715,13 +1716,17 @@ function readlinkSync(path, options) {
17151716
* Creates the link called `path` pointing to `target`.
17161717
* @param {string | Buffer | URL} target
17171718
* @param {string | Buffer | URL} path
1718-
* @param {string | null} [type_]
1719-
* @param {(err?: Error) => any} callback_
1719+
* @param {string | null} [type]
1720+
* @param {(err?: Error) => any} callback
17201721
* @returns {void}
17211722
*/
1722-
function symlink(target, path, type_, callback_) {
1723-
const type = (typeof type_ === 'string' ? type_ : null);
1724-
const callback = makeCallback(arguments[arguments.length - 1]);
1723+
function symlink(target, path, type, callback) {
1724+
if (callback === undefined) {
1725+
callback = makeCallback(type);
1726+
type = undefined;
1727+
} else {
1728+
validateOneOf(type, 'type', ['dir', 'file', 'junction', null, undefined]);
1729+
}
17251730

17261731
if (permission.isEnabled()) {
17271732
// The permission model's security guarantees fall apart in the presence of
@@ -1740,7 +1745,7 @@ function symlink(target, path, type_, callback_) {
17401745
target = getValidatedPath(target, 'target');
17411746
path = getValidatedPath(path);
17421747

1743-
if (isWindows && type === null) {
1748+
if (isWindows && type == null) {
17441749
let absoluteTarget;
17451750
try {
17461751
// Symlinks targets can be relative to the newly created path.
@@ -1786,8 +1791,8 @@ function symlink(target, path, type_, callback_) {
17861791
* @returns {void}
17871792
*/
17881793
function symlinkSync(target, path, type) {
1789-
type = (typeof type === 'string' ? type : null);
1790-
if (isWindows && type === null) {
1794+
validateOneOf(type, 'type', ['dir', 'file', 'junction', null, undefined]);
1795+
if (isWindows && type == null) {
17911796
const absoluteTarget = pathModule.resolve(`${path}`, '..', `${target}`);
17921797
if (statSync(absoluteTarget, { throwIfNoEntry: false })?.isDirectory()) {
17931798
type = 'dir';

‎lib/internal/errors.js‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,9 +1217,6 @@ E('ERR_FS_CP_SYMLINK_TO_SUBDIRECTORY',
12171217
E('ERR_FS_CP_UNKNOWN', 'Cannot copy an unknown file type', SystemError);
12181218
E('ERR_FS_EISDIR', 'Path is a directory', SystemError, HideStackFramesError);
12191219
E('ERR_FS_FILE_TOO_LARGE', 'File size (%s) is greater than 2 GiB', RangeError);
1220-
E('ERR_FS_INVALID_SYMLINK_TYPE',
1221-
'Symlink type must be one of "dir", "file", or "junction". Received "%s"',
1222-
Error); // Switch to TypeError. The current implementation does not seem right
12231220
E('ERR_HTTP2_ALTSVC_INVALID_ORIGIN',
12241221
'HTTP/2 ALTSVC frames require a valid origin', TypeError);
12251222
E('ERR_HTTP2_ALTSVC_LENGTH',

‎lib/internal/fs/promises.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ const {
8585
validateEncoding,
8686
validateInteger,
8787
validateObject,
88+
validateOneOf,
8889
validateString,
8990
kValidateObjectAllowNullable,
9091
} = require('internal/validators');
@@ -973,9 +974,9 @@ async function readlink(path, options) {
973974
);
974975
}
975976

976-
async function symlink(target, path, type_) {
977-
let type = (typeof type_ === 'string' ? type_ : null);
978-
if (isWindows && type === null) {
977+
async function symlink(target, path, type) {
978+
validateOneOf(type, 'type', ['dir', 'file', 'junction', null, undefined]);
979+
if (isWindows && type == null) {
979980
try {
980981
const absoluteTarget = pathModule.resolve(`${path}`, '..', `${target}`);
981982
type = (await stat(absoluteTarget)).isDirectory() ? 'dir' : 'file';

‎lib/internal/fs/utils.js‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ const {
3131
UVException,
3232
codes: {
3333
ERR_FS_EISDIR,
34-
ERR_FS_INVALID_SYMLINK_TYPE,
3534
ERR_INCOMPATIBLE_OPTION_PAIR,
3635
ERR_INVALID_ARG_TYPE,
3736
ERR_INVALID_ARG_VALUE,
@@ -647,22 +646,16 @@ function stringToFlags(flags, name = 'flags') {
647646
}
648647

649648
const stringToSymlinkType = hideStackFrames((type) => {
650-
let flags = 0;
651-
if (typeof type === 'string') {
652-
switch (type) {
653-
case 'dir':
654-
flags |= UV_FS_SYMLINK_DIR;
655-
break;
656-
case 'junction':
657-
flags |= UV_FS_SYMLINK_JUNCTION;
658-
break;
659-
case 'file':
660-
break;
661-
default:
662-
throw new ERR_FS_INVALID_SYMLINK_TYPE(type);
663-
}
649+
switch (type) {
650+
case undefined:
651+
case null:
652+
case 'file':
653+
return 0;
654+
case 'dir':
655+
return UV_FS_SYMLINK_DIR;
656+
case 'junction':
657+
return UV_FS_SYMLINK_JUNCTION;
664658
}
665-
return flags;
666659
});
667660

668661
// converts Date or number to a fractional UNIX timestamp

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,27 @@ fs.symlink(linkData, linkPath, common.mustSucceed(() => {
7676
});
7777

7878
const errObj = {
79-
code: 'ERR_FS_INVALID_SYMLINK_TYPE',
80-
name: 'Error',
81-
message:
82-
'Symlink type must be one of "dir", "file", or "junction". Received "🍏"'
79+
code: 'ERR_INVALID_ARG_VALUE',
80+
name: 'TypeError',
8381
};
8482
assert.throws(() => fs.symlink('', '', '🍏', common.mustNotCall()), errObj);
8583
assert.throws(() => fs.symlinkSync('', '', '🍏'), errObj);
8684

85+
assert.throws(() => fs.symlink('', '', 'nonExistentType', common.mustNotCall()), errObj);
86+
assert.throws(() => fs.symlinkSync('', '', 'nonExistentType'), errObj);
87+
assert.rejects(() => fs.promises.symlink('', '', 'nonExistentType'), errObj)
88+
.then(common.mustCall());
89+
90+
assert.throws(() => fs.symlink('', '', false, common.mustNotCall()), errObj);
91+
assert.throws(() => fs.symlinkSync('', '', false), errObj);
92+
assert.rejects(() => fs.promises.symlink('', '', false), errObj)
93+
.then(common.mustCall());
94+
95+
assert.throws(() => fs.symlink('', '', {}, common.mustNotCall()), errObj);
96+
assert.throws(() => fs.symlinkSync('', '', {}), errObj);
97+
assert.rejects(() => fs.promises.symlink('', '', {}), errObj)
98+
.then(common.mustCall());
99+
87100
process.on('exit', () => {
88101
assert.notStrictEqual(linkTime, fileTime);
89102
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL