GeezerVison is showing github.com — v8: fix native `serdes` constructors · nodejs/node@ba4788d
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/v8.js +2-9Lines changed: 2 additions & 9 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -34,8 +34,8 @@ const { | 34 | 34 | const { Buffer } = require('buffer'); | 35 | 35 | const { validateString } = require('internal/validators'); | 36 | 36 | const { | 37 | | - Serializer: _Serializer, | 38 | | - Deserializer: _Deserializer | | 37 | + Serializer, | | 38 | + Deserializer | 39 | 39 | } = internalBinding('serdes'); | 40 | 40 | const assert = require('internal/assert'); | 41 | 41 | const { copy } = internalBinding('buffer'); | @@ -63,13 +63,6 @@ function getHeapSnapshot() { | 63 | 63 | return new HeapSnapshotStream(handle); | 64 | 64 | } | 65 | 65 |
| 66 | | -// Calling exposed c++ functions directly throws exception as it expected to be | 67 | | -// called with new operator and caused an assert to fire. | 68 | | -// Creating JS wrapper so that it gets caught at JS layer. | 69 | | -class Serializer extends _Serializer { } | 70 | | - | 71 | | -class Deserializer extends _Deserializer { } | 72 | | - | 73 | 66 | const { | 74 | 67 | cachedDataVersionTag, | 75 | 68 | setFlagsFromString: _setFlagsFromString, | |
Collapse file Copy file name to clipboardExpand all lines: src/node_serdes.cc +11Lines changed: 11 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -169,6 +169,10 @@ Maybe<bool> SerializerContext::WriteHostObject(Isolate* isolate, | 169 | 169 |
| 170 | 170 | void SerializerContext::New(const FunctionCallbackInfo<Value>& args) { | 171 | 171 | Environment* env = Environment::GetCurrent(args); | | 172 | + if (!args.IsConstructCall()) { | | 173 | + return THROW_ERR_CONSTRUCT_CALL_REQUIRED( | | 174 | + env, "Class constructor Serializer cannot be invoked without 'new'"); | | 175 | + } | 172 | 176 |
| 173 | 177 | new SerializerContext(env, args.This()); | 174 | 178 | } | @@ -319,6 +323,10 @@ MaybeLocal<Object> DeserializerContext::ReadHostObject(Isolate* isolate) { | 319 | 323 |
| 320 | 324 | void DeserializerContext::New(const FunctionCallbackInfo<Value>& args) { | 321 | 325 | Environment* env = Environment::GetCurrent(args); | | 326 | + if (!args.IsConstructCall()) { | | 327 | + return THROW_ERR_CONSTRUCT_CALL_REQUIRED( | | 328 | + env, "Class constructor Deserializer cannot be invoked without 'new'"); | | 329 | + } | 322 | 330 |
| 323 | 331 | if (!args[0]->IsArrayBufferView()) { | 324 | 332 | return node::THROW_ERR_INVALID_ARG_TYPE( | @@ -470,6 +478,7 @@ void Initialize(Local<Object> target, | 470 | 478 | Local<String> serializerString = | 471 | 479 | FIXED_ONE_BYTE_STRING(env->isolate(), "Serializer"); | 472 | 480 | ser->SetClassName(serializerString); | | 481 | + ser->ReadOnlyPrototype(); | 473 | 482 | target->Set(env->context(), | 474 | 483 | serializerString, | 475 | 484 | ser->GetFunction(env->context()).ToLocalChecked()).Check(); | @@ -496,6 +505,8 @@ void Initialize(Local<Object> target, | 496 | 505 |
| 497 | 506 | Local<String> deserializerString = | 498 | 507 | FIXED_ONE_BYTE_STRING(env->isolate(), "Deserializer"); | | 508 | + des->SetLength(1); | | 509 | + des->ReadOnlyPrototype(); | 499 | 510 | des->SetClassName(deserializerString); | 500 | 511 | target->Set(env->context(), | 501 | 512 | deserializerString, | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-v8-serdes.js +10-7Lines changed: 10 additions & 7 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -25,11 +25,6 @@ const objects = [ | 25 | 25 |
| 26 | 26 | const hostObject = new (internalBinding('js_stream').JSStream)(); | 27 | 27 |
| 28 | | -const serializerTypeError = | 29 | | - /^TypeError: Class constructor Serializer cannot be invoked without 'new'$/; | 30 | | -const deserializerTypeError = | 31 | | - /^TypeError: Class constructor Deserializer cannot be invoked without 'new'$/; | 32 | | - | 33 | 28 | { | 34 | 29 | const ser = new v8.DefaultSerializer(); | 35 | 30 | ser.writeHeader(); | @@ -186,8 +181,16 @@ const deserializerTypeError = | 186 | 181 | } | 187 | 182 |
| 188 | 183 | { | 189 | | - assert.throws(v8.Serializer, serializerTypeError); | 190 | | - assert.throws(v8.Deserializer, deserializerTypeError); | | 184 | + assert.throws(() => v8.Serializer(), { | | 185 | + constructor: TypeError, | | 186 | + message: "Class constructor Serializer cannot be invoked without 'new'", | | 187 | + code: 'ERR_CONSTRUCT_CALL_REQUIRED' | | 188 | + }); | | 189 | + assert.throws(() => v8.Deserializer(), { | | 190 | + constructor: TypeError, | | 191 | + message: "Class constructor Deserializer cannot be invoked without 'new'", | | 192 | + code: 'ERR_CONSTRUCT_CALL_REQUIRED' | | 193 | + }); | 191 | 194 | } | 192 | 195 |
| 193 | 196 |
| |
|