Hide Image.prototype.source (#1247)

* Hide Image.prototype.source * Add NAN_METHOD
Automattic · Sep 19, 2018 · 1046abf · 1046abf
1 parent 5301420
commit 1046abf
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -88,6 +88,7 @@ canvas.createJPEGStream() // new
    example, provide an error with code 'ENOENT' if setting `img.src` to a path
    that does not exist.)
  * Support reading CMYK, YCCK JPEGs.
+ * Hide `Image.prototype.source`
 
 ### Added
  * Prebuilds (#992) with different libc versions to the prebuilt binary (#1140)

diff --git a/lib/image.js b/lib/image.js
@@ -15,6 +15,13 @@ const Image = module.exports = bindings.Image
 const http = require("http")
 const https = require("https")
 
+const proto = Image.prototype;
+const _getSource = proto.getSource;
+const _setSource = proto.setSource;
+
+delete proto.getSource;
+delete proto.setSource;
+
 Object.defineProperty(Image.prototype, 'src', {
   /**
    * src setter. Valid values:
@@ -33,8 +40,7 @@ Object.defineProperty(Image.prototype, 'src', {
         // 'base64' must come before the comma
         const isBase64 = val.lastIndexOf('base64', commaI)
         const content = val.slice(commaI + 1)
-        this.source = Buffer.from(content, isBase64 ? 'base64' : 'utf8')
-        this._originalSource = val
+        setSource(this, Buffer.from(content, isBase64 ? 'base64' : 'utf8'), val);
       } else if (/^\s*https?:\/\//.test(val)) { // remote URL
         const onerror = err => {
           if (typeof this.onerror === 'function') {
@@ -52,23 +58,20 @@ Object.defineProperty(Image.prototype, 'src', {
           const buffers = []
           res.on('data', buffer => buffers.push(buffer))
           res.on('end', () => {
-            this.source = Buffer.concat(buffers)
-            this._originalSource = undefined
+            setSource(this, Buffer.concat(buffers));
           })
         }).on('error', onerror)
       } else { // local file path assumed
-        this.source = val
-        this._originalSource = undefined
+        setSource(this, val);
       }
     } else if (Buffer.isBuffer(val)) {
-      this.source = val
-      this._originalSource = undefined
+      setSource(this, val);
     }
   },
 
   get() {
     // TODO https://github.com/Automattic/node-canvas/issues/118
-    return this._originalSource || this.source;
+    return getSource(this);
   },
 
   configurable: true
@@ -90,3 +93,12 @@ Image.prototype.inspect = function(){
     + (this.complete ? ' complete' : '')
     + ']';
 };
+
+function getSource(img){
+  return img._originalSource || _getSource.call(img);
+}
+
+function setSource(img, src, origSrc){
+  _setSource.call(img, src);
+  img._originalSource = origSrc;
+}
diff --git a/src/Image.cc b/src/Image.cc
@@ -56,12 +56,14 @@ Image::Initialize(Nan::ADDON_REGISTER_FUNCTION_ARGS_TYPE target) {
 
   // Prototype
   Local<ObjectTemplate> proto = ctor->PrototypeTemplate();
-  SetProtoAccessor(proto, Nan::New("source").ToLocalChecked(), GetSource, SetSource, ctor);
   SetProtoAccessor(proto, Nan::New("complete").ToLocalChecked(), GetComplete, NULL, ctor);
   SetProtoAccessor(proto, Nan::New("width").ToLocalChecked(), GetWidth, SetWidth, ctor);
   SetProtoAccessor(proto, Nan::New("height").ToLocalChecked(), GetHeight, SetHeight, ctor);
   SetProtoAccessor(proto, Nan::New("naturalWidth").ToLocalChecked(), GetNaturalWidth, NULL, ctor);
   SetProtoAccessor(proto, Nan::New("naturalHeight").ToLocalChecked(), GetNaturalHeight, NULL, ctor);
+
+  Nan::SetMethod(proto, "getSource", GetSource);
+  Nan::SetMethod(proto, "setSource", SetSource);
 #if CAIRO_VERSION_MINOR >= 10
   SetProtoAccessor(proto, Nan::New("dataMode").ToLocalChecked(), GetDataMode, SetDataMode, ctor);
   ctor->Set(Nan::New("MODE_IMAGE").ToLocalChecked(), Nan::New<Number>(DATA_IMAGE));
@@ -182,7 +184,7 @@ NAN_SETTER(Image::SetHeight) {
  * Get src path.
  */
 
-NAN_GETTER(Image::GetSource) {
+NAN_METHOD(Image::GetSource){
   Image *img = Nan::ObjectWrap::Unwrap<Image>(info.This());
   info.GetReturnValue().Set(Nan::New<String>(img->filename ? img->filename : "").ToLocalChecked());
 }
@@ -222,10 +224,12 @@ Image::clearData() {
  * Set src path.
  */
 
-NAN_SETTER(Image::SetSource) {
+NAN_METHOD(Image::SetSource){
   Image *img = Nan::ObjectWrap::Unwrap<Image>(info.This());
   cairo_status_t status = CAIRO_STATUS_READ_ERROR;
 
+  Local<Value> value = info[0];
+
   img->clearData();
   // Clear errno in case some unrelated previous syscall failed
   errno = 0;

diff --git a/src/Image.h b/src/Image.h
@@ -45,17 +45,17 @@ class Image: public Nan::ObjectWrap {
     static Nan::Persistent<FunctionTemplate> constructor;
     static void Initialize(Nan::ADDON_REGISTER_FUNCTION_ARGS_TYPE target);
     static NAN_METHOD(New);
-    static NAN_GETTER(GetSource);
     static NAN_GETTER(GetComplete);
     static NAN_GETTER(GetWidth);
     static NAN_GETTER(GetHeight);
     static NAN_GETTER(GetNaturalWidth);
     static NAN_GETTER(GetNaturalHeight);
     static NAN_GETTER(GetDataMode);
-    static NAN_SETTER(SetSource);
     static NAN_SETTER(SetDataMode);
     static NAN_SETTER(SetWidth);
     static NAN_SETTER(SetHeight);
+    static NAN_METHOD(GetSource);
+    static NAN_METHOD(SetSource);
     inline uint8_t *data(){ return cairo_image_surface_get_data(_surface); }
     inline int stride(){ return cairo_image_surface_get_stride(_surface); }
     static int isPNG(uint8_t *data);

diff --git a/test/image.test.js b/test/image.test.js
@@ -268,4 +268,11 @@ describe('Image', function () {
 
     return Promise.all(corruptSources.map(src => loadImage(src).catch(() => null)))
   })
+
+  it('does not contain `source` property', function () {
+    var keys = Reflect.ownKeys(Image.prototype);
+    assert.ok(!keys.includes('source'));
+    assert.ok(!keys.includes('getSource'));
+    assert.ok(!keys.includes('setSource'));
+  });
 })