Merge "New AMI Command Output Format"

UPGRADE.txt

@@ -46,5 +46,13 @@ Logging:
  - 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.
+
 ===========================================================
 ===========================================================

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 */
 

main/cli.c

@@ -2681,12 +2681,12 @@ int ast_cli_command_full(int uid, int gid, int fd, const char *s)
 	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 @@ int ast_cli_command_full(int uid, int gid, int fd, const char *s)
 	/* 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 @@ int ast_cli_command_full(int uid, int gid, int fd, const char *s)
 
 	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)

main/manager.c

@@ -4839,11 +4839,10 @@ static int check_blacklist(const char *cmd)
 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 @@ static int action_command(struct mansession *s, const struct message *m)
 	}
 
 	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);