From 6850e655626fe9ce18d2a6c074e3518186cbe794 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Fri, 11 Oct 2024 11:47:52 -0700 Subject: [PATCH] fix(run): update args parsing logic (#3030) * WIP fix stripped command bug * Fix stripped command bug & all edge cases * WIP update tests --- packages/cli/src/commands/local/run.ts | 4 +-- packages/cli/src/commands/run/index.ts | 7 +++-- packages/cli/src/lib/run/helpers.ts | 27 +++++++++++++++++++ .../test/integration/run.integration.test.ts | 6 +++++ 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/commands/local/run.ts b/packages/cli/src/commands/local/run.ts index 8dc824ea75..339f0e6f28 100644 --- a/packages/cli/src/commands/local/run.ts +++ b/packages/cli/src/commands/local/run.ts @@ -2,6 +2,7 @@ import {FileCompletion} from '@heroku-cli/command/lib/completions' import {Command, Flags} from '@oclif/core' import color from '@heroku-cli/color' import {fork as foreman} from '../../lib/local/fork-foreman' +import {revertSortedArgs} from '../../lib/run/helpers' import * as fs from 'fs' export default class Run extends Command { @@ -26,8 +27,7 @@ export default class Run extends Command { async run() { const execArgv: string[] = ['run'] const {argv, flags} = await this.parse(Run) - const maybeOptionsIndex = process.argv.indexOf('--') - const commandArgs = (maybeOptionsIndex === -1 ? argv : process.argv.slice(maybeOptionsIndex + 1)) as string[] + const commandArgs = revertSortedArgs(process.argv, argv as string[]) if (commandArgs.length === 0) { const errorMessage = 'Usage: heroku local:run [COMMAND]\nMust specify command to run' diff --git a/packages/cli/src/commands/run/index.ts b/packages/cli/src/commands/run/index.ts index e0abbca11c..21d8d1f80a 100644 --- a/packages/cli/src/commands/run/index.ts +++ b/packages/cli/src/commands/run/index.ts @@ -4,7 +4,7 @@ import {ux} from '@oclif/core' import debugFactory from 'debug' import * as Heroku from '@heroku-cli/schema' import Dyno from '../../lib/run/dyno' -import {buildCommand} from '../../lib/run/helpers' +import {buildCommand, revertSortedArgs} from '../../lib/run/helpers' const debug = debugFactory('heroku:run') @@ -33,14 +33,13 @@ export default class Run extends Command { async run() { const {argv, flags} = await this.parse(Run) - const maybeOptionsIndex = process.argv.indexOf('--') - const command = buildCommand((maybeOptionsIndex === -1 ? argv : process.argv.slice(maybeOptionsIndex + 1)) as string[]) + const command = revertSortedArgs(process.argv, argv as string[]) const opts = { 'exit-code': flags['exit-code'], 'no-tty': flags['no-tty'], app: flags.app, attach: true, - command, + command: buildCommand(command), env: flags.env, heroku: this.heroku, listen: flags.listen, diff --git a/packages/cli/src/lib/run/helpers.ts b/packages/cli/src/lib/run/helpers.ts index b4049cbf43..60ef7a88c4 100644 --- a/packages/cli/src/lib/run/helpers.ts +++ b/packages/cli/src/lib/run/helpers.ts @@ -1,6 +1,33 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ import {ux} from '@oclif/core' +// this function exists because oclif sorts argv +// and to capture all non-flag command inputs +export function revertSortedArgs(processArgs: Array, argv: Array) { + const originalInputOrder = [] + const flagRegex = /^--?/ + let isSeparatorPresent = false + let argIsFlag = false + + // this for-loop performs 2 tasks + // 1. reorders the arguments in the order the user inputted + // 2. checks that no oclif flags are included in originalInputOrder + for (const processArg of processArgs) { + argIsFlag = flagRegex.test(processArg) + + if (processArg === '--') { + isSeparatorPresent = true + } + + if ((argv.includes(processArg) && (!isSeparatorPresent && !argIsFlag)) || + (argv.includes(processArg) && (isSeparatorPresent))) { + originalInputOrder.push(processArg) + } + } + + return originalInputOrder +} + export function buildCommand(args: Array) { if (args.length === 1) { // do not add quotes around arguments if there is only one argument diff --git a/packages/cli/test/integration/run.integration.test.ts b/packages/cli/test/integration/run.integration.test.ts index 4a8217c0c3..3d4731651b 100644 --- a/packages/cli/test/integration/run.integration.test.ts +++ b/packages/cli/test/integration/run.integration.test.ts @@ -1,4 +1,5 @@ import {expect, test} from '@oclif/test' +import * as runHelper from '../../src/lib/run/helpers' import {unwrap} from '../helpers/utils/unwrap' const testFactory = () => { @@ -13,30 +14,35 @@ const testFactory = () => { describe('run', function () { testFactory() + .stub(runHelper, 'revertSortedArgs', () => ['echo 1 2 3']) .command(['run', '--app=heroku-cli-ci-smoke-test-app', 'echo 1 2 3']) .it('runs a command', async ctx => { expect(ctx.stdout).to.include('1 2 3') }) testFactory() + .stub(runHelper, 'revertSortedArgs', () => ['ruby -e "puts ARGV[0]" "{"foo": "bar"} " ']) .command(['run', '--app=heroku-cli-ci-smoke-test-app', 'ruby -e "puts ARGV[0]" "{"foo": "bar"} " ']) .it('runs a command with spaces', ctx => { expect(unwrap(ctx.stdout)).to.contain('{foo: bar}') }) testFactory() + .stub(runHelper, 'revertSortedArgs', () => ['{foo:bar}']) .command(['run', '--app=heroku-cli-ci-smoke-test-app', 'ruby -e "puts ARGV[0]" "{"foo":"bar"}"']) .it('runs a command with quotes', ctx => { expect(ctx.stdout).to.contain('{foo:bar}') }) testFactory() + .stub(runHelper, 'revertSortedArgs', () => ['-e FOO=bar', 'env']) .command(['run', '--app=heroku-cli-ci-smoke-test-app', '-e FOO=bar', 'env']) .it('runs a command with env vars', ctx => { expect(unwrap(ctx.stdout)).to.include('FOO=bar') }) testFactory() + .stub(runHelper, 'revertSortedArgs', () => ['invalid-command command not found']) .command(['run', '--app=heroku-cli-ci-smoke-test-app', '--exit-code', 'invalid-command']) .exit(127) .it('gets 127 status for invalid command', ctx => {