From 806857712f76398a786874d77aa65e2f3cbf7dab Mon Sep 17 00:00:00 2001
From: Timothy Gu <timothygu99@gmail.com>
Date: Fri, 29 Sep 2017 22:15:24 -0700
Subject: [PATCH] src: do not add .domain to promises in VM contexts

The promises are still tracked, and their handlers will still execute in
the correct domain. The creation domain is simply hidden.

PR-URL: https://github.com/nodejs/node/pull/15695
Fixes: https://github.com/nodejs/node/issues/15673
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
---
 doc/api/domain.md                    |  6 ++++++
 src/env.h                            |  1 +
 src/node.cc                          | 29 ++++++++++++++++++++++------
 test/parallel/test-domain-promise.js | 10 +++++++---
 4 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/doc/api/domain.md b/doc/api/domain.md
index 285378c3bd5..30e93e2bea4 100644
--- a/doc/api/domain.md
+++ b/doc/api/domain.md
@@ -1,6 +1,12 @@
 # Domain
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/REPLACEME
+    description: Any `Promise`s created in VM contexts no longer have a
+                 `.domain` property. Their handlers are still executed in the
+                 proper domain, however, and `Promise`s created in the main
+                 context still possess a `.domain` property.
   - version: v8.0.0
     pr-url: https://github.com/nodejs/node/pull/12489
     description: Handlers for `Promise`s are now invoked in the domain in which
diff --git a/src/env.h b/src/env.h
index 0ebaf745256..567d87a313a 100644
--- a/src/env.h
+++ b/src/env.h
@@ -93,6 +93,7 @@ class ModuleWrap;
   V(npn_buffer_private_symbol, "node:npnBuffer")                              \
   V(processed_private_symbol, "node:processed")                               \
   V(selected_npn_buffer_private_symbol, "node:selectedNpnBuffer")             \
+  V(domain_private_symbol, "node:domain")                                     \
 
 // Strings are per-isolate primitives but Environment proxies them
 // for the sake of convenience.  Strings should be ASCII-only.
diff --git a/src/node.cc b/src/node.cc
index 0c69ece87ad..6c1b11d4caa 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -1156,8 +1156,19 @@ bool ShouldAbortOnUncaughtException(Isolate* isolate) {
 }
 
 
+Local<Value> GetDomainProperty(Environment* env, Local<Object> object) {
+  Local<Value> domain_v =
+      object->GetPrivate(env->context(), env->domain_private_symbol())
+          .ToLocalChecked();
+  if (domain_v->IsObject()) {
+    return domain_v;
+  }
+  return object->Get(env->context(), env->domain_string()).ToLocalChecked();
+}
+
+
 void DomainEnter(Environment* env, Local<Object> object) {
-  Local<Value> domain_v = object->Get(env->domain_string());
+  Local<Value> domain_v = GetDomainProperty(env, object);
   if (domain_v->IsObject()) {
     Local<Object> domain = domain_v.As<Object>();
     Local<Value> enter_v = domain->Get(env->enter_string());
@@ -1172,7 +1183,7 @@ void DomainEnter(Environment* env, Local<Object> object) {
 
 
 void DomainExit(Environment* env, v8::Local<v8::Object> object) {
-  Local<Value> domain_v = object->Get(env->domain_string());
+  Local<Value> domain_v = GetDomainProperty(env, object);
   if (domain_v->IsObject()) {
     Local<Object> domain = domain_v.As<Object>();
     Local<Value> exit_v = domain->Get(env->exit_string());
@@ -1194,10 +1205,16 @@ void DomainPromiseHook(PromiseHookType type,
   Local<Context> context = env->context();
 
   if (type == PromiseHookType::kInit && env->in_domain()) {
-    promise->Set(context,
-                 env->domain_string(),
-                 env->domain_array()->Get(context,
-                                          0).ToLocalChecked()).FromJust();
+    Local<Value> domain_obj =
+        env->domain_array()->Get(context, 0).ToLocalChecked();
+    if (promise->CreationContext() == context) {
+      promise->Set(context, env->domain_string(), domain_obj).FromJust();
+    } else {
+      // Do not expose object from another context publicly in promises created
+      // in non-main contexts.
+      promise->SetPrivate(context, env->domain_private_symbol(), domain_obj)
+          .FromJust();
+    }
     return;
   }
 
diff --git a/test/parallel/test-domain-promise.js b/test/parallel/test-domain-promise.js
index 8bae75eb63b..f1c966d4afd 100644
--- a/test/parallel/test-domain-promise.js
+++ b/test/parallel/test-domain-promise.js
@@ -31,9 +31,13 @@ common.crashOnUnhandledRejection();
   const d = domain.create();
 
   d.run(common.mustCall(() => {
-    vm.runInNewContext(`Promise.resolve().then(common.mustCall(() => {
-      assert.strictEqual(process.domain, d);
-    }));`, { common, assert, process, d });
+    vm.runInNewContext(`
+      const promise = Promise.resolve();
+      assert.strictEqual(promise.domain, undefined);
+      promise.then(common.mustCall(() => {
+        assert.strictEqual(process.domain, d);
+      }));
+    `, { common, assert, process, d });
   }));
 }