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.

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

feat: write script and args to temp file and run that #78

Merged
merged 3 commits into from
Jun 21, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
65 changes: 65 additions & 0 deletions lib/escape.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
'use strict'

const cmd = (input) => {
if (!input.length) {
return '""'
}

let result
if (!/[ \t\n\v"]/.test(input)) {
result = input
} else {
result = '"'
for (let i = 0; i <= input.length; ++i) {
let slashCount = 0
while (input[i] === '\\') {
++i
++slashCount
}

if (i === input.length) {
result += '\\'.repeat(slashCount * 2)
break
}

if (input[i] === '"') {
result += '\\'.repeat(slashCount * 2 + 1)
result += input[i]
} else {
result += '\\'.repeat(slashCount)
result += input[i]
}
}
result += '"'
}

// and finally, prefix shell meta chars with a ^
result = result.replace(/[!^&()<>|"]/g, '^$&')
// except for % which is escaped with another %
result = result.replace(/%/g, '%%')

return result
}

const sh = (input) => {
if (!input.length) {
return `''`
}

if (!/[\t\n\r "#$&'()*;<>?\\`|~]/.test(input)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanna be sure it's intentional that we're only looking for certain whitespace characters, and not using \s. Will approve assuming this is intentional.

Copy link
Contributor Author

@nlf nlf Jun 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was meant to be a list of each individual character that causes us to need single quotes to avoid expansion/keep the argument in one piece, the switch could likely be made but i felt like this was more explicit and clear

return input
}

// replace single quotes with '\'' and wrap the whole result in a fresh set of quotes
const result = `'${input.replace(/'/g, `'\\''`)}'`
// if the input string already had single quotes around it, clean those up
.replace(/^(?:'')+(?!$)/, '')
.replace(/\\'''/g, `\\'`)

return result
}

module.exports = {
cmd,
sh,
}
32 changes: 30 additions & 2 deletions lib/make-spawn-args.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
/* eslint camelcase: "off" */
const isWindows = require('./is-windows.js')
const setPATH = require('./set-path.js')
const { chmodSync: chmod, 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 makeSpawnArgs = options => {
const {
Expand All @@ -12,11 +16,28 @@ const makeSpawnArgs = options => {
env = {},
stdio,
cmd,
args = [],
stdioString = false,
} = options

let scriptFile
let script = ''
const isCmd = /(?:^|\\)cmd(?:\.exe)?$/i.test(scriptShell)
const args = isCmd ? ['/d', '/s', '/c', cmd] : ['-c', cmd]
if (isCmd) {
scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.cmd`)
script += '@echo off\n'
script += `${cmd} ${args.map((arg) => escape.cmd(arg)).join(' ')}`
} else {
const shellPath = which.sync(scriptShell)
scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.sh`)
script += `#!${shellPath}\n`
script += `${cmd} ${args.map((arg) => escape.sh(arg)).join(' ')}`
}
writeFile(scriptFile, script)
if (!isCmd) {
chmod(scriptFile, '0775')
}
const spawnArgs = isCmd ? ['/d', '/s', '/c', scriptFile] : ['-c', scriptFile]

const spawnOpts = {
env: setPATH(path, {
Expand All @@ -34,7 +55,14 @@ const makeSpawnArgs = options => {
...(isCmd ? { windowsVerbatimArguments: true } : {}),
}

return [scriptShell, args, spawnOpts]
const cleanup = () => {
// delete the script, this is just a best effort
try {
unlink(scriptFile)
} catch (err) {}
}

return [scriptShell, spawnArgs, spawnOpts, cleanup]
}

module.exports = makeSpawnArgs
13 changes: 8 additions & 5 deletions lib/run-script-pkg.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const runScriptPkg = async options => {
if (options.cmd) {
cmd = options.cmd
} else if (pkg.scripts && pkg.scripts[event]) {
cmd = pkg.scripts[event] + args.map(a => ` ${JSON.stringify(a)}`).join('')
cmd = pkg.scripts[event]
} else if (
// If there is no preinstall or install script, default to rebuilding node-gyp packages.
event === 'install' &&
Expand All @@ -42,7 +42,7 @@ const runScriptPkg = async options => {
) {
cmd = defaultGypInstallScript
} else if (event === 'start' && await isServerPackage(path)) {
cmd = 'node server.js' + args.map(a => ` ${JSON.stringify(a)}`).join('')
cmd = 'node server.js'
}

if (!cmd) {
Expand All @@ -54,15 +54,18 @@ const runScriptPkg = async options => {
console.log(bruce(pkg._id, event, cmd))
}

const p = promiseSpawn(...makeSpawnArgs({
const [spawnShell, spawnArgs, spawnOpts, cleanup] = makeSpawnArgs({
event,
path,
scriptShell,
env: packageEnvs(env, pkg),
stdio,
cmd,
args,
stdioString,
}), {
})

const p = promiseSpawn(spawnShell, spawnArgs, spawnOpts, {
event,
script: cmd,
pkgid: pkg._id,
Expand All @@ -88,7 +91,7 @@ const runScriptPkg = async options => {
} else {
throw er
}
})
}).finally(cleanup)
}

module.exports = runScriptPkg
63 changes: 63 additions & 0 deletions test/escape.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
const t = require('tap')

const escape = require('../lib/escape.js')

t.test('sh', (t) => {
t.test('returns empty quotes when input is empty', async (t) => {
const input = ''
const output = escape.sh(input)
t.equal(output, `''`, 'returned empty single quotes')
})

t.test('returns plain string if quotes are not necessary', async (t) => {
const input = 'test'
const output = escape.sh(input)
t.equal(output, input, 'returned plain string')
})

t.test('wraps in single quotes if special character is present', async (t) => {
const input = 'test words'
const output = escape.sh(input)
t.equal(output, `'test words'`, 'wrapped in single quotes')
})
t.end()
})

t.test('cmd', (t) => {
t.test('returns empty quotes when input is empty', async (t) => {
const input = ''
const output = escape.cmd(input)
t.equal(output, '""', 'returned empty double quotes')
})

t.test('returns plain string if quotes are not necessary', async (t) => {
const input = 'test'
const output = escape.cmd(input)
t.equal(output, input, 'returned plain string')
})

t.test('wraps in double quotes when necessary', async (t) => {
const input = 'test words'
const output = escape.cmd(input)
t.equal(output, '^"test words^"', 'wrapped in double quotes')
})

t.test('doubles up backslashes at end of input', async (t) => {
const input = 'one \\ two \\'
const output = escape.cmd(input)
t.equal(output, '^"one \\ two \\\\^"', 'doubles backslash at end of string')
})

t.test('doubles up backslashes immediately before a double quote', async (t) => {
const input = 'one \\"'
const output = escape.cmd(input)
t.equal(output, '^"one \\\\\\^"^"', 'doubles backslash before double quote')
})

t.test('backslash escapes double quotes', async (t) => {
const input = '"test"'
const output = escape.cmd(input)
t.equal(output, '^"\\^"test\\^"^"', 'escaped double quotes')
})
t.end()
})
Loading