Revise fix for bug 32178 (spaces at end of log msg).

The loop in the earlier patch would invoke undefined behavior in two
ways: First, it would check whether it was looking at a space before
it checked whether the pointer was in-range.  Second, it would let a
pointer reach a position _before_ the start of a string, which is
not allowed.

I've removed the assertion about empty messages: empty messages can
be their own warning IMO.

I've also added tests for this formatting code, to make sure it
actually works.

2 files changed, 24 insertions, 14 deletions

diff --git a/src/feature/control/control_events.c b/src/feature/control/control_events.c
index 57cfb0d026..0dd52659ec 100644
--- a/src/feature/control/control_events.c
+++ b/src/feature/control/control_events.c
@@ -1352,6 +1352,27 @@ enable_control_logging(void)
     tor_assert(0);
 }
 
+/** Remove newline and carriage-return characters from @a msg, replacing them
+ * with spaces, and discarding any that appear at the end of the message */
+void
+control_logmsg_strip_newlines(char *msg)
+{
+  char *cp;
+  for (cp = msg; *cp; ++cp) {
+    if (*cp == '\r' || *cp == '\n') {
+      *cp = ' ';
+    }
+  }
+  if (cp == msg)
+    return;
+  /* Remove trailing spaces */
+  for (--cp; *cp == ' '; --cp) {
+    *cp = '\0';
+    if (cp == msg)
+      break;
+  }
+}
+
 /** We got a log message: tell any interested control connections. */
 void
 control_event_logmsg(int severity, log_domain_mask_t domain, const char *msg)
@@ -1380,21 +1401,8 @@ control_event_logmsg(int severity, log_domain_mask_t domain, const char *msg)
     char *b = NULL;
     const char *s;
     if (strchr(msg, '\n')) {
-      char *cp;
       b = tor_strdup(msg);
-      for (cp = b; *cp; ++cp)
-        if (*cp == '\r' || *cp == '\n')
-          *cp = ' ';
-
-      /* Remove trailing spaces */
-      for (--cp; *cp == ' ' && cp >= b; --cp)
-        *cp = '\0';
-
-      if ( cp == b ){
-        ++disable_log_messages;
-        tor_assert_nonfatal(*b);
-        --disable_log_messages;
-      }
+      control_logmsg_strip_newlines(b);
     }
     switch (severity) {
       case LOG_DEBUG: s = "DEBUG"; break;
diff --git a/src/feature/control/control_events.h b/src/feature/control/control_events.h
index 6e3cfef4e9..0ac233cc6e 100644
--- a/src/feature/control/control_events.h
+++ b/src/feature/control/control_events.h
@@ -341,6 +341,8 @@ struct control_event_t {
 
 extern const struct control_event_t control_event_table[];
 
+void control_logmsg_strip_newlines(char *msg);
+
 #ifdef TOR_UNIT_TESTS
 MOCK_DECL(STATIC void,
           send_control_event_string,(uint16_t event, const char *msg));