From b779c072d05c8deb4bf53a048b610ede6b38073d Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 26 Jan 2019 00:13:01 +0100
Subject: [PATCH] src: make `StreamPipe::Unpipe()` more resilient

Clean up `StreamPipe::Unpipe()` to be more resilient against
unexpected exceptions, in particular while executing its
`MakeCallback()` line (which can fail in the presence of
termination exceptions), and clean up the getter/setter part
of the code to match that pattern as well (even though it should
not fail as part of regular operations).

PR-URL: https://github.com/nodejs/node/pull/25716
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
---
 src/stream_pipe.cc | 40 +++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc
index e58aa929e4b30e..19d732d6592aaa 100644
--- a/src/stream_pipe.cc
+++ b/src/stream_pipe.cc
@@ -4,6 +4,7 @@
 
 using v8::Context;
 using v8::External;
+using v8::Function;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
 using v8::Local;
@@ -77,8 +78,12 @@ void StreamPipe::Unpipe() {
     Context::Scope context_scope(env->context());
     Local<Object> object = pipe->object();
 
-    if (object->Has(env->context(), env->onunpipe_string()).FromJust()) {
-      pipe->MakeCallback(env->onunpipe_string(), 0, nullptr).ToLocalChecked();
+    Local<Value> onunpipe;
+    if (!object->Get(env->context(), env->onunpipe_string()).ToLocal(&onunpipe))
+      return;
+    if (onunpipe->IsFunction() &&
+        pipe->MakeCallback(onunpipe.As<Function>(), 0, nullptr).IsEmpty()) {
+      return;
     }
 
     // Set all the links established in the constructor to `null`.
@@ -86,21 +91,22 @@ void StreamPipe::Unpipe() {
 
     Local<Value> source_v;
     Local<Value> sink_v;
-    source_v = object->Get(env->context(), env->source_string())
-        .ToLocalChecked();
-    sink_v = object->Get(env->context(), env->sink_string())
-        .ToLocalChecked();
-    CHECK(source_v->IsObject());
-    CHECK(sink_v->IsObject());
-
-    object->Set(env->context(), env->source_string(), null).FromJust();
-    object->Set(env->context(), env->sink_string(), null).FromJust();
-    source_v.As<Object>()->Set(env->context(),
-                               env->pipe_target_string(),
-                               null).FromJust();
-    sink_v.As<Object>()->Set(env->context(),
-                             env->pipe_source_string(),
-                             null).FromJust();
+    if (!object->Get(env->context(), env->source_string()).ToLocal(&source_v) ||
+        !object->Get(env->context(), env->sink_string()).ToLocal(&sink_v) ||
+        !source_v->IsObject() || !sink_v->IsObject()) {
+      return;
+    }
+
+    if (object->Set(env->context(), env->source_string(), null).IsNothing() ||
+        object->Set(env->context(), env->sink_string(), null).IsNothing() ||
+        source_v.As<Object>()
+            ->Set(env->context(), env->pipe_target_string(), null)
+            .IsNothing() ||
+        sink_v.As<Object>()
+            ->Set(env->context(), env->pipe_source_string(), null)
+            .IsNothing()) {
+      return;
+    }
   }, static_cast<void*>(this), object());
 }