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

Correctly quote passthrough arguments #355

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions declarations/escape-it.d.ts
@@ -0,0 +1,4 @@
declare module 'escape-it' {
function escape(platform?: string): (...args: string[]) => string;
export default escape;
}
22 changes: 11 additions & 11 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -49,9 +49,9 @@
"dependencies": {
"chalk": "^4.1.0",
"date-fns": "^2.29.1",
"escape-it": "^0.3.0",
"lodash": "^4.17.21",
"rxjs": "^7.0.0",
"shell-quote": "^1.7.3",
"spawn-command": "^0.0.2-1",
"supports-color": "^8.1.0",
"tree-kill": "^1.2.2",
Expand Down
12 changes: 10 additions & 2 deletions src/command-parser/expand-arguments.spec.ts
@@ -1,6 +1,8 @@
import { CommandInfo } from '../command';
import { ExpandArguments } from './expand-arguments';

const isWindows = process.platform === 'win32';

const createCommandInfo = (command: string): CommandInfo => ({
command,
name: '',
Expand All @@ -21,7 +23,10 @@ it('single argument placeholder is replaced', () => {
it('argument placeholder is replaced and quoted properly', () => {
const parser = new ExpandArguments(['foo bar']);
const commandInfo = createCommandInfo('echo {1}');
expect(parser.parse(commandInfo)).toEqual({ ...commandInfo, command: "echo 'foo bar'" });
expect(parser.parse(commandInfo)).toEqual({
...commandInfo,
command: isWindows ? 'echo "foo bar"' : "echo 'foo bar'",
});
});

it('multiple single argument placeholders are replaced', () => {
Expand Down Expand Up @@ -57,7 +62,10 @@ it('all arguments placeholder is replaced', () => {
it('combined arguments placeholder is replaced', () => {
const parser = new ExpandArguments(['foo', 'bar']);
const commandInfo = createCommandInfo('echo {*}');
expect(parser.parse(commandInfo)).toEqual({ ...commandInfo, command: "echo 'foo bar'" });
expect(parser.parse(commandInfo)).toEqual({
...commandInfo,
command: isWindows ? 'echo "foo bar"' : "echo 'foo bar'",
});
});

it('escaped argument placeholders are not replaced', () => {
Expand Down
10 changes: 6 additions & 4 deletions src/command-parser/expand-arguments.ts
@@ -1,8 +1,10 @@
import { quote } from 'shell-quote';
import escapeFn from 'escape-it';

import { CommandInfo } from '../command';
import { CommandParser } from './command-parser';

const escape = escapeFn();

/**
* Replace placeholders with additional arguments.
*/
Expand All @@ -22,15 +24,15 @@ export class ExpandArguments implements CommandParser {
!isNaN(placeholderTarget) &&
placeholderTarget <= this.additionalArguments.length
) {
return quote([this.additionalArguments[placeholderTarget - 1]]);
return escape(this.additionalArguments[placeholderTarget - 1]);
}
// Replace all arguments placeholder.
if (placeholderTarget === '@') {
return quote(this.additionalArguments);
return escape(...this.additionalArguments);
}
// Replace combined arguments placeholder.
if (placeholderTarget === '*') {
return quote([this.additionalArguments.join(' ')]);
return escape(this.additionalArguments.join(' '));
}
// Replace placeholder with empty string
// if value doesn't exist in additional arguments.
Expand Down
7 changes: 6 additions & 1 deletion src/concurrently.spec.ts
Expand Up @@ -7,6 +7,8 @@ import { createFakeProcess, FakeCommand } from './fixtures/fake-command';
import { FlowController } from './flow-control/flow-controller';
import { Logger } from './logger';

const isWindows = process.platform === 'win32';

let spawn: SpawnCommand;
let kill: KillProcess;
let onFinishHooks: (() => void)[];
Expand Down Expand Up @@ -237,7 +239,10 @@ it('argument placeholders are properly replaced when additional arguments are pa
expect(spawn).toHaveBeenCalledTimes(4);
expect(spawn).toHaveBeenCalledWith('echo foo', expect.objectContaining({}));
expect(spawn).toHaveBeenCalledWith('echo foo bar', expect.objectContaining({}));
expect(spawn).toHaveBeenCalledWith("echo 'foo bar'", expect.objectContaining({}));
expect(spawn).toHaveBeenCalledWith(
isWindows ? 'echo "foo bar"' : "echo 'foo bar'",
expect.objectContaining({})
);
expect(spawn).toHaveBeenCalledWith('echo {@}', expect.objectContaining({}));
});

Expand Down