node-mcrypt icon indicating copy to clipboard operation
node-mcrypt copied to clipboard

Core dump when invoked as a function

Open pluma opened this issue 7 years ago • 1 comments

This isn't a massive problem but it would be nice if this were handled more gracefully:

> new MCrypt() // fails as expected
Error: MCrypt module could not open
> MCrypt() // results in core dump
FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal.
 1: 0x8daaa0 node::Abort() [node]
 2: 0x8daaec  [node]
 3: 0xad71aa v8::Utils::ReportApiFailure(char const*, char const*) [node]
 4: 0x7f1ce95bb02d MCrypt::New(Nan::FunctionCallbackInfo<v8::Value> const&) [/home/pluma/projects/test/node_modules/mcrypt/build/Release/mcrypt.node]
 5: 0x7f1ce95b7487  [/home/pluma/projects/test/node_modules/mcrypt/build/Release/mcrypt.node]
 6: 0xb5faef  [node]
 7: 0xb60659 v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) [node]
 8: 0x27bebec5be1d
Aborted (core dumped)

The problem is probably that in a regular function invocation this is left undefined. I think newer versions of V8 (i.e. ever since classes are supported) allow distinguishing between regular functions and constructors so this could me adjusted to result in a TypeError: Constructor MCrypt requires 'new' instead.

pluma avatar Nov 23 '18 16:11 pluma

There is a check for non-construct calls of constructor method. https://github.com/tugrul/node-mcrypt/blob/master/src/mcrypt.cc#L99-L103

Probably some changes in new v8 about this.

So I don't want to make effort about this library because I'm working on a new repository https://github.com/tugrul/cryptian replacement of mcrypt. New library is completed and I'm going to write documentation in a free time.

tugrul avatar Nov 24 '18 12:11 tugrul