Skip to content

Commit 82ef491

Browse files
authored
fix: remove the temp file entirely (#98)
* fix: remove the temp file entirely * chore: remove testing script from package.json
1 parent f14673d commit 82ef491

File tree

5 files changed

+124
-261
lines changed

5 files changed

+124
-261
lines changed

lib/escape.js

+2-11
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,11 @@ const cmd = (input, doubleEscape) => {
3636
}
3737

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

44-
// except for % which is escaped with another %, and only once
45-
result = result.replace(/%/g, '%%')
46-
4744
return result
4845
}
4946

@@ -65,13 +62,7 @@ const sh = (input) => {
6562
return result
6663
}
6764

68-
// disabling the no-control-regex rule for this line as we very specifically _do_ want to
69-
// replace those characters if they somehow exist at this point, which is highly unlikely
70-
// eslint-disable-next-line no-control-regex
71-
const filename = (input) => input.replace(/[<>:"/\\|?*\x00-\x1F]/g, '')
72-
7365
module.exports = {
7466
cmd,
7567
sh,
76-
filename,
7768
}

lib/make-spawn-args.js

+11-40
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,10 @@
11
/* eslint camelcase: "off" */
22
const isWindows = require('./is-windows.js')
33
const setPATH = require('./set-path.js')
4-
const { unlinkSync: unlink, writeFileSync: writeFile } = require('fs')
5-
const { tmpdir } = require('os')
64
const { resolve } = require('path')
75
const which = require('which')
86
const npm_config_node_gyp = require.resolve('node-gyp/bin/node-gyp.js')
97
const escape = require('./escape.js')
10-
const { randomBytes } = require('crypto')
11-
12-
const translateWinPathToPosix = (path) => {
13-
return path
14-
.replace(/^([A-z]):/, '/$1')
15-
.replace(/\\/g, '/')
16-
}
178

189
const makeSpawnArgs = options => {
1910
const {
@@ -38,10 +29,7 @@ const makeSpawnArgs = options => {
3829
npm_config_node_gyp,
3930
})
4031

41-
const fileName = escape.filename(`${event}-${randomBytes(4).toString('hex')}`)
42-
let scriptFile
43-
let script = ''
44-
32+
let doubleEscape = false
4533
const isCmd = /(?:^|\\)cmd(?:\.exe)?$/i.test(scriptShell)
4634
if (isCmd) {
4735
let initialCmd = ''
@@ -68,26 +56,18 @@ const makeSpawnArgs = options => {
6856
pathToInitial = initialCmd.toLowerCase()
6957
}
7058

71-
const doubleEscape = pathToInitial.endsWith('.cmd') || pathToInitial.endsWith('.bat')
72-
73-
scriptFile = resolve(tmpdir(), `${fileName}.cmd`)
74-
script += '@echo off\n'
75-
script += cmd
76-
if (args.length) {
77-
script += ` ${args.map((arg) => escape.cmd(arg, doubleEscape)).join(' ')}`
78-
}
79-
} else {
80-
scriptFile = resolve(tmpdir(), `${fileName}.sh`)
81-
script = cmd
82-
if (args.length) {
83-
script += ` ${args.map((arg) => escape.sh(arg)).join(' ')}`
84-
}
59+
doubleEscape = pathToInitial.endsWith('.cmd') || pathToInitial.endsWith('.bat')
8560
}
8661

87-
writeFile(scriptFile, script)
62+
let script = cmd
63+
for (const arg of args) {
64+
script += isCmd
65+
? ` ${escape.cmd(arg, doubleEscape)}`
66+
: ` ${escape.sh(arg)}`
67+
}
8868
const spawnArgs = isCmd
89-
? ['/d', '/s', '/c', escape.cmd(scriptFile)]
90-
: [isWindows ? translateWinPathToPosix(scriptFile) : scriptFile]
69+
? ['/d', '/s', '/c', script]
70+
: ['-c', '--', script]
9171

9272
const spawnOpts = {
9373
env: spawnEnv,
@@ -97,16 +77,7 @@ const makeSpawnArgs = options => {
9777
...(isCmd ? { windowsVerbatimArguments: true } : {}),
9878
}
9979

100-
const cleanup = () => {
101-
// delete the script, this is just a best effort
102-
try {
103-
unlink(scriptFile)
104-
} catch (err) {
105-
// ignore errors
106-
}
107-
}
108-
109-
return [scriptShell, spawnArgs, spawnOpts, cleanup]
80+
return [scriptShell, spawnArgs, spawnOpts]
11081
}
11182

11283
module.exports = makeSpawnArgs

lib/run-script-pkg.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ const runScriptPkg = async options => {
5555
console.log(bruce(pkg._id, event, cmd))
5656
}
5757

58-
const [spawnShell, spawnArgs, spawnOpts, cleanup] = makeSpawnArgs({
58+
const [spawnShell, spawnArgs, spawnOpts] = makeSpawnArgs({
5959
event,
6060
path,
6161
scriptShell,
@@ -93,7 +93,7 @@ const runScriptPkg = async options => {
9393
} else {
9494
throw er
9595
}
96-
}).finally(cleanup)
96+
})
9797
}
9898

9999
module.exports = runScriptPkg

test/escape.js

+38-46
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict'
22

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

3131
t.test('integration', { skip: isWindows && 'posix only' }, async (t) => {
32-
const dir = t.testdir()
33-
3432
for (const [input] of expectations) {
35-
const filename = join(dir, 'posix.sh')
36-
const script = `#!/usr/bin/env sh\nnode -p process.argv[1] -- ${escape.sh(input)}`
37-
writeFile(filename, script)
38-
chmod(filename, '0755')
39-
const p = await promiseSpawn('sh', ['-c', filename], { stdioString: true })
33+
const script = `node -p process.argv[1] -- ${escape.sh(input)}`
34+
const p = await promiseSpawn('sh', ['-c', '--', script], { stdioString: true })
4035
const stdout = p.stdout.trim()
41-
t.equal(input, stdout, 'actual output matches input')
42-
unlink(filename)
36+
t.equal(stdout, input, `expected \`${stdout}\` to equal \`${input}\``)
4337
}
4438

4539
t.end()
@@ -52,30 +46,31 @@ t.test('cmd', (t) => {
5246
const expectations = [
5347
['', '""'],
5448
['test', 'test'],
55-
['%PATH%', '%%PATH%%'],
56-
['%PATH%', '%%PATH%%', true],
57-
['"%PATH%"', '^"\\^"%%PATH%%\\^"^"'],
58-
['"%PATH%"', '^^^"\\^^^"%%PATH%%\\^^^"^^^"', true],
59-
[`'%PATH%'`, `'%%PATH%%'`],
60-
[`'%PATH%'`, `'%%PATH%%'`, true],
61-
['\\%PATH%', '\\%%PATH%%'],
62-
['\\%PATH%', '\\%%PATH%%', true],
63-
['--arg="%PATH%"', '^"--arg=\\^"%%PATH%%\\^"^"'],
64-
['--arg="%PATH%"', '^^^"--arg=\\^^^"%%PATH%%\\^^^"^^^"', true],
65-
['--arg=npm exec -c "%PATH%"', '^"--arg=npm^ exec^ -c^ \\^"%%PATH%%\\^"^"'],
66-
['--arg=npm exec -c "%PATH%"', '^^^"--arg=npm^^^ exec^^^ -c^^^ \\^^^"%%PATH%%\\^^^"^^^"', true],
67-
[`--arg=npm exec -c '%PATH%'`, `^"--arg=npm^ exec^ -c^ '%%PATH%%'^"`],
68-
[`--arg=npm exec -c '%PATH%'`, `^^^"--arg=npm^^^ exec^^^ -c^^^ '%%PATH%%'^^^"`, true],
69-
[`'--arg=npm exec -c "%PATH%"'`, `^"'--arg=npm^ exec^ -c^ \\^"%%PATH%%\\^"'^"`],
49+
['%PATH%', '^%PATH^%'],
50+
['%PATH%', '^^^%PATH^^^%', true],
51+
['"%PATH%"', '^"\\^"^%PATH^%\\^"^"'],
52+
['"%PATH%"', '^^^"\\^^^"^^^%PATH^^^%\\^^^"^^^"', true],
53+
[`'%PATH%'`, `'^%PATH^%'`],
54+
[`'%PATH%'`, `'^^^%PATH^^^%'`, true],
55+
['\\%PATH%', '\\^%PATH^%'],
56+
['\\%PATH%', '\\^^^%PATH^^^%', true],
57+
['--arg="%PATH%"', '^"--arg=\\^"^%PATH^%\\^"^"'],
58+
['--arg="%PATH%"', '^^^"--arg=\\^^^"^^^%PATH^^^%\\^^^"^^^"', true],
59+
['--arg=npm exec -c "%PATH%"', '^"--arg=npm^ exec^ -c^ \\^"^%PATH^%\\^"^"'],
60+
['--arg=npm exec -c "%PATH%"',
61+
'^^^"--arg=npm^^^ exec^^^ -c^^^ \\^^^"^^^%PATH^^^%\\^^^"^^^"', true],
62+
[`--arg=npm exec -c '%PATH%'`, `^"--arg=npm^ exec^ -c^ '^%PATH^%'^"`],
63+
[`--arg=npm exec -c '%PATH%'`, `^^^"--arg=npm^^^ exec^^^ -c^^^ '^^^%PATH^^^%'^^^"`, true],
64+
[`'--arg=npm exec -c "%PATH%"'`, `^"'--arg=npm^ exec^ -c^ \\^"^%PATH^%\\^"'^"`],
7065
[`'--arg=npm exec -c "%PATH%"'`,
71-
`^^^"'--arg=npm^^^ exec^^^ -c^^^ \\^^^"%%PATH%%\\^^^"'^^^"`, true],
66+
`^^^"'--arg=npm^^^ exec^^^ -c^^^ \\^^^"^^^%PATH^^^%\\^^^"'^^^"`, true],
7267
['"C:\\Program Files\\test.bat"', '^"\\^"C:\\Program^ Files\\test.bat\\^"^"'],
7368
['"C:\\Program Files\\test.bat"', '^^^"\\^^^"C:\\Program^^^ Files\\test.bat\\^^^"^^^"', true],
74-
['"C:\\Program Files\\test%.bat"', '^"\\^"C:\\Program^ Files\\test%%.bat\\^"^"'],
69+
['"C:\\Program Files\\test%.bat"', '^"\\^"C:\\Program^ Files\\test^%.bat\\^"^"'],
7570
['"C:\\Program Files\\test%.bat"',
76-
'^^^"\\^^^"C:\\Program^^^ Files\\test%%.bat\\^^^"^^^"', true],
77-
['% % %', '^"%%^ %%^ %%^"'],
78-
['% % %', '^^^"%%^^^ %%^^^ %%^^^"', true],
71+
'^^^"\\^^^"C:\\Program^^^ Files\\test^^^%.bat\\^^^"^^^"', true],
72+
['% % %', '^"^%^ ^%^ ^%^"'],
73+
['% % %', '^^^"^^^%^^^ ^^^%^^^ ^^^%^^^"', true],
7974
['hello^^^^^^', 'hello^^^^^^^^^^^^'],
8075
['hello^^^^^^', 'hello^^^^^^^^^^^^^^^^^^^^^^^^', true],
8176
['hello world', '^"hello^ world^"'],
@@ -94,8 +89,8 @@ t.test('cmd', (t) => {
9489
['hello\\\\"world', '^^^"hello\\\\\\\\\\^^^"world^^^"', true],
9590
['hello world\\', '^"hello^ world\\\\^"'],
9691
['hello world\\', '^^^"hello^^^ world\\\\^^^"', true],
97-
['hello %PATH%', '^"hello^ %%PATH%%^"'],
98-
['hello %PATH%', '^^^"hello^^^ %%PATH%%^^^"', true],
92+
['hello %PATH%', '^"hello^ ^%PATH^%^"'],
93+
['hello %PATH%', '^^^"hello^^^ ^^^%PATH^^^%^^^"', true],
9994
]
10095

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

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

109107
for (const [input,, double] of expectations) {
110-
const filename = join(dir, 'win.cmd')
111-
if (double) {
112-
const shimFile = join(dir, 'shim.cmd')
113-
const shim = `@echo off\nnode -p process.argv[1] -- %*`
114-
writeFile(shimFile, shim)
115-
const script = `@echo off\n"${shimFile}" ${escape.cmd(input, double)}`
116-
writeFile(filename, script)
117-
} else {
118-
const script = `@echo off\nnode -p process.argv[1] -- ${escape.cmd(input)}`
119-
writeFile(filename, script)
120-
}
121-
const p = await promiseSpawn('cmd', ['/d', '/s', '/c', filename], { stdioString: true })
108+
const script = double
109+
? `${escape.cmd(shimFile)} ${escape.cmd(input, double)}`
110+
: `node -p process.argv[1] -- ${escape.cmd(input)}`
111+
const p = await promiseSpawn('cmd', ['/d', '/s', '/c', script], {
112+
stdioString: true,
113+
windowsVerbatimArguments: true,
114+
})
122115
const stdout = p.stdout.trim()
123-
t.equal(input, stdout, 'actual output matches input')
124-
unlink(filename)
116+
t.equal(stdout, input, `expected \`${stdout}\` to equal \`${input}\``)
125117
}
126118

127119
t.end()

0 commit comments

Comments
 (0)