Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fs: validate fd synchronously on c++ #51027

Merged
merged 1 commit into from
Dec 18, 2023
Merged

fs: validate fd synchronously on c++ #51027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 6 additions & 21 deletions lib/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
anonrig marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -528,9 +527,7 @@ function close(fd, callback = defaultCloseCallback) {
* @returns {void}
*/
function closeSync(fd) {
fd = getValidatedFd(fd);

binding.close(fd);
binding.close(getValidatedFd(fd));
}

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -1289,7 +1283,6 @@ function fdatasync(fd, callback) {
* @returns {void}
*/
function fdatasyncSync(fd) {
fd = getValidatedFd(fd);
binding.fdatasync(fd);
}

Expand All @@ -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);
Expand All @@ -1314,7 +1306,6 @@ function fsync(fd, callback) {
* @returns {void}
*/
function fsyncSync(fd) {
fd = getValidatedFd(fd);
binding.fsync(fd);
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1884,7 +1873,6 @@ function unlinkSync(path) {
* @returns {void}
*/
function fchmod(fd, mode, callback) {
fd = getValidatedFd(fd);
mode = parseFileMode(mode, 'mode');
callback = makeCallback(callback);

Expand All @@ -1901,7 +1889,7 @@ function fchmod(fd, mode, callback) {
*/
function fchmodSync(fd, mode) {
binding.fchmod(
getValidatedFd(fd),
fd,
parseFileMode(mode, 'mode'),
);
}
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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'),
);
Expand Down
40 changes: 26 additions & 14 deletions src/node_file.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
85 changes: 85 additions & 0 deletions src/util.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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 changes: 5 additions & 0 deletions src/util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading