GeezerVison is showing github.com — fs: adjust typecheck for `type` in `fs.symlink()` · nodejs/node@f202322
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: doc/api/errors.md +11-7Lines changed: 11 additions & 7 deletions - Display the source diff
- Display the rich diff
| Original file line number | Diff line number | Diff line change |
|---|
@@ -1377,13 +1377,6 @@ Path is a directory. | 1377 | 1377 | An attempt has been made to read a file whose size is larger than the maximum | 1378 | 1378 | allowed size for a `Buffer`. | 1379 | 1379 |
| 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 | | - | 1387 | 1380 | <a id="ERR_HTTP_HEADERS_SENT"></a> | 1388 | 1381 |
| 1389 | 1382 | ### `ERR_HTTP_HEADERS_SENT` | @@ -3276,6 +3269,17 @@ The UTF-16 encoding was used with [`hash.digest()`][]. While the | 3276 | 3269 | causing the method to return a string rather than a `Buffer`, the UTF-16 | 3277 | 3270 | encoding (e.g. `ucs` or `utf16le`) is not supported. | 3278 | 3271 |
| | 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 | + | 3279 | 3283 | <a id="ERR_HTTP2_FRAME_ERROR"></a> | 3280 | 3284 |
| 3281 | 3285 | ### `ERR_HTTP2_FRAME_ERROR` | |
Collapse file Copy file name to clipboardExpand all lines: doc/api/fs.md +2-2Lines changed: 2 additions & 2 deletions - Display the source diff
- Display the rich diff
| Original file line number | Diff line number | Diff line change |
|---|
@@ -1693,7 +1693,7 @@ changes: | 1693 | 1693 | Creates a symbolic link. | 1694 | 1694 |
| 1695 | 1695 | 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 | 1697 | 1697 | autodetect `target` type and use `'file'` or `'dir'`. If the `target` does not | 1698 | 1698 | exist, `'file'` will be used. Windows junction points require the destination | 1699 | 1699 | path to be absolute. When using `'junction'`, the `target` argument will | @@ -4444,7 +4444,7 @@ See the POSIX symlink(2) documentation for more details. | 4444 | 4444 |
| 4445 | 4445 | The `type` argument is only available on Windows and ignored on other platforms. | 4446 | 4446 | 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'`. | 4448 | 4448 | If the `target` does not exist, `'file'` will be used. Windows junction points | 4449 | 4449 | require the destination path to be absolute. When using `'junction'`, the | 4450 | 4450 | `target` argument will automatically be normalized to absolute path. Junction | |
Collapse file Copy file name to clipboardExpand all lines: lib/fs.js +13-8Lines changed: 13 additions & 8 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -142,6 +142,7 @@ const { | 142 | 142 | validateFunction, | 143 | 143 | validateInteger, | 144 | 144 | validateObject, | | 145 | + validateOneOf, | 145 | 146 | validateString, | 146 | 147 | kValidateObjectAllowNullable, | 147 | 148 | } = require('internal/validators'); | @@ -1715,13 +1716,17 @@ function readlinkSync(path, options) { | 1715 | 1716 | * Creates the link called `path` pointing to `target`. | 1716 | 1717 | * @param {string | Buffer | URL} target | 1717 | 1718 | * @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 | 1720 | 1721 | * @returns {void} | 1721 | 1722 | */ | 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 | + } | 1725 | 1730 |
| 1726 | 1731 | if (permission.isEnabled()) { | 1727 | 1732 | // The permission model's security guarantees fall apart in the presence of | @@ -1740,7 +1745,7 @@ function symlink(target, path, type_, callback_) { | 1740 | 1745 | target = getValidatedPath(target, 'target'); | 1741 | 1746 | path = getValidatedPath(path); | 1742 | 1747 |
| 1743 | | - if (isWindows && type === null) { | | 1748 | + if (isWindows && type == null) { | 1744 | 1749 | let absoluteTarget; | 1745 | 1750 | try { | 1746 | 1751 | // Symlinks targets can be relative to the newly created path. | @@ -1786,8 +1791,8 @@ function symlink(target, path, type_, callback_) { | 1786 | 1791 | * @returns {void} | 1787 | 1792 | */ | 1788 | 1793 | 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) { | 1791 | 1796 | const absoluteTarget = pathModule.resolve(`${path}`, '..', `${target}`); | 1792 | 1797 | if (statSync(absoluteTarget, { throwIfNoEntry: false })?.isDirectory()) { | 1793 | 1798 | type = 'dir'; | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/errors.js -3Lines changed: 0 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1217,9 +1217,6 @@ E('ERR_FS_CP_SYMLINK_TO_SUBDIRECTORY', | 1217 | 1217 | E('ERR_FS_CP_UNKNOWN', 'Cannot copy an unknown file type', SystemError); | 1218 | 1218 | E('ERR_FS_EISDIR', 'Path is a directory', SystemError, HideStackFramesError); | 1219 | 1219 | 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 | 1223 | 1220 | E('ERR_HTTP2_ALTSVC_INVALID_ORIGIN', | 1224 | 1221 | 'HTTP/2 ALTSVC frames require a valid origin', TypeError); | 1225 | 1222 | E('ERR_HTTP2_ALTSVC_LENGTH', | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/fs/promises.js +4-3Lines changed: 4 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -85,6 +85,7 @@ const { | 85 | 85 | validateEncoding, | 86 | 86 | validateInteger, | 87 | 87 | validateObject, | | 88 | + validateOneOf, | 88 | 89 | validateString, | 89 | 90 | kValidateObjectAllowNullable, | 90 | 91 | } = require('internal/validators'); | @@ -973,9 +974,9 @@ async function readlink(path, options) { | 973 | 974 | ); | 974 | 975 | } | 975 | 976 |
| 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) { | 979 | 980 | try { | 980 | 981 | const absoluteTarget = pathModule.resolve(`${path}`, '..', `${target}`); | 981 | 982 | type = (await stat(absoluteTarget)).isDirectory() ? 'dir' : 'file'; | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/fs/utils.js +9-16Lines changed: 9 additions & 16 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -31,7 +31,6 @@ const { | 31 | 31 | UVException, | 32 | 32 | codes: { | 33 | 33 | ERR_FS_EISDIR, | 34 | | - ERR_FS_INVALID_SYMLINK_TYPE, | 35 | 34 | ERR_INCOMPATIBLE_OPTION_PAIR, | 36 | 35 | ERR_INVALID_ARG_TYPE, | 37 | 36 | ERR_INVALID_ARG_VALUE, | @@ -647,22 +646,16 @@ function stringToFlags(flags, name = 'flags') { | 647 | 646 | } | 648 | 647 |
| 649 | 648 | 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; | 664 | 658 | } | 665 | | - return flags; | 666 | 659 | }); | 667 | 660 |
| 668 | 661 | // converts Date or number to a fractional UNIX timestamp | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-fs-symlink.js +17-4Lines changed: 17 additions & 4 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -76,14 +76,27 @@ fs.symlink(linkData, linkPath, common.mustSucceed(() => { | 76 | 76 | }); | 77 | 77 |
| 78 | 78 | 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', | 83 | 81 | }; | 84 | 82 | assert.throws(() => fs.symlink('', '', '🍏', common.mustNotCall()), errObj); | 85 | 83 | assert.throws(() => fs.symlinkSync('', '', '🍏'), errObj); | 86 | 84 |
| | 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 | + | 87 | 100 | process.on('exit', () => { | 88 | 101 | assert.notStrictEqual(linkTime, fileTime); | 89 | 102 | }); |
|