From 06ecb9432f4596f10e73f82bb1ff6677060756f9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 11 Apr 2017 15:50:06 -0400 Subject: conscache.c: do not match entries that are slated for removal. --- src/test/test_conscache.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src/test') diff --git a/src/test/test_conscache.c b/src/test/test_conscache.c index 12184f0cc6..c316411a79 100644 --- a/src/test/test_conscache.c +++ b/src/test/test_conscache.c @@ -200,8 +200,7 @@ test_conscache_cleanup(void *arg) tt_assert(e_tmp); tt_assert(consensus_cache_entry_is_mapped(e_tmp)); e_tmp = consensus_cache_find_first(cache, "index", "7"); - tt_assert(e_tmp); - tt_assert(consensus_cache_entry_is_mapped(e_tmp)); + tt_assert(e_tmp == NULL); // not found because pending deletion. /* Delete the pending-deletion items. */ consensus_cache_delete_pending(cache); @@ -210,12 +209,12 @@ test_conscache_cleanup(void *arg) consensus_cache_find_all(entries, cache, NULL, NULL); int n = smartlist_len(entries); smartlist_free(entries); - tt_int_op(n, OP_EQ, 20 - 1); /* 1 entry was deleted */ + tt_int_op(n, OP_EQ, 20 - 2); /* 1 entry was deleted; 1 is not-found. */ } e_tmp = consensus_cache_find_first(cache, "index", "7"); // refcnt == 1... tt_assert(e_tmp == NULL); // so deleted. e_tmp = consensus_cache_find_first(cache, "index", "14"); // refcnt == 2 - tt_assert(e_tmp); // so, not deleted. + tt_assert(e_tmp == NULL); // not deleted; but not found. /* Now do lazy unmapping. */ // should do nothing. -- cgit v1.2.3-54-g00ecf From 7fc37d41b47f106939cae559a6874fa46bca6150 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 14 Apr 2017 12:35:02 -0400 Subject: Unit tests for consdiffmgr module Initial tests. These just try adding a few consensuses, looking them up, and making sure that consensus diffs are generated in a more or less reasonable-looking way. It's enough for 87% coverage, but it leaves out a lot of functionality. --- src/or/consdiffmgr.c | 4 +- src/or/consdiffmgr.h | 5 + src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_consdiffmgr.c | 305 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 316 insertions(+), 1 deletion(-) create mode 100644 src/test/test_consdiffmgr.c (limited to 'src/test') diff --git a/src/or/consdiffmgr.c b/src/or/consdiffmgr.c index 7d26b0c68b..0920d08750 100644 --- a/src/or/consdiffmgr.c +++ b/src/or/consdiffmgr.c @@ -11,6 +11,8 @@ * and serve the diffs from those documents to the latest consensus. */ +#define CONSDIFFMGR_PRIVATE + #include "or.h" #include "conscache.h" #include "consdiff.h" @@ -122,7 +124,7 @@ cdm_labels_prepend_sha3(config_line_t **labels, * valid_after time in the cache. Return that consensus if it's * present, or NULL if it's missing. */ -static consensus_cache_entry_t * +STATIC consensus_cache_entry_t * cdm_cache_lookup_consensus(consensus_flavor_t flavor, time_t valid_after) { char formatted_time[ISO_TIME_LEN+1]; diff --git a/src/or/consdiffmgr.h b/src/or/consdiffmgr.h index 860e055bb5..474876e432 100644 --- a/src/or/consdiffmgr.h +++ b/src/or/consdiffmgr.h @@ -34,5 +34,10 @@ int consdiffmgr_cleanup(void); void consdiffmgr_configure(const consdiff_cfg_t *cfg); void consdiffmgr_free_all(void); +#ifdef CONSDIFFMGR_PRIVATE +STATIC consensus_cache_entry_t *cdm_cache_lookup_consensus( + consensus_flavor_t flavor, time_t valid_after); +#endif + #endif diff --git a/src/test/include.am b/src/test/include.am index c92eab13c9..1b16d0f1a2 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -89,6 +89,7 @@ src_test_test_SOURCES = \ src/test/test_connection.c \ src/test/test_conscache.c \ src/test/test_consdiff.c \ + src/test/test_consdiffmgr.c \ src/test/test_containers.c \ src/test/test_controller.c \ src/test/test_controller_events.c \ diff --git a/src/test/test.c b/src/test/test.c index 77ea44970c..90e3d02f93 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1197,6 +1197,7 @@ struct testgroup_t testgroups[] = { { "connection/", connection_tests }, { "conscache/", conscache_tests }, { "consdiff/", consdiff_tests }, + { "consdiffmgr/", consdiffmgr_tests }, { "container/", container_tests }, { "control/", controller_tests }, { "control/event/", controller_event_tests }, diff --git a/src/test/test.h b/src/test/test.h index 252a239afc..3d7d05e771 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -192,6 +192,7 @@ extern struct testcase_t config_tests[]; extern struct testcase_t connection_tests[]; extern struct testcase_t conscache_tests[]; extern struct testcase_t consdiff_tests[]; +extern struct testcase_t consdiffmgr_tests[]; extern struct testcase_t container_tests[]; extern struct testcase_t controller_tests[]; extern struct testcase_t controller_event_tests[]; diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c new file mode 100644 index 0000000000..e80a39e960 --- /dev/null +++ b/src/test/test_consdiffmgr.c @@ -0,0 +1,305 @@ +/* Copyright (c) 2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#define CONSDIFFMGR_PRIVATE + +#include "or.h" +#include "config.h" +#include "conscache.h" +#include "consdiff.h" +#include "consdiffmgr.h" +#include "cpuworker.h" +#include "networkstatus.h" +#include "workqueue.h" + +#include "test.h" +#include "log_test_helpers.h" + +// ============================== Setup/teardown the consdiffmgr +// These functions get run before/after each test in this module + +static void * +consdiffmgr_test_setup(const struct testcase_t *arg) +{ + (void)arg; + char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cdm")); + tor_free(get_options_mutable()->DataDirectory); + get_options_mutable()->DataDirectory = ddir_fname; // now owns the pointer. + check_private_dir(ddir_fname, CPD_CREATE, NULL); + + consdiff_cfg_t consdiff_cfg = { 7200, 300 }; + consdiffmgr_configure(&consdiff_cfg); + return (void *)1; // must return something non-null. +} +static int +consdiffmgr_test_teardown(const struct testcase_t *arg, void *ignore) +{ + (void)arg; + (void)ignore; + consdiffmgr_free_all(); + return 1; +} +static struct testcase_setup_t setup_diffmgr = { + consdiffmgr_test_setup, + consdiffmgr_test_teardown +}; + +// ============================== NS faking functions +// These functions are for making quick fake consensus objects and +// strings that are just good enough for consdiff and consdiffmgr. + +static networkstatus_t * +fake_ns_new(consensus_flavor_t flav, time_t valid_after) +{ + networkstatus_t *ns = tor_malloc_zero(sizeof(networkstatus_t)); + ns->type = NS_TYPE_CONSENSUS; + ns->flavor = flav; + ns->valid_after = valid_after; + return ns; +} + +static char * +fake_ns_body_new(consensus_flavor_t flav, time_t valid_after) +{ + const char *flavor_string = flav == FLAV_NS ? "" : " microdesc"; + char valid_after_string[ISO_TIME_LEN+1]; + + format_iso_time(valid_after_string, valid_after); + char *random_stuff = crypto_random_hostname(3, 25, "junk ", ""); + + char *consensus; + tor_asprintf(&consensus, + "network-status-version 3%s\n" + "vote-status consensus\n" + "valid-after %s\n" + "r name ccccccccccccccccc etc\nsample\n" + "r name eeeeeeeeeeeeeeeee etc\nbar\n" + "%s\n", + flavor_string, + valid_after_string, + random_stuff); + tor_free(random_stuff); + return consensus; +} + +// ============================== Cpuworker mocking code +// These mocking functions and types capture the cpuworker calls +// so we can inspect them and run them in the main thread. +static smartlist_t *fake_cpuworker_queue = NULL; +typedef struct fake_work_queue_ent_t { + enum workqueue_reply_t (*fn)(void *, void *); + void (*reply_fn)(void *); + void *arg; +} fake_work_queue_ent_t; +static struct workqueue_entry_s * +mock_cpuworker_queue_work(enum workqueue_reply_t (*fn)(void *, void *), + void (*reply_fn)(void *), + void *arg) +{ + if (! fake_cpuworker_queue) + fake_cpuworker_queue = smartlist_new(); + + fake_work_queue_ent_t *ent = tor_malloc_zero(sizeof(*ent)); + ent->fn = fn; + ent->reply_fn = reply_fn; + ent->arg = arg; + smartlist_add(fake_cpuworker_queue, ent); + return (struct workqueue_entry_s *)ent; +} +static int +mock_cpuworker_run_work(void) +{ + if (! fake_cpuworker_queue) + return 0; + SMARTLIST_FOREACH(fake_cpuworker_queue, fake_work_queue_ent_t *, ent, { + enum workqueue_reply_t r = ent->fn(NULL, ent->arg); + if (r != WQ_RPL_REPLY) + return -1; + }); + return 0; +} +static void +mock_cpuworker_handle_replies(void) +{ + if (! fake_cpuworker_queue) + return; + SMARTLIST_FOREACH(fake_cpuworker_queue, fake_work_queue_ent_t *, ent, { + ent->reply_fn(ent->arg); + }); + smartlist_free(fake_cpuworker_queue); + fake_cpuworker_queue = NULL; +} + +// ============================== Beginning of tests + +static void +test_consdiffmgr_add(void *arg) +{ + (void) arg; + time_t now = approx_time(); + + consensus_cache_entry_t *ent = NULL; + networkstatus_t *ns_tmp = fake_ns_new(FLAV_NS, now); + const char *dummy = "foo"; + int r = consdiffmgr_add_consensus(dummy, ns_tmp); + tt_int_op(r, OP_EQ, 0); + + /* If we add it again, it won't work */ + setup_capture_of_logs(LOG_INFO); + dummy = "bar"; + r = consdiffmgr_add_consensus(dummy, ns_tmp); + tt_int_op(r, OP_EQ, -1); + expect_single_log_msg_containing("We already have a copy of that " + "consensus"); + mock_clean_saved_logs(); + + /* But it will work fine if the flavor is different */ + dummy = "baz"; + ns_tmp->flavor = FLAV_MICRODESC; + r = consdiffmgr_add_consensus(dummy, ns_tmp); + tt_int_op(r, OP_EQ, 0); + + /* And it will work fine if the time is different */ + dummy = "quux"; + ns_tmp->flavor = FLAV_NS; + ns_tmp->valid_after = now - 60; + r = consdiffmgr_add_consensus(dummy, ns_tmp); + tt_int_op(r, OP_EQ, 0); + + /* If we add one a long long time ago, it will fail. */ + dummy = "xyzzy"; + ns_tmp->valid_after = 86400 * 100; /* A few months into 1970 */ + r = consdiffmgr_add_consensus(dummy, ns_tmp); + tt_int_op(r, OP_EQ, -1); + expect_single_log_msg_containing("it's too old."); + + /* Try looking up a consensuses. */ + ent = cdm_cache_lookup_consensus(FLAV_NS, now-60); + tt_assert(ent); + consensus_cache_entry_incref(ent); + size_t s; + const uint8_t *body; + r = consensus_cache_entry_get_body(ent, &body, &s); + tt_int_op(r, OP_EQ, 0); + tt_int_op(s, OP_EQ, 4); + tt_mem_op(body, OP_EQ, "quux", 4); + + /* Try looking up another entry, but fail */ + tt_assert(NULL == cdm_cache_lookup_consensus(FLAV_MICRODESC, now-60)); + tt_assert(NULL == cdm_cache_lookup_consensus(FLAV_NS, now-61)); + + done: + networkstatus_vote_free(ns_tmp); + teardown_capture_of_logs(); + consensus_cache_entry_decref(ent); +} + +static void +test_consdiffmgr_make_diffs(void *arg) +{ + (void)arg; + networkstatus_t *ns = NULL; + char *ns_body = NULL, *md_ns_body = NULL, *md_ns_body_2 = NULL; + char *applied = NULL, *diff_text = NULL; + time_t now = approx_time(); + int r; + consensus_cache_entry_t *diff = NULL; + uint8_t md_ns_sha3[DIGEST256_LEN]; + consdiff_status_t diff_status; + + MOCK(cpuworker_queue_work, mock_cpuworker_queue_work); + + // Try rescan with no consensuses: shouldn't crash or queue work. + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_EQ, fake_cpuworker_queue); + + // Make two consensuses, 1 hour sec ago. + ns = fake_ns_new(FLAV_NS, now-3600); + ns_body = fake_ns_body_new(FLAV_NS, now-3600); + r = consdiffmgr_add_consensus(ns_body, ns); + networkstatus_vote_free(ns); + tor_free(ns_body); + tt_int_op(r, OP_EQ, 0); + + ns = fake_ns_new(FLAV_MICRODESC, now-3600); + md_ns_body = fake_ns_body_new(FLAV_MICRODESC, now-3600); + r = consdiffmgr_add_consensus(md_ns_body, ns); + crypto_digest256((char*)md_ns_sha3, md_ns_body, strlen(md_ns_body), + DIGEST_SHA3_256); + networkstatus_vote_free(ns); + tt_int_op(r, OP_EQ, 0); + + // No diffs will be generated. + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_EQ, fake_cpuworker_queue); + + // Add a MD consensus from 45 minutes ago. This should cause one diff + // worth of work to get queued. + ns = fake_ns_new(FLAV_MICRODESC, now-45*60); + md_ns_body_2 = fake_ns_body_new(FLAV_MICRODESC, now-45*60); + r = consdiffmgr_add_consensus(md_ns_body_2, ns); + networkstatus_vote_free(ns); + tt_int_op(r, OP_EQ, 0); + + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_NE, fake_cpuworker_queue); + tt_int_op(1, OP_EQ, smartlist_len(fake_cpuworker_queue)); + // XXXX not working yet + /* + diff_status = consdiffmgr_find_diff_from(&diff, FLAV_MICRODESC, + DIGEST_SHA3_256, + md_ns_sha3, DIGEST256_LEN); + tt_int_op(CONSDIFF_IN_PROGRESS, OP_EQ, diff_status); + */ + + // Now run that process and get the diff. + r = mock_cpuworker_run_work(); + tt_int_op(r, OP_EQ, 0); + mock_cpuworker_handle_replies(); + + // At this point we should be able to get that diff. + diff_status = consdiffmgr_find_diff_from(&diff, FLAV_MICRODESC, + DIGEST_SHA3_256, + md_ns_sha3, DIGEST256_LEN); + tt_int_op(CONSDIFF_AVAILABLE, OP_EQ, diff_status); + tt_assert(diff); + + /* Make sure applying the diff actually works */ + const uint8_t *diff_body; + size_t diff_size; + r = consensus_cache_entry_get_body(diff, &diff_body, &diff_size); + tt_int_op(r, OP_EQ, 0); + diff_text = tor_memdup_nulterm(diff_body, diff_size); + applied = consensus_diff_apply(md_ns_body, diff_text); + tt_assert(applied); + tt_str_op(applied, OP_EQ, md_ns_body_2); + + /* Rescan again: no more work to do. */ + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_EQ, fake_cpuworker_queue); + + done: + tor_free(md_ns_body); + tor_free(md_ns_body_2); + tor_free(diff_text); + tor_free(applied); +} + +#define TEST(name) \ + { #name, test_consdiffmgr_ ## name , TT_FORK, &setup_diffmgr, NULL } + +struct testcase_t consdiffmgr_tests[] = { + TEST(add), + TEST(make_diffs), + // XXXX Test: deleting consensuses for being too old + // XXXX Test: deleting diffs for not being to most recent consensus + // XXXX Test: Objects of unrecognized doctype are not cleaned. + // XXXX Test: Objects with bad iso time are not cleaned. + // XXXX Test: only generate diffs to most recent consensus + // XXXX Test: making diffs from some old consensuses, but having diffs + // for others. + // XXXX Test: Failure to open cache??? + // XXXX Test: failure to create consensus diff. + END_OF_TESTCASES +}; + -- cgit v1.2.3-54-g00ecf From b9c2f135bd0d0bb9f0b1d40cda794ead912f75d0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 15 Apr 2017 09:58:03 -0400 Subject: Another consdiffmgr test: only generate the diffs that are needed This test makes sure that we only generate the diffs we actually want, rather than regenerating all the diffs every time anything changes. --- src/test/test_consdiffmgr.c | 74 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 3 deletions(-) (limited to 'src/test') diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index e80a39e960..f550482304 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -285,19 +285,87 @@ test_consdiffmgr_make_diffs(void *arg) tor_free(applied); } +static void +test_consdiffmgr_diff_rules(void *arg) +{ + (void)arg; +#define N 6 + char *md_body[N], *ns_body[N]; + networkstatus_t *md_ns[N], *ns_ns[N]; + uint8_t md_ns_sha3[N][DIGEST256_LEN], ns_ns_sha3[N][DIGEST256_LEN]; + int i; + + MOCK(cpuworker_queue_work, mock_cpuworker_queue_work); + + /* Create a bunch of consensus things at 15-second intervals. */ + time_t start = approx_time() - 120; + for (i = 0; i < N; ++i) { + time_t when = start + i * 15; + md_body[i] = fake_ns_body_new(FLAV_MICRODESC, when); + ns_body[i] = fake_ns_body_new(FLAV_NS, when); + md_ns[i] = fake_ns_new(FLAV_MICRODESC, when); + ns_ns[i] = fake_ns_new(FLAV_NS, when); + crypto_digest256((char *)md_ns_sha3[i], md_body[i], strlen(md_body[i]), + DIGEST_SHA3_256); + crypto_digest256((char *)ns_ns_sha3[i], ns_body[i], strlen(ns_body[i]), + DIGEST_SHA3_256); + } + + /* For the MD consensuses: add 4 of them, and make sure that + * diffs are created to one consensus (the most recent) only. */ + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[1], md_ns[1])); + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[2], md_ns[2])); + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[3], md_ns[3])); + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[4], md_ns[4])); + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_NE, fake_cpuworker_queue); + tt_int_op(3, OP_EQ, smartlist_len(fake_cpuworker_queue)); + tt_int_op(0, OP_EQ, mock_cpuworker_run_work()); + mock_cpuworker_handle_replies(); + tt_ptr_op(NULL, OP_EQ, fake_cpuworker_queue); + + /* For the NS consensuses: add 3, generate, and add one older one and + * make sure that older one is the only one whose diff is generated */ + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(ns_body[0], ns_ns[0])); + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(ns_body[1], ns_ns[1])); + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(ns_body[5], ns_ns[5])); + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_NE, fake_cpuworker_queue); + tt_int_op(2, OP_EQ, smartlist_len(fake_cpuworker_queue)); + tt_int_op(0, OP_EQ, mock_cpuworker_run_work()); + mock_cpuworker_handle_replies(); + + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(ns_body[2], ns_ns[2])); + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_NE, fake_cpuworker_queue); + tt_int_op(1, OP_EQ, smartlist_len(fake_cpuworker_queue)); + tt_int_op(0, OP_EQ, mock_cpuworker_run_work()); + mock_cpuworker_handle_replies(); + + done: + for (i = 0; i < N; ++i) { + tor_free(md_body[i]); + tor_free(ns_body[i]); + networkstatus_vote_free(md_ns[i]); + networkstatus_vote_free(ns_ns[i]); + } + UNMOCK(cpuworker_queue_work); +#undef N +} + #define TEST(name) \ { #name, test_consdiffmgr_ ## name , TT_FORK, &setup_diffmgr, NULL } struct testcase_t consdiffmgr_tests[] = { TEST(add), TEST(make_diffs), + TEST(diff_rules), + // XXXX Test: deleting consensuses for being too old // XXXX Test: deleting diffs for not being to most recent consensus // XXXX Test: Objects of unrecognized doctype are not cleaned. // XXXX Test: Objects with bad iso time are not cleaned. - // XXXX Test: only generate diffs to most recent consensus - // XXXX Test: making diffs from some old consensuses, but having diffs - // for others. + // XXXX Test: Failure to open cache??? // XXXX Test: failure to create consensus diff. END_OF_TESTCASES -- cgit v1.2.3-54-g00ecf From d418f28cb5cd175f224768fe214e2e2c1bc2b413 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 15 Apr 2017 10:05:10 -0400 Subject: consdiffmgr test: survive failures to compute a diff. --- src/test/test_consdiffmgr.c | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) (limited to 'src/test') diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index f550482304..1a666cc85b 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -353,6 +353,45 @@ test_consdiffmgr_diff_rules(void *arg) #undef N } +static void +test_consdiffmgr_diff_failure(void *arg) +{ + (void)arg; + MOCK(cpuworker_queue_work, mock_cpuworker_queue_work); + + /* We're going to make sure that if we have a bogus request where + * we can't actually compute a diff, the world must not end. */ + networkstatus_t *ns1 = NULL; + networkstatus_t *ns2 = NULL; + int r; + + ns1 = fake_ns_new(FLAV_NS, approx_time()-100); + ns2 = fake_ns_new(FLAV_NS, approx_time()-50); + r = consdiffmgr_add_consensus("foo bar baz\n", ns1); + tt_int_op(r, OP_EQ, 0); + // We refuse to compute a diff to or from a line holding only a single dot. + // We can add it here, though. + r = consdiffmgr_add_consensus("foo bar baz\n.\n.\n", ns2); + tt_int_op(r, OP_EQ, 0); + + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_NE, fake_cpuworker_queue); + setup_capture_of_logs(LOG_WARN); + tt_int_op(1, OP_EQ, smartlist_len(fake_cpuworker_queue)); + tt_int_op(0, OP_EQ, mock_cpuworker_run_work()); + expect_single_log_msg_containing("one of the lines to be added is \".\"."); + mock_clean_saved_logs(); + mock_cpuworker_handle_replies(); + expect_single_log_msg_containing("Worker was unable to compute consensus " + "diff from "); + + done: + teardown_capture_of_logs(); + UNMOCK(cpuworker_queue_work); + networkstatus_vote_free(ns1); + networkstatus_vote_free(ns2); +} + #define TEST(name) \ { #name, test_consdiffmgr_ ## name , TT_FORK, &setup_diffmgr, NULL } @@ -360,6 +399,7 @@ struct testcase_t consdiffmgr_tests[] = { TEST(add), TEST(make_diffs), TEST(diff_rules), + TEST(diff_failure), // XXXX Test: deleting consensuses for being too old // XXXX Test: deleting diffs for not being to most recent consensus @@ -367,7 +407,6 @@ struct testcase_t consdiffmgr_tests[] = { // XXXX Test: Objects with bad iso time are not cleaned. // XXXX Test: Failure to open cache??? - // XXXX Test: failure to create consensus diff. END_OF_TESTCASES }; -- cgit v1.2.3-54-g00ecf From 1fade372872130dca4057845eeb0a303b0ac52f6 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 15 Apr 2017 10:19:29 -0400 Subject: consdiffmgr non-test: check for initialization failure Unfortunately, this test doesn't work, so I've left it defined-out. There is currently no way in our unit tests to catch a fatal assertion failure. --- src/or/consdiffmgr.c | 2 ++ src/test/test_consdiffmgr.c | 41 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) (limited to 'src/test') diff --git a/src/or/consdiffmgr.c b/src/or/consdiffmgr.c index 0920d08750..27c95e8566 100644 --- a/src/or/consdiffmgr.c +++ b/src/or/consdiffmgr.c @@ -80,8 +80,10 @@ cdm_cache_init(void) tor_assert(cons_diff_cache == NULL); cons_diff_cache = consensus_cache_open("diff-cache", n_entries); if (cons_diff_cache == NULL) { + // LCOV_EXCL_START log_err(LD_FS, "Error: Couldn't open storage for consensus diffs."); tor_assert_unreached(); + // LCOV_EXCL_STOP } cdm_cache_dirty = 1; } diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index 1a666cc85b..fc21369e18 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -132,6 +132,42 @@ mock_cpuworker_handle_replies(void) // ============================== Beginning of tests +#if 0 +static int got_failure = 0; +static void +got_assertion_failure(void) +{ + ++got_failure; +} + +/* XXXX This test won't work, because there is currently no way to actually + * XXXX capture a real assertion failure. */ +static void +test_consdiffmgr_init_failure(void *arg) +{ + (void)arg; + // Capture assertions and bugs. + + /* As in ...test_setup, but do not create the datadir. The missing directory + * will cause a failure. */ + char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cdm")); + tor_free(get_options_mutable()->DataDirectory); + get_options_mutable()->DataDirectory = ddir_fname; // now owns the pointer. + + consdiff_cfg_t consdiff_cfg = { 7200, 300 }; + + tor_set_failed_assertion_callback(got_assertion_failure); + tor_capture_bugs_(1); + consdiffmgr_configure(&consdiff_cfg); // This should fail. + tt_int_op(got_failure, OP_EQ, 1); + const smartlist_t *bugs = tor_get_captured_bug_log_(); + tt_int_op(smartlist_len(bugs), OP_EQ, 1); + + done: + tor_end_capture_bugs_(); +} +#endif + static void test_consdiffmgr_add(void *arg) { @@ -396,6 +432,9 @@ test_consdiffmgr_diff_failure(void *arg) { #name, test_consdiffmgr_ ## name , TT_FORK, &setup_diffmgr, NULL } struct testcase_t consdiffmgr_tests[] = { +#if 0 + { "init_failure", test_consdiffmgr_init_failure, TT_FORK, NULL, NULL }, +#endif TEST(add), TEST(make_diffs), TEST(diff_rules), @@ -405,8 +444,6 @@ struct testcase_t consdiffmgr_tests[] = { // XXXX Test: deleting diffs for not being to most recent consensus // XXXX Test: Objects of unrecognized doctype are not cleaned. // XXXX Test: Objects with bad iso time are not cleaned. - - // XXXX Test: Failure to open cache??? END_OF_TESTCASES }; -- cgit v1.2.3-54-g00ecf From 35f6b678abaa298857e6b5cef187813c4c161571 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 15 Apr 2017 10:59:05 -0400 Subject: Test the easiest cases of consdiffmgr_cleanup. One more to go: deleting the old diffs. --- src/or/consdiffmgr.c | 2 +- src/or/consdiffmgr.h | 1 + src/test/test_consdiffmgr.c | 91 +++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 89 insertions(+), 5 deletions(-) (limited to 'src/test') diff --git a/src/or/consdiffmgr.c b/src/or/consdiffmgr.c index 27c95e8566..df9c5b9e08 100644 --- a/src/or/consdiffmgr.c +++ b/src/or/consdiffmgr.c @@ -92,7 +92,7 @@ cdm_cache_init(void) * Helper: return the consensus_cache_t * that backs this manager, * initializing it if needed. */ -static consensus_cache_t * +STATIC consensus_cache_t * cdm_cache_get(void) { if (PREDICT_UNLIKELY(cons_diff_cache == NULL)) { diff --git a/src/or/consdiffmgr.h b/src/or/consdiffmgr.h index 474876e432..2b3cee264a 100644 --- a/src/or/consdiffmgr.h +++ b/src/or/consdiffmgr.h @@ -35,6 +35,7 @@ void consdiffmgr_configure(const consdiff_cfg_t *cfg); void consdiffmgr_free_all(void); #ifdef CONSDIFFMGR_PRIVATE +STATIC consensus_cache_t *cdm_cache_get(void); STATIC consensus_cache_entry_t *cdm_cache_lookup_consensus( consensus_flavor_t flavor, time_t valid_after); #endif diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index fc21369e18..624c772b8a 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -428,6 +428,89 @@ test_consdiffmgr_diff_failure(void *arg) networkstatus_vote_free(ns2); } +static void +test_consdiffmgr_cleanup_old(void *arg) +{ + (void)arg; + config_line_t *labels = NULL; + consensus_cache_entry_t *ent = NULL; + consensus_cache_t *cache = cdm_cache_get(); // violate abstraction barrier + + /* This item will be will be cleanable because it has a valid-after + * time far in the past. */ + config_line_prepend(&labels, "document-type", "confribble-blarg"); + config_line_prepend(&labels, "consensus-valid-after", + "1980-10-10T10:10:10"); + ent = consensus_cache_add(cache, labels, (const uint8_t*)"Foo", 3); + tt_assert(ent); + consensus_cache_entry_decref(ent); + + setup_capture_of_logs(LOG_DEBUG); + tt_int_op(1, OP_EQ, consdiffmgr_cleanup()); + expect_log_msg_containing("Deleting entry because its consensus-valid-" + "after value (1980-10-10T10:10:10) was too old"); + + done: + teardown_capture_of_logs(); + config_free_lines(labels); +} + +static void +test_consdiffmgr_cleanup_bad_valid_after(void *arg) +{ + /* This will seem cleanable, but isn't, because its valid-after time is + * misformed. */ + + (void)arg; + config_line_t *labels = NULL; + consensus_cache_entry_t *ent = NULL; + consensus_cache_t *cache = cdm_cache_get(); // violate abstraction barrier + + config_line_prepend(&labels, "document-type", "consensus"); + config_line_prepend(&labels, "consensus-valid-after", + "whan that aprille with his shoures soote"); // (~1385?) + ent = consensus_cache_add(cache, labels, (const uint8_t*)"Foo", 3); + tt_assert(ent); + consensus_cache_entry_decref(ent); + + setup_capture_of_logs(LOG_DEBUG); + tt_int_op(0, OP_EQ, consdiffmgr_cleanup()); + expect_log_msg_containing("Ignoring entry because its consensus-valid-" + "after value (\"whan that aprille with his " + "shoures soote\") was unparseable"); + + done: + teardown_capture_of_logs(); + config_free_lines(labels); +} + +static void +test_consdiffmgr_cleanup_no_valid_after(void *arg) +{ + (void)arg; + config_line_t *labels = NULL; + consensus_cache_entry_t *ent = NULL; + consensus_cache_t *cache = cdm_cache_get(); // violate abstraction barrier + + /* This item will be will be uncleanable because it has no recognized + * valid-after. */ + config_line_prepend(&labels, "document-type", "consensus"); + config_line_prepend(&labels, "confrooble-voolid-oofter", + "2010-10-10T09:08:07"); + ent = consensus_cache_add(cache, labels, (const uint8_t*)"Foo", 3); + tt_assert(ent); + consensus_cache_entry_decref(ent); + + setup_capture_of_logs(LOG_DEBUG); + tt_int_op(0, OP_EQ, consdiffmgr_cleanup()); + expect_log_msg_containing("Ignoring entry because it had no consensus-" + "valid-after label"); + + done: + teardown_capture_of_logs(); + config_free_lines(labels); +} + #define TEST(name) \ { #name, test_consdiffmgr_ ## name , TT_FORK, &setup_diffmgr, NULL } @@ -439,11 +522,11 @@ struct testcase_t consdiffmgr_tests[] = { TEST(make_diffs), TEST(diff_rules), TEST(diff_failure), + TEST(cleanup_old), + TEST(cleanup_bad_valid_after), + TEST(cleanup_no_valid_after), + //TEST(cleanup_old_diffs), - // XXXX Test: deleting consensuses for being too old - // XXXX Test: deleting diffs for not being to most recent consensus - // XXXX Test: Objects of unrecognized doctype are not cleaned. - // XXXX Test: Objects with bad iso time are not cleaned. END_OF_TESTCASES }; -- cgit v1.2.3-54-g00ecf From 5726fec9c22c53c77f67b26782de1803ff6cca49 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 15 Apr 2017 11:13:39 -0400 Subject: Consdiffmgr test: Make sure that diffs are removable A diff is removable as soon as it no longer takes you to the most recent consensus of the appropriate flavor. --- src/test/test_consdiffmgr.c | 56 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) (limited to 'src/test') diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index 624c772b8a..4eb9e7afba 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -511,6 +511,60 @@ test_consdiffmgr_cleanup_no_valid_after(void *arg) config_free_lines(labels); } +static void +test_consdiffmgr_cleanup_old_diffs(void *arg) +{ + (void)arg; +#define N 4 + char *md_body[N]; + networkstatus_t *md_ns[N]; + uint8_t md_ns_sha3[N][DIGEST256_LEN]; + int i; + + /* Make sure that the cleanup function removes diffs to the not-most-recent + * consensus. */ + + MOCK(cpuworker_queue_work, mock_cpuworker_queue_work); + + /* Create a bunch of consensus things at 15-second intervals. */ + time_t start = approx_time() - 120; + for (i = 0; i < N; ++i) { + time_t when = start + i * 15; + md_body[i] = fake_ns_body_new(FLAV_MICRODESC, when); + md_ns[i] = fake_ns_new(FLAV_MICRODESC, when); + crypto_digest256((char *)md_ns_sha3[i], md_body[i], strlen(md_body[i]), + DIGEST_SHA3_256); + } + + /* add the first 3. */ + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[0], md_ns[0])); + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[1], md_ns[1])); + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[2], md_ns[2])); + /* Make diffs. */ + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_NE, fake_cpuworker_queue); + tt_int_op(2, OP_EQ, smartlist_len(fake_cpuworker_queue)); + tt_int_op(0, OP_EQ, mock_cpuworker_run_work()); + mock_cpuworker_handle_replies(); + tt_ptr_op(NULL, OP_EQ, fake_cpuworker_queue); + + /* Nothing is deletable now */ + tt_int_op(0, OP_EQ, consdiffmgr_cleanup()); + + /* Now add an even-more-recent consensus; this should make all previous + * diffs deletable */ + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[3], md_ns[3])); + tt_int_op(2, OP_EQ, consdiffmgr_cleanup()); + + done: + for (i = 0; i < N; ++i) { + tor_free(md_body[i]); + networkstatus_vote_free(md_ns[i]); + } + UNMOCK(cpuworker_queue_work); +#undef N +} + #define TEST(name) \ { #name, test_consdiffmgr_ ## name , TT_FORK, &setup_diffmgr, NULL } @@ -525,7 +579,7 @@ struct testcase_t consdiffmgr_tests[] = { TEST(cleanup_old), TEST(cleanup_bad_valid_after), TEST(cleanup_no_valid_after), - //TEST(cleanup_old_diffs), + TEST(cleanup_old_diffs), END_OF_TESTCASES }; -- cgit v1.2.3-54-g00ecf From 655f1c8e011a52910152ba52fc24c6e5b85023b4 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 15 Apr 2017 11:43:53 -0400 Subject: consdiffmgr: function to re-validate stored sha3 digests at startup --- src/or/consdiffmgr.c | 57 +++++++++++++++++++++++++++++++++++++++++++-- src/or/consdiffmgr.h | 1 + src/test/test_consdiffmgr.c | 3 +++ 3 files changed, 59 insertions(+), 2 deletions(-) (limited to 'src/test') diff --git a/src/or/consdiffmgr.c b/src/or/consdiffmgr.c index a9938d2f8b..dce8c8b437 100644 --- a/src/or/consdiffmgr.c +++ b/src/or/consdiffmgr.c @@ -370,8 +370,6 @@ consdiffmgr_cleanup(void) smartlist_free(objects); smartlist_free(consensuses); smartlist_free(diffs); - // XXXX for anything where the sha3 doesn't match -- delete it. But not - // XXXX here. Somewhere else? // Actually remove files, if they're not used. consensus_cache_delete_pending(cdm_cache_get()); @@ -390,6 +388,61 @@ consdiffmgr_configure(const consdiff_cfg_t *cfg) (void) cdm_cache_get(); } +/** + * Scan the consensus diff manager's cache for any grossly malformed entries, + * and mark them as deletable. Return 0 if no problems were found; 1 + * if problems were found and fixed. + */ +int +consdiffmgr_validate(void) +{ + /* Right now, we only check for entries that have bad sha3 values */ + int problems = 0; + + smartlist_t *objects = smartlist_new(); + consensus_cache_find_all(objects, cdm_cache_get(), + NULL, NULL); + SMARTLIST_FOREACH_BEGIN(objects, consensus_cache_entry_t *, obj) { + const char *lv_sha3 = + consensus_cache_entry_get_value(obj, LABEL_SHA3_DIGEST); + if (lv_sha3 == NULL) + continue; + + uint8_t sha3_expected[DIGEST256_LEN]; + uint8_t sha3_received[DIGEST256_LEN]; + int r = cdm_entry_get_sha3_value(sha3_expected, obj, LABEL_SHA3_DIGEST); + if (r == -1) { + /* digest isn't there; that's allowed */ + continue; + } else if (r == -2) { + /* digest is malformed; that's not allowed */ + problems = 1; + consensus_cache_entry_mark_for_removal(obj); + continue; + } + const uint8_t *body; + size_t bodylen; + consensus_cache_entry_incref(obj); + r = consensus_cache_entry_get_body(obj, &body, &bodylen); + if (r == 0) { + crypto_digest256((char *)sha3_received, (const char *)body, bodylen, + DIGEST_SHA3_256); + } + consensus_cache_entry_decref(obj); + if (r < 0) + continue; + + if (fast_memneq(sha3_received, sha3_expected, DIGEST256_LEN)) { + problems = 1; + consensus_cache_entry_mark_for_removal(obj); + continue; + } + + } SMARTLIST_FOREACH_END(obj); + smartlist_free(objects); + return problems; +} + /** * Helper: build new diffs of flavor as needed */ diff --git a/src/or/consdiffmgr.h b/src/or/consdiffmgr.h index 2b3cee264a..3e89ea2b23 100644 --- a/src/or/consdiffmgr.h +++ b/src/or/consdiffmgr.h @@ -33,6 +33,7 @@ void consdiffmgr_rescan(void); int consdiffmgr_cleanup(void); void consdiffmgr_configure(const consdiff_cfg_t *cfg); void consdiffmgr_free_all(void); +int consdiffmgr_validate(void); #ifdef CONSDIFFMGR_PRIVATE STATIC consensus_cache_t *cdm_cache_get(void); diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index 4eb9e7afba..5161587873 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -556,6 +556,9 @@ test_consdiffmgr_cleanup_old_diffs(void *arg) tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[3], md_ns[3])); tt_int_op(2, OP_EQ, consdiffmgr_cleanup()); + /* Everything should be valid at this point */ + tt_int_op(0, OP_EQ, consdiffmgr_validate()); + done: for (i = 0; i < N; ++i) { tor_free(md_body[i]); -- cgit v1.2.3-54-g00ecf From 605bcfbf711291d154c69e27fc49e3162cf4d7d0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 16 Apr 2017 11:28:25 -0400 Subject: consdiffmgr: Enable in-progress test that was not previously working Also, add a list of additional tests to write. --- src/test/test_consdiffmgr.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) (limited to 'src/test') diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index 5161587873..e637c11c47 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -280,13 +280,10 @@ test_consdiffmgr_make_diffs(void *arg) consdiffmgr_rescan(); tt_ptr_op(NULL, OP_NE, fake_cpuworker_queue); tt_int_op(1, OP_EQ, smartlist_len(fake_cpuworker_queue)); - // XXXX not working yet - /* diff_status = consdiffmgr_find_diff_from(&diff, FLAV_MICRODESC, DIGEST_SHA3_256, md_ns_sha3, DIGEST256_LEN); tt_int_op(CONSDIFF_IN_PROGRESS, OP_EQ, diff_status); - */ // Now run that process and get the diff. r = mock_cpuworker_run_work(); @@ -584,6 +581,17 @@ struct testcase_t consdiffmgr_tests[] = { TEST(cleanup_no_valid_after), TEST(cleanup_old_diffs), + // XXXX Test: Look up consensuses by digest in existing cases. + // XXXX Test: no duplicate diff job is launched when a job is pending. + // XXXX Test: register status when no pending entry existed?? (bug) + // XXXX Test: lookup entry, find in-progress. + // XXXX Test: lookup entry, find error. + // XXXX Test: clean up hashtable after most-recent-consensus changes. + // XXXX Test: cdm_entry_get_sha3_value cases. + // XXXX Test: sha3 mismatch on validation + // XXXX Test: initial loading of diffs from disk. + // XXXX Test: non-cacheing cases of replyfn(). + // (Bug: must release handle) END_OF_TESTCASES }; -- cgit v1.2.3-54-g00ecf From 831e656baa0e43925e106c6029a92866cec6ca74 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 16 Apr 2017 11:51:14 -0400 Subject: consdiffmgr tests: add tests to validate diff lookup/application This commit adds some helper functions to look up the diff from one consensus and to make sure that applying it leads to another. Then we add them throughout the existing test cases. Doing this turned up a reference-leaking bug in consensus_diff_worker_replyfn. --- src/test/test_consdiffmgr.c | 101 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 96 insertions(+), 5 deletions(-) (limited to 'src/test') diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index e637c11c47..4be36a4a45 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -130,6 +130,50 @@ mock_cpuworker_handle_replies(void) fake_cpuworker_queue = NULL; } +// ============================== Other helpers + +static consdiff_status_t +lookup_diff_from(consensus_cache_entry_t **out, + consensus_flavor_t flav, + const char *str1) +{ + uint8_t digest[DIGEST256_LEN]; + crypto_digest256((char*)digest, str1, strlen(str1), DIGEST_SHA3_256); + return consdiffmgr_find_diff_from(out, flav, + DIGEST_SHA3_256, digest, sizeof(digest)); +} + +static int +lookup_apply_and_verify_diff(consensus_flavor_t flav, + const char *str1, + const char *str2) +{ + char *diff_string = NULL; + consensus_cache_entry_t *ent = NULL; + consdiff_status_t status = lookup_diff_from(&ent, flav, str1); + if (ent == NULL || status != CONSDIFF_AVAILABLE) + return -1; + + consensus_cache_entry_incref(ent); + size_t size; + const uint8_t *body; + int r = consensus_cache_entry_get_body(ent, &body, &size); + if (r == 0) + diff_string = tor_memdup_nulterm(body, size); + consensus_cache_entry_decref(ent); + if (diff_string == NULL) + return -1; + + char *applied = consensus_diff_apply(str1, diff_string); + tor_free(diff_string); + if (applied == NULL) + return -1; + + int match = !strcmp(applied, str2); + tor_free(applied); + return match ? 0 : -1; +} + // ============================== Beginning of tests #if 0 @@ -368,6 +412,35 @@ test_consdiffmgr_diff_rules(void *arg) tt_int_op(0, OP_EQ, mock_cpuworker_run_work()); mock_cpuworker_handle_replies(); + /* At this point, we should actually have working diffs! */ + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_NS, ns_body[0], ns_body[5])); + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_NS, ns_body[1], ns_body[5])); + + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[1], md_body[4])); + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[2], md_body[4])); + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[3], md_body[4])); + + /* Self-to-self diff won't be present */ + consensus_cache_entry_t *ent; + tt_int_op(CONSDIFF_NOT_FOUND, OP_EQ, + lookup_diff_from(&ent, FLAV_NS, ns_body[5])); + /* No diff from 2 has been added yet */ + tt_int_op(CONSDIFF_NOT_FOUND, OP_EQ, + lookup_diff_from(&ent, FLAV_NS, ns_body[2])); + /* No diff arriving at old things. */ + tt_int_op(-1, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[1], md_body[2])); + /* No backwards diff */ + tt_int_op(-1, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[4], md_body[3])); + + /* Now, an update: add number 2 and make sure it's the only one whose diff + * is regenerated. */ tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(ns_body[2], ns_ns[2])); consdiffmgr_rescan(); tt_ptr_op(NULL, OP_NE, fake_cpuworker_queue); @@ -375,6 +448,9 @@ test_consdiffmgr_diff_rules(void *arg) tt_int_op(0, OP_EQ, mock_cpuworker_run_work()); mock_cpuworker_handle_replies(); + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_NS, ns_body[2], ns_body[5])); + done: for (i = 0; i < N; ++i) { tor_free(md_body[i]); @@ -418,6 +494,11 @@ test_consdiffmgr_diff_failure(void *arg) expect_single_log_msg_containing("Worker was unable to compute consensus " "diff from "); + /* Make sure the diff is not present */ + consensus_cache_entry_t *ent; + tt_int_op(CONSDIFF_NOT_FOUND, OP_EQ, + lookup_diff_from(&ent, FLAV_NS, "foo bar baz\n")); + done: teardown_capture_of_logs(); UNMOCK(cpuworker_queue_work); @@ -547,12 +628,24 @@ test_consdiffmgr_cleanup_old_diffs(void *arg) /* Nothing is deletable now */ tt_int_op(0, OP_EQ, consdiffmgr_cleanup()); + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[0], md_body[2])); + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[1], md_body[2])); /* Now add an even-more-recent consensus; this should make all previous * diffs deletable */ tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[3], md_ns[3])); tt_int_op(2, OP_EQ, consdiffmgr_cleanup()); + consensus_cache_entry_t *ent; + tt_int_op(CONSDIFF_NOT_FOUND, OP_EQ, + lookup_diff_from(&ent, FLAV_MICRODESC, md_body[0])); + tt_int_op(CONSDIFF_NOT_FOUND, OP_EQ, + lookup_diff_from(&ent, FLAV_MICRODESC, md_body[1])); + tt_int_op(CONSDIFF_NOT_FOUND, OP_EQ, + lookup_diff_from(&ent, FLAV_MICRODESC, md_body[2])); + /* Everything should be valid at this point */ tt_int_op(0, OP_EQ, consdiffmgr_validate()); @@ -581,17 +674,15 @@ struct testcase_t consdiffmgr_tests[] = { TEST(cleanup_no_valid_after), TEST(cleanup_old_diffs), - // XXXX Test: Look up consensuses by digest in existing cases. // XXXX Test: no duplicate diff job is launched when a job is pending. // XXXX Test: register status when no pending entry existed?? (bug) - // XXXX Test: lookup entry, find in-progress. - // XXXX Test: lookup entry, find error. - // XXXX Test: clean up hashtable after most-recent-consensus changes. + // XXXX Test: clean up hashtable after most-recent-consensus changes + // in cdm_diff_ht_purge(). // XXXX Test: cdm_entry_get_sha3_value cases. // XXXX Test: sha3 mismatch on validation // XXXX Test: initial loading of diffs from disk. // XXXX Test: non-cacheing cases of replyfn(). - // (Bug: must release handle) + END_OF_TESTCASES }; -- cgit v1.2.3-54-g00ecf From de0142cd9d980e3f08caf48d2c0b42c4149c1230 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 16 Apr 2017 12:01:45 -0400 Subject: Expand diff-management test to cover reloading items from disk --- src/test/test_consdiffmgr.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) (limited to 'src/test') diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index 4be36a4a45..f3cce2362d 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -174,6 +174,14 @@ lookup_apply_and_verify_diff(consensus_flavor_t flav, return match ? 0 : -1; } +static void +cdm_reload(void) +{ + consdiffmgr_free_all(); + cdm_cache_get(); + consdiffmgr_rescan(); +} + // ============================== Beginning of tests #if 0 @@ -451,6 +459,23 @@ test_consdiffmgr_diff_rules(void *arg) tt_int_op(0, OP_EQ, lookup_apply_and_verify_diff(FLAV_NS, ns_body[2], ns_body[5])); + /* Finally: reload, and make sure that the information is still indexed */ + cdm_reload(); + + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_NS, ns_body[0], ns_body[5])); + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_NS, ns_body[2], ns_body[5])); + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_NS, ns_body[1], ns_body[5])); + + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[1], md_body[4])); + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[2], md_body[4])); + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[3], md_body[4])); + done: for (i = 0; i < N; ++i) { tor_free(md_body[i]); @@ -680,7 +705,6 @@ struct testcase_t consdiffmgr_tests[] = { // in cdm_diff_ht_purge(). // XXXX Test: cdm_entry_get_sha3_value cases. // XXXX Test: sha3 mismatch on validation - // XXXX Test: initial loading of diffs from disk. // XXXX Test: non-cacheing cases of replyfn(). END_OF_TESTCASES -- cgit v1.2.3-54-g00ecf From bb38657b7790934635facb569db79f7792bbb199 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 16 Apr 2017 12:17:08 -0400 Subject: consdiffmgr test: add a test for updating ht on clean/rescan. This brings us back up to ~94% coverage --- src/test/test_consdiffmgr.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) (limited to 'src/test') diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index f3cce2362d..a1b5983440 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -623,6 +623,7 @@ test_consdiffmgr_cleanup_old_diffs(void *arg) networkstatus_t *md_ns[N]; uint8_t md_ns_sha3[N][DIGEST256_LEN]; int i; + consensus_cache_entry_t *hold_ent = NULL, *ent; /* Make sure that the cleanup function removes diffs to the not-most-recent * consensus. */ @@ -658,15 +659,19 @@ test_consdiffmgr_cleanup_old_diffs(void *arg) tt_int_op(0, OP_EQ, lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[1], md_body[2])); + tt_int_op(CONSDIFF_AVAILABLE, OP_EQ, + lookup_diff_from(&hold_ent, FLAV_MICRODESC, md_body[1])); + consensus_cache_entry_incref(hold_ent); // incref, so it is preserved. + /* Now add an even-more-recent consensus; this should make all previous * diffs deletable */ tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[3], md_ns[3])); tt_int_op(2, OP_EQ, consdiffmgr_cleanup()); - consensus_cache_entry_t *ent; tt_int_op(CONSDIFF_NOT_FOUND, OP_EQ, lookup_diff_from(&ent, FLAV_MICRODESC, md_body[0])); - tt_int_op(CONSDIFF_NOT_FOUND, OP_EQ, + /* This one is marked deletable but still in the hashtable */ + tt_int_op(CONSDIFF_AVAILABLE, OP_EQ, lookup_diff_from(&ent, FLAV_MICRODESC, md_body[1])); tt_int_op(CONSDIFF_NOT_FOUND, OP_EQ, lookup_diff_from(&ent, FLAV_MICRODESC, md_body[2])); @@ -674,11 +679,29 @@ test_consdiffmgr_cleanup_old_diffs(void *arg) /* Everything should be valid at this point */ tt_int_op(0, OP_EQ, consdiffmgr_validate()); + /* And if we recan NOW, we'll purge the hashtable of the entries, + * and launch attempts to generate new ones */ + consdiffmgr_rescan(); + tt_int_op(CONSDIFF_IN_PROGRESS, OP_EQ, + lookup_diff_from(&ent, FLAV_MICRODESC, md_body[0])); + tt_int_op(CONSDIFF_IN_PROGRESS, OP_EQ, + lookup_diff_from(&ent, FLAV_MICRODESC, md_body[1])); + tt_int_op(CONSDIFF_IN_PROGRESS, OP_EQ, + lookup_diff_from(&ent, FLAV_MICRODESC, md_body[2])); + + /* We're still holding on to this, though, so we can still map it! */ + const uint8_t *t1 = NULL; + size_t s; + int r = consensus_cache_entry_get_body(hold_ent, &t1, &s); + tt_int_op(r, OP_EQ, 0); + tt_assert(t1); + done: for (i = 0; i < N; ++i) { tor_free(md_body[i]); networkstatus_vote_free(md_ns[i]); } + consensus_cache_entry_decref(hold_ent); UNMOCK(cpuworker_queue_work); #undef N } @@ -701,8 +724,6 @@ struct testcase_t consdiffmgr_tests[] = { // XXXX Test: no duplicate diff job is launched when a job is pending. // XXXX Test: register status when no pending entry existed?? (bug) - // XXXX Test: clean up hashtable after most-recent-consensus changes - // in cdm_diff_ht_purge(). // XXXX Test: cdm_entry_get_sha3_value cases. // XXXX Test: sha3 mismatch on validation // XXXX Test: non-cacheing cases of replyfn(). -- cgit v1.2.3-54-g00ecf From 6cc21aa89cc1b9dea6e5256722597e3bf07d1a20 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 16 Apr 2017 15:36:20 -0400 Subject: consdiffmgr: add tests for cdm_entry_get_sha3_value --- src/or/consdiffmgr.c | 2 +- src/or/consdiffmgr.h | 3 +++ src/test/test_consdiffmgr.c | 41 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 44 insertions(+), 2 deletions(-) (limited to 'src/test') diff --git a/src/or/consdiffmgr.c b/src/or/consdiffmgr.c index 1c9c2a9932..59d0f28f51 100644 --- a/src/or/consdiffmgr.c +++ b/src/or/consdiffmgr.c @@ -320,7 +320,7 @@ cdm_labels_prepend_sha3(config_line_t **labels, * given label, set digest_out to that value (decoded), and return 0. * * Return -1 if there is no such label, and -2 if it is badly formatted. */ -static int +STATIC int cdm_entry_get_sha3_value(uint8_t *digest_out, consensus_cache_entry_t *ent, const char *label) diff --git a/src/or/consdiffmgr.h b/src/or/consdiffmgr.h index 3e89ea2b23..b6b7555ad9 100644 --- a/src/or/consdiffmgr.h +++ b/src/or/consdiffmgr.h @@ -39,6 +39,9 @@ int consdiffmgr_validate(void); STATIC consensus_cache_t *cdm_cache_get(void); STATIC consensus_cache_entry_t *cdm_cache_lookup_consensus( consensus_flavor_t flavor, time_t valid_after); +STATIC int cdm_entry_get_sha3_value(uint8_t *digest_out, + consensus_cache_entry_t *ent, + const char *label); #endif #endif diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index a1b5983440..0035029a9b 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -220,6 +220,45 @@ test_consdiffmgr_init_failure(void *arg) } #endif +static void +test_consdiffmgr_sha3_helper(void *arg) +{ + (void) arg; + consensus_cache_t *cache = cdm_cache_get(); // violate abstraction barrier + config_line_t *lines = NULL; + char *mem_op_hex_tmp = NULL; + config_line_prepend(&lines, "good-sha", + "F00DF00DF00DF00DF00DF00DF00DF00D" + "F00DF00DF00DF00DF00DF00DF00DF00D"); + config_line_prepend(&lines, "short-sha", + "F00DF00DF00DF00DF00DF00DF00DF00D" + "F00DF00DF00DF00DF00DF00DF00DF0"); + config_line_prepend(&lines, "long-sha", + "F00DF00DF00DF00DF00DF00DF00DF00D" + "F00DF00DF00DF00DF00DF00DF00DF00DF00D"); + config_line_prepend(&lines, "not-sha", + "F00DF00DF00DF00DF00DF00DF00DF00D" + "F00DF00DF00DF00DF00DF00DF00DXXXX"); + consensus_cache_entry_t *ent = + consensus_cache_add(cache, lines, (const uint8_t *)"Hi there", 8); + + uint8_t buf[DIGEST256_LEN]; + tt_int_op(-1, OP_EQ, cdm_entry_get_sha3_value(buf, NULL, "good-sha")); + tt_int_op(0, OP_EQ, cdm_entry_get_sha3_value(buf, ent, "good-sha")); + test_memeq_hex(buf, "F00DF00DF00DF00DF00DF00DF00DF00D" + "F00DF00DF00DF00DF00DF00DF00DF00D"); + + tt_int_op(-1, OP_EQ, cdm_entry_get_sha3_value(buf, ent, "missing-sha")); + tt_int_op(-2, OP_EQ, cdm_entry_get_sha3_value(buf, ent, "short-sha")); + tt_int_op(-2, OP_EQ, cdm_entry_get_sha3_value(buf, ent, "long-sha")); + tt_int_op(-2, OP_EQ, cdm_entry_get_sha3_value(buf, ent, "not-sha")); + + done: + consensus_cache_entry_decref(ent); + config_free_lines(lines); + tor_free(mem_op_hex_tmp); +} + static void test_consdiffmgr_add(void *arg) { @@ -713,6 +752,7 @@ struct testcase_t consdiffmgr_tests[] = { #if 0 { "init_failure", test_consdiffmgr_init_failure, TT_FORK, NULL, NULL }, #endif + TEST(sha3_helper), TEST(add), TEST(make_diffs), TEST(diff_rules), @@ -724,7 +764,6 @@ struct testcase_t consdiffmgr_tests[] = { // XXXX Test: no duplicate diff job is launched when a job is pending. // XXXX Test: register status when no pending entry existed?? (bug) - // XXXX Test: cdm_entry_get_sha3_value cases. // XXXX Test: sha3 mismatch on validation // XXXX Test: non-cacheing cases of replyfn(). -- cgit v1.2.3-54-g00ecf From 2e9e2d023bfa7c524d82387d8d05427ab57f1fec Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 16 Apr 2017 15:49:32 -0400 Subject: consdiffmgr: tests for consdiffmgr_validate() --- src/test/test_consdiffmgr.c | 66 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) (limited to 'src/test') diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index 0035029a9b..eb2d20751d 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -745,6 +745,70 @@ test_consdiffmgr_cleanup_old_diffs(void *arg) #undef N } +static void +test_consdiffmgr_validate(void *arg) +{ + (void)arg; + config_line_t *lines = NULL; + consensus_cache_entry_t *ent = NULL; + consensus_cache_t *cache = cdm_cache_get(); // violate abstraction barrier + smartlist_t *vals = smartlist_new(); + + /* Put these: objects in the cache: one with a good sha3, one with bad sha3, + * one with a wrong sha3, and one with no sha3. */ + config_line_prepend(&lines, "id", "wrong sha3"); + config_line_prepend(&lines, "sha3-digest", + "F00DF00DF00DF00DF00DF00DF00DF00D" + "F00DF00DF00DF00DF00DF00DF00DF00D"); + ent = consensus_cache_add(cache, lines, (const uint8_t *)"Hi there", 8); + consensus_cache_entry_decref(ent); + config_free_lines(lines); + lines = NULL; + + config_line_prepend(&lines, "id", "bad sha3"); + config_line_prepend(&lines, "sha3-digest", + "now is the winter of our dicotheque"); + ent = consensus_cache_add(cache, lines, (const uint8_t *)"Hi there", 8); + consensus_cache_entry_decref(ent); + config_free_lines(lines); + lines = NULL; + + config_line_prepend(&lines, "id", "no sha3"); + ent = consensus_cache_add(cache, lines, (const uint8_t *)"Hi there", 8); + consensus_cache_entry_decref(ent); + config_free_lines(lines); + lines = NULL; + + config_line_prepend(&lines, "id", "good sha3"); + config_line_prepend(&lines, "sha3-digest", + "8d8b1998616cd6b4c4055da8d38728dc" + "93c758d4131a53c7d81aa6337dee1c05"); + ent = consensus_cache_add(cache, lines, (const uint8_t *)"Hi there", 8); + consensus_cache_entry_decref(ent); + config_free_lines(lines); + lines = NULL; + + cdm_reload(); + cache = cdm_cache_get(); + tt_int_op(1, OP_EQ, consdiffmgr_validate()); + + consensus_cache_find_all(vals, cache, "id", "good sha3"); + tt_int_op(smartlist_len(vals), OP_EQ, 1); + smartlist_clear(vals); + + consensus_cache_find_all(vals, cache, "id", "no sha3"); + tt_int_op(smartlist_len(vals), OP_EQ, 1); + smartlist_clear(vals); + + consensus_cache_find_all(vals, cache, "id", "wrong sha3"); + tt_int_op(smartlist_len(vals), OP_EQ, 0); + consensus_cache_find_all(vals, cache, "id", "bad sha3"); + tt_int_op(smartlist_len(vals), OP_EQ, 0); + + done: + smartlist_free(vals); +} + #define TEST(name) \ { #name, test_consdiffmgr_ ## name , TT_FORK, &setup_diffmgr, NULL } @@ -761,10 +825,10 @@ struct testcase_t consdiffmgr_tests[] = { TEST(cleanup_bad_valid_after), TEST(cleanup_no_valid_after), TEST(cleanup_old_diffs), + TEST(validate), // XXXX Test: no duplicate diff job is launched when a job is pending. // XXXX Test: register status when no pending entry existed?? (bug) - // XXXX Test: sha3 mismatch on validation // XXXX Test: non-cacheing cases of replyfn(). END_OF_TESTCASES -- cgit v1.2.3-54-g00ecf From af868955814812aca711f504b1415ad355917406 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 16 Apr 2017 17:07:35 -0400 Subject: consdiffmgr test: do not launch a diff task that is already pending --- src/test/test_consdiffmgr.c | 50 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) (limited to 'src/test') diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index eb2d20751d..10da50259c 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -570,6 +570,54 @@ test_consdiffmgr_diff_failure(void *arg) networkstatus_vote_free(ns2); } +static void +test_consdiffmgr_diff_pending(void *arg) +{ +#define N 3 + (void)arg; + char *md_body[N]; + networkstatus_t *md_ns[N]; + time_t start = approx_time() - 120; + int i; + for (i = 0; i < N; ++i) { + time_t when = start + i * 30; + md_body[i] = fake_ns_body_new(FLAV_MICRODESC, when); + md_ns[i] = fake_ns_new(FLAV_MICRODESC, when); + } + + MOCK(cpuworker_queue_work, mock_cpuworker_queue_work); + + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[1], md_ns[1])); + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[2], md_ns[2])); + /* Make a diff */ + consdiffmgr_rescan(); + tt_int_op(1, OP_EQ, smartlist_len(fake_cpuworker_queue)); + + /* Look it up. Is it pending? */ + consensus_cache_entry_t *ent = NULL; + consdiff_status_t diff_status; + diff_status = lookup_diff_from(&ent, FLAV_MICRODESC, md_body[1]); + tt_int_op(CONSDIFF_IN_PROGRESS, OP_EQ, diff_status); + tt_ptr_op(ent, OP_EQ, NULL); + + /* Add another old consensus. only one new diff should launch! */ + tt_int_op(0, OP_EQ, consdiffmgr_add_consensus(md_body[0], md_ns[0])); + consdiffmgr_rescan(); + tt_int_op(2, OP_EQ, smartlist_len(fake_cpuworker_queue)); + + tt_int_op(0, OP_EQ, mock_cpuworker_run_work()); + mock_cpuworker_handle_replies(); + + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[0], md_body[2])); + tt_int_op(0, OP_EQ, + lookup_apply_and_verify_diff(FLAV_MICRODESC, md_body[1], md_body[2])); + + done: + UNMOCK(cpuworker_queue_work); +#undef N +} + static void test_consdiffmgr_cleanup_old(void *arg) { @@ -821,13 +869,13 @@ struct testcase_t consdiffmgr_tests[] = { TEST(make_diffs), TEST(diff_rules), TEST(diff_failure), + TEST(diff_pending), TEST(cleanup_old), TEST(cleanup_bad_valid_after), TEST(cleanup_no_valid_after), TEST(cleanup_old_diffs), TEST(validate), - // XXXX Test: no duplicate diff job is launched when a job is pending. // XXXX Test: register status when no pending entry existed?? (bug) // XXXX Test: non-cacheing cases of replyfn(). -- cgit v1.2.3-54-g00ecf From eb14faa0c1cb9f83d39e8b7aeb35bbe2788bea8c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 16 Apr 2017 17:13:39 -0400 Subject: Remove a checklist item that was already tested The item referred to the cdm_ht_set_status() case where the item was not already in the hashtable. But that already happens naturally when we scan the directory on startup... and we already have a test for that. --- src/test/test_consdiffmgr.c | 1 - 1 file changed, 1 deletion(-) (limited to 'src/test') diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index 10da50259c..ec4e56ccf8 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -876,7 +876,6 @@ struct testcase_t consdiffmgr_tests[] = { TEST(cleanup_old_diffs), TEST(validate), - // XXXX Test: register status when no pending entry existed?? (bug) // XXXX Test: non-cacheing cases of replyfn(). END_OF_TESTCASES -- cgit v1.2.3-54-g00ecf