Skip to content

Commit

Permalink
src: disallow constructor behaviour for native methods
Browse files Browse the repository at this point in the history
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: #26700
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
addaleax committed Mar 21, 2019
1 parent 11f8024 commit 1935625
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 6 deletions.
8 changes: 2 additions & 6 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions test/parallel/test-process-chdir.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

0 comments on commit 1935625

Please sign in to comment.