Skip to content

Commit a8f4289

Browse files
zbjornsonzbjornson
committed
Fix assertion failure from accessors
Accessors should not attempt to unwrap the non-existent 'this' when accessed via the prototype. Fixes Automattic#803 Fixes Automattic#847 Fixes Automattic#885 Fixes nodejs/node#15099
1 parent aa52bcd commit a8f4289

8 files changed

+90
-32
lines changed

‎src/Canvas.cc

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <cairo-pdf.h>
1414
#include <cairo-svg.h>
1515

16+
#include "Util.h"
1617
#include "Canvas.h"
1718
#include "PNG.h"
1819
#include "CanvasRenderingContext2d.h"
@@ -51,16 +52,17 @@ Canvas::Initialize(Nan::ADDON_REGISTER_FUNCTION_ARGS_TYPE target) {
5152

5253
// Prototype
5354
Local<ObjectTemplate> proto = ctor->PrototypeTemplate();
55+
proto->SetInternalFieldCount(1);
5456
Nan::SetPrototypeMethod(ctor, "toBuffer", ToBuffer);
5557
Nan::SetPrototypeMethod(ctor, "streamPNGSync", StreamPNGSync);
5658
Nan::SetPrototypeMethod(ctor, "streamPDFSync", StreamPDFSync);
5759
#ifdef HAVE_JPEG
5860
Nan::SetPrototypeMethod(ctor, "streamJPEGSync", StreamJPEGSync);
5961
#endif
60-
Nan::SetAccessor(proto, Nan::New("type").ToLocalChecked(), GetType);
61-
Nan::SetAccessor(proto, Nan::New("stride").ToLocalChecked(), GetStride);
62-
Nan::SetAccessor(proto, Nan::New("width").ToLocalChecked(), GetWidth, SetWidth);
63-
Nan::SetAccessor(proto, Nan::New("height").ToLocalChecked(), GetHeight, SetHeight);
62+
SetProtoAccessor(proto, Nan::New("type").ToLocalChecked(), GetType, NULL, ctor);
63+
SetProtoAccessor(proto, Nan::New("stride").ToLocalChecked(), GetStride, NULL, ctor);
64+
SetProtoAccessor(proto, Nan::New("width").ToLocalChecked(), GetWidth, SetWidth, ctor);
65+
SetProtoAccessor(proto, Nan::New("height").ToLocalChecked(), GetHeight, SetHeight, ctor);
6466

6567
Nan::SetTemplate(proto, "PNG_NO_FILTERS", Nan::New<Uint32>(PNG_NO_FILTERS));
6668
Nan::SetTemplate(proto, "PNG_FILTER_NONE", Nan::New<Uint32>(PNG_FILTER_NONE));
@@ -755,7 +757,9 @@ Canvas::ResolveFontDescription(const PangoFontDescription *desc) {
755757

756758
// Avoid sending duplicate SFNT font names due to a bug in Pango for macOS:
757759
// https://bugzilla.gnome.org/show_bug.cgi?id=762873
758-
if (g_hash_table_add(seen_families, name)) {
760+
bool existed = g_hash_table_lookup(seen_families, name) == NULL;
761+
g_hash_table_replace(seen_families, name, name);
762+
if (existed) {
759763
if (renamed_families->len) g_string_append(renamed_families, ",");
760764
g_string_append(renamed_families, name);
761765
}

‎src/CanvasRenderingContext2d.cc

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <algorithm>
1313
#include <string>
1414

15+
#include "Util.h"
1516
#include "Canvas.h"
1617
#include "Point.h"
1718
#include "Image.h"
@@ -86,6 +87,7 @@ Context2d::Initialize(Nan::ADDON_REGISTER_FUNCTION_ARGS_TYPE target) {
8687

8788
// Prototype
8889
Local<ObjectTemplate> proto = ctor->PrototypeTemplate();
90+
proto->SetInternalFieldCount(1);
8991
Nan::SetPrototypeMethod(ctor, "drawImage", DrawImage);
9092
Nan::SetPrototypeMethod(ctor, "putImageData", PutImageData);
9193
Nan::SetPrototypeMethod(ctor, "getImageData", GetImageData);
@@ -126,24 +128,24 @@ Context2d::Initialize(Nan::ADDON_REGISTER_FUNCTION_ARGS_TYPE target) {
126128
Nan::SetPrototypeMethod(ctor, "_setStrokePattern", SetStrokePattern);
127129
Nan::SetPrototypeMethod(ctor, "_setTextBaseline", SetTextBaseline);
128130
Nan::SetPrototypeMethod(ctor, "_setTextAlignment", SetTextAlignment);
129-
Nan::SetAccessor(proto, Nan::New("pixelFormat").ToLocalChecked(), GetFormat);
130-
Nan::SetAccessor(proto, Nan::New("patternQuality").ToLocalChecked(), GetPatternQuality, SetPatternQuality);
131-
Nan::SetAccessor(proto, Nan::New("globalCompositeOperation").ToLocalChecked(), GetGlobalCompositeOperation, SetGlobalCompositeOperation);
132-
Nan::SetAccessor(proto, Nan::New("globalAlpha").ToLocalChecked(), GetGlobalAlpha, SetGlobalAlpha);
133-
Nan::SetAccessor(proto, Nan::New("shadowColor").ToLocalChecked(), GetShadowColor, SetShadowColor);
134-
Nan::SetAccessor(proto, Nan::New("fillColor").ToLocalChecked(), GetFillColor);
135-
Nan::SetAccessor(proto, Nan::New("strokeColor").ToLocalChecked(), GetStrokeColor);
136-
Nan::SetAccessor(proto, Nan::New("miterLimit").ToLocalChecked(), GetMiterLimit, SetMiterLimit);
137-
Nan::SetAccessor(proto, Nan::New("lineWidth").ToLocalChecked(), GetLineWidth, SetLineWidth);
138-
Nan::SetAccessor(proto, Nan::New("lineCap").ToLocalChecked(), GetLineCap, SetLineCap);
139-
Nan::SetAccessor(proto, Nan::New("lineJoin").ToLocalChecked(), GetLineJoin, SetLineJoin);
140-
Nan::SetAccessor(proto, Nan::New("lineDashOffset").ToLocalChecked(), GetLineDashOffset, SetLineDashOffset);
141-
Nan::SetAccessor(proto, Nan::New("shadowOffsetX").ToLocalChecked(), GetShadowOffsetX, SetShadowOffsetX);
142-
Nan::SetAccessor(proto, Nan::New("shadowOffsetY").ToLocalChecked(), GetShadowOffsetY, SetShadowOffsetY);
143-
Nan::SetAccessor(proto, Nan::New("shadowBlur").ToLocalChecked(), GetShadowBlur, SetShadowBlur);
144-
Nan::SetAccessor(proto, Nan::New("antialias").ToLocalChecked(), GetAntiAlias, SetAntiAlias);
145-
Nan::SetAccessor(proto, Nan::New("textDrawingMode").ToLocalChecked(), GetTextDrawingMode, SetTextDrawingMode);
146-
Nan::SetAccessor(proto, Nan::New("filter").ToLocalChecked(), GetFilter, SetFilter);
131+
SetProtoAccessor(proto, Nan::New("pixelFormat").ToLocalChecked(), GetFormat, NULL, ctor);
132+
SetProtoAccessor(proto, Nan::New("patternQuality").ToLocalChecked(), GetPatternQuality, SetPatternQuality, ctor);
133+
SetProtoAccessor(proto, Nan::New("globalCompositeOperation").ToLocalChecked(), GetGlobalCompositeOperation, SetGlobalCompositeOperation, ctor);
134+
SetProtoAccessor(proto, Nan::New("globalAlpha").ToLocalChecked(), GetGlobalAlpha, SetGlobalAlpha, ctor);
135+
SetProtoAccessor(proto, Nan::New("shadowColor").ToLocalChecked(), GetShadowColor, SetShadowColor, ctor);
136+
SetProtoAccessor(proto, Nan::New("fillColor").ToLocalChecked(), GetFillColor, NULL, ctor);
137+
SetProtoAccessor(proto, Nan::New("strokeColor").ToLocalChecked(), GetStrokeColor, NULL, ctor);
138+
SetProtoAccessor(proto, Nan::New("miterLimit").ToLocalChecked(), GetMiterLimit, SetMiterLimit, ctor);
139+
SetProtoAccessor(proto, Nan::New("lineWidth").ToLocalChecked(), GetLineWidth, SetLineWidth, ctor);
140+
SetProtoAccessor(proto, Nan::New("lineCap").ToLocalChecked(), GetLineCap, SetLineCap, ctor);
141+
SetProtoAccessor(proto, Nan::New("lineJoin").ToLocalChecked(), GetLineJoin, SetLineJoin, ctor);
142+
SetProtoAccessor(proto, Nan::New("lineDashOffset").ToLocalChecked(), GetLineDashOffset, SetLineDashOffset, ctor);
143+
SetProtoAccessor(proto, Nan::New("shadowOffsetX").ToLocalChecked(), GetShadowOffsetX, SetShadowOffsetX, ctor);
144+
SetProtoAccessor(proto, Nan::New("shadowOffsetY").ToLocalChecked(), GetShadowOffsetY, SetShadowOffsetY, ctor);
145+
SetProtoAccessor(proto, Nan::New("shadowBlur").ToLocalChecked(), GetShadowBlur, SetShadowBlur, ctor);
146+
SetProtoAccessor(proto, Nan::New("antialias").ToLocalChecked(), GetAntiAlias, SetAntiAlias, ctor);
147+
SetProtoAccessor(proto, Nan::New("textDrawingMode").ToLocalChecked(), GetTextDrawingMode, SetTextDrawingMode, ctor);
148+
SetProtoAccessor(proto, Nan::New("filter").ToLocalChecked(), GetFilter, SetFilter, ctor);
147149
Nan::Set(target, Nan::New("CanvasRenderingContext2d").ToLocalChecked(), ctor->GetFunction());
148150
}
149151

‎src/Image.cc

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
// Copyright (c) 2010 LearnBoost <[email protected]>
55
//
66

7+
#include "Util.h"
78
#include "Canvas.h"
89
#include "Image.h"
910
#include <stdlib.h>
@@ -45,14 +46,15 @@ Image::Initialize(Nan::ADDON_REGISTER_FUNCTION_ARGS_TYPE target) {
4546

4647
// Prototype
4748
Local<ObjectTemplate> proto = ctor->PrototypeTemplate();
48-
Nan::SetAccessor(proto, Nan::New("source").ToLocalChecked(), GetSource, SetSource);
49-
Nan::SetAccessor(proto, Nan::New("complete").ToLocalChecked(), GetComplete);
50-
Nan::SetAccessor(proto, Nan::New("width").ToLocalChecked(), GetWidth);
51-
Nan::SetAccessor(proto, Nan::New("height").ToLocalChecked(), GetHeight);
52-
Nan::SetAccessor(proto, Nan::New("onload").ToLocalChecked(), GetOnload, SetOnload);
53-
Nan::SetAccessor(proto, Nan::New("onerror").ToLocalChecked(), GetOnerror, SetOnerror);
49+
proto->SetInternalFieldCount(1);
50+
SetProtoAccessor(proto, Nan::New("source").ToLocalChecked(), GetSource, SetSource, ctor);
51+
SetProtoAccessor(proto, Nan::New("complete").ToLocalChecked(), GetComplete, NULL, ctor);
52+
SetProtoAccessor(proto, Nan::New("width").ToLocalChecked(), GetWidth, NULL, ctor);
53+
SetProtoAccessor(proto, Nan::New("height").ToLocalChecked(), GetHeight, NULL, ctor);
54+
SetProtoAccessor(proto, Nan::New("onload").ToLocalChecked(), GetOnload, SetOnload, ctor);
55+
SetProtoAccessor(proto, Nan::New("onerror").ToLocalChecked(), GetOnerror, SetOnerror, ctor);
5456
#if CAIRO_VERSION_MINOR >= 10
55-
Nan::SetAccessor(proto, Nan::New("dataMode").ToLocalChecked(), GetDataMode, SetDataMode);
57+
SetProtoAccessor(proto, Nan::New("dataMode").ToLocalChecked(), GetDataMode, SetDataMode, ctor);
5658
ctor->Set(Nan::New("MODE_IMAGE").ToLocalChecked(), Nan::New<Number>(DATA_IMAGE));
5759
ctor->Set(Nan::New("MODE_MIME").ToLocalChecked(), Nan::New<Number>(DATA_MIME));
5860
#endif

‎src/ImageData.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// Copyright (c) 2010 LearnBoost <[email protected]>
66
//
77

8+
#include "Util.h"
89
#include "ImageData.h"
910

1011
Nan::Persistent<FunctionTemplate> ImageData::constructor;
@@ -25,8 +26,9 @@ ImageData::Initialize(Nan::ADDON_REGISTER_FUNCTION_ARGS_TYPE target) {
2526

2627
// Prototype
2728
Local<ObjectTemplate> proto = ctor->PrototypeTemplate();
28-
Nan::SetAccessor(proto, Nan::New("width").ToLocalChecked(), GetWidth);
29-
Nan::SetAccessor(proto, Nan::New("height").ToLocalChecked(), GetHeight);
29+
proto->SetInternalFieldCount(1);
30+
SetProtoAccessor(proto, Nan::New("width").ToLocalChecked(), GetWidth, NULL, ctor);
31+
SetProtoAccessor(proto, Nan::New("height").ToLocalChecked(), GetHeight, NULL, ctor);
3032
Nan::Set(target, Nan::New("ImageData").ToLocalChecked(), ctor->GetFunction());
3133
}
3234

‎src/Util.h

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#include <v8.h>
2+
#include <nan.h>
3+
4+
// Wrapper around Nan::SetAccessor that makes it easier to change the last
5+
// argument (signature). Getters/setters must be accessed only when there is
6+
// actually an instance, i.e. MyClass.prototype.getter1 should not try to
7+
// unwrap the non-existent 'this'. See #803, #847, #885, nodejs/node#15099, ...
8+
inline void SetProtoAccessor(
9+
v8::Local<v8::ObjectTemplate> tpl,
10+
v8::Local<v8::String> name,
11+
Nan::GetterCallback getter,
12+
Nan::SetterCallback setter,
13+
v8::Local<v8::FunctionTemplate> ctor
14+
) {
15+
Nan::SetAccessor(
16+
tpl,
17+
name,
18+
getter,
19+
setter,
20+
v8::Local<v8::Value>(),
21+
v8::DEFAULT,
22+
v8::None,
23+
v8::AccessorSignature::New(v8::Isolate::GetCurrent(), ctor)
24+
);
25+
}

‎test/canvas.test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,15 @@ const os = require('os')
1717
const Readable = require('stream').Readable
1818

1919
describe('Canvas', function () {
20+
it('Prototype and ctor are well-shaped, don\'t hit asserts on accessors (GH-803)', function () {
21+
const Canvas = require('../').Canvas;
22+
var c = new Canvas(10, 10);
23+
assert.throws(function () { Canvas.prototype.width; }, /incompatible receiver/);
24+
assert(!c.hasOwnProperty('width'));
25+
assert('width' in c);
26+
assert(Canvas.prototype.hasOwnProperty('width'));
27+
});
28+
2029
it('.parseFont()', function () {
2130
var tests = [
2231
'20px Arial'

‎test/image.test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88

99
const loadImage = require('../').loadImage
10+
const Image = require('../').Image;
1011

1112
const assert = require('assert')
1213
const assertRejects = require('assert-rejects')
@@ -16,6 +17,14 @@ const png_clock = `${__dirname}/fixtures/clock.png`
1617
const jpg_face = `${__dirname}/fixtures/face.jpeg`
1718

1819
describe('Image', function () {
20+
it('Prototype and ctor are well-shaped, don\'t hit asserts on accessors (GH-803)', function () {
21+
var img = new Image();
22+
assert.throws(function () { Image.prototype.width; }, /incompatible receiver/);
23+
assert(!img.hasOwnProperty('width'));
24+
assert('width' in img);
25+
assert(Image.prototype.hasOwnProperty('width'));
26+
});
27+
1928
it('loads JPEG image', function () {
2029
return loadImage(jpg_face).then((img) => {
2130
assert.strictEqual(img.onerror, null)

‎test/imageData.test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,15 @@
33
'use strict'
44

55
const createImageData = require('../').createImageData
6+
const ImageData = require('../').ImageData;
67

78
const assert = require('assert')
89

910
describe('ImageData', function () {
11+
it('Prototype and ctor are well-shaped, don\'t hit asserts on accessors (GH-803)', function () {
12+
assert.throws(function () { ImageData.prototype.width; }, /incompatible receiver/);
13+
});
14+
1015
it('should throw with invalid numeric arguments', function () {
1116
assert.throws(() => { createImageData(0, 0) }, /width is zero/)
1217
assert.throws(() => { createImageData(1, 0) }, /height is zero/)

0 commit comments

Comments
 (0)