Skip to content

Commit 07d3409

Browse files
committed
fs: throw fs.access errors in JS
- Migrate the type check of path to ERR_INVALID_ARG_TYPE - Add template counterparts of ASYNC_CALL, ASYNC_DEST_CALL, SYNC_CALL, SYNC_DEST_CALL - Port StringFromPath and UVException to JavaScript - Migrate the access binding to collect the error context in C++, then throw the error in JS PR-URL: #17160 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 73154c0 commit 07d3409

File tree

5 files changed

+137
-17
lines changed

5 files changed

+137
-17
lines changed

lib/fs.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,11 @@ fs.access = function(path, mode, callback) {
297297
if (handleError((path = getPathFromURL(path)), callback))
298298
return;
299299

300+
if (typeof path !== 'string' && !(path instanceof Buffer)) {
301+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'path',
302+
['string', 'Buffer', 'URL']);
303+
}
304+
300305
if (!nullCheck(path, callback))
301306
return;
302307

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

309314
fs.accessSync = function(path, mode) {
310315
handleError((path = getPathFromURL(path)));
316+
317+
if (typeof path !== 'string' && !(path instanceof Buffer)) {
318+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'path',
319+
['string', 'Buffer', 'URL']);
320+
}
321+
311322
nullCheck(path);
312323

313324
if (mode === undefined)
314325
mode = fs.F_OK;
315326
else
316327
mode = mode | 0;
317328

318-
binding.access(pathModule.toNamespacedPath(path), mode);
329+
const ctx = {};
330+
binding.access(pathModule.toNamespacedPath(path), mode, undefined, ctx);
331+
332+
if (ctx.code !== undefined) {
333+
throw new errors.uvException(ctx);
334+
}
319335
};
320336

321337
fs.exists = function(path, callback) {

lib/internal/errors.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,49 @@ function E(sym, val) {
194194
messages.set(sym, typeof val === 'function' ? val : String(val));
195195
}
196196

197+
// JS counterpart of StringFromPath, although here path is a buffer.
198+
function stringFromPath(path) {
199+
const str = path.toString();
200+
if (process.platform !== 'win32') {
201+
return str;
202+
}
203+
204+
if (str.startsWith('\\\\?\\UNC\\')) {
205+
return '\\\\' + str.slice(8);
206+
} else if (str.startsWith('\\\\?\\')) {
207+
return str.slice(4);
208+
}
209+
return str;
210+
}
211+
212+
// This creates an error compatible with errors produced in UVException
213+
// using the context collected in CollectUVExceptionInfo
214+
// The goal is to migrate them to ERR_* errors later when
215+
// compatibility is not a concern
216+
function uvException(ctx) {
217+
const err = new Error();
218+
err.errno = ctx.errno;
219+
err.code = ctx.code;
220+
err.syscall = ctx.syscall;
221+
222+
let message = `${ctx.code}: ${ctx.message}, ${ctx.syscall}`;
223+
if (ctx.path) {
224+
const path = stringFromPath(ctx.path);
225+
message += ` '${path}'`;
226+
err.path = path;
227+
}
228+
if (ctx.dest) {
229+
const dest = stringFromPath(ctx.dest);
230+
message += ` -> '${dest}'`;
231+
err.dest = dest;
232+
}
233+
err.message = message;
234+
Error.captureStackTrace(err, uvException);
235+
return err;
236+
}
237+
197238
module.exports = exports = {
239+
uvException,
198240
message,
199241
Error: makeNodeError(Error),
200242
TypeError: makeNodeError(TypeError),

src/node_file.cc

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,31 @@ class fs_req_wrap {
349349
DISALLOW_COPY_AND_ASSIGN(fs_req_wrap);
350350
};
351351

352+
// Template counterpart of ASYNC_DEST_CALL
353+
template <typename Func, typename... Args>
354+
inline FSReqWrap* AsyncDestCall(Environment* env, Local<Object> req,
355+
const char* dest, enum encoding enc, const char* syscall,
356+
Func fn, Args... args) {
357+
FSReqWrap* req_wrap = FSReqWrap::New(env, req, syscall, dest, enc);
358+
int err = fn(env->event_loop(), req_wrap->req(), args..., After);
359+
req_wrap->Dispatched();
360+
if (err < 0) {
361+
uv_fs_t* uv_req = req_wrap->req();
362+
uv_req->result = err;
363+
uv_req->path = nullptr;
364+
After(uv_req);
365+
req_wrap = nullptr;
366+
}
367+
368+
return req_wrap;
369+
}
370+
371+
// Template counterpart of ASYNC_CALL
372+
template <typename Func, typename... Args>
373+
inline FSReqWrap* AsyncCall(Environment* env, Local<Object> req,
374+
enum encoding enc, const char* syscall, Func fn, Args... args) {
375+
return AsyncDestCall(env, req, nullptr, enc, syscall, fn, args...);
376+
}
352377

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

401+
// Template counterpart of SYNC_DEST_CALL
402+
template <typename Func, typename... Args>
403+
inline void SyncDestCall(Environment* env, Local<Value> ctx,
404+
const char* path, const char* dest, const char* syscall,
405+
Func fn, Args... args) {
406+
fs_req_wrap req_wrap;
407+
env->PrintSyncTrace();
408+
int err = fn(env->event_loop(), &req_wrap.req, args..., nullptr);
409+
if (err) {
410+
Local<Context> context = env->context();
411+
Local<Object> ctx_obj = ctx->ToObject(context).ToLocalChecked();
412+
env->CollectUVExceptionInfo(ctx_obj, err, syscall, nullptr, path, dest);
413+
}
414+
}
415+
416+
// Template counterpart of SYNC_CALL
417+
template <typename Func, typename... Args>
418+
inline void SyncCall(Environment* env, Local<Value> ctx,
419+
const char* path, const char* syscall, Func fn, Args... args) {
420+
return SyncDestCall(env, ctx, path, nullptr, syscall, fn, args...);
421+
}
422+
376423
#define SYNC_DEST_CALL(func, path, dest, ...) \
377424
fs_req_wrap req_wrap; \
378425
env->PrintSyncTrace(); \
@@ -394,21 +441,22 @@ class fs_req_wrap {
394441
void Access(const FunctionCallbackInfo<Value>& args) {
395442
Environment* env = Environment::GetCurrent(args.GetIsolate());
396443
HandleScope scope(env->isolate());
397-
398-
if (args.Length() < 2)
399-
return TYPE_ERROR("path and mode are required");
400-
if (!args[1]->IsInt32())
401-
return TYPE_ERROR("mode must be an integer");
444+
Local<Context> context = env->context();
445+
CHECK_GE(args.Length(), 2);
446+
CHECK(args[1]->IsInt32());
402447

403448
BufferValue path(env->isolate(), args[0]);
404-
ASSERT_PATH(path)
405-
406-
int mode = static_cast<int>(args[1]->Int32Value());
449+
int mode = static_cast<int>(args[1]->Int32Value(context).FromJust());
407450

408451
if (args[2]->IsObject()) {
409-
ASYNC_CALL(access, args[2], UTF8, *path, mode);
452+
Local<Object> req_obj = args[2]->ToObject(context).ToLocalChecked();
453+
FSReqWrap* req_wrap = AsyncCall(
454+
env, req_obj, UTF8, "access", uv_fs_access, *path, mode);
455+
if (req_wrap != nullptr) {
456+
args.GetReturnValue().Set(req_wrap->persistent());
457+
}
410458
} else {
411-
SYNC_CALL(access, *path, *path, mode);
459+
SyncCall(env, args[3], *path, "access", uv_fs_access, *path, mode);
412460
}
413461
}
414462

test/parallel/test-fs-access.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,16 @@ fs.access(readOnlyFile, fs.W_OK, common.mustCall((err) => {
8181
}
8282
}));
8383

84-
assert.throws(() => {
85-
fs.access(100, fs.F_OK, common.mustNotCall());
86-
}, /^TypeError: path must be a string or Buffer$/);
84+
common.expectsError(
85+
() => {
86+
fs.access(100, fs.F_OK, common.mustNotCall());
87+
},
88+
{
89+
code: 'ERR_INVALID_ARG_TYPE',
90+
type: TypeError,
91+
message: 'The "path" argument must be one of type string, Buffer, or URL'
92+
}
93+
);
8794

8895
common.expectsError(
8996
() => {

test/parallel/test-fs-buffer.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,16 @@ assert.doesNotThrow(() => {
2525
}));
2626
});
2727

28-
assert.throws(() => {
29-
fs.accessSync(true);
30-
}, /path must be a string or Buffer/);
28+
common.expectsError(
29+
() => {
30+
fs.accessSync(true);
31+
},
32+
{
33+
code: 'ERR_INVALID_ARG_TYPE',
34+
type: TypeError,
35+
message: 'The "path" argument must be one of type string, Buffer, or URL'
36+
}
37+
);
3138

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

0 commit comments

Comments
 (0)