hush: disallow "{echo hi; }" (require whitespace)

 and "{ echo hi }" (require semicolon or &)

function                                             old     new   delta
parse_stream                                        2098    2176     +78
done_command                                          98      84     -14
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/1 up/down: 78/-14)             Total: 64 bytes

1 files changed, 35 insertions, 20 deletions

diff --git a/shell/hush.c b/shell/hush.c
index 61b6a7925..8dda988ee 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -319,6 +319,10 @@ struct command {
  */
 	struct redir_struct *redirects; /* I/O redirections */
 };
+/* Is there anything in this command at all? */
+#define IS_NULL_CMD(cmd) \
+	(!(cmd)->group && !(cmd)->argv && !(cmd)->redirects)
+
 
 struct pipe {
 	struct pipe *next;
@@ -341,6 +345,9 @@ typedef enum pipe_style {
 	PIPE_OR  = 3,
 	PIPE_BG  = 4,
 } pipe_style;
+/* Is there anything in this pipe at all? */
+#define IS_NULL_PIPE(pi) \
+	((pi)->num_cmds == 0 IF_HAS_KEYWORDS( && (pi)->res_word == RES_NONE))
 
 /* This holds pointers to the various results of parsing */
 struct parse_context {
@@ -3971,8 +3978,10 @@ static struct pipe *new_pipe(void)
 	return pi;
 }
 
-/* Command (member of a pipe) is complete. The only possible error here
- * is out of memory, in which case xmalloc exits. */
+/* Command (member of a pipe) is complete, or we start a new pipe
+ * if ctx->command is NULL.
+ * No errors possible here.
+ */
 static int done_command(struct parse_context *ctx)
 {
 	/* The command is really already in the pipe structure, so
@@ -3981,13 +3990,9 @@ static int done_command(struct parse_context *ctx)
 	struct command *command = ctx->command;
 
 	if (command) {
-		if (command->group == NULL
-		 && command->argv == NULL
-		 && command->redirects == NULL
-		) {
+		if (IS_NULL_CMD(command)) {
 			debug_printf_parse("done_command: skipping null cmd, num_cmds=%d\n", pi->num_cmds);
-			memset(command, 0, sizeof(*command)); /* paranoia */
-			return pi->num_cmds;
+			goto clear_and_ret;
 		}
 		pi->num_cmds++;
 		debug_printf_parse("done_command: ++num_cmds=%d\n", pi->num_cmds);
@@ -3999,12 +4004,9 @@ static int done_command(struct parse_context *ctx)
 	/* Only real trickiness here is that the uncommitted
 	 * command structure is not counted in pi->num_cmds. */
 	pi->cmds = xrealloc(pi->cmds, sizeof(*pi->cmds) * (pi->num_cmds+1));
-	command = &pi->cmds[pi->num_cmds];
+	ctx->command = command = &pi->cmds[pi->num_cmds];
+ clear_and_ret:
 	memset(command, 0, sizeof(*command));
-
-	ctx->command = command;
-	/* but ctx->pipe and ctx->list_head remain unchanged */
-
 	return pi->num_cmds; /* used only for 0/nonzero check */
 }
 
@@ -4024,9 +4026,7 @@ static void done_pipe(struct parse_context *ctx, pipe_style type)
 
 	/* Without this check, even just <enter> on command line generates
 	 * tree of three NOPs (!). Which is harmless but annoying.
-	 * IOW: it is safe to do it unconditionally.
-	 * RES_NONE case is for "for a in; do ..." (empty IN set)
-	 * and other cases to work. */
+	 * IOW: it is safe to do it unconditionally. */
 	if (not_null
 #if ENABLE_HUSH_IF
 	 || ctx->ctx_res_w == RES_FI
@@ -4048,7 +4048,7 @@ static void done_pipe(struct parse_context *ctx, pipe_style type)
 		ctx->pipe->next = new_p;
 		ctx->pipe = new_p;
 		/* RES_THEN, RES_DO etc are "sticky" -
-		 * they remain set for commands inside if/while.
+		 * they remain set for pipes inside if/while.
 		 * This is used to control execution.
 		 * RES_FOR and RES_IN are NOT sticky (needed to support
 		 * cases where variable or value happens to match a keyword):
@@ -4304,7 +4304,7 @@ static int done_word(o_string *word, struct parse_context *ctx)
 		 && ctx->ctx_res_w != RES_IN
 # endif
 		) {
-			debug_printf_parse(": checking '%s' for reserved-ness\n", word->data);
+			debug_printf_parse("checking '%s' for reserved-ness\n", word->data);
 			if (reserved_word(word, ctx)) {
 				o_reset_to_empty_unquoted(word);
 				debug_printf_parse("done_word return %d\n",
@@ -4775,6 +4775,14 @@ static int parse_group(o_string *dest, struct parse_context *ctx,
 	if (ch == '(') {
 		endch = ')';
 		command->grp_type = GRP_SUBSHELL;
+	} else {
+		/* bash does not allow "{echo...", requires whitespace */
+		ch = i_getch(input);
+		if (ch != ' ' && ch != '\t' && ch != '\n') {
+			syntax_error_unexpected_ch(ch);
+			return 1;
+		}
+		nommu_addchr(&ctx->as_string, ch);
 	}
 
 	{
@@ -5352,13 +5360,11 @@ static struct pipe *parse_stream(char **pstring,
 		if (end_trigger && end_trigger == ch
 		 && (heredoc_cnt == 0 || end_trigger != ';')
 		) {
-//TODO: disallow "{ cmd }" without semicolon
 			if (heredoc_cnt) {
 				/* This is technically valid:
 				 * { cat <<HERE; }; echo Ok
 				 * heredoc
 				 * heredoc
-				 * heredoc
 				 * HERE
 				 * but we don't support this.
 				 * We require heredoc to be in enclosing {}/(),
@@ -5370,6 +5376,15 @@ static struct pipe *parse_stream(char **pstring,
 			if (done_word(&dest, &ctx)) {
 				goto parse_error;
 			}
+			/* Disallow "{ cmd }" without semicolon or & */
+			//debug_printf_parse("null pi %d\n", IS_NULL_PIPE(ctx.pipe))
+			//debug_printf_parse("null cmd %d\n", IS_NULL_CMD(ctx.command))
+			if (ch == '}'
+			 && !(IS_NULL_PIPE(ctx.pipe) && IS_NULL_CMD(ctx.command))
+			) {
+				syntax_error_unexpected_ch(ch);
+				goto parse_error;
+			}
 			done_pipe(&ctx, PIPE_SEQ);
 			dest.o_assignment = MAYBE_ASSIGNMENT;
 			/* Do we sit outside of any if's, loops or case's? */