From 76118694bf30719e1c44be9321d0b6ac669d3ce7 Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Sun, 17 Aug 2014 15:12:08 -0400 Subject: [PATCH 1/5] Failing test --- tests/bugs/issue254.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 tests/bugs/issue254.js diff --git a/tests/bugs/issue254.js b/tests/bugs/issue254.js new file mode 100644 index 0000000..0d595fb --- /dev/null +++ b/tests/bugs/issue254.js @@ -0,0 +1,17 @@ +var Filer = require('../..'); +var util = require('../lib/test-utils.js'); +var expect = require('chai').expect; + +describe('fs.readFile on a dir path', function() { + beforeEach(util.setup); + afterEach(util.cleanup); + + it('should fail with EISDIR', function(done) { + var fs = util.fs(); + + fs.readFile('/', function(err) { + expect(err.code).to.equal('EISDIR'); + done(); + }); + }); +}); From c916c0a40777cec5c5d678e3fceba56f5718572e Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Sun, 17 Aug 2014 15:12:44 -0400 Subject: [PATCH 2/5] Update tests/index.js to run regression --- tests/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/index.js b/tests/index.js index 5d9632a..d797526 100644 --- a/tests/index.js +++ b/tests/index.js @@ -71,3 +71,4 @@ require("./bugs/issue239"); require("./bugs/issue249"); require("./bugs/ls-depth-bug"); require("./bugs/issue247.js"); +require("./bugs/readFile-dir"); From b3da2f76816b52beb169759636458e054a458ac6 Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Sun, 17 Aug 2014 15:21:16 -0400 Subject: [PATCH 3/5] Fix file name --- tests/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/index.js b/tests/index.js index d797526..c0684ff 100644 --- a/tests/index.js +++ b/tests/index.js @@ -71,4 +71,5 @@ require("./bugs/issue239"); require("./bugs/issue249"); require("./bugs/ls-depth-bug"); require("./bugs/issue247.js"); -require("./bugs/readFile-dir"); +require("./bugs/issue254.js"); + From 1a2774b152d78dc999ec45ce61bf6cf0c2f04bdf Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Sun, 17 Aug 2014 17:15:00 -0400 Subject: [PATCH 4/5] ack's fix + more tests for various cases --- src/filesystem/implementation.js | 10 +++------- tests/bugs/issue254.js | 34 ++++++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/filesystem/implementation.js b/src/filesystem/implementation.js index f80acf3..35dd1c4 100644 --- a/src/filesystem/implementation.js +++ b/src/filesystem/implementation.js @@ -573,11 +573,7 @@ function open_file(context, path, flags, callback) { var followedCount = 0; if(ROOT_DIRECTORY_NAME == name) { - if(_(flags).contains(O_WRITE)) { - callback(new Errors.EISDIR('the named file is a directory and O_WRITE is set')); - } else { - find_node(context, path, set_file_node); - } + callback(new Errors.EISDIR('the named file is the root directory')); } else { find_node(context, parentPath, read_directory_data); } @@ -603,8 +599,8 @@ function open_file(context, path, flags, callback) { callback(new Errors.ENOENT('O_CREATE and O_EXCLUSIVE are set, and the named file exists')); } else { directoryEntry = directoryData[name]; - if(directoryEntry.type == MODE_DIRECTORY && _(flags).contains(O_WRITE)) { - callback(new Errors.EISDIR('the named file is a directory and O_WRITE is set')); + if(directoryEntry.type == MODE_DIRECTORY) { + callback(new Errors.EISDIR('the named file is a directory')); } else { context.getObject(directoryEntry.id, check_if_symbolic_link); } diff --git a/tests/bugs/issue254.js b/tests/bugs/issue254.js index 0d595fb..95f4a5f 100644 --- a/tests/bugs/issue254.js +++ b/tests/bugs/issue254.js @@ -2,11 +2,11 @@ var Filer = require('../..'); var util = require('../lib/test-utils.js'); var expect = require('chai').expect; -describe('fs.readFile on a dir path', function() { +describe('EISDIR when trying to open a dir path - issue 254', function() { beforeEach(util.setup); afterEach(util.cleanup); - it('should fail with EISDIR', function(done) { + it('should fail with EISDIR for root dir', function(done) { var fs = util.fs(); fs.readFile('/', function(err) { @@ -14,4 +14,34 @@ describe('fs.readFile on a dir path', function() { done(); }); }); + + it('should fail with EISDIR for regular dir', function(done) { + var fs = util.fs(); + + fs.mkdir('/dir', function(err) { + if(err) throw err; + + fs.readFile('/dir', function(err) { + expect(err.code).to.equal('EISDIR'); + done(); + }); + }); + }); + + it('should fail with EISDIR for symlinked dir', function(done) { + var fs = util.fs(); + + fs.mkdir('/dir', function(err) { + if(err) throw err; + + fs.symlink('/dir', '/link', function(err) { + if(err) throw err; + + fs.readFile('/link', function(err) { + expect(err.code).to.equal('EISDIR'); + done(); + }); + }); + }); + }); }); From 2590a886ace60de706db1fda3380afefbc6c2c5c Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Mon, 18 Aug 2014 11:03:46 -0400 Subject: [PATCH 5/5] Fix in readFile only, stop leaking descriptors in readFile --- src/filesystem/implementation.js | 39 +++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/filesystem/implementation.js b/src/filesystem/implementation.js index 35dd1c4..e050076 100644 --- a/src/filesystem/implementation.js +++ b/src/filesystem/implementation.js @@ -573,7 +573,11 @@ function open_file(context, path, flags, callback) { var followedCount = 0; if(ROOT_DIRECTORY_NAME == name) { - callback(new Errors.EISDIR('the named file is the root directory')); + if(_(flags).contains(O_WRITE)) { + callback(new Errors.EISDIR('the named file is a directory and O_WRITE is set')); + } else { + find_node(context, path, set_file_node); + } } else { find_node(context, parentPath, read_directory_data); } @@ -599,8 +603,8 @@ function open_file(context, path, flags, callback) { callback(new Errors.ENOENT('O_CREATE and O_EXCLUSIVE are set, and the named file exists')); } else { directoryEntry = directoryData[name]; - if(directoryEntry.type == MODE_DIRECTORY) { - callback(new Errors.EISDIR('the named file is a directory')); + if(directoryEntry.type == MODE_DIRECTORY && _(flags).contains(O_WRITE)) { + callback(new Errors.EISDIR('the named file is a directory and O_WRITE is set')); } else { context.getObject(directoryEntry.id, check_if_symbolic_link); } @@ -1687,7 +1691,7 @@ function readFile(fs, context, path, options, callback) { var flags = validate_flags(options.flag || 'r'); if(!flags) { - callback(new Errors.EINVAL('flags is not valid')); + return callback(new Errors.EINVAL('flags is not valid')); } open_file(context, path, flags, function(err, fileNode) { @@ -1697,21 +1701,34 @@ function readFile(fs, context, path, options, callback) { var ofd = new OpenFileDescription(path, fileNode.id, flags, 0); var fd = fs.allocDescriptor(ofd); - fstat_file(context, ofd, function(err2, fstatResult) { - if(err2) { - return callback(err2); + function cleanup() { + fs.releaseDescriptor(fd); + ofd = null; + } + + fstat_file(context, ofd, function(err, fstatResult) { + if(err) { + cleanup(); + return callback(err); } var stats = new Stats(fstatResult, fs.name); + + if(stats.isDirectory()) { + cleanup(); + return callback(new Errors.EISDIR('illegal operation on directory')); + } + var size = stats.size; var buffer = new Buffer(size); buffer.fill(0); - read_data(context, ofd, buffer, 0, size, 0, function(err3, nbytes) { - if(err3) { - return callback(err3); + read_data(context, ofd, buffer, 0, size, 0, function(err, nbytes) { + cleanup(); + + if(err) { + return callback(err); } - fs.releaseDescriptor(fd); var data; if(options.encoding === 'utf8') {