hide readResult method (#131)

js/preload/preloadMain.js

@@ -107,7 +107,7 @@ function createAPI() {
          * provides api to allow user to capture portion of screen, see api
          * details in screenSnipper/ScreenSnippet.js
          */
-        ScreenSnippet: remote.require('./screenSnippet/ScreenSnippet.js'),
+        ScreenSnippet: remote.require('./screenSnippet/ScreenSnippet.js').ScreenSnippet,
 
         /**
          * Brings window forward and gives focus.

js/screenSnippet/ScreenSnippet.js

@@ -77,76 +77,80 @@ class ScreenSnippet {
                     // processs was killed, just resolve with no data.
                     resolve();
                 } else {
-                    this._readResult(outputFileName, resolve, reject, error);
+                    readResult.call(this, outputFileName, resolve, reject, error);
                 }
             });
         });
     }
-
-    // private methods below here
-
-    _readResult(outputFileName, resolve, reject, childProcessErr) {
-        fs.readFile(outputFileName, (readErr, data) => {
-            if (readErr) {
-                let returnErr;
-                if (readErr.code === 'ENOENT') {
-                    // no such file exists, user likely aborted
-                    // creating snippet. also include any error when
-                    // creating child process.
-                    returnErr = this._createWarn('file does not exist ' +
-                        childProcessErr);
-                } else {
-                    returnErr = this._createError(readErr + ',' +
-                        childProcessErr);
-                }
-
-                reject(returnErr);
-                return;
-            }
-
-            if (!data) {
-                reject(this._createWarn('no file data provided'));
-                return;
-            }
-
-            try {
-                // convert binary data to base64 encoded string
-                let output = Buffer(data).toString('base64');
-                resolve({
-                    type: 'image/jpg;base64',
-                    data: output
-                });
-            } catch (error) {
-                reject(this._createError(error));
-            }
-            finally {
-                // remove tmp file (async)
-                fs.unlink(outputFileName, function(removeErr) {
-                    // note: node complains if calling async
-                    // func without callback.
-                    if (removeErr) {
-                        log.send(logLevels.ERROR, 'ScreenSnippet: error removing temp snippet file: ' +
-                            outputFileName + ', err:' + removeErr);
-                    }
-                });
-            }
-        });
-
-    }
-
-    /* eslint-disable class-methods-use-this */
-    _createError(msg) {
-        var err = new Error(msg);
-        err.type = 'ERROR';
-        return err;
-    }
-
-    _createWarn(msg) {
-        var err = new Error(msg);
-        err.type = 'WARN';
-        return err;
-    }
-    /* eslint-enable class-methods-use-this */
 }
 
-module.exports = ScreenSnippet;
+// this function was moved outside of class since class is exposed to web
+// client via preload API, we do NOT want web client to be able to call this
+// method - then they could read any file on the disk!
+function readResult(outputFileName, resolve, reject, childProcessErr) {
+    fs.readFile(outputFileName, (readErr, data) => {
+        if (readErr) {
+            let returnErr;
+            if (readErr.code === 'ENOENT') {
+                // no such file exists, user likely aborted
+                // creating snippet. also include any error when
+                // creating child process.
+                returnErr = createWarn('file does not exist ' +
+                    childProcessErr);
+            } else {
+                returnErr = createError(readErr + ',' +
+                    childProcessErr);
+            }
+
+            reject(returnErr);
+            return;
+        }
+
+        if (!data) {
+            reject(createWarn('no file data provided'));
+            return;
+        }
+
+        try {
+            // convert binary data to base64 encoded string
+            let output = Buffer(data).toString('base64');
+            resolve({
+                type: 'image/jpg;base64',
+                data: output
+            });
+        } catch (error) {
+            reject(createError(error));
+        }
+        finally {
+            // remove tmp file (async)
+            fs.unlink(outputFileName, function(removeErr) {
+                // note: node complains if calling async
+                // func without callback.
+                if (removeErr) {
+                    log.send(logLevels.ERROR, 'ScreenSnippet: error removing temp snippet file: ' +
+                        outputFileName + ', err:' + removeErr);
+                }
+            });
+        }
+    });
+}
+
+/* eslint-disable class-methods-use-this */
+function createError(msg) {
+    var err = new Error(msg);
+    err.type = 'ERROR';
+    return err;
+}
+
+function createWarn(msg) {
+    var err = new Error(msg);
+    err.type = 'WARN';
+    return err;
+}
+/* eslint-enable class-methods-use-this */
+
+module.exports = {
+    ScreenSnippet: ScreenSnippet,
+    // note: readResult only exposed for testing purposes
+    readResult: readResult
+}

package.json

@@ -19,7 +19,7 @@
     "prebuild": "npm run lint && npm run test && npm run browserify-preload",
     "rebuild": "electron-rebuild -f",
     "lint": "eslint --ext .js js/",
-    "test": "jest --testPathPattern test",
+    "test": "jest --verbose --testPathPattern test",
     "browserify-preload": "browserify -o js/preload/_preloadMain.js -x electron --insert-global-vars=__filename,__dirname js/preload/preloadMain.js"
   },
   "jest": {

tests/ScreenSnippet.test.js

@@ -1,5 +1,5 @@
 
-const ScreenSnippet = require('../js/screenSnippet/ScreenSnippet.js');
+const { ScreenSnippet, readResult } = require('../js/screenSnippet/ScreenSnippet.js');
 const path = require('path');
 const fs = require('fs');
 const os = require('os');
@@ -71,8 +71,7 @@ describe('Tests for ScreenSnippet', function() {
 
         it('should remove output file after completed', function(done) {
             createTestFile(function(testfileName) {
-                let s = new ScreenSnippet();
+                readResult(testfileName, resolve);
-                s._readResult(testfileName, resolve);
 
                 function resolve() {
                     // should be long enough before file
@@ -88,9 +87,8 @@ describe('Tests for ScreenSnippet', function() {
     });
 
     it('should fail if output file does not exist', function(done) {
-        let s = new ScreenSnippet();
+        let nonExistentFile = 'bogus.jpeg';
-        let nonExistentFile = 'bogus.jpeg'
+        readResult(nonExistentFile, resolve, reject);
-        s._readResult(nonExistentFile, resolve, reject);
 
         function resolve() {
             // shouldn't get here