From fdcfe9911c1832b945e15cd25effe9fd2f78db19 Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Wed, 22 Mar 2017 05:24:51 -0500 Subject: [PATCH] fs: avoid circular dependency in SyncWriteStream Previously, there was a circular dependency on the public fs module in SyncWriteStream. Moving the implementations of fs.writeSync and fs.closeSync to internal/fs allows us to avoid that circular dependency. Fixes: https://github.com/nodejs/node/issues/11257 --- lib/fs.js | 23 ++++----------------- lib/internal/fs.js | 32 +++++++++++++++++++++++++---- test/parallel/test-stdin-require.js | 27 ++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 23 deletions(-) create mode 100644 test/parallel/test-stdin-require.js diff --git a/lib/fs.js b/lib/fs.js index 8a028bf79943e7..77126a07235bb1 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -42,6 +42,8 @@ const internalURL = require('internal/url'); const internalUtil = require('internal/util'); const assertEncoding = internalFS.assertEncoding; const stringToFlags = internalFS.stringToFlags; +const writeSync = internalFS.writeSync; +const closeSync = internalFS.closeSync; const getPathFromURL = internalURL.getPathFromURL; const { StorageObject } = require('internal/querystring'); @@ -600,9 +602,7 @@ fs.close = function(fd, callback) { binding.close(fd, req); }; -fs.closeSync = function(fd) { - return binding.close(fd); -}; +fs.closeSync = closeSync; function modeNum(m, def) { if (typeof m === 'number') @@ -710,22 +710,7 @@ fs.write = function(fd, buffer, offset, length, position, callback) { // fs.writeSync(fd, buffer[, offset[, length[, position]]]); // OR // fs.writeSync(fd, string[, position[, encoding]]); -fs.writeSync = function(fd, buffer, offset, length, position) { - if (isUint8Array(buffer)) { - if (position === undefined) - position = null; - if (typeof offset !== 'number') - offset = 0; - if (typeof length !== 'number') - length = buffer.length - offset; - return binding.writeBuffer(fd, buffer, offset, length, position); - } - if (typeof buffer !== 'string') - buffer += ''; - if (offset === undefined) - offset = null; - return binding.writeString(fd, buffer, offset, length, position); -}; +fs.writeSync = writeSync; fs.rename = function(oldPath, newPath, callback) { callback = makeCallback(callback); diff --git a/lib/internal/fs.js b/lib/internal/fs.js index 97a4c16aed19b5..cce4d199c0670f 100644 --- a/lib/internal/fs.js +++ b/lib/internal/fs.js @@ -2,9 +2,10 @@ const Buffer = require('buffer').Buffer; const Writable = require('stream').Writable; -const fs = require('fs'); const util = require('util'); const constants = process.binding('constants').fs; +const binding = process.binding('fs'); +const { isUint8Array } = process.binding('util'); const O_APPEND = constants.O_APPEND | 0; const O_CREAT = constants.O_CREAT | 0; @@ -54,6 +55,27 @@ function stringToFlags(flag) { throw new Error('Unknown file open flag: ' + flag); } +function writeSync(fd, buffer, offset, length, position) { + if (isUint8Array(buffer)) { + if (position === undefined) + position = null; + if (typeof offset !== 'number') + offset = 0; + if (typeof length !== 'number') + length = buffer.length - offset; + return binding.writeBuffer(fd, buffer, offset, length, position); + } + if (typeof buffer !== 'string') + buffer += ''; + if (offset === undefined) + offset = null; + return binding.writeString(fd, buffer, offset, length, position); +} + +function closeSync(fd) { + return binding.close(fd); +} + // Temporary hack for process.stdout and process.stderr when piped to files. function SyncWriteStream(fd, options) { Writable.call(this); @@ -70,7 +92,7 @@ function SyncWriteStream(fd, options) { util.inherits(SyncWriteStream, Writable); SyncWriteStream.prototype._write = function(chunk, encoding, cb) { - fs.writeSync(this.fd, chunk, 0, chunk.length); + writeSync(this.fd, chunk, 0, chunk.length); cb(); return true; }; @@ -80,7 +102,7 @@ SyncWriteStream.prototype._destroy = function() { return; if (this.autoClose) - fs.closeSync(this.fd); + closeSync(this.fd); this.fd = null; return true; @@ -97,5 +119,7 @@ module.exports = { assertEncoding, stringToFlags, SyncWriteStream, - realpathCacheKey: Symbol('realpathCacheKey') + realpathCacheKey: Symbol('realpathCacheKey'), + writeSync, + closeSync }; diff --git a/test/parallel/test-stdin-require.js b/test/parallel/test-stdin-require.js new file mode 100644 index 00000000000000..ba7dc1c904979b --- /dev/null +++ b/test/parallel/test-stdin-require.js @@ -0,0 +1,27 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const execSync = require('child_process').execSync; +const fs = require('fs'); +const path = require('path'); + +// Prevents a regression where redirecting stderr and receiving input script +// via stdin works properly. + +common.refreshTmpDir(); +const filename = path.join(common.fixturesDir, 'baz.js'); +const out = path.join(common.tmpDir, 'js.out'); +const bin = process.execPath; +const input = `require('${filename}'); console.log('PASS');`; +const cmd = common.isWindows ? + `echo "${input}" | ${bin} > ${out} 2>&1` : + `echo "${input}" | ${bin} &> ${out}`; + +// This will throw if internal/fs has a circular dependency. +assert.strictEqual(execSync(cmd).toString(), ''); + +const result = fs.readFileSync(out, 'utf8').trim(); +assert.strictEqual(result, 'PASS'); + +fs.unlinkSync(out);