Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
fs: add access() and accessSync()
Browse files Browse the repository at this point in the history
fs.exists() and fs.existsSync() do not follow the typical error first
callback convention. access() and accessSync() are added as alternatives
in this commit.

PR-URL: #8714
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
cjihrig authored and trevnorris committed Jan 13, 2015
1 parent 1fad373 commit 2944934
Show file tree
Hide file tree
Showing 5 changed files with 213 additions and 0 deletions.
33 changes: 33 additions & 0 deletions doc/api/fs.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -656,10 +656,43 @@ that leaves you vulnerable to race conditions: another process may remove the
file between the calls to `fs.exists()` and `fs.open()`. Just open the file
and handle the error when it's not there.

`fs.exists()` will be deprecated.

This comment has been minimized.

Copy link
@ronkorving

ronkorving Jan 13, 2015

Why deprecate this though? Is there really no use case left?
There must be so many lines of code these days that depend on fs.exists that deprecation in a module as stable as fs seems very questionable.

This comment has been minimized.

Copy link
@cjihrig

cjihrig Jan 13, 2015

Author

fs.access() provides the same functionality as fs.exists() and more:

  • exists() does not follow the error first callback style, which confuses people and causes issues with promisify and similar tools.
  • exists() can only check for file existence, while access() can also check for specific permissions (read, write, execute).
  • exists() performs a stat() inside of a try...catch. This causes any potentially useful error information to be lost. The full stat() is also slower.

This comment has been minimized.

Copy link
@Fishrock123

Fishrock123 Jan 13, 2015

Perhaps we should say "use access instead"?

This comment has been minimized.

Copy link
@reqshark

reqshark Jan 13, 2015

what happens when you call fs.accessSync() on something that doesn't exist?

This comment has been minimized.

Copy link
@ronkorving

ronkorving Jan 14, 2015

The fact that access() can check for more is cool (and I'm not arguing there's no use case for that), but does it also call more or heavier syscalls? If so, there is a place for exists(). I don't like running expensive system calls that give me more information than I need, when there's a faster way.

This comment has been minimized.

Copy link
@trevnorris

trevnorris Jan 14, 2015

@ronkorving actually it's the opposite. fs.exists() simply did an fs.stat() and returned whether the file existed or not. access(2) is a much cheaper way of finding out if a file exists or not.

This comment has been minimized.

Copy link
@ronkorving

ronkorving Jan 15, 2015

In that case, I'm fully in support of deprecation. Thanks for the clarification.


## fs.existsSync(path)

Synchronous version of `fs.exists`.

`fs.existsSync()` will be deprecated.

## fs.access(path[, mode], callback)

Tests a user's permissions for the file specified by `path`. `mode` is an
optional integer that specifies the accessibility checks to be performed. The
following constants define the possible values of `mode`. It is possible to
create a mask consisting of the bitwise OR of two or more values.

- `fs.F_OK` - File is visible to the calling process. This is useful for
determining if a file exists, but says nothing about `rwx` permissions.
Default if no `mode` is specified.
- `fs.R_OK` - File can be read by the calling process.
- `fs.W_OK` - File can be written by the calling process.
- `fs.X_OK` - File can be executed by the calling process. This has no effect
on Windows (will behave like `fs.F_OK`).

The final argument, `callback`, is a callback function that is invoked with
a possible error argument. If any of the accessibility checks fail, the error
argument will be populated. The following example checks if the file
`/etc/passwd` can be read and written by the current process.

fs.access('/etc/passwd', fs.R_OK | fs.W_OK, function(err) {
util.debug(err ? 'no access!' : 'can read/write');
});

## fs.accessSync(path[, mode])

Synchronous version of `fs.access`. This throws if any accessibility checks
fail, and does nothing otherwise.

## Class: fs.Stats

Objects returned from `fs.stat()`, `fs.lstat()` and `fs.fstat()` and their
Expand Down
37 changes: 37 additions & 0 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ var O_RDWR = constants.O_RDWR || 0;
var O_SYNC = constants.O_SYNC || 0;
var O_TRUNC = constants.O_TRUNC || 0;
var O_WRONLY = constants.O_WRONLY || 0;
var F_OK = constants.F_OK || 0;
var R_OK = constants.R_OK || 0;
var W_OK = constants.W_OK || 0;
var X_OK = constants.X_OK || 0;

var isWindows = process.platform === 'win32';

Expand Down Expand Up @@ -181,6 +185,39 @@ fs.Stats.prototype.isSocket = function() {
return this._checkModeProperty(constants.S_IFSOCK);
};

fs.F_OK = F_OK;
fs.R_OK = R_OK;
fs.W_OK = W_OK;
fs.X_OK = X_OK;

fs.access = function(path, mode, callback) {
if (!nullCheck(path, callback))
return;

if (typeof mode === 'function') {
callback = mode;
mode = F_OK;
} else if (typeof callback !== 'function') {
throw new TypeError('callback must be a function');
}

mode = mode | 0;
var req = new FSReqWrap();
req.oncomplete = makeCallback(callback);
binding.access(pathModule._makeLong(path), mode, req);
};

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

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

binding.access(pathModule._makeLong(path), mode);
};

fs.exists = function(path, callback) {
if (!nullCheck(path, cb)) return;
var req = new FSReqWrap();
Expand Down
16 changes: 16 additions & 0 deletions src/node_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,22 @@ void DefineSystemConstants(Handle<Object> target) {
#ifdef S_IXOTH
NODE_DEFINE_CONSTANT(target, S_IXOTH);
#endif

#ifdef F_OK
NODE_DEFINE_CONSTANT(target, F_OK);
#endif

#ifdef R_OK
NODE_DEFINE_CONSTANT(target, R_OK);
#endif

#ifdef W_OK
NODE_DEFINE_CONSTANT(target, W_OK);
#endif

#ifdef X_OK
NODE_DEFINE_CONSTANT(target, X_OK);
#endif
}

void DefineUVConstants(Handle<Object> target) {
Expand Down
28 changes: 28 additions & 0 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ static void After(uv_fs_t *req) {
argc = 1;
break;

case UV_FS_ACCESS:
argv[1] = Integer::New(env->isolate(), req->result);
break;

case UV_FS_UTIME:
case UV_FS_FUTIME:
argc = 0;
Expand Down Expand Up @@ -320,6 +324,29 @@ struct fs_req_wrap {
#define SYNC_RESULT err


static void Access(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args.GetIsolate());
HandleScope scope(env->isolate());

if (args.Length() < 2)
return THROW_BAD_ARGS;
if (!args[0]->IsString())
return TYPE_ERROR("path must be a string");
if (!args[1]->IsInt32())
return TYPE_ERROR("mode must be an integer");

node::Utf8Value path(args[0]);
int mode = static_cast<int>(args[1]->Int32Value());

if (args[2]->IsObject()) {
ASYNC_CALL(access, args[2], *path, mode);
} else {
SYNC_CALL(access, *path, *path, mode);
args.GetReturnValue().Set(SYNC_RESULT);
}
}


static void Close(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args.GetIsolate());
HandleScope scope(env->isolate());
Expand Down Expand Up @@ -1141,6 +1168,7 @@ void InitFs(Handle<Object> target,
FIXED_ONE_BYTE_STRING(env->isolate(), "FSInitialize"),
FunctionTemplate::New(env->isolate(), FSInitialize)->GetFunction());

NODE_SET_METHOD(target, "access", Access);
NODE_SET_METHOD(target, "close", Close);
NODE_SET_METHOD(target, "open", Open);
NODE_SET_METHOD(target, "read", Read);
Expand Down
99 changes: 99 additions & 0 deletions test/simple/test-fs-access.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// Copyright Joyent, Inc. and other Node contributors.
//
// Permission is hereby granted, free of charge, to any person obtaining a
// copy of this software and associated documentation files (the
// "Software"), to deal in the Software without restriction, including
// without limitation the rights to use, copy, modify, merge, publish,
// distribute, sublicense, and/or sell copies of the Software, and to permit
// persons to whom the Software is furnished to do so, subject to the
// following conditions:
//
// The above copyright notice and this permission notice shall be included
// in all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

var common = require('../common');
var assert = require('assert');
var fs = require('fs');
var path = require('path');
var doesNotExist = __filename + '__this_should_not_exist';
var readOnlyFile = path.join(common.tmpDir, 'read_only_file');

var removeFile = function(file) {
try {
fs.unlinkSync(file);
} catch (err) {
// Ignore error
}
};

var createReadOnlyFile = function(file) {
removeFile(file);
fs.writeFileSync(file, '');
fs.chmodSync(file, 0444);
};

createReadOnlyFile(readOnlyFile);

assert(typeof fs.F_OK === 'number');
assert(typeof fs.R_OK === 'number');
assert(typeof fs.W_OK === 'number');
assert(typeof fs.X_OK === 'number');

fs.access(__filename, function(err) {
assert.strictEqual(err, null, 'error should not exist');
});

fs.access(__filename, fs.R_OK, function(err) {
assert.strictEqual(err, null, 'error should not exist');
});

fs.access(doesNotExist, function(err) {
assert.notEqual(err, null, 'error should exist');
assert.strictEqual(err.code, 'ENOENT');
assert.strictEqual(err.path, doesNotExist);
});

fs.access(readOnlyFile, fs.F_OK | fs.R_OK, function(err) {
assert.strictEqual(err, null, 'error should not exist');
});

fs.access(readOnlyFile, fs.W_OK, function(err) {
assert.notEqual(err, null, 'error should exist');
assert.strictEqual(err.path, readOnlyFile);
});

assert.throws(function() {
fs.access(100, fs.F_OK, function(err) {});
}, /path must be a string/);

assert.throws(function() {
fs.access(__filename, fs.F_OK);
}, /callback must be a function/);

assert.doesNotThrow(function() {
fs.accessSync(__filename);
});

assert.doesNotThrow(function() {
var mode = fs.F_OK | fs.R_OK | fs.W_OK;

fs.accessSync(__filename, mode);
});

assert.throws(function() {
fs.accessSync(doesNotExist);
}, function (err) {
return err.code === 'ENOENT' && err.path === doesNotExist;
});

process.on('exit', function() {
removeFile(readOnlyFile);
});

0 comments on commit 2944934

Please sign in to comment.