Merge branch 'tor-github/pr/980'

Signed-off-by: David Goulet <dgoulet@torproject.org>

27 files changed, 1488 insertions, 872 deletions

diff --git a/changes/ticket29984 b/changes/ticket29984
new file mode 100644
index 0000000000..8631dff27b
--- /dev/null
+++ b/changes/ticket29984
@@ -0,0 +1,5 @@
+  o Minor bugfixes (controller protocol):
+    - Teach the controller parser to correctly distinguish an object
+      preceded by an argument list from one without. Previously, it
+      couldn't distinguish an argument list from the first line of a
+      multiline object. Fixes bug 29984; bugfix on 0.2.3.8-alpha.
diff --git a/changes/ticket30091 b/changes/ticket30091
new file mode 100644
index 0000000000..968ea01f4a
--- /dev/null
+++ b/changes/ticket30091
@@ -0,0 +1,4 @@
+  o Major features (controller protocol):
+    - Controller commands are now parsed using a generalized parsing
+      subsystem. Previously, each controller command was responsible for
+      parsing its own input.  Closes ticket 30091.
diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index 37f11bb440..677e7f0d42 100644
--- a/scripts/maint/practracker/exceptions.txt
+++ b/scripts/maint/practracker/exceptions.txt
@@ -54,9 +54,9 @@ problem function-size /src/app/main/main.c:sandbox_init_filter() 291
 problem function-size /src/app/main/main.c:run_tor_main_loop() 105
 problem function-size /src/app/main/ntmain.c:nt_service_install() 125
 problem include-count /src/app/main/shutdown.c 52
-problem file-size /src/core/mainloop/connection.c 5558
+problem file-size /src/core/mainloop/connection.c 5559
 problem include-count /src/core/mainloop/connection.c 61
-problem function-size /src/core/mainloop/connection.c:connection_free_minimal() 184
+problem function-size /src/core/mainloop/connection.c:connection_free_minimal() 185
 problem function-size /src/core/mainloop/connection.c:connection_listener_new() 328
 problem function-size /src/core/mainloop/connection.c:connection_handle_listener_read() 161
 problem function-size /src/core/mainloop/connection.c:connection_connect_sockaddr() 103
@@ -152,7 +152,7 @@ problem function-size /src/feature/control/control_cmd.c:handle_control_add_onio
 problem function-size /src/feature/control/control_cmd.c:add_onion_helper_keyarg() 125
 problem function-size /src/feature/control/control_cmd.c:handle_control_command() 104
 problem function-size /src/feature/control/control_events.c:control_event_stream_status() 119
-problem include-count /src/feature/control/control_getinfo.c 52
+problem include-count /src/feature/control/control_getinfo.c 53
 problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_misc() 109
 problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_dir() 304
 problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_events() 236
diff --git a/src/core/include.am b/src/core/include.am
index 9824601725..b927f17a9c 100644
--- a/src/core/include.am
+++ b/src/core/include.am
@@ -298,6 +298,7 @@ noinst_HEADERS +=					\
 	src/feature/control/control.h			\
 	src/feature/control/control_auth.h		\
 	src/feature/control/control_cmd.h		\
+	src/feature/control/control_cmd_args_st.h	\
 	src/feature/control/control_connection_st.h	\
 	src/feature/control/control_events.h	        \
 	src/feature/control/control_fmt.h		\
diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c
index 51c19b4c4c..30504e4edb 100644
--- a/src/core/mainloop/connection.c
+++ b/src/core/mainloop/connection.c
@@ -697,6 +697,7 @@ connection_free_minimal(connection_t *conn)
     control_connection_t *control_conn = TO_CONTROL_CONN(conn);
     tor_free(control_conn->safecookie_client_hash);
     tor_free(control_conn->incoming_cmd);
+    tor_free(control_conn->current_cmd);
     if (control_conn->ephemeral_onion_services) {
       SMARTLIST_FOREACH(control_conn->ephemeral_onion_services, char *, cp, {
         memwipe(cp, 0, strlen(cp));
diff --git a/src/feature/control/control.c b/src/feature/control/control.c
index 41e21c0a14..23ef83ef95 100644
--- a/src/feature/control/control.c
+++ b/src/feature/control/control.c
@@ -33,6 +33,7 @@
  **/
 
 #define CONTROL_MODULE_PRIVATE
+#define CONTROL_PRIVATE
 
 #include "core/or/or.h"
 #include "app/config/config.h"
@@ -274,6 +275,44 @@ peek_connection_has_http_command(connection_t *conn)
   return peek_buf_has_http_command(conn->inbuf);
 }
 
+/**
+ * Helper: take a nul-terminated command of given length, and find where the
+ * command starts and the arguments begin.  Separate them, allocate a new
+ * string in <b>current_cmd_out</b> for the command, and return a pointer
+ * to the arguments.
+ **/
+STATIC char *
+control_split_incoming_command(char *incoming_cmd,
+                               size_t *data_len,
+                               char **current_cmd_out)
+{
+  const bool is_multiline = *data_len && incoming_cmd[0] == '+';
+  size_t cmd_len = 0;
+  while (cmd_len < *data_len
+         && !TOR_ISSPACE(incoming_cmd[cmd_len]))
+    ++cmd_len;
+
+  *current_cmd_out = tor_memdup_nulterm(incoming_cmd, cmd_len);
+  char *args = incoming_cmd+cmd_len;
+  tor_assert(*data_len>=cmd_len);
+  *data_len -= cmd_len;
+  if (is_multiline) {
+    // Only match horizontal space: any line after the first is data,
+    // not arguments.
+    while ((*args == '\t' || *args == ' ') && *data_len) {
+      ++args;
+      --*data_len;
+    }
+  } else {
+    while (TOR_ISSPACE(*args) && *data_len) {
+      ++args;
+      --*data_len;
+    }
+  }
+
+  return args;
+}
+
 static const char CONTROLPORT_IS_NOT_AN_HTTP_PROXY_MSG[] =
   "HTTP/1.0 501 Tor ControlPort is not an HTTP proxy"
   "\r\nContent-Type: text/html; charset=iso-8859-1\r\n\r\n"
@@ -308,7 +347,6 @@ connection_control_process_inbuf(control_connection_t *conn)
 {
   size_t data_len;
   uint32_t cmd_data_len;
-  int cmd_len;
   char *args;
 
   tor_assert(conn);
@@ -400,22 +438,15 @@ connection_control_process_inbuf(control_connection_t *conn)
     /* Otherwise, read another line. */
   }
   data_len = conn->incoming_cmd_cur_len;
+
   /* Okay, we now have a command sitting on conn->incoming_cmd. See if we
    * recognize it.
    */
-  cmd_len = 0;
-  while ((size_t)cmd_len < data_len
-         && !TOR_ISSPACE(conn->incoming_cmd[cmd_len]))
-    ++cmd_len;
-
-  conn->incoming_cmd[cmd_len]='\0';
-  args = conn->incoming_cmd+cmd_len+1;
-  tor_assert(data_len>(size_t)cmd_len);
-  data_len -= (cmd_len+1); /* skip the command and NUL we added after it */
-  while (TOR_ISSPACE(*args)) {
-    ++args;
-    --data_len;
-  }
+  tor_free(conn->current_cmd);
+  args = control_split_incoming_command(conn->incoming_cmd, &data_len,
+                                        &conn->current_cmd);
+  if (BUG(!conn->current_cmd))
+    return -1;
 
   /* If the connection is already closing, ignore further commands */
   if (TO_CONN(conn)->marked_for_close) {
@@ -423,14 +454,14 @@ connection_control_process_inbuf(control_connection_t *conn)
   }
 
   /* Otherwise, Quit is always valid. */
-  if (!strcasecmp(conn->incoming_cmd, "QUIT")) {
+  if (!strcasecmp(conn->current_cmd, "QUIT")) {
     connection_write_str_to_buf("250 closing connection\r\n", conn);
     connection_mark_and_flush(TO_CONN(conn));
     return 0;
   }
 
   if (conn->base_.state == CONTROL_CONN_STATE_NEEDAUTH &&
-      !is_valid_initial_command(conn, conn->incoming_cmd)) {
+      !is_valid_initial_command(conn, conn->current_cmd)) {
     connection_write_str_to_buf("514 Authentication required.\r\n", conn);
     connection_mark_for_close(TO_CONN(conn));
     return 0;
diff --git a/src/feature/control/control.h b/src/feature/control/control.h
index 3083837931..8d3595d2ed 100644
--- a/src/feature/control/control.h
+++ b/src/feature/control/control.h
@@ -60,4 +60,10 @@ int get_cached_network_liveness(void);
 void set_cached_network_liveness(int liveness);
 #endif /* defined(CONTROL_MODULE_PRIVATE) */
 
+#ifdef CONTROL_PRIVATE
+STATIC char *control_split_incoming_command(char *incoming_cmd,
+                                            size_t *data_len,
+                                            char **current_cmd_out);
+#endif
+
 #endif /* !defined(TOR_CONTROL_H) */
diff --git a/src/feature/control/control_auth.c b/src/feature/control/control_auth.c
index 927115a308..a86442c21f 100644
--- a/src/feature/control/control_auth.c
+++ b/src/feature/control/control_auth.c
@@ -11,12 +11,16 @@
 #include "app/config/config.h"
 #include "core/mainloop/connection.h"
 #include "feature/control/control.h"
+#include "feature/control/control_cmd.h"
 #include "feature/control/control_auth.h"
+#include "feature/control/control_cmd_args_st.h"
 #include "feature/control/control_connection_st.h"
 #include "feature/control/control_fmt.h"
 #include "lib/crypt_ops/crypto_rand.h"
 #include "lib/crypt_ops/crypto_util.h"
 #include "lib/encoding/confline.h"
+#include "lib/encoding/kvline.h"
+#include "lib/encoding/qstring.h"
 
 #include "lib/crypt_ops/crypto_s2k.h"
 
@@ -116,12 +120,19 @@ decode_hashed_passwords(config_line_t *passwords)
   return NULL;
 }
 
+const control_cmd_syntax_t authchallenge_syntax = {
+   .min_args = 1,
+   .max_args = 1,
+   .accept_keywords=true,
+   .kvline_flags=KV_OMIT_KEYS|KV_QUOTED_QSTRING,
+   .store_raw_body=true
+};
+
 /** Called when we get an AUTHCHALLENGE command. */
 int
-handle_control_authchallenge(control_connection_t *conn, uint32_t len,
-                             const char *body)
+handle_control_authchallenge(control_connection_t *conn,
+                             const control_cmd_args_t *args)
 {
-  const char *cp = body;
   char *client_nonce;
   size_t client_nonce_len;
   char server_hash[DIGEST256_LEN];
@@ -129,63 +140,50 @@ handle_control_authchallenge(control_connection_t *conn, uint32_t len,
   char server_nonce[SAFECOOKIE_SERVER_NONCE_LEN];
   char server_nonce_encoded[(2*SAFECOOKIE_SERVER_NONCE_LEN) + 1];
 
-  cp += strspn(cp, " \t\n\r");
-  if (!strcasecmpstart(cp, "SAFECOOKIE")) {
-    cp += strlen("SAFECOOKIE");
-  } else {
+  if (strcasecmp(smartlist_get(args->args, 0), "SAFECOOKIE")) {
     connection_write_str_to_buf("513 AUTHCHALLENGE only supports SAFECOOKIE "
                                 "authentication\r\n", conn);
-    connection_mark_for_close(TO_CONN(conn));
-    return -1;
+    goto fail;
   }
-
   if (!authentication_cookie_is_set) {
     connection_write_str_to_buf("515 Cookie authentication is disabled\r\n",
                                 conn);
-    connection_mark_for_close(TO_CONN(conn));
-    return -1;
+    goto fail;
+  }
+  if (args->kwargs == NULL || args->kwargs->next != NULL) {
+    /*    connection_write_str_to_buf("512 AUTHCHALLENGE requires exactly "
+                                "2 arguments.\r\n", conn);
+    */
+    connection_printf_to_buf(conn,
+                             "512 AUTHCHALLENGE dislikes argument list %s\r\n",
+                             escaped(args->raw_body));
+    goto fail;
+  }
+  if (strcmp(args->kwargs->key, "")) {
+    connection_write_str_to_buf("512 AUTHCHALLENGE does not accept keyword "
+                                "arguments.\r\n", conn);
+    goto fail;
   }
 
-  cp += strspn(cp, " \t\n\r");
-  if (*cp == '"') {
-    const char *newcp =
-      decode_escaped_string(cp, len - (cp - body),
-                            &client_nonce, &client_nonce_len);
-    if (newcp == NULL) {
-      connection_write_str_to_buf("513 Invalid quoted client nonce\r\n",
-                                  conn);
-      connection_mark_for_close(TO_CONN(conn));
-      return -1;
-    }
-    cp = newcp;
+  bool contains_quote = strchr(args->raw_body, '\"');
+  if (contains_quote) {
+    /* The nonce was quoted */
+    client_nonce = tor_strdup(args->kwargs->value);
+    client_nonce_len = strlen(client_nonce);
   } else {
-    size_t client_nonce_encoded_len = strspn(cp, "0123456789ABCDEFabcdef");
-
-    client_nonce_len = client_nonce_encoded_len / 2;
-    client_nonce = tor_malloc_zero(client_nonce_len);
-
-    if (base16_decode(client_nonce, client_nonce_len,
-                      cp, client_nonce_encoded_len)
-                      != (int) client_nonce_len) {
+    /* The nonce was should be in hex. */
+    const char *hex_nonce = args->kwargs->value;
+    client_nonce_len = strlen(hex_nonce) / 2;
+    client_nonce = tor_malloc(client_nonce_len);
+    if (base16_decode(client_nonce, client_nonce_len, hex_nonce,
+                      strlen(hex_nonce)) != (int)client_nonce_len) {
       connection_write_str_to_buf("513 Invalid base16 client nonce\r\n",
                                   conn);
-      connection_mark_for_close(TO_CONN(conn));
       tor_free(client_nonce);
-      return -1;
+      goto fail;
     }
-
-    cp += client_nonce_encoded_len;
   }
 
-  cp += strspn(cp, " \t\n\r");
-  if (*cp != '\0' ||
-      cp != body + len) {
-    connection_write_str_to_buf("513 Junk at end of AUTHCHALLENGE command\r\n",
-                                conn);
-    connection_mark_for_close(TO_CONN(conn));
-    tor_free(client_nonce);
-    return -1;
-  }
   crypto_rand(server_nonce, SAFECOOKIE_SERVER_NONCE_LEN);
 
   /* Now compute and send the server-to-controller response, and the
@@ -233,38 +231,56 @@ handle_control_authchallenge(control_connection_t *conn, uint32_t len,
 
   tor_free(client_nonce);
   return 0;
+ fail:
+  connection_mark_for_close(TO_CONN(conn));
+  return -1;
 }
 
+const control_cmd_syntax_t authenticate_syntax = {
+   .max_args = 0,
+   .accept_keywords=true,
+   .kvline_flags=KV_OMIT_KEYS|KV_QUOTED_QSTRING,
+   .store_raw_body=true
+};
+
 /** Called when we get an AUTHENTICATE message.  Check whether the
  * authentication is valid, and if so, update the connection's state to
  * OPEN.  Reply with DONE or ERROR.
  */
 int
-handle_control_authenticate(control_connection_t *conn, uint32_t len,
-                            const char *body)
+handle_control_authenticate(control_connection_t *conn,
+                            const control_cmd_args_t *args)
 {
-  int used_quoted_string = 0;
+  bool used_quoted_string = false;
   const or_options_t *options = get_options();
   const char *errstr = "Unknown error";
   char *password;
   size_t password_len;
-  const char *cp;
-  int i;
   int bad_cookie=0, bad_password=0;
   smartlist_t *sl = NULL;
 
-  if (!len) {
+  if (args->kwargs == NULL) {
     password = tor_strdup("");
     password_len = 0;
-  } else if (TOR_ISXDIGIT(body[0])) {
-    cp = body;
-    while (TOR_ISXDIGIT(*cp))
-      ++cp;
-    i = (int)(cp - body);
-    tor_assert(i>0);
-    password_len = i/2;
-    password = tor_malloc(password_len + 1);
-    if (base16_decode(password, password_len+1, body, i)
+  } else if (args->kwargs->next) {
+    connection_write_str_to_buf(
+             "512 Too many arguments to AUTHENTICATE.\r\n", conn);
+    connection_mark_for_close(TO_CONN(conn));
+    return 0;
+  } else if (strcmp(args->kwargs->key, "")) {
+    connection_write_str_to_buf(
+             "512 AUTHENTICATE does not accept keyword arguments.\r\n", conn);
+    connection_mark_for_close(TO_CONN(conn));
+    return 0;
+  } else if (strchr(args->raw_body, '\"')) {
+    used_quoted_string = true;
+    password = tor_strdup(args->kwargs->value);
+    password_len = strlen(password);
+  } else {
+    const char *hex_passwd = args->kwargs->value;
+    password_len = strlen(hex_passwd) / 2;
+    password = tor_malloc(password_len+1);
+    if (base16_decode(password, password_len+1, hex_passwd, strlen(hex_passwd))
                       != (int) password_len) {
       connection_write_str_to_buf(
             "551 Invalid hexadecimal encoding.  Maybe you tried a plain text "
@@ -274,14 +290,6 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len,
       tor_free(password);
       return 0;
     }
-  } else {
-    if (!decode_escaped_string(body, len, &password, &password_len)) {
-      connection_write_str_to_buf("551 Invalid quoted string.  You need "
-            "to put the password in double quotes.\r\n", conn);
-      connection_mark_for_close(TO_CONN(conn));
-      return 0;
-    }
-    used_quoted_string = 1;
   }
 
   if (conn->safecookie_client_hash != NULL) {
diff --git a/src/feature/control/control_auth.h b/src/feature/control/control_auth.h
index f436482e4a..246e18ccbc 100644
--- a/src/feature/control/control_auth.h
+++ b/src/feature/control/control_auth.h
@@ -12,16 +12,21 @@
 #ifndef TOR_CONTROL_AUTH_H
 #define TOR_CONTROL_AUTH_H
 
+struct control_cmd_args_t;
+struct control_cmd_syntax_t;
+
 int init_control_cookie_authentication(int enabled);
 char *get_controller_cookie_file_name(void);
 struct config_line_t;
 smartlist_t *decode_hashed_passwords(struct config_line_t *passwords);
 
-int handle_control_authchallenge(control_connection_t *conn, uint32_t len,
-                                 const char *body);
+int handle_control_authchallenge(control_connection_t *conn,
+                                 const struct control_cmd_args_t *args);
 int handle_control_authenticate(control_connection_t *conn,
-                           uint32_t cmd_data_len,
-                           const char *args);
+                                const struct control_cmd_args_t *args);
 void control_auth_free_all(void);
 
+extern const struct control_cmd_syntax_t authchallenge_syntax;
+extern const struct control_cmd_syntax_t authenticate_syntax;
+
 #endif /* !defined(TOR_CONTROL_AUTH_H) */
diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index 95cf0d561e..9afa734d86 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -40,11 +40,13 @@
 #include "lib/crypt_ops/crypto_rand.h"
 #include "lib/crypt_ops/crypto_util.h"
 #include "lib/encoding/confline.h"
+#include "lib/encoding/kvline.h"
 
 #include "core/or/cpath_build_state_st.h"
 #include "core/or/entry_connection_st.h"
 #include "core/or/origin_circuit_st.h"
 #include "core/or/socks_request_st.h"
+#include "feature/control/control_cmd_args_st.h"
 #include "feature/control/control_connection_st.h"
 #include "feature/nodelist/node_st.h"
 #include "feature/nodelist/routerinfo_st.h"
@@ -52,39 +54,238 @@
 #include "feature/rend/rend_encoded_v2_service_descriptor_st.h"
 #include "feature/rend/rend_service_descriptor_st.h"
 
-static int control_setconf_helper(control_connection_t *conn, uint32_t len,
-                                  char *body,
+static int control_setconf_helper(control_connection_t *conn,
+                                  const control_cmd_args_t *args,
                                   int use_defaults);
 
 /** Yield true iff <b>s</b> is the state of a control_connection_t that has
  * finished authentication and is accepting commands. */
 #define STATE_IS_OPEN(s) ((s) == CONTROL_CONN_STATE_OPEN)
 
+/**
+ * Release all storage held in <b>args</b>
+ **/
+void
+control_cmd_args_free_(control_cmd_args_t *args)
+{
+  if (! args)
+    return;
+
+  if (args->args) {
+    SMARTLIST_FOREACH(args->args, char *, c, tor_free(c));
+    smartlist_free(args->args);
+  }
+  config_free_lines(args->kwargs);
+  tor_free(args->cmddata);
+
+  tor_free(args);
+}
+
+/** Erase all memory held in <b>args</b>. */
+void
+control_cmd_args_wipe(control_cmd_args_t *args)
+{
+  if (!args)
+    return;
+
+  if (args->args) {
+    SMARTLIST_FOREACH(args->args, char *, c, memwipe(c, 0, strlen(c)));
+  }
+  for (config_line_t *line = args->kwargs; line; line = line->next) {
+    memwipe(line->key, 0, strlen(line->key));
+    memwipe(line->value, 0, strlen(line->value));
+  }
+  if (args->cmddata)
+    memwipe(args->cmddata, 0, args->cmddata_len);
+}
+
+/**
+ * Return true iff any element of the NULL-terminated <b>array</b> matches
+ * <b>kwd</b>. Case-insensitive.
+ **/
+static bool
+string_array_contains_keyword(const char **array, const char *kwd)
+{
+  for (unsigned i = 0; array[i]; ++i) {
+    if (! strcasecmp(array[i], kwd))
+      return true;
+  }
+  return false;
+}
+
+/** Helper for argument parsing: check whether the keyword arguments just
+ * parsed in <b>result</b> were well-formed according to <b>syntax</b>.
+ *
+ * On success, return 0.  On failure, return -1 and set *<b>error_out</b>
+ * to a newly allocated error string.
+ **/
+static int
+kvline_check_keyword_args(const control_cmd_args_t *result,
+                          const control_cmd_syntax_t *syntax,
+                          char **error_out)
+{
+  if (result->kwargs == NULL) {
+    tor_asprintf(error_out, "Cannot parse keyword argument(s)");
+    return -1;
+  }
+
+  if (! syntax->allowed_keywords) {
+    /* All keywords are permitted. */
+    return 0;
+  }
+
+  /* Check for unpermitted arguments */
+  const config_line_t *line;
+  for (line = result->kwargs; line; line = line->next) {
+    if (! string_array_contains_keyword(syntax->allowed_keywords,
+                                        line->key)) {
+      tor_asprintf(error_out, "Unrecognized keyword argument %s",
+                   escaped(line->key));
+      return -1;
+    }
+  }
+
+  return 0;
+}
+
+/**
+ * Helper: parse the arguments to a command according to <b>syntax</b>.  On
+ * success, set *<b>error_out</b> to NULL and return a newly allocated
+ * control_cmd_args_t.  On failure, set *<b>error_out</b> to newly allocated
+ * error string, and return NULL.
+ **/
+STATIC control_cmd_args_t *
+control_cmd_parse_args(const char *command,
+                       const control_cmd_syntax_t *syntax,
+                       size_t body_len,
+                       const char *body,
+                       char **error_out)
+{
+  *error_out = NULL;
+  control_cmd_args_t *result = tor_malloc_zero(sizeof(control_cmd_args_t));
+  const char *cmdline;
+  char *cmdline_alloc = NULL;
+  tor_assert(syntax->max_args < INT_MAX || syntax->max_args == UINT_MAX);
+
+  result->command = command;
+
+  if (syntax->store_raw_body) {
+    tor_assert(body[body_len] == 0);
+    result->raw_body = body;
+  }
+
+  const char *eol = memchr(body, '\n', body_len);
+  if (syntax->want_cmddata) {
+    if (! eol || (eol+1) == body+body_len) {
+      *error_out = tor_strdup("Empty body");
+      goto err;
+    }
+    cmdline_alloc = tor_memdup_nulterm(body, eol-body);
+    cmdline = cmdline_alloc;
+    ++eol;
+    result->cmddata_len = read_escaped_data(eol, (body+body_len)-eol,
+                                           &result->cmddata);
+  } else {
+    if (eol && (eol+1) != body+body_len) {
+      *error_out = tor_strdup("Unexpected body");
+      goto err;
+    }
+    cmdline = body;
+  }
+
+  result->args = smartlist_new();
+  smartlist_split_string(result->args, cmdline, " ",
+                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK,
+                         (int)(syntax->max_args+1));
+  size_t n_args = smartlist_len(result->args);
+  if (n_args < syntax->min_args) {
+    tor_asprintf(error_out, "Need at least %u argument(s)",
+                 syntax->min_args);
+    goto err;
+  } else if (n_args > syntax->max_args && ! syntax->accept_keywords) {
+    tor_asprintf(error_out, "Cannot accept more than %u argument(s)",
+                 syntax->max_args);
+    goto err;
+  }
+
+  if (n_args > syntax->max_args) {
+    /* We have extra arguments after the positional arguments, and we didn't
+       treat them as an error, so they must count as keyword arguments: Either
+       K=V pairs, or flags, or both. */
+    tor_assert(n_args == syntax->max_args + 1);
+    tor_assert(syntax->accept_keywords);
+    char *remainder = smartlist_pop_last(result->args);
+    result->kwargs = kvline_parse(remainder, syntax->kvline_flags);
+    tor_free(remainder);
+    if (kvline_check_keyword_args(result, syntax, error_out) < 0) {
+      goto err;
+    }
+  }
+
+  tor_assert_nonfatal(*error_out == NULL);
+  goto done;
+ err:
+  tor_assert_nonfatal(*error_out != NULL);
+  control_cmd_args_free(result);
+ done:
+  tor_free(cmdline_alloc);
+  return result;
+}
+
+/**
+ * Return true iff <b>lines</b> contains <b>flags</b> as a no-value
+ * (keyword-only) entry.
+ **/
+static bool
+config_lines_contain_flag(const config_line_t *lines, const char *flag)
+{
+  const config_line_t *line = config_line_find_case(lines, flag);
+  return line && !strcmp(line->value, "");
+}
+
+static const control_cmd_syntax_t setconf_syntax = {
+  .max_args=0,
+  .accept_keywords=true,
+  .kvline_flags=KV_OMIT_VALS|KV_QUOTED,
+};
+
 /** Called when we receive a SETCONF message: parse the body and try
  * to update our configuration.  Reply with a DONE or ERROR message.
  * Modifies the contents of body.*/
 static int
-handle_control_setconf(control_connection_t *conn, uint32_t len, char *body)
+handle_control_setconf(control_connection_t *conn,
+                       const control_cmd_args_t *args)
 {
-  return control_setconf_helper(conn, len, body, 0);
+  return control_setconf_helper(conn, args, 0);
 }
 
+static const control_cmd_syntax_t resetconf_syntax = {
+  .max_args=0,
+  .accept_keywords=true,
+  .kvline_flags=KV_OMIT_VALS|KV_QUOTED,
+};
+
 /** Called when we receive a RESETCONF message: parse the body and try
  * to update our configuration.  Reply with a DONE or ERROR message.
  * Modifies the contents of body. */
 static int
-handle_control_resetconf(control_connection_t *conn, uint32_t len, char *body)
+handle_control_resetconf(control_connection_t *conn,
+                         const control_cmd_args_t *args)
 {
-  return control_setconf_helper(conn, len, body, 1);
+  return control_setconf_helper(conn, args, 1);
 }
 
+static const control_cmd_syntax_t getconf_syntax = {
+  .max_args=UINT_MAX
+};
+
 /** Called when we receive a GETCONF message.  Parse the request, and
  * reply with a CONFVALUE or an ERROR message */
 static int
-handle_control_getconf(control_connection_t *conn, uint32_t body_len,
-                       const char *body)
+handle_control_getconf(control_connection_t *conn,
+                       const control_cmd_args_t *args)
 {
-  smartlist_t *questions = smartlist_new();
+  const smartlist_t *questions = args->args;
   smartlist_t *answers = smartlist_new();
   smartlist_t *unrecognized = smartlist_new();
   char *msg = NULL;
@@ -92,9 +293,6 @@ handle_control_getconf(control_connection_t *conn, uint32_t body_len,
   const or_options_t *options = get_options();
   int i, len;
 
-  (void) body_len; /* body is NUL-terminated; so we can ignore len. */
-  smartlist_split_string(questions, body, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
   SMARTLIST_FOREACH_BEGIN(questions, const char *, q) {
     if (!option_is_recognized(q)) {
       smartlist_add(unrecognized, (char*) q);
@@ -139,8 +337,6 @@ handle_control_getconf(control_connection_t *conn, uint32_t body_len,
 
   SMARTLIST_FOREACH(answers, char *, cp, tor_free(cp));
   smartlist_free(answers);
-  SMARTLIST_FOREACH(questions, char *, cp, tor_free(cp));
-  smartlist_free(questions);
   smartlist_free(unrecognized);
 
   tor_free(msg);
@@ -148,17 +344,21 @@ handle_control_getconf(control_connection_t *conn, uint32_t body_len,
   return 0;
 }
 
+static const control_cmd_syntax_t loadconf_syntax = {
+  .want_cmddata = true
+};
+
 /** Called when we get a +LOADCONF message. */
 static int
-handle_control_loadconf(control_connection_t *conn, uint32_t len,
-                         const char *body)
+handle_control_loadconf(control_connection_t *conn,
+                        const control_cmd_args_t *args)
 {
   setopt_err_t retval;
   char *errstring = NULL;
   const char *msg = NULL;
-  (void) len;
 
-  retval = options_init_from_string(NULL, body, CMD_RUN_TOR, NULL, &errstring);
+  retval = options_init_from_string(NULL, args->cmddata,
+                                    CMD_RUN_TOR, NULL, &errstring);
 
   if (retval != SETOPT_OK)
     log_warn(LD_CONTROL,
@@ -194,20 +394,20 @@ handle_control_loadconf(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const control_cmd_syntax_t setevents_syntax = {
+  .max_args = UINT_MAX
+};
+
 /** Called when we get a SETEVENTS message: update conn->event_mask,
  * and reply with DONE or ERROR. */
 static int
-handle_control_setevents(control_connection_t *conn, uint32_t len,
-                         const char *body)
+handle_control_setevents(control_connection_t *conn,
+                         const control_cmd_args_t *args)
 {
   int event_code;
   event_mask_t event_mask = 0;
-  smartlist_t *events = smartlist_new();
+  const smartlist_t *events = args->args;
 
-  (void) len;
-
-  smartlist_split_string(events, body, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
   SMARTLIST_FOREACH_BEGIN(events, const char *, ev)
     {
       if (!strcasecmp(ev, "EXTENDED") ||
@@ -229,16 +429,12 @@ handle_control_setevents(control_connection_t *conn, uint32_t len,
         if (event_code == -1) {
           connection_printf_to_buf(conn, "552 Unrecognized event \"%s\"\r\n",
                                    ev);
-          SMARTLIST_FOREACH(events, char *, e, tor_free(e));
-          smartlist_free(events);
           return 0;
         }
       }
       event_mask |= (((event_mask_t)1) << event_code);
     }
   SMARTLIST_FOREACH_END(ev);
-  SMARTLIST_FOREACH(events, char *, e, tor_free(e));
-  smartlist_free(events);
 
   conn->event_mask = event_mask;
 
@@ -247,15 +443,19 @@ handle_control_setevents(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const control_cmd_syntax_t saveconf_syntax = {
+  .max_args = 0,
+  .accept_keywords = true,
+  .kvline_flags=KV_OMIT_VALS,
+};
+
 /** Called when we get a SAVECONF command. Try to flush the current options to
  * disk, and report success or failure. */
 static int
-handle_control_saveconf(control_connection_t *conn, uint32_t len,
-                        const char *body)
+handle_control_saveconf(control_connection_t *conn,
+                        const control_cmd_args_t *args)
 {
-  (void) len;
-
-  int force = !strcmpstart(body, "FORCE");
+  bool force = config_lines_contain_flag(args->kwargs, "FORCE");
   const or_options_t *options = get_options();
   if ((!force && options->IncludeUsed) || options_save_current() < 0) {
     connection_write_str_to_buf(
@@ -266,23 +466,23 @@ handle_control_saveconf(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const control_cmd_syntax_t signal_syntax = {
+  .min_args = 1,
+  .max_args = 1,
+};
+
 /** Called when we get a SIGNAL command. React to the provided signal, and
  * report success or failure. (If the signal results in a shutdown, success
  * may not be reported.) */
 static int
-handle_control_signal(control_connection_t *conn, uint32_t len,
-                      const char *body)
+handle_control_signal(control_connection_t *conn,
+                      const control_cmd_args_t *args)
 {
   int sig = -1;
   int i;
-  int n = 0;
-  char *s;
 
-  (void) len;
-
-  while (body[n] && ! TOR_ISSPACE(body[n]))
-    ++n;
-  s = tor_strndup(body, n);
+  tor_assert(smartlist_len(args->args) == 1);
+  const char *s = smartlist_get(args->args, 0);
 
   for (i = 0; signal_table[i].signal_name != NULL; ++i) {
     if (!strcasecmp(s, signal_table[i].signal_name)) {
@@ -294,7 +494,6 @@ handle_control_signal(control_connection_t *conn, uint32_t len,
   if (sig < 0)
     connection_printf_to_buf(conn, "552 Unrecognized signal code \"%s\"\r\n",
                              s);
-  tor_free(s);
   if (sig < 0)
     return 0;
 
@@ -308,15 +507,18 @@ handle_control_signal(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const control_cmd_syntax_t takeownership_syntax = {
+  .max_args = UINT_MAX, // This should probably become zero. XXXXX
+};
+
 /** Called when we get a TAKEOWNERSHIP command.  Mark this connection
  * as an owning connection, so that we will exit if the connection
  * closes. */
 static int
-handle_control_takeownership(control_connection_t *conn, uint32_t len,
-                             const char *body)
+handle_control_takeownership(control_connection_t *conn,
+                             const control_cmd_args_t *args)
 {
-  (void)len;
-  (void)body;
+  (void)args;
 
   conn->is_owning_control_connection = 1;
 
@@ -328,15 +530,18 @@ handle_control_takeownership(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const control_cmd_syntax_t dropownership_syntax = {
+  .max_args = UINT_MAX, // This should probably become zero. XXXXX
+};
+
 /** Called when we get a DROPOWNERSHIP command.  Mark this connection
  * as a non-owning connection, so that we will not exit if the connection
  * closes. */
 static int
-handle_control_dropownership(control_connection_t *conn, uint32_t len,
-                             const char *body)
+handle_control_dropownership(control_connection_t *conn,
+                             const control_cmd_args_t *args)
 {
-  (void)len;
-  (void)body;
+  (void)args;
 
   conn->is_owning_control_connection = 0;
 
@@ -381,73 +586,17 @@ get_stream(const char *id)
  * contents of body.
  */
 static int
-control_setconf_helper(control_connection_t *conn, uint32_t len, char *body,
+control_setconf_helper(control_connection_t *conn,
+                       const control_cmd_args_t *args,
                        int use_defaults)
 {
   setopt_err_t opt_err;
-  config_line_t *lines=NULL;
-  char *start = body;
   char *errstring = NULL;
   const unsigned flags =
     CAL_CLEAR_FIRST | (use_defaults ? CAL_USE_DEFAULTS : 0);
 
-  char *config;
-  smartlist_t *entries = smartlist_new();
-
-  /* We have a string, "body", of the format '(key(=val|="val")?)' entries
-   * separated by space.  break it into a list of configuration entries. */
-  while (*body) {
-    char *eq = body;
-    char *key;
-    char *entry;
-    while (!TOR_ISSPACE(*eq) && *eq != '=')
-      ++eq;
-    key = tor_strndup(body, eq-body);
-    body = eq+1;
-    if (*eq == '=') {
-      char *val=NULL;
-      size_t val_len=0;
-      if (*body != '\"') {
-        char *val_start = body;
-        while (!TOR_ISSPACE(*body))
-          body++;
-        val = tor_strndup(val_start, body-val_start);
-        val_len = strlen(val);
-      } else {
-        body = (char*)extract_escaped_string(body, (len - (body-start)),
-                                             &val, &val_len);
-        if (!body) {
-          connection_write_str_to_buf("551 Couldn't parse string\r\n", conn);
-          SMARTLIST_FOREACH(entries, char *, cp, tor_free(cp));
-          smartlist_free(entries);
-          tor_free(key);
-          return 0;
-        }
-      }
-      tor_asprintf(&entry, "%s %s", key, val);
-      tor_free(key);
-      tor_free(val);
-    } else {
-      entry = key;
-    }
-    smartlist_add(entries, entry);
-    while (TOR_ISSPACE(*body))
-      ++body;
-  }
-
-  smartlist_add_strdup(entries, "");
-  config = smartlist_join_strings(entries, "\n", 0, NULL);
-  SMARTLIST_FOREACH(entries, char *, cp, tor_free(cp));
-  smartlist_free(entries);
-
-  if (config_get_lines(config, &lines, 0) < 0) {
-    log_warn(LD_CONTROL,"Controller gave us config lines we can't parse.");
-    connection_write_str_to_buf("551 Couldn't parse configuration\r\n",
-                                conn);
-    tor_free(config);
-    return 0;
-  }
-  tor_free(config);
+  // We need a copy here, since confparse.c wants to canonicalize cases.
+  config_line_t *lines = config_lines_dup(args->kwargs);
 
   opt_err = options_trial_assign(lines, flags, &errstring);
   {
@@ -492,30 +641,27 @@ address_is_invalid_mapaddress_target(const char *addr)
     return address_is_invalid_destination(addr, 1);
 }
 
+static const control_cmd_syntax_t mapaddress_syntax = {
+  .max_args=1,
+  .accept_keywords=true,
+};
+
 /** Called when we get a MAPADDRESS command; try to bind all listed addresses,
  * and report success or failure. */
 static int
-handle_control_mapaddress(control_connection_t *conn, uint32_t len,
-                          const char *body)
+handle_control_mapaddress(control_connection_t *conn,
+                          const control_cmd_args_t *args)
 {
-  smartlist_t *elts;
-  smartlist_t *lines;
   smartlist_t *reply;
   char *r;
   size_t sz;
-  (void) len; /* body is NUL-terminated, so it's safe to ignore the length. */
 
-  lines = smartlist_new();
-  elts = smartlist_new();
   reply = smartlist_new();
-  smartlist_split_string(lines, body, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-  SMARTLIST_FOREACH_BEGIN(lines, char *, line) {
-    tor_strlower(line);
-    smartlist_split_string(elts, line, "=", 0, 2);
-    if (smartlist_len(elts) == 2) {
-      const char *from = smartlist_get(elts,0);
-      const char *to = smartlist_get(elts,1);
+  const config_line_t *line;
+  for (line = args->kwargs; line; line = line->next) {
+    const char *from = line->key;
+    const char *to = line->value;
+    {
       if (address_is_invalid_mapaddress_target(to)) {
         smartlist_add_asprintf(reply,
                      "512-syntax error: invalid address '%s'", to);
@@ -530,10 +676,10 @@ handle_control_mapaddress(control_connection_t *conn, uint32_t len,
                                                      type, tor_strdup(to));
         if (!address) {
           smartlist_add_asprintf(reply,
-                       "451-resource exhausted: skipping '%s'", line);
+                   "451-resource exhausted: skipping '%s=%s'", from,to);
           log_warn(LD_CONTROL,
                    "Unable to allocate address for '%s' in MapAddress msg",
-                   safe_str_client(line));
+                   safe_str_client(to));
         } else {
           smartlist_add_asprintf(reply, "250-%s=%s", address, to);
         }
@@ -543,27 +689,16 @@ handle_control_mapaddress(control_connection_t *conn, uint32_t len,
                                      ADDRMAPSRC_CONTROLLER, &msg) < 0) {
           smartlist_add_asprintf(reply,
                                  "512-syntax error: invalid address mapping "
-                                 " '%s': %s", line, msg);
+                                 " '%s=%s': %s", from, to, msg);
           log_warn(LD_CONTROL,
-                   "Skipping invalid argument '%s' in MapAddress msg: %s",
-                   line, msg);
+                   "Skipping invalid argument '%s=%s' in MapAddress msg: %s",
+                   from, to, msg);
         } else {
-          smartlist_add_asprintf(reply, "250-%s", line);
+          smartlist_add_asprintf(reply, "250-%s=%s", from, to);
         }
       }
-    } else {
-      smartlist_add_asprintf(reply, "512-syntax error: mapping '%s' is "
-                   "not of expected form 'foo=bar'.", line);
-      log_info(LD_CONTROL, "Skipping MapAddress '%s': wrong "
-                           "number of items.",
-                           safe_str_client(line));
     }
-    SMARTLIST_FOREACH(elts, char *, cp, tor_free(cp));
-    smartlist_clear(elts);
-  } SMARTLIST_FOREACH_END(line);
-  SMARTLIST_FOREACH(lines, char *, cp, tor_free(cp));
-  smartlist_free(lines);
-  smartlist_free(elts);
+  }
 
   if (smartlist_len(reply)) {
     ((char*)smartlist_get(reply,smartlist_len(reply)-1))[3] = ' ';
@@ -596,94 +731,61 @@ circuit_purpose_from_string(const char *string)
     return CIRCUIT_PURPOSE_UNKNOWN;
 }
 
-/** Return a newly allocated smartlist containing the arguments to the command
- * waiting in <b>body</b>. If there are fewer than <b>min_args</b> arguments,
- * or if <b>max_args</b> is nonnegative and there are more than
- * <b>max_args</b> arguments, send a 512 error to the controller, using
- * <b>command</b> as the command name in the error message. */
-static smartlist_t *
-getargs_helper(const char *command, control_connection_t *conn,
-               const char *body, int min_args, int max_args)
-{
-  smartlist_t *args = smartlist_new();
-  smartlist_split_string(args, body, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-  if (smartlist_len(args) < min_args) {
-    connection_printf_to_buf(conn, "512 Missing argument to %s\r\n",command);
-    goto err;
-  } else if (max_args >= 0 && smartlist_len(args) > max_args) {
-    connection_printf_to_buf(conn, "512 Too many arguments to %s\r\n",command);
-    goto err;
-  }
-  return args;
- err:
-  SMARTLIST_FOREACH(args, char *, s, tor_free(s));
-  smartlist_free(args);
-  return NULL;
-}
-
-/** Helper.  Return the first element of <b>sl</b> at index <b>start_at</b> or
- * higher that starts with <b>prefix</b>, case-insensitive.  Return NULL if no
- * such element exists. */
-static const char *
-find_element_starting_with(smartlist_t *sl, int start_at, const char *prefix)
-{
-  int i;
-  for (i = start_at; i < smartlist_len(sl); ++i) {
-    const char *elt = smartlist_get(sl, i);
-    if (!strcasecmpstart(elt, prefix))
-      return elt;
-  }
-  return NULL;
-}
-
-/** Helper.  Return true iff s is an argument that we should treat as a
- * key-value pair. */
-static int
-is_keyval_pair(const char *s)
-{
-  /* An argument is a key-value pair if it has an =, and it isn't of the form
-   * $fingeprint=name */
-  return strchr(s, '=') && s[0] != '$';
-}
+static const control_cmd_syntax_t extendcircuit_syntax = {
+  .min_args=1,
+  .max_args=1, // see note in function
+  .accept_keywords=true,
+  .kvline_flags=KV_OMIT_VALS
+};
 
 /** Called when we get an EXTENDCIRCUIT message.  Try to extend the listed
  * circuit, and report success or failure. */
 static int
-handle_control_extendcircuit(control_connection_t *conn, uint32_t len,
-                             const char *body)
+handle_control_extendcircuit(control_connection_t *conn,
+                             const control_cmd_args_t *args)
 {
-  smartlist_t *router_nicknames=NULL, *nodes=NULL;
+  smartlist_t *router_nicknames=smartlist_new(), *nodes=NULL;
   origin_circuit_t *circ = NULL;
-  int zero_circ;
   uint8_t intended_purpose = CIRCUIT_PURPOSE_C_GENERAL;
-  smartlist_t *args;
-  (void) len;
-
-  router_nicknames = smartlist_new();
-
-  args = getargs_helper("EXTENDCIRCUIT", conn, body, 1, -1);
-  if (!args)
-    goto done;
+  const config_line_t *kwargs = args->kwargs;
+  const char *circ_id = smartlist_get(args->args, 0);
+  const char *path_str = NULL;
+  char *path_str_alloc = NULL;
+
+  /* The syntax for this command is unfortunate. The second argument is
+     optional, and is a comma-separated list long-format fingerprints, which
+     can (historically!) contain an equals sign.
+
+     Here we check the second argument to see if it's a path, and if so we
+     remove it from the kwargs list and put it in path_str.
+  */
+  if (kwargs) {
+    const config_line_t *arg1 = kwargs;
+    if (!strcmp(arg1->value, "")) {
+      path_str = arg1->key;
+      kwargs = kwargs->next;
+    } else if (arg1->key[0] == '$') {
+      tor_asprintf(&path_str_alloc, "%s=%s", arg1->key, arg1->value);
+      path_str = path_str_alloc;
+      kwargs = kwargs->next;
+    }
+  }
 
-  zero_circ = !strcmp("0", (char*)smartlist_get(args,0));
+  const config_line_t *purpose_line = config_line_find_case(kwargs, "PURPOSE");
+  bool zero_circ = !strcmp("0", circ_id);
 
-  if (zero_circ) {
-    const char *purp = find_element_starting_with(args, 1, "PURPOSE=");
-
-    if (purp) {
-      intended_purpose = circuit_purpose_from_string(purp);
-      if (intended_purpose == CIRCUIT_PURPOSE_UNKNOWN) {
-        connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n", purp);
-        SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-        smartlist_free(args);
-        goto done;
-      }
+  if (purpose_line) {
+    intended_purpose = circuit_purpose_from_string(purpose_line->value);
+    if (intended_purpose == CIRCUIT_PURPOSE_UNKNOWN) {
+      connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n",
+                               purpose_line->value);
+      goto done;
     }
+  }
 
-    if ((smartlist_len(args) == 1) ||
-        (smartlist_len(args) >= 2 && is_keyval_pair(smartlist_get(args, 1)))) {
-      // "EXTENDCIRCUIT 0" || EXTENDCIRCUIT 0 foo=bar"
+  if (zero_circ) {
+    if (!path_str) {
+      // "EXTENDCIRCUIT 0" with no path.
       circ = circuit_launch(intended_purpose, CIRCLAUNCH_NEED_CAPACITY);
       if (!circ) {
         connection_write_str_to_buf("551 Couldn't start circuit\r\n", conn);
@@ -691,37 +793,24 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len,
         connection_printf_to_buf(conn, "250 EXTENDED %lu\r\n",
                   (unsigned long)circ->global_identifier);
       }
-      SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-      smartlist_free(args);
       goto done;
     }
-    // "EXTENDCIRCUIT 0 router1,router2" ||
-    // "EXTENDCIRCUIT 0 router1,router2 PURPOSE=foo"
   }
 
-  if (!zero_circ && !(circ = get_circ(smartlist_get(args,0)))) {
-    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n",
-                             (char*)smartlist_get(args, 0));
-    SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-    smartlist_free(args);
+  if (!zero_circ && !(circ = get_circ(circ_id))) {
+    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", circ_id);
     goto done;
   }
 
-  if (smartlist_len(args) < 2) {
-    connection_printf_to_buf(conn,
-                             "512 syntax error: not enough arguments.\r\n");
-    SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-    smartlist_free(args);
+  if (!path_str) {
+    connection_printf_to_buf(conn, "512 syntax error: path required.\r\n");
     goto done;
   }
 
-  smartlist_split_string(router_nicknames, smartlist_get(args,1), ",", 0, 0);
-
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
+  smartlist_split_string(router_nicknames, path_str, ",", 0, 0);
 
   nodes = smartlist_new();
-  int first_node = zero_circ;
+  bool first_node = zero_circ;
   SMARTLIST_FOREACH_BEGIN(router_nicknames, const char *, n) {
     const node_t *node = node_get_by_nickname(n, 0);
     if (!node) {
@@ -733,8 +822,9 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len,
       goto done;
     }
     smartlist_add(nodes, (void*)node);
-    first_node = 0;
+    first_node = false;
   } SMARTLIST_FOREACH_END(n);
+
   if (!smartlist_len(nodes)) {
     connection_write_str_to_buf("512 No router names provided\r\n", conn);
     goto done;
@@ -800,39 +890,40 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len,
   SMARTLIST_FOREACH(router_nicknames, char *, n, tor_free(n));
   smartlist_free(router_nicknames);
   smartlist_free(nodes);
+  tor_free(path_str_alloc);
   return 0;
 }
 
+static const control_cmd_syntax_t setcircuitpurpose_syntax = {
+  .max_args=1,
+  .accept_keywords=true,
+};
+
 /** Called when we get a SETCIRCUITPURPOSE message. If we can find the
  * circuit and it's a valid purpose, change it. */
 static int
 handle_control_setcircuitpurpose(control_connection_t *conn,
-                                 uint32_t len, const char *body)
+                                 const control_cmd_args_t *args)
 {
   origin_circuit_t *circ = NULL;
   uint8_t new_purpose;
-  smartlist_t *args;
-  (void) len; /* body is NUL-terminated, so it's safe to ignore the length. */
-
-  args = getargs_helper("SETCIRCUITPURPOSE", conn, body, 2, -1);
-  if (!args)
-    goto done;
+  const char *circ_id = smartlist_get(args->args,0);
 
-  if (!(circ = get_circ(smartlist_get(args,0)))) {
-    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n",
-                             (char*)smartlist_get(args, 0));
+  if (!(circ = get_circ(circ_id))) {
+    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", circ_id);
     goto done;
   }
 
   {
-    const char *purp = find_element_starting_with(args,1,"PURPOSE=");
+    const config_line_t *purp = config_line_find_case(args->kwargs, "PURPOSE");
     if (!purp) {
       connection_write_str_to_buf("552 No purpose given\r\n", conn);
       goto done;
     }
-    new_purpose = circuit_purpose_from_string(purp);
+    new_purpose = circuit_purpose_from_string(purp->value);
     if (new_purpose == CIRCUIT_PURPOSE_UNKNOWN) {
-      connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n", purp);
+      connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n",
+                               purp->value);
       goto done;
     }
   }
@@ -841,54 +932,50 @@ handle_control_setcircuitpurpose(control_connection_t *conn,
   connection_write_str_to_buf("250 OK\r\n", conn);
 
  done:
-  if (args) {
-    SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-    smartlist_free(args);
-  }
   return 0;
 }
 
+static const char *attachstream_keywords[] = {
+  "HOP", NULL
+};
+static const control_cmd_syntax_t attachstream_syntax = {
+  .min_args=2, .max_args=2,
+  .accept_keywords=true,
+  .allowed_keywords=attachstream_keywords
+};
+
 /** Called when we get an ATTACHSTREAM message.  Try to attach the requested
  * stream, and report success or failure. */
 static int
-handle_control_attachstream(control_connection_t *conn, uint32_t len,
-                            const char *body)
+handle_control_attachstream(control_connection_t *conn,
+                            const control_cmd_args_t *args)
 {
   entry_connection_t *ap_conn = NULL;
   origin_circuit_t *circ = NULL;
-  int zero_circ;
-  smartlist_t *args;
   crypt_path_t *cpath=NULL;
   int hop=0, hop_line_ok=1;
-  (void) len;
+  const char *stream_id = smartlist_get(args->args, 0);
+  const char *circ_id = smartlist_get(args->args, 1);
+  int zero_circ = !strcmp(circ_id, "0");
+  const config_line_t *hoparg = config_line_find_case(args->kwargs, "HOP");
 
-  args = getargs_helper("ATTACHSTREAM", conn, body, 2, -1);
-  if (!args)
+  if (!(ap_conn = get_stream(stream_id))) {
+    connection_printf_to_buf(conn, "552 Unknown stream \"%s\"\r\n", stream_id);
+    return 0;
+  } else if (!zero_circ && !(circ = get_circ(circ_id))) {
+    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", circ_id);
     return 0;
-
-  zero_circ = !strcmp("0", (char*)smartlist_get(args,1));
-
-  if (!(ap_conn = get_stream(smartlist_get(args, 0)))) {
-    connection_printf_to_buf(conn, "552 Unknown stream \"%s\"\r\n",
-                             (char*)smartlist_get(args, 0));
-  } else if (!zero_circ && !(circ = get_circ(smartlist_get(args, 1)))) {
-    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n",
-                             (char*)smartlist_get(args, 1));
   } else if (circ) {
-    const char *hopstring = find_element_starting_with(args,2,"HOP=");
-    if (hopstring) {
-      hopstring += strlen("HOP=");
-      hop = (int) tor_parse_ulong(hopstring, 10, 0, INT_MAX,
+    if (hoparg) {
+      hop = (int) tor_parse_ulong(hoparg->value, 10, 0, INT_MAX,
                                   &hop_line_ok, NULL);
       if (!hop_line_ok) { /* broken hop line */
-        connection_printf_to_buf(conn, "552 Bad value hop=%s\r\n", hopstring);
+        connection_printf_to_buf(conn, "552 Bad value hop=%s\r\n",
+                                 hoparg->value);
+        return 0;
       }
     }
   }
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
-  if (!ap_conn || (!zero_circ && !circ) || !hop_line_ok)
-    return 0;
 
   if (ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_CONTROLLER_WAIT &&
       ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_CONNECT_WAIT &&
@@ -943,59 +1030,49 @@ handle_control_attachstream(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const char *postdescriptor_keywords[] = {
+  "cache", "purpose", NULL,
+};
+
+static const control_cmd_syntax_t postdescriptor_syntax = {
+  .max_args = 0,
+  .accept_keywords = true,
+  .allowed_keywords = postdescriptor_keywords,
+  .want_cmddata = true,
+};
+
 /** Called when we get a POSTDESCRIPTOR message.  Try to learn the provided
  * descriptor, and report success or failure. */
 static int
-handle_control_postdescriptor(control_connection_t *conn, uint32_t len,
-                              const char *body)
+handle_control_postdescriptor(control_connection_t *conn,
+                              const control_cmd_args_t *args)
 {
-  char *desc;
   const char *msg=NULL;
   uint8_t purpose = ROUTER_PURPOSE_GENERAL;
   int cache = 0; /* eventually, we may switch this to 1 */
+  const config_line_t *line;
 
-  const char *cp = memchr(body, '\n', len);
-
-  if (cp == NULL) {
-    connection_printf_to_buf(conn, "251 Empty body\r\n");
-    return 0;
+  line = config_line_find_case(args->kwargs, "purpose");
+  if (line) {
+    purpose = router_purpose_from_string(line->value);
+    connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n",
+                             line->value);
+    goto done;
   }
-  ++cp;
-
-  char *cmdline = tor_memdup_nulterm(body, cp-body);
-  smartlist_t *args = smartlist_new();
-  smartlist_split_string(args, cmdline, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-  SMARTLIST_FOREACH_BEGIN(args, char *, option) {
-    if (!strcasecmpstart(option, "purpose=")) {
-      option += strlen("purpose=");
-      purpose = router_purpose_from_string(option);
-      if (purpose == ROUTER_PURPOSE_UNKNOWN) {
-        connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n",
-                                 option);
-        goto done;
-      }
-    } else if (!strcasecmpstart(option, "cache=")) {
-      option += strlen("cache=");
-      if (!strcasecmp(option, "no"))
-        cache = 0;
-      else if (!strcasecmp(option, "yes"))
-        cache = 1;
-      else {
-        connection_printf_to_buf(conn, "552 Unknown cache request \"%s\"\r\n",
-                                 option);
-        goto done;
-      }
-    } else { /* unrecognized argument? */
-      connection_printf_to_buf(conn,
-        "512 Unexpected argument \"%s\" to postdescriptor\r\n", option);
+  line = config_line_find_case(args->kwargs, "cache");
+  if (line) {
+    if (!strcasecmp(line->value, "no"))
+      cache = 0;
+    else if (!strcasecmp(line->value, "yes"))
+      cache = 1;
+    else {
+      connection_printf_to_buf(conn, "552 Unknown cache request \"%s\"\r\n",
+                               line->value);
       goto done;
     }
-  } SMARTLIST_FOREACH_END(option);
-
-  read_escaped_data(cp, len-(cp-body), &desc);
+  }
 
-  switch (router_load_single_router(desc, purpose, cache, &msg)) {
+  switch (router_load_single_router(args->cmddata, purpose, cache, &msg)) {
   case -1:
     if (!msg) msg = "Could not parse descriptor";
     connection_printf_to_buf(conn, "554 %s\r\n", msg);
@@ -1009,29 +1086,25 @@ handle_control_postdescriptor(control_connection_t *conn, uint32_t len,
     break;
   }
 
-  tor_free(desc);
  done:
-  SMARTLIST_FOREACH(args, char *, arg, tor_free(arg));
-  smartlist_free(args);
-  tor_free(cmdline);
   return 0;
 }
 
+static const control_cmd_syntax_t redirectstream_syntax = {
+  .min_args = 2,
+  .max_args = UINT_MAX, // XXX should be 3.
+};
+
 /** Called when we receive a REDIRECTSTERAM command.  Try to change the target
  * address of the named AP stream, and report success or failure. */
 static int
-handle_control_redirectstream(control_connection_t *conn, uint32_t len,
-                              const char *body)
+handle_control_redirectstream(control_connection_t *conn,
+                              const control_cmd_args_t *cmd_args)
 {
   entry_connection_t *ap_conn = NULL;
   char *new_addr = NULL;
   uint16_t new_port = 0;
-  smartlist_t *args;
-  (void) len;
-
-  args = getargs_helper("REDIRECTSTREAM", conn, body, 2, -1);
-  if (!args)
-    return 0;
+  const smartlist_t *args = cmd_args->args;
 
   if (!(ap_conn = get_stream(smartlist_get(args, 0)))
            || !ap_conn->socks_request) {
@@ -1051,8 +1124,6 @@ handle_control_redirectstream(control_connection_t *conn, uint32_t len,
     }
   }
 
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
   if (!new_addr)
     return 0;
 
@@ -1065,23 +1136,26 @@ handle_control_redirectstream(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const control_cmd_syntax_t closestream_syntax = {
+  .min_args = 2,
+  .max_args = UINT_MAX, /* XXXX This is the original behavior, but
+                         * maybe we should change the spec. */
+};
+
 /** Called when we get a CLOSESTREAM command; try to close the named stream
  * and report success or failure. */
 static int
-handle_control_closestream(control_connection_t *conn, uint32_t len,
-                           const char *body)
+handle_control_closestream(control_connection_t *conn,
+                           const control_cmd_args_t *cmd_args)
 {
   entry_connection_t *ap_conn=NULL;
   uint8_t reason=0;
-  smartlist_t *args;
   int ok;
-  (void) len;
+  const smartlist_t *args = cmd_args->args;
 
-  args = getargs_helper("CLOSESTREAM", conn, body, 2, -1);
-  if (!args)
-    return 0;
+  tor_assert(smartlist_len(args) >= 2);
 
-  else if (!(ap_conn = get_stream(smartlist_get(args, 0))))
+  if (!(ap_conn = get_stream(smartlist_get(args, 0))))
     connection_printf_to_buf(conn, "552 Unknown stream \"%s\"\r\n",
                              (char*)smartlist_get(args, 0));
   else {
@@ -1093,8 +1167,6 @@ handle_control_closestream(control_connection_t *conn, uint32_t len,
       ap_conn = NULL;
     }
   }
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
   if (!ap_conn)
     return 0;
 
@@ -1103,38 +1175,29 @@ handle_control_closestream(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const control_cmd_syntax_t closecircuit_syntax = {
+  .min_args=1, .max_args=1,
+  .accept_keywords=true,
+  .kvline_flags=KV_OMIT_VALS,
+  // XXXX we might want to exclude unrecognized flags, but for now we
+  // XXXX just ignore them for backward compatibility.
+};
+
 /** Called when we get a CLOSECIRCUIT command; try to close the named circuit
  * and report success or failure. */
 static int
-handle_control_closecircuit(control_connection_t *conn, uint32_t len,
-                            const char *body)
+handle_control_closecircuit(control_connection_t *conn,
+                            const control_cmd_args_t *args)
 {
+  const char *circ_id = smartlist_get(args->args, 0);
   origin_circuit_t *circ = NULL;
-  int safe = 0;
-  smartlist_t *args;
-  (void) len;
 
-  args = getargs_helper("CLOSECIRCUIT", conn, body, 1, -1);
-  if (!args)
+  if (!(circ=get_circ(circ_id))) {
+    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", circ_id);
     return 0;
-
-  if (!(circ=get_circ(smartlist_get(args, 0))))
-    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n",
-                             (char*)smartlist_get(args, 0));
-  else {
-    int i;
-    for (i=1; i < smartlist_len(args); ++i) {
-      if (!strcasecmp(smartlist_get(args, i), "IfUnused"))
-        safe = 1;
-      else
-        log_info(LD_CONTROL, "Skipping unknown option %s",
-                 (char*)smartlist_get(args,i));
-    }
   }
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
-  if (!circ)
-    return 0;
+
+  bool safe =  config_lines_contain_flag(args->kwargs, "IfUnused");
 
   if (!safe || !circ->p_streams) {
     circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_REQUESTED);
@@ -1144,36 +1207,43 @@ handle_control_closecircuit(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const control_cmd_syntax_t resolve_syntax = {
+  .max_args=0,
+  .accept_keywords=true,
+  .kvline_flags=KV_OMIT_VALS,
+};
+
 /** Called when we get a RESOLVE command: start trying to resolve
  * the listed addresses. */
 static int
-handle_control_resolve(control_connection_t *conn, uint32_t len,
-                       const char *body)
+handle_control_resolve(control_connection_t *conn,
+                       const control_cmd_args_t *args)
 {
-  smartlist_t *args, *failed;
+  smartlist_t *failed;
   int is_reverse = 0;
-  (void) len; /* body is nul-terminated; it's safe to ignore the length */
 
   if (!(conn->event_mask & (((event_mask_t)1)<<EVENT_ADDRMAP))) {
     log_warn(LD_CONTROL, "Controller asked us to resolve an address, but "
              "isn't listening for ADDRMAP events.  It probably won't see "
              "the answer.");
   }
-  args = smartlist_new();
-  smartlist_split_string(args, body, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
+
   {
-    const char *modearg = find_element_starting_with(args, 0, "mode=");
-    if (modearg && !strcasecmp(modearg, "mode=reverse"))
+    const config_line_t *modearg = config_line_find_case(args->kwargs, "mode");
+    if (modearg && !strcasecmp(modearg->value, "reverse"))
       is_reverse = 1;
   }
   failed = smartlist_new();
-  SMARTLIST_FOREACH(args, const char *, arg, {
-      if (!is_keyval_pair(arg)) {
-          if (dnsserv_launch_request(arg, is_reverse, conn)<0)
-            smartlist_add(failed, (char*)arg);
-      }
-  });
+  for (const config_line_t *line = args->kwargs; line; line = line->next) {
+    if (!strlen(line->value)) {
+      const char *addr = line->key;
+      if (dnsserv_launch_request(addr, is_reverse, conn)<0)
+        smartlist_add(failed, (char*)addr);
+    } else {
+      // XXXX arguably we should reject unrecognized keyword arguments,
+      // XXXX but the old implementation didn't do that.
+    }
+  }
 
   send_control_done(conn);
   SMARTLIST_FOREACH(failed, const char *, arg, {
@@ -1181,25 +1251,24 @@ handle_control_resolve(control_connection_t *conn, uint32_t len,
                                    "internal", 0);
   });
 
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
   smartlist_free(failed);
   return 0;
 }
 
+static const control_cmd_syntax_t protocolinfo_syntax = {
+  .max_args = UINT_MAX
+};
+
 /** Called when we get a PROTOCOLINFO command: send back a reply. */
 static int
-handle_control_protocolinfo(control_connection_t *conn, uint32_t len,
-                            const char *body)
+handle_control_protocolinfo(control_connection_t *conn,
+                            const control_cmd_args_t *cmd_args)
 {
   const char *bad_arg = NULL;
-  smartlist_t *args;
-  (void)len;
+  const smartlist_t *args = cmd_args->args;
 
   conn->have_sent_protocolinfo = 1;
-  args = smartlist_new();
-  smartlist_split_string(args, body, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
+
   SMARTLIST_FOREACH(args, const char *, arg, {
       int ok;
       tor_parse_long(arg, 10, 0, LONG_MAX, &ok, NULL);
@@ -1255,24 +1324,21 @@ handle_control_protocolinfo(control_connection_t *conn, uint32_t len,
     tor_free(esc_cfile);
   }
  done:
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
   return 0;
 }
 
+static const control_cmd_syntax_t usefeature_syntax = {
+  .max_args = UINT_MAX
+};
+
 /** Called when we get a USEFEATURE command: parse the feature list, and
  * set up the control_connection's options properly. */
 static int
 handle_control_usefeature(control_connection_t *conn,
-                          uint32_t len,
-                          const char *body)
+                          const control_cmd_args_t *cmd_args)
 {
-  smartlist_t *args;
+  const smartlist_t *args = cmd_args->args;
   int bad = 0;
-  (void) len; /* body is nul-terminated; it's safe to ignore the length */
-  args = smartlist_new();
-  smartlist_split_string(args, body, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
   SMARTLIST_FOREACH_BEGIN(args, const char *, arg) {
       if (!strcasecmp(arg, "VERBOSE_NAMES"))
         ;
@@ -1290,22 +1356,19 @@ handle_control_usefeature(control_connection_t *conn,
     send_control_done(conn);
   }
 
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
   return 0;
 }
 
+static const control_cmd_syntax_t dropguards_syntax = {
+  .max_args = 0,
+};
+
 /** Implementation for the DROPGUARDS command. */
 static int
 handle_control_dropguards(control_connection_t *conn,
-                          uint32_t len,
-                          const char *body)
+                          const control_cmd_args_t *args)
 {
-  smartlist_t *args;
-  (void) len; /* body is nul-terminated; it's safe to ignore the length */
-  args = smartlist_new();
-  smartlist_split_string(args, body, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
+  (void) args; /* We don't take arguments. */
 
   static int have_warned = 0;
   if (! have_warned) {
@@ -1315,42 +1378,39 @@ handle_control_dropguards(control_connection_t *conn,
     have_warned = 1;
   }
 
-  if (smartlist_len(args)) {
-    connection_printf_to_buf(conn, "512 Too many arguments to DROPGUARDS\r\n");
-  } else {
-    remove_all_entry_guards();
-    send_control_done(conn);
-  }
+  remove_all_entry_guards();
+  send_control_done(conn);
 
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
   return 0;
 }
 
+static const char *hsfetch_keywords[] = {
+  "SERVER", NULL,
+};
+static const control_cmd_syntax_t hsfetch_syntax = {
+  .min_args = 1, .max_args = 1,
+  .accept_keywords = true,
+  .allowed_keywords = hsfetch_keywords,
+  .want_cmddata = true,
+};
+
 /** Implementation for the HSFETCH command. */
 static int
-handle_control_hsfetch(control_connection_t *conn, uint32_t len,
-                       const char *body)
+handle_control_hsfetch(control_connection_t *conn,
+                       const control_cmd_args_t *args)
+
 {
-  int i;
-  char digest[DIGEST_LEN], *hsaddress = NULL, *arg1 = NULL, *desc_id = NULL;
-  smartlist_t *args = NULL, *hsdirs = NULL;
-  (void) len; /* body is nul-terminated; it's safe to ignore the length */
-  static const char *hsfetch_command = "HSFETCH";
+  char digest[DIGEST_LEN], *desc_id = NULL;
+  smartlist_t *hsdirs = NULL;
   static const char *v2_str = "v2-";
   const size_t v2_str_len = strlen(v2_str);
   rend_data_t *rend_query = NULL;
   ed25519_public_key_t v3_pk;
   uint32_t version;
-
-  /* Make sure we have at least one argument, the HSAddress. */
-  args = getargs_helper(hsfetch_command, conn, body, 1, -1);
-  if (!args) {
-    goto exit;
-  }
+  const char *hsaddress = NULL;
 
   /* Extract the first argument (either HSAddress or DescID). */
-  arg1 = smartlist_get(args, 0);
+  const char *arg1 = smartlist_get(args->args, 0);
   /* Test if it's an HS address without the .onion part. */
   if (rend_valid_v2_service_id(arg1)) {
     hsaddress = arg1;
@@ -1374,33 +1434,24 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
     goto done;
   }
 
-  static const char *opt_server = "SERVER=";
-
-  /* Skip first argument because it's the HSAddress or DescID. */
-  for (i = 1; i < smartlist_len(args); ++i) {
-    const char *arg = smartlist_get(args, i);
-    const node_t *node;
-
-    if (!strcasecmpstart(arg, opt_server)) {
-      const char *server;
+  for (const config_line_t *line = args->kwargs; line; line = line->next) {
+    if (!strcasecmp(line->key, "SERVER")) {
+      const char *server = line->value;
 
-      server = arg + strlen(opt_server);
-      node = node_get_by_hex_id(server, 0);
+      const node_t *node = node_get_by_hex_id(server, 0);
       if (!node) {
         connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n",
                                  server);
         goto done;
       }
       if (!hsdirs) {
-        /* Stores routerstatus_t object for each specified server. */
+        /* Stores routerstatus_t cmddata for each specified server. */
         hsdirs = smartlist_new();
       }
       /* Valid server, add it to our local list. */
       smartlist_add(hsdirs, node->rs);
     } else {
-      connection_printf_to_buf(conn, "513 Unexpected argument \"%s\"\r\n",
-                               arg);
-      goto done;
+      tor_assert_nonfatal_unreached();
     }
   }
 
@@ -1416,9 +1467,8 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
   /* Using a descriptor ID, we force the user to provide at least one
    * hsdir server using the SERVER= option. */
   if (desc_id && (!hsdirs || !smartlist_len(hsdirs))) {
-      connection_printf_to_buf(conn, "512 %s option is required\r\n",
-                               opt_server);
-      goto done;
+    connection_printf_to_buf(conn, "512 SERVER option is required\r\n");
+    goto done;
   }
 
   /* We are about to trigger HSDir fetch so send the OK now because after
@@ -1436,96 +1486,75 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
   }
 
  done:
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
   /* Contains data pointer that we don't own thus no cleanup. */
   smartlist_free(hsdirs);
   rend_data_free(rend_query);
- exit:
   return 0;
 }
 
+static const char *hspost_keywords[] = {
+  "SERVER", "HSADDRESS", NULL
+};
+static const control_cmd_syntax_t hspost_syntax = {
+  .min_args = 0, .max_args = 0,
+  .accept_keywords = true,
+  .want_cmddata = true,
+  .allowed_keywords = hspost_keywords
+};
+
 /** Implementation for the HSPOST command. */
 static int
 handle_control_hspost(control_connection_t *conn,
-                      uint32_t len,
-                      const char *body)
+                      const control_cmd_args_t *args)
 {
-  static const char *opt_server = "SERVER=";
-  static const char *opt_hsaddress = "HSADDRESS=";
   smartlist_t *hs_dirs = NULL;
-  const char *encoded_desc = body;
-  size_t encoded_desc_len = len;
+  const char *encoded_desc = args->cmddata;
+  size_t encoded_desc_len = args->cmddata_len;
   const char *onion_address = NULL;
+  const config_line_t *line;
 
-  char *cp = memchr(body, '\n', len);
-  if (cp == NULL) {
-    connection_printf_to_buf(conn, "251 Empty body\r\n");
-    return 0;
-  }
-  char *argline = tor_strndup(body, cp-body);
-
-  smartlist_t *args = smartlist_new();
-
-  /* If any SERVER= or HSADDRESS= options were specified, try to parse
-   * the options line. */
-  if (!strcasecmpstart(argline, opt_server) ||
-      !strcasecmpstart(argline, opt_hsaddress)) {
-    /* encoded_desc begins after a newline character */
-    cp = cp + 1;
-    encoded_desc = cp;
-    encoded_desc_len = len-(cp-body);
-
-    smartlist_split_string(args, argline, " ",
-                           SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-    SMARTLIST_FOREACH_BEGIN(args, const char *, arg) {
-      if (!strcasecmpstart(arg, opt_server)) {
-        const char *server = arg + strlen(opt_server);
-        const node_t *node = node_get_by_hex_id(server, 0);
-
-        if (!node || !node->rs) {
-          connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n",
-                                   server);
-          goto done;
-        }
-        /* Valid server, add it to our local list. */
-        if (!hs_dirs)
-          hs_dirs = smartlist_new();
-        smartlist_add(hs_dirs, node->rs);
-      } else if (!strcasecmpstart(arg, opt_hsaddress)) {
-        const char *address = arg + strlen(opt_hsaddress);
-        if (!hs_address_is_valid(address)) {
-          connection_printf_to_buf(conn, "512 Malformed onion address\r\n");
-          goto done;
-        }
-        onion_address = address;
-      } else {
-        connection_printf_to_buf(conn, "512 Unexpected argument \"%s\"\r\n",
-                                 arg);
+  for (line = args->kwargs; line; line = line->next) {
+    if (!strcasecmpstart(line->key, "SERVER")) {
+      const char *server = line->value;
+      const node_t *node = node_get_by_hex_id(server, 0);
+
+      if (!node || !node->rs) {
+        connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n",
+                                 server);
         goto done;
       }
-    } SMARTLIST_FOREACH_END(arg);
+      /* Valid server, add it to our local list. */
+      if (!hs_dirs)
+        hs_dirs = smartlist_new();
+      smartlist_add(hs_dirs, node->rs);
+    } else if (!strcasecmpstart(line->key, "HSADDRESS")) {
+      const char *address = line->value;
+      if (!hs_address_is_valid(address)) {
+        connection_printf_to_buf(conn, "512 Malformed onion address\r\n");
+        goto done;
+      }
+      onion_address = address;
+    } else {
+      tor_assert_nonfatal_unreached();
+    }
   }
 
   /* Handle the v3 case. */
   if (onion_address) {
-    char *desc_str = NULL;
-    read_escaped_data(encoded_desc, encoded_desc_len, &desc_str);
-    if (hs_control_hspost_command(desc_str, onion_address, hs_dirs) < 0) {
+    if (hs_control_hspost_command(encoded_desc, onion_address, hs_dirs) < 0) {
       connection_printf_to_buf(conn, "554 Invalid descriptor\r\n");
     } else {
       send_control_done(conn);
     }
-    tor_free(desc_str);
     goto done;
   }
 
   /* From this point on, it is only v2. */
 
-  /* Read the dot encoded descriptor, and parse it. */
+  /*  parse it. */
   rend_encoded_v2_service_descriptor_t *desc =
       tor_malloc_zero(sizeof(rend_encoded_v2_service_descriptor_t));
-  read_escaped_data(encoded_desc, encoded_desc_len, &desc->desc_str);
+  desc->desc_str = tor_memdup_nulterm(encoded_desc, encoded_desc_len);
 
   rend_service_descriptor_t *parsed = NULL;
   char *intro_content = NULL;
@@ -1559,10 +1588,7 @@ handle_control_hspost(control_connection_t *conn,
   tor_free(intro_content);
   rend_encoded_v2_service_descriptor_free(desc);
  done:
-  tor_free(argline);
   smartlist_free(hs_dirs); /* Contents belong to the rend service code. */
-  SMARTLIST_FOREACH(args, char *, arg, tor_free(arg));
-  smartlist_free(args);
   return 0;
 }
 
@@ -1626,21 +1652,21 @@ get_detached_onion_services(void)
   return detached_onion_services;
 }
 
+static const char *add_onion_keywords[] = {
+   "Port", "Flags", "MaxStreams", "ClientAuth", NULL
+};
+static const control_cmd_syntax_t add_onion_syntax = {
+  .min_args = 1, .max_args = 1,
+  .accept_keywords = true,
+  .allowed_keywords = add_onion_keywords
+};
+
 /** Called when we get a ADD_ONION command; parse the body, and set up
  * the new ephemeral Onion Service. */
 static int
 handle_control_add_onion(control_connection_t *conn,
-                         uint32_t len,
-                         const char *body)
+                         const control_cmd_args_t *args)
 {
-  smartlist_t *args;
-  int arg_len;
-  (void) len; /* body is nul-terminated; it's safe to ignore the length */
-  args = getargs_helper("ADD_ONION", conn, body, 2, -1);
-  if (!args)
-    return 0;
-  arg_len = smartlist_len(args);
-
   /* Parse all of the arguments that do not involve handling cryptographic
    * material first, since there's no reason to touch that at all if any of
    * the other arguments are malformed.
@@ -1653,36 +1679,28 @@ handle_control_add_onion(control_connection_t *conn,
   int max_streams = 0;
   int max_streams_close_circuit = 0;
   rend_auth_type_t auth_type = REND_NO_AUTH;
-  /* Default to adding an anonymous hidden service if no flag is given */
   int non_anonymous = 0;
-  for (int i = 1; i < arg_len; i++) {
-    static const char *port_prefix = "Port=";
-    static const char *flags_prefix = "Flags=";
-    static const char *max_s_prefix = "MaxStreams=";
-    static const char *auth_prefix = "ClientAuth=";
-
-    const char *arg = smartlist_get(args, (int)i);
-    if (!strcasecmpstart(arg, port_prefix)) {
-      /* "Port=VIRTPORT[,TARGET]". */
-      const char *port_str = arg + strlen(port_prefix);
+  const config_line_t *arg;
 
+  for (arg = args->kwargs; arg; arg = arg->next) {
+    if (!strcasecmp(arg->key, "Port")) {
+      /* "Port=VIRTPORT[,TARGET]". */
       rend_service_port_config_t *cfg =
-          rend_service_parse_port_config(port_str, ",", NULL);
+          rend_service_parse_port_config(arg->value, ",", NULL);
       if (!cfg) {
         connection_printf_to_buf(conn, "512 Invalid VIRTPORT/TARGET\r\n");
         goto out;
       }
       smartlist_add(port_cfgs, cfg);
-    } else if (!strcasecmpstart(arg, max_s_prefix)) {
+    } else if (!strcasecmp(arg->key, "MaxStreams")) {
       /* "MaxStreams=[0..65535]". */
-      const char *max_s_str = arg + strlen(max_s_prefix);
       int ok = 0;
-      max_streams = (int)tor_parse_long(max_s_str, 10, 0, 65535, &ok, NULL);
+      max_streams = (int)tor_parse_long(arg->value, 10, 0, 65535, &ok, NULL);
       if (!ok) {
         connection_printf_to_buf(conn, "512 Invalid MaxStreams\r\n");
         goto out;
       }
-    } else if (!strcasecmpstart(arg, flags_prefix)) {
+    } else if (!strcasecmp(arg->key, "Flags")) {
       /* "Flags=Flag[,Flag]", where Flag can be:
        *   * 'DiscardPK' - If tor generates the keypair, do not include it in
        *                   the response.
@@ -1705,8 +1723,7 @@ handle_control_add_onion(control_connection_t *conn,
       smartlist_t *flags = smartlist_new();
       int bad = 0;
 
-      smartlist_split_string(flags, arg + strlen(flags_prefix), ",",
-                             SPLIT_IGNORE_BLANK, 0);
+      smartlist_split_string(flags, arg->value, ",", SPLIT_IGNORE_BLANK, 0);
       if (smartlist_len(flags) < 1) {
         connection_printf_to_buf(conn, "512 Invalid 'Flags' argument\r\n");
         bad = 1;
@@ -1735,11 +1752,12 @@ handle_control_add_onion(control_connection_t *conn,
       smartlist_free(flags);
       if (bad)
         goto out;
-    } else if (!strcasecmpstart(arg, auth_prefix)) {
+
+    } else if (!strcasecmp(arg->key, "ClientAuth")) {
       char *err_msg = NULL;
       int created = 0;
       rend_authorized_client_t *client =
-        add_onion_helper_clientauth(arg + strlen(auth_prefix),
+        add_onion_helper_clientauth(arg->value,
                                     &created, &err_msg);
       if (!client) {
         if (err_msg) {
@@ -1772,7 +1790,7 @@ handle_control_add_onion(control_connection_t *conn,
         smartlist_add(auth_created_clients, client);
       }
     } else {
-      connection_printf_to_buf(conn, "513 Invalid argument\r\n");
+      tor_assert_nonfatal_unreached();
       goto out;
     }
   }
@@ -1813,7 +1831,8 @@ handle_control_add_onion(control_connection_t *conn,
   char *key_new_blob = NULL;
   char *err_msg = NULL;
 
-  if (add_onion_helper_keyarg(smartlist_get(args, 0), discard_pk,
+  const char *onionkey = smartlist_get(args->args, 0);
+  if (add_onion_helper_keyarg(onionkey, discard_pk,
                               &key_new_alg, &key_new_blob, &pk, &hs_version,
                               &err_msg) < 0) {
     if (err_msg) {
@@ -1915,12 +1934,6 @@ handle_control_add_onion(control_connection_t *conn,
     // Do not free entries; they are the same as auth_clients
     smartlist_free(auth_created_clients);
   }
-
-  SMARTLIST_FOREACH(args, char *, cp, {
-    memwipe(cp, 0, strlen(cp));
-    tor_free(cp);
-  });
-  smartlist_free(args);
   return 0;
 }
 
@@ -2120,19 +2133,19 @@ add_onion_helper_clientauth(const char *arg, int *created, char **err_msg)
   return client;
 }
 
+static const control_cmd_syntax_t del_onion_syntax = {
+  .min_args = 1, .max_args = 1,
+};
+
 /** Called when we get a DEL_ONION command; parse the body, and remove
  * the existing ephemeral Onion Service. */
 static int
 handle_control_del_onion(control_connection_t *conn,
-                          uint32_t len,
-                          const char *body)
+                         const control_cmd_args_t *cmd_args)
 {
   int hs_version = 0;
-  smartlist_t *args;
-  (void) len; /* body is nul-terminated; it's safe to ignore the length */
-  args = getargs_helper("DEL_ONION", conn, body, 1, 1);
-  if (!args)
-    return 0;
+  smartlist_t *args = cmd_args->args;
+  tor_assert(smartlist_len(args) == 1);
 
   const char *service_id = smartlist_get(args, 0);
   if (rend_valid_v2_service_id(service_id)) {
@@ -2198,118 +2211,202 @@ handle_control_del_onion(control_connection_t *conn,
   }
 
  out:
-  SMARTLIST_FOREACH(args, char *, cp, {
-    memwipe(cp, 0, strlen(cp));
-    tor_free(cp);
-  });
-  smartlist_free(args);
   return 0;
 }
 
-int
-handle_control_command(control_connection_t *conn,
-                            uint32_t cmd_data_len,
-                            char *args)
+static const control_cmd_syntax_t obsolete_syntax = {
+  .max_args = UINT_MAX
+};
+
+/**
+ * Called when we get an obsolete command: tell the controller that it is
+ * obsolete.
+ */
+static int
+handle_control_obsolete(control_connection_t *conn,
+                        const control_cmd_args_t *args)
 {
-  /* XXXX Why is this not implemented as a table like the GETINFO
-   * items are?  Even handling the plus signs at the beginnings of
-   * commands wouldn't be very hard with proper macros. */
+  (void)args;
+  char *command = tor_strdup(conn->current_cmd);
+  tor_strupper(command);
+  connection_printf_to_buf(conn, "511 %s is obsolete.\r\n", command);
+  tor_free(command);
+  return 0;
+}
 
-  if (!strcasecmp(conn->incoming_cmd, "SETCONF")) {
-    if (handle_control_setconf(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "RESETCONF")) {
-    if (handle_control_resetconf(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "GETCONF")) {
-    if (handle_control_getconf(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "+LOADCONF")) {
-    if (handle_control_loadconf(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "SETEVENTS")) {
-    if (handle_control_setevents(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "AUTHENTICATE")) {
-    if (handle_control_authenticate(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "SAVECONF")) {
-    if (handle_control_saveconf(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "SIGNAL")) {
-    if (handle_control_signal(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "TAKEOWNERSHIP")) {
-    if (handle_control_takeownership(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "DROPOWNERSHIP")) {
-    if (handle_control_dropownership(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "MAPADDRESS")) {
-    if (handle_control_mapaddress(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "GETINFO")) {
-    if (handle_control_getinfo(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "EXTENDCIRCUIT")) {
-    if (handle_control_extendcircuit(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "SETCIRCUITPURPOSE")) {
-    if (handle_control_setcircuitpurpose(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "SETROUTERPURPOSE")) {
-    connection_write_str_to_buf("511 SETROUTERPURPOSE is obsolete.\r\n", conn);
-  } else if (!strcasecmp(conn->incoming_cmd, "ATTACHSTREAM")) {
-    if (handle_control_attachstream(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "+POSTDESCRIPTOR")) {
-    if (handle_control_postdescriptor(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "REDIRECTSTREAM")) {
-    if (handle_control_redirectstream(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "CLOSESTREAM")) {
-    if (handle_control_closestream(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "CLOSECIRCUIT")) {
-    if (handle_control_closecircuit(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "USEFEATURE")) {
-    if (handle_control_usefeature(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "RESOLVE")) {
-    if (handle_control_resolve(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "PROTOCOLINFO")) {
-    if (handle_control_protocolinfo(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "AUTHCHALLENGE")) {
-    if (handle_control_authchallenge(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "DROPGUARDS")) {
-    if (handle_control_dropguards(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "HSFETCH")) {
-    if (handle_control_hsfetch(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "+HSPOST")) {
-    if (handle_control_hspost(conn, cmd_data_len, args))
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "ADD_ONION")) {
-    int ret = handle_control_add_onion(conn, cmd_data_len, args);
-    memwipe(args, 0, cmd_data_len); /* Scrub the private key. */
-    if (ret)
-      return -1;
-  } else if (!strcasecmp(conn->incoming_cmd, "DEL_ONION")) {
-    int ret = handle_control_del_onion(conn, cmd_data_len, args);
-    memwipe(args, 0, cmd_data_len); /* Scrub the service id/pk. */
-    if (ret)
-      return -1;
+/**
+ * Function pointer to a handler function for a controller command.
+ **/
+typedef int (*handler_fn_t) (control_connection_t *conn,
+                             const control_cmd_args_t *args);
+
+/**
+ * Definition for a controller command.
+ */
+typedef struct control_cmd_def_t {
+  /**
+   * The name of the command. If the command is multiline, the name must
+   * begin with "+".  This is not case-sensitive. */
+  const char *name;
+  /**
+   * A function to execute the command.
+   */
+  handler_fn_t handler;
+  /**
+   * Zero or more CMD_FL_* flags, or'd together.
+   */
+  unsigned flags;
+  /**
+   * For parsed command: a syntax description.
+   */
+  const control_cmd_syntax_t *syntax;
+} control_cmd_def_t;
+
+/**
+ * Indicates that the command's arguments are sensitive, and should be
+ * memwiped after use.
+ */
+#define CMD_FL_WIPE (1u<<0)
+
+/** Macro: declare a command with a one-line argument, a given set of flags,
+ * and a syntax definition.
+ **/
+#define ONE_LINE(name, flags)                                   \
+  {                                                             \
+    #name,                                                      \
+      handle_control_ ##name,                                   \
+      flags,                                                    \
+      &name##_syntax,                                           \
+   }
+
+/**
+ * Macro: declare a command with a multi-line argument and a given set of
+ * flags.
+ **/
+#define MULTLINE(name, flags)                                   \
+  { "+"#name,                                                   \
+      handle_control_ ##name,                                   \
+      flags,                                                    \
+      &name##_syntax                                            \
+  }
+
+/**
+ * Macro: declare an obsolete command. (Obsolete commands give a different
+ * error than non-existent ones.)
+ **/
+#define OBSOLETE(name)                          \
+  { #name,                                      \
+      handle_control_obsolete,                  \
+      0,                                        \
+      &obsolete_syntax,                         \
+  }
+
+/**
+ * An array defining all the recognized controller commands.
+ **/
+static const control_cmd_def_t CONTROL_COMMANDS[] =
+{
+  ONE_LINE(setconf, 0),
+  ONE_LINE(resetconf, 0),
+  ONE_LINE(getconf, 0),
+  MULTLINE(loadconf, 0),
+  ONE_LINE(setevents, 0),
+  ONE_LINE(authenticate, CMD_FL_WIPE),
+  ONE_LINE(saveconf, 0),
+  ONE_LINE(signal, 0),
+  ONE_LINE(takeownership, 0),
+  ONE_LINE(dropownership, 0),
+  ONE_LINE(mapaddress, 0),
+  ONE_LINE(getinfo, 0),
+  ONE_LINE(extendcircuit, 0),
+  ONE_LINE(setcircuitpurpose, 0),
+  OBSOLETE(setrouterpurpose),
+  ONE_LINE(attachstream, 0),
+  MULTLINE(postdescriptor, 0),
+  ONE_LINE(redirectstream, 0),
+  ONE_LINE(closestream, 0),
+  ONE_LINE(closecircuit, 0),
+  ONE_LINE(usefeature, 0),
+  ONE_LINE(resolve, 0),
+  ONE_LINE(protocolinfo, 0),
+  ONE_LINE(authchallenge, CMD_FL_WIPE),
+  ONE_LINE(dropguards, 0),
+  ONE_LINE(hsfetch, 0),
+  MULTLINE(hspost, 0),
+  ONE_LINE(add_onion, CMD_FL_WIPE),
+  ONE_LINE(del_onion, CMD_FL_WIPE),
+};
+
+/**
+ * The number of entries in CONTROL_COMMANDS.
+ **/
+static const size_t N_CONTROL_COMMANDS = ARRAY_LENGTH(CONTROL_COMMANDS);
+
+/**
+ * Run a single control command, as defined by a control_cmd_def_t,
+ * with a given set of arguments.
+ */
+static int
+handle_single_control_command(const control_cmd_def_t *def,
+                              control_connection_t *conn,
+                              uint32_t cmd_data_len,
+                              char *args)
+{
+  int rv = 0;
+
+  control_cmd_args_t *parsed_args;
+  char *err=NULL;
+  tor_assert(def->syntax);
+  parsed_args = control_cmd_parse_args(conn->current_cmd,
+                                       def->syntax,
+                                       cmd_data_len, args,
+                                       &err);
+  if (!parsed_args) {
+    connection_printf_to_buf(conn,
+                             "512 Bad arguments to %s: %s\r\n",
+                             conn->current_cmd, err?err:"");
+    tor_free(err);
   } else {
-    connection_printf_to_buf(conn, "510 Unrecognized command \"%s\"\r\n",
-                             conn->incoming_cmd);
+    if (BUG(err))
+      tor_free(err);
+    if (def->handler(conn, parsed_args))
+      rv = 0;
+
+    if (def->flags & CMD_FL_WIPE)
+      control_cmd_args_wipe(parsed_args);
+
+    control_cmd_args_free(parsed_args);
   }
 
+  if (def->flags & CMD_FL_WIPE)
+    memwipe(args, 0, cmd_data_len);
+
+  return rv;
+}
+
+/**
+ * Run a given controller command, as selected by the current_cmd field of
+ * <b>conn</b>.
+ */
+int
+handle_control_command(control_connection_t *conn,
+                       uint32_t cmd_data_len,
+                       char *args)
+{
+  tor_assert(conn);
+  tor_assert(args);
+  tor_assert(args[cmd_data_len] == '\0');
+
+  for (unsigned i = 0; i < N_CONTROL_COMMANDS; ++i) {
+    const control_cmd_def_t *def = &CONTROL_COMMANDS[i];
+    if (!strcasecmp(conn->current_cmd, def->name)) {
+      return handle_single_control_command(def, conn, cmd_data_len, args);
+    }
+  }
+
+  connection_printf_to_buf(conn, "510 Unrecognized command \"%s\"\r\n",
+                           conn->current_cmd);
+
   return 0;
 }
 
diff --git a/src/feature/control/control_cmd.h b/src/feature/control/control_cmd.h
index a417e10da3..5c3d1a1cec 100644
--- a/src/feature/control/control_cmd.h
+++ b/src/feature/control/control_cmd.h
@@ -12,11 +12,68 @@
 #ifndef TOR_CONTROL_CMD_H
 #define TOR_CONTROL_CMD_H
 
+#include "lib/malloc/malloc.h"
+
 int handle_control_command(control_connection_t *conn,
                            uint32_t cmd_data_len,
                            char *args);
 void control_cmd_free_all(void);
 
+typedef struct control_cmd_args_t control_cmd_args_t;
+void control_cmd_args_free_(control_cmd_args_t *args);
+void control_cmd_args_wipe(control_cmd_args_t *args);
+
+#define control_cmd_args_free(v) \
+  FREE_AND_NULL(control_cmd_args_t, control_cmd_args_free_, (v))
+
+/**
+ * Definition for the syntax of a controller command, as parsed by
+ * control_cmd_parse_args.
+ *
+ * WORK IN PROGRESS: This structure is going to get more complex as this
+ * branch goes on.
+ **/
+typedef struct control_cmd_syntax_t {
+  /**
+   * Lowest number of positional arguments that this command accepts.
+   * 0 for "it's okay not to have positional arguments."
+   **/
+  unsigned int min_args;
+  /**
+   * Highest number of positional arguments that this command accepts.
+   * UINT_MAX for no limit.
+   **/
+  unsigned int max_args;
+  /**
+   * If true, we should parse options after the positional arguments
+   * as a set of unordered flags and key=value arguments.
+   *
+   * Requires that max_args is not UINT_MAX.
+   **/
+  bool accept_keywords;
+  /**
+   * If accept_keywords is true, then only the keywords listed in this
+   * (NULL-terminated) array are valid keywords for this command.
+   **/
+  const char **allowed_keywords;
+  /**
+   * If accept_keywords is true, this option is passed to kvline_parse() as
+   * its flags.
+   **/
+  unsigned kvline_flags;
+  /**
+   * True iff this command wants to be followed by a multiline object.
+   **/
+  bool want_cmddata;
+  /**
+   * True iff this command needs access to the raw body of the input.
+   *
+   * This should not be needed for pure commands; it is purely a legacy
+   * option.
+   **/
+  bool store_raw_body;
+} control_cmd_syntax_t;
+
 #ifdef CONTROL_CMD_PRIVATE
 #include "lib/crypt_ops/crypto_ed25519.h"
 
@@ -39,6 +96,13 @@ STATIC int add_onion_helper_keyarg(const char *arg, int discard_pk,
 STATIC rend_authorized_client_t *add_onion_helper_clientauth(const char *arg,
                                    int *created, char **err_msg_out);
 
+STATIC control_cmd_args_t *control_cmd_parse_args(
+                                   const char *command,
+                                   const control_cmd_syntax_t *syntax,
+                                   size_t body_len,
+                                   const char *body,
+                                   char **error_out);
+
 #endif /* defined(CONTROL_CMD_PRIVATE) */
 
 #ifdef CONTROL_MODULE_PRIVATE
diff --git a/src/feature/control/control_cmd_args_st.h b/src/feature/control/control_cmd_args_st.h
new file mode 100644
index 0000000000..8d7a4f55b3
--- /dev/null
+++ b/src/feature/control/control_cmd_args_st.h
@@ -0,0 +1,52 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * \file control_cmd_args_st.h
+ * \brief Definition for control_cmd_args_t
+ **/
+
+#ifndef TOR_CONTROL_CMD_ST_H
+#define TOR_CONTROL_CMD_ST_H
+
+struct smartlist_t;
+struct config_line_t;
+
+/**
+ * Parsed arguments for a control command.
+ *
+ * WORK IN PROGRESS: This structure is going to get more complex as this
+ * branch goes on.
+ **/
+struct control_cmd_args_t {
+  /**
+   * The command itself, as provided by the controller.  Not owned by this
+   * structure.
+   **/
+  const char *command;
+  /**
+   * Positional arguments to the command.
+   **/
+  struct smartlist_t *args;
+  /**
+   * Keyword arguments to the command.
+   **/
+  struct config_line_t *kwargs;
+  /**
+   * Number of bytes in <b>cmddata</b>; 0 if <b>cmddata</b> is not set.
+   **/
+  size_t cmddata_len;
+  /**
+   * A multiline object passed with this command.
+   **/
+  char *cmddata;
+  /**
+   * If set, a nul-terminated string containing the raw unparsed arguments.
+   **/
+  const char *raw_body;
+};
+
+#endif /* !defined(TOR_CONTROL_CMD_ST_H) */
diff --git a/src/feature/control/control_connection_st.h b/src/feature/control/control_connection_st.h
index 177a916257..cace6bb36f 100644
--- a/src/feature/control/control_connection_st.h
+++ b/src/feature/control/control_connection_st.h
@@ -40,7 +40,8 @@ struct control_connection_t {
   /** A control command that we're reading from the inbuf, but which has not
    * yet arrived completely. */
   char *incoming_cmd;
+  /** The control command that we are currently processing. */
+  char *current_cmd;
 };
 
 #endif
-
diff --git a/src/feature/control/control_fmt.c b/src/feature/control/control_fmt.c
index 71f9d82163..b2ab4f10bb 100644
--- a/src/feature/control/control_fmt.c
+++ b/src/feature/control/control_fmt.c
@@ -305,94 +305,6 @@ send_control_done(control_connection_t *conn)
   connection_write_str_to_buf("250 OK\r\n", conn);
 }
 
-/** If the first <b>in_len_max</b> characters in <b>start</b> contain a
- * double-quoted string with escaped characters, return the length of that
- * string (as encoded, including quotes).  Otherwise return -1. */
-static inline int
-get_escaped_string_length(const char *start, size_t in_len_max,
-                          int *chars_out)
-{
-  const char *cp, *end;
-  int chars = 0;
-
-  if (*start != '\"')
-    return -1;
-
-  cp = start+1;
-  end = start+in_len_max;
-
-  /* Calculate length. */
-  while (1) {
-    if (cp >= end) {
-      return -1; /* Too long. */
-    } else if (*cp == '\\') {
-      if (++cp == end)
-        return -1; /* Can't escape EOS. */
-      ++cp;
-      ++chars;
-    } else if (*cp == '\"') {
-      break;
-    } else {
-      ++cp;
-      ++chars;
-    }
-  }
-  if (chars_out)
-    *chars_out = chars;
-  return (int)(cp - start+1);
-}
-
-/** As decode_escaped_string, but does not decode the string: copies the
- * entire thing, including quotation marks. */
-const char *
-extract_escaped_string(const char *start, size_t in_len_max,
-                       char **out, size_t *out_len)
-{
-  int length = get_escaped_string_length(start, in_len_max, NULL);
-  if (length<0)
-    return NULL;
-  *out_len = length;
-  *out = tor_strndup(start, *out_len);
-  return start+length;
-}
-
-/** Given a pointer to a string starting at <b>start</b> containing
- * <b>in_len_max</b> characters, decode a string beginning with one double
- * quote, containing any number of non-quote characters or characters escaped
- * with a backslash, and ending with a final double quote.  Place the resulting
- * string (unquoted, unescaped) into a newly allocated string in *<b>out</b>;
- * store its length in <b>out_len</b>.  On success, return a pointer to the
- * character immediately following the escaped string.  On failure, return
- * NULL. */
-const char *
-decode_escaped_string(const char *start, size_t in_len_max,
-                   char **out, size_t *out_len)
-{
-  const char *cp, *end;
-  char *outp;
-  int len, n_chars = 0;
-
-  len = get_escaped_string_length(start, in_len_max, &n_chars);
-  if (len<0)
-    return NULL;
-
-  end = start+len-1; /* Index of last quote. */
-  tor_assert(*end == '\"');
-  outp = *out = tor_malloc(len+1);
-  *out_len = n_chars;
-
-  cp = start+1;
-  while (cp < end) {
-    if (*cp == '\\')
-      ++cp;
-    *outp++ = *cp++;
-  }
-  *outp = '\0';
-  tor_assert((outp - *out) == (int)*out_len);
-
-  return end+1;
-}
-
 /** Return a longname the node whose identity is <b>id_digest</b>. If
  * node_get_by_id() returns NULL, base 16 encoding of <b>id_digest</b> is
  * returned instead.
diff --git a/src/feature/control/control_fmt.h b/src/feature/control/control_fmt.h
index 74545eb309..8bbbaa95d0 100644
--- a/src/feature/control/control_fmt.h
+++ b/src/feature/control/control_fmt.h
@@ -25,10 +25,6 @@ char *circuit_describe_status_for_controller(origin_circuit_t *circ);
 
 size_t write_escaped_data(const char *data, size_t len, char **out);
 size_t read_escaped_data(const char *data, size_t len, char **out);
-const char *extract_escaped_string(const char *start, size_t in_len_max,
-                                   char **out, size_t *out_len);
-const char *decode_escaped_string(const char *start, size_t in_len_max,
-                                  char **out, size_t *out_len);
 void send_control_done(control_connection_t *conn);
 
 MOCK_DECL(const char *, node_describe_longname_by_id,(const char *id_digest));
diff --git a/src/feature/control/control_getinfo.c b/src/feature/control/control_getinfo.c
index a7a85f2fdf..5c6a0d4aa2 100644
--- a/src/feature/control/control_getinfo.c
+++ b/src/feature/control/control_getinfo.c
@@ -55,6 +55,7 @@
 #include "core/or/origin_circuit_st.h"
 #include "core/or/socks_request_st.h"
 #include "feature/control/control_connection_st.h"
+#include "feature/control/control_cmd_args_st.h"
 #include "feature/dircache/cached_dir_st.h"
 #include "feature/nodelist/extrainfo_st.h"
 #include "feature/nodelist/microdesc_st.h"
@@ -1584,21 +1585,22 @@ handle_getinfo_helper(control_connection_t *control_conn,
   return 0; /* unrecognized */
 }
 
+const control_cmd_syntax_t getinfo_syntax = {
+  .max_args = UINT_MAX,
+};
+
 /** Called when we receive a GETINFO command.  Try to fetch all requested
  * information, and reply with information or error message. */
 int
-handle_control_getinfo(control_connection_t *conn, uint32_t len,
-                       const char *body)
+handle_control_getinfo(control_connection_t *conn,
+                       const control_cmd_args_t *args)
 {
-  smartlist_t *questions = smartlist_new();
+  const smartlist_t *questions = args->args;
   smartlist_t *answers = smartlist_new();
   smartlist_t *unrecognized = smartlist_new();
   char *ans = NULL;
   int i;
-  (void) len; /* body is NUL-terminated, so it's safe to ignore the length. */
 
-  smartlist_split_string(questions, body, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
   SMARTLIST_FOREACH_BEGIN(questions, const char *, q) {
     const char *errmsg = NULL;
 
@@ -1653,8 +1655,6 @@ handle_control_getinfo(control_connection_t *conn, uint32_t len,
  done:
   SMARTLIST_FOREACH(answers, char *, cp, tor_free(cp));
   smartlist_free(answers);
-  SMARTLIST_FOREACH(questions, char *, cp, tor_free(cp));
-  smartlist_free(questions);
   SMARTLIST_FOREACH(unrecognized, char *, cp, tor_free(cp));
   smartlist_free(unrecognized);
 
diff --git a/src/feature/control/control_getinfo.h b/src/feature/control/control_getinfo.h
index d5a2feb3e0..2d56586f6d 100644
--- a/src/feature/control/control_getinfo.h
+++ b/src/feature/control/control_getinfo.h
@@ -12,8 +12,12 @@
 #ifndef TOR_CONTROL_GETINFO_H
 #define TOR_CONTROL_GETINFO_H
 
-int handle_control_getinfo(control_connection_t *conn, uint32_t len,
-                           const char *body);
+struct control_cmd_syntax_t;
+struct control_cmd_args_t;
+extern const struct control_cmd_syntax_t getinfo_syntax;
+
+int handle_control_getinfo(control_connection_t *conn,
+                           const struct control_cmd_args_t *args);
 
 #ifdef CONTROL_GETINFO_PRIVATE
 STATIC int getinfo_helper_onions(
diff --git a/src/lib/encoding/confline.c b/src/lib/encoding/confline.c
index 8110f3dd9c..fdb575e03f 100644
--- a/src/lib/encoding/confline.c
+++ b/src/lib/encoding/confline.c
@@ -82,6 +82,19 @@ config_line_find(const config_line_t *lines,
   return NULL;
 }
 
+/** As config_line_find(), but perform a case-insensitive comparison. */
+const config_line_t *
+config_line_find_case(const config_line_t *lines,
+                      const char *key)
+{
+  const config_line_t *cl;
+  for (cl = lines; cl; cl = cl->next) {
+    if (!strcasecmp(cl->key, key))
+      return cl;
+  }
+  return NULL;
+}
+
 /** Auxiliary function that does all the work of config_get_lines.
  * <b>recursion_level</b> is the count of how many nested %includes we have.
  * <b>opened_lst</b> will have a list of opened files if provided.
diff --git a/src/lib/encoding/confline.h b/src/lib/encoding/confline.h
index 3d9ae8a662..56ea36bf61 100644
--- a/src/lib/encoding/confline.h
+++ b/src/lib/encoding/confline.h
@@ -48,6 +48,8 @@ config_line_t *config_lines_dup_and_filter(const config_line_t *inp,
                                            const char *key);
 const config_line_t *config_line_find(const config_line_t *lines,
                                       const char *key);
+const config_line_t *config_line_find_case(const config_line_t *lines,
+                                           const char *key);
 int config_lines_eq(config_line_t *a, config_line_t *b);
 int config_count_key(const config_line_t *a, const char *key);
 void config_free_lines_(config_line_t *front);
diff --git a/src/lib/encoding/include.am b/src/lib/encoding/include.am
index 83e9211b6f..8272e4e5fa 100644
--- a/src/lib/encoding/include.am
+++ b/src/lib/encoding/include.am
@@ -11,6 +11,7 @@ src_lib_libtor_encoding_a_SOURCES =			\
 	src/lib/encoding/keyval.c			\
 	src/lib/encoding/kvline.c			\
 	src/lib/encoding/pem.c				\
+	src/lib/encoding/qstring.c			\
 	src/lib/encoding/time_fmt.c
 
 src_lib_libtor_encoding_testing_a_SOURCES = \
@@ -25,4 +26,5 @@ noinst_HEADERS +=					\
 	src/lib/encoding/keyval.h			\
 	src/lib/encoding/kvline.h			\
 	src/lib/encoding/pem.h				\
+	src/lib/encoding/qstring.h			\
 	src/lib/encoding/time_fmt.h
diff --git a/src/lib/encoding/kvline.c b/src/lib/encoding/kvline.c
index 307adc3f12..d4a8f510ba 100644
--- a/src/lib/encoding/kvline.c
+++ b/src/lib/encoding/kvline.c
@@ -16,6 +16,7 @@
 #include "lib/encoding/confline.h"
 #include "lib/encoding/cstring.h"
 #include "lib/encoding/kvline.h"
+#include "lib/encoding/qstring.h"
 #include "lib/malloc/malloc.h"
 #include "lib/string/compat_ctype.h"
 #include "lib/string/printf.h"
@@ -54,6 +55,15 @@ line_has_no_key(const config_line_t *line)
 }
 
 /**
+ * Return true iff the value in <b>line</b> is not set.
+ **/
+static bool
+line_has_no_val(const config_line_t *line)
+{
+  return line->value == NULL || strlen(line->value) == 0;
+}
+
+/**
  * Return true iff the all the lines in <b>line</b> can be encoded
  * using <b>flags</b>.
  **/
@@ -98,14 +108,25 @@ kvline_can_encode_lines(const config_line_t *line, unsigned flags)
  * If KV_OMIT_KEYS is set in <b>flags</b>, then pairs with empty keys are
  * allowed, and are encoded as 'Value'.  Otherwise, such pairs are not
  * allowed.
+ *
+ * If KV_OMIT_VALS is set in <b>flags</b>, then an empty value is
+ * encoded as 'Key', not as 'Key=' or 'Key=""'.  Mutually exclusive with
+ * KV_OMIT_KEYS.
+ *
+ * KV_QUOTED_QSTRING is not supported.
  */
 char *
 kvline_encode(const config_line_t *line,
               unsigned flags)
 {
+  tor_assert(! (flags & KV_QUOTED_QSTRING));
+
   if (!kvline_can_encode_lines(line, flags))
     return NULL;
 
+  tor_assert((flags & (KV_OMIT_KEYS|KV_OMIT_VALS)) !=
+             (KV_OMIT_KEYS|KV_OMIT_VALS));
+
   smartlist_t *elements = smartlist_new();
 
   for (; line; line = line->next) {
@@ -126,7 +147,10 @@ kvline_encode(const config_line_t *line,
       }
     }
 
-    if (esc) {
+    if ((flags & KV_OMIT_VALS) && line_has_no_val(line)) {
+      eq = "";
+      v = "";
+    } else if (esc) {
       tmp = esc_for_log(line->value);
       v = tmp;
     } else {
@@ -151,17 +175,30 @@ kvline_encode(const config_line_t *line,
  * allocated list of pairs on success, or NULL on failure.
  *
  * If KV_QUOTED is set in <b>flags</b>, then (double-)quoted values are
- * allowed. Otherwise, such values are not allowed.
+ * allowed and handled as C strings. Otherwise, such values are not allowed.
  *
  * If KV_OMIT_KEYS is set in <b>flags</b>, then values without keys are
  * allowed.  Otherwise, such values are not allowed.
+ *
+ * If KV_OMIT_VALS is set in <b>flags</b>, then keys without values are
+ * allowed.  Otherwise, such keys are not allowed.  Mutually exclusive with
+ * KV_OMIT_KEYS.
+ *
+ * If KV_QUOTED_QSTRING is set in <b>flags</b>, then double-quoted values
+ * are allowed and handled as QuotedStrings per qstring.c.  Do not add
+ * new users of this flag.
  */
 config_line_t *
 kvline_parse(const char *line, unsigned flags)
 {
+  tor_assert((flags & (KV_OMIT_KEYS|KV_OMIT_VALS)) !=
+             (KV_OMIT_KEYS|KV_OMIT_VALS));
+
   const char *cp = line, *cplast = NULL;
-  bool omit_keys = (flags & KV_OMIT_KEYS) != 0;
-  bool quoted = (flags & KV_QUOTED) != 0;
+  const bool omit_keys = (flags & KV_OMIT_KEYS) != 0;
+  const bool omit_vals = (flags & KV_OMIT_VALS) != 0;
+  const bool quoted = (flags & (KV_QUOTED|KV_QUOTED_QSTRING)) != 0;
+  const bool c_quoted = (flags & (KV_QUOTED)) != 0;
 
   config_line_t *result = NULL;
   config_line_t **next_line = &result;
@@ -171,27 +208,33 @@ kvline_parse(const char *line, unsigned flags)
 
   while (*cp) {
     key = val = NULL;
+    /* skip all spaces */
     {
       size_t idx = strspn(cp, " \t\r\v\n");
       cp += idx;
     }
     if (BUG(cp == cplast)) {
-      /* If we didn't parse anything, this code is broken. */
+      /* If we didn't parse anything since the last loop, this code is
+       * broken. */
       goto err; // LCOV_EXCL_LINE
     }
     cplast = cp;
     if (! *cp)
       break; /* End of string; we're done. */
 
-    /* Possible formats are K=V, K="V", V, and "V", depending on flags. */
+    /* Possible formats are K=V, K="V", K, V, and "V", depending on flags. */
 
-    /* Find the key. */
+    /* Find where the key ends */
     if (*cp != '\"') {
       size_t idx = strcspn(cp, " \t\r\v\n=");
 
       if (cp[idx] == '=') {
         key = tor_memdup_nulterm(cp, idx);
         cp += idx + 1;
+      } else if (omit_vals) {
+        key = tor_memdup_nulterm(cp, idx);
+        cp += idx;
+        goto commit;
       } else {
         if (!omit_keys)
           goto err;
@@ -203,7 +246,11 @@ kvline_parse(const char *line, unsigned flags)
       if (!quoted)
         goto err;
       size_t len=0;
-      cp = unescape_string(cp, &val, &len);
+      if (c_quoted) {
+        cp = unescape_string(cp, &val, &len);
+      } else {
+        cp = decode_qstring(cp, strlen(cp), &val, &len);
+      }
       if (cp == NULL || len != strlen(val)) {
         // The string contains a NUL or is badly coded.
         goto err;
@@ -214,6 +261,7 @@ kvline_parse(const char *line, unsigned flags)
       cp += idx;
     }
 
+  commit:
     if (key && strlen(key) == 0) {
       /* We don't allow empty keys. */
       goto err;
@@ -221,13 +269,15 @@ kvline_parse(const char *line, unsigned flags)
 
     *next_line = tor_malloc_zero(sizeof(config_line_t));
     (*next_line)->key = key ? key : tor_strdup("");
-    (*next_line)->value = val;
+    (*next_line)->value = val ? val : tor_strdup("");
     next_line = &(*next_line)->next;
     key = val = NULL;
   }
 
-  if (!kvline_can_encode_lines(result, flags)) {
-    goto err;
+  if (! (flags & KV_QUOTED_QSTRING)) {
+    if (!kvline_can_encode_lines(result, flags)) {
+      goto err;
+    }
   }
   return result;
 
diff --git a/src/lib/encoding/kvline.h b/src/lib/encoding/kvline.h
index 4eed30a223..dea2ce1809 100644
--- a/src/lib/encoding/kvline.h
+++ b/src/lib/encoding/kvline.h
@@ -17,6 +17,8 @@ struct config_line_t;
 
 #define KV_QUOTED    (1u<<0)
 #define KV_OMIT_KEYS (1u<<1)
+#define KV_OMIT_VALS (1u<<2)
+#define KV_QUOTED_QSTRING (1u<<3)
 
 struct config_line_t *kvline_parse(const char *line, unsigned flags);
 char *kvline_encode(const struct config_line_t *line, unsigned flags);
diff --git a/src/lib/encoding/qstring.c b/src/lib/encoding/qstring.c
new file mode 100644
index 0000000000..a92d28c706
--- /dev/null
+++ b/src/lib/encoding/qstring.c
@@ -0,0 +1,90 @@
+/* Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * \file qstring.c
+ * \brief Implement QuotedString parsing.
+ *
+ * Note that this is only used for controller authentication; do not
+ * create new users for this.  Instead, prefer the cstring.c functions.
+ **/
+
+#include "orconfig.h"
+#include "lib/encoding/qstring.h"
+#include "lib/malloc/malloc.h"
+#include "lib/log/util_bug.h"
+
+/** If the first <b>in_len_max</b> characters in <b>start</b> contain a
+ * QuotedString, return the length of that
+ * string (as encoded, including quotes).  Otherwise return -1. */
+static inline int
+get_qstring_length(const char *start, size_t in_len_max,
+                          int *chars_out)
+{
+  const char *cp, *end;
+  int chars = 0;
+
+  if (*start != '\"')
+    return -1;
+
+  cp = start+1;
+  end = start+in_len_max;
+
+  /* Calculate length. */
+  while (1) {
+    if (cp >= end) {
+      return -1; /* Too long. */
+    } else if (*cp == '\\') {
+      if (++cp == end)
+        return -1; /* Can't escape EOS. */
+      ++cp;
+      ++chars;
+    } else if (*cp == '\"') {
+      break;
+    } else {
+      ++cp;
+      ++chars;
+    }
+  }
+  if (chars_out)
+    *chars_out = chars;
+  return (int)(cp - start+1);
+}
+
+/** Given a pointer to a string starting at <b>start</b> containing
+ * <b>in_len_max</b> characters, decode a string beginning with one double
+ * quote, containing any number of non-quote characters or characters escaped
+ * with a backslash, and ending with a final double quote.  Place the resulting
+ * string (unquoted, unescaped) into a newly allocated string in *<b>out</b>;
+ * store its length in <b>out_len</b>.  On success, return a pointer to the
+ * character immediately following the escaped string.  On failure, return
+ * NULL. */
+const char *
+decode_qstring(const char *start, size_t in_len_max,
+               char **out, size_t *out_len)
+{
+  const char *cp, *end;
+  char *outp;
+  int len, n_chars = 0;
+
+  len = get_qstring_length(start, in_len_max, &n_chars);
+  if (len<0)
+    return NULL;
+
+  end = start+len-1; /* Index of last quote. */
+  tor_assert(*end == '\"');
+  outp = *out = tor_malloc(len+1);
+  *out_len = n_chars;
+
+  cp = start+1;
+  while (cp < end) {
+    if (*cp == '\\')
+      ++cp;
+    *outp++ = *cp++;
+  }
+  *outp = '\0';
+  tor_assert((outp - *out) == (int)*out_len);
+
+  return end+1;
+}
diff --git a/src/lib/encoding/qstring.h b/src/lib/encoding/qstring.h
new file mode 100644
index 0000000000..fe15b655f1
--- /dev/null
+++ b/src/lib/encoding/qstring.h
@@ -0,0 +1,18 @@
+/* Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * \file qstring.h
+ * \brief Header for qstring.c
+ */
+
+#ifndef TOR_ENCODING_QSTRING_H
+#define TOR_ENCODING_QSTRING_H
+
+#include <stddef.h>
+
+const char *decode_qstring(const char *start, size_t in_len_max,
+                           char **out, size_t *out_len);
+
+#endif
diff --git a/src/test/fuzz/fuzz_strops.c b/src/test/fuzz/fuzz_strops.c
index a37cbb5be8..459b4e21aa 100644
--- a/src/test/fuzz/fuzz_strops.c
+++ b/src/test/fuzz/fuzz_strops.c
@@ -235,6 +235,18 @@ fuzz_main(const uint8_t *stdin_buf, size_t data_size)
       kv_flags = 0;
       ENCODE_ROUNDTRIP(kv_enc, kv_dec, config_free_lines_);
       break;
+    case 7:
+      kv_flags = KV_OMIT_VALS;
+      ENCODE_ROUNDTRIP(kv_enc, kv_dec, config_free_lines_);
+      break;
+    case 8:
+      kv_flags = KV_QUOTED;
+      ENCODE_ROUNDTRIP(kv_enc, kv_dec, config_free_lines_);
+      break;
+    case 9:
+      kv_flags = KV_QUOTED|KV_OMIT_VALS;
+      ENCODE_ROUNDTRIP(kv_enc, kv_dec, config_free_lines_);
+      break;
     }
 
   return 0;
diff --git a/src/test/test_config.c b/src/test/test_config.c
index 72649dd9b1..6cfb7b764b 100644
--- a/src/test/test_config.c
+++ b/src/test/test_config.c
@@ -5886,6 +5886,61 @@ test_config_kvline_parse(void *arg)
   tt_assert(lines);
   tt_str_op(lines->key, OP_EQ, "AB");
   tt_str_op(lines->value, OP_EQ, "");
+  config_free_lines(lines);
+
+  lines = kvline_parse("AB=", KV_OMIT_VALS);
+  tt_assert(lines);
+  tt_str_op(lines->key, OP_EQ, "AB");
+  tt_str_op(lines->value, OP_EQ, "");
+  config_free_lines(lines);
+
+  lines = kvline_parse(" AB ", KV_OMIT_VALS);
+  tt_assert(lines);
+  tt_str_op(lines->key, OP_EQ, "AB");
+  tt_str_op(lines->value, OP_EQ, "");
+  config_free_lines(lines);
+
+  lines = kvline_parse("AB", KV_OMIT_VALS);
+  tt_assert(lines);
+  tt_str_op(lines->key, OP_EQ, "AB");
+  tt_str_op(lines->value, OP_EQ, "");
+  enc = kvline_encode(lines, KV_OMIT_VALS);
+  tt_str_op(enc, OP_EQ, "AB");
+  tor_free(enc);
+  config_free_lines(lines);
+
+  lines = kvline_parse("AB=CD", KV_OMIT_VALS);
+  tt_assert(lines);
+  tt_str_op(lines->key, OP_EQ, "AB");
+  tt_str_op(lines->value, OP_EQ, "CD");
+  enc = kvline_encode(lines, KV_OMIT_VALS);
+  tt_str_op(enc, OP_EQ, "AB=CD");
+  tor_free(enc);
+  config_free_lines(lines);
+
+  lines = kvline_parse("AB=CD DE FGH=I", KV_OMIT_VALS);
+  tt_assert(lines);
+  tt_str_op(lines->key, OP_EQ, "AB");
+  tt_str_op(lines->value, OP_EQ, "CD");
+  tt_str_op(lines->next->key, OP_EQ, "DE");
+  tt_str_op(lines->next->value, OP_EQ, "");
+  tt_str_op(lines->next->next->key, OP_EQ, "FGH");
+  tt_str_op(lines->next->next->value, OP_EQ, "I");
+  enc = kvline_encode(lines, KV_OMIT_VALS);
+  tt_str_op(enc, OP_EQ, "AB=CD DE FGH=I");
+  tor_free(enc);
+  config_free_lines(lines);
+
+  lines = kvline_parse("AB=\"CD E\" DE FGH=\"I\"", KV_OMIT_VALS|KV_QUOTED);
+  tt_assert(lines);
+  tt_str_op(lines->key, OP_EQ, "AB");
+  tt_str_op(lines->value, OP_EQ, "CD E");
+  tt_str_op(lines->next->key, OP_EQ, "DE");
+  tt_str_op(lines->next->value, OP_EQ, "");
+  tt_str_op(lines->next->next->key, OP_EQ, "FGH");
+  tt_str_op(lines->next->next->value, OP_EQ, "I");
+  enc = kvline_encode(lines, KV_OMIT_VALS|KV_QUOTED);
+  tt_str_op(enc, OP_EQ, "AB=\"CD E\" DE FGH=I");
 
  done:
   config_free_lines(lines);
diff --git a/src/test/test_controller.c b/src/test/test_controller.c
index f3af6d2ec0..c130248592 100644
--- a/src/test/test_controller.c
+++ b/src/test/test_controller.c
@@ -18,12 +18,189 @@
 #include "test/test.h"
 #include "test/test_helpers.h"
 #include "lib/net/resolve.h"
+#include "lib/encoding/confline.h"
+#include "lib/encoding/kvline.h"
 
 #include "feature/control/control_connection_st.h"
+#include "feature/control/control_cmd_args_st.h"
 #include "feature/dirclient/download_status_st.h"
 #include "feature/nodelist/microdesc_st.h"
 #include "feature/nodelist/node_st.h"
 
+typedef struct {
+  const char *input;
+  const char *expected_parse;
+  const char *expected_error;
+} parser_testcase_t;
+
+typedef struct {
+  const control_cmd_syntax_t *syntax;
+  size_t n_testcases;
+  const parser_testcase_t *testcases;
+} parse_test_params_t;
+
+static char *
+control_cmd_dump_args(const control_cmd_args_t *result)
+{
+  buf_t *buf = buf_new();
+  buf_add_string(buf, "{ args=[");
+  if (result->args) {
+    if (smartlist_len(result->args)) {
+        buf_add_string(buf, " ");
+    }
+    SMARTLIST_FOREACH_BEGIN(result->args, const char *, s) {
+      const bool last = (s_sl_idx == smartlist_len(result->args)-1);
+      buf_add_printf(buf, "%s%s ",
+                     escaped(s),
+                     last ? "" : ",");
+    } SMARTLIST_FOREACH_END(s);
+  }
+  buf_add_string(buf, "]");
+  if (result->cmddata) {
+    buf_add_string(buf, ", obj=");
+    buf_add_string(buf, escaped(result->cmddata));
+  }
+  if (result->kwargs) {
+    buf_add_string(buf, ", { ");
+    const config_line_t *line;
+    for (line = result->kwargs; line; line = line->next) {
+      const bool last = (line->next == NULL);
+      buf_add_printf(buf, "%s=%s%s ", line->key, escaped(line->value),
+                     last ? "" : ",");
+    }
+    buf_add_string(buf, "}");
+  }
+  buf_add_string(buf, " }");
+
+  char *encoded = buf_extract(buf, NULL);
+  buf_free(buf);
+  return encoded;
+}
+
+static void
+test_controller_parse_cmd(void *arg)
+{
+  const parse_test_params_t *params = arg;
+  control_cmd_args_t *result = NULL;
+  char *error = NULL;
+  char *encoded = NULL;
+
+  for (size_t i = 0; i < params->n_testcases; ++i) {
+    const parser_testcase_t *t = &params->testcases[i];
+    result = control_cmd_parse_args("EXAMPLE",
+                                    params->syntax,
+                                    strlen(t->input),
+                                    t->input,
+                                    &error);
+    // A valid test should expect exactly one parse or error.
+    tt_int_op((t->expected_parse == NULL), OP_NE,
+              (t->expected_error == NULL));
+    // We get a result or an error, not both.
+    tt_int_op((result == NULL), OP_EQ, (error != NULL));
+    // We got the one we expected.
+    tt_int_op((result == NULL), OP_EQ, (t->expected_parse == NULL));
+
+    if (result) {
+      encoded = control_cmd_dump_args(result);
+      tt_str_op(encoded, OP_EQ, t->expected_parse);
+    } else {
+      tt_str_op(error, OP_EQ, t->expected_error);
+    }
+
+    tor_free(error);
+    tor_free(encoded);
+    control_cmd_args_free(result);
+  }
+
+ done:
+  tor_free(error);
+  tor_free(encoded);
+  control_cmd_args_free(result);
+}
+
+#define OK(inp, out) \
+  { inp "\r\n", out, NULL }
+#define ERR(inp, err) \
+  { inp "\r\n", NULL, err }
+
+#define TESTPARAMS(syntax, array)                \
+  { &syntax,                                     \
+      ARRAY_LENGTH(array),                       \
+      array }
+
+static const parser_testcase_t one_to_three_tests[] = {
+   ERR("", "Need at least 1 argument(s)"),
+   ERR("   \t", "Need at least 1 argument(s)"),
+   OK("hello", "{ args=[ \"hello\" ] }"),
+   OK("hello world", "{ args=[ \"hello\", \"world\" ] }"),
+   OK("hello  world", "{ args=[ \"hello\", \"world\" ] }"),
+   OK("  hello  world", "{ args=[ \"hello\", \"world\" ] }"),
+   OK("  hello  world      ", "{ args=[ \"hello\", \"world\" ] }"),
+   OK("hello there world", "{ args=[ \"hello\", \"there\", \"world\" ] }"),
+   ERR("why hello there world", "Cannot accept more than 3 argument(s)"),
+   ERR("hello\r\nworld.\r\n.", "Unexpected body"),
+};
+
+static const control_cmd_syntax_t one_to_three_syntax = {
+   .min_args=1, .max_args=3
+};
+
+static const parse_test_params_t parse_one_to_three_params =
+  TESTPARAMS( one_to_three_syntax, one_to_three_tests );
+
+// =
+static const parser_testcase_t no_args_one_obj_tests[] = {
+  ERR("Hi there!\r\n.", "Cannot accept more than 0 argument(s)"),
+  ERR("", "Empty body"),
+  OK("\r\n", "{ args=[], obj=\"\\n\" }"),
+  OK("\r\nHello world\r\n", "{ args=[], obj=\"Hello world\\n\\n\" }"),
+  OK("\r\nHello\r\nworld\r\n", "{ args=[], obj=\"Hello\\nworld\\n\\n\" }"),
+  OK("\r\nHello\r\n..\r\nworld\r\n",
+     "{ args=[], obj=\"Hello\\n.\\nworld\\n\\n\" }"),
+};
+static const control_cmd_syntax_t no_args_one_obj_syntax = {
+   .min_args=0, .max_args=0,
+   .want_cmddata=true,
+};
+static const parse_test_params_t parse_no_args_one_obj_params =
+  TESTPARAMS( no_args_one_obj_syntax, no_args_one_obj_tests );
+
+static const parser_testcase_t no_args_kwargs_tests[] = {
+  OK("", "{ args=[] }"),
+  OK(" ", "{ args=[] }"),
+  OK("hello there=world", "{ args=[], { hello=\"\", there=\"world\" } }"),
+  OK("hello there=world today",
+     "{ args=[], { hello=\"\", there=\"world\", today=\"\" } }"),
+  ERR("=Foo", "Cannot parse keyword argument(s)"),
+};
+static const control_cmd_syntax_t no_args_kwargs_syntax = {
+   .min_args=0, .max_args=0,
+   .accept_keywords=true,
+   .kvline_flags=KV_OMIT_VALS
+};
+static const parse_test_params_t parse_no_args_kwargs_params =
+  TESTPARAMS( no_args_kwargs_syntax, no_args_kwargs_tests );
+
+static const char *one_arg_kwargs_allow_keywords[] = {
+  "Hello", "world", NULL
+};
+static const parser_testcase_t one_arg_kwargs_tests[] = {
+  ERR("", "Need at least 1 argument(s)"),
+  OK("Hi", "{ args=[ \"Hi\" ] }"),
+  ERR("hello there=world", "Unrecognized keyword argument \"there\""),
+  OK("Hi HELLO=foo", "{ args=[ \"Hi\" ], { HELLO=\"foo\" } }"),
+  OK("Hi world=\"bar baz\" hello  ",
+     "{ args=[ \"Hi\" ], { world=\"bar baz\", hello=\"\" } }"),
+};
+static const control_cmd_syntax_t one_arg_kwargs_syntax = {
+   .min_args=1, .max_args=1,
+   .accept_keywords=true,
+   .allowed_keywords=one_arg_kwargs_allow_keywords,
+   .kvline_flags=KV_OMIT_VALS|KV_QUOTED,
+};
+static const parse_test_params_t parse_one_arg_kwargs_params =
+  TESTPARAMS( one_arg_kwargs_syntax, one_arg_kwargs_tests );
+
 static void
 test_add_onion_helper_keyarg_v3(void *arg)
 {
@@ -1617,7 +1794,15 @@ test_getinfo_md_all(void *arg)
   return;
 }
 
+#define PARSER_TEST(type)                                             \
+  { "parse/" #type, test_controller_parse_cmd, 0, &passthrough_setup, \
+      (void*)&parse_ ## type ## _params }
+
 struct testcase_t controller_tests[] = {
+  PARSER_TEST(one_to_three),
+  PARSER_TEST(no_args_one_obj),
+  PARSER_TEST(no_args_kwargs),
+  PARSER_TEST(one_arg_kwargs),
   { "add_onion_helper_keyarg_v2", test_add_onion_helper_keyarg_v2, 0,
     NULL, NULL },
   { "add_onion_helper_keyarg_v3", test_add_onion_helper_keyarg_v3, 0,