From 90223198789b3c444d36463a3f64ec3d3ad75b9e Mon Sep 17 00:00:00 2001 From: cjihrig Date: Tue, 11 Nov 2014 17:43:18 -0500 Subject: [PATCH 1/2] fs: add access() and accessSync() fs.exists() and fs.existsSync() do not follow typical node conventions. access() and accessSync() are added as alternatives in this commit. Conflicts: src/node_file.cc --- doc/api/fs.markdown | 35 ++++++++++++ lib/fs.js | 37 ++++++++++++ src/node_constants.cc | 16 ++++++ src/node_file.cc | 28 +++++++++ test/simple/test-fs-access.js | 105 ++++++++++++++++++++++++++++++++++ 5 files changed, 221 insertions(+) create mode 100644 test/simple/test-fs-access.js diff --git a/doc/api/fs.markdown b/doc/api/fs.markdown index c6bfa2456bc17d..24929672e289b3 100644 --- a/doc/api/fs.markdown +++ b/doc/api/fs.markdown @@ -656,10 +656,45 @@ 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. + ## 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, and the result of `access()`. If any of the +accessibility checks fail, the error argument will be populated. If all of the +checks are successful, the result of `access()` will be `true`. 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, access) { + util.debug(access ? 'can read/write' : 'no access!'); + }); + +## fs.accessSync(path[, mode]) + +Synchronous version of `fs.access`. This returns `true` on success, and throws +if any accessibility checks fail. + ## Class: fs.Stats Objects returned from `fs.stat()`, `fs.lstat()` and `fs.fstat()` and their diff --git a/lib/fs.js b/lib/fs.js index 85bfe2b630825f..f0b87bdeab8dbc 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -50,6 +50,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'; @@ -182,6 +186,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; + binding.access(pathModule._makeLong(path), mode, function(err, access) { + callback(err, access === 0); + }); +}; + +fs.accessSync = function(path, mode) { + nullCheck(path); + + if (mode === undefined) + mode = F_OK; + else + mode = mode | 0; + + return binding.access(pathModule._makeLong(path), mode) === 0; +}; + fs.exists = function(path, callback) { if (!nullCheck(path, cb)) return; binding.stat(pathModule._makeLong(path), cb); diff --git a/src/node_constants.cc b/src/node_constants.cc index 45840d8653eef8..86fa544d19ab5c 100644 --- a/src/node_constants.cc +++ b/src/node_constants.cc @@ -1107,6 +1107,22 @@ void DefineSystemConstants(Handle 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 target) { diff --git a/src/node_file.cc b/src/node_file.cc index 6d9e3ec91beea0..9e4de777c8cde5 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -174,6 +174,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; @@ -308,6 +312,29 @@ struct fs_req_wrap { #define SYNC_RESULT err +static void Access(const FunctionCallbackInfo& 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(args[1]->Int32Value()); + + if (args[2]->IsFunction()) { + ASYNC_CALL(access, args[2], *path, mode); + } else { + SYNC_CALL(access, *path, *path, mode); + args.GetReturnValue().Set(SYNC_RESULT); + } +} + + static void Close(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -1103,6 +1130,7 @@ void InitFs(Handle target, target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "FSInitialize"), env->NewFunctionTemplate(FSInitialize)->GetFunction()); + env->SetMethod(target, "access", Access); env->SetMethod(target, "close", Close); env->SetMethod(target, "open", Open); env->SetMethod(target, "read", Read); diff --git a/test/simple/test-fs-access.js b/test/simple/test-fs-access.js new file mode 100644 index 00000000000000..50a6ea7b39eb57 --- /dev/null +++ b/test/simple/test-fs-access.js @@ -0,0 +1,105 @@ +// Copyright io.js 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, access) { + assert.strictEqual(err, null, 'error should not exist'); + assert.strictEqual(access, true, 'access should be true'); +}); + +fs.access(__filename, fs.R_OK, function(err, access) { + assert.strictEqual(err, null, 'error should not exist'); + assert.strictEqual(access, true, 'access should be true'); +}); + +fs.access(doesNotExist, function(err, access) { + assert.notEqual(err, null, 'error should exist'); + assert.strictEqual(err.code, 'ENOENT'); + assert.strictEqual(err.path, doesNotExist); + assert.strictEqual(access, false, 'access should be false'); +}); + +fs.access(readOnlyFile, fs.F_OK | fs.R_OK, function(err, access) { + assert.strictEqual(err, null, 'error should not exist'); + assert.strictEqual(access, true, 'access should be true'); +}); + +fs.access(readOnlyFile, fs.W_OK, function(err, access) { + assert.notEqual(err, null, 'error should exist'); + assert.strictEqual(err.path, readOnlyFile); + assert.strictEqual(access, false, 'access should be false'); +}); + +assert.throws(function() { + fs.access(100, fs.F_OK, function(err, access) {}); +}, /path must be a string/); + +assert.throws(function() { + fs.access(__filename, fs.F_OK); +}, /callback must be a function/); + +assert.doesNotThrow(function() { + var access = fs.accessSync(__filename); + assert.strictEqual(access, true, 'access should be true'); +}); + +assert.doesNotThrow(function() { + var mode = fs.F_OK | fs.R_OK | fs.W_OK; + var access = fs.accessSync(__filename, mode); + assert.strictEqual(access, true, 'access should be true'); +}); + +assert.throws(function() { + fs.accessSync(doesNotExist); +}, function (err) { + return err.code === 'ENOENT' && err.path === doesNotExist; +}); + +process.on('exit', function() { + removeFile(readOnlyFile); +}); From 52cdcf8f326af1fa55c047a929ce8ad933c72b42 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Tue, 9 Dec 2014 21:26:13 -0500 Subject: [PATCH 2/2] remove the excess value returned by access{Sync}(). --- doc/api/fs.markdown | 18 ++++++++---------- lib/fs.js | 6 ++---- src/node_file.cc | 6 +----- test/simple/test-fs-access.js | 24 +++++++++--------------- 4 files changed, 20 insertions(+), 34 deletions(-) diff --git a/doc/api/fs.markdown b/doc/api/fs.markdown index 24929672e289b3..17bc55ca5ed561 100644 --- a/doc/api/fs.markdown +++ b/doc/api/fs.markdown @@ -680,20 +680,18 @@ Default if no `mode` is specified. on Windows (will behave like `fs.F_OK`). The final argument, `callback`, is a callback function that is invoked with -a possible error argument, and the result of `access()`. If any of the -accessibility checks fail, the error argument will be populated. If all of the -checks are successful, the result of `access()` will be `true`. 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, access) { - util.debug(access ? 'can read/write' : 'no access!'); +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 returns `true` on success, and throws -if any accessibility checks fail. +Synchronous version of `fs.access`. This throws if any accessibility checks +fail, and does nothing otherwise. ## Class: fs.Stats diff --git a/lib/fs.js b/lib/fs.js index f0b87bdeab8dbc..ba2d9f23d3ae62 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -203,9 +203,7 @@ fs.access = function(path, mode, callback) { } mode = mode | 0; - binding.access(pathModule._makeLong(path), mode, function(err, access) { - callback(err, access === 0); - }); + binding.access(pathModule._makeLong(path), mode, callback); }; fs.accessSync = function(path, mode) { @@ -216,7 +214,7 @@ fs.accessSync = function(path, mode) { else mode = mode | 0; - return binding.access(pathModule._makeLong(path), mode) === 0; + binding.access(pathModule._makeLong(path), mode); }; fs.exists = function(path, callback) { diff --git a/src/node_file.cc b/src/node_file.cc index 9e4de777c8cde5..9317edcbb3dbbd 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -156,6 +156,7 @@ static void After(uv_fs_t *req) { switch (req->fs_type) { // These all have no data to pass. + case UV_FS_ACCESS: case UV_FS_CLOSE: case UV_FS_RENAME: case UV_FS_UNLINK: @@ -174,10 +175,6 @@ 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; @@ -330,7 +327,6 @@ static void Access(const FunctionCallbackInfo& args) { ASYNC_CALL(access, args[2], *path, mode); } else { SYNC_CALL(access, *path, *path, mode); - args.GetReturnValue().Set(SYNC_RESULT); } } diff --git a/test/simple/test-fs-access.js b/test/simple/test-fs-access.js index 50a6ea7b39eb57..3aa4b849f99b3c 100644 --- a/test/simple/test-fs-access.js +++ b/test/simple/test-fs-access.js @@ -47,36 +47,31 @@ assert(typeof fs.R_OK === 'number'); assert(typeof fs.W_OK === 'number'); assert(typeof fs.X_OK === 'number'); -fs.access(__filename, function(err, access) { +fs.access(__filename, function(err) { assert.strictEqual(err, null, 'error should not exist'); - assert.strictEqual(access, true, 'access should be true'); }); -fs.access(__filename, fs.R_OK, function(err, access) { +fs.access(__filename, fs.R_OK, function(err) { assert.strictEqual(err, null, 'error should not exist'); - assert.strictEqual(access, true, 'access should be true'); }); -fs.access(doesNotExist, function(err, access) { +fs.access(doesNotExist, function(err) { assert.notEqual(err, null, 'error should exist'); assert.strictEqual(err.code, 'ENOENT'); assert.strictEqual(err.path, doesNotExist); - assert.strictEqual(access, false, 'access should be false'); }); -fs.access(readOnlyFile, fs.F_OK | fs.R_OK, function(err, access) { +fs.access(readOnlyFile, fs.F_OK | fs.R_OK, function(err) { assert.strictEqual(err, null, 'error should not exist'); - assert.strictEqual(access, true, 'access should be true'); }); -fs.access(readOnlyFile, fs.W_OK, function(err, access) { +fs.access(readOnlyFile, fs.W_OK, function(err) { assert.notEqual(err, null, 'error should exist'); assert.strictEqual(err.path, readOnlyFile); - assert.strictEqual(access, false, 'access should be false'); }); assert.throws(function() { - fs.access(100, fs.F_OK, function(err, access) {}); + fs.access(100, fs.F_OK, function(err) {}); }, /path must be a string/); assert.throws(function() { @@ -84,14 +79,13 @@ assert.throws(function() { }, /callback must be a function/); assert.doesNotThrow(function() { - var access = fs.accessSync(__filename); - assert.strictEqual(access, true, 'access should be true'); + fs.accessSync(__filename); }); assert.doesNotThrow(function() { var mode = fs.F_OK | fs.R_OK | fs.W_OK; - var access = fs.accessSync(__filename, mode); - assert.strictEqual(access, true, 'access should be true'); + + fs.accessSync(__filename, mode); }); assert.throws(function() {