Skip to content

Commit bf51026

Browse files
himself65aduh95
himself65
authored and
aduh95
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 1b076ef commit bf51026

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
@@ -2768,10 +2768,10 @@ static void Chown(const FunctionCallbackInfo<Value>& args) {
27682768
ToNamespacedPath(env, &path);
27692769

27702770
CHECK(IsSafeJsInt(args[1]));
2771-
const uv_uid_t uid = FromV8Value<uv_uid_t>(args[1]);
2771+
const auto uid = FromV8Value<uv_uid_t, true>(args[1]);
27722772

27732773
CHECK(IsSafeJsInt(args[2]));
2774-
const uv_gid_t gid = FromV8Value<uv_gid_t>(args[2]);
2774+
const auto gid = FromV8Value<uv_gid_t, true>(args[2]);
27752775

27762776
if (argc > 3) { // chown(path, uid, gid, req)
27772777
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
@@ -2813,10 +2813,10 @@ static void FChown(const FunctionCallbackInfo<Value>& args) {
28132813
}
28142814

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

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

28212821
if (argc > 3) { // fchown(fd, uid, gid, req)
28222822
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
@@ -2843,10 +2843,10 @@ static void LChown(const FunctionCallbackInfo<Value>& args) {
28432843
ToNamespacedPath(env, &path);
28442844

28452845
CHECK(IsSafeJsInt(args[1]));
2846-
const uv_uid_t uid = FromV8Value<uv_uid_t>(args[1]);
2846+
const auto uid = FromV8Value<uv_uid_t, true>(args[1]);
28472847

28482848
CHECK(IsSafeJsInt(args[2]));
2849-
const uv_gid_t gid = FromV8Value<uv_gid_t>(args[2]);
2849+
const auto gid = FromV8Value<uv_gid_t, true>(args[2]);
28502850

28512851
if (argc > 3) { // lchown(path, uid, gid, req)
28522852
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
@@ -607,27 +607,36 @@ constexpr std::string_view FastStringKey::as_string_view() const {
607607

608608
// Converts a V8 numeric value to a corresponding C++ primitive or enum type.
609609
template <typename T,
610+
bool loose = false,
610611
typename = std::enable_if_t<std::numeric_limits<T>::is_specialized ||
611612
std::is_enum_v<T>>>
612613
T FromV8Value(v8::Local<v8::Value> value) {
613614
if constexpr (std::is_enum_v<T>) {
614615
using Underlying = std::underlying_type_t<T>;
615-
return static_cast<T>(FromV8Value<Underlying>(value));
616+
return static_cast<T>(FromV8Value<Underlying, loose>(value));
616617
} else if constexpr (std::is_integral_v<T> && std::is_unsigned_v<T>) {
617618
static_assert(
618619
std::numeric_limits<T>::max() <= std::numeric_limits<uint32_t>::max() &&
619620
std::numeric_limits<T>::min() >=
620621
std::numeric_limits<uint32_t>::min(),
621622
"Type is out of unsigned integer range");
622-
CHECK(value->IsUint32());
623+
if constexpr (!loose) {
624+
CHECK(value->IsUint32());
625+
} else {
626+
CHECK(value->IsNumber());
627+
}
623628
return static_cast<T>(value.As<v8::Uint32>()->Value());
624629
} else if constexpr (std::is_integral_v<T> && std::is_signed_v<T>) {
625630
static_assert(
626631
std::numeric_limits<T>::max() <= std::numeric_limits<int32_t>::max() &&
627632
std::numeric_limits<T>::min() >=
628633
std::numeric_limits<int32_t>::min(),
629634
"Type is out of signed integer range");
630-
CHECK(value->IsInt32());
635+
if constexpr (!loose) {
636+
CHECK(value->IsInt32());
637+
} else {
638+
CHECK(value->IsNumber());
639+
}
631640
return static_cast<T>(value.As<v8::Int32>()->Value());
632641
} else {
633642
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)