fs: ignore fstat errors on user fds in fs.readFile{Sync}

On Windows fstat cannot be called on std fds. In those cases,
we already know that the fd is not for a regular file and can
just use a list of buffers to store the data.

Also makes sure that we don't close fds provided by users
if we cannot call fstat on them.

lib/fs.js

@@ -242,7 +242,7 @@ function readFileAfterOpen(err, fd) {
   }
 
   context.fd = fd;
-
+  // Call fstat to attempt to get the size of the file
   const req = new FSReqWrap();
   req.oncomplete = readFileAfterStat;
   req.context = context;
@@ -252,19 +252,34 @@ function readFileAfterOpen(err, fd) {
 function readFileAfterStat(err, stats) {
   const context = this.context;
 
-  if (err)
-    return context.close(err);
-
-  const size = context.size = isFileType(stats, S_IFREG) ? stats[8] : 0;
+  if (!err) {
+    const size = isFileType(stats, S_IFREG) ? stats[8] : 0;
+    return readFileAfterSize(context, size);
+  }
+  if (context.isUserFd) {
+    // If it's a user-provided fd, it might still be readable even if
+    // we cannot call fstat on it e.g. on Windows, calling fstat on std
+    // fds that are not redirected returns EISDIR but they can still be read,
+    // so we just need to avoid treating them as regular files.
+    // If the fd ends up being actually unreadable, an error will appear
+    // later during `read()`
+    return readFileAfterSize(context, 0);
+  }
+  // If it's our own fd, close it and throw an error since
+  // we are not able to read it anyway
+  context.close(err);
+}
 
+function readFileAfterSize(context, size) {
+  context.size = size;
   if (size === 0) {
     context.buffers = [];
     context.read();
     return;
   }
 
   if (size > kMaxLength) {
-    err = new ERR_FS_FILE_TOO_LARGE(size);
+    const err = new ERR_FS_FILE_TOO_LARGE(size);
     return context.close(err);
   }
 
@@ -303,16 +318,6 @@ function readFile(path, options, callback) {
                req);
 }
 
-function tryStatSync(fd, isUserFd) {
-  const ctx = {};
-  const stats = binding.fstat(fd, false, undefined, ctx);
-  if (ctx.errno !== undefined && !isUserFd) {
-    fs.closeSync(fd);
-    throw errors.uvException(ctx);
-  }
-  return stats;
-}
-
 function tryCreateBuffer(size, fd, isUserFd) {
   let threw = true;
   let buffer;
@@ -340,13 +345,35 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {
   return bytesRead;
 }
 
+function getFdSizeSync(fd, isUserFd) {
+  // Call fstat to attempt to get the size of the file
+  const ctx = {};
+  const stats = binding.fstat(fd, false, undefined, ctx);
+  if (ctx.errno === undefined) {
+    return isFileType(stats, S_IFREG) ? stats[8] : 0;
+  }
+  if (isUserFd) {
+    // If it's a user-provided fd, it might still be readable even if
+    // we cannot call fstat on it e.g. on Windows, calling fstat on std
+    // fds that are not redirected returns EISDIR but they can still be read,
+    // so we just need to avoid treating them as regular files.
+    // If the fd ends up being actually unreadable, an error will appear
+    // later during `read()`
+    return 0;
+  }
+  // If it's our own fd, close it and throw an error since
+  // we are not able to read it anyway
+  fs.closeSync(fd);
+  throw errors.uvException(ctx);
+}
+
 function readFileSync(path, options) {
   options = getOptions(options, { flag: 'r' });
   const isUserFd = isFd(path); // file descriptor ownership
   const fd = isUserFd ? path : fs.openSync(path, options.flag || 'r', 0o666);
 
-  const stats = tryStatSync(fd, isUserFd);
-  const size = isFileType(stats, S_IFREG) ? stats[8] : 0;
+  const size = getFdSizeSync(fd, isUserFd);
+
   let pos = 0;
   let buffer; // single buffer with file data
   let buffers; // list for when size is unknown
@@ -365,11 +392,15 @@ function readFileSync(path, options) {
       pos += bytesRead;
     } while (bytesRead !== 0 && pos < size);
   } else {
+    // This is a redefinition of kReadFileBufferLength in read_file_context.js
+    // because it does not make sense to load that module just for this constant
+    const kReadFileBufferLength = 8 * 1024;
+
     do {
       // the kernel lies about many files.
       // Go ahead and try to read some bytes.
-      buffer = Buffer.allocUnsafe(8192);
-      bytesRead = tryReadSync(fd, isUserFd, buffer, 0, 8192);
+      buffer = Buffer.allocUnsafe(kReadFileBufferLength);
+      bytesRead = tryReadSync(fd, isUserFd, buffer, 0, kReadFileBufferLength);
       if (bytesRead !== 0) {
         buffers.push(buffer.slice(0, bytesRead));
       }

test/fixtures/readfile-stdin-async.js

@@ -0,0 +1,6 @@
+'use strict';
+
+const fs = require('fs');
+fs.readFile(0, (err, data) => {
+  console.log(data.toString());
+});

test/fixtures/readfile-stdin-sync.js

@@ -0,0 +1,5 @@
+'use strict';
+
+const fs = require('fs');
+const stdin = fs.readFileSync(0).toString();
+console.log(stdin);

test/parallel/test-fs-readfile-stdin.js

@@ -0,0 +1,42 @@
+'use strict';
+
+// This tests that fs.readFile{Sync} on closed stdin works.
+
+const { spawn } = require('child_process');
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+const assert = require('assert');
+
+// fs.readFileSync(0)
+{
+  const script = fixtures.path('readfile-stdin-sync.js');
+  const child = spawn(process.execPath, [script], {
+    stdio: ['ignore', 'pipe', 'pipe']
+  });
+  child.stdout.on('data', common.mustCall((chunk) => {
+    assert.strictEqual(chunk.toString(), '\n');
+  }));
+  child.stderr.on('data', (chunk) => {
+    assert.fail(chunk.toString());
+  });
+  child.on('close', common.mustCall((code) => {
+    assert.strictEqual(code, 0);
+  }));
+}
+
+// fs.readFile(0)
+{
+  const script = fixtures.path('readfile-stdin-async.js');
+  const child = spawn(process.execPath, [script], {
+    stdio: ['ignore', 'pipe', 'pipe']
+  });
+  child.stdout.on('data', common.mustCall((chunk) => {
+    assert.strictEqual(chunk.toString(), '\n');
+  }));
+  child.stderr.on('data', (chunk) => {
+    assert.fail(chunk.toString());
+  });
+  child.on('close', common.mustCall((code) => {
+    assert.strictEqual(code, 0);
+  }));
+}