October 2019 - tor-commits - lists.torproject.org

[tor/master] test_options: use testing options with (almost) real defaults.
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit a10e4d737e687fc848edbf27f2bada199545f77b Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 21 14:00:06 2019 -0400 test_options: use testing options with (almost) real defaults. Previously, we just used options set to all 0s, but this causes some pretty severe workarounds throughout the code, as our options fail to validate, or count as "default" for the wrong reasons. Note that in some places, we stop getting spurious warnings or failures which the tests previously demanded. In these cases, I've changed the test behavior. Fixes 32175. --- src/test/test_options.c | 52 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/src/test/test_options.c b/src/test/test_options.c index 69407a999..1506e2f8a 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -391,11 +391,7 @@ fixed_get_uname(void) } #define TEST_OPTIONS_OLD_VALUES "TestingV3AuthInitialVotingInterval 1800\n" \ - "ClientBootstrapConsensusMaxInProgressTries 3\n" \ - "TestingV3AuthInitialVoteDelay 300\n" \ - "TestingV3AuthInitialDistDelay 300\n" \ - "TestingClientMaxIntervalWithoutRequest 600\n" \ - "TestingDirConnectionMaxStall 600\n" \ + "ClientBootstrapConsensusMaxInProgressTries 3\n" #define TEST_OPTIONS_DEFAULT_VALUES TEST_OPTIONS_OLD_VALUES \ "MaxClientCircuitsPending 1\n" \ @@ -432,12 +428,17 @@ get_options_test_data(const char *conf) result->old_opt = options_new(); result->def_opt = options_new(); - // XXX: Really, all of these options should be set to defaults - // with options_init(), but about a dozen tests break when I do that. - // Being kinda lame and just fixing the immedate breakage for now.. - result->opt->ConnectionPadding = -1; // default must be "auto" - result->opt->DormantClientTimeout = 1800; // must be over 600. - result->opt->CircuitPadding = 1; // default must be "1" + options_init(result->opt); + options_init(result->old_opt); + options_init(result->def_opt); + + /* Various of the tests below expect that unless explicitly set, these + * options will be invalid. We override them for that reason. + * + * Later in this branch, I will remove these one by one. */ + result->opt->ConnLimit = 0; + result->opt->MaxClientCircuitsPending = 0; + result->opt->KeepalivePeriod = 0; rv = config_get_lines(conf, &cl, 1); tt_int_op(rv, OP_EQ, 0); @@ -1589,10 +1590,11 @@ test_options_validate__reachable_addresses(void *ignored) options_test_data_t *tdata = get_options_test_data( "FascistFirewall 1\n" "MaxClientCircuitsPending 1\n" + "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); expect_log_msg("Converting FascistFirewall config " "option to new format: \"ReachableDirAddresses *:80\"\n"); tt_str_op(tdata->opt->ReachableDirAddresses->value, OP_EQ, "*:80"); @@ -1607,11 +1609,16 @@ test_options_validate__reachable_addresses(void *ignored) "ReachableDirAddresses *:81\n" "ReachableORAddresses *:444\n" "MaxClientCircuitsPending 1\n" + "KeepalivePeriod 1\n" "ConnLimit 1\n"); tdata->opt->FirewallPorts = smartlist_new(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); +#if 0 + /* This does not actually produce any logs, and did not produce any relevant + * logs before. */ expect_log_entry(); +#endif tt_str_op(tdata->opt->ReachableDirAddresses->value, OP_EQ, "*:81"); tt_str_op(tdata->opt->ReachableORAddresses->value, OP_EQ, "*:444"); tor_free(msg); @@ -1621,10 +1628,11 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("FascistFirewall 1\n" "FirewallPort 123\n" "MaxClientCircuitsPending 1\n" + "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); expect_log_msg("Converting FascistFirewall and " "FirewallPorts config options to new format: " "\"ReachableAddresses *:123\"\n"); @@ -1638,11 +1646,16 @@ test_options_validate__reachable_addresses(void *ignored) "ReachableAddresses *:83\n" "ReachableAddresses reject *:*\n" "MaxClientCircuitsPending 1\n" + "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); +#if 0 + /* This does not actually produce any logs, and did not produce any relevant + * logs before. */ expect_log_entry(); +#endif tt_str_op(tdata->opt->ReachableAddresses->value, OP_EQ, "*:82"); tor_free(msg); @@ -1651,10 +1664,11 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("FascistFirewall 1\n" "ReachableAddresses *:82\n" "MaxClientCircuitsPending 1\n" + "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_ptr_op(tdata->opt->ReachableAddresses->next, OP_EQ, NULL); tor_free(msg); @@ -1666,6 +1680,7 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("ReachableAddresses *:82\n" "ORPort 127.0.0.1:5555\n" "MaxClientCircuitsPending 1\n" + "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1677,6 +1692,7 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("ReachableORAddresses *:82\n" "ORPort 127.0.0.1:5555\n" "MaxClientCircuitsPending 1\n" + "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1688,6 +1704,7 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("ReachableDirAddresses *:82\n" "ORPort 127.0.0.1:5555\n" "MaxClientCircuitsPending 1\n" + "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1699,6 +1716,7 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("ClientUseIPv4 0\n" "ORPort 127.0.0.1:5555\n" "MaxClientCircuitsPending 1\n" + "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1913,6 +1931,7 @@ test_options_validate__safe_logging(void *ignored) int ret; char *msg; options_test_data_t *tdata = get_options_test_data( + "SafeLogging 0\n" "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); @@ -2420,6 +2439,7 @@ test_options_validate__circuits(void *ignored) free_options_test_data(tdata); mock_clean_saved_logs(); tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES + "LearnCircuitBuildTimeout 0\n" "CircuitBuildTimeout 1\n" ); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg);

1 0

[tor/master] test_options.c: remove weird usage of MaxClientCircuitsPending
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit b501cd907f76a5b137182efbe0efdc7453c65148 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 21 14:11:07 2019 -0400 test_options.c: remove weird usage of MaxClientCircuitsPending Several of our tests assumed that MaxClientCircuitsPending would be set to 0 by default, causing the default options not to be parseable. These tests had nothing to do with MaxClientCircuitsPending. --- src/test/test_options.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/test/test_options.c b/src/test/test_options.c index c80b8c279..13893f38c 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -391,7 +391,6 @@ fixed_get_uname(void) } #define TEST_OPTIONS_DEFAULT_VALUES \ - "MaxClientCircuitsPending 1\n" \ "ConnLimit 1\n" typedef struct { @@ -422,7 +421,6 @@ get_options_test_data(const char *conf) * * Later in this branch, I will remove these one by one. */ result->opt->ConnLimit = 0; - result->opt->MaxClientCircuitsPending = 0; rv = config_get_lines(conf, &cl, 1); tt_int_op(rv, OP_EQ, 0); @@ -1383,9 +1381,7 @@ test_options_validate__conn_limit(void *ignored) tdata = get_options_test_data("ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "MaxClientCircuitsPending must be between 1 and 1024, " - "but was set to 0"); + tt_int_op(ret, OP_EQ, 0); tor_free(msg); done: @@ -1412,7 +1408,7 @@ test_options_validate__paths_needed(void *ignored) "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_assert(tdata->opt->PathsNeededToBuildCircuits > 0.24 && tdata->opt->PathsNeededToBuildCircuits < 0.26); expect_log_msg("PathsNeededToBuildCircuits is too low. " @@ -1425,7 +1421,7 @@ test_options_validate__paths_needed(void *ignored) "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_assert(tdata->opt->PathsNeededToBuildCircuits > 0.94 && tdata->opt->PathsNeededToBuildCircuits < 0.96); expect_log_msg("PathsNeededToBuildCircuits is " @@ -1438,7 +1434,7 @@ test_options_validate__paths_needed(void *ignored) "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_assert(tdata->opt->PathsNeededToBuildCircuits > 0.90 && tdata->opt->PathsNeededToBuildCircuits < 0.92); expect_no_log_entry();

1 0

[tor/master] test_options.c: remove weird usage of KeepalivePeriod.
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 79cd5912287e77e0ce2cef404b77e0bb4922fadc Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 21 14:11:07 2019 -0400 test_options.c: remove weird usage of KeepalivePeriod. Several of our tests assumed that KeepalivePeriod would be set to 0 by default, causing the default options not to be parseable. These tests had nothing to do with KeepalivePeriod. --- src/test/test_options.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/test/test_options.c b/src/test/test_options.c index 9169ff914..6baebb3fa 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -392,7 +392,6 @@ fixed_get_uname(void) #define TEST_OPTIONS_DEFAULT_VALUES \ "MaxClientCircuitsPending 1\n" \ - "KeepAlivePeriod 1\n" \ "ConnLimit 1\n" typedef struct { @@ -424,7 +423,6 @@ get_options_test_data(const char *conf) * Later in this branch, I will remove these one by one. */ result->opt->ConnLimit = 0; result->opt->MaxClientCircuitsPending = 0; - result->opt->KeepalivePeriod = 0; rv = config_get_lines(conf, &cl, 1); tt_int_op(rv, OP_EQ, 0); @@ -1487,8 +1485,7 @@ test_options_validate__max_client_circuits(void *ignored) "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "KeepalivePeriod option must be positive."); + tt_int_op(ret, OP_EQ, 0); tor_free(msg); done: @@ -1557,8 +1554,7 @@ test_options_validate__ports(void *ignored) "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "KeepalivePeriod option must be positive."); + tt_int_op(ret, OP_EQ, 0); tor_free(msg); done: @@ -1900,8 +1896,7 @@ test_options_validate__entry_nodes(void *ignored) "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "KeepalivePeriod option must be positive."); + tt_int_op(ret, OP_EQ, 0); tor_free(msg); done: @@ -1922,7 +1917,7 @@ test_options_validate__safe_logging(void *ignored) "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_int_op(tdata->opt->SafeLogging_, OP_EQ, SAFELOG_SCRUB_NONE); tor_free(msg); @@ -1932,7 +1927,7 @@ test_options_validate__safe_logging(void *ignored) "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_int_op(tdata->opt->SafeLogging_, OP_EQ, SAFELOG_SCRUB_NONE); tor_free(msg); @@ -1942,7 +1937,7 @@ test_options_validate__safe_logging(void *ignored) "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_int_op(tdata->opt->SafeLogging_, OP_EQ, SAFELOG_SCRUB_RELAY); tor_free(msg); @@ -1952,7 +1947,7 @@ test_options_validate__safe_logging(void *ignored) "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_int_op(tdata->opt->SafeLogging_, OP_EQ, SAFELOG_SCRUB_ALL); tor_free(msg);

1 0

[tor/master] Stop overriding KeepalivePeriod in tests.
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit b6efe37661020df8895467679c80aad6c3b2eb96 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 21 14:14:49 2019 -0400 Stop overriding KeepalivePeriod in tests. Now that KeepalivePeriod is set to the default value in the testing helper functions, the individual tests don't all need to make sure it is set to something valid. --- src/test/test_options.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/test/test_options.c b/src/test/test_options.c index 6baebb3fa..c80b8c279 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -1572,7 +1572,6 @@ test_options_validate__reachable_addresses(void *ignored) options_test_data_t *tdata = get_options_test_data( "FascistFirewall 1\n" "MaxClientCircuitsPending 1\n" - "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1591,7 +1590,6 @@ test_options_validate__reachable_addresses(void *ignored) "ReachableDirAddresses *:81\n" "ReachableORAddresses *:444\n" "MaxClientCircuitsPending 1\n" - "KeepalivePeriod 1\n" "ConnLimit 1\n"); tdata->opt->FirewallPorts = smartlist_new(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1610,7 +1608,6 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("FascistFirewall 1\n" "FirewallPort 123\n" "MaxClientCircuitsPending 1\n" - "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1628,7 +1625,6 @@ test_options_validate__reachable_addresses(void *ignored) "ReachableAddresses *:83\n" "ReachableAddresses reject *:*\n" "MaxClientCircuitsPending 1\n" - "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1646,7 +1642,6 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("FascistFirewall 1\n" "ReachableAddresses *:82\n" "MaxClientCircuitsPending 1\n" - "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1662,7 +1657,6 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("ReachableAddresses *:82\n" "ORPort 127.0.0.1:5555\n" "MaxClientCircuitsPending 1\n" - "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1674,7 +1668,6 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("ReachableORAddresses *:82\n" "ORPort 127.0.0.1:5555\n" "MaxClientCircuitsPending 1\n" - "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1686,7 +1679,6 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("ReachableDirAddresses *:82\n" "ORPort 127.0.0.1:5555\n" "MaxClientCircuitsPending 1\n" - "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1698,7 +1690,6 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("ClientUseIPv4 0\n" "ORPort 127.0.0.1:5555\n" "MaxClientCircuitsPending 1\n" - "KeepalivePeriod 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg);

1 0

[tor/master] test_options.c: remove weird usage of ConnLimit
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 56dddcbbcc08f7b3647c62c630a480f43ca249a5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 21 14:11:07 2019 -0400 test_options.c: remove weird usage of ConnLimit Several of our tests assumed that ConnLimit would be set to 0 by default, causing the default options not to be parseable. These tests had nothing to do with ConnLimit. --- src/test/test_options.c | 60 +++++++++++++++++++------------------------------ 1 file changed, 23 insertions(+), 37 deletions(-) diff --git a/src/test/test_options.c b/src/test/test_options.c index b72713c6a..3066b6db9 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -391,7 +391,7 @@ fixed_get_uname(void) } #define TEST_OPTIONS_DEFAULT_VALUES \ - "ConnLimit 1\n" + "" typedef struct { or_options_t *old_opt; @@ -416,12 +416,6 @@ get_options_test_data(const char *conf) options_init(result->old_opt); options_init(result->def_opt); - /* Various of the tests below expect that unless explicitly set, these - * options will be invalid. We override them for that reason. - * - * Later in this branch, I will remove these one by one. */ - result->opt->ConnLimit = 0; - rv = config_get_lines(conf, &cl, 1); tt_int_op(rv, OP_EQ, 0); rv = config_assign(get_options_mgr(), result->opt, cl, 0, &msg); @@ -596,15 +590,13 @@ test_options_validate__nickname(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("Nickname AMoreValidNick"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "ConnLimit must be greater than 0, but was set to 0"); + tt_int_op(ret, OP_EQ, 0); tor_free(msg); free_options_test_data(tdata); tdata = get_options_test_data("DataDirectory /tmp/somewhere"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "ConnLimit must be greater than 0, but was set to 0"); + tt_int_op(ret, OP_EQ, 0); done: free_options_test_data(tdata); @@ -623,7 +615,7 @@ test_options_validate__contactinfo(void *ignored) tdata->opt->ContactInfo = NULL; ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); expect_log_msg( "Your ContactInfo config option is not" " set. Please consider setting it, so we can contact you if your" @@ -635,7 +627,7 @@ test_options_validate__contactinfo(void *ignored) "ContactInfo hella(a)example.org"); mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); expect_no_log_msg( "Your ContactInfo config option is not" " set. Please consider setting it, so we can contact you if your" @@ -664,7 +656,7 @@ test_options_validate__logs(void *ignored) tt_str_op(tdata->opt->Logs->key, OP_EQ, "Log"); tt_str_op(tdata->opt->Logs->value, OP_EQ, "notice stdout"); tor_free(msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); free_options_test_data(tdata); tdata = get_options_test_data(""); @@ -675,7 +667,7 @@ test_options_validate__logs(void *ignored) tt_str_op(tdata->opt->Logs->key, OP_EQ, "Log"); tt_str_op(tdata->opt->Logs->value, OP_EQ, "warn stdout"); tor_free(msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); free_options_test_data(tdata); tdata = get_options_test_data(""); @@ -685,7 +677,7 @@ test_options_validate__logs(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_assert(!tdata->opt->Logs); tor_free(msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); free_options_test_data(tdata); tdata = get_options_test_data(""); @@ -694,7 +686,7 @@ test_options_validate__logs(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 1, &msg); tt_assert(!tdata->opt->Logs); tor_free(msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); free_options_test_data(tdata); tdata = get_options_test_data(""); @@ -703,7 +695,7 @@ test_options_validate__logs(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_assert(!tdata->opt->Logs); tor_free(msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); free_options_test_data(tdata); tdata = get_options_test_data(""); @@ -1063,7 +1055,7 @@ test_options_validate__transproxy(void *ignored) tdata = get_options_test_data("TransProxyType default\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_int_op(tdata->opt->TransProxyType_parsed, OP_EQ, TPT_DEFAULT); tor_free(msg); @@ -1130,28 +1122,24 @@ test_options_validate__transproxy(void *ignored) tdata = get_options_test_data("TransProxyType tproxy\n" "TransPort 127.0.0.1:123\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "ConnLimit must be greater than 0, but was set to 0"); + tt_int_op(ret, OP_EQ, 0); #elif defined(KERNEL_MAY_SUPPORT_IPFW) tdata = get_options_test_data("TransProxyType ipfw\n" "TransPort 127.0.0.1:123\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "ConnLimit must be greater than 0, but was set to 0"); + tt_int_op(ret, OP_EQ, 0); tor_free(msg); #elif defined(OpenBSD) tdata = get_options_test_data("TransProxyType pf-divert\n" "TransPort 127.0.0.1:123\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "ConnLimit must be greater than 0, but was set to 0"); + tt_int_op(ret, OP_EQ, 0); tor_free(msg); #elif defined(__NetBSD__) tdata = get_options_test_data("TransProxyType default\n" "TransPort 127.0.0.1:123\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "ConnLimit must be greater than 0, but was set to 0"); + tt_int_op(ret, OP_EQ, 0); tor_free(msg); #endif /* defined(__linux__) || ... */ @@ -1197,7 +1185,7 @@ test_options_validate__exclude_nodes(void *ignored) "ExcludeExitNodes {us}\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_int_op(smartlist_len(tdata->opt->ExcludeExitNodesUnion_->list), OP_EQ, 1); tt_str_op((char *) (smartlist_get(tdata->opt->ExcludeExitNodesUnion_->list, 0)), @@ -1207,7 +1195,7 @@ test_options_validate__exclude_nodes(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("ExcludeNodes {cn}\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_int_op(smartlist_len(tdata->opt->ExcludeExitNodesUnion_->list), OP_EQ, 1); tt_str_op((char *) (smartlist_get(tdata->opt->ExcludeExitNodesUnion_->list, 0)), @@ -1218,7 +1206,7 @@ test_options_validate__exclude_nodes(void *ignored) tdata = get_options_test_data("ExcludeNodes {cn}\n" "ExcludeExitNodes {us} {cn}\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_int_op(smartlist_len(tdata->opt->ExcludeExitNodesUnion_->list), OP_EQ, 2); tt_str_op((char *) (smartlist_get(tdata->opt->ExcludeExitNodesUnion_->list, 0)), @@ -1233,7 +1221,7 @@ test_options_validate__exclude_nodes(void *ignored) "StrictNodes 1\n"); mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); expect_log_msg( "You have asked to exclude certain relays from all positions " "in your circuits. Expect hidden services and other Tor " @@ -1244,7 +1232,7 @@ test_options_validate__exclude_nodes(void *ignored) tdata = get_options_test_data("ExcludeNodes {cn}\n"); mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); expect_no_log_msg( "You have asked to exclude certain relays from all positions " "in your circuits. Expect hidden services and other Tor " @@ -1269,7 +1257,7 @@ test_options_validate__node_families(void *ignored) "NodeFamily somewhere\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_assert(tdata->opt->NodeFamilySets); tt_int_op(smartlist_len(tdata->opt->NodeFamilySets), OP_EQ, 2); tt_str_op((char *)(smartlist_get( @@ -1287,7 +1275,7 @@ test_options_validate__node_families(void *ignored) tdata = get_options_test_data(""); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); + tt_int_op(ret, OP_EQ, 0); tt_assert(!tdata->opt->NodeFamilySets); tor_free(msg); @@ -1353,9 +1341,7 @@ test_options_validate__fetch_dir(void *ignored) "FetchDirInfoEarly 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_NE, "FetchDirInfoExtraEarly requires that you" - " also set FetchDirInfoEarly"); + tt_int_op(ret, OP_EQ, 0); tor_free(msg); done:

1 0

[tor/master] Fix up some Windows CI issues.
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 5e2ffd6fd74aa27dee061199150e0625639f2240 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 21 15:31:56 2019 -0400 Fix up some Windows CI issues. --- src/test/test_options.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/test/test_options.c b/src/test/test_options.c index 11a7ecfd3..6ea037d50 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -498,7 +498,7 @@ test_options_validate__uname_for_server(void *ignored) fixed_get_uname_result = "Windows 2000"; mock_clean_saved_logs(); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - expect_no_log_entry(); + expect_no_log_msg("Tor is running as a server, but you "); tor_free(msg); done: @@ -692,7 +692,12 @@ test_options_validate__logs(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_assert(!tdata->opt->Logs); tor_free(msg); +#ifdef _WIN32 + /* Can't RunAsDaemon on Windows. */ + tt_int_op(ret, OP_EQ, -1); +#else tt_int_op(ret, OP_EQ, 0); +#endif free_options_test_data(tdata); tdata = get_options_test_data(""); @@ -1417,7 +1422,7 @@ test_options_validate__paths_needed(void *ignored) tt_int_op(ret, OP_EQ, 0); tt_assert(tdata->opt->PathsNeededToBuildCircuits > 0.90 && tdata->opt->PathsNeededToBuildCircuits < 0.92); - expect_no_log_entry(); + expect_no_log_msg_containing("PathsNeededToBuildCircuits"); tor_free(msg); done:

1 0

[tor/master] Changes file for 32175 (test_options.c cleanup)
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit b064e0420ab363a5cd5832bac5e39dd1992c2b3c Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 21 14:46:04 2019 -0400 Changes file for 32175 (test_options.c cleanup) --- changes/bug32175 | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/changes/bug32175 b/changes/bug32175 new file mode 100644 index 000000000..ac78dcf34 --- /dev/null +++ b/changes/bug32175 @@ -0,0 +1,6 @@ + o Minor bugfixes (tests): + - Our option-validation tests no longer depend on specially configured + non-default, non-passing set of options. Previously, the tests had + been written to assume that options would _not_ be set to their + defaults, which led to needless complexity and verbosity. + Fixes bug 32175; bugfix on 0.2.8.1-alpha.

1 0

[tor/master] Stop overriding MaxClientCircuitsPending in tests.
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 78bf01166386732273e817839327be53a84b26df Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 21 14:14:49 2019 -0400 Stop overriding MaxClientCircuitsPending in tests. Now that MaxClientCircuitsPending is set to the default value in the testing helper functions, the individual tests don't all need to make sure it is set to something valid. --- src/test/test_options.c | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/src/test/test_options.c b/src/test/test_options.c index 13893f38c..b72713c6a 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -1497,7 +1497,6 @@ test_options_validate__ports(void *ignored) char *msg; options_test_data_t *tdata = get_options_test_data( "FirewallPorts 65537\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1508,7 +1507,6 @@ test_options_validate__ports(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("FirewallPorts 1\n" "LongLivedPorts 124444\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1520,7 +1518,6 @@ test_options_validate__ports(void *ignored) tdata = get_options_test_data("FirewallPorts 1\n" "LongLivedPorts 2\n" "RejectPlaintextPorts 112233\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1533,7 +1530,6 @@ test_options_validate__ports(void *ignored) "LongLivedPorts 2\n" "RejectPlaintextPorts 3\n" "WarnPlaintextPorts 65536\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1546,7 +1542,6 @@ test_options_validate__ports(void *ignored) "LongLivedPorts 2\n" "RejectPlaintextPorts 3\n" "WarnPlaintextPorts 4\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1567,7 +1562,6 @@ test_options_validate__reachable_addresses(void *ignored) setup_capture_of_logs(LOG_NOTICE); options_test_data_t *tdata = get_options_test_data( "FascistFirewall 1\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1585,7 +1579,6 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("FascistFirewall 1\n" "ReachableDirAddresses *:81\n" "ReachableORAddresses *:444\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); tdata->opt->FirewallPorts = smartlist_new(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1603,7 +1596,6 @@ test_options_validate__reachable_addresses(void *ignored) mock_clean_saved_logs(); tdata = get_options_test_data("FascistFirewall 1\n" "FirewallPort 123\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1620,7 +1612,6 @@ test_options_validate__reachable_addresses(void *ignored) "ReachableAddresses *:82\n" "ReachableAddresses *:83\n" "ReachableAddresses reject *:*\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1637,7 +1628,6 @@ test_options_validate__reachable_addresses(void *ignored) mock_clean_saved_logs(); tdata = get_options_test_data("FascistFirewall 1\n" "ReachableAddresses *:82\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1652,7 +1642,6 @@ test_options_validate__reachable_addresses(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("ReachableAddresses *:82\n" "ORPort 127.0.0.1:5555\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1663,7 +1652,6 @@ test_options_validate__reachable_addresses(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("ReachableORAddresses *:82\n" "ORPort 127.0.0.1:5555\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1674,7 +1662,6 @@ test_options_validate__reachable_addresses(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("ReachableDirAddresses *:82\n" "ORPort 127.0.0.1:5555\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1685,7 +1672,6 @@ test_options_validate__reachable_addresses(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("ClientUseIPv4 0\n" "ORPort 127.0.0.1:5555\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1783,7 +1769,6 @@ test_options_validate__use_bridges(void *ignored) "UseBridges 1\n" "ClientUseIPv4 1\n" "ORPort 127.0.0.1:5555\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1794,7 +1779,6 @@ test_options_validate__use_bridges(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("UseBridges 1\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1807,7 +1791,6 @@ test_options_validate__use_bridges(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("UseBridges 1\n" "EntryNodes {cn}\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1867,7 +1850,6 @@ test_options_validate__entry_nodes(void *ignored) options_test_data_t *tdata = get_options_test_data( "EntryNodes {cn}\n" "UseEntryGuards 0\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1879,7 +1861,6 @@ test_options_validate__entry_nodes(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("EntryNodes {cn}\n" "UseEntryGuards 1\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1900,7 +1881,6 @@ test_options_validate__safe_logging(void *ignored) char *msg; options_test_data_t *tdata = get_options_test_data( "SafeLogging 0\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1910,7 +1890,6 @@ test_options_validate__safe_logging(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("SafeLogging 0\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1920,7 +1899,6 @@ test_options_validate__safe_logging(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("SafeLogging Relay\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1930,7 +1908,6 @@ test_options_validate__safe_logging(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("SafeLogging 1\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); @@ -1940,7 +1917,6 @@ test_options_validate__safe_logging(void *ignored) free_options_test_data(tdata); tdata = get_options_test_data("SafeLogging stuffy\n" - "MaxClientCircuitsPending 1\n" "ConnLimit 1\n"); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg);

1 0

[tor/master] Fix a memory leak introduced by the changes to test_options.c
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit cb13930f2bc21c09bc0e5a4a01487a4a9f82707f Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 21 18:28:53 2019 -0400 Fix a memory leak introduced by the changes to test_options.c Since this code passes the same options to options_validate() more than once, options_validate() needs to be prepared for that. (This previously worked by accident, since the smartlist of schedulers wasn't initialized.) --- src/app/config/config.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/app/config/config.c b/src/app/config/config.c index c7ae453d5..a202454e2 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -3330,6 +3330,10 @@ options_validate_scheduler(or_options_t *options, char **msg) "can be used or set at least one value."); } /* Ok, we do have scheduler types, validate them. */ + if (options->SchedulerTypes_) { + SMARTLIST_FOREACH(options->SchedulerTypes_, int *, iptr, tor_free(iptr)); + smartlist_free(options->SchedulerTypes_); + } options->SchedulerTypes_ = smartlist_new(); SMARTLIST_FOREACH_BEGIN(options->Schedulers, const char *, type) { int *sched_type;

1 0

[tor/master] Fix a memory leak introduced by changes to test_options.c
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit bd518b9ab1815a28cbc43a03924182bbca2c4a12 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 21 18:28:08 2019 -0400 Fix a memory leak introduced by changes to test_options.c Since the FirewallPorts smartlist is now initialized, we can't just overwrite it. --- src/test/test_options.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/test_options.c b/src/test/test_options.c index 6ea037d50..1f5c85b40 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -1553,7 +1553,9 @@ test_options_validate__reachable_addresses(void *ignored) tdata = get_options_test_data("FascistFirewall 1\n" "ReachableDirAddresses *:81\n" "ReachableORAddresses *:444\n"); - tdata->opt->FirewallPorts = smartlist_new(); + tt_assert(tdata->opt->FirewallPorts); + SMARTLIST_FOREACH(tdata->opt->FirewallPorts, char *, cp, tor_free(cp)); + smartlist_clear(tdata->opt->FirewallPorts); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); #if 0

1 0