Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 2e07d45

Browse filesBrowse files
cjihrigtargos
cjihrig
authored and
targos
committed
fs: undeprecate lchown()
uv_fs_lchown() exists, as of libuv 1.21.0. fs.lchown() can now be undeprecated. This commit also adds tests, as there were none. PR-URL: #21498 Fixes: #19868 Reviewed-By: Wyatt Preul <wpreul@gmail.com>
1 parent a7505c0 commit 2e07d45
Copy full SHA for 2e07d45

File tree

Expand file treeCollapse file tree

9 files changed

+140
-55
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

9 files changed

+140
-55
lines changed
Open diff view settings
Collapse file

‎doc/api/deprecations.md‎

Copy file name to clipboardExpand all lines: doc/api/deprecations.md
-16Lines changed: 0 additions & 16 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -351,20 +351,6 @@ Type: Documentation-only
351351

352352
The [`fs.lchmodSync(path, mode)`][] API is deprecated.
353353

354-
<a id="DEP0037"></a>
355-
### DEP0037: fs.lchown(path, uid, gid, callback)
356-
357-
Type: Documentation-only
358-
359-
The [`fs.lchown(path, uid, gid, callback)`][] API is deprecated.
360-
361-
<a id="DEP0038"></a>
362-
### DEP0038: fs.lchownSync(path, uid, gid)
363-
364-
Type: Documentation-only
365-
366-
The [`fs.lchownSync(path, uid, gid)`][] API is deprecated.
367-
368354
<a id="DEP0039"></a>
369355
### DEP0039: require.extensions
370356

@@ -1037,8 +1023,6 @@ The option `produceCachedData` has been deprecated. Use
10371023
[`fs.exists(path, callback)`]: fs.html#fs_fs_exists_path_callback
10381024
[`fs.lchmod(path, mode, callback)`]: fs.html#fs_fs_lchmod_path_mode_callback
10391025
[`fs.lchmodSync(path, mode)`]: fs.html#fs_fs_lchmodsync_path_mode
1040-
[`fs.lchown(path, uid, gid, callback)`]: fs.html#fs_fs_lchown_path_uid_gid_callback
1041-
[`fs.lchownSync(path, uid, gid)`]: fs.html#fs_fs_lchownsync_path_uid_gid
10421026
[`fs.read()`]: fs.html#fs_fs_read_fd_buffer_offset_length_position_callback
10431027
[`fs.readSync()`]: fs.html#fs_fs_readsync_fd_buffer_offset_length_position
10441028
[`fs.stat()`]: fs.html#fs_fs_stat_path_callback
Collapse file

‎doc/api/documentation.md‎

Copy file name to clipboardExpand all lines: doc/api/documentation.md
-5Lines changed: 0 additions & 5 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,6 @@ which simply wrap a syscall,
8383
like [`fs.open()`][], will document that. The docs link to the corresponding man
8484
pages (short for manual pages) which describe how the syscalls work.
8585

86-
Some syscalls, like lchown(2), are BSD-specific. That means, for
87-
example, that [`fs.lchown()`][] only works on macOS and other BSD-derived
88-
systems, and is not available on Linux.
89-
9086
Most Unix syscalls have Windows equivalents, but behavior may differ on Windows
9187
relative to Linux and macOS. For an example of the subtle ways in which it's
9288
sometimes impossible to replace Unix syscall semantics on Windows, see [Node
@@ -95,6 +91,5 @@ issue 4760](https://github.com/nodejs/node/issues/4760).
9591
[`'warning'`]: process.html#process_event_warning
9692
[`stderr`]: process.html#process_process_stderr
9793
[`fs.open()`]: fs.html#fs_fs_open_path_flags_mode_callback
98-
[`fs.lchown()`]: fs.html#fs_fs_lchown_path_uid_gid_callback
9994
[submit an issue]: https://github.com/nodejs/node/issues/new
10095
[the contributing guide]: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md
Collapse file

‎doc/api/fs.md‎

Copy file name to clipboardExpand all lines: doc/api/fs.md
+13-4Lines changed: 13 additions & 4 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -1903,8 +1903,10 @@ Synchronous lchmod(2). Returns `undefined`.
19031903

19041904
## fs.lchown(path, uid, gid, callback)
19051905
<!-- YAML
1906-
deprecated: v0.4.7
19071906
changes:
1907+
- version: REPLACEME
1908+
pr-url: https://github.com/nodejs/node/pull/21498
1909+
description: This API is no longer deprecated.
19081910
- version: v10.0.0
19091911
pr-url: https://github.com/nodejs/node/pull/12562
19101912
description: The `callback` parameter is no longer optional. Not passing
@@ -1926,7 +1928,10 @@ to the completion callback.
19261928

19271929
## fs.lchownSync(path, uid, gid)
19281930
<!-- YAML
1929-
deprecated: v0.4.7
1931+
changes:
1932+
- version: REPLACEME
1933+
pr-url: https://github.com/nodejs/node/pull/21498
1934+
description: This API is no longer deprecated.
19301935
-->
19311936

19321937
* `path` {string|Buffer|URL}
@@ -3899,7 +3904,11 @@ no arguments upon success. This method is only implemented on macOS.
38993904

39003905
### fsPromises.lchown(path, uid, gid)
39013906
<!-- YAML
3902-
deprecated: v10.0.0
3907+
added: v10.0.0
3908+
changes:
3909+
- version: REPLACEME
3910+
pr-url: https://github.com/nodejs/node/pull/21498
3911+
description: This API is no longer deprecated.
39033912
-->
39043913

39053914
* `path` {string|Buffer|URL}
@@ -3908,7 +3917,7 @@ deprecated: v10.0.0
39083917
* Returns: {Promise}
39093918

39103919
Changes the ownership on a symbolic link then resolves the `Promise` with
3911-
no arguments upon success. This method is only implemented on macOS.
3920+
no arguments upon success.
39123921

39133922
### fsPromises.link(existingPath, newPath)
39143923
<!-- YAML
Collapse file

‎lib/fs.js‎

Copy file name to clipboardExpand all lines: lib/fs.js
+17-24Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -997,31 +997,24 @@ function chmodSync(path, mode) {
997997
}
998998

999999
function lchown(path, uid, gid, callback) {
1000-
callback = maybeCallback(callback);
1001-
fs.open(path, O_WRONLY | O_SYMLINK, function(err, fd) {
1002-
if (err) {
1003-
callback(err);
1004-
return;
1005-
}
1006-
// Prefer to return the chown error, if one occurs,
1007-
// but still try to close, and report closing errors if they occur.
1008-
fs.fchown(fd, uid, gid, function(err) {
1009-
fs.close(fd, function(err2) {
1010-
callback(err || err2);
1011-
});
1012-
});
1013-
});
1000+
callback = makeCallback(callback);
1001+
path = getPathFromURL(path);
1002+
validatePath(path);
1003+
validateUint32(uid, 'uid');
1004+
validateUint32(gid, 'gid');
1005+
const req = new FSReqWrap();
1006+
req.oncomplete = callback;
1007+
binding.lchown(pathModule.toNamespacedPath(path), uid, gid, req);
10141008
}
10151009

10161010
function lchownSync(path, uid, gid) {
1017-
const fd = fs.openSync(path, O_WRONLY | O_SYMLINK);
1018-
let ret;
1019-
try {
1020-
ret = fs.fchownSync(fd, uid, gid);
1021-
} finally {
1022-
fs.closeSync(fd);
1023-
}
1024-
return ret;
1011+
path = getPathFromURL(path);
1012+
validatePath(path);
1013+
validateUint32(uid, 'uid');
1014+
validateUint32(gid, 'gid');
1015+
const ctx = { path };
1016+
binding.lchown(pathModule.toNamespacedPath(path), uid, gid, undefined, ctx);
1017+
handleErrorFromBinding(ctx);
10251018
}
10261019

10271020
function fchown(fd, uid, gid, callback) {
@@ -1753,8 +1746,8 @@ module.exports = fs = {
17531746
ftruncateSync,
17541747
futimes,
17551748
futimesSync,
1756-
lchown: constants.O_SYMLINK !== undefined ? lchown : undefined,
1757-
lchownSync: constants.O_SYMLINK !== undefined ? lchownSync : undefined,
1749+
lchown,
1750+
lchownSync,
17581751
lchmod: constants.O_SYMLINK !== undefined ? lchmod : undefined,
17591752
lchmodSync: constants.O_SYMLINK !== undefined ? lchmodSync : undefined,
17601753
link,
Collapse file

‎lib/internal/fs/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/promises.js
+6-5Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -377,11 +377,12 @@ async function lchmod(path, mode) {
377377
}
378378

379379
async function lchown(path, uid, gid) {
380-
if (O_SYMLINK === undefined)
381-
throw new ERR_METHOD_NOT_IMPLEMENTED();
382-
383-
const fd = await open(path, O_WRONLY | O_SYMLINK);
384-
return fchown(fd, uid, gid).finally(fd.close.bind(fd));
380+
path = getPathFromURL(path);
381+
validatePath(path);
382+
validateUint32(uid, 'uid');
383+
validateUint32(gid, 'gid');
384+
return binding.lchown(pathModule.toNamespacedPath(path),
385+
uid, gid, kUsePromises);
385386
}
386387

387388
async function fchown(handle, uid, gid) {
Collapse file

‎src/node_file.cc‎

Copy file name to clipboardExpand all lines: src/node_file.cc
+31-1Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1771,6 +1771,36 @@ static void FChown(const FunctionCallbackInfo<Value>& args) {
17711771
}
17721772

17731773

1774+
static void LChown(const FunctionCallbackInfo<Value>& args) {
1775+
Environment* env = Environment::GetCurrent(args);
1776+
1777+
const int argc = args.Length();
1778+
CHECK_GE(argc, 3);
1779+
1780+
BufferValue path(env->isolate(), args[0]);
1781+
CHECK_NOT_NULL(*path);
1782+
1783+
CHECK(args[1]->IsUint32());
1784+
const uv_uid_t uid = static_cast<uv_uid_t>(args[1].As<Uint32>()->Value());
1785+
1786+
CHECK(args[2]->IsUint32());
1787+
const uv_gid_t gid = static_cast<uv_gid_t>(args[2].As<Uint32>()->Value());
1788+
1789+
FSReqBase* req_wrap_async = GetReqWrap(env, args[3]);
1790+
if (req_wrap_async != nullptr) { // lchown(path, uid, gid, req)
1791+
AsyncCall(env, req_wrap_async, args, "lchown", UTF8, AfterNoArgs,
1792+
uv_fs_lchown, *path, uid, gid);
1793+
} else { // lchown(path, uid, gid, undefined, ctx)
1794+
CHECK_EQ(argc, 5);
1795+
FSReqWrapSync req_wrap_sync;
1796+
FS_SYNC_TRACE_BEGIN(lchown);
1797+
SyncCall(env, args[4], &req_wrap_sync, "lchown",
1798+
uv_fs_lchown, *path, uid, gid);
1799+
FS_SYNC_TRACE_END(lchown);
1800+
}
1801+
}
1802+
1803+
17741804
static void UTimes(const FunctionCallbackInfo<Value>& args) {
17751805
Environment* env = Environment::GetCurrent(args);
17761806

@@ -1904,7 +1934,7 @@ void Initialize(Local<Object> target,
19041934

19051935
env->SetMethod(target, "chown", Chown);
19061936
env->SetMethod(target, "fchown", FChown);
1907-
// env->SetMethod(target, "lchown", LChown);
1937+
env->SetMethod(target, "lchown", LChown);
19081938

19091939
env->SetMethod(target, "utimes", UTimes);
19101940
env->SetMethod(target, "futimes", FUTimes);
Collapse file

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

Copy file name to clipboard
+67Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const tmpdir = require('../common/tmpdir');
5+
const assert = require('assert');
6+
const fs = require('fs');
7+
const path = require('path');
8+
const { promises } = fs;
9+
10+
common.crashOnUnhandledRejection();
11+
12+
// Validate the path argument.
13+
[false, 1, {}, [], null, undefined].forEach((i) => {
14+
const err = { type: TypeError, code: 'ERR_INVALID_ARG_TYPE' };
15+
16+
common.expectsError(() => fs.lchown(i, 1, 1, common.mustNotCall()), err);
17+
common.expectsError(() => fs.lchownSync(i, 1, 1), err);
18+
promises.lchown(false, 1, 1)
19+
.then(common.mustNotCall())
20+
.catch(common.expectsError(err));
21+
});
22+
23+
// Validate the uid and gid arguments.
24+
[false, 'test', {}, [], null, undefined].forEach((i) => {
25+
const err = { type: TypeError, code: 'ERR_INVALID_ARG_TYPE' };
26+
27+
common.expectsError(
28+
() => fs.lchown('not_a_file_that_exists', i, 1, common.mustNotCall()),
29+
err
30+
);
31+
common.expectsError(
32+
() => fs.lchown('not_a_file_that_exists', 1, i, common.mustNotCall()),
33+
err
34+
);
35+
common.expectsError(() => fs.lchownSync('not_a_file_that_exists', i, 1), err);
36+
common.expectsError(() => fs.lchownSync('not_a_file_that_exists', 1, i), err);
37+
38+
promises.lchown('not_a_file_that_exists', i, 1)
39+
.then(common.mustNotCall())
40+
.catch(common.expectsError(err));
41+
42+
promises.lchown('not_a_file_that_exists', 1, i)
43+
.then(common.mustNotCall())
44+
.catch(common.expectsError(err));
45+
});
46+
47+
// Validate the callback argument.
48+
[false, 1, 'test', {}, [], null, undefined].forEach((i) => {
49+
common.expectsError(() => fs.lchown('not_a_file_that_exists', 1, 1, i), {
50+
type: TypeError,
51+
code: 'ERR_INVALID_CALLBACK'
52+
});
53+
});
54+
55+
if (!common.isWindows) {
56+
const testFile = path.join(tmpdir.path, path.basename(__filename));
57+
const uid = process.geteuid();
58+
const gid = process.getegid();
59+
60+
tmpdir.refresh();
61+
fs.copyFileSync(__filename, testFile);
62+
fs.lchownSync(testFile, uid, gid);
63+
fs.lchown(testFile, uid, gid, common.mustCall(async (err) => {
64+
assert.ifError(err);
65+
await promises.lchown(testFile, uid, gid);
66+
}));
67+
}
Collapse file

‎test/parallel/test-fs-null-bytes.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-fs-null-bytes.js
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ check(fs.chmod, fs.chmodSync, 'foo\u0000bar', '0644');
5959
check(fs.chown, fs.chownSync, 'foo\u0000bar', 12, 34);
6060
check(fs.copyFile, fs.copyFileSync, 'foo\u0000bar', 'abc');
6161
check(fs.copyFile, fs.copyFileSync, 'abc', 'foo\u0000bar');
62+
check(fs.lchown, fs.lchownSync, 'foo\u0000bar', 12, 34);
6263
check(fs.link, fs.linkSync, 'foo\u0000bar', 'foobar');
6364
check(fs.link, fs.linkSync, 'foobar', 'foo\u0000bar');
6465
check(fs.lstat, fs.lstatSync, 'foo\u0000bar');
@@ -92,6 +93,7 @@ check(fs.chmod, fs.chmodSync, fileUrl, '0644');
9293
check(fs.chown, fs.chownSync, fileUrl, 12, 34);
9394
check(fs.copyFile, fs.copyFileSync, fileUrl, 'abc');
9495
check(fs.copyFile, fs.copyFileSync, 'abc', fileUrl);
96+
check(fs.lchown, fs.lchownSync, fileUrl, 12, 34);
9597
check(fs.link, fs.linkSync, fileUrl, 'foobar');
9698
check(fs.link, fs.linkSync, 'foobar', fileUrl);
9799
check(fs.lstat, fs.lstatSync, fileUrl);
@@ -122,6 +124,7 @@ check(fs.chmod, fs.chmodSync, fileUrl2, '0644');
122124
check(fs.chown, fs.chownSync, fileUrl2, 12, 34);
123125
check(fs.copyFile, fs.copyFileSync, fileUrl2, 'abc');
124126
check(fs.copyFile, fs.copyFileSync, 'abc', fileUrl2);
127+
check(fs.lchown, fs.lchownSync, fileUrl2, 12, 34);
125128
check(fs.link, fs.linkSync, fileUrl2, 'foobar');
126129
check(fs.link, fs.linkSync, 'foobar', fileUrl2);
127130
check(fs.lstat, fs.lstatSync, fileUrl2);
Collapse file

‎test/parallel/test-trace-events-fs-sync.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-trace-events-fs-sync.js
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ tests['fs.sync.futimes'] = 'fs.writeFileSync("fs.txt", "123", "utf8");' +
6060
'const fd = fs.openSync("fs.txt", "r+");' +
6161
'fs.futimesSync(fd,1,1);' +
6262
'fs.unlinkSync("fs.txt")';
63+
tests['fs.sync.lchown'] = 'fs.writeFileSync("fs.txt", "123", "utf8");' +
64+
'fs.lchownSync("fs.txt",' + uid + ',' + gid + ');' +
65+
'fs.unlinkSync("fs.txt")';
6366
tests['fs.sync.link'] = 'fs.writeFileSync("fs.txt", "123", "utf8");' +
6467
'fs.linkSync("fs.txt", "linkx");' +
6568
'fs.unlinkSync("linkx");' +

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.