[tor-commits] [tor/master] Merge remote-tracking branch 'andrea/ticket6456'

nickm at torproject.org nickm at torproject.org
Tue Nov 4 14:56:57 UTC 2014


commit 9619c395ac7e2a8c2e57f55cd1885fe5f8e5f8b5
Merge: b1ca05d 2d4241d
Author: Nick Mathewson <nickm at torproject.org>
Date:   Tue Nov 4 09:49:35 2014 -0500

    Merge remote-tracking branch 'andrea/ticket6456'
    
    Somewhat tricky conflicts:
    	src/or/config.c
    
    Also, s/test_assert/tt_assert in test_config.c

 src/or/config.c        |  287 ++++++++++++++++--------------------------------
 src/or/config.h        |    3 +
 src/or/entrynodes.c    |    4 +-
 src/or/entrynodes.h    |    2 +-
 src/or/transports.c    |   12 +-
 src/or/transports.h    |   10 +-
 src/test/test_config.c |  248 +++++++++++++++++++++++++++++++++++++++++
 7 files changed, 361 insertions(+), 205 deletions(-)

diff --cc src/or/config.c
index 681955e,10b118c..cb27dac
--- a/src/or/config.c
+++ b/src/or/config.c
@@@ -1410,26 -1421,24 +1404,26 @@@ options_act(const or_options_t *old_opt
  
    mark_transport_list();
    pt_prepare_proxy_list_for_config_read();
 -  if (options->ClientTransportPlugin) {
 -    for (cl = options->ClientTransportPlugin; cl; cl = cl->next) {
 -      if (parse_transport_line(options, cl->value, 0, 0) < 0) {
 -        log_warn(LD_BUG,
 -                 "Previously validated ClientTransportPlugin line "
 -                 "could not be added!");
 -        return -1;
 +  if (!options->DisableNetwork) {
 +    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!");
 +          return -1;
 +        }
        }
      }
 -  }
  
 -  if (options->ServerTransportPlugin && server_mode(options)) {
 -    for (cl = options->ServerTransportPlugin; cl; cl = cl->next) {
 -      if (parse_transport_line(options, cl->value, 0, 1) < 0) {
 -        log_warn(LD_BUG,
 -                 "Previously validated ServerTransportPlugin line "
 -                 "could not be added!");
 -        return -1;
 +    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!");
 +          return -1;
 +        }
        }
      }
    }
@@@ -4844,40 -4846,49 +4852,57 @@@ parse_transport_line(const or_options_
      goto err;
    }
  
-   if (is_managed) { /* managed */
-     if (!validate_only && is_useless_proxy) {
-       log_info(LD_GENERAL, "Pluggable transport proxy (%s) does not provide "
-                "any needed transports and will not be launched.", line);
+   if (is_managed) {
+     /* managed */
+ 
+     if (!server && !validate_only && is_useless_proxy) {
 -      log_notice(LD_GENERAL,
 -                 "Pluggable transport proxy (%s) does not provide "
 -                 "any needed transports and will not be launched.",
 -                 line);
++      log_info(LD_GENERAL,
++               "Pluggable transport proxy (%s) does not provide "
++               "any needed transports and will not be launched.",
++               line);
      }
  
-     /* If we are not just validating, use the rest of the line as the
-        argv of the proxy to be launched. Also, make sure that we are
-        only launching proxies that contribute useful transports.  */
-     if (!validate_only && !is_useless_proxy) {
-       proxy_argc = line_length-2;
+     /*
+      * If we are not just validating, use the rest of the line as the
+      * argv of the proxy to be launched. Also, make sure that we are
+      * only launching proxies that contribute useful transports.
+      */
+ 
+     if (!validate_only && (server || !is_useless_proxy)) {
+       proxy_argc = line_length - 2;
        tor_assert(proxy_argc > 0);
 -      proxy_argv = tor_malloc_zero(sizeof(char *) * (proxy_argc + 1));
 +      proxy_argv = tor_calloc((proxy_argc + 1), sizeof(char *));
        tmp = proxy_argv;
-       for (i=0;i<proxy_argc;i++) { /* store arguments */
+ 
+       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*/
+       *tmp = NULL; /* terminated with NULL, just like execve() likes it */
  
        /* kickstart the thing */
-       pt_kickstart_client_proxy(transport_list, proxy_argv);
+       if (server) {
+         pt_kickstart_server_proxy(transport_list, proxy_argv);
+       } else {
+         pt_kickstart_client_proxy(transport_list, proxy_argv);
+       }
      }
-   } else { /* external */
+   } else {
+     /* external */
+ 
 +    /* ClientTransportPlugins connecting through a proxy is managed only. */
-     if (options->Socks4Proxy || options->Socks5Proxy || options->HTTPSProxy) {
++    if (!server &&
++        (options->Socks4Proxy || options->Socks5Proxy || options->HTTPSProxy)) {
 +      log_warn(LD_CONFIG, "You have configured an external proxy with another "
 +                          "proxy type. (Socks4Proxy|Socks5Proxy|HTTPSProxy)");
 +      goto err;
 +    }
 +
      if (smartlist_len(transport_list) != 1) {
-       log_warn(LD_CONFIG, "You can't have an external proxy with "
-                "more than one transports.");
+       log_warn(LD_CONFIG,
+                "You can't have an external proxy with more than "
+                "one transport.");
        goto err;
      }
  
diff --cc src/test/test_config.c
index 823d067,ae01fac..1ecb816
--- a/src/test/test_config.c
+++ b/src/test/test_config.c
@@@ -552,6 -554,251 +554,251 @@@ test_config_parse_transport_options_lin
    }
  }
  
+ /* Mocks needed for the transport plugin line test */
+ 
+ static void pt_kickstart_proxy_mock(const smartlist_t *transport_list,
+                                     char **proxy_argv, int is_server);
+ static int transport_add_from_config_mock(const tor_addr_t *addr,
+                                           uint16_t port, const char *name,
+                                           int socks_ver);
+ static int transport_is_needed_mock(const char *transport_name);
+ 
+ static int pt_kickstart_proxy_mock_call_count = 0;
+ static int transport_add_from_config_mock_call_count = 0;
+ static int transport_is_needed_mock_call_count = 0;
+ static int transport_is_needed_mock_return = 0;
+ 
+ static void
+ pt_kickstart_proxy_mock(const smartlist_t *transport_list,
+                         char **proxy_argv, int is_server)
+ {
+   ++pt_kickstart_proxy_mock_call_count;
+ }
+ 
+ static int
+ transport_add_from_config_mock(const tor_addr_t *addr,
+                                uint16_t port, const char *name,
+                                int socks_ver)
+ {
+   ++transport_add_from_config_mock_call_count;
+ 
+   return 0;
+ }
+ 
+ static int
+ transport_is_needed_mock(const char *transport_name)
+ {
+   ++transport_is_needed_mock_call_count;
+ 
+   return transport_is_needed_mock_return;
+ }
+ 
+ /**
+  * Test parsing for the ClientTransportPlugin and ServerTransportPlugin config
+  * options.
+  */
+ 
+ static void
+ test_config_parse_transport_plugin_line(void *arg)
+ {
+   or_options_t *options = get_options_mutable();
+   int r, tmp;
+   int old_pt_kickstart_proxy_mock_call_count;
+   int old_transport_add_from_config_mock_call_count;
+   int old_transport_is_needed_mock_call_count;
+ 
+   /* Bad transport lines - too short */
+   r = parse_transport_line(options, "bad", 1, 0);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+   r = parse_transport_line(options, "bad", 1, 1);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+   r = parse_transport_line(options, "bad bad", 1, 0);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+   r = parse_transport_line(options, "bad bad", 1, 1);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+ 
+   /* Test transport list parsing */
+   r = parse_transport_line(options,
+       "transport_1 exec /usr/bin/fake-transport", 1, 0);
 -  test_assert(r == 0);
++  tt_assert(r == 0);
+   r = parse_transport_line(options,
+    "transport_1 exec /usr/bin/fake-transport", 1, 1);
 -  test_assert(r == 0);
++  tt_assert(r == 0);
+   r = parse_transport_line(options,
+       "transport_1,transport_2 exec /usr/bin/fake-transport", 1, 0);
 -  test_assert(r == 0);
++  tt_assert(r == 0);
+   r = parse_transport_line(options,
+       "transport_1,transport_2 exec /usr/bin/fake-transport", 1, 1);
 -  test_assert(r == 0);
++  tt_assert(r == 0);
+   /* Bad transport identifiers */
+   r = parse_transport_line(options,
+       "transport_* exec /usr/bin/fake-transport", 1, 0);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+   r = parse_transport_line(options,
+       "transport_* exec /usr/bin/fake-transport", 1, 1);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+ 
+   /* Check SOCKS cases for client transport */
+   r = parse_transport_line(options,
+       "transport_1 socks4 1.2.3.4:567", 1, 0);
 -  test_assert(r == 0);
++  tt_assert(r == 0);
+   r = parse_transport_line(options,
+       "transport_1 socks5 1.2.3.4:567", 1, 0);
 -  test_assert(r == 0);
++  tt_assert(r == 0);
+   /* Proxy case for server transport */
+   r = parse_transport_line(options,
+       "transport_1 proxy 1.2.3.4:567", 1, 1);
 -  test_assert(r == 0);
++  tt_assert(r == 0);
+   /* Multiple-transport error exit */
+   r = parse_transport_line(options,
+       "transport_1,transport_2 socks5 1.2.3.4:567", 1, 0);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+   r = parse_transport_line(options,
+       "transport_1,transport_2 proxy 1.2.3.4:567", 1, 1);
+   /* No port error exit */
+   r = parse_transport_line(options,
+       "transport_1 socks5 1.2.3.4", 1, 0);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+   r = parse_transport_line(options,
+      "transport_1 proxy 1.2.3.4", 1, 1);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+   /* Unparsable address error exit */
+   r = parse_transport_line(options,
+       "transport_1 socks5 1.2.3:6x7", 1, 0);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+   r = parse_transport_line(options,
+       "transport_1 proxy 1.2.3:6x7", 1, 1);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+ 
+   /* "Strange {Client|Server}TransportPlugin field" error exit */
+   r = parse_transport_line(options,
+       "transport_1 foo bar", 1, 0);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+   r = parse_transport_line(options,
+       "transport_1 foo bar", 1, 1);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+ 
+   /* No sandbox mode error exit */
+   tmp = options->Sandbox;
+   options->Sandbox = 1;
+   r = parse_transport_line(options,
+       "transport_1 exec /usr/bin/fake-transport", 1, 0);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+   r = parse_transport_line(options,
+       "transport_1 exec /usr/bin/fake-transport", 1, 1);
 -  test_assert(r < 0);
++  tt_assert(r < 0);
+   options->Sandbox = tmp;
+ 
+   /*
+    * These final test cases cover code paths that only activate without
+    * validate_only, so they need mocks in place.
+    */
+   MOCK(pt_kickstart_proxy, pt_kickstart_proxy_mock);
+   old_pt_kickstart_proxy_mock_call_count =
+     pt_kickstart_proxy_mock_call_count;
+   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 ==
++  tt_assert(r == 0);
++  tt_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_transport_line(options,
+       "transport_1 proxy 1.2.3.4:567", 0, 1);
 -  test_assert(r == 0);
++  tt_assert(r == 0);
+ 
+   /* Check mocked client transport cases */
+   MOCK(pt_kickstart_proxy, pt_kickstart_proxy_mock);
+   MOCK(transport_add_from_config, transport_add_from_config_mock);
+   MOCK(transport_is_needed, transport_is_needed_mock);
+ 
+   /* Unnecessary transport case */
+   transport_is_needed_mock_return = 0;
+   old_pt_kickstart_proxy_mock_call_count =
+     pt_kickstart_proxy_mock_call_count;
+   old_transport_add_from_config_mock_call_count =
+     transport_add_from_config_mock_call_count;
+   old_transport_is_needed_mock_call_count =
+     transport_is_needed_mock_call_count;
+   r = parse_transport_line(options,
+       "transport_1 exec /usr/bin/fake-transport", 0, 0);
+   /* Should have succeeded */
 -  test_assert(r == 0);
++  tt_assert(r == 0);
+   /* transport_is_needed() should have been called */
 -  test_assert(transport_is_needed_mock_call_count ==
++  tt_assert(transport_is_needed_mock_call_count ==
+       old_transport_is_needed_mock_call_count + 1);
+   /*
+    * pt_kickstart_proxy() and transport_add_from_config() should
+    * not have been called.
+    */
 -  test_assert(pt_kickstart_proxy_mock_call_count ==
++  tt_assert(pt_kickstart_proxy_mock_call_count ==
+       old_pt_kickstart_proxy_mock_call_count);
 -  test_assert(transport_add_from_config_mock_call_count ==
++  tt_assert(transport_add_from_config_mock_call_count ==
+       old_transport_add_from_config_mock_call_count);
+ 
+   /* Necessary transport case */
+   transport_is_needed_mock_return = 1;
+   old_pt_kickstart_proxy_mock_call_count =
+     pt_kickstart_proxy_mock_call_count;
+   old_transport_add_from_config_mock_call_count =
+     transport_add_from_config_mock_call_count;
+   old_transport_is_needed_mock_call_count =
+     transport_is_needed_mock_call_count;
+   r = parse_transport_line(options,
+       "transport_1 exec /usr/bin/fake-transport", 0, 0);
+   /* Should have succeeded */
 -  test_assert(r == 0);
++  tt_assert(r == 0);
+   /*
+    * transport_is_needed() and pt_kickstart_proxy() should have been
+    * called.
+    */
 -  test_assert(pt_kickstart_proxy_mock_call_count ==
++  tt_assert(pt_kickstart_proxy_mock_call_count ==
+       old_pt_kickstart_proxy_mock_call_count + 1);
 -  test_assert(transport_is_needed_mock_call_count ==
++  tt_assert(transport_is_needed_mock_call_count ==
+       old_transport_is_needed_mock_call_count + 1);
+   /* transport_add_from_config() should not have been called. */
 -  test_assert(transport_add_from_config_mock_call_count ==
++  tt_assert(transport_add_from_config_mock_call_count ==
+       old_transport_add_from_config_mock_call_count);
+ 
+   /* proxy case */
+   transport_is_needed_mock_return = 1;
+   old_pt_kickstart_proxy_mock_call_count =
+     pt_kickstart_proxy_mock_call_count;
+   old_transport_add_from_config_mock_call_count =
+     transport_add_from_config_mock_call_count;
+   old_transport_is_needed_mock_call_count =
+     transport_is_needed_mock_call_count;
+   r = parse_transport_line(options,
+       "transport_1 socks5 1.2.3.4:567", 0, 0);
+   /* Should have succeeded */
 -  test_assert(r == 0);
++  tt_assert(r == 0);
+   /*
+    * transport_is_needed() and transport_add_from_config() should have
+    * been called.
+    */
 -  test_assert(transport_add_from_config_mock_call_count ==
++  tt_assert(transport_add_from_config_mock_call_count ==
+       old_transport_add_from_config_mock_call_count + 1);
 -  test_assert(transport_is_needed_mock_call_count ==
++  tt_assert(transport_is_needed_mock_call_count ==
+       old_transport_is_needed_mock_call_count + 1);
+   /* pt_kickstart_proxy() should not have been called. */
 -  test_assert(pt_kickstart_proxy_mock_call_count ==
++  tt_assert(pt_kickstart_proxy_mock_call_count ==
+       old_pt_kickstart_proxy_mock_call_count);
+ 
+   /* Done with mocked client transport cases */
+   UNMOCK(transport_is_needed);
+   UNMOCK(transport_add_from_config);
+   UNMOCK(pt_kickstart_proxy);
+ 
+  done:
+   /* Make sure we undo all mocks */
+   UNMOCK(pt_kickstart_proxy);
+   UNMOCK(transport_add_from_config);
+   UNMOCK(transport_is_needed);
+ 
+   return;
+ }
+ 
  // Tests if an options with MyFamily fingerprints missing '$' normalises
  // them correctly and also ensure it also works with multiple fingerprints
  static void





More information about the tor-commits mailing list