From 4b9d84df51f7c1d879e27f651cd84de4cb46e229 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Tue, 11 Apr 2017 16:29:39 -0600 Subject: [PATCH] tty_wrap: throw when uv_tty_init() returns error Also add checks in lib/tty.js and tests. PR-URL: https://github.com/nodejs/node/pull/12892 Ref: https://github.com/nodejs/node/pull/11883 Ref: https://github.com/nodejs/node/pull/8531 Reviewed-By: Andreas Madsen Reviewed-By: Anna Henningsen Reviewed-By: Sam Roberts Reviewed-By: Matteo Collina Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel --- lib/tty.js | 8 +++++- src/tty_wrap.cc | 14 +++++++-- src/tty_wrap.h | 3 +- test/parallel/test-ttywrap-invalid-fd.js | 36 ++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-ttywrap-invalid-fd.js diff --git a/lib/tty.js b/lib/tty.js index 3812e9c56b..d467c82781 100644 --- a/lib/tty.js +++ b/lib/tty.js @@ -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, @@ -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, diff --git a/src/tty_wrap.cc b/src/tty_wrap.cc index 82476e755d..b6e3efcc10 100644 --- a/src/tty_wrap.cc +++ b/src/tty_wrap.cc @@ -150,17 +150,25 @@ void TTYWrap::New(const FunctionCallbackInfo& 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, int fd, bool readable) +TTYWrap::TTYWrap(Environment* env, + Local object, + int fd, + bool readable, + int* init_err) : StreamWrap(env, object, reinterpret_cast(&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 diff --git a/src/tty_wrap.h b/src/tty_wrap.h index 8eadbf0a9f..7b7cb5ece8 100644 --- a/src/tty_wrap.h +++ b/src/tty_wrap.h @@ -44,7 +44,8 @@ class TTYWrap : public StreamWrap { TTYWrap(Environment* env, v8::Local object, int fd, - bool readable); + bool readable, + int* init_err); static void GuessHandleType(const v8::FunctionCallbackInfo& args); static void IsTTY(const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-ttywrap-invalid-fd.js b/test/parallel/test-ttywrap-invalid-fd.js new file mode 100644 index 0000000000..b78bc689b2 --- /dev/null +++ b/test/parallel/test-ttywrap-invalid-fd.js @@ -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);