From 57359cd1e45084ac107ca5b16fe2a1423139d711 Mon Sep 17 00:00:00 2001
From: Evan Lucas <evanlucas@me.com>
Date: Fri, 23 Oct 2015 16:54:04 -0500
Subject: [PATCH] fs: don't throw in read if buffer too big

If the resulting buffer.toString() call in fs.read throws, catch the
error and pass it back in the callback.

This issue only presents itself when fs.read is called using the legacy
string interface:

fs.read(fd, length, position, encoding, callback)

PR-URL: https://github.com/nodejs/node/pull/3503
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
---
 lib/fs.js                                     | 19 +++++-
 .../test-fs-read-buffer-tostring-fail.js      | 58 +++++++++++++++++++
 2 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100644 test/parallel/test-fs-read-buffer-tostring-fail.js

diff --git a/lib/fs.js b/lib/fs.js
index 6345497c4f..42d02baa9e 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -599,10 +599,13 @@ fs.read = function(fd, buffer, offset, length, position, callback) {
 
     callback = function(err, bytesRead) {
       if (!cb) return;
+      if (err) return cb(err);
 
-      var str = (bytesRead > 0) ? buffer.toString(encoding, 0, bytesRead) : '';
-
-      (cb)(err, str, bytesRead);
+      if (bytesRead > 0) {
+        tryToStringWithEnd(buffer, encoding, bytesRead, cb);
+      } else {
+        (cb)(err, '', bytesRead);
+      }
     };
   }
 
@@ -617,6 +620,16 @@ fs.read = function(fd, buffer, offset, length, position, callback) {
   binding.read(fd, buffer, offset, length, position, req);
 };
 
+function tryToStringWithEnd(buf, encoding, end, callback) {
+  var e;
+  try {
+    buf = buf.toString(encoding, 0, end);
+  } catch (err) {
+    e = err;
+  }
+  callback(e, buf, end);
+}
+
 fs.readSync = function(fd, buffer, offset, length, position) {
   var legacy = false;
   var encoding;
diff --git a/test/parallel/test-fs-read-buffer-tostring-fail.js b/test/parallel/test-fs-read-buffer-tostring-fail.js
new file mode 100644
index 0000000000..3ec1491d2b
--- /dev/null
+++ b/test/parallel/test-fs-read-buffer-tostring-fail.js
@@ -0,0 +1,58 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+const Buffer = require('buffer').Buffer;
+const kStringMaxLength = process.binding('buffer').kStringMaxLength;
+const kMaxLength = process.binding('buffer').kMaxLength;
+
+var fd;
+
+common.refreshTmpDir();
+
+const file = path.join(common.tmpDir, 'toobig2.txt');
+const stream = fs.createWriteStream(file, {
+  flags: 'a'
+});
+
+const size = kStringMaxLength / 200;
+const a = new Buffer(size).fill('a');
+
+for (var i = 0; i < 201; i++) {
+  stream.write(a);
+}
+
+stream.end();
+stream.on('finish', common.mustCall(function() {
+  fd = fs.openSync(file, 'r');
+  fs.read(fd, kStringMaxLength + 1, 0, 'utf8', common.mustCall(function(err) {
+    assert.ok(err instanceof Error);
+    assert.strictEqual('toString failed', err.message);
+  }));
+}));
+
+function destroy() {
+  try {
+    // Make sure we close fd and unlink the file
+    fs.closeSync(fd);
+    fs.unlinkSync(file);
+  } catch (err) {
+    // it may not exist
+  }
+}
+
+process.on('exit', destroy);
+
+process.on('SIGINT', function() {
+  destroy();
+  process.exit();
+});
+
+// To make sure we don't leave a very large file
+// on test machines in the event this test fails.
+process.on('uncaughtException', function(err) {
+  destroy();
+  throw err;
+});