From a786a7d71325445775dd6e663c24ee2f9fa2ba29 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 19 Aug 2019 16:39:56 -0400 Subject: [PATCH] Ensure `fs.Stats` (and other classes on `fs`) are not mutated. `fs.Stats` is a standard prototypal class (e.g. `function Stats() {}`), so it was being wrapped by our `fs` monitoring system. Unfortunately, wrapping `fs.Stats` in a plain function (which did `return original.apply(this, arguments)`) doesn't properly emulate a number of "standard" class behaviors. Specifically: * static methods and properties are lost * accessing `fs.Stats.prototype` does not refer to the original prototype This last item was causing the following error when using `esm`: ``` TypeError: Function.prototype.apply was called on undefined, which is a undefined and not a function at $o (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:224377) at xu (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:226413) at wu (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:227391) at Eu (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:227999) at Module. (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:295976) at n (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:279589) at getFeatures (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/@ember-data/-build-infra/src/features.js:5:33) at Object. (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/@ember-data/-build-infra/src/features.js:31:18) ``` This is because `esm` is _essentially_ doing: ```js let isFile = fs.Stats.prototype.isFile; ``` To stash off the `isFile` implementation for recurring usage. The fact that when wrapped `fs.Stat.prototype.isFile` was undefined caused the error. --- index.js | 10 +++++++++- tests.js | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 26fc68b..3e51090 100644 --- a/index.js +++ b/index.js @@ -13,7 +13,15 @@ let hasActiveInstance = false; class FSMonitor { constructor() { this.state = 'idle'; - this.blacklist = ['createReadStream', 'createWriteStream', 'ReadStream', 'WriteStream']; + this.blacklist = [ + 'createReadStream', + 'createWriteStream', + 'Dirent', + 'FSWatcher', + 'ReadStream', + 'Stats', + 'WriteStream' + ]; } start() { diff --git a/tests.js b/tests.js index 71c2ad6..e10cfee 100644 --- a/tests.js +++ b/tests.js @@ -3,7 +3,8 @@ const FSMonitor = require('./'); const expect = require('chai').expect; const fs = require('fs'); -const originalStatSync = fs.statSync; + +const originalFS = Object.assign({}, fs); describe('FSMonitor', function() { it('will only allow one active instance at a time', function() { @@ -28,17 +29,65 @@ describe('FSMonitor', function() { monitor0.stop(); }); + it('does not mutate the prototype of classes on fs [GH#22]', function() { + let monitor = new FSMonitor(); + + expect(typeof fs.Stats.prototype.isFile).to.equal('function'); + + monitor.start(); + + expect(typeof fs.Stats.prototype.isFile).to.equal('function', 'after updating fs'); + + monitor.stop(); + + expect(typeof fs.Stats.prototype.isFile).to.equal('function'); + }); + + it('avoids mutating known classes on `fs` [GH#22]', function() { + let monitor = new FSMonitor(); + + expect(fs.Stats.prototype.isFile).to.be; + expect(fs.Stats).to.equal(originalFS.Stats); + expect(fs.Dirent).to.equal(originalFS.Dirent); + expect(fs.FSWatcher).to.equal(originalFS.FSWatcher); + expect(fs.FileHandle).to.equal(originalFS.FileHandle); + expect(fs.ReadStream).to.equal(originalFS.ReadStream); + expect(fs.WriteStream).to.equal(originalFS.WriteStream); + + try { + monitor.start(); + + // should not have been changed + expect(fs.Stats).to.equal(originalFS.Stats); + expect(fs.Dirent).to.equal(originalFS.Dirent); + expect(fs.FSWatcher).to.equal(originalFS.FSWatcher); + expect(fs.FileHandle).to.equal(originalFS.FileHandle); + expect(fs.ReadStream).to.equal(originalFS.ReadStream); + expect(fs.WriteStream).to.equal(originalFS.WriteStream); + } finally { + // ensure we stop and detach even if we fail an assertion + monitor.stop(); + } + + expect(fs.Stats).to.equal(originalFS.Stats); + expect(fs.Dirent).to.equal(originalFS.Dirent); + expect(fs.FSWatcher).to.equal(originalFS.FSWatcher); + expect(fs.FileHandle).to.equal(originalFS.FileHandle); + expect(fs.ReadStream).to.equal(originalFS.ReadStream); + expect(fs.WriteStream).to.equal(originalFS.WriteStream); + }); + describe('.prototype.stop', function() { it('restores fs functions to their defaults', function() { let monitor = new FSMonitor(); - expect(fs.statSync).to.equal(originalStatSync); + expect(fs.statSync).to.equal(originalFS.statSync); monitor.start(); - expect(fs.statSync).to.not.equal(originalStatSync); + expect(fs.statSync).to.not.equal(originalFS.statSync); monitor.stop(); - expect(fs.statSync).to.equal(originalStatSync); + expect(fs.statSync).to.equal(originalFS.statSync); }); }); });