Merge pull request #255 from humphd/issue254

Fix #254, with tests

src/filesystem/implementation.js

@@ -1691,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) {
@@ -1701,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) {
+    function cleanup() {
-      if(err2) {
+      fs.releaseDescriptor(fd);
-        return callback(err2);
+      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) {
+      read_data(context, ofd, buffer, 0, size, 0, function(err, nbytes) {
-        if(err3) {
+        cleanup();
-          return callback(err3);
+
+        if(err) {
+          return callback(err);
         }
-        fs.releaseDescriptor(fd);
 
         var data;
         if(options.encoding === 'utf8') {

tests/bugs/issue254.js

@@ -0,0 +1,47 @@
+var Filer = require('../..');
+var util = require('../lib/test-utils.js');
+var expect = require('chai').expect;
+
+describe('EISDIR when trying to open a dir path - issue 254', function() {
+  beforeEach(util.setup);
+  afterEach(util.cleanup);
+
+  it('should fail with EISDIR for root dir', function(done) {
+    var fs = util.fs();
+
+    fs.readFile('/', function(err) {
+      expect(err.code).to.equal('EISDIR');
+      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();
+        });
+      });
+    });
+  });
+});

tests/index.js

@@ -71,3 +71,5 @@ require("./bugs/issue239");
 require("./bugs/issue249");
 require("./bugs/ls-depth-bug");
 require("./bugs/issue247.js");
+require("./bugs/issue254.js");
+