From b90fc9d5d307131badcaa0cc5efa08fcd72b91fe Mon Sep 17 00:00:00 2001 From: Dan Allen Date: Wed, 16 Oct 2024 19:02:12 -0600 Subject: [PATCH] resolves #42 revert to running batch command with properly escaped arguments in shell on Windows --- CHANGELOG.adoc | 4 +++ docs/modules/ROOT/pages/configure-run.adoc | 3 ++ .../lib/util/run-command.js | 32 +++++++++++++++---- .../test/collector-extension-test.js | 20 ++++++++++++ .../fixtures/at-root/.gen-file-from-arg.bat | 7 ++++ packages/collector-test-harness/lib/index.js | 3 ++ 6 files changed, 63 insertions(+), 6 deletions(-) create mode 100644 packages/collector-extension/test/fixtures/at-root/.gen-file-from-arg.bat diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index a6996c3..6d10cdf 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -10,6 +10,10 @@ For a detailed view of what's changed, refer to the {url-repo}/commits[commit hi * only report the base call instead of the whole command when the command cannot be found +=== Fixed + +* revert to running batch command with properly escaped arguments in shell on Windows if required by Node.js (#42) + == 1.0.0-beta.3 (2024-10-10) === Added diff --git a/docs/modules/ROOT/pages/configure-run.adoc b/docs/modules/ROOT/pages/configure-run.adoc index e39bc1c..b5bef46 100644 --- a/docs/modules/ROOT/pages/configure-run.adoc +++ b/docs/modules/ROOT/pages/configure-run.adoc @@ -233,6 +233,9 @@ When the `shell` key is `false` (or not set) and the command has no file extensi If it finds a match, it will invoke that file instead. This is the default behavior in Windows when running in a shell. +NOTE: To work around a policy restriction in Node.js on Windows, Collector will always run a batch script using the shell, even if the shell is not requested. +However, no additional escpaing of arguments is required in this case as Collector will handle that task. + The value of the `command` key may include context variable expressions. The `env` context variable includes all environment variables added by the `env` key. diff --git a/packages/collector-extension/lib/util/run-command.js b/packages/collector-extension/lib/util/run-command.js index 40241e0..a3b1503 100644 --- a/packages/collector-extension/lib/util/run-command.js +++ b/packages/collector-extension/lib/util/run-command.js @@ -4,12 +4,14 @@ const fs = require('node:fs') const { promises: fsp } = fs const ospath = require('node:path') const parseCommand = require('./parse-command') -const { spawn, execFile } = require('node:child_process') +const { execFile, spawn, spawnSync } = require('node:child_process') const invariably = { true: () => true, false: () => false } const ENV_NAME_RX = /\$(\w+)/g const IS_WIN = process.platform === 'win32' +const WIN_BATCH_USE_SHELL = IS_WIN && spawnSync('npm.cmd', ['-v'], { windowsHide: true }).error?.code === 'EINVAL' +const WIN_SHELL_ESCAPE_CHARS = { ' ': true, '"': true, '<': true, '>': true, '&': true, '%': true, '^': true } async function runCommand (cmd = '', opts = {}) { const shell = opts.shell @@ -23,11 +25,12 @@ async function runCommand (cmd = '', opts = {}) { cmdv = cmdv.map((it) => (~it.indexOf('$') ? it.replace(ENV_NAME_RX, '%$1%') : it)) } else { const bare = !~cmd0.indexOf(ospath.sep) - if (ospath.extname(cmd0)) { + let extname = ospath.extname(cmd0) + if (extname) { if (bare && local) cmdv[0] = '.' + ospath.sep + cmd0 } else if (bare && !local) { if (cmd0 in where) { - cmdv[0] = where[cmd0] + extname = ospath.extname((cmdv[0] = where[cmd0])) } else { cmdv[0] = await new Promise((resolve) => { execFile('where', [cmd0 + '.???'], { cwd: opts.cwd, windowsHide: true }, (_, results) => { @@ -36,7 +39,7 @@ async function runCommand (cmd = '', opts = {}) { const ext = ospath.extname(it) return ext ? Object.assign(accum, { [ext.slice(1)]: ext }) : accum }, {}) - resolve(cmd0 + (exts.exe || exts.bat || exts.cmd || '')) + resolve(cmd0 + (extname = exts.exe || exts.bat || exts.cmd || '')) }) }) } @@ -44,14 +47,18 @@ async function runCommand (cmd = '', opts = {}) { let absCmd0 = cmd0 if (!ospath.isAbsolute(absCmd0)) { absCmd0 = ospath.join(opts.cwd || '', cmd0) - if (bare) cmdv[0] = cmd0 = '.' + ospath.sep + cmd0 + if (bare) cmdv[0] = '.' + ospath.sep + cmd0 } for (const resolvedExt of ['.exe', '.bat', '.cmd']) { if (!(await fsp.access(absCmd0 + resolvedExt).then(invariably.true, invariably.false))) continue - cmdv[0] = cmd0 + resolvedExt + cmdv[0] = cmd0 + (extname = resolvedExt) break } } + if (WIN_BATCH_USE_SHELL && (extname === '.bat' || extname === '.cmd')) { + cmdv = cmdv.map(winShellEscape) + spawnOpts.shell = true + } } Object.assign(spawnOpts, { windowsHide: true, windowsVerbatimArguments: false }) } else if (local && !~cmd0.indexOf('/')) { @@ -87,4 +94,17 @@ async function spawnCommand (cmdv, opts, quiet) { }) } +const winShellEscape = (val) => { + const result = [] + for (const c of val) { + if (WIN_SHELL_ESCAPE_CHARS[c]) { + result[result.length - 1] === '"' ? result.pop() : result.push('"') + result.push(c, '"') + } else { + result.push(c) + } + } + return result.join('') +} + module.exports = runCommand diff --git a/packages/collector-extension/test/collector-extension-test.js b/packages/collector-extension/test/collector-extension-test.js index 466f402..55e6f5f 100644 --- a/packages/collector-extension/test/collector-extension-test.js +++ b/packages/collector-extension/test/collector-extension-test.js @@ -18,6 +18,7 @@ const { trapAsyncError, updateYamlFile, windows, + windowsBatchRequiresShell, } = require('@antora/collector-test-harness') const aggregateContent = require('@antora/content-aggregator') const { createHash } = require('node:crypto') @@ -899,6 +900,25 @@ describe('collector extension', () => { }) } + if (windowsBatchRequiresShell) { + it('should revert to shell with escaped arguments if command is batch script', async () => { + const collectorConfig = { + run: [{ command: '.gen-file-from-arg.bat "&bar b^z\\""', local: true }], + scan: { dir: 'build' }, + } + await runScenario({ + repoName: 'at-root', + collectorConfig, + after: (contentAggregate) => { + assert.equal(contentAggregate.length, 1) + assert.equal(contentAggregate[0].files.length, 1) + assert.equal(contentAggregate[0].files[0].path, 'modules/ROOT/pages/index.adoc') + assert(contentAggregate[0].files[0].contents.includes('"<"foo">&"bar" "b"^"z"""')) + }, + }) + }) + } + it('should run specified command from content root if dir option is .', async () => { const collectorConfig = { run: { command: '.gen-start-page', local: true, dir: '.' }, diff --git a/packages/collector-extension/test/fixtures/at-root/.gen-file-from-arg.bat b/packages/collector-extension/test/fixtures/at-root/.gen-file-from-arg.bat new file mode 100644 index 0000000..eda2333 --- /dev/null +++ b/packages/collector-extension/test/fixtures/at-root/.gen-file-from-arg.bat @@ -0,0 +1,7 @@ +@echo off +mkdir build\modules\ROOT\pages +( + echo.= Page Title + echo. + echo.%1 +) > build\modules\ROOT\pages\index.adoc diff --git a/packages/collector-test-harness/lib/index.js b/packages/collector-test-harness/lib/index.js index a796bdf..3879443 100644 --- a/packages/collector-test-harness/lib/index.js +++ b/packages/collector-test-harness/lib/index.js @@ -11,6 +11,7 @@ const fs = require('node:fs') const fsp = require('node:fs/promises') const { Git: GitServer } = require('node-git-server') const { once } = require('node:events') +const { spawnSync } = require('node:child_process') const yaml = require('js-yaml') beforeEach(() => configureLogger({ level: 'silent' })) @@ -132,4 +133,6 @@ module.exports = { startGitServer, trapAsyncError, windows: process.platform === 'win32', + windowsBatchRequiresShell: + process.platform === 'win32' && spawnSync('npm.cmd', ['-v'], { windowsHide: true }).error?.code === 'EINVAL', } -- GitLab