FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

worker: make MessagePort constructor non-callable · nodejs/node@303a9a3 · GitHub

/ node Public

Commit 303a9a3

Browse files
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

6 files changed

Lines changed: 52 additions & 30 deletions

File tree

‎doc/api/errors.md‎

Lines changed: 8 additions & 0 deletions
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

‎src/node_errors.h‎

Lines 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") \

‎src/node_messaging.cc‎

Lines 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.

‎src/node_messaging.h‎

Lines 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

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

Lines 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+
});

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

Lines 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)
Back | FazBrowse Home | New Git URL