[Asterisk-code-review] New AMI Command Output Format (asterisk[master])
Matt Jordan
asteriskteam at digium.com
Thu Apr 23 06:31:16 CDT 2015
Matt Jordan has submitted this change and it was merged.
Change subject: New AMI Command Output Format
......................................................................
New AMI Command Output Format
This change modifies how the the output from a CLI command is sent
to a client over AMI.
Output from the CLI command is now sent as a series of zero-or-more
Output: headers.
Additionally, commands that fail to execute (eg: no such command,
invalid syntax etc.) now cause an Error response instead of Success.
If the command executed successfully, but the manager unable to
provide the output the reason will be included in the Message:
header. Otherwise it will contain 'Command output follows'.
Depends on a new version of starpy (> 1.0.2) that supports the new
output format.
See pull-request https://github.com/asterisk/starpy/pull/34
ASTERISK-24730
Change-Id: I6718d95490f0a6b3f171c1a5cdad9207f9a44888
---
M UPGRADE.txt
M include/asterisk/manager.h
M main/cli.c
M main/manager.c
4 files changed, 46 insertions(+), 34 deletions(-)
Approvals:
Mark Michelson: Looks good to me, but someone else must approve
Matt Jordan: Looks good to me, approved; Verified
George Joseph: Looks good to me, but someone else must approve
Corey Farrell: Looks good to me, but someone else must approve
diff --git a/UPGRADE.txt b/UPGRADE.txt
index 8a349f0..0696adb 100644
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -46,5 +46,13 @@
- The first callid created is now 1 instead of 0. The value 0
is now reserved to represent a lack of callid.
+AMI:
+ - The Command action now sends the output from the CLI command as a series
+ of Output headers for each line instead of as a block of text with the
+ --END COMMAND-- delimiter to match the output from other actions.
+
+ Commands that fail to execute (no such command, invalid syntax etc.) now
+ return an Error response instead of Success.
+
===========================================================
===========================================================
diff --git a/include/asterisk/manager.h b/include/asterisk/manager.h
index 43031d1..b5ede54 100644
--- a/include/asterisk/manager.h
+++ b/include/asterisk/manager.h
@@ -54,7 +54,7 @@
- \ref manager.c Main manager code file
*/
-#define AMI_VERSION "2.7.0"
+#define AMI_VERSION "2.8.0"
#define DEFAULT_MANAGER_PORT 5038 /* Default port for Asterisk management via TCP */
#define DEFAULT_MANAGER_TLS_PORT 5039 /* Default port for Asterisk management via TCP */
diff --git a/main/cli.c b/main/cli.c
index 9d3cdf3..a230c20 100644
--- a/main/cli.c
+++ b/main/cli.c
@@ -2681,12 +2681,12 @@
int x;
char *duplicate = parse_args(s, &x, args + 1, AST_MAX_ARGS, NULL);
char tmp[AST_MAX_ARGS + 1];
- char *retval = NULL;
+ char *retval = CLI_FAILURE;
struct ast_cli_args a = {
.fd = fd, .argc = x, .argv = args+1 };
if (duplicate == NULL)
- return -1;
+ return RESULT_FAILURE;
if (x < 1) /* We need at least one entry, otherwise ignore */
goto done;
@@ -2705,8 +2705,7 @@
/* Check if the user has rights to run this command. */
if (!cli_has_permissions(uid, gid, tmp)) {
ast_cli(fd, "You don't have permissions to run '%s' command\n", tmp);
- ast_free(duplicate);
- return 0;
+ goto done;
}
/*
@@ -2719,14 +2718,13 @@
if (retval == CLI_SHOWUSAGE) {
ast_cli(fd, "%s", S_OR(e->usage, "Invalid usage, but no usage information available.\n"));
- } else {
- if (retval == CLI_FAILURE)
- ast_cli(fd, "Command '%s' failed.\n", s);
+ } else if (retval == CLI_FAILURE) {
+ ast_cli(fd, "Command '%s' failed.\n", s);
}
ast_atomic_fetchadd_int(&e->inuse, -1);
done:
ast_free(duplicate);
- return 0;
+ return retval == CLI_SUCCESS ? RESULT_SUCCESS : RESULT_FAILURE;
}
int ast_cli_command_multiple_full(int uid, int gid, int fd, size_t size, const char *s)
diff --git a/main/manager.c b/main/manager.c
index f2a516f..2ff9df9 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -4839,11 +4839,10 @@
static int action_command(struct mansession *s, const struct message *m)
{
const char *cmd = astman_get_header(m, "Command");
- const char *id = astman_get_header(m, "ActionID");
- char *buf = NULL, *final_buf = NULL;
+ char *buf = NULL, *final_buf = NULL, *delim, *output;
char template[] = "/tmp/ast-ami-XXXXXX"; /* template for temporary file */
- int fd;
- off_t l;
+ int fd, ret;
+ off_t len;
if (ast_strlen_zero(cmd)) {
astman_send_error(s, m, "No command provided");
@@ -4856,52 +4855,59 @@
}
if ((fd = mkstemp(template)) < 0) {
- ast_log(AST_LOG_WARNING, "Failed to create temporary file for command: %s\n", strerror(errno));
- astman_send_error(s, m, "Command response construction error");
+ astman_send_error_va(s, m, "Failed to create temporary file: %s", strerror(errno));
return 0;
}
- astman_append(s, "Response: Follows\r\nPrivilege: Command\r\n");
- if (!ast_strlen_zero(id)) {
- astman_append(s, "ActionID: %s\r\n", id);
- }
- /* FIXME: Wedge a ActionID response in here, waiting for later changes */
- ast_cli_command(fd, cmd); /* XXX need to change this to use a FILE * */
+ ret = ast_cli_command(fd, cmd);
+ astman_send_response_full(s, m, ret == RESULT_SUCCESS ? "Success" : "Error", MSG_MOREDATA, NULL);
+
/* Determine number of characters available */
- if ((l = lseek(fd, 0, SEEK_END)) < 0) {
- ast_log(LOG_WARNING, "Failed to determine number of characters for command: %s\n", strerror(errno));
+ if ((len = lseek(fd, 0, SEEK_END)) < 0) {
+ astman_append(s, "Message: Failed to determine number of characters: %s\r\n", strerror(errno));
goto action_command_cleanup;
}
/* This has a potential to overflow the stack. Hence, use the heap. */
- buf = ast_malloc(l + 1);
- final_buf = ast_malloc(l + 1);
+ buf = ast_malloc(len + 1);
+ final_buf = ast_malloc(len + 1);
if (!buf || !final_buf) {
- ast_log(LOG_WARNING, "Failed to allocate memory for temporary buffer\n");
+ astman_append(s, "Message: Memory allocation failure\r\n");
goto action_command_cleanup;
}
if (lseek(fd, 0, SEEK_SET) < 0) {
- ast_log(LOG_WARNING, "Failed to set position on temporary file for command: %s\n", strerror(errno));
+ astman_append(s, "Message: Failed to set position on temporary file: %s\r\n", strerror(errno));
goto action_command_cleanup;
}
- if (read(fd, buf, l) < 0) {
- ast_log(LOG_WARNING, "read() failed: %s\n", strerror(errno));
+ if (read(fd, buf, len) < 0) {
+ astman_append(s, "Message: Failed to read from temporary file: %s\r\n", strerror(errno));
goto action_command_cleanup;
}
- buf[l] = '\0';
- term_strip(final_buf, buf, l);
- final_buf[l] = '\0';
- astman_append(s, "%s", final_buf);
+ buf[len] = '\0';
+ term_strip(final_buf, buf, len);
+ final_buf[len] = '\0';
+
+ /* Trim trailing newline */
+ if (len && final_buf[len - 1] == '\n') {
+ final_buf[len - 1] = '\0';
+ }
+
+ astman_append(s, "Message: Command output follows\r\n");
+
+ delim = final_buf;
+ while ((output = strsep(&delim, "\n"))) {
+ astman_append(s, "Output: %s\r\n", output);
+ }
action_command_cleanup:
+ astman_append(s, "\r\n");
close(fd);
unlink(template);
- astman_append(s, "--END COMMAND--\r\n\r\n");
ast_free(buf);
ast_free(final_buf);
--
To view, visit https://gerrit.asterisk.org/139
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6718d95490f0a6b3f171c1a5cdad9207f9a44888
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Gareth Palmer <gareth at acsdata.co.nz>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
More information about the asterisk-code-review
mailing list