commit a10e4d737e687fc848edbf27f2bada199545f77b Author: Nick Mathewson nickm@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);