OSSForks/node
1
0
Fork 0
mirror of https://github.com/zebrajr/node.git synced 2026-01-15 12:15:26 +00:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

fs: validate fd synchronously on c++

Browse Source 
PR-URL: https://github.com/nodejs/node/pull/51027
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
Yagiz Nizipli
2023-12-18 17:06:45 -05:00
committed by GitHub
parent 135948d584
commit 65e70bf54e
8 changed files with 198 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
lib/fs.js
View File

@@ -513,13 +513,12 @@ function defaultCloseCallback(err) {
* @returns {void}
*/
function close(fd, callback = defaultCloseCallback) {
fd = getValidatedFd(fd);
if (callback !== defaultCloseCallback)
callback = makeCallback(callback);
const req = new FSReqCallback();
req.oncomplete = callback;
binding.close(fd, req);
binding.close(getValidatedFd(fd), req);
}
/**
@@ -528,9 +527,7 @@ function close(fd, callback = defaultCloseCallback) {
* @returns {void}
*/
function closeSync(fd) {
fd = getValidatedFd(fd);
binding.close(fd);
binding.close(getValidatedFd(fd));
}
/**
@@ -1106,7 +1103,6 @@ function ftruncate(fd, len = 0, callback) {
callback = len;
len = 0;
}
fd = getValidatedFd(fd);
validateInteger(len, 'len');
len = MathMax(0, len);
callback = makeCallback(callback);
@@ -1123,7 +1119,6 @@ function ftruncate(fd, len = 0, callback) {
* @returns {void}
*/
function ftruncateSync(fd, len = 0) {
fd = getValidatedFd(fd);
validateInteger(len, 'len');
len = MathMax(0, len);
binding.ftruncate(fd, len);
@@ -1275,7 +1270,6 @@ function rmSync(path, options) {
* @returns {void}
*/
function fdatasync(fd, callback) {
fd = getValidatedFd(fd);
const req = new FSReqCallback();
req.oncomplete = makeCallback(callback);
binding.fdatasync(fd, req);
@@ -1289,7 +1283,6 @@ function fdatasync(fd, callback) {
* @returns {void}
*/
function fdatasyncSync(fd) {
fd = getValidatedFd(fd);
binding.fdatasync(fd);
}
@@ -1301,7 +1294,6 @@ function fdatasyncSync(fd) {
* @returns {void}
*/
function fsync(fd, callback) {
fd = getValidatedFd(fd);
const req = new FSReqCallback();
req.oncomplete = makeCallback(callback);
binding.fsync(fd, req);
@@ -1314,7 +1306,6 @@ function fsync(fd, callback) {
* @returns {void}
*/
function fsyncSync(fd) {
fd = getValidatedFd(fd);
binding.fsync(fd);
}
@@ -1535,7 +1526,6 @@ function fstat(fd, options = { bigint: false }, callback) {
callback = options;
options = kEmptyObject;
}
fd = getValidatedFd(fd);
callback = makeStatsCallback(callback);
const req = new FSReqCallback(options.bigint);
@@ -1618,7 +1608,6 @@ function statfs(path, options = { bigint: false }, callback) {
* @returns {Stats | undefined}
*/
function fstatSync(fd, options = { bigint: false }) {
fd = getValidatedFd(fd);
const stats = binding.fstat(fd, options.bigint, undefined, false);
if (stats === undefined) {
return;
@@ -1884,7 +1873,6 @@ function unlinkSync(path) {
* @returns {void}
*/
function fchmod(fd, mode, callback) {
fd = getValidatedFd(fd);
mode = parseFileMode(mode, 'mode');
callback = makeCallback(callback);
@@ -1901,7 +1889,7 @@ function fchmod(fd, mode, callback) {
*/
function fchmodSync(fd, mode) {
binding.fchmod(
getValidatedFd(fd),
fd,
parseFileMode(mode, 'mode'),
);
}
@@ -2029,14 +2017,13 @@ function lchownSync(path, uid, gid) {
* @returns {void}
*/
function fchown(fd, uid, gid, callback) {
fd = getValidatedFd(fd);
validateInteger(uid, 'uid', -1, kMaxUserId);
validateInteger(gid, 'gid', -1, kMaxUserId);
callback = makeCallback(callback);
const req = new FSReqCallback();
req.oncomplete = callback;
binding.fchown(fd, uid, gid, req);
binding.fchown(getValidatedFd(fd), uid, gid, req);
}
/**
@@ -2047,11 +2034,10 @@ function fchown(fd, uid, gid, callback) {
* @returns {void}
*/
function fchownSync(fd, uid, gid) {
fd = getValidatedFd(fd);
validateInteger(uid, 'uid', -1, kMaxUserId);
validateInteger(gid, 'gid', -1, kMaxUserId);
binding.fchown(fd, uid, gid);
binding.fchown(getValidatedFd(fd), uid, gid);
}
/**
@@ -2141,7 +2127,6 @@ function utimesSync(path, atime, mtime) {
* @returns {void}
*/
function futimes(fd, atime, mtime, callback) {
fd = getValidatedFd(fd);
atime = toUnixTimestamp(atime, 'atime');
mtime = toUnixTimestamp(mtime, 'mtime');
callback = makeCallback(callback);
@@ -2162,7 +2147,7 @@ function futimes(fd, atime, mtime, callback) {
*/
function futimesSync(fd, atime, mtime) {
binding.futimes(
getValidatedFd(fd),
fd,
toUnixTimestamp(atime, 'atime'),
toUnixTimestamp(mtime, 'mtime'),
);

40
src/node_file.cc
View File

@@ -1145,8 +1145,10 @@ static void FStat(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 2);
CHECK(args[0]->IsInt32());
int fd = args[0].As<Int32>()->Value();
int fd;
if (!GetValidatedFd(env, args[0]).To(&fd)) {
return;
}
bool use_bigint = args[1]->IsTrue();
if (!args[2]->IsUndefined()) { // fstat(fd, use_bigint, req)
@@ -1396,18 +1398,20 @@ static void FTruncate(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 2);
CHECK(args[0]->IsInt32());
const int fd = args[0].As<Int32>()->Value();
int fd;
if (!GetValidatedFd(env, args[0]).To(&fd)) {
return;
}
CHECK(IsSafeJsInt(args[1]));
const int64_t len = args[1].As<Integer>()->Value();
if (argc > 2) {
if (argc > 2) { // ftruncate(fd, len, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
FS_ASYNC_TRACE_BEGIN0(UV_FS_FTRUNCATE, req_wrap_async)
AsyncCall(env, req_wrap_async, args, "ftruncate", UTF8, AfterNoArgs,
uv_fs_ftruncate, fd, len);
} else {
} else { // ftruncate(fd, len)
FSReqWrapSync req_wrap_sync("ftruncate");
FS_SYNC_TRACE_BEGIN(ftruncate);
SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_ftruncate, fd, len);
@@ -1421,8 +1425,10 @@ static void Fdatasync(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 1);
CHECK(args[0]->IsInt32());
const int fd = args[0].As<Int32>()->Value();
int fd;
if (!GetValidatedFd(env, args[0]).To(&fd)) {
return;
}
if (argc > 1) { // fdatasync(fd, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 1);
@@ -1444,8 +1450,10 @@ static void Fsync(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 1);
CHECK(args[0]->IsInt32());
const int fd = args[0].As<Int32>()->Value();
int fd;
if (!GetValidatedFd(env, args[0]).To(&fd)) {
return;
}
if (argc > 1) {
FSReqBase* req_wrap_async = GetReqWrap(args, 1);
@@ -2518,8 +2526,10 @@ static void FChmod(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 2);
CHECK(args[0]->IsInt32());
const int fd = args[0].As<Int32>()->Value();
int fd;
if (!GetValidatedFd(env, args[0]).To(&fd)) {
return;
}
CHECK(args[1]->IsInt32());
const int mode = args[1].As<Int32>()->Value();
@@ -2674,8 +2684,10 @@ static void FUTimes(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 3);
CHECK(args[0]->IsInt32());
const int fd = args[0].As<Int32>()->Value();
int fd;
if (!GetValidatedFd(env, args[0]).To(&fd)) {
return;
}
CHECK(args[1]->IsNumber());
const double atime = args[1].As<Number>()->Value();

85
src/util.cc
View File

@@ -20,6 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "util.h" // NOLINT(build/include_inline)
#include <cmath>
#include "util-inl.h"
#include "debug_utils-inl.h"
@@ -31,6 +32,7 @@
#include "node_v8_platform-inl.h"
#include "string_bytes.h"
#include "uv.h"
#include "v8-value.h"
#ifdef _WIN32
#include <io.h> // _S_IREAD _S_IWRITE
@@ -702,4 +704,87 @@ RAIIIsolate::RAIIIsolate(const SnapshotData* data)
RAIIIsolate::~RAIIIsolate() {}
// Returns a string representation of the input value, including type.
// JavaScript implementation is available in lib/internal/errors.js
std::string DetermineSpecificErrorType(Environment* env,
v8::Local<v8::Value> input) {
if (input->IsFunction()) {
return "function";
} else if (input->IsString()) {
auto value = Utf8Value(env->isolate(), input).ToString();
if (value.size() > 28) {
value = value.substr(0, 25) + "...";
}
if (value.find('\'') == std::string::npos) {
return SPrintF("type string ('%s')", value);
}
// Stringify the input value.
Local<String> stringified =
v8::JSON::Stringify(env->context(), input).ToLocalChecked();
Utf8Value stringified_value(env->isolate(), stringified);
return SPrintF("type string (%s)", stringified_value.out());
} else if (input->IsObject()) {
v8::Local<v8::String> constructor_name =
input.As<v8::Object>()->GetConstructorName();
Utf8Value name(env->isolate(), constructor_name);
return SPrintF("an instance of %s", name.out());
}
Utf8Value utf8_value(env->isolate(),
input->ToString(env->context()).ToLocalChecked());
if (input->IsNumber() || input->IsInt32() || input->IsUint32()) {
auto value = input.As<v8::Number>()->Value();
if (std::isnan(value)) {
return "type number (NaN)";
} else if (std::isinf(value)) {
return "type number (Infinity)";
}
return SPrintF("type number (%s)", utf8_value.out());
} else if (input->IsBigInt() || input->IsBoolean() || input->IsSymbol()) {
Utf8Value type(env->isolate(), input->TypeOf(env->isolate()));
return SPrintF("type %s (%s)", type.out(), utf8_value.out());
}
// For example: null, undefined
return utf8_value.ToString();
}
v8::Maybe<int32_t> GetValidatedFd(Environment* env,
v8::Local<v8::Value> input) {
if (!input->IsInt32() && !input->IsNumber()) {
std::string error_type = node::DetermineSpecificErrorType(env, input);
THROW_ERR_INVALID_ARG_TYPE(env,
"The \"fd\" argument must be of type "
"number. Received %s",
error_type.c_str());
return v8::Nothing<int32_t>();
}
const double fd = input.As<v8::Number>()->Value();
const bool is_out_of_range = fd < 0 || fd > INT32_MAX;
if (is_out_of_range || !IsSafeJsInt(input)) {
Utf8Value utf8_value(
env->isolate(), input->ToDetailString(env->context()).ToLocalChecked());
if (is_out_of_range && !std::isinf(fd)) {
THROW_ERR_OUT_OF_RANGE(env,
"The value of \"fd\" is out of range. "
"It must be >= 0 && <= %s. Received %s",
std::to_string(INT32_MAX),
utf8_value.out());
} else {
THROW_ERR_OUT_OF_RANGE(
env,
"The value of \"fd\" is out of range. It must be an integer. "
"Received %s",
utf8_value.out());
}
return v8::Nothing<int32_t>();
}
return v8::Just(static_cast<int32_t>(fd));
}
} // namespace node

5
src/util.h
View File

@@ -1013,6 +1013,11 @@ class RAIIIsolate {
v8::Isolate::Scope isolate_scope_;
};
std::string DetermineSpecificErrorType(Environment* env,
v8::Local<v8::Value> input);
v8::Maybe<int32_t> GetValidatedFd(Environment* env, v8::Local<v8::Value> input);
} // namespace node
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

73
test/cctest/test_util.cc
View File

@@ -1,8 +1,13 @@
#include "debug_utils-inl.h"
#include "env-inl.h"
#include "gtest/gtest.h"
#include "node_options-inl.h"
#include "node_test_fixture.h"
#include "simdutf.h"
#include "util-inl.h"
#include "v8-function-callback.h"
#include "v8-primitive.h"
#include "v8.h"
using node::Calloc;
using node::Malloc;
@@ -14,7 +19,9 @@ using node::ToLower;
using node::UncheckedCalloc;
using node::UncheckedMalloc;
TEST(UtilTest, ListHead) {
class UtilTest : public EnvironmentTestFixture {};
TEST_F(UtilTest, ListHead) {
struct Item { node::ListNode<Item> node_; };
typedef node::ListHead<Item, &Item::node_> List;
@@ -68,7 +75,7 @@ TEST(UtilTest, ListHead) {
EXPECT_FALSE(list.begin() != list.end());
}
TEST(UtilTest, StringEqualNoCase) {
TEST_F(UtilTest, StringEqualNoCase) {
EXPECT_FALSE(StringEqualNoCase("a", "b"));
EXPECT_TRUE(StringEqualNoCase("", ""));
EXPECT_TRUE(StringEqualNoCase("equal", "equal"));
@@ -78,7 +85,7 @@ TEST(UtilTest, StringEqualNoCase) {
EXPECT_FALSE(StringEqualNoCase("equals", "equal"));
}
TEST(UtilTest, StringEqualNoCaseN) {
TEST_F(UtilTest, StringEqualNoCaseN) {
EXPECT_FALSE(StringEqualNoCaseN("a", "b", strlen("a")));
EXPECT_TRUE(StringEqualNoCaseN("", "", strlen("")));
EXPECT_TRUE(StringEqualNoCaseN("equal", "equal", strlen("equal")));
@@ -92,7 +99,7 @@ TEST(UtilTest, StringEqualNoCaseN) {
EXPECT_FALSE(StringEqualNoCaseN("abc\0abc", "abcd\0efg", strlen("abcdefgh")));
}
TEST(UtilTest, ToLower) {
TEST_F(UtilTest, ToLower) {
EXPECT_EQ('0', ToLower('0'));
EXPECT_EQ('a', ToLower('a'));
EXPECT_EQ('a', ToLower('A'));
@@ -105,28 +112,28 @@ TEST(UtilTest, ToLower) {
free(pointer); \
} while (0)
TEST(UtilTest, Malloc) {
TEST_F(UtilTest, Malloc) {
TEST_AND_FREE(Malloc<char>, 0);
TEST_AND_FREE(Malloc<char>, 1);
TEST_AND_FREE(Malloc, 0);
TEST_AND_FREE(Malloc, 1);
}
TEST(UtilTest, Calloc) {
TEST_F(UtilTest, Calloc) {
TEST_AND_FREE(Calloc<char>, 0);
TEST_AND_FREE(Calloc<char>, 1);
TEST_AND_FREE(Calloc, 0);
TEST_AND_FREE(Calloc, 1);
}
TEST(UtilTest, UncheckedMalloc) {
TEST_F(UtilTest, UncheckedMalloc) {
TEST_AND_FREE(UncheckedMalloc<char>, 0);
TEST_AND_FREE(UncheckedMalloc<char>, 1);
TEST_AND_FREE(UncheckedMalloc, 0);
TEST_AND_FREE(UncheckedMalloc, 1);
}
TEST(UtilTest, UncheckedCalloc) {
TEST_F(UtilTest, UncheckedCalloc) {
TEST_AND_FREE(UncheckedCalloc<char>, 0);
TEST_AND_FREE(UncheckedCalloc<char>, 1);
TEST_AND_FREE(UncheckedCalloc, 0);
@@ -212,7 +219,7 @@ static void MaybeStackBufferBasic() {
free(rawbuf);
}
TEST(UtilTest, MaybeStackBuffer) {
TEST_F(UtilTest, MaybeStackBuffer) {
MaybeStackBufferBasic<uint8_t>();
MaybeStackBufferBasic<uint16_t>();
@@ -253,7 +260,7 @@ TEST(UtilTest, MaybeStackBuffer) {
}
}
TEST(UtilTest, SPrintF) {
TEST_F(UtilTest, SPrintF) {
// %d, %u and %s all do the same thing. The actual C++ type is used to infer
// the right representation.
EXPECT_EQ(SPrintF("%s", false), "false");
@@ -300,6 +307,50 @@ TEST(UtilTest, SPrintF) {
EXPECT_EQ(SPrintF("%s", with_zero), with_zero);
}
TEST(UtilTest, DumpJavaScriptStackWithNoIsolate) {
TEST_F(UtilTest, DumpJavaScriptStackWithNoIsolate) {
node::DumpJavaScriptBacktrace(stderr);
}
TEST_F(UtilTest, DetermineSpecificErrorType) {
const v8::HandleScope handle_scope(isolate_);
Argv argv;
Env env{handle_scope, argv, node::EnvironmentFlags::kNoBrowserGlobals};
// Boolean
EXPECT_EQ(
node::DetermineSpecificErrorType(*env, v8::Boolean::New(isolate_, true)),
"type boolean (true)");
// BigInt
EXPECT_EQ(
node::DetermineSpecificErrorType(*env, v8::BigInt::New(isolate_, 255)),
"type bigint (255)");
// String
EXPECT_EQ(
node::DetermineSpecificErrorType(
*env, v8::String::NewFromUtf8(isolate_, "input").ToLocalChecked()),
"type string ('input')");
// String that calls JSONStringify
EXPECT_EQ(
node::DetermineSpecificErrorType(
*env, v8::String::NewFromUtf8(isolate_, "'input'").ToLocalChecked()),
"type string (\"'input'\")");
EXPECT_EQ(node::DetermineSpecificErrorType(
*env,
v8::String::NewFromUtf8(isolate_,
"string with more than 26 characters")
.ToLocalChecked()),
"type string ('string with more than 26 ...')");
// Number, Int32, Uint32
EXPECT_EQ(
node::DetermineSpecificErrorType(*env, v8::Number::New(isolate_, 10)),
"type number (10)");
EXPECT_EQ(
node::DetermineSpecificErrorType(*env, v8::Int32::New(isolate_, -255)),
"type number (-255)");
EXPECT_EQ(
node::DetermineSpecificErrorType(*env, v8::Uint32::New(isolate_, 255)),
"type number (255)");
}

22
test/parallel/test-fs-fchmod.js
View File

@@ -14,8 +14,8 @@ const fs = require('fs');
message: 'The "fd" argument must be of type number.' +
common.invalidArgTypeHelper(input)
};
assert.throws(() => fs.fchmod(input), errObj);
assert.throws(() => fs.fchmodSync(input), errObj);
assert.throws(() => fs.fchmod(input, 0o666, () => {}), errObj);
assert.throws(() => fs.fchmodSync(input, 0o666), errObj);
});
@@ -38,8 +38,8 @@ assert.throws(() => fs.fchmod(1, '123x'), {
message: 'The value of "fd" is out of range. It must be >= 0 && <= ' +
`2147483647. Received ${input}`
};
assert.throws(() => fs.fchmod(input), errObj);
assert.throws(() => fs.fchmodSync(input), errObj);
assert.throws(() => fs.fchmod(input, 0o666, () => {}), errObj);
assert.throws(() => fs.fchmodSync(input, 0o666), errObj);
});
[-1, 2 ** 32].forEach((input) => {
@@ -50,7 +50,7 @@ assert.throws(() => fs.fchmod(1, '123x'), {
`4294967295. Received ${input}`
};
assert.throws(() => fs.fchmod(1, input), errObj);
assert.throws(() => fs.fchmod(1, input, () => {}), errObj);
assert.throws(() => fs.fchmodSync(1, input), errObj);
});
@@ -61,10 +61,10 @@ assert.throws(() => fs.fchmod(1, '123x'), {
message: 'The value of "fd" is out of range. It must be an integer. ' +
`Received ${input}`
};
assert.throws(() => fs.fchmod(input), errObj);
assert.throws(() => fs.fchmodSync(input), errObj);
assert.throws(() => fs.fchmod(input, 0o666, () => {}), errObj);
assert.throws(() => fs.fchmodSync(input, 0o666), errObj);
errObj.message = errObj.message.replace('fd', 'mode');
assert.throws(() => fs.fchmod(1, input), errObj);
assert.throws(() => fs.fchmod(1, input, () => {}), errObj);
assert.throws(() => fs.fchmodSync(1, input), errObj);
});
@@ -75,9 +75,9 @@ assert.throws(() => fs.fchmod(1, '123x'), {
message: 'The value of "fd" is out of range. It must be an integer. ' +
`Received ${input}`
};
assert.throws(() => fs.fchmod(input), errObj);
assert.throws(() => fs.fchmodSync(input), errObj);
assert.throws(() => fs.fchmod(input, 0o666, () => {}), errObj);
assert.throws(() => fs.fchmodSync(input, 0o666), errObj);
errObj.message = errObj.message.replace('fd', 'mode');
assert.throws(() => fs.fchmod(1, input), errObj);
assert.throws(() => fs.fchmod(1, input, () => {}), errObj);
assert.throws(() => fs.fchmodSync(1, input), errObj);
});

4
test/parallel/test-fs-fchown.js
View File

@@ -5,8 +5,8 @@ const assert = require('assert');
const fs = require('fs');
function testFd(input, errObj) {
assert.throws(() => fs.fchown(input), errObj);
assert.throws(() => fs.fchownSync(input), errObj);
assert.throws(() => fs.fchown(input, 0, 0, () => {}), errObj);
assert.throws(() => fs.fchownSync(input, 0, 0), errObj);
}
function testUid(input, errObj) {

2
test/parallel/test-fs-truncate.js
View File

@@ -270,7 +270,7 @@ function testFtruncate(cb) {
['', false, null, undefined, {}, []].forEach((input) => {
['ftruncate', 'ftruncateSync'].forEach((fnName) => {
assert.throws(
() => fs[fnName](input),
() => fs[fnName](input, 1, () => {}),
{
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',