[tor/master] Manually fix some control replies - tor-commits

3 May 2019

commit 769eb07a7a3bd93091ca1677564d3de62e3c5c2c
Author: Taylor Yu catalyst@torproject.org
Date:   Wed Apr 10 15:11:36 2019 -0500
Manually fix some control replies
Manually fix up some reply-generating code that the Coccinelle scripts
    won't match.  Some more complicated ones remain -- these are mostly
    ones that accumulate data to send, and then call connection_buf_add()
    or connection_write_str_to_buf() directly.
---
 src/feature/control/control_cmd.c     | 53 +++++++++++++++++------------------
 src/feature/control/control_getinfo.c | 12 ++------
 2 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index d69177f4c..5aa828ca5 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -355,7 +355,6 @@ handle_control_loadconf(control_connection_t *conn,
 {
   setopt_err_t retval;
   char *errstring = NULL;
-  const char *msg = NULL;
retval = options_init_from_string(NULL, args->cmddata,
                                     CMD_RUN_TOR, NULL, &errstring);
@@ -365,31 +364,29 @@ handle_control_loadconf(control_connection_t *conn,
              "Controller gave us config file that didn't validate: %s",
              errstring);
+#define SEND_ERRMSG(code, msg)                          \
+  control_printf_endreply(conn, code, msg "%s%s",       \
+                          errstring ? ": " : "",        \
+                          errstring ? errstring : "")
   switch (retval) {
   case SETOPT_ERR_PARSE:
-    msg = "552 Invalid config file";
+    SEND_ERRMSG(552, "Invalid config file");
     break;
   case SETOPT_ERR_TRANSITION:
-    msg = "553 Transition not allowed";
+    SEND_ERRMSG(553, "Transition not allowed");
     break;
   case SETOPT_ERR_SETTING:
-    msg = "553 Unable to set option";
+    SEND_ERRMSG(553, "Unable to set option");
     break;
   case SETOPT_ERR_MISC:
   default:
-    msg = "550 Unable to load config";
+    SEND_ERRMSG(550, "Unable to load config");
     break;
   case SETOPT_OK:
-    break;
-  }
-  if (msg) {
-    if (errstring)
-      connection_printf_to_buf(conn, "%s: %s\r\n", msg, errstring);
-    else
-      connection_printf_to_buf(conn, "%s\r\n", msg);
-  } else {
     send_control_done(conn);
+    break;
   }
+#undef SEND_ERRMSG
   tor_free(errstring);
   return 0;
 }
@@ -600,30 +597,32 @@ control_setconf_helper(control_connection_t *conn,
opt_err = options_trial_assign(lines, flags, &errstring);
   {
-    const char *msg;
+#define SEND_ERRMSG(code, msg)                                  \
+    control_printf_endreply(conn, code, msg ": %s", errstring);
+
     switch (opt_err) {
       case SETOPT_ERR_MISC:
-        msg = "552 Unrecognized option";
+        SEND_ERRMSG(552, "Unrecognized option");
         break;
       case SETOPT_ERR_PARSE:
-        msg = "513 Unacceptable option value";
+        SEND_ERRMSG(513, "Unacceptable option value");
         break;
       case SETOPT_ERR_TRANSITION:
-        msg = "553 Transition not allowed";
+        SEND_ERRMSG(553, "Transition not allowed");
         break;
       case SETOPT_ERR_SETTING:
       default:
-        msg = "553 Unable to set option";
+        SEND_ERRMSG(553, "Unable to set option");
         break;
       case SETOPT_OK:
         config_free_lines(lines);
         send_control_done(conn);
         return 0;
     }
+#undef SEND_ERRMSG
     log_warn(LD_CONTROL,
              "Controller gave us config lines that didn't validate: %s",
              errstring);
-    connection_printf_to_buf(conn, "%s: %s\r\n", msg, errstring);
     config_free_lines(lines);
     tor_free(errstring);
     return 0;
@@ -1309,15 +1308,13 @@ handle_control_protocolinfo(control_connection_t *conn,
       smartlist_free(mlist);
     }
-    connection_printf_to_buf(conn,
-                             "250-PROTOCOLINFO 1\r\n"
-                             "250-AUTH METHODS=%s%s%s\r\n"
-                             "250-VERSION Tor=%s\r\n"
-                             "250 OK\r\n",
-                             methods,
-                             cookies?" COOKIEFILE=":"",
-                             cookies?esc_cfile:"",
-                             escaped(VERSION));
+    control_write_midreply(conn, 250, "PROTOCOLINFO 1");
+    control_printf_midreply(conn, 250, "AUTH METHODS=%s%s%s", methods,
+                            cookies?" COOKIEFILE=":"",
+                            cookies?esc_cfile:"");
+    control_printf_midreply(conn, 250, "VERSION Tor=%s", escaped(VERSION));
+    send_control_done(conn);
+
     tor_free(methods);
     tor_free(cfile);
     tor_free(abs_cfile);
diff --git a/src/feature/control/control_getinfo.c b/src/feature/control/control_getinfo.c
index 6f5e69dc0..c10229bfb 100644
--- a/src/feature/control/control_getinfo.c
+++ b/src/feature/control/control_getinfo.c
@@ -1639,16 +1639,10 @@ handle_control_getinfo(control_connection_t *conn,
     char *k = smartlist_get(answers, i);
     char *v = smartlist_get(answers, i+1);
     if (!strchr(v, '\n') && !strchr(v, '\r')) {
-      connection_printf_to_buf(conn, "250-%s=", k);
-      connection_write_str_to_buf(v, conn);
-      connection_write_str_to_buf("\r\n", conn);
+      control_printf_midreply(conn, 250, "%s=%s", k, v);
     } else {
-      char *esc = NULL;
-      size_t esc_len;
-      esc_len = write_escaped_data(v, strlen(v), &esc);
-      connection_printf_to_buf(conn, "250+%s=\r\n", k);
-      connection_buf_add(esc, esc_len, TO_CONN(conn));
-      tor_free(esc);
+      control_printf_datareply(conn, 250, "%s=", k);
+      control_write_data(conn, v);
     }
   }
   connection_write_str_to_buf("250 OK\r\n", conn);

    