Skip to content

fs: throw fs.access errors in JS #17160

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.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 17 additions & 1 deletion lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,11 @@ fs.access = function(path, mode, callback) {
if (handleError((path = getPathFromURL(path)), callback))
return;

if (typeof path !== 'string' && !(path instanceof Buffer)) {
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'path',
['string', 'Buffer', 'URL']);
}

if (!nullCheck(path, callback))
return;

Expand All @@ -308,14 +313,25 @@ fs.access = function(path, mode, callback) {

fs.accessSync = function(path, mode) {
handleError((path = getPathFromURL(path)));

if (typeof path !== 'string' && !(path instanceof Buffer)) {
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'path',
['string', 'Buffer', 'URL']);
}

nullCheck(path);

if (mode === undefined)
mode = fs.F_OK;
else
mode = mode | 0;

binding.access(pathModule.toNamespacedPath(path), mode);
const ctx = {};
binding.access(pathModule.toNamespacedPath(path), mode, undefined, ctx);

if (ctx.code !== undefined) {
throw new errors.uvException(ctx);
}
};

fs.exists = function(path, callback) {
Expand Down
42 changes: 42 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,49 @@ function E(sym, val) {
messages.set(sym, typeof val === 'function' ? val : String(val));
}

// JS counterpart of StringFromPath, although here path is a buffer.
function stringFromPath(path) {
const str = path.toString();
if (process.platform !== 'win32') {
return str;
}

if (str.startsWith('\\\\?\\UNC\\')) {
return '\\\\' + str.slice(8);
} else if (str.startsWith('\\\\?\\')) {
return str.slice(4);
}
return str;
}

// This creates an error compatible with errors produced in UVException
// using the context collected in CollectUVExceptionInfo
// The goal is to migrate them to ERR_* errors later when
// compatibility is not a concern
function uvException(ctx) {
const err = new Error();
err.errno = ctx.errno;
err.code = ctx.code;
err.syscall = ctx.syscall;

let message = `${ctx.code}: ${ctx.message}, ${ctx.syscall}`;
if (ctx.path) {
const path = stringFromPath(ctx.path);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also @fhinkel does this still LGTY with the stringFromPath changes for fixing Windows?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still LGTM

message += ` '${path}'`;
err.path = path;
}
if (ctx.dest) {
const dest = stringFromPath(ctx.dest);
message += ` -> '${dest}'`;
err.dest = dest;
}
err.message = message;
Error.captureStackTrace(err, uvException);
return err;
}

module.exports = exports = {
uvException,
message,
Error: makeNodeError(Error),
TypeError: makeNodeError(TypeError),
Expand Down
68 changes: 58 additions & 10 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,31 @@ class fs_req_wrap {
DISALLOW_COPY_AND_ASSIGN(fs_req_wrap);
};

// Template counterpart of ASYNC_DEST_CALL
template <typename Func, typename... Args>
inline FSReqWrap* AsyncDestCall(Environment* env, Local<Object> req,
const char* dest, enum encoding enc, const char* syscall,
Func fn, Args... args) {
FSReqWrap* req_wrap = FSReqWrap::New(env, req, syscall, dest, enc);
int err = fn(env->event_loop(), req_wrap->req(), args..., After);
req_wrap->Dispatched();
if (err < 0) {
uv_fs_t* uv_req = req_wrap->req();
uv_req->result = err;
uv_req->path = nullptr;
After(uv_req);
req_wrap = nullptr;
}

return req_wrap;
}

// Template counterpart of ASYNC_CALL
template <typename Func, typename... Args>
inline FSReqWrap* AsyncCall(Environment* env, Local<Object> req,
enum encoding enc, const char* syscall, Func fn, Args... args) {
return AsyncDestCall(env, req, nullptr, enc, syscall, fn, args...);
}

#define ASYNC_DEST_CALL(func, request, dest, encoding, ...) \
Environment* env = Environment::GetCurrent(args); \
Expand All @@ -373,6 +398,28 @@ class fs_req_wrap {
#define ASYNC_CALL(func, req, encoding, ...) \
ASYNC_DEST_CALL(func, req, nullptr, encoding, __VA_ARGS__) \

// Template counterpart of SYNC_DEST_CALL
template <typename Func, typename... Args>
inline void SyncDestCall(Environment* env, Local<Value> ctx,
const char* path, const char* dest, const char* syscall,
Func fn, Args... args) {
fs_req_wrap req_wrap;
env->PrintSyncTrace();
int err = fn(env->event_loop(), &req_wrap.req, args..., nullptr);
if (err) {
Local<Context> context = env->context();
Local<Object> ctx_obj = ctx->ToObject(context).ToLocalChecked();
env->CollectUVExceptionInfo(ctx_obj, err, syscall, nullptr, path, dest);
}
}

// Template counterpart of SYNC_CALL
template <typename Func, typename... Args>
inline void SyncCall(Environment* env, Local<Value> ctx,
const char* path, const char* syscall, Func fn, Args... args) {
return SyncDestCall(env, ctx, path, nullptr, syscall, fn, args...);
}

#define SYNC_DEST_CALL(func, path, dest, ...) \
fs_req_wrap req_wrap; \
env->PrintSyncTrace(); \
Expand All @@ -394,21 +441,22 @@ class fs_req_wrap {
void Access(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args.GetIsolate());
HandleScope scope(env->isolate());

if (args.Length() < 2)
return TYPE_ERROR("path and mode are required");
if (!args[1]->IsInt32())
return TYPE_ERROR("mode must be an integer");
Local<Context> context = env->context();
CHECK_GE(args.Length(), 2);
CHECK(args[1]->IsInt32());

BufferValue path(env->isolate(), args[0]);
ASSERT_PATH(path)

int mode = static_cast<int>(args[1]->Int32Value());
int mode = static_cast<int>(args[1]->Int32Value(context).FromJust());

if (args[2]->IsObject()) {
ASYNC_CALL(access, args[2], UTF8, *path, mode);
Local<Object> req_obj = args[2]->ToObject(context).ToLocalChecked();
FSReqWrap* req_wrap = AsyncCall(
env, req_obj, UTF8, "access", uv_fs_access, *path, mode);
if (req_wrap != nullptr) {
args.GetReturnValue().Set(req_wrap->persistent());
}
} else {
SYNC_CALL(access, *path, *path, mode);
SyncCall(env, args[3], *path, "access", uv_fs_access, *path, mode);
}
}

Expand Down
13 changes: 10 additions & 3 deletions test/parallel/test-fs-access.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,16 @@ fs.access(readOnlyFile, fs.W_OK, common.mustCall((err) => {
}
}));

assert.throws(() => {
fs.access(100, fs.F_OK, common.mustNotCall());
}, /^TypeError: path must be a string or Buffer$/);
common.expectsError(
() => {
fs.access(100, fs.F_OK, common.mustNotCall());
},
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "path" argument must be one of type string, Buffer, or URL'
}
);

common.expectsError(
() => {
Expand Down
13 changes: 10 additions & 3 deletions test/parallel/test-fs-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,16 @@ assert.doesNotThrow(() => {
}));
});

assert.throws(() => {
fs.accessSync(true);
}, /path must be a string or Buffer/);
common.expectsError(
() => {
fs.accessSync(true);
},
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "path" argument must be one of type string, Buffer, or URL'
}
);

const dir = Buffer.from(fixtures.fixturesDir);
fs.readdir(dir, 'hex', common.mustCall((err, hexList) => {
Expand Down