From 9d1a76756dc3399d7a4a0dff1baabdcc42a8340b Mon Sep 17 00:00:00 2001 From: Andrew Koung Date: Sun, 7 Apr 2019 00:26:19 -0400 Subject: [PATCH 1/4] implementing lchown() --- README.md | 14 +++++++++++ src/filesystem/implementation.js | 34 +++++++++++++++++++++++++- src/filesystem/interface.js | 2 +- tests/spec/fs.chown.spec.js | 42 +++++++++++++++++++++++++++++++- 4 files changed, 89 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 61e72bc1..5ad3aaeb 100644 --- a/README.md +++ b/README.md @@ -372,6 +372,7 @@ const fs = new Filer.FileSystem(options, callback); * [fs.utimes(path, atime, mtime, callback)](#utimes) * [fs.chown(path, uid, gid, callback)](#chown) * [fs.fchown(fd, uid, gid, callback)](#fchown) +* [fs.lchown(path, uid, gid, callback)](#lchown) * [fs.chmod(path, mode, callback)](#chmod) * [fs.fchmod(fd, mode, callback)](#fchmod) * [fs.futimes(fd, atime, mtime, callback)](#fsutimes) @@ -923,6 +924,19 @@ fs.open('/myfile.txt', function(err, fd) { }); ``` +#### fs.lchown(path, uid, gid, callback) + +lchown() is like chown(), but does not dereference symbolic links. Asynchronous [lchown(2)](http://pubs.opengroup.org/onlinepubs/009695399/functions/chown.html). Callback gets no additional arguments. Both `uid` (user id) and `gid` (group id) arguments should be a JavaScript Number. By default, `0x0` is used (i.e., `root:root` ownership). + +Example: + +```javascript +fs.lchown('/myfile.txt', 500, 500, function(err) { + if(err) throw err; + // /myfile.txt is now owned by user with id 500, group 500 +}); +``` + #### fs.chmod(path, mode, callback) Changes the mode of a file. Asynchronous [chmod(2)](http://pubs.opengroup.org/onlinepubs/009695399/functions/chmod.html). Callback gets no additional arguments. The `mode` argument should be a JavaScript Number, which combines file type and permission information. Here are a list of common values useful for setting the `mode`: diff --git a/src/filesystem/implementation.js b/src/filesystem/implementation.js index b5a8f8bf..e9a6a9b8 100644 --- a/src/filesystem/implementation.js +++ b/src/filesystem/implementation.js @@ -278,6 +278,14 @@ function find_node(context, path, callback) { } } +/** + * find_lnode + */ +// in: file or directory path +// out: node structure, or error +function find_symbolic_node(context, path, callback) { + lstat_file(context, path, callback); +} /** * set extended attribute (refactor) @@ -2060,6 +2068,19 @@ function fchown_file(context, ofd, uid, gid, callback) { ofd.getNode(context, update_owner); } +function lchown_file(context, path, uid, gid, callback) { + function update_owner(error, node) { + if (error) { + callback(error); + } else { + node.uid = uid; + node.gid = gid; + update_node_times(context, path, node, { mtime: Date.now() }, callback); + } + } + find_symbolic_node(context, path, update_owner); +} + function getxattr(context, path, name, callback) { getxattr_file(context, path, name, callback); } @@ -2244,6 +2265,17 @@ function fchown(context, fd, uid, gid, callback) { } } +function lchown(context, path, uid, gid, callback) { + if(!isUint32(uid)) { + return callback(new Errors.EINVAL('uid must be a valid integer', uid)); + } + if(!isUint32(gid)) { + return callback(new Errors.EINVAL('gid must be a valid integer', gid)); + } + + lchown_file(context, path, uid, gid, callback); +} + function rename(context, oldpath, newpath, callback) { oldpath = normalize(oldpath); newpath = normalize(newpath); @@ -2425,7 +2457,7 @@ module.exports = { ftruncate, futimes, getxattr, - // lchown - https://github.com/filerjs/filer/issues/620 + lchown, // lchmod - https://github.com/filerjs/filer/issues/619 link, lseek, diff --git a/src/filesystem/interface.js b/src/filesystem/interface.js index 824128b3..d6e93905 100644 --- a/src/filesystem/interface.js +++ b/src/filesystem/interface.js @@ -340,7 +340,7 @@ function FileSystem(options, callback) { { name: 'ftruncate' }, { name: 'futimes' }, { name: 'getxattr', promises: true, absPathArgs: [0] }, - // lchown - https://github.com/filerjs/filer/issues/620 + { name: 'lchown' }, // lchmod - https://github.com/filerjs/filer/issues/619 { name: 'link', promises: true, absPathArgs: [0, 1] }, { name: 'lseek' }, diff --git a/tests/spec/fs.chown.spec.js b/tests/spec/fs.chown.spec.js index e1413211..575f779e 100644 --- a/tests/spec/fs.chown.spec.js +++ b/tests/spec/fs.chown.spec.js @@ -1,7 +1,7 @@ var util = require('../lib/test-utils.js'); var expect = require('chai').expect; -describe('fs.chown, fs.fchown', function() { +describe('fs.chown, fs.fchown, fs.lchown', function() { beforeEach(util.setup); afterEach(util.cleanup); @@ -27,6 +27,20 @@ describe('fs.chown, fs.fchown', function() { }); }); + it('lchown should expect an interger value for uid', function(done) { + var fs = util.fs(); + + fs.open('/file', 'w', function(err, fd) { + if(err) throw err; + + fs.lchown(fd, '1001', 1001, function(err) { + expect(err).to.exist; + expect(err.code).to.equal('EINVAL'); + fs.close(fd, done); + }); + }); + }); + it('fchown should expect an interger value for uid', function(done) { var fs = util.fs(); @@ -55,6 +69,20 @@ describe('fs.chown, fs.fchown', function() { }); }); + it('lchown should expect an interger value for gid', function(done) { + var fs = util.fs(); + + fs.open('/file', 'w', function(err, fd) { + if(err) throw err; + + fs.lchown(fd, 1001, '1001', function(err) { + expect(err).to.exist; + expect(err.code).to.equal('EINVAL'); + fs.close(fd, done); + }); + }); + }); + it('fchown should expect an interger value for gid', function(done) { var fs = util.fs(); @@ -107,6 +135,18 @@ describe('fs.chown, fs.fchown', function() { fs.stat('/file', function(err, stats) { if(err) throw err; + expect(stats.uid).to.equal(500); + expect(stats.gid).to.equal(500); + //done(); + }); + }); + + fs.lchown('/file', 500, 500, function(err) { + if(err) throw err; + + fs.lstat('/file', function(err, stats) { + if(err) throw err; + expect(stats.uid).to.equal(500); expect(stats.gid).to.equal(500); done(); From b5914f78af690712a500a499db7cdda27b24f0fb Mon Sep 17 00:00:00 2001 From: Andrew Koung Date: Sun, 7 Apr 2019 01:45:40 -0400 Subject: [PATCH 2/4] made request review changes --- src/filesystem/implementation.js | 50 ++++++++++++++++++++++++++++---- src/filesystem/interface.js | 1 - 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/filesystem/implementation.js b/src/filesystem/implementation.js index e9a6a9b8..dbb371e0 100644 --- a/src/filesystem/implementation.js +++ b/src/filesystem/implementation.js @@ -279,12 +279,52 @@ function find_node(context, path, callback) { } /** - * find_lnode + * find_symlink_node */ // in: file or directory path -// out: node structure, or error -function find_symbolic_node(context, path, callback) { - lstat_file(context, path, callback); +// out: symlink node structure, or error +function find_symlink_node(context, path, callback) { + path = normalize(path); + var name = basename(path); + var parentPath = dirname(path); + + var directoryNode; + var directoryData; + + if(ROOT_DIRECTORY_NAME === name) { + find_node(context, path, callback); + } else { + find_node(context, parentPath, read_directory_data); + } + + function read_directory_data(error, result) { + if(error) { + callback(error); + } else { + directoryNode = result; + context.getObject(directoryNode.data, check_if_file_exists); + } + } + + function create_node(error, data) { + if(error) { + return callback(error); + } + Node.create(data, callback); + } + + function check_if_file_exists(error, result) { + if(error) { + callback(error); + } else { + directoryData = result; + if(!directoryData.hasOwnProperty(name)) { + callback(new Errors.ENOENT('a component of the path does not name an existing file', path)); + } else { + context.getObject(directoryData[name].id, create_node); + } + } + } } /** @@ -2078,7 +2118,7 @@ function lchown_file(context, path, uid, gid, callback) { update_node_times(context, path, node, { mtime: Date.now() }, callback); } } - find_symbolic_node(context, path, update_owner); + find_symlink_node(context, path, update_owner); } function getxattr(context, path, name, callback) { diff --git a/src/filesystem/interface.js b/src/filesystem/interface.js index d6e93905..93e491bb 100644 --- a/src/filesystem/interface.js +++ b/src/filesystem/interface.js @@ -341,7 +341,6 @@ function FileSystem(options, callback) { { name: 'futimes' }, { name: 'getxattr', promises: true, absPathArgs: [0] }, { name: 'lchown' }, - // lchmod - https://github.com/filerjs/filer/issues/619 { name: 'link', promises: true, absPathArgs: [0, 1] }, { name: 'lseek' }, { name: 'lstat', promises: true }, From fa0cd27cd2752b478cec5da26bdb1565877fdced Mon Sep 17 00:00:00 2001 From: Andrew Koung Date: Tue, 9 Apr 2019 20:51:40 -0400 Subject: [PATCH 3/4] lstat_file now calls find_symlink_node --- src/filesystem/implementation.js | 42 +------------------------------- 1 file changed, 1 insertion(+), 41 deletions(-) diff --git a/src/filesystem/implementation.js b/src/filesystem/implementation.js index dbb371e0..28442650 100644 --- a/src/filesystem/implementation.js +++ b/src/filesystem/implementation.js @@ -943,47 +943,7 @@ function fstat_file(context, ofd, callback) { } function lstat_file(context, path, callback) { - path = normalize(path); - var name = basename(path); - var parentPath = dirname(path); - - var directoryNode; - var directoryData; - - if(ROOT_DIRECTORY_NAME === name) { - find_node(context, path, callback); - } else { - find_node(context, parentPath, read_directory_data); - } - - function read_directory_data(error, result) { - if(error) { - callback(error); - } else { - directoryNode = result; - context.getObject(directoryNode.data, check_if_file_exists); - } - } - - function create_node(error, data) { - if(error) { - return callback(error); - } - Node.create(data, callback); - } - - function check_if_file_exists(error, result) { - if(error) { - callback(error); - } else { - directoryData = result; - if(!directoryData.hasOwnProperty(name)) { - callback(new Errors.ENOENT('a component of the path does not name an existing file', path)); - } else { - context.getObject(directoryData[name].id, create_node); - } - } - } + find_symlink_node(context, path, callback); } function link_node(context, oldpath, newpath, callback) { From 34434b05047ac4004979345521091261d688c35e Mon Sep 17 00:00:00 2001 From: Andrew Koung Date: Tue, 16 Apr 2019 23:31:26 -0400 Subject: [PATCH 4/4] made review changes for creating lstat test and property add on for interface.js --- src/filesystem/interface.js | 2 +- tests/spec/fs.chown.spec.js | 36 ++++++++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/filesystem/interface.js b/src/filesystem/interface.js index 93e491bb..67f5016d 100644 --- a/src/filesystem/interface.js +++ b/src/filesystem/interface.js @@ -340,7 +340,7 @@ function FileSystem(options, callback) { { name: 'ftruncate' }, { name: 'futimes' }, { name: 'getxattr', promises: true, absPathArgs: [0] }, - { name: 'lchown' }, + { name: 'lchown', promises:true }, { name: 'link', promises: true, absPathArgs: [0, 1] }, { name: 'lseek' }, { name: 'lstat', promises: true }, diff --git a/tests/spec/fs.chown.spec.js b/tests/spec/fs.chown.spec.js index 575f779e..d9bfc73c 100644 --- a/tests/spec/fs.chown.spec.js +++ b/tests/spec/fs.chown.spec.js @@ -137,20 +137,36 @@ describe('fs.chown, fs.fchown, fs.lchown', function() { expect(stats.uid).to.equal(500); expect(stats.gid).to.equal(500); - //done(); + done(); }); }); + }); + }); + }); + }); + }); - fs.lchown('/file', 500, 500, function(err) { - if(err) throw err; - - fs.lstat('/file', function(err, stats) { - if(err) throw err; + it('should allow updating gid and uid for a symlink file', function(done) { + var fs = util.fs(); + + fs.open('/file', 'w', function(err, fd) { + if(err) throw err; + + fs.symlink('/file', '/link', function(err) { + if(err) throw err; - expect(stats.uid).to.equal(500); - expect(stats.gid).to.equal(500); - done(); - }); + fs.lchown('/link', 600, 600, function(err) { + if(err) throw err; + + fs.lstat('/link', function(err, stats) { + if(err) throw err; + + expect(stats.uid).to.equal(600); + expect(stats.gid).to.equal(600); + + fs.close(fd, function(err) { + if(err) throw err; + done(); }); }); });