From a2f6ea968c3f5a84d0bd5aef7f15aa39b088db18 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@pond.sub.org>
Date: Sun, 27 Dec 2015 15:54:10 +0100
Subject: [PATCH] client: Improve the client's messages

Use a "Warning: " prefix for server output violating the protocol and
for rogue redirections and executes.  Don't shout "WARNING!"

In redir_authorized(), check for server issues (conflicting
redirections, rogue redirections and executes) before enforcing
restrictions (restricted mode, executing batch file), so server issues
aren't masked.

Surprisingly, popen() may not set errno on failure.  Avoid reporting a
bogus errno in dopipe().

doexecute() complains about an "execute file".  We call that a "batch
file" elsewhere.  Reword for consistency.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
---
 src/client/servcmd.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/src/client/servcmd.c b/src/client/servcmd.c
index b8264651..5bd4fff6 100644
--- a/src/client/servcmd.c
+++ b/src/client/servcmd.c
@@ -69,7 +69,8 @@ servercmd(int code, char *arg, int len)
     switch (code) {
     case C_PROMPT:
 	if (sscanf(arg, "%d %d", &nmin, &nbtu) != 2) {
-	    fprintf(stderr, "prompt: bad server prompt %s", arg);
+	    fprintf(stderr, "Warning: server sent malformed prompt %s",
+		    arg);
 	}
 	snprintf(the_prompt, sizeof(the_prompt), "[%d:%d] Command : ",
 		 nmin, nbtu);
@@ -162,6 +163,19 @@ redir_authorized(char *arg, char *attempt, int expected)
 {
     size_t seen = seen_input(arg);
 
+    if (!expected) {
+	fprintf(stderr, "Warning: dropped conflicting %s %s",
+		attempt, arg);
+	return 0;
+    }
+
+    if (!seen || (input_to_forget && input_to_forget != seen)) {
+	fprintf(stderr, "Warning: server attempted to %s %s",
+		attempt, arg);
+	return 0;
+    }
+    input_to_forget = seen;
+
     if (restricted) {
 	fprintf(stderr, "Can't %s in restricted mode\n", attempt);
 	return 0;
@@ -171,19 +185,6 @@ redir_authorized(char *arg, char *attempt, int expected)
 	fprintf(stderr, "Can't %s in a batch file\n", attempt);
 	return 0;
     }
-
-    if (!expected) {
-	fprintf(stderr, "WARNING!  Server attempted to %s unexpectedly\n",
-		attempt);
-	return 0;
-    }
-
-    if (!seen || (input_to_forget && input_to_forget != seen)) {
-	fprintf(stderr, "WARNING!  Server attempted to %s %s",
-		attempt, arg);
-	return 0;
-    }
-    input_to_forget = seen;
     return 1;
 }
 
@@ -196,7 +197,7 @@ doredir(char *p)
     if (!redir_authorized(p, "redirect to file", !redir_fp))
 	return;
     if (*p++ != '>') {
-	fprintf(stderr, "WARNING!  Weird redirection %s", p);
+	fprintf(stderr, "Warning: dropped weird redirection %s", p);
 	return;
     }
 
@@ -231,7 +232,7 @@ dopipe(char *p)
     if (!redir_authorized(p, "pipe to shell command", !redir_fp))
 	return;
     if (*p++ != '|') {
-	fprintf(stderr, "WARNING!  Weird pipe %s", p);
+	fprintf(stderr, "Warning: dropped weird pipe %s", p);
 	return;
     }
 
@@ -245,9 +246,10 @@ dopipe(char *p)
     p[strlen(p) - 1] = 0;
 
     redir_is_pipe = 1;
+    errno = 0;
     if ((redir_fp = popen(p, "w")) == NULL) {
-	fprintf(stderr, "Can't redirect to pipe %s: %s\n",
-		p, strerror(errno));
+	fprintf(stderr, "Can't redirect to pipe %s%s%s\n",
+		p, errno ? ": " : "", errno ? strerror(errno) : "");
     }
 }
 
@@ -266,7 +268,7 @@ doexecute(char *p)
     }
 
     if ((fd = open(p, O_RDONLY)) < 0) {
-	fprintf(stderr, "Can't open execute file %s: %s\n",
+	fprintf(stderr, "Can't open batch file %s: %s\n",
 		p, strerror(errno));
 	return -1;
     }