Skip to content

Commit

Permalink
v8: fix native serdes constructors
Browse files Browse the repository at this point in the history
Fixes: #13326
Refs: #13541
PR-URL: #36549
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
  • Loading branch information
ExE-Boss authored and danielleadams committed Jan 12, 2021
1 parent c844d22 commit 3170636
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 16 deletions.
11 changes: 2 additions & 9 deletions lib/v8.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 11 additions & 0 deletions src/node_serdes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand Down
17 changes: 10 additions & 7 deletions test/parallel/test-v8-serdes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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'
});
}


Expand Down

0 comments on commit 3170636

Please sign in to comment.