From 1935625df4f56bd77c57ec100a9222c77e9d440c Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 16 Mar 2019 13:21:25 +0100
Subject: [PATCH] src: disallow constructor behaviour for native methods
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Disallow constructor behaviour and setting up prototypes
for native methods that are not constructors (i.e. make
them behave like ES6 class methods).

PR-URL: https://github.com/nodejs/node/pull/26700
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/env-inl.h                       | 8 ++------
 test/parallel/test-process-chdir.js | 5 +++++
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/env-inl.h b/src/env-inl.h
index b32f3ee63d9b21..33c7868e247c7d 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -883,9 +883,7 @@ inline void Environment::SetMethod(v8::Local<v8::Object> that,
   v8::Local<v8::Context> context = isolate()->GetCurrentContext();
   v8::Local<v8::Function> function =
       NewFunctionTemplate(callback, v8::Local<v8::Signature>(),
-                          // TODO(TimothyGu): Investigate if SetMethod is ever
-                          // used for constructors.
-                          v8::ConstructorBehavior::kAllow,
+                          v8::ConstructorBehavior::kThrow,
                           v8::SideEffectType::kHasSideEffect)
           ->GetFunction(context)
           .ToLocalChecked();
@@ -903,9 +901,7 @@ inline void Environment::SetMethodNoSideEffect(v8::Local<v8::Object> that,
   v8::Local<v8::Context> context = isolate()->GetCurrentContext();
   v8::Local<v8::Function> function =
       NewFunctionTemplate(callback, v8::Local<v8::Signature>(),
-                          // TODO(TimothyGu): Investigate if SetMethod is ever
-                          // used for constructors.
-                          v8::ConstructorBehavior::kAllow,
+                          v8::ConstructorBehavior::kThrow,
                           v8::SideEffectType::kHasNoSideEffect)
           ->GetFunction(context)
           .ToLocalChecked();
diff --git a/test/parallel/test-process-chdir.js b/test/parallel/test-process-chdir.js
index e66d366fb7874f..005e17fac25a90 100644
--- a/test/parallel/test-process-chdir.js
+++ b/test/parallel/test-process-chdir.js
@@ -42,3 +42,8 @@ const err = {
 };
 common.expectsError(function() { process.chdir({}); }, err);
 common.expectsError(function() { process.chdir(); }, err);
+
+// Check that our built-in methods do not have a prototype/constructor behaviour
+// if they don't need to. This could be tested for any of our C++ methods.
+assert.strictEqual(process.cwd.prototype, undefined);
+assert.throws(() => new process.cwd(), TypeError);