Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 303a9a3

Browse filesBrowse files
addaleaxBridgeAR
addaleax
authored and
BridgeAR
committed
worker: make MessagePort constructor non-callable
Refactor the C++ code for creating `MessagePort`s to skip calling the constructor and instead directly instantiating the `InstanceTemplate`, and always throw an error from the `MessagePort` constructor. This aligns behaviour with the web, and creating single `MessagePort`s does not make sense anyway. PR-URL: #28032 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent c67642a commit 303a9a3
Copy full SHA for 303a9a3

File tree

Expand file treeCollapse file tree

6 files changed

+52
-30
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+52
-30
lines changed
Open diff view settings
Collapse file

‎doc/api/errors.md‎

Copy file name to clipboardExpand all lines: doc/api/errors.md
+8Lines changed: 8 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -703,6 +703,14 @@ non-writable `stdout` or `stderr` stream.
703703

704704
A constructor for a class was called without `new`.
705705

706+
<a id="ERR_CONSTRUCT_CALL_INVALID"></a>
707+
### ERR_CONSTRUCT_CALL_INVALID
708+
<!--
709+
added: REPLACEME
710+
-->
711+
712+
A class constructor was called that is not callable.
713+
706714
<a id="ERR_CPU_USAGE"></a>
707715
### ERR_CPU_USAGE
708716

Collapse file

‎src/node_errors.h‎

Copy file name to clipboardExpand all lines: src/node_errors.h
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ void FatalException(v8::Isolate* isolate,
5454
V(ERR_BUFFER_CONTEXT_NOT_AVAILABLE, Error) \
5555
V(ERR_BUFFER_OUT_OF_BOUNDS, RangeError) \
5656
V(ERR_BUFFER_TOO_LARGE, Error) \
57-
V(ERR_CONSTRUCT_CALL_REQUIRED, Error) \
57+
V(ERR_CONSTRUCT_CALL_REQUIRED, TypeError) \
58+
V(ERR_CONSTRUCT_CALL_INVALID, TypeError) \
5859
V(ERR_INVALID_ARG_VALUE, TypeError) \
5960
V(ERR_INVALID_ARG_TYPE, TypeError) \
6061
V(ERR_INVALID_MODULE_SPECIFIER, TypeError) \
@@ -99,6 +100,7 @@ void FatalException(v8::Isolate* isolate,
99100
#define PREDEFINED_ERROR_MESSAGES(V) \
100101
V(ERR_BUFFER_CONTEXT_NOT_AVAILABLE, \
101102
"Buffer is not available for the current Context") \
103+
V(ERR_CONSTRUCT_CALL_INVALID, "Constructor cannot be called") \
102104
V(ERR_CONSTRUCT_CALL_REQUIRED, "Cannot call constructor without `new`") \
103105
V(ERR_INVALID_TRANSFER_OBJECT, "Found invalid object in transferList") \
104106
V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory") \
Collapse file

‎src/node_messaging.cc‎

Copy file name to clipboardExpand all lines: src/node_messaging.cc
+12-20Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -529,33 +529,26 @@ void MessagePort::Close(v8::Local<v8::Value> close_callback) {
529529
}
530530

531531
void MessagePort::New(const FunctionCallbackInfo<Value>& args) {
532+
// This constructor just throws an error. Unfortunately, we can’t use V8’s
533+
// ConstructorBehavior::kThrow, as that also removes the prototype from the
534+
// class (i.e. makes it behave like an arrow function).
532535
Environment* env = Environment::GetCurrent(args);
533-
if (!args.IsConstructCall()) {
534-
THROW_ERR_CONSTRUCT_CALL_REQUIRED(env);
535-
return;
536-
}
537-
538-
Local<Context> context = args.This()->CreationContext();
539-
Context::Scope context_scope(context);
540-
541-
new MessagePort(env, context, args.This());
536+
THROW_ERR_CONSTRUCT_CALL_INVALID(env);
542537
}
543538

544539
MessagePort* MessagePort::New(
545540
Environment* env,
546541
Local<Context> context,
547542
std::unique_ptr<MessagePortData> data) {
548543
Context::Scope context_scope(context);
549-
Local<Function> ctor;
550-
if (!GetMessagePortConstructor(env, context).ToLocal(&ctor))
551-
return nullptr;
544+
Local<FunctionTemplate> ctor_templ = GetMessagePortConstructorTemplate(env);
552545

553546
// Construct a new instance, then assign the listener instance and possibly
554547
// the MessagePortData to it.
555548
Local<Object> instance;
556-
if (!ctor->NewInstance(context).ToLocal(&instance))
549+
if (!ctor_templ->InstanceTemplate()->NewInstance(context).ToLocal(&instance))
557550
return nullptr;
558-
MessagePort* port = Unwrap<MessagePort>(instance);
551+
MessagePort* port = new MessagePort(env, context, instance);
559552
CHECK_NOT_NULL(port);
560553
if (data) {
561554
port->Detach();
@@ -830,13 +823,12 @@ void MessagePort::Entangle(MessagePort* a, MessagePortData* b) {
830823
MessagePortData::Entangle(a->data_.get(), b);
831824
}
832825

833-
MaybeLocal<Function> GetMessagePortConstructor(
834-
Environment* env, Local<Context> context) {
826+
Local<FunctionTemplate> GetMessagePortConstructorTemplate(Environment* env) {
835827
// Factor generating the MessagePort JS constructor into its own piece
836828
// of code, because it is needed early on in the child environment setup.
837829
Local<FunctionTemplate> templ = env->message_port_constructor_template();
838830
if (!templ.IsEmpty())
839-
return templ->GetFunction(context);
831+
return templ;
840832

841833
Isolate* isolate = env->isolate();
842834

@@ -859,7 +851,7 @@ MaybeLocal<Function> GetMessagePortConstructor(
859851
env->set_message_event_object_template(e);
860852
}
861853

862-
return GetMessagePortConstructor(env, context);
854+
return GetMessagePortConstructorTemplate(env);
863855
}
864856

865857
namespace {
@@ -902,8 +894,8 @@ static void InitMessaging(Local<Object> target,
902894

903895
target->Set(context,
904896
env->message_port_constructor_string(),
905-
GetMessagePortConstructor(env, context).ToLocalChecked())
906-
.Check();
897+
GetMessagePortConstructorTemplate(env)
898+
->GetFunction(context).ToLocalChecked()).Check();
907899

908900
// These are not methods on the MessagePort prototype, because
909901
// the browser equivalents do not provide them.
Collapse file

‎src/node_messaging.h‎

Copy file name to clipboardExpand all lines: src/node_messaging.h
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,8 @@ class MessagePort : public HandleWrap {
211211
friend class MessagePortData;
212212
};
213213

214-
v8::MaybeLocal<v8::Function> GetMessagePortConstructor(
215-
Environment* env, v8::Local<v8::Context> context);
214+
v8::Local<v8::FunctionTemplate> GetMessagePortConstructorTemplate(
215+
Environment* env);
216216

217217
} // namespace worker
218218
} // namespace node
Collapse file

‎test/parallel/test-worker-message-port-constructor.js‎

Copy file name to clipboard
+27Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
require('../common');
3+
const assert = require('assert');
4+
5+
const { MessageChannel, MessagePort } = require('worker_threads');
6+
7+
// Make sure that `MessagePort` is the constructor for MessagePort instances,
8+
// but not callable.
9+
const { port1 } = new MessageChannel();
10+
11+
assert(port1 instanceof MessagePort);
12+
assert.strictEqual(port1.constructor, MessagePort);
13+
14+
assert.throws(() => MessagePort(), {
15+
constructor: TypeError,
16+
code: 'ERR_CONSTRUCT_CALL_INVALID'
17+
});
18+
19+
assert.throws(() => new MessagePort(), {
20+
constructor: TypeError,
21+
code: 'ERR_CONSTRUCT_CALL_INVALID'
22+
});
23+
24+
assert.throws(() => MessageChannel(), {
25+
constructor: TypeError,
26+
code: 'ERR_CONSTRUCT_CALL_REQUIRED'
27+
});
Collapse file

‎test/parallel/test-worker-message-port-move.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-worker-message-port-move.js
-7Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,6 @@ vm.runInContext('(' + function() {
5353
}
5454
assert(threw);
5555
}
56-
57-
{
58-
const newDummyPort = new (port.constructor)();
59-
assert(!(newDummyPort instanceof MessagePort));
60-
assert(newDummyPort.close instanceof Function);
61-
newDummyPort.close();
62-
}
6356
} + ')()', context);
6457

6558
port2.on('message', common.mustCall((msg) => {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.