From 6251f5ee9dbaede1c36e7f277939beef019d48d5 Mon Sep 17 00:00:00 2001 From: Peter Evans Date: Thu, 23 Jul 2020 10:30:59 +0900 Subject: [PATCH] Add support for quoted arguments --- README.md | 16 ++--- __test__/command-helper.test.ts | 118 ++++++++++++++++++++++++++++---- dist/index.js | 59 ++++++++++------ docs/updating.md | 10 +++ src/command-helper.ts | 37 +++++++--- src/main.ts | 21 +++--- 6 files changed, 202 insertions(+), 59 deletions(-) diff --git a/README.md b/README.md index 4e29dd5..63c9ed9 100644 --- a/README.md +++ b/README.md @@ -162,23 +162,22 @@ To demonstrate, take the following configuration as an example. region=us-east-1 ``` -For the above example configuration, the slash command `/deploy branch=master some other args` will be converted to a JSON payload as follows. +For the above example configuration, the slash command `/deploy branch=master dry-run reason="new feature"` will be converted to a JSON payload as follows. ```json "slash_command": { "command": "deploy", "args": { - "all": "production region=us-east-1 branch=master some other args", + "all": "production region=us-east-1 branch=master dry-run reason=\"new feature\"", "unnamed": { - "all": "production some other args", + "all": "production dry-run", "arg1": "production", - "arg2": "some", - "arg3": "other", - "arg4": "args" + "arg2": "dry-run" }, "named": { "region": "us-east-1", - "branch": "master" + "branch": "master", + "reason": "new feature" }, } } @@ -194,10 +193,9 @@ The properties in the `slash_command` context from the above example can be used echo ${{ github.event.client_payload.slash_command.args.unnamed.all }} echo ${{ github.event.client_payload.slash_command.args.unnamed.arg1 }} echo ${{ github.event.client_payload.slash_command.args.unnamed.arg2 }} - echo ${{ github.event.client_payload.slash_command.args.unnamed.arg3 }} - echo ${{ github.event.client_payload.slash_command.args.unnamed.arg4 }} echo ${{ github.event.client_payload.slash_command.args.named.region }} echo ${{ github.event.client_payload.slash_command.args.named.branch }} + echo ${{ github.event.client_payload.slash_command.args.named.reason }} # etc. ``` diff --git a/__test__/command-helper.test.ts b/__test__/command-helper.test.ts index 568ee71..de6ac88 100644 --- a/__test__/command-helper.test.ts +++ b/__test__/command-helper.test.ts @@ -7,6 +7,7 @@ import { getCommandsConfigFromJson, actorHasPermission, configIsValid, + tokeniseCommand, getSlashCommandPayload } from '../lib/command-helper' @@ -216,8 +217,36 @@ describe('command-helper tests', () => { expect(actorHasPermission('write', 'write')).toBeTruthy() }) - test('slash command payload', async () => { - const commandWords = ['test', 'arg1', 'arg2', 'arg3'] + test('command arguments are correctly tokenised', async () => { + const command = `a b=c "d e" f-g="h i" "j \\"k\\"" l="m \\"n\\" o"` + const commandTokens = [ + `a`, + `b=c`, + `"d e"`, + `f-g="h i"`, + `"j \\"k\\""`, + `l="m \\"n\\" o"` + ] + expect(tokeniseCommand(command)).toEqual(commandTokens) + }) + + test('tokenisation of malformed command arguments', async () => { + const command = `test arg named= quoted arg" named-arg="with \\"quoted value` + const commandTokens = [ + 'test', + 'arg', + 'named=', + 'quoted', + `arg"`, + `named-arg="with`, + '\\"quoted', + 'value' + ] + expect(tokeniseCommand(command)).toEqual(commandTokens) + }) + + test('slash command payload with unnamed args', async () => { + const commandTokens = ['test', 'arg1', 'arg2', 'arg3'] const staticArgs = [] const payload: SlashCommandPayload = { command: 'test', @@ -232,11 +261,11 @@ describe('command-helper tests', () => { named: {} } } - expect(getSlashCommandPayload(commandWords, staticArgs)).toEqual(payload) + expect(getSlashCommandPayload(commandTokens, staticArgs)).toEqual(payload) }) test('slash command payload with named args', async () => { - const commandWords = [ + const commandTokens = [ 'test', 'branch_name=master', 'arg1', @@ -259,21 +288,21 @@ describe('command-helper tests', () => { } } } - expect(getSlashCommandPayload(commandWords, staticArgs)).toEqual(payload) + expect(getSlashCommandPayload(commandTokens, staticArgs)).toEqual(payload) }) test('slash command payload with named args and static args', async () => { - const commandWords = ['test', 'branch=master', 'arg1', 'arg2'] + const commandTokens = ['test', 'branch=master', 'arg1', 'dry-run'] const staticArgs = ['production', 'region=us-east-1'] const payload: SlashCommandPayload = { command: 'test', args: { - all: 'production region=us-east-1 branch=master arg1 arg2', + all: 'production region=us-east-1 branch=master arg1 dry-run', unnamed: { - all: 'production arg1 arg2', + all: 'production arg1 dry-run', arg1: 'production', arg2: 'arg1', - arg3: 'arg2' + arg3: 'dry-run' }, named: { region: 'us-east-1', @@ -281,11 +310,43 @@ describe('command-helper tests', () => { } } } - expect(getSlashCommandPayload(commandWords, staticArgs)).toEqual(payload) + expect(getSlashCommandPayload(commandTokens, staticArgs)).toEqual(payload) + }) + + test('slash command payload with quoted args', async () => { + const commandTokens = [ + `test`, + `a`, + `b=c`, + `"d e"`, + `f-g="h i"`, + `"j \\"k\\""`, + `l="m \\"n\\" o"` + ] + const staticArgs = [`msg="x y z"`] + const payload: SlashCommandPayload = { + command: `test`, + args: { + all: `msg="x y z" a b=c "d e" f-g="h i" "j \\"k\\"" l="m \\"n\\" o"`, + unnamed: { + all: `a "d e" "j \\"k\\""`, + arg1: `a`, + arg2: `d e`, + arg3: `j \\"k\\"` + }, + named: { + msg: `x y z`, + b: `c`, + 'f-g': `h i`, + l: `m \\"n\\" o` + } + } + } + expect(getSlashCommandPayload(commandTokens, staticArgs)).toEqual(payload) }) test('slash command payload with malformed named args', async () => { - const commandWords = ['test', 'branch=', 'arg1', 'e.nv=prod', 'arg2'] + const commandTokens = ['test', 'branch=', 'arg1', 'e.nv=prod', 'arg2'] const staticArgs = [] const payload: SlashCommandPayload = { command: 'test', @@ -301,6 +362,39 @@ describe('command-helper tests', () => { named: {} } } - expect(getSlashCommandPayload(commandWords, staticArgs)).toEqual(payload) + expect(getSlashCommandPayload(commandTokens, staticArgs)).toEqual(payload) + }) + + test('slash command payload with malformed quoted args', async () => { + const commandTokens = [ + 'test', + 'arg', + 'named=', + 'quoted', + `arg"`, + `named-arg="with`, + `\\"quoted`, + 'value' + ] + const staticArgs = [] + const payload: SlashCommandPayload = { + command: 'test', + args: { + all: `arg named= quoted arg" named-arg="with \\"quoted value`, + unnamed: { + all: `arg named= quoted arg" \\"quoted value`, + arg1: 'arg', + arg2: 'named=', + arg3: 'quoted', + arg4: `arg"`, + arg5: `\\"quoted`, + arg6: 'value' + }, + named: { + 'named-arg': `"with` + } + } + } + expect(getSlashCommandPayload(commandTokens, staticArgs)).toEqual(payload) }) }) diff --git a/dist/index.js b/dist/index.js index 14c4b72..d8f430f 100644 --- a/dist/index.js +++ b/dist/index.js @@ -559,16 +559,16 @@ function run() { console.debug('The first line of the comment is not a valid slash command.'); return; } - // Split the first line into "words" - const commandWords = firstLine.slice(1).split(' '); - core.debug(`Command words: ${util_1.inspect(commandWords)}`); + // Tokenise the first line (minus the leading slash) + const commandTokens = command_helper_1.tokeniseCommand(firstLine.slice(1)); + core.debug(`Command tokens: ${util_1.inspect(commandTokens)}`); // Check if the command is registered for dispatch let configMatches = config.filter(function (cmd) { - return cmd.command == commandWords[0]; + return cmd.command == commandTokens[0]; }); core.debug(`Config matches on 'command': ${util_1.inspect(configMatches)}`); if (configMatches.length == 0) { - core.info(`Command '${commandWords[0]}' is not registered for dispatch.`); + core.info(`Command '${commandTokens[0]}' is not registered for dispatch.`); return; } // Filter matching commands by issue type @@ -581,7 +581,7 @@ function run() { core.debug(`Config matches on 'issue_type': ${util_1.inspect(configMatches)}`); if (configMatches.length == 0) { const issueType = isPullRequest ? 'pull request' : 'issue'; - core.info(`Command '${commandWords[0]}' is not configured for the issue type '${issueType}'.`); + core.info(`Command '${commandTokens[0]}' is not configured for the issue type '${issueType}'.`); return; } // Filter matching commands by whether or not to allow edits @@ -591,7 +591,7 @@ function run() { }); core.debug(`Config matches on 'allow_edits': ${util_1.inspect(configMatches)}`); if (configMatches.length == 0) { - core.info(`Command '${commandWords[0]}' is not configured to allow edits.`); + core.info(`Command '${commandTokens[0]}' is not configured to allow edits.`); return; } } @@ -611,11 +611,11 @@ function run() { }); core.debug(`Config matches on 'permission': ${util_1.inspect(configMatches)}`); if (configMatches.length == 0) { - core.info(`Command '${commandWords[0]}' is not configured for the user's permission level '${actorPermission}'.`); + core.info(`Command '${commandTokens[0]}' is not configured for the user's permission level '${actorPermission}'.`); return; } // Determined that the command should be dispatched - core.info(`Command '${commandWords[0]}' to be dispatched.`); + core.info(`Command '${commandTokens[0]}' to be dispatched.`); // Define payload const clientPayload = { github: github.context @@ -635,7 +635,7 @@ function run() { // Dispatch for each matching configuration for (const cmd of configMatches) { // Generate slash command payload - clientPayload.slash_command = command_helper_1.getSlashCommandPayload(commandWords, cmd.static_args); + clientPayload.slash_command = command_helper_1.getSlashCommandPayload(commandTokens, cmd.static_args); core.debug(`Slash command payload: ${util_1.inspect(clientPayload.slash_command)}`); // Dispatch the command yield githubHelper.createDispatch(cmd, clientPayload); @@ -932,12 +932,15 @@ var __importStar = (this && this.__importStar) || function (mod) { return result; }; Object.defineProperty(exports, "__esModule", { value: true }); -exports.getSlashCommandPayload = exports.actorHasPermission = exports.configIsValid = exports.getCommandsConfigFromJson = exports.getCommandsConfigFromInputs = exports.getCommandsConfig = exports.getInputs = exports.toBool = exports.commandDefaults = exports.MAX_ARGS = void 0; +exports.getSlashCommandPayload = exports.tokeniseCommand = exports.actorHasPermission = exports.configIsValid = exports.getCommandsConfigFromJson = exports.getCommandsConfigFromInputs = exports.getCommandsConfig = exports.getInputs = exports.toBool = exports.commandDefaults = exports.MAX_ARGS = void 0; const core = __importStar(__webpack_require__(470)); const fs = __importStar(__webpack_require__(747)); const util_1 = __webpack_require__(669); const utils = __importStar(__webpack_require__(611)); -const NAMED_ARG_PATTERN = /^(?[a-zA-Z0-9_-]+)=(?[^\s]+)$/; +// Tokenise command and arguments +// Support escaped quotes within quotes. https://stackoverflow.com/a/5696141/11934042 +const TOKENISE_REGEX = /\S+="[^"\\]*(?:\\.[^"\\]*)*"|"[^"\\]*(?:\\.[^"\\]*)*"|\S+/g; +const NAMED_ARG_REGEX = /^(?[a-zA-Z0-9_-]+)=(?.+)$/; exports.MAX_ARGS = 50; exports.commandDefaults = Object.freeze({ permission: 'write', @@ -1073,9 +1076,26 @@ function actorHasPermission(actorPermission, commandPermission) { return (permissionLevels[actorPermission] >= permissionLevels[commandPermission]); } exports.actorHasPermission = actorHasPermission; -function getSlashCommandPayload(commandWords, staticArgs) { +function tokeniseCommand(command) { + let matches; + const output = []; + while ((matches = TOKENISE_REGEX.exec(command))) { + output.push(matches[0]); + } + return output; +} +exports.tokeniseCommand = tokeniseCommand; +function stripQuotes(input) { + if (input.startsWith(`"`) && input.endsWith(`"`)) { + return input.slice(1, input.length - 1); + } + else { + return input; + } +} +function getSlashCommandPayload(commandTokens, staticArgs) { const payload = { - command: commandWords[0], + command: commandTokens[0], args: { all: '', unnamed: { @@ -1085,7 +1105,7 @@ function getSlashCommandPayload(commandWords, staticArgs) { } }; // Get arguments if they exist - const argWords = commandWords.length > 1 ? commandWords.slice(1, exports.MAX_ARGS + 1) : []; + const argWords = commandTokens.length > 1 ? commandTokens.slice(1, exports.MAX_ARGS + 1) : []; // Add static arguments if they exist argWords.unshift(...staticArgs); if (argWords.length > 0) { @@ -1094,16 +1114,15 @@ function getSlashCommandPayload(commandWords, staticArgs) { let unnamedCount = 1; const unnamedArgs = []; for (const argWord of argWords) { - if (NAMED_ARG_PATTERN.test(argWord)) { - const result = NAMED_ARG_PATTERN.exec(argWord); + if (NAMED_ARG_REGEX.test(argWord)) { + const result = NAMED_ARG_REGEX.exec(argWord); if (result && result.groups) { - payload.args.named[`${result.groups['name']}`] = - result.groups['value']; + payload.args.named[`${result.groups['name']}`] = stripQuotes(result.groups['value']); } } else { unnamedArgs.push(argWord); - payload.args.unnamed[`arg${unnamedCount}`] = argWord; + payload.args.unnamed[`arg${unnamedCount}`] = stripQuotes(argWord); unnamedCount += 1; } } diff --git a/docs/updating.md b/docs/updating.md index 9916ed4..c8f769e 100644 --- a/docs/updating.md +++ b/docs/updating.md @@ -57,5 +57,15 @@ ] ``` +- Slash command arguments can now be double-quoted to allow for argument values containing spaces. + + e.g. + ``` + /deploy branch=master dry-run reason="new feature" + ``` + ``` + /send "hello world!" + ``` + - Commands can now be dispatched via the new [workflow_dispatch](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#workflow_dispatch) event. For standard configuration, set the new `dispatch-type` input to `workflow`. For advanced configuration, set the `dispatch_type` JSON property of a command to `workflow`. There are significant differences in the action's behaviour when using `workflow` dispatch. See [workflow dispatch](workflow-dispatch.md) for usage details. diff --git a/src/command-helper.ts b/src/command-helper.ts index 56714d0..e5cac6e 100644 --- a/src/command-helper.ts +++ b/src/command-helper.ts @@ -3,7 +3,10 @@ import * as fs from 'fs' import {inspect} from 'util' import * as utils from './utils' -const NAMED_ARG_PATTERN = /^(?[a-zA-Z0-9_-]+)=(?[^\s]+)$/ +// Tokenise command and arguments +// Support escaped quotes within quotes. https://stackoverflow.com/a/5696141/11934042 +const TOKENISE_REGEX = /\S+="[^"\\]*(?:\\.[^"\\]*)*"|"[^"\\]*(?:\\.[^"\\]*)*"|\S+/g +const NAMED_ARG_REGEX = /^(?[a-zA-Z0-9_-]+)=(?.+)$/ export const MAX_ARGS = 50 @@ -188,12 +191,29 @@ export function actorHasPermission( ) } +export function tokeniseCommand(command: string): string[] { + let matches + const output: string[] = [] + while ((matches = TOKENISE_REGEX.exec(command))) { + output.push(matches[0]) + } + return output +} + +function stripQuotes(input: string): string { + if (input.startsWith(`"`) && input.endsWith(`"`)) { + return input.slice(1, input.length - 1) + } else { + return input + } +} + export function getSlashCommandPayload( - commandWords: string[], + commandTokens: string[], staticArgs: string[] ): SlashCommandPayload { const payload: SlashCommandPayload = { - command: commandWords[0], + command: commandTokens[0], args: { all: '', unnamed: { @@ -204,7 +224,7 @@ export function getSlashCommandPayload( } // Get arguments if they exist const argWords = - commandWords.length > 1 ? commandWords.slice(1, MAX_ARGS + 1) : [] + commandTokens.length > 1 ? commandTokens.slice(1, MAX_ARGS + 1) : [] // Add static arguments if they exist argWords.unshift(...staticArgs) if (argWords.length > 0) { @@ -213,15 +233,16 @@ export function getSlashCommandPayload( let unnamedCount = 1 const unnamedArgs: string[] = [] for (const argWord of argWords) { - if (NAMED_ARG_PATTERN.test(argWord)) { - const result = NAMED_ARG_PATTERN.exec(argWord) + if (NAMED_ARG_REGEX.test(argWord)) { + const result = NAMED_ARG_REGEX.exec(argWord) if (result && result.groups) { - payload.args.named[`${result.groups['name']}`] = + payload.args.named[`${result.groups['name']}`] = stripQuotes( result.groups['value'] + ) } } else { unnamedArgs.push(argWord) - payload.args.unnamed[`arg${unnamedCount}`] = argWord + payload.args.unnamed[`arg${unnamedCount}`] = stripQuotes(argWord) unnamedCount += 1 } } diff --git a/src/main.ts b/src/main.ts index 9586a17..f507cf0 100644 --- a/src/main.ts +++ b/src/main.ts @@ -3,6 +3,7 @@ import * as github from '@actions/github' import {inspect} from 'util' import { getInputs, + tokeniseCommand, getCommandsConfig, configIsValid, actorHasPermission, @@ -60,17 +61,17 @@ async function run(): Promise { return } - // Split the first line into "words" - const commandWords = firstLine.slice(1).split(' ') - core.debug(`Command words: ${inspect(commandWords)}`) + // Tokenise the first line (minus the leading slash) + const commandTokens = tokeniseCommand(firstLine.slice(1)) + core.debug(`Command tokens: ${inspect(commandTokens)}`) // Check if the command is registered for dispatch let configMatches = config.filter(function (cmd) { - return cmd.command == commandWords[0] + return cmd.command == commandTokens[0] }) core.debug(`Config matches on 'command': ${inspect(configMatches)}`) if (configMatches.length == 0) { - core.info(`Command '${commandWords[0]}' is not registered for dispatch.`) + core.info(`Command '${commandTokens[0]}' is not registered for dispatch.`) return } @@ -87,7 +88,7 @@ async function run(): Promise { if (configMatches.length == 0) { const issueType = isPullRequest ? 'pull request' : 'issue' core.info( - `Command '${commandWords[0]}' is not configured for the issue type '${issueType}'.` + `Command '${commandTokens[0]}' is not configured for the issue type '${issueType}'.` ) return } @@ -100,7 +101,7 @@ async function run(): Promise { core.debug(`Config matches on 'allow_edits': ${inspect(configMatches)}`) if (configMatches.length == 0) { core.info( - `Command '${commandWords[0]}' is not configured to allow edits.` + `Command '${commandTokens[0]}' is not configured to allow edits.` ) return } @@ -133,13 +134,13 @@ async function run(): Promise { core.debug(`Config matches on 'permission': ${inspect(configMatches)}`) if (configMatches.length == 0) { core.info( - `Command '${commandWords[0]}' is not configured for the user's permission level '${actorPermission}'.` + `Command '${commandTokens[0]}' is not configured for the user's permission level '${actorPermission}'.` ) return } // Determined that the command should be dispatched - core.info(`Command '${commandWords[0]}' to be dispatched.`) + core.info(`Command '${commandTokens[0]}' to be dispatched.`) // Define payload const clientPayload: ClientPayload = { @@ -171,7 +172,7 @@ async function run(): Promise { for (const cmd of configMatches) { // Generate slash command payload clientPayload.slash_command = getSlashCommandPayload( - commandWords, + commandTokens, cmd.static_args ) core.debug(