Commit c6046f4d authored by Nick Mathewson's avatar Nick Mathewson 🤹
Browse files

Tweak&test log messages on apply_diff

parent 5766eed3

src/or/consdiff.c

+6 −5
Original line number Diff line number Diff line
@@ -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];

src/test/test_consdiff.c

+24 −0
Original line number Diff line number Diff line
@@ -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);