Replace all calls to parse_client_transport_line() or parse_server_transport_line() with new parse_transport_line() stub

src/or/config.c

@@ -538,6 +538,10 @@ static int options_transition_affects_descriptor(
 static int check_nickname_list(char **lst, const char *name, char **msg);
 static char *get_bindaddr_from_transport_listen_line(const char *line,
                                                      const char *transport);
+static int parse_client_transport_line(const or_options_t *options,
+                                       const char *line, int validate_only);
+static int parse_server_transport_line(const or_options_t *options,
+                                       const char *line, int validate_only);
 static int parse_dir_authority_line(const char *line,
                                  dirinfo_type_t required_type,
                                  int validate_only);
@@ -1423,7 +1427,7 @@ options_act(const or_options_t *old_options)
   pt_prepare_proxy_list_for_config_read();
   if (options->ClientTransportPlugin) {
     for (cl = options->ClientTransportPlugin; cl; cl = cl->next) {
-      if (parse_client_transport_line(options, cl->value, 0)<0) {
+      if (parse_transport_line(options, cl->value, 0, 0) < 0) {
         log_warn(LD_BUG,
                  "Previously validated ClientTransportPlugin line "
                  "could not be added!");
@@ -1434,7 +1438,7 @@ options_act(const or_options_t *old_options)
 
   if (options->ServerTransportPlugin && server_mode(options)) {
     for (cl = options->ServerTransportPlugin; cl; cl = cl->next) {
-      if (parse_server_transport_line(options, cl->value, 0)<0) {
+      if (parse_transport_line(options, cl->value, 0, 1) < 0) {
         log_warn(LD_BUG,
                  "Previously validated ServerTransportPlugin line "
                  "could not be added!");
@@ -3287,12 +3291,12 @@ options_validate(or_options_t *old_options, or_options_t *options,
   }
 
   for (cl = options->ClientTransportPlugin; cl; cl = cl->next) {
-    if (parse_client_transport_line(options, cl->value, 1)<0)
+    if (parse_transport_line(options, cl->value, 1, 0) < 0)
       REJECT("Invalid client transport line. See logs for details.");
   }
 
   for (cl = options->ServerTransportPlugin; cl; cl = cl->next) {
-    if (parse_server_transport_line(options, cl->value, 1)<0)
+    if (parse_transport_line(options, cl->value, 1, 1) < 0)
       REJECT("Invalid server transport line. See logs for details.");
   }
 
@@ -4748,6 +4752,29 @@ parse_bridge_line(const char *line)
   return bridge_line;
 }
 
+/** Read the contents of a ClientTransportPlugin or ServerTransportPlugin
+ * line from <b>line</b>, depending on the value of <b>server</b>. Return 0
+ * if the line is well-formed, and -1 if it isn't.
+ *
+ * If <b>validate_only</b> is 0, the line is well-formed, and the transport is
+ * needed by some bridge:
+ * - If it's an external proxy line, add the transport described in the line to
+ * our internal transport list.
+ * - If it's a managed proxy line, launch the managed proxy.
+ */
+
+STATIC int
+parse_transport_line(const or_options_t *options,
+                     const char *line, int validate_only,
+                     int server)
+{
+  if (server) {
+    return parse_server_transport_line(options, line, validate_only);
+  } else {
+    return parse_client_transport_line(options, line, validate_only);
+  }
+}
+
 /** Read the contents of a ClientTransportPlugin line from
  * <b>line</b>. Return 0 if the line is well-formed, and -1 if it
  * isn't.
@@ -4757,7 +4784,7 @@ parse_bridge_line(const char *line)
  * - If it's an external proxy line, add the transport described in the line to
  * our internal transport list.
  * - If it's a managed proxy line, launch the managed proxy. */
-STATIC int
+static int
 parse_client_transport_line(const or_options_t *options,
                             const char *line, int validate_only)
 {
@@ -4901,6 +4928,133 @@ parse_client_transport_line(const or_options_t *options,
   return r;
 }
 
+/** Read the contents of a ServerTransportPlugin line from
+ * <b>line</b>. Return 0 if the line is well-formed, and -1 if it
+ * isn't.
+ * If <b>validate_only</b> is 0, the line is well-formed, and it's a
+ * managed proxy line, launch the managed proxy. */
+static int
+parse_server_transport_line(const or_options_t *options,
+                            const char *line, int validate_only)
+{
+  smartlist_t *items = NULL;
+  int r;
+  const char *transports=NULL;
+  smartlist_t *transport_list=NULL;
+  char *type=NULL;
+  char *addrport=NULL;
+  tor_addr_t addr;
+  uint16_t port = 0;
+
+  /* managed proxy options */
+  int is_managed=0;
+  char **proxy_argv=NULL;
+  char **tmp=NULL;
+  int proxy_argc,i;
+
+  int line_length;
+
+  items = smartlist_new();
+  smartlist_split_string(items, line, NULL,
+                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, -1);
+
+  line_length =  smartlist_len(items);
+  if (line_length < 3) {
+    log_warn(LD_CONFIG, "Too few arguments on ServerTransportPlugin line.");
+    goto err;
+  }
+
+  /* Get the first line element, split it to commas into
+     transport_list (in case it's multiple transports) and validate
+     the transport names. */
+  transports = smartlist_get(items, 0);
+  transport_list = smartlist_new();
+  smartlist_split_string(transport_list, transports, ",",
+                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
+  SMARTLIST_FOREACH_BEGIN(transport_list, const char *, transport_name) {
+    if (!string_is_C_identifier(transport_name)) {
+      log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).",
+               transport_name);
+      goto err;
+    }
+  } SMARTLIST_FOREACH_END(transport_name);
+
+  type = smartlist_get(items, 1);
+
+  if (!strcmp(type, "exec")) {
+    is_managed=1;
+  } else if (!strcmp(type, "proxy")) {
+    is_managed=0;
+  } else {
+    log_warn(LD_CONFIG, "Strange ServerTransportPlugin type '%s'", type);
+    goto err;
+  }
+
+  if (is_managed && options->Sandbox) {
+    log_warn(LD_CONFIG, "Managed proxies are not compatible with Sandbox mode."
+             "(ServerTransportPlugin line was %s)", escaped(line));
+    goto err;
+  }
+
+  if (is_managed) { /* managed */
+    if (!validate_only) {
+      proxy_argc = line_length-2;
+      tor_assert(proxy_argc > 0);
+      proxy_argv = tor_malloc_zero(sizeof(char*)*(proxy_argc+1));
+      tmp = proxy_argv;
+
+      for (i=0;i<proxy_argc;i++) { /* store arguments */
+        *tmp++ = smartlist_get(items, 2);
+        smartlist_del_keeporder(items, 2);
+      }
+      *tmp = NULL; /*terminated with NULL, just like execve() likes it*/
+
+      /* kickstart the thing */
+      pt_kickstart_server_proxy(transport_list, proxy_argv);
+    }
+  } else { /* external */
+    if (smartlist_len(transport_list) != 1) {
+      log_warn(LD_CONFIG, "You can't have an external proxy with "
+               "more than one transports.");
+      goto err;
+    }
+
+    addrport = smartlist_get(items, 2);
+
+    if (tor_addr_port_lookup(addrport, &addr, &port)<0) {
+      log_warn(LD_CONFIG, "Error parsing transport "
+               "address '%s'", addrport);
+      goto err;
+    }
+    if (!port) {
+      log_warn(LD_CONFIG,
+               "Transport address '%s' has no port.", addrport);
+      goto err;
+    }
+
+    if (!validate_only) {
+      log_info(LD_DIR, "Server transport '%s' at %s.",
+               transports, fmt_addrport(&addr, port));
+    }
+  }
+
+  r = 0;
+  goto done;
+
+ err:
+  r = -1;
+
+ done:
+  SMARTLIST_FOREACH(items, char*, s, tor_free(s));
+  smartlist_free(items);
+  if (transport_list) {
+    SMARTLIST_FOREACH(transport_list, char*, s, tor_free(s));
+    smartlist_free(transport_list);
+  }
+
+  return r;
+}
+
 /** Given a ServerTransportListenAddr <b>line</b>, return its
  *  <address:port> string. Return NULL if the line was not
  *  well-formed.
@@ -5052,133 +5206,6 @@ get_options_for_server_transport(const char *transport)
   return NULL;
 }
 
-/** Read the contents of a ServerTransportPlugin line from
- * <b>line</b>. Return 0 if the line is well-formed, and -1 if it
- * isn't.
- * If <b>validate_only</b> is 0, the line is well-formed, and it's a
- * managed proxy line, launch the managed proxy. */
-STATIC int
-parse_server_transport_line(const or_options_t *options,
-                            const char *line, int validate_only)
-{
-  smartlist_t *items = NULL;
-  int r;
-  const char *transports=NULL;
-  smartlist_t *transport_list=NULL;
-  char *type=NULL;
-  char *addrport=NULL;
-  tor_addr_t addr;
-  uint16_t port = 0;
-
-  /* managed proxy options */
-  int is_managed=0;
-  char **proxy_argv=NULL;
-  char **tmp=NULL;
-  int proxy_argc,i;
-
-  int line_length;
-
-  items = smartlist_new();
-  smartlist_split_string(items, line, NULL,
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, -1);
-
-  line_length =  smartlist_len(items);
-  if (line_length < 3) {
-    log_warn(LD_CONFIG, "Too few arguments on ServerTransportPlugin line.");
-    goto err;
-  }
-
-  /* Get the first line element, split it to commas into
-     transport_list (in case it's multiple transports) and validate
-     the transport names. */
-  transports = smartlist_get(items, 0);
-  transport_list = smartlist_new();
-  smartlist_split_string(transport_list, transports, ",",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-  SMARTLIST_FOREACH_BEGIN(transport_list, const char *, transport_name) {
-    if (!string_is_C_identifier(transport_name)) {
-      log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).",
-               transport_name);
-      goto err;
-    }
-  } SMARTLIST_FOREACH_END(transport_name);
-
-  type = smartlist_get(items, 1);
-
-  if (!strcmp(type, "exec")) {
-    is_managed=1;
-  } else if (!strcmp(type, "proxy")) {
-    is_managed=0;
-  } else {
-    log_warn(LD_CONFIG, "Strange ServerTransportPlugin type '%s'", type);
-    goto err;
-  }
-
-  if (is_managed && options->Sandbox) {
-    log_warn(LD_CONFIG, "Managed proxies are not compatible with Sandbox mode."
-             "(ServerTransportPlugin line was %s)", escaped(line));
-    goto err;
-  }
-
-  if (is_managed) { /* managed */
-    if (!validate_only) {
-      proxy_argc = line_length-2;
-      tor_assert(proxy_argc > 0);
-      proxy_argv = tor_malloc_zero(sizeof(char*)*(proxy_argc+1));
-      tmp = proxy_argv;
-
-      for (i=0;i<proxy_argc;i++) { /* store arguments */
-        *tmp++ = smartlist_get(items, 2);
-        smartlist_del_keeporder(items, 2);
-      }
-      *tmp = NULL; /*terminated with NULL, just like execve() likes it*/
-
-      /* kickstart the thing */
-      pt_kickstart_server_proxy(transport_list, proxy_argv);
-    }
-  } else { /* external */
-    if (smartlist_len(transport_list) != 1) {
-      log_warn(LD_CONFIG, "You can't have an external proxy with "
-               "more than one transports.");
-      goto err;
-    }
-
-    addrport = smartlist_get(items, 2);
-
-    if (tor_addr_port_lookup(addrport, &addr, &port)<0) {
-      log_warn(LD_CONFIG, "Error parsing transport "
-               "address '%s'", addrport);
-      goto err;
-    }
-    if (!port) {
-      log_warn(LD_CONFIG,
-               "Transport address '%s' has no port.", addrport);
-      goto err;
-    }
-
-    if (!validate_only) {
-      log_info(LD_DIR, "Server transport '%s' at %s.",
-               transports, fmt_addrport(&addr, port));
-    }
-  }
-
-  r = 0;
-  goto done;
-
- err:
-  r = -1;
-
- done:
-  SMARTLIST_FOREACH(items, char*, s, tor_free(s));
-  smartlist_free(items);
-  if (transport_list) {
-    SMARTLIST_FOREACH(transport_list, char*, s, tor_free(s));
-    smartlist_free(transport_list);
-  }
-
-  return r;
-}
-
 /** Read the contents of a DirAuthority line from <b>line</b>. If
  * <b>validate_only</b> is 0, and the line is well-formed, and it
  * shares any bits with <b>required_type</b> or <b>required_type</b>

src/or/config.h

@@ -140,10 +140,9 @@ STATIC int options_validate(or_options_t *old_options,
                             or_options_t *options,
                             or_options_t *default_options,
                             int from_setconf, char **msg);
-STATIC int parse_client_transport_line(const or_options_t *options,
-                                       const char *line, int validate_only);
-STATIC int parse_server_transport_line(const or_options_t *options,
-                                       const char *line, int validate_only);
+STATIC int parse_transport_line(const or_options_t *options,
+                                const char *line, int validate_only,
+                                int server);
 #endif
 
 #endif

src/test/test_config.c

@@ -608,84 +608,84 @@ test_config_parse_transport_plugin_line(void *arg)
   int old_transport_is_needed_mock_call_count;
 
   /* Bad transport lines - too short */
-  r = parse_client_transport_line(options, "bad", 1);
+  r = parse_transport_line(options, "bad", 1, 0);
   test_assert(r < 0);
-  r = parse_server_transport_line(options, "bad", 1);
+  r = parse_transport_line(options, "bad", 1, 1);
   test_assert(r < 0);
-  r = parse_client_transport_line(options, "bad bad", 1);
+  r = parse_transport_line(options, "bad bad", 1, 0);
   test_assert(r < 0);
-  r = parse_server_transport_line(options, "bad bad", 1);
+  r = parse_transport_line(options, "bad bad", 1, 1);
   test_assert(r < 0);
 
   /* Test transport list parsing */
-  r = parse_client_transport_line(options,
-      "transport_1 exec /usr/bin/fake-transport", 1);
+  r = parse_transport_line(options,
+      "transport_1 exec /usr/bin/fake-transport", 1, 0);
   test_assert(r == 0);
-  r = parse_server_transport_line(options,
-   "transport_1 exec /usr/bin/fake-transport", 1);
+  r = parse_transport_line(options,
+   "transport_1 exec /usr/bin/fake-transport", 1, 1);
   test_assert(r == 0);
-  r = parse_client_transport_line(options,
-      "transport_1,transport_2 exec /usr/bin/fake-transport", 1);
+  r = parse_transport_line(options,
+      "transport_1,transport_2 exec /usr/bin/fake-transport", 1, 0);
   test_assert(r == 0);
-  r = parse_server_transport_line(options,
-      "transport_1,transport_2 exec /usr/bin/fake-transport", 1);
+  r = parse_transport_line(options,
+      "transport_1,transport_2 exec /usr/bin/fake-transport", 1, 1);
   test_assert(r == 0);
   /* Bad transport identifiers */
-  r = parse_client_transport_line(options,
-      "transport_* exec /usr/bin/fake-transport", 1);
+  r = parse_transport_line(options,
+      "transport_* exec /usr/bin/fake-transport", 1, 0);
   test_assert(r < 0);
-  r = parse_server_transport_line(options,
-      "transport_* exec /usr/bin/fake-transport", 1);
+  r = parse_transport_line(options,
+      "transport_* exec /usr/bin/fake-transport", 1, 1);
   test_assert(r < 0);
 
   /* Check SOCKS cases for client transport */
-  r = parse_client_transport_line(options,
-      "transport_1 socks4 1.2.3.4:567", 1);
+  r = parse_transport_line(options,
+      "transport_1 socks4 1.2.3.4:567", 1, 0);
   test_assert(r == 0);
-  r = parse_client_transport_line(options,
-      "transport_1 socks5 1.2.3.4:567", 1);
+  r = parse_transport_line(options,
+      "transport_1 socks5 1.2.3.4:567", 1, 0);
   test_assert(r == 0);
   /* Proxy case for server transport */
-  r = parse_server_transport_line(options,
-      "transport_1 proxy 1.2.3.4:567", 1);
+  r = parse_transport_line(options,
+      "transport_1 proxy 1.2.3.4:567", 1, 1);
   test_assert(r == 0);
   /* Multiple-transport error exit */
-  r = parse_client_transport_line(options,
-      "transport_1,transport_2 socks5 1.2.3.4:567", 1);
+  r = parse_transport_line(options,
+      "transport_1,transport_2 socks5 1.2.3.4:567", 1, 0);
   test_assert(r < 0);
-  r = parse_server_transport_line(options,
-      "transport_1,transport_2 proxy 1.2.3.4:567", 1);
+  r = parse_transport_line(options,
+      "transport_1,transport_2 proxy 1.2.3.4:567", 1, 1);
   /* No port error exit */
-  r = parse_client_transport_line(options,
-      "transport_1 socks5 1.2.3.4", 1);
+  r = parse_transport_line(options,
+      "transport_1 socks5 1.2.3.4", 1, 0);
   test_assert(r < 0);
-  r = parse_server_transport_line(options,
-     "transport_1 proxy 1.2.3.4", 1);
+  r = parse_transport_line(options,
+     "transport_1 proxy 1.2.3.4", 1, 1);
   test_assert(r < 0);
   /* Unparsable address error exit */
-  r = parse_client_transport_line(options,
-      "transport_1 socks5 1.2.3:6x7", 1);
+  r = parse_transport_line(options,
+      "transport_1 socks5 1.2.3:6x7", 1, 0);
   test_assert(r < 0);
-  r = parse_server_transport_line(options,
-      "transport_1 proxy 1.2.3:6x7", 1);
+  r = parse_transport_line(options,
+      "transport_1 proxy 1.2.3:6x7", 1, 1);
   test_assert(r < 0);
 
   /* "Strange {Client|Server}TransportPlugin field" error exit */
-  r = parse_client_transport_line(options,
-      "transport_1 foo bar", 1);
+  r = parse_transport_line(options,
+      "transport_1 foo bar", 1, 0);
   test_assert(r < 0);
-  r = parse_server_transport_line(options,
-      "transport_1 foo bar", 1);
+  r = parse_transport_line(options,
+      "transport_1 foo bar", 1, 1);
   test_assert(r < 0);
 
   /* No sandbox mode error exit */
   tmp = options->Sandbox;
   options->Sandbox = 1;
-  r = parse_client_transport_line(options,
-      "transport_1 exec /usr/bin/fake-transport", 1);
+  r = parse_transport_line(options,
+      "transport_1 exec /usr/bin/fake-transport", 1, 0);
   test_assert(r < 0);
-  r = parse_server_transport_line(options,
-      "transport_1 exec /usr/bin/fake-transport", 1);
+  r = parse_transport_line(options,
+      "transport_1 exec /usr/bin/fake-transport", 1, 1);
   test_assert(r < 0);
   options->Sandbox = tmp;
 
@@ -696,16 +696,16 @@ test_config_parse_transport_plugin_line(void *arg)
   MOCK(pt_kickstart_proxy, pt_kickstart_proxy_mock);
   old_pt_kickstart_proxy_mock_call_count =
     pt_kickstart_proxy_mock_call_count;
-  r = parse_server_transport_line(options,
-      "transport_1 exec /usr/bin/fake-transport", 0);
+  r = parse_transport_line(options,
+      "transport_1 exec /usr/bin/fake-transport", 0, 1);
   test_assert(r == 0);
   test_assert(pt_kickstart_proxy_mock_call_count ==
       old_pt_kickstart_proxy_mock_call_count + 1);
   UNMOCK(pt_kickstart_proxy);
 
   /* This one hits a log line in the !validate_only case only */
-  r = parse_server_transport_line(options,
-      "transport_1 proxy 1.2.3.4:567", 0);
+  r = parse_transport_line(options,
+      "transport_1 proxy 1.2.3.4:567", 0, 1);
   test_assert(r == 0);
 
   /* Check mocked client transport cases */
@@ -721,8 +721,8 @@ test_config_parse_transport_plugin_line(void *arg)
     transport_add_from_config_mock_call_count;
   old_transport_is_needed_mock_call_count =
     transport_is_needed_mock_call_count;
-  r = parse_client_transport_line(options,
-      "transport_1 exec /usr/bin/fake-transport", 0);
+  r = parse_transport_line(options,
+      "transport_1 exec /usr/bin/fake-transport", 0, 0);
   /* Should have succeeded */
   test_assert(r == 0);
   /* transport_is_needed() should have been called */
@@ -745,8 +745,8 @@ test_config_parse_transport_plugin_line(void *arg)
     transport_add_from_config_mock_call_count;
   old_transport_is_needed_mock_call_count =
     transport_is_needed_mock_call_count;
-  r = parse_client_transport_line(options,
-      "transport_1 exec /usr/bin/fake-transport", 0);
+  r = parse_transport_line(options,
+      "transport_1 exec /usr/bin/fake-transport", 0, 0);
   /* Should have succeeded */
   test_assert(r == 0);
   /*
@@ -769,8 +769,8 @@ test_config_parse_transport_plugin_line(void *arg)
     transport_add_from_config_mock_call_count;
   old_transport_is_needed_mock_call_count =
     transport_is_needed_mock_call_count;
-  r = parse_client_transport_line(options,
-      "transport_1 socks5 1.2.3.4:567", 0);
+  r = parse_transport_line(options,
+      "transport_1 socks5 1.2.3.4:567", 0, 0);
   /* Should have succeeded */
   test_assert(r == 0);
   /*