Skip to content
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

Fix Issue 747 #753

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 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
27 changes: 0 additions & 27 deletions src/constants.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
var O_READ = 'READ';
var O_WRITE = 'WRITE';
var O_CREATE = 'CREATE';
var O_EXCLUSIVE = 'EXCLUSIVE';
var O_TRUNCATE = 'TRUNCATE';
var O_APPEND = 'APPEND';
var XATTR_CREATE = 'CREATE';
var XATTR_REPLACE = 'REPLACE';

Expand Down Expand Up @@ -43,27 +37,6 @@ module.exports = {
FS_NOMTIME: 'NOMTIME',
FS_NODUPEIDCHECK: 'FS_NODUPEIDCHECK',

// FS File Open Flags
O_READ: O_READ,
O_WRITE: O_WRITE,
O_CREATE: O_CREATE,
O_EXCLUSIVE: O_EXCLUSIVE,
O_TRUNCATE: O_TRUNCATE,
O_APPEND: O_APPEND,

O_FLAGS: {
'r': [O_READ],
'r+': [O_READ, O_WRITE],
'w': [O_WRITE, O_CREATE, O_TRUNCATE],
'w+': [O_WRITE, O_READ, O_CREATE, O_TRUNCATE],
'wx': [O_WRITE, O_CREATE, O_EXCLUSIVE, O_TRUNCATE],
'wx+': [O_WRITE, O_READ, O_CREATE, O_EXCLUSIVE, O_TRUNCATE],
'a': [O_WRITE, O_CREATE, O_APPEND],
'a+': [O_WRITE, O_READ, O_CREATE, O_APPEND],
'ax': [O_WRITE, O_CREATE, O_EXCLUSIVE, O_APPEND],
'ax+': [O_WRITE, O_READ, O_CREATE, O_EXCLUSIVE, O_APPEND]
},

XATTR_CREATE: XATTR_CREATE,
XATTR_REPLACE: XATTR_REPLACE,

Expand Down
142 changes: 116 additions & 26 deletions src/filesystem/implementation.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,6 @@ var ROOT_DIRECTORY_NAME = Constants.ROOT_DIRECTORY_NAME;
var SUPER_NODE_ID = Constants.SUPER_NODE_ID;
var SYMLOOP_MAX = Constants.SYMLOOP_MAX;

var O_READ = Constants.O_READ;
var O_WRITE = Constants.O_WRITE;
var O_CREATE = Constants.O_CREATE;
var O_EXCLUSIVE = Constants.O_EXCLUSIVE;
var O_APPEND = Constants.O_APPEND;
var O_FLAGS = Constants.O_FLAGS;

var XATTR_CREATE = Constants.XATTR_CREATE;
var XATTR_REPLACE = Constants.XATTR_REPLACE;
var FS_NOMTIME = Constants.FS_NOMTIME;
Expand Down Expand Up @@ -591,9 +584,10 @@ function open_file(context, path, flags, mode, callback) {
var fileData;

var followedCount = 0;
flags = stringToFlags(flags);

if(ROOT_DIRECTORY_NAME === name) {
if(flags.includes(O_WRITE)) {
if(flags & Constants.fsConstants.O_WRONLY) {
callback(new Errors.EISDIR('the named file is a directory and O_WRITE is set', path));
} else {
find_node(context, path, set_file_node);
Expand All @@ -619,18 +613,18 @@ function open_file(context, path, flags, mode, callback) {
} else {
directoryData = result;
if(directoryData.hasOwnProperty(name)) {
if(flags.includes(O_EXCLUSIVE)) {
if(flags & Constants.fsConstants.O_EXCL) {
callback(new Errors.EEXIST('O_CREATE and O_EXCLUSIVE are set, and the named file exists', path));
} else {
directoryEntry = directoryData[name];
if(directoryEntry.type === NODE_TYPE_DIRECTORY && flags.includes(O_WRITE)) {
if(directoryEntry.type === NODE_TYPE_DIRECTORY && flags & Constants.fsConstants.O_WRONLY) {
callback(new Errors.EISDIR('the named file is a directory and O_WRITE is set', path));
} else {
context.getObject(directoryEntry.id, check_if_symbolic_link);
}
}
} else {
if(!flags.includes(O_CREATE)) {
if(!(flags & Constants.fsConstants.O_CREAT)) {
callback(new Errors.ENOENT('O_CREATE is not set and the named file does not exist', path));
} else {
write_file_node();
Expand Down Expand Up @@ -662,7 +656,7 @@ function open_file(context, path, flags, mode, callback) {
parentPath = dirname(data);
name = basename(data);
if(ROOT_DIRECTORY_NAME === name) {
if(flags.includes(O_WRITE)) {
if(flags & Constants.fsConstants.O_WRONLY) {
callback(new Errors.EISDIR('the named file is a directory and O_WRITE is set', path));
} else {
find_node(context, path, set_file_node);
Expand Down Expand Up @@ -1618,7 +1612,43 @@ function fremovexattr_file (context, ofd, name, callback) {
}

function validate_flags(flags) {
return O_FLAGS.hasOwnProperty(flags) ? O_FLAGS[flags] : null;
flags = stringToFlags(flags);

if(flags === stringToFlags('r')) {
return flags;
}
else if(flags & stringToFlags('r+')) {
return flags;
}
else if(flags & stringToFlags('w')) {
return flags;
}
else if(flags & stringToFlags('w+')) {
return flags;
}
else if(flags & stringToFlags('wx')) {
return flags;
}
else if(flags & stringToFlags('wx+')) {
return flags;
}
else if(flags & stringToFlags('a')) {
return flags;
}
else if(flags & stringToFlags('a+')) {
return flags;
}
else if(flags & stringToFlags('ax')) {
return flags;
}
else if(flags & stringToFlags('ax+')) {
return flags;
}

return null;

CDNRae marked this conversation as resolved.
Show resolved Hide resolved

//return Constants.fsConstants.hasOwnProperty(flags) ? Constants.fsConstants[flags] : null;
}

function validate_file_options(options, enc, fileMode){
Expand All @@ -1633,6 +1663,7 @@ function validate_file_options(options, enc, fileMode){
}

function open(context, path, flags, mode, callback) {
flags = stringToFlags(flags);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move this down to be with the other flags validation happening on 1691

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up moving that line, along with the validation, up a little further in the function -- flags gets used prior to line 1691, but until this most recent change, wasn't actually validated until that line. I figured it would make more sense to validate them almost immediately, or at least do that before they're used. Thank you for pointing this out, though -- I don't think I would have caught that mistake if it weren't for this suggestion!

if (arguments.length < 5 ){
callback = arguments[arguments.length - 1];
mode = 0o644;
Expand All @@ -1646,7 +1677,7 @@ function open(context, path, flags, mode, callback) {
callback(error);
} else {
var position;
if(flags.includes(O_APPEND)) {
if(flags & Constants.fsConstants.O_APPEND) {
position = fileNode.size;
} else {
position = 0;
Expand All @@ -1658,7 +1689,7 @@ function open(context, path, flags, mode, callback) {
}

flags = validate_flags(flags);
if(!flags) {
if(!flags && flags !== 0) {
return callback(new Errors.EINVAL('flags is not valid'), path);
}

Expand Down Expand Up @@ -1769,9 +1800,14 @@ function read(context, fd, buffer, offset, length, position, callback) {
callback = arguments[arguments.length - 1];

var ofd = openFiles.getOpenFileDescription(fd);

if(ofd) {
ofd.flags = stringToFlags(ofd.flags);
}

if(!ofd) {
callback(new Errors.EBADF());
} else if(!ofd.flags.includes(O_READ)) {
} else if(!ofd.flags & Constants.fsConstants.O_RDONLY) {
callback(new Errors.EBADF('descriptor does not permit reading'));
} else {
read_data(context, ofd, buffer, offset, length, position, wrapped_cb);
Expand All @@ -1793,7 +1829,7 @@ function readFile(context, path, options, callback) {
options = validate_file_options(options, null, 'r');

var flags = validate_flags(options.flag || 'r');
if(!flags) {
if(!flags && flags !== 0) {
return callback(new Errors.EINVAL('flags is not valid', path));
}

Expand Down Expand Up @@ -1849,9 +1885,14 @@ function write(context, fd, buffer, offset, length, position, callback) {
length = (undefined === length) ? buffer.length - offset : length;

var ofd = openFiles.getOpenFileDescription(fd);

if(ofd) {
ofd.flags = stringToFlags(ofd.flags);
}

if(!ofd) {
callback(new Errors.EBADF());
} else if(!ofd.flags.includes(O_WRITE)) {
} else if(!ofd.flags & Constants.fsConstants.O_WRONLY) {
callback(new Errors.EBADF('descriptor does not permit writing'));
} else if(buffer.length - offset < length) {
callback(new Errors.EIO('input buffer is too small'));
Expand All @@ -1865,7 +1906,7 @@ function writeFile(context, path, data, options, callback) {
options = validate_file_options(options, 'utf8', 'w');

var flags = validate_flags(options.flag || 'w');
if(!flags) {
if(!flags && flags !== 0) {
return callback(new Errors.EINVAL('flags is not valid', path));
}

Expand Down Expand Up @@ -1900,7 +1941,7 @@ function appendFile(context, path, data, options, callback) {
options = validate_file_options(options, 'utf8', 'a');

var flags = validate_flags(options.flag || 'a');
if(!flags) {
if(!flags && flags !== 0) {
return callback(new Errors.EINVAL('flags is not valid', path));
}

Expand Down Expand Up @@ -2090,10 +2131,15 @@ function fsetxattr(context, fd, name, value, flag, callback) {
}

var ofd = openFiles.getOpenFileDescription(fd);

if(ofd) {
ofd.flags = stringToFlags(ofd.flags);
}

if (!ofd) {
callback(new Errors.EBADF());
}
else if (!ofd.flags.includes(O_WRITE)) {
else if (!ofd.flags & Constants.fsConstants.O_WRONLY) {
callback(new Errors.EBADF('descriptor does not permit writing'));
}
else {
Expand All @@ -2107,10 +2153,15 @@ function removexattr(context, path, name, callback) {

function fremovexattr(context, fd, name, callback) {
var ofd = openFiles.getOpenFileDescription(fd);

if(ofd) {
ofd.flags = stringToFlags(ofd.flags);
}

if (!ofd) {
callback(new Errors.EBADF());
}
else if (!ofd.flags.includes(O_WRITE)) {
else if (!ofd.flags & Constants.fsConstants.O_WRONLY) {
callback(new Errors.EBADF('descriptor does not permit writing'));
}
else {
Expand Down Expand Up @@ -2185,9 +2236,14 @@ function futimes(context, fd, atime, mtime, callback) {
mtime = (mtime) ? toUnixTimestamp(mtime) : toUnixTimestamp(currentTime);

var ofd = openFiles.getOpenFileDescription(fd);

if(ofd) {
ofd.flags = stringToFlags(ofd.flags);
}

if(!ofd) {
callback(new Errors.EBADF());
} else if(!ofd.flags.includes(O_WRITE)) {
} else if(!ofd.flags & Constants.fsConstants.O_WRONLY) {
callback(new Errors.EBADF('descriptor does not permit writing'));
} else {
futimes_file(context, ofd, atime, mtime, callback);
Expand All @@ -2206,9 +2262,14 @@ function fchmod(context, fd, mode, callback) {
if(!mode) return;

var ofd = openFiles.getOpenFileDescription(fd);

if(ofd) {
ofd.flags = stringToFlags(ofd.flags);
}

if(!ofd) {
callback(new Errors.EBADF());
} else if(!ofd.flags.includes(O_WRITE)) {
} else if(!ofd.flags & Constants.fsConstants.O_WRONLY) {
callback(new Errors.EBADF('descriptor does not permit writing'));
} else {
fchmod_file(context, ofd, mode, callback);
Expand All @@ -2235,9 +2296,14 @@ function fchown(context, fd, uid, gid, callback) {
}

var ofd = openFiles.getOpenFileDescription(fd);

if(ofd) {
ofd.flags = stringToFlags(ofd.flags);
}

if(!ofd) {
callback(new Errors.EBADF());
} else if(!ofd.flags.includes(O_WRITE)) {
} else if(!ofd.flags & Constants.fsConstants.O_WRONLY) {
callback(new Errors.EBADF('descriptor does not permit writing'));
} else {
fchown_file(context, ofd, uid, gid, callback);
Expand Down Expand Up @@ -2395,16 +2461,40 @@ function ftruncate(context, fd, length, callback) {
length = length || 0;

var ofd = openFiles.getOpenFileDescription(fd);

if(ofd) {
ofd.flags = stringToFlags(ofd.flags);
}

if(!ofd) {
callback(new Errors.EBADF());
} else if(!ofd.flags.includes(O_WRITE)) {
} else if(!ofd.flags & Constants.fsConstants.O_WRONLY) {
callback(new Errors.EBADF('descriptor does not permit writing'));
} else {
if(validateInteger(length, callback) !== length) return;
ftruncate_file(context, ofd, length, callback);
}
}

function stringToFlags(flags) {
if(typeof flags === 'number') {
return flags;
}
switch(flags) {
case 'r': return Constants.fsConstants.O_RDONLY;
case 'r+': return Constants.fsConstants.O_RDONLY | Constants.fsConstants.O_WRONLY;
case 'w': return Constants.fsConstants.O_WRONLY | Constants.fsConstants.O_CREAT | Constants.fsConstants.O_TRUNC;
case 'w+': return Constants.fsConstants.O_WRONLY | Constants.fsConstants.O_RDONLY | Constants.fsConstants.O_CREAT | Constants.fsConstants.O_TRUNC;
case 'wx': return Constants.fsConstants.O_WRONLY | Constants.fsConstants.O_CREAT | Constants.fsConstants.O_EXCL | Constants.fsConstants.O_TRUNC;
case 'wx+': return Constants.fsConstants.O_WRONLY | Constants.fsConstants.O_RDONLY | Constants.fsConstants.O_CREAT | Constants.fsConstants.O_EXCL | Constants.fsConstants.O_TRUNC;
case 'a': return Constants.fsConstants.O_WRONLY | Constants.fsConstants.O_CREAT | Constants.fsConstants.O_APPEND;
case 'a+': return Constants.fsConstants.O_WRONLY | Constants.fsConstants.O_RDONLY | Constants.fsConstants.O_CREAT | Constants.fsConstants.O_APPEND;
case 'ax': return Constants.fsConstants.O_WRONLY | Constants.fsConstants.O_CREAT | Constants.fsConstants.O_EXCL | Constants.fsConstants.O_APPEND;
case 'ax+': return Constants.fsConstants.O_WRONLY | Constants.fsConstants.O_RDONLY | Constants.fsConstants.O_CREAT | Constants.fsConstants.O_EXCL | Constants.fsConstants.O_APPEND;
default: return null;
}
}

module.exports = {
appendFile,
access,
Expand Down
12 changes: 12 additions & 0 deletions tests/spec/fs.open.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
var util = require('../lib/test-utils.js');
var expect = require('chai').expect;
var { FIRST_DESCRIPTOR } = require('../../src/constants.js');
var Constants = require('../../src/constants.js');

describe('fs.open', function() {
beforeEach(util.setup);
Expand Down Expand Up @@ -33,6 +34,17 @@ describe('fs.open', function() {
});
});

it('should create a file when passed the flag as a bitwise number', function(done) {
var fs = util.fs();

fs.open('/myfile', Constants.fsConstants.O_CREAT, function(error, fd) {
expect(error).not.to.exist;
expect(fd).to.exist;
fs.close(fd);
done();
});
});

it('should return an error when flagged for write and the path is a directory', function(done) {
var fs = util.fs();

Expand Down