From ba4788dd9f253acb9172c6ca4399492c53ae9155 Mon Sep 17 00:00:00 2001 From: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> Date: Thu, 17 Dec 2020 12:10:00 +0100 Subject: [PATCH] =?UTF-8?q?v8:=20fix=C2=A0native=20`serdes`=C2=A0construct?= =?UTF-8?q?ors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/nodejs/node/issues/13326 Refs: https://github.com/nodejs/node/pull/13541 PR-URL: https://github.com/nodejs/node/pull/36549 Reviewed-By: Rich Trott Reviewed-By: Benjamin Gruenbaum --- lib/v8.js | 11 ++--------- src/node_serdes.cc | 11 +++++++++++ test/parallel/test-v8-serdes.js | 17 ++++++++++------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/lib/v8.js b/lib/v8.js index 56e3e6056214ca..ef84a8b406771d 100644 --- a/lib/v8.js +++ b/lib/v8.js @@ -34,8 +34,8 @@ const { const { Buffer } = require('buffer'); const { validateString } = require('internal/validators'); const { - Serializer: _Serializer, - Deserializer: _Deserializer + Serializer, + Deserializer } = internalBinding('serdes'); const assert = require('internal/assert'); const { copy } = internalBinding('buffer'); @@ -63,13 +63,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 index 28844f1858ff3d..b51d315989ce71 100644 --- a/src/node_serdes.cc +++ b/src/node_serdes.cc @@ -169,6 +169,10 @@ Maybe SerializerContext::WriteHostObject(Isolate* isolate, void SerializerContext::New(const FunctionCallbackInfo& 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 DeserializerContext::ReadHostObject(Isolate* isolate) { void DeserializerContext::New(const FunctionCallbackInfo& 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 target, Local 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 target, Local 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 index 593fb34ad73594..7485aa19a7d3d9 100644 --- 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' + }); }