Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 2bb1475

Browse filesBrowse files
evanlucasrvagg
authored andcommitted
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: #3503 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
1 parent 9d8d752 commit 2bb1475
Copy full SHA for 2bb1475

File tree

Expand file treeCollapse file tree

2 files changed

+74
-3
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+74
-3
lines changed
Open diff view settings
Collapse file

‎lib/fs.js‎

Copy file name to clipboardExpand all lines: lib/fs.js
+16-3Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -599,10 +599,13 @@ fs.read = function(fd, buffer, offset, length, position, callback) {
599599

600600
callback = function(err, bytesRead) {
601601
if (!cb) return;
602+
if (err) return cb(err);
602603

603-
var str = (bytesRead > 0) ? buffer.toString(encoding, 0, bytesRead) : '';
604-
605-
(cb)(err, str, bytesRead);
604+
if (bytesRead > 0) {
605+
tryToStringWithEnd(buffer, encoding, bytesRead, cb);
606+
} else {
607+
(cb)(err, '', bytesRead);
608+
}
606609
};
607610
}
608611

@@ -617,6 +620,16 @@ fs.read = function(fd, buffer, offset, length, position, callback) {
617620
binding.read(fd, buffer, offset, length, position, req);
618621
};
619622

623+
function tryToStringWithEnd(buf, encoding, end, callback) {
624+
var e;
625+
try {
626+
buf = buf.toString(encoding, 0, end);
627+
} catch (err) {
628+
e = err;
629+
}
630+
callback(e, buf, end);
631+
}
632+
620633
fs.readSync = function(fd, buffer, offset, length, position) {
621634
var legacy = false;
622635
var encoding;
Collapse file
+58Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const fs = require('fs');
6+
const path = require('path');
7+
const Buffer = require('buffer').Buffer;
8+
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
9+
const kMaxLength = process.binding('buffer').kMaxLength;
10+
11+
var fd;
12+
13+
common.refreshTmpDir();
14+
15+
const file = path.join(common.tmpDir, 'toobig2.txt');
16+
const stream = fs.createWriteStream(file, {
17+
flags: 'a'
18+
});
19+
20+
const size = kStringMaxLength / 200;
21+
const a = new Buffer(size).fill('a');
22+
23+
for (var i = 0; i < 201; i++) {
24+
stream.write(a);
25+
}
26+
27+
stream.end();
28+
stream.on('finish', common.mustCall(function() {
29+
fd = fs.openSync(file, 'r');
30+
fs.read(fd, kStringMaxLength + 1, 0, 'utf8', common.mustCall(function(err) {
31+
assert.ok(err instanceof Error);
32+
assert.strictEqual('toString failed', err.message);
33+
}));
34+
}));
35+
36+
function destroy() {
37+
try {
38+
// Make sure we close fd and unlink the file
39+
fs.closeSync(fd);
40+
fs.unlinkSync(file);
41+
} catch (err) {
42+
// it may not exist
43+
}
44+
}
45+
46+
process.on('exit', destroy);
47+
48+
process.on('SIGINT', function() {
49+
destroy();
50+
process.exit();
51+
});
52+
53+
// To make sure we don't leave a very large file
54+
// on test machines in the event this test fails.
55+
process.on('uncaughtException', function(err) {
56+
destroy();
57+
throw err;
58+
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.