Skip to content

Commit

Permalink
Fix crash if exception thrown from onload/onerror
Browse files Browse the repository at this point in the history
If a user-provided .onload or .onerror handler on an Image throws an
exception, the Node.js process crashes:

    FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal.
     1: 0x94d778 node::Abort() [node]
     2: 0x94edb1 node::OnFatalError(char const*, char const*) [node]
     3: 0xad5b0d v8::Utils::ReportApiFailure(char const*, char const*) [node]
     4: 0x7f98f47799ae Image::SetSource(Nan::FunctionCallbackInfo<v8::Value> const&) [/home/strager/tmp/Projects/node-canvas/build/Release/canvas.node]
     5: 0x7f98f4774fec  [/home/strager/tmp/Projects/node-canvas/build/Release/canvas.node]
     6: 0xb395b9 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [node]
     7: 0xb39970  [node]
     8: 0xb3a1ea  [node]
     9: 0xb3aa99 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [node]
    10: 0x12e89b9  [node]

Fix this issue by not requiring a return value from the .onload and
.onerror functions.
  • Loading branch information
strager authored and zbjornson committed Feb 28, 2021
1 parent 8cd191c commit 12e671d
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -22,6 +22,7 @@ project adheres to [Semantic Versioning](http://semver.org/).
* Fix benchmark for createPNGStream (#1672)
* Fix dangling reference in BackendOperationNotAvailable exception (#1740)
* Fix always-false comparison warning in Canvas.cc.
* Fix Node.js crash when throwing from an onload or onerror handler.

2.7.0
==================
Expand Down
4 changes: 2 additions & 2 deletions src/Image.cc
Expand Up @@ -271,14 +271,14 @@ NAN_METHOD(Image::SetSource){
argv[0] = Nan::Error(Nan::New(cairo_status_to_string(status)).ToLocalChecked());
}
Local<Context> ctx = Nan::GetCurrentContext();
Nan::Call(onerrorFn.As<Function>(), ctx->Global(), 1, argv).ToLocalChecked();
Nan::Call(onerrorFn.As<Function>(), ctx->Global(), 1, argv);
}
} else {
img->loaded();
Local<Value> onloadFn = Nan::Get(info.This(), Nan::New("onload").ToLocalChecked()).ToLocalChecked();
if (onloadFn->IsFunction()) {
Local<Context> ctx = Nan::GetCurrentContext();
Nan::Call(onloadFn.As<Function>(), ctx->Global(), 0, NULL).ToLocalChecked();
Nan::Call(onloadFn.As<Function>(), ctx->Global(), 0, NULL);
}
}
}
Expand Down
22 changes: 22 additions & 0 deletions test/image.test.js
Expand Up @@ -95,6 +95,28 @@ describe('Image', function () {
img.src = Buffer.from('89504E470D', 'hex')
})

it('propagates exceptions thrown by onload', function () {
class MyError extends Error {}
const img = new Image()
img.onload = () => {
throw new MyError()
}
assert.throws(() => {
img.src = jpg_face
}, MyError);
})

it('propagates exceptions thrown by onerror', function () {
class MyError extends Error {}
const img = new Image()
img.onerror = () => {
throw new MyError()
}
assert.throws(() => {
img.src = Buffer.from('', 'hex')
}, MyError);
})

it('loads SVG data URL base64', function () {
if (!HAVE_SVG) this.skip();
const base64Enc = fs.readFileSync(svg_tree, 'base64')
Expand Down

0 comments on commit 12e671d

Please sign in to comment.