Skip to content

Commit 23bd4d1

Browse files
himself65targos
himself65
authored and
targos
committed
fs: special input -1 on chown, lchown and fchown
PR-URL: #58836 Fixes: #58826 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent a118bfc commit 23bd4d1

File tree

5 files changed

+126
-9
lines changed

5 files changed

+126
-9
lines changed

‎src/node_file.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2859,10 +2859,10 @@ static void Chown(const FunctionCallbackInfo<Value>& args) {
28592859
ToNamespacedPath(env, &path);
28602860

28612861
CHECK(IsSafeJsInt(args[1]));
2862-
const uv_uid_t uid = FromV8Value<uv_uid_t>(args[1]);
2862+
const auto uid = FromV8Value<uv_uid_t, true>(args[1]);
28632863

28642864
CHECK(IsSafeJsInt(args[2]));
2865-
const uv_gid_t gid = FromV8Value<uv_gid_t>(args[2]);
2865+
const auto gid = FromV8Value<uv_gid_t, true>(args[2]);
28662866

28672867
if (argc > 3) { // chown(path, uid, gid, req)
28682868
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
@@ -2904,10 +2904,10 @@ static void FChown(const FunctionCallbackInfo<Value>& args) {
29042904
}
29052905

29062906
CHECK(IsSafeJsInt(args[1]));
2907-
const uv_uid_t uid = static_cast<uv_uid_t>(args[1].As<Integer>()->Value());
2907+
const auto uid = FromV8Value<uv_uid_t, true>(args[1]);
29082908

29092909
CHECK(IsSafeJsInt(args[2]));
2910-
const uv_gid_t gid = static_cast<uv_gid_t>(args[2].As<Integer>()->Value());
2910+
const auto gid = FromV8Value<uv_gid_t, true>(args[2]);
29112911

29122912
if (argc > 3) { // fchown(fd, uid, gid, req)
29132913
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
@@ -2934,10 +2934,10 @@ static void LChown(const FunctionCallbackInfo<Value>& args) {
29342934
ToNamespacedPath(env, &path);
29352935

29362936
CHECK(IsSafeJsInt(args[1]));
2937-
const uv_uid_t uid = FromV8Value<uv_uid_t>(args[1]);
2937+
const auto uid = FromV8Value<uv_uid_t, true>(args[1]);
29382938

29392939
CHECK(IsSafeJsInt(args[2]));
2940-
const uv_gid_t gid = FromV8Value<uv_gid_t>(args[2]);
2940+
const auto gid = FromV8Value<uv_gid_t, true>(args[2]);
29412941

29422942
if (argc > 3) { // lchown(path, uid, gid, req)
29432943
FSReqBase* req_wrap_async = GetReqWrap(args, 3);

‎src/util-inl.h‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -627,27 +627,36 @@ constexpr std::string_view FastStringKey::as_string_view() const {
627627

628628
// Converts a V8 numeric value to a corresponding C++ primitive or enum type.
629629
template <typename T,
630+
bool loose = false,
630631
typename = std::enable_if_t<std::numeric_limits<T>::is_specialized ||
631632
std::is_enum_v<T>>>
632633
T FromV8Value(v8::Local<v8::Value> value) {
633634
if constexpr (std::is_enum_v<T>) {
634635
using Underlying = std::underlying_type_t<T>;
635-
return static_cast<T>(FromV8Value<Underlying>(value));
636+
return static_cast<T>(FromV8Value<Underlying, loose>(value));
636637
} else if constexpr (std::is_integral_v<T> && std::is_unsigned_v<T>) {
637638
static_assert(
638639
std::numeric_limits<T>::max() <= std::numeric_limits<uint32_t>::max() &&
639640
std::numeric_limits<T>::min() >=
640641
std::numeric_limits<uint32_t>::min(),
641642
"Type is out of unsigned integer range");
642-
CHECK(value->IsUint32());
643+
if constexpr (!loose) {
644+
CHECK(value->IsUint32());
645+
} else {
646+
CHECK(value->IsNumber());
647+
}
643648
return static_cast<T>(value.As<v8::Uint32>()->Value());
644649
} else if constexpr (std::is_integral_v<T> && std::is_signed_v<T>) {
645650
static_assert(
646651
std::numeric_limits<T>::max() <= std::numeric_limits<int32_t>::max() &&
647652
std::numeric_limits<T>::min() >=
648653
std::numeric_limits<int32_t>::min(),
649654
"Type is out of signed integer range");
650-
CHECK(value->IsInt32());
655+
if constexpr (!loose) {
656+
CHECK(value->IsInt32());
657+
} else {
658+
CHECK(value->IsNumber());
659+
}
651660
return static_cast<T>(value.As<v8::Int32>()->Value());
652661
} else {
653662
static_assert(std::is_floating_point_v<T>,

‎test/parallel/test-fs-chown-negative-one.js‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const fs = require('fs');
6+
const path = require('path');
7+
const tmpdir = require('../common/tmpdir');
8+
9+
tmpdir.refresh();
10+
11+
const testFile = path.join(tmpdir.path, 'chown-test-file.txt');
12+
13+
fs.writeFileSync(testFile, 'test content for chown');
14+
15+
const stats = fs.statSync(testFile);
16+
const uid = stats.uid;
17+
const gid = stats.gid;
18+
19+
// -1 for uid and gid means "don't change the value"
20+
{
21+
fs.chown(testFile, -1, -1, common.mustSucceed(() => {
22+
const stats = fs.statSync(testFile);
23+
assert.strictEqual(stats.uid, uid);
24+
assert.strictEqual(stats.gid, gid);
25+
}));
26+
}
27+
{
28+
fs.chownSync(testFile, -1, -1);
29+
const stats = fs.statSync(testFile);
30+
assert.strictEqual(stats.uid, uid);
31+
assert.strictEqual(stats.gid, gid);
32+
}

‎test/parallel/test-fs-fchown-negative-one.js‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const fs = require('fs');
6+
const path = require('path');
7+
const tmpdir = require('../common/tmpdir');
8+
9+
tmpdir.refresh();
10+
11+
const testFilePath = path.join(tmpdir.path, 'fchown-test-file.txt');
12+
13+
fs.writeFileSync(testFilePath, 'test content for fchown');
14+
15+
{
16+
const fd = fs.openSync(testFilePath, 'r+');
17+
const stats = fs.fstatSync(fd);
18+
const uid = stats.uid;
19+
const gid = stats.gid;
20+
21+
fs.fchown(fd, -1, -1, common.mustSucceed(() => {
22+
const stats = fs.fstatSync(fd);
23+
assert.strictEqual(stats.uid, uid);
24+
assert.strictEqual(stats.gid, gid);
25+
fs.closeSync(fd);
26+
}));
27+
}
28+
29+
// Test sync fchown with -1 values
30+
{
31+
const fd = fs.openSync(testFilePath, 'r+');
32+
const stats = fs.fstatSync(fd);
33+
const uid = stats.uid;
34+
const gid = stats.gid;
35+
36+
fs.fchownSync(fd, -1, -1);
37+
const statsAfter = fs.fstatSync(fd);
38+
assert.strictEqual(statsAfter.uid, uid);
39+
assert.strictEqual(statsAfter.gid, gid);
40+
41+
fs.closeSync(fd);
42+
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const fs = require('fs');
6+
const path = require('path');
7+
const tmpdir = require('../common/tmpdir');
8+
9+
tmpdir.refresh();
10+
11+
const testFile = path.join(tmpdir.path, 'lchown-test-file.txt');
12+
const testLink = path.join(tmpdir.path, 'lchown-test-link');
13+
14+
fs.writeFileSync(testFile, 'test content for lchown');
15+
fs.symlinkSync(testFile, testLink);
16+
17+
const stats = fs.lstatSync(testLink);
18+
const uid = stats.uid;
19+
const gid = stats.gid;
20+
21+
// -1 for uid and gid means "don't change the value"
22+
{
23+
fs.lchown(testLink, -1, -1, common.mustSucceed(() => {
24+
const stats = fs.lstatSync(testLink);
25+
assert.strictEqual(stats.uid, uid);
26+
assert.strictEqual(stats.gid, gid);
27+
}));
28+
}
29+
{
30+
fs.lchownSync(testLink, -1, -1);
31+
const stats = fs.lstatSync(testLink);
32+
assert.strictEqual(stats.uid, uid);
33+
assert.strictEqual(stats.gid, gid);
34+
}

0 commit comments

Comments
 (0)