Skip to content

Commit

Permalink
tty_wrap: throw when uv_tty_init() returns error
Browse files Browse the repository at this point in the history
Also add checks in lib/tty.js and tests.

PR-URL: #12892
Ref: #11883
Ref: #8531
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
  • Loading branch information
trevnorris authored and addaleax committed May 10, 2017
1 parent dd6e3f6 commit 4b9d84d
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 5 deletions.
8 changes: 7 additions & 1 deletion lib/tty.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ exports.isatty = function(fd) {
function ReadStream(fd, options) {
if (!(this instanceof ReadStream))
return new ReadStream(fd, options);
if (fd >> 0 !== fd || fd < 0)
throw new RangeError('fd must be positive integer: ' + fd);

options = util._extend({
highWaterMark: 0,
Expand All @@ -62,7 +64,11 @@ ReadStream.prototype.setRawMode = function(flag) {


function WriteStream(fd) {
if (!(this instanceof WriteStream)) return new WriteStream(fd);
if (!(this instanceof WriteStream))
return new WriteStream(fd);
if (fd >> 0 !== fd || fd < 0)
throw new RangeError('fd must be positive integer: ' + fd);

net.Socket.call(this, {
handle: new TTY(fd, false),
readable: false,
Expand Down
14 changes: 11 additions & 3 deletions src/tty_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,25 @@ void TTYWrap::New(const FunctionCallbackInfo<Value>& args) {
int fd = args[0]->Int32Value();
CHECK_GE(fd, 0);

TTYWrap* wrap = new TTYWrap(env, args.This(), fd, args[1]->IsTrue());
int err = 0;
TTYWrap* wrap = new TTYWrap(env, args.This(), fd, args[1]->IsTrue(), &err);
if (err != 0)
return env->ThrowUVException(err, "uv_tty_init");

wrap->UpdateWriteQueueSize();
}


TTYWrap::TTYWrap(Environment* env, Local<Object> object, int fd, bool readable)
TTYWrap::TTYWrap(Environment* env,
Local<Object> object,
int fd,
bool readable,
int* init_err)
: StreamWrap(env,
object,
reinterpret_cast<uv_stream_t*>(&handle_),
AsyncWrap::PROVIDER_TTYWRAP) {
uv_tty_init(env->event_loop(), &handle_, fd, readable);
*init_err = uv_tty_init(env->event_loop(), &handle_, fd, readable);
}

} // namespace node
Expand Down
3 changes: 2 additions & 1 deletion src/tty_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ class TTYWrap : public StreamWrap {
TTYWrap(Environment* env,
v8::Local<v8::Object> object,
int fd,
bool readable);
bool readable,
int* init_err);

static void GuessHandleType(const v8::FunctionCallbackInfo<v8::Value>& args);
static void IsTTY(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down
36 changes: 36 additions & 0 deletions test/parallel/test-ttywrap-invalid-fd.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const fs = require('fs');
const tty = require('tty');


assert.throws(() => {
new tty.WriteStream(-1);
}, /fd must be positive integer:/);

const err_regex = common.isWindows ?
/^Error: EBADF: bad file descriptor, uv_tty_init$/ :
/^Error: EINVAL: invalid argument, uv_tty_init$/;
assert.throws(() => {
let fd = 2;
// Get first known bad file descriptor.
try {
while (fs.fstatSync(++fd));
} catch (e) { }
new tty.WriteStream(fd);
}, err_regex);

assert.throws(() => {
new tty.ReadStream(-1);
}, /fd must be positive integer:/);

assert.throws(() => {
let fd = 2;
// Get first known bad file descriptor.
try {
while (fs.fstatSync(++fd));
} catch (e) { }
new tty.ReadStream(fd);
}, err_regex);

0 comments on commit 4b9d84d

Please sign in to comment.