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.

Sign up for GitHub

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

Jump to bottom

fix: remove the temp file entirely #98

Merged
merged 2 commits into from
Aug 8, 2022
Merged

fix: remove the temp file entirely #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5944c2
fix: remove the temp file entirely
nlf Jul 12, 2022
3def9af
chore: remove testing script from package.json
nlf Jul 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions lib/escape.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,11 @@ const cmd = (input, doubleEscape) => {
}

// and finally, prefix shell meta chars with a ^
result = result.replace(/[ !^&()<>|"]/g, '^$&')
result = result.replace(/[ !%^&()<>|"]/g, '^$&')
if (doubleEscape) {
result = result.replace(/[ !^&()<>|"]/g, '^$&')
result = result.replace(/[ !%^&()<>|"]/g, '^$&')
}

// except for % which is escaped with another %, and only once
result = result.replace(/%/g, '%%')

return result
}

Expand All @@ -65,13 +62,7 @@ const sh = (input) => {
return result
}

// disabling the no-control-regex rule for this line as we very specifically _do_ want to
// replace those characters if they somehow exist at this point, which is highly unlikely
// eslint-disable-next-line no-control-regex
const filename = (input) => input.replace(/[<>:"/\\|?*\x00-\x1F]/g, '')

module.exports = {
cmd,
sh,
filename,
}
51 changes: 11 additions & 40 deletions lib/make-spawn-args.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,10 @@
/* eslint camelcase: "off" */
const isWindows = require('./is-windows.js')
const setPATH = require('./set-path.js')
const { unlinkSync: unlink, writeFileSync: writeFile } = require('fs')
const { tmpdir } = require('os')
const { resolve } = require('path')
const which = require('which')
const npm_config_node_gyp = require.resolve('node-gyp/bin/node-gyp.js')
const escape = require('./escape.js')
const { randomBytes } = require('crypto')

const translateWinPathToPosix = (path) => {
return path
.replace(/^([A-z]):/, '/$1')
.replace(/\\/g, '/')
}

const makeSpawnArgs = options => {
const {
Expand All @@ -38,10 +29,7 @@ const makeSpawnArgs = options => {
npm_config_node_gyp,
})

const fileName = escape.filename(`${event}-${randomBytes(4).toString('hex')}`)
let scriptFile
let script = ''

let doubleEscape = false
const isCmd = /(?:^|\\)cmd(?:\.exe)?$/i.test(scriptShell)
if (isCmd) {
let initialCmd = ''
Expand All @@ -68,26 +56,18 @@ const makeSpawnArgs = options => {
pathToInitial = initialCmd.toLowerCase()
}

const doubleEscape = pathToInitial.endsWith('.cmd') || pathToInitial.endsWith('.bat')

scriptFile = resolve(tmpdir(), `${fileName}.cmd`)
script += '@echo off\n'
script += cmd
if (args.length) {
script += ` ${args.map((arg) => escape.cmd(arg, doubleEscape)).join(' ')}`
}
} else {
scriptFile = resolve(tmpdir(), `${fileName}.sh`)
script = cmd
if (args.length) {
script += ` ${args.map((arg) => escape.sh(arg)).join(' ')}`
}
doubleEscape = pathToInitial.endsWith('.cmd') || pathToInitial.endsWith('.bat')
}

writeFile(scriptFile, script)
let script = cmd
for (const arg of args) {
script += isCmd
? ` ${escape.cmd(arg, doubleEscape)}`
: ` ${escape.sh(arg)}`
}
const spawnArgs = isCmd
? ['/d', '/s', '/c', escape.cmd(scriptFile)]
: [isWindows ? translateWinPathToPosix(scriptFile) : scriptFile]
? ['/d', '/s', '/c', script]
: ['-c', '--', script]

const spawnOpts = {
env: spawnEnv,
Expand All @@ -97,16 +77,7 @@ const makeSpawnArgs = options => {
...(isCmd ? { windowsVerbatimArguments: true } : {}),
}

const cleanup = () => {
// delete the script, this is just a best effort
try {
unlink(scriptFile)
} catch (err) {
// ignore errors
}
}

return [scriptShell, spawnArgs, spawnOpts, cleanup]
return [scriptShell, spawnArgs, spawnOpts]
}

module.exports = makeSpawnArgs
4 changes: 2 additions & 2 deletions lib/run-script-pkg.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const runScriptPkg = async options => {
console.log(bruce(pkg._id, event, cmd))
}

const [spawnShell, spawnArgs, spawnOpts, cleanup] = makeSpawnArgs({
const [spawnShell, spawnArgs, spawnOpts] = makeSpawnArgs({
event,
path,
scriptShell,
Expand Down Expand Up @@ -93,7 +93,7 @@ const runScriptPkg = async options => {
} else {
throw er
}
}).finally(cleanup)
})
}

module.exports = runScriptPkg
84 changes: 38 additions & 46 deletions test/escape.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const { writeFileSync: writeFile, unlinkSync: unlink, chmodSync: chmod } = require('fs')
const { writeFileSync: writeFile } = require('fs')
const { join } = require('path')
const t = require('tap')
const promiseSpawn = require('@npmcli/promise-spawn')
Expand Down Expand Up @@ -29,17 +29,11 @@ t.test('sh', (t) => {
}

t.test('integration', { skip: isWindows && 'posix only' }, async (t) => {
const dir = t.testdir()

for (const [input] of expectations) {
const filename = join(dir, 'posix.sh')
const script = `#!/usr/bin/env sh\nnode -p process.argv[1] -- ${escape.sh(input)}`
writeFile(filename, script)
chmod(filename, '0755')
const p = await promiseSpawn('sh', ['-c', filename], { stdioString: true })
const script = `node -p process.argv[1] -- ${escape.sh(input)}`
const p = await promiseSpawn('sh', ['-c', '--', script], { stdioString: true })
const stdout = p.stdout.trim()
t.equal(input, stdout, 'actual output matches input')
unlink(filename)
t.equal(stdout, input, `expected \`${stdout}\` to equal \`${input}\``)
}

t.end()
Expand All @@ -52,30 +46,31 @@ t.test('cmd', (t) => {
const expectations = [
['', '""'],
['test', 'test'],
['%PATH%', '%%PATH%%'],
['%PATH%', '%%PATH%%', true],
['"%PATH%"', '^"\\^"%%PATH%%\\^"^"'],
['"%PATH%"', '^^^"\\^^^"%%PATH%%\\^^^"^^^"', true],
[`'%PATH%'`, `'%%PATH%%'`],
[`'%PATH%'`, `'%%PATH%%'`, true],
['\\%PATH%', '\\%%PATH%%'],
['\\%PATH%', '\\%%PATH%%', true],
['--arg="%PATH%"', '^"--arg=\\^"%%PATH%%\\^"^"'],
['--arg="%PATH%"', '^^^"--arg=\\^^^"%%PATH%%\\^^^"^^^"', true],
['--arg=npm exec -c "%PATH%"', '^"--arg=npm^ exec^ -c^ \\^"%%PATH%%\\^"^"'],
['--arg=npm exec -c "%PATH%"', '^^^"--arg=npm^^^ exec^^^ -c^^^ \\^^^"%%PATH%%\\^^^"^^^"', true],
[`--arg=npm exec -c '%PATH%'`, `^"--arg=npm^ exec^ -c^ '%%PATH%%'^"`],
[`--arg=npm exec -c '%PATH%'`, `^^^"--arg=npm^^^ exec^^^ -c^^^ '%%PATH%%'^^^"`, true],
[`'--arg=npm exec -c "%PATH%"'`, `^"'--arg=npm^ exec^ -c^ \\^"%%PATH%%\\^"'^"`],
['%PATH%', '^%PATH^%'],
['%PATH%', '^^^%PATH^^^%', true],
['"%PATH%"', '^"\\^"^%PATH^%\\^"^"'],
['"%PATH%"', '^^^"\\^^^"^^^%PATH^^^%\\^^^"^^^"', true],
[`'%PATH%'`, `'^%PATH^%'`],
[`'%PATH%'`, `'^^^%PATH^^^%'`, true],
['\\%PATH%', '\\^%PATH^%'],
['\\%PATH%', '\\^^^%PATH^^^%', true],
['--arg="%PATH%"', '^"--arg=\\^"^%PATH^%\\^"^"'],
['--arg="%PATH%"', '^^^"--arg=\\^^^"^^^%PATH^^^%\\^^^"^^^"', true],
['--arg=npm exec -c "%PATH%"', '^"--arg=npm^ exec^ -c^ \\^"^%PATH^%\\^"^"'],
['--arg=npm exec -c "%PATH%"',
'^^^"--arg=npm^^^ exec^^^ -c^^^ \\^^^"^^^%PATH^^^%\\^^^"^^^"', true],
[`--arg=npm exec -c '%PATH%'`, `^"--arg=npm^ exec^ -c^ '^%PATH^%'^"`],
[`--arg=npm exec -c '%PATH%'`, `^^^"--arg=npm^^^ exec^^^ -c^^^ '^^^%PATH^^^%'^^^"`, true],
[`'--arg=npm exec -c "%PATH%"'`, `^"'--arg=npm^ exec^ -c^ \\^"^%PATH^%\\^"'^"`],
[`'--arg=npm exec -c "%PATH%"'`,
`^^^"'--arg=npm^^^ exec^^^ -c^^^ \\^^^"%%PATH%%\\^^^"'^^^"`, true],
`^^^"'--arg=npm^^^ exec^^^ -c^^^ \\^^^"^^^%PATH^^^%\\^^^"'^^^"`, true],
['"C:\\Program Files\\test.bat"', '^"\\^"C:\\Program^ Files\\test.bat\\^"^"'],
['"C:\\Program Files\\test.bat"', '^^^"\\^^^"C:\\Program^^^ Files\\test.bat\\^^^"^^^"', true],
['"C:\\Program Files\\test%.bat"', '^"\\^"C:\\Program^ Files\\test%%.bat\\^"^"'],
['"C:\\Program Files\\test%.bat"', '^"\\^"C:\\Program^ Files\\test^%.bat\\^"^"'],
['"C:\\Program Files\\test%.bat"',
'^^^"\\^^^"C:\\Program^^^ Files\\test%%.bat\\^^^"^^^"', true],
['% % %', '^"%%^ %%^ %%^"'],
['% % %', '^^^"%%^^^ %%^^^ %%^^^"', true],
'^^^"\\^^^"C:\\Program^^^ Files\\test^^^%.bat\\^^^"^^^"', true],
['% % %', '^"^%^ ^%^ ^%^"'],
['% % %', '^^^"^^^%^^^ ^^^%^^^ ^^^%^^^"', true],
['hello^^^^^^', 'hello^^^^^^^^^^^^'],
['hello^^^^^^', 'hello^^^^^^^^^^^^^^^^^^^^^^^^', true],
['hello world', '^"hello^ world^"'],
Expand All @@ -94,8 +89,8 @@ t.test('cmd', (t) => {
['hello\\\\"world', '^^^"hello\\\\\\\\\\^^^"world^^^"', true],
['hello world\\', '^"hello^ world\\\\^"'],
['hello world\\', '^^^"hello^^^ world\\\\^^^"', true],
['hello %PATH%', '^"hello^ %%PATH%%^"'],
['hello %PATH%', '^^^"hello^^^ %%PATH%%^^^"', true],
['hello %PATH%', '^"hello^ ^%PATH^%^"'],
['hello %PATH%', '^^^"hello^^^ ^^^%PATH^^^%^^^"', true],
]

for (const [input, expectation, double] of expectations) {
Expand All @@ -105,23 +100,20 @@ t.test('cmd', (t) => {

t.test('integration', { skip: !isWindows && 'Windows only' }, async (t) => {
const dir = t.testdir()
const shimFile = join(dir, 'shim.cmd')
const shim = `@echo off\nnode -p process.argv[1] -- %*`
writeFile(shimFile, shim)

for (const [input,, double] of expectations) {
const filename = join(dir, 'win.cmd')
if (double) {
const shimFile = join(dir, 'shim.cmd')
const shim = `@echo off\nnode -p process.argv[1] -- %*`
writeFile(shimFile, shim)
const script = `@echo off\n"${shimFile}" ${escape.cmd(input, double)}`
writeFile(filename, script)
} else {
const script = `@echo off\nnode -p process.argv[1] -- ${escape.cmd(input)}`
writeFile(filename, script)
}
const p = await promiseSpawn('cmd', ['/d', '/s', '/c', filename], { stdioString: true })
const script = double
? `${escape.cmd(shimFile)} ${escape.cmd(input, double)}`
: `node -p process.argv[1] -- ${escape.cmd(input)}`
const p = await promiseSpawn('cmd', ['/d', '/s', '/c', script], {
stdioString: true,
windowsVerbatimArguments: true,
})
const stdout = p.stdout.trim()
t.equal(input, stdout, 'actual output matches input')
unlink(filename)
t.equal(stdout, input, `expected \`${stdout}\` to equal \`${input}\``)
}

t.end()
Expand Down
Loading