commit c6046f4db8b182133ccc756529b508522be180c8 Author: Nick Mathewson nickm@torproject.org Date: Tue Mar 7 11:36:07 2017 -0500
Tweak&test log messages on apply_diff --- src/or/consdiff.c | 11 ++++++----- src/test/test_consdiff.c | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/src/or/consdiff.c b/src/or/consdiff.c index defa1cf..73ac797 100644 --- a/src/or/consdiff.c +++ b/src/or/consdiff.c @@ -879,7 +879,7 @@ consdiff_get_digests(smartlist_t *diff, if (smartlist_len(hash_words) != 3 || strcmp(smartlist_get(hash_words, 0), hash_token)) { log_info(LD_CONSDIFF, "The provided consensus diff does not include " - "the necessary sha256 digests."); + "the necessary digests."); goto error_cleanup; }
@@ -893,7 +893,7 @@ consdiff_get_digests(smartlist_t *diff, if (strlen(cons1_hash_hex) != HEX_DIGEST256_LEN || strlen(cons2_hash_hex) != HEX_DIGEST256_LEN) { log_info(LD_CONSDIFF, "The provided consensus diff includes " - "base16-encoded sha256 digests of incorrect size."); + "base16-encoded digests of incorrect size."); goto error_cleanup; }
@@ -909,7 +909,7 @@ consdiff_get_digests(smartlist_t *diff, base16_decode(cons2_hash, DIGEST256_LEN, cons2_hash_hex, HEX_DIGEST256_LEN) != DIGEST256_LEN) { log_info(LD_CONSDIFF, "The provided consensus diff includes " - "malformed sha256 digests."); + "malformed digests."); goto error_cleanup; }
@@ -958,7 +958,7 @@ consdiff_apply_diff(smartlist_t *cons1, smartlist_t *diff, char hex_digest1[HEX_DIGEST256_LEN+1]; char e_hex_digest1[HEX_DIGEST256_LEN+1]; log_warn(LD_CONSDIFF, "Refusing to apply consensus diff because " - "the base consensus doesn't match its own digest as found in " + "the base consensus doesn't match the digest as found in " "the consensus diff header."); base16_encode(hex_digest1, HEX_DIGEST256_LEN+1, digests1->d[DIGEST_SHA256], DIGEST256_LEN); @@ -973,6 +973,7 @@ consdiff_apply_diff(smartlist_t *cons1, smartlist_t *diff, /* To avoid copying memory or iterating over all the elements, make a * read-only smartlist without the two header lines. */ + /* XXXX prop140 abstraction violation; never do this. */ smartlist_t *ed_diff = tor_malloc(sizeof(smartlist_t)); ed_diff->list = diff->list+2; ed_diff->num_used = diff->num_used-2; @@ -999,7 +1000,7 @@ consdiff_apply_diff(smartlist_t *cons1, smartlist_t *diff, if (fast_memneq(cons2_digests.d[DIGEST_SHA256], e_cons2_hash, DIGEST256_LEN)) { log_warn(LD_CONSDIFF, "Refusing to apply consensus diff because " - "the resulting consensus doesn't match its own digest as found in " + "the resulting consensus doesn't match the digest as found in " "the consensus diff header."); char hex_digest2[HEX_DIGEST256_LEN+1]; char e_hex_digest2[HEX_DIGEST256_LEN+1]; diff --git a/src/test/test_consdiff.c b/src/test/test_consdiff.c index 15f5575..00fba67 100644 --- a/src/test/test_consdiff.c +++ b/src/test/test_consdiff.c @@ -856,6 +856,7 @@ test_consdiff_apply_diff(void *arg) (void)arg; cons1 = smartlist_new(); diff = smartlist_new(); + setup_capture_of_logs(LOG_INFO);
cons1_str = tor_strdup( "header\nnetwork-status-version foo\n" @@ -870,33 +871,44 @@ test_consdiff_apply_diff(void *arg) /* diff doesn't have enough lines. */ cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("too short")
/* first line doesn't match format-version string. */ smartlist_add(diff, (char*)"foo-bar"); smartlist_add(diff, (char*)"header-line"); + mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("format is not known")
/* The first word of the second header line is not "hash". */ smartlist_clear(diff); smartlist_add(diff, (char*)"network-status-diff-version 1"); smartlist_add(diff, (char*)"word a b"); + smartlist_add(diff, (char*)"x"); + mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("does not include the necessary digests")
/* Wrong number of words after "hash". */ smartlist_clear(diff); smartlist_add(diff, (char*)"network-status-diff-version 1"); smartlist_add(diff, (char*)"hash a b c"); + mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("does not include the necessary digests")
/* base16 sha256 digests do not have the expected length. */ smartlist_clear(diff); smartlist_add(diff, (char*)"network-status-diff-version 1"); smartlist_add(diff, (char*)"hash aaa bbb"); + mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("includes base16-encoded digests of " + "incorrect size")
/* base16 sha256 digests contain non-base16 characters. */ smartlist_clear(diff); @@ -904,8 +916,10 @@ test_consdiff_apply_diff(void *arg) smartlist_add(diff, (char*)"hash" " ????????????????????????????????????????????????????????????????" " ----------------------------------------------------------------"); + mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("includes malformed digests")
/* Invalid ed diff. * As tested in apply_ed_diff, but check that apply_diff does return NULL if @@ -918,8 +932,11 @@ test_consdiff_apply_diff(void *arg) /* sha256 of cons2. */ " 635D34593020C08E5ECD865F9986E29D50028EFA62843766A8197AD228A7F6AA"); smartlist_add(diff, (char*)"foobar"); + mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("because an ed command was missing a line " + "number")
/* Base consensus doesn't match its digest as found in the diff. */ smartlist_clear(diff); @@ -929,8 +946,11 @@ test_consdiff_apply_diff(void *arg) " 3333333333333333333333333333333333333333333333333333333333333333" /* sha256 of cons2. */ " 635D34593020C08E5ECD865F9986E29D50028EFA62843766A8197AD228A7F6AA"); + mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); + expect_log_msg_containing("base consensus doesn't match the digest " + "as found");
/* Resulting consensus doesn't match its digest as found in the diff. */ smartlist_clear(diff); @@ -940,8 +960,11 @@ test_consdiff_apply_diff(void *arg) " C2199B6827514F39ED9B3F2E2E73735C6C5468FD636240BB454C526220DE702A" /* bogus sha256. */ " 3333333333333333333333333333333333333333333333333333333333333333"); + mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); + expect_log_msg_containing("resulting consensus doesn't match the " + "digest as found")
/* Very simple test, only to see that nothing errors. */ smartlist_clear(diff); @@ -988,6 +1011,7 @@ test_consdiff_apply_diff(void *arg) smartlist_clear(diff);
done: + teardown_capture_of_logs(); tor_free(cons1_str); smartlist_free(cons1); smartlist_free(diff);