Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs.*Stream classes are (inadvertantly?) monkey-patched in current versions #170

Closed
coreyfarrell opened this issue Aug 7, 2019 · 0 comments · May be fixed by #173
Closed

fs.*Stream classes are (inadvertantly?) monkey-patched in current versions #170

coreyfarrell opened this issue Aug 7, 2019 · 0 comments · May be fixed by #173

Comments

@coreyfarrell
Copy link
Collaborator

fs is cloned by copying property descriptors to a new object. We replace the stream classes:

fs.FileReadStream = ReadStream;  // Legacy name.
fs.FileWriteStream = WriteStream;  // Legacy name.

And later:

fs.ReadStream = ReadStream
fs.WriteStream = WriteStream

Unfortunately these classes are defined in the node.js fs module using property getter/setter functions. The getter/setter from the fs module are copied to the cloned object, so we are actually updating the internal fs variable.

The fix for this is to set the updated values using Object.defineProperty with an updated get/set mimicking the core fs module but pointing to a gfs variable. Not sure if this would be considered a breaking change for gfs? On specific way this can be a real issue is fs.WriteStream.open does not honor the autoClose: false option which was added in node.js 5.

Failing test-case:

'use strict';

const fs = require('fs');
const t = require('tap');

// Save originals before loading graceful-fs
const names = ['ReadStream', 'WriteStream', 'FileReadStream', 'FileWriteStream'];
const orig = names.reduce((orig, name) => Object.assign(orig, {[name]: fs[name]}), {});

const gfs = require('graceful-fs');

if (names.some(name => gfs[name] === orig[name])) {
  t.fail('This test cannot be run under nyc');
  process.exit(1);
}

names.forEach(name => {
  t.ok(fs[name] === orig[name], `fs.${name} unchanged`);
});

fs.createReadStream and fs.createWriteStream are not monkey-patched like this but they are effected as they use fs.ReadStream and fs.WriteStream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant