fix fs.readFile with lying size=0 stat results

nodejs · Jun 11, 2012 · 6ce013d · 6ce013d
1 parent d53cdc5
commit 6ce013d
Show file tree

Hide file tree

Showing 2 changed files with 118 additions and 13 deletions.
diff --git a/lib/fs.js b/lib/fs.js
@@ -108,7 +108,8 @@ fs.readFile = function(path, encoding_) {
 
   // first, stat the file, so we know the size.
   var size;
-  var buffer;
+  var buffer; // single buffer with file data
+  var buffers; // list for when size is unknown
   var pos = 0;
   var fd;
 
@@ -120,8 +121,10 @@ fs.readFile = function(path, encoding_) {
       if (er) return callback(er);
       size = st.size;
       if (size === 0) {
-        buffer = new Buffer(0);
-        return afterRead(null, 0);
+        // the kernel lies about many files.
+        // Go ahead and try to read some bytes.
+        buffers = [];
+        return read();
       }
 
       buffer = new Buffer(size);
@@ -130,7 +133,12 @@ fs.readFile = function(path, encoding_) {
   });
 
   function read() {
-    fs.read(fd, buffer, pos, size - pos, pos, afterRead);
+    if (size === 0) {
+      buffer = new Buffer(8192);
+      fs.read(fd, buffer, 0, 8192, pos, afterRead);
+    } else {
+      fs.read(fd, buffer, pos, size - pos, pos, afterRead);
+    }
   }
 
   function afterRead(er, bytesRead) {
@@ -141,12 +149,27 @@ fs.readFile = function(path, encoding_) {
     }
 
     pos += bytesRead;
-    if (pos === size) close();
-    else read();
+    if (size !== 0) {
+      if (pos === size) close();
+      else read();
+    } else {
+      // unknown size, just read until we don't get bytes.
+      if (bytesRead > 0) {
+        buffers.push(buffer.slice(0, bytesRead));
+        read();
+      } else {
+        close();
+      }
+    }
   }
 
   function close() {
     fs.close(fd, function(er) {
+      if (size === 0) {
+        // collected the data into the buffers list.
+        buffer = Buffer.concat(buffer.length, pos);
+      }
+
       if (encoding) buffer = buffer.toString(encoding);
       return callback(er, buffer);
     });
@@ -165,28 +188,52 @@ fs.readFileSync = function(path, encoding) {
     if (threw) fs.closeSync(fd);
   }
 
+  var pos = 0;
+  var buffer; // single buffer with file data
+  var buffers; // list for when size is unknown
+
   if (size === 0) {
-    fs.closeSync(fd);
-    return encoding ? '' : new Buffer(0);
+    buffers = [];
+  } else {
+    buffer = new Buffer(size);
   }
 
-  var buffer = new Buffer(size);
-  var pos = 0;
-
-  while (pos < size) {
+  var done = false;
+  while (!done) {
     var threw = true;
     try {
-      var bytesRead = fs.readSync(fd, buffer, pos, size - pos, pos);
+      if (size !== 0) {
+        var bytesRead = fs.readSync(fd, buffer, pos, size - pos, pos);
+      } else {
+        // the kernel lies about many files.
+        // Go ahead and try to read some bytes.
+        buffer = new Buffer(8192);
+        var bytesRead = fs.readSync(fd, buffer, 0, 8192, pos);
+        if (bytesRead) {
+          buffers.push(buffer.slice(0, bytesRead));
+        }
+      }
       threw = false;
     } finally {
       if (threw) fs.closeSync(fd);
     }
 
     pos += bytesRead;
+
+    if (size !== 0) {
+      done = pos >= size;
+    } else {
+      done = bytesRead > 0;
+    }
   }
 
   fs.closeSync(fd);
 
+  if (size === 0) {
+    // data was collected into the buffers list.
+    buffer = Buffer.concat(buffers, pos);
+  }
+
   if (encoding) buffer = buffer.toString(encoding);
   return buffer;
 };

diff --git a/test/simple/test-fs-readfile-zero-byte-liar.js b/test/simple/test-fs-readfile-zero-byte-liar.js
@@ -0,0 +1,58 @@
+// Copyright Joyent, Inc. and other Node 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 dataExpected = fs.readFileSync(__filename, 'utf8');
+
+// sometimes stat returns size=0, but it's a lie.
+fs._fstat = fs.fstat;
+fs._fstatSync = fs.fstatSync;
+
+fs.fstat = function(fd, cb) {
+  fs._fstat(fd, function(er, st) {
+    if (er) return cb(er);
+    st.size = 0;
+    return cb(er, st);
+  });
+};
+
+fs.fstatSync = function(fd) {
+  var st = fs._fstatSync;
+  st.size = 0;
+  return st;
+};
+
+var d = fs.readFileSync(__filename, 'utf8');
+assert.equal(d, dataExpected);
+
+var called = false;
+fs.readFile(__filename, 'utf8', function (er, d) {
+  assert.equal(d, dataExpected);
+  called = true;
+});
+
+process.on('exit', function() {
+  assert(called);
+  console.log("ok");
+});