v8: fix native serdes constructors

Fixes: #13326 Refs: #13541 PR-URL: #36549 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
nodejs · Dec 22, 2020 · d90fa19 · d90fa19
1 parent 656ce92
commit d90fa19
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 16 deletions.
diff --git a/lib/v8.js b/lib/v8.js
@@ -35,8 +35,8 @@ const {
 const { Buffer } = require('buffer');
 const { validateString } = require('internal/validators');
 const {
-  Serializer: _Serializer,
-  Deserializer: _Deserializer
+  Serializer,
+  Deserializer
 } = internalBinding('serdes');
 
 let profiler = {};
@@ -70,13 +70,6 @@ function getHeapSnapshot() {
   return new HeapSnapshotStream(handle);
 }
 
-// Calling exposed c++ functions directly throws exception as it expected to be
-// called with new operator and caused an assert to fire.
-// Creating JS wrapper so that it gets caught at JS layer.
-class Serializer extends _Serializer { }
-
-class Deserializer extends _Deserializer { }
-
 const {
   cachedDataVersionTag,
   setFlagsFromString: _setFlagsFromString,

diff --git a/src/node_serdes.cc b/src/node_serdes.cc
@@ -169,6 +169,10 @@ Maybe<bool> SerializerContext::WriteHostObject(Isolate* isolate,
 
 void SerializerContext::New(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
+  if (!args.IsConstructCall()) {
+    return THROW_ERR_CONSTRUCT_CALL_REQUIRED(
+        env, "Class constructor Serializer cannot be invoked without 'new'");
+  }
 
   new SerializerContext(env, args.This());
 }
@@ -319,6 +323,10 @@ MaybeLocal<Object> DeserializerContext::ReadHostObject(Isolate* isolate) {
 
 void DeserializerContext::New(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
+  if (!args.IsConstructCall()) {
+    return THROW_ERR_CONSTRUCT_CALL_REQUIRED(
+        env, "Class constructor Deserializer cannot be invoked without 'new'");
+  }
 
   if (!args[0]->IsArrayBufferView()) {
     return node::THROW_ERR_INVALID_ARG_TYPE(
@@ -470,6 +478,7 @@ void Initialize(Local<Object> target,
   Local<String> serializerString =
       FIXED_ONE_BYTE_STRING(env->isolate(), "Serializer");
   ser->SetClassName(serializerString);
+  ser->ReadOnlyPrototype();
   target->Set(env->context(),
               serializerString,
               ser->GetFunction(env->context()).ToLocalChecked()).Check();
@@ -496,6 +505,8 @@ void Initialize(Local<Object> target,
 
   Local<String> deserializerString =
       FIXED_ONE_BYTE_STRING(env->isolate(), "Deserializer");
+  des->SetLength(1);
+  des->ReadOnlyPrototype();
   des->SetClassName(deserializerString);
   target->Set(env->context(),
               deserializerString,

diff --git a/test/parallel/test-v8-serdes.js b/test/parallel/test-v8-serdes.js
@@ -25,11 +25,6 @@ const objects = [
 
 const hostObject = new (internalBinding('js_stream').JSStream)();
 
-const serializerTypeError =
-  /^TypeError: Class constructor Serializer cannot be invoked without 'new'$/;
-const deserializerTypeError =
-  /^TypeError: Class constructor Deserializer cannot be invoked without 'new'$/;
-
 {
   const ser = new v8.DefaultSerializer();
   ser.writeHeader();
@@ -186,8 +181,16 @@ const deserializerTypeError =
 }
 
 {
-  assert.throws(v8.Serializer, serializerTypeError);
-  assert.throws(v8.Deserializer, deserializerTypeError);
+  assert.throws(() => v8.Serializer(), {
+    constructor: TypeError,
+    message: "Class constructor Serializer cannot be invoked without 'new'",
+    code: 'ERR_CONSTRUCT_CALL_REQUIRED'
+  });
+  assert.throws(() => v8.Deserializer(), {
+    constructor: TypeError,
+    message: "Class constructor Deserializer cannot be invoked without 'new'",
+    code: 'ERR_CONSTRUCT_CALL_REQUIRED'
+  });
 }