From 36daf1d6344137c7ab0e6b39998d1030721de45c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 7 Jan 2018 22:07:13 +0100 Subject: [PATCH] src: harden JSStream callbacks Since these are executing JS code, and in particular parts of that code may be provided by userland, handle such exceptions in C++. Refs: https://github.com/nodejs/node/pull/17938#issuecomment-354683850 PR-URL: https://github.com/nodejs/node/pull/18028 Reviewed-By: Ben Noordhuis Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Daniel Bevenius Reviewed-By: Tiancheng "Timothy" Gu --- src/js_stream.cc | 57 ++++++++++++++----- .../test-wrap-js-stream-exceptions.js | 19 +++++++ 2 files changed, 63 insertions(+), 13 deletions(-) create mode 100644 test/parallel/test-wrap-js-stream-exceptions.js diff --git a/src/js_stream.cc b/src/js_stream.cc index dba6d1a52b8013..7d1115f12ac3e2 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -14,9 +14,9 @@ using v8::FunctionCallbackInfo; using v8::FunctionTemplate; using v8::HandleScope; using v8::Local; -using v8::MaybeLocal; using v8::Object; using v8::String; +using v8::TryCatch; using v8::Value; @@ -87,24 +87,41 @@ bool JSStream::IsAlive() { bool JSStream::IsClosing() { HandleScope scope(env()->isolate()); Context::Scope context_scope(env()->context()); - return MakeCallback(env()->isclosing_string(), 0, nullptr) - .ToLocalChecked()->IsTrue(); + TryCatch try_catch(env()->isolate()); + Local value; + if (!MakeCallback(env()->isclosing_string(), 0, nullptr).ToLocal(&value)) { + FatalException(env()->isolate(), try_catch); + return true; + } + return value->IsTrue(); } int JSStream::ReadStart() { HandleScope scope(env()->isolate()); Context::Scope context_scope(env()->context()); - return MakeCallback(env()->onreadstart_string(), 0, nullptr) - .ToLocalChecked()->Int32Value(); + TryCatch try_catch(env()->isolate()); + Local value; + int value_int = UV_EPROTO; + if (!MakeCallback(env()->onreadstart_string(), 0, nullptr).ToLocal(&value) || + !value->Int32Value(env()->context()).To(&value_int)) { + FatalException(env()->isolate(), try_catch); + } + return value_int; } int JSStream::ReadStop() { HandleScope scope(env()->isolate()); Context::Scope context_scope(env()->context()); - return MakeCallback(env()->onreadstop_string(), 0, nullptr) - .ToLocalChecked()->Int32Value(); + TryCatch try_catch(env()->isolate()); + Local value; + int value_int = UV_EPROTO; + if (!MakeCallback(env()->onreadstop_string(), 0, nullptr).ToLocal(&value) || + !value->Int32Value(env()->context()).To(&value_int)) { + FatalException(env()->isolate(), try_catch); + } + return value_int; } @@ -117,10 +134,17 @@ int JSStream::DoShutdown(ShutdownWrap* req_wrap) { }; req_wrap->Dispatched(); - MaybeLocal res = - MakeCallback(env()->onshutdown_string(), arraysize(argv), argv); - return res.ToLocalChecked()->Int32Value(); + TryCatch try_catch(env()->isolate()); + Local value; + int value_int = UV_EPROTO; + if (!MakeCallback(env()->onshutdown_string(), + arraysize(argv), + argv).ToLocal(&value) || + !value->Int32Value(env()->context()).To(&value_int)) { + FatalException(env()->isolate(), try_catch); + } + return value_int; } @@ -146,10 +170,17 @@ int JSStream::DoWrite(WriteWrap* w, }; w->Dispatched(); - MaybeLocal res = - MakeCallback(env()->onwrite_string(), arraysize(argv), argv); - return res.ToLocalChecked()->Int32Value(); + TryCatch try_catch(env()->isolate()); + Local value; + int value_int = UV_EPROTO; + if (!MakeCallback(env()->onwrite_string(), + arraysize(argv), + argv).ToLocal(&value) || + !value->Int32Value(env()->context()).To(&value_int)) { + FatalException(env()->isolate(), try_catch); + } + return value_int; } diff --git a/test/parallel/test-wrap-js-stream-exceptions.js b/test/parallel/test-wrap-js-stream-exceptions.js new file mode 100644 index 00000000000000..57ecd70189d106 --- /dev/null +++ b/test/parallel/test-wrap-js-stream-exceptions.js @@ -0,0 +1,19 @@ +// Flags: --expose-internals +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const JSStreamWrap = require('internal/wrap_js_stream'); +const { Duplex } = require('stream'); + +process.once('uncaughtException', common.mustCall((err) => { + assert.strictEqual(err.message, 'exception!'); +})); + +const socket = new JSStreamWrap(new Duplex({ + read: common.mustCall(), + write: common.mustCall((buffer, data, cb) => { + throw new Error('exception!'); + }) +})); + +assert.throws(() => socket.end('foo'), /Error: write EPROTO/);