diff --git a/src/docker.js b/src/docker.js index e01b92369..6654ee720 100644 --- a/src/docker.js +++ b/src/docker.js @@ -648,12 +648,10 @@ async function update(name, memory, memorySwap) { assert.strictEqual(typeof memory, 'number'); assert.strictEqual(typeof memorySwap, 'number'); - const args = `update --memory ${memory} --memory-swap ${memorySwap} ${name}`.split(' '); // scale back db containers, if possible. this is retried because updating memory constraints can fail // with failed to write to memory.memsw.limit_in_bytes: write /sys/fs/cgroup/memory/docker/xx/memory.memsw.limit_in_bytes: device or resource busy - for (let times = 0; times < 10; times++) { - const [error] = await safe(shell.promises.spawn(`update(${name})`, '/usr/bin/docker', args, { })); + const [error] = await safe(shell.promises.exec(`update(${name})`, `docker update --memory ${memory} --memory-swap ${memorySwap} ${name}`, {})); if (!error) return; await timers.setTimeout(60 * 1000); } diff --git a/src/shell.js b/src/shell.js index cfd540320..a5d613c47 100644 --- a/src/shell.js +++ b/src/shell.js @@ -8,12 +8,11 @@ const assert = require('assert'), util = require('util'); exports = module.exports = { - spawn, - exec, sudo, promises: { exec: util.promisify(exec), + execFile: util.promisify(execFile), spawn: util.promisify(spawn), sudo: util.promisify(sudo) } @@ -21,6 +20,7 @@ exports = module.exports = { const SUDO = '/usr/bin/sudo'; +// default encoding utf8, shell, handles input. full command function exec(tag, cmd, options, callback) { assert.strictEqual(typeof tag, 'string'); assert.strictEqual(typeof cmd, 'string'); @@ -29,6 +29,7 @@ function exec(tag, cmd, options, callback) { debug(`${tag} exec: ${cmd}`); + // https://github.com/nodejs/node/issues/25231 const cp = child_process.exec(cmd, options, function (error, stdout, stderr) { let e = null; if (error) { @@ -48,6 +49,36 @@ function exec(tag, cmd, options, callback) { } } +// no shell, utf8 encoding, separate args +function execFile(tag, file, args, options, callback) { + assert.strictEqual(typeof tag, 'string'); + assert.strictEqual(typeof file, 'string'); + assert(Array.isArray(args)); + assert.strictEqual(typeof options, 'object'); + assert.strictEqual(typeof callback, 'function'); + + debug(`${tag} exec: ${file}`); + + // https://github.com/nodejs/node/issues/25231 + const cp = child_process.execFile(file, args, options, function (error, stdout, stderr) { + let e = null; + if (error) { + e = new BoxError(BoxError.SHELL_ERROR, `${tag} errored with code ${error.code} message ${error.message}`); + e.stdout = stdout; // when promisified, this is the way to get stdout + e.stderr = stderr; // when promisified, this is the way to get stderr + + debug(`${tag}: ${file} with args ${args.join(' ')} errored`, error); + } + + callback(e, stdout); + }); + + if (options.input) { + cp.stdin.write(options.input); + cp.stdin.end(); + } +} + // use this when you are afraid of how arguments will split up function spawn(tag, file, args, options, callback) { assert.strictEqual(typeof tag, 'string'); diff --git a/src/storage/filesystem.js b/src/storage/filesystem.js index bc616fe14..78515ea0f 100644 --- a/src/storage/filesystem.js +++ b/src/storage/filesystem.js @@ -187,7 +187,7 @@ async function copy(apiConfig, oldFilePath, newFilePath, progressCallback) { let cpOptions = ((apiConfig.provider !== PROVIDER_MOUNTPOINT && apiConfig.provider !== PROVIDER_CIFS) || apiConfig.preserveAttributes) ? '-a' : '-dR'; cpOptions += apiConfig.noHardlinks ? '' : 'l'; // this will hardlink backups saving space - const [copyError] = await safe(shell.promises.spawn('copy', '/bin/cp', [ cpOptions, oldFilePath, newFilePath ], { })); + const [copyError] = await safe(shell.promises.execFile('copy', 'cp', [ cpOptions, oldFilePath, newFilePath ], {})); if (copyError) throw new BoxError(BoxError.EXTERNAL_ERROR, copyError.message); } @@ -212,7 +212,7 @@ async function removeDir(apiConfig, pathPrefix, progressCallback) { progressCallback({ message: `Removing directory ${pathPrefix}` }); - const [error] = await safe(shell.promises.spawn('removeDir', '/bin/rm', [ '-rf', pathPrefix ], { })); + const [error] = await safe(shell.promises.execFile('removeDir', 'rm', [ '-rf', pathPrefix ], {})); if (error) throw new BoxError(BoxError.EXTERNAL_ERROR, error.message); } diff --git a/src/test/shell-test.js b/src/test/shell-test.js index 3c8e56bc8..297a49a4c 100644 --- a/src/test/shell-test.js +++ b/src/test/shell-test.js @@ -11,74 +11,62 @@ const BoxError = require('../boxerror.js'), shell = require('../shell.js'); describe('shell', function () { - it('can run valid program', function (done) { - let cp = shell.spawn('test', 'ls', [ '-l' ], { }, function (error) { - expect(cp).to.be.ok(); - expect(error).to.be(null); - done(); + describe('execFile', function () { + it('can run valid program', async function () { + await shell.promises.execFile('test', 'ls', [ '-l' ], {}); + }); + + it('fails on invalid program', async function () { + const [error] = await safe(shell.promises.execFile('test', 'randomprogram', [ ], {})); + expect(error.reason).to.be(BoxError.SHELL_ERROR); + }); + + it('fails on failing program', async function () { + const [error] = await safe(shell.promises.execFile('test', '/usr/bin/false', [ ], {})); + expect(error.reason).to.be(BoxError.SHELL_ERROR); }); }); - it('fails on invalid program', function (done) { - shell.spawn('test', 'randomprogram', [ ], { }, function (error) { - expect(error).to.be.ok(); - done(); + describe('sudo', function () { + it('cannot sudo invalid program', function (done) { + shell.sudo('test', [ 'randomprogram' ], {}, function (error) { + expect(error).to.be.ok(); + done(); + }); + }); + + it('can sudo valid program', function (done) { + let RELOAD_NGINX_CMD = path.join(__dirname, '../src/scripts/restartservice.sh'); + shell.sudo('test', [ RELOAD_NGINX_CMD, 'nginx' ], {}, function (error) { + expect(error).to.be.ok(); + done(); + }); + }); + + it('can run valid program (promises)', async function () { + let RELOAD_NGINX_CMD = path.join(__dirname, '../src/scripts/restartservice.sh'); + await safe(shell.promises.sudo('test', [ RELOAD_NGINX_CMD, 'nginx' ], {})); }); }); - it('fails on failing program', function (done) { - shell.spawn('test', '/usr/bin/false', [ ], { }, function (error) { - expect(error).to.be.ok(); - done(); + describe('exec', function () { + it('exec a valid shell program', async function () { + await shell.promises.exec('test', 'ls -l | wc -c', {}); }); - }); - it('cannot sudo invalid program', function (done) { - shell.sudo('test', [ 'randomprogram' ], {}, function (error) { - expect(error).to.be.ok(); - done(); + it('exec throws for invalid program', async function () { + const [error] = await safe(shell.promises.exec('test', 'cannotexist', {})); + expect(error.reason).to.be(BoxError.SHELL_ERROR); }); - }); - it('can sudo valid program', function (done) { - let RELOAD_NGINX_CMD = path.join(__dirname, '../src/scripts/restartservice.sh'); - shell.sudo('test', [ RELOAD_NGINX_CMD, 'nginx' ], {}, function (error) { - expect(error).to.be.ok(); - done(); + it('exec throws for failed program', async function () { + const [error] = await safe(shell.promises.exec('test', 'false', {})); + expect(error.reason).to.be(BoxError.SHELL_ERROR); }); - }); - it('can run valid program (promises)', async function () { - let RELOAD_NGINX_CMD = path.join(__dirname, '../src/scripts/restartservice.sh'); - await safe(shell.promises.sudo('test', [ RELOAD_NGINX_CMD, 'nginx' ], {})); - }); - - it('exec a valid shell program', function (done) { - shell.exec('test', 'ls -l | wc -c', {}, function (error) { - done(error); + it('exec times out properly', async function () { + const [error] = await safe(shell.promises.exec('sleeping', 'sleep 20', { timeout: 1000 })); + expect(error.reason).to.be(BoxError.SHELL_ERROR); }); }); - - it('exec a valid shell program (promises)', async function () { - await shell.promises.exec('test', 'ls -l | wc -c', {}); - }); - - it('exec throws for invalid program', function (done) { - shell.exec('test', 'cannotexist', {}, function (error) { - expect(error).to.be.ok(); - done(); - }); - }); - - it('exec throws for failed program', function (done) { - shell.exec('test', 'false', {}, function (error) { - expect(error).to.be.ok(); - done(); - }); - }); - - it('exec times out properly', async function () { - const [error] = await safe(shell.promises.exec('sleeping', 'sleep 20', { timeout: 1000 })); - expect(error.reason).to.be(BoxError.SHELL_ERROR); - }); }); diff --git a/src/updater.js b/src/updater.js index 1038e80a0..a4dfa1e23 100644 --- a/src/updater.js +++ b/src/updater.js @@ -65,16 +65,10 @@ async function downloadUrl(url, file) { safe.fs.unlinkSync(file); await promiseRetry({ times: 10, interval: 5000, debug }, async function () { - debug(`Downloading ${url} to ${file}`); - - const args = `-s --fail ${url} -o ${file}`; - - debug(`downloadUrl: curl ${args}`); - - const [error] = await safe(shell.promises.spawn('downloadUrl', '/usr/bin/curl', args.split(' '), {})); + debug(`downloadUrl: downloading ${url} to ${file}`); + const [error] = await safe(shell.promises.exec('downloadUrl', `curl -s --fail ${url} -o ${file}`, {})); if (error) throw new BoxError(BoxError.NETWORK_ERROR, `Failed to download ${url}: ${error.message}`); - - debug(`downloadUrl: downloaded ${url} to ${file}`); + debug('downloadUrl: done'); }); } @@ -103,16 +97,13 @@ async function extractTarball(tarball, dir) { assert.strictEqual(typeof tarball, 'string'); assert.strictEqual(typeof dir, 'string'); - const args = `-zxf ${tarball} -C ${dir}`; + debug(`extractTarball: extracting ${tarball} to ${dir}`); - debug(`extractTarball: tar ${args}`); - - const [error] = await safe(shell.promises.spawn('extractTarball', '/bin/tar', args.split(' '), {})); + const [error] = await safe(shell.promises.exec('extractTarball', `tar -zxf ${tarball} -C ${dir}`, {})); if (error) throw new BoxError(BoxError.FS_ERROR, `Failed to extract release package: ${error.message}`); - safe.fs.unlinkSync(tarball); - debug(`extractTarball: extracted ${tarball} to ${dir}`); + debug('extractTarball: extracted'); } async function verifyUpdateInfo(versionsFile, updateInfo) {