commit 0a6f4627a4292e4bed94cc1b44b6f53fc8f9ce37
Author: Nick Mathewson <nickm(a)torproject.org>
Date: Wed Jun 20 08:29:38 2018 -0400
eol@eof in test-dir.c
---
changes/ticket25947 | 4 ++
changes/ticket25960 | 5 ++
src/or/dirserv.c | 33 +++++++++++--
src/or/dirserv.h | 3 +-
src/test/test_dir.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++--
5 files changed, 175 insertions(+), 9 deletions(-)
diff --git a/changes/ticket25947 b/changes/ticket25947
new file mode 100644
index 000000000..68559a73f
--- /dev/null
+++ b/changes/ticket25947
@@ -0,0 +1,4 @@
+ o Minor feature (unit tests):
+ - Test complete bandwidth measurements files and test that incomplete lines
+ only give warnings when the end of the header has not been
+ detected. Fixes bug 25947; bugfix on 0.2.2.1-alpha
diff --git a/changes/ticket25960 b/changes/ticket25960
new file mode 100644
index 000000000..0d1be2119
--- /dev/null
+++ b/changes/ticket25960
@@ -0,0 +1,5 @@
+ o Minor feature (directory authorities):
+ - Stop warning about incomplete bw lines before the first complete bw line
+ has been found, so that additional header lines can be ignored.
+ Fixes bug 25960; bugfix on 0.2.2.1-alpha
+
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index c01234e0b..2362089d3 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -2448,11 +2448,20 @@ dirserv_read_guardfraction_file(const char *fname,
/**
* Helper function to parse out a line in the measured bandwidth file
- * into a measured_bw_line_t output structure. Returns -1 on failure
- * or 0 on success.
+ * into a measured_bw_line_t output structure.
+ *
+ * If <b>line_is_after_headers</b> is true, then if we encounter an incomplete
+ * bw line, return -1 and warn, since we are after the headers and we should
+ * only parse bw lines. Return 0 otherwise.
+ *
+ * If <b>line_is_after_headers</b> is false then it means that we are not past
+ * the header block yet. If we encounter an incomplete bw line, return -1 but
+ * don't warn since there could be additional header lines coming. If we
+ * encounter a proper bw line, return 0 (and we got past the headers).
*/
STATIC int
-measured_bw_line_parse(measured_bw_line_t *out, const char *orig_line)
+measured_bw_line_parse(measured_bw_line_t *out, const char *orig_line,
+ int line_is_after_headers)
{
char *line = tor_strdup(orig_line);
char *cp = line;
@@ -2532,6 +2541,13 @@ measured_bw_line_parse(measured_bw_line_t *out, const char *orig_line)
if (got_bw && got_node_id) {
tor_free(line);
return 0;
+ } else if (line_is_after_headers == 0) {
+ /* There could be additional header lines, therefore do not give warnings
+ * but returns -1 since it's not a complete bw line. */
+ log_debug(LD_DIRSERV, "Missing bw or node_id in bandwidth file line: %s",
+ escaped(orig_line));
+ tor_free(line);
+ return -1;
} else {
log_warn(LD_DIRSERV, "Incomplete line in bandwidth file: %s",
escaped(orig_line));
@@ -2580,6 +2596,11 @@ dirserv_read_measured_bandwidths(const char *from_file,
int applied_lines = 0;
time_t file_time, now;
int ok;
+ /* This flag will be 1 only when the first successful bw measurement line
+ * has been encountered, so that measured_bw_line_parse don't give warnings
+ * if there are additional header lines, as introduced in Bandwidth List spec
+ * version 1.1.0 */
+ int line_is_after_headers = 0;
/* Initialise line, so that we can't possibly run off the end. */
memset(line, 0, sizeof(line));
@@ -2627,7 +2648,11 @@ dirserv_read_measured_bandwidths(const char *from_file,
while (!feof(fp)) {
measured_bw_line_t parsed_line;
if (fgets(line, sizeof(line), fp) && strlen(line)) {
- if (measured_bw_line_parse(&parsed_line, line) != -1) {
+ if (measured_bw_line_parse(&parsed_line, line,
+ line_is_after_headers) != -1) {
+ /* This condition will be true when the first complete valid bw line
+ * has been encountered, which means the end of the header lines. */
+ line_is_after_headers = 1;
/* Also cache the line for dirserv_get_bandwidth_for_router() */
dirserv_cache_measured_bw(&parsed_line, file_time);
if (measured_bw_line_apply(&parsed_line, routerstatuses) > 0)
diff --git a/src/or/dirserv.h b/src/or/dirserv.h
index f0b8913c5..9026f332b 100644
--- a/src/or/dirserv.h
+++ b/src/or/dirserv.h
@@ -174,7 +174,8 @@ STATIC void dirserv_set_routerstatus_testing(routerstatus_t *rs);
/* Put the MAX_MEASUREMENT_AGE #define here so unit tests can see it */
#define MAX_MEASUREMENT_AGE (3*24*60*60) /* 3 days */
-STATIC int measured_bw_line_parse(measured_bw_line_t *out, const char *line);
+STATIC int measured_bw_line_parse(measured_bw_line_t *out, const char *line,
+ int line_is_after_headers);
STATIC int measured_bw_line_apply(measured_bw_line_t *parsed_line,
smartlist_t *routerstatuses);
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index 0106e40d9..96adb5ed5 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -1547,12 +1547,18 @@ test_dir_measured_bw_kb(void *arg)
(void)arg;
for (i = 0; strcmp(lines_fail[i], "end"); i++) {
//fprintf(stderr, "Testing: %s\n", lines_fail[i]);
- tt_int_op(measured_bw_line_parse(&mbwl, lines_fail[i]), OP_EQ, -1);
+ /* Testing only with line_is_after_headers = 1. Tests with
+ * line_is_after_headers = 0 in
+ * test_dir_measured_bw_kb_line_is_after_headers */
+ tt_assert(measured_bw_line_parse(&mbwl, lines_fail[i], 1) == -1);
}
for (i = 0; strcmp(lines_pass[i], "end"); i++) {
//fprintf(stderr, "Testing: %s %d\n", lines_pass[i], TOR_ISSPACE('\n'));
- tt_int_op(measured_bw_line_parse(&mbwl, lines_pass[i]), OP_EQ, 0);
+ /* Testing only with line_is_after_headers = 1. Tests with
+ * line_is_after_headers = 0 in
+ * test_dir_measured_bw_kb_line_is_after_headers */
+ tt_assert(measured_bw_line_parse(&mbwl, lines_pass[i], 1) == 0);
tt_assert(mbwl.bw_kb == 1024);
tt_assert(strcmp(mbwl.node_hex,
"557365204145532d32353620696e73746561642e") == 0);
@@ -1564,7 +1570,7 @@ test_dir_measured_bw_kb(void *arg)
/* Test dirserv_read_measured_bandwidths */
static void
-test_dir_dirserv_read_measured_bandwidths(void *arg)
+test_dir_dirserv_read_measured_bandwidths_empty(void *arg)
{
char *fname=NULL;
(void)arg;
@@ -1581,6 +1587,129 @@ test_dir_dirserv_read_measured_bandwidths(void *arg)
teardown_capture_of_logs();
}
+/* Unit tests for measured_bw_line_parse using line_is_after_headers flag.
+ * When the end of the header is detected (a first complete bw line is parsed),
+ * incomplete lines fail and give warnings, but do not give warnings if
+ * the header is not ended, allowing to ignore additional header lines. */
+static void
+test_dir_measured_bw_kb_line_is_after_headers(void *arg)
+{
+ (void)arg;
+ measured_bw_line_t mbwl;
+ const char *line_pass = \
+ "node_id=$557365204145532d32353620696e73746561642e bw=1024\n";
+ int i;
+ const char *lines_fail[] = {
+ "node_id=$557365204145532d32353620696e73746561642e \n",
+ "bw=1024\n",
+ "rtt=300\n",
+ "end"
+ };
+
+ setup_capture_of_logs(LOG_DEBUG);
+
+ /* Test bw lines when header has ended */
+ for (i = 0; strcmp(lines_fail[i], "end"); i++) {
+ tt_assert(measured_bw_line_parse(&mbwl, lines_fail[i], 1) == -1);
+ expect_log_msg_containing("Incomplete line in bandwidth file:");
+ mock_clean_saved_logs();
+ }
+
+ tt_assert(measured_bw_line_parse(&mbwl, line_pass, 1) == 0);
+
+ /* Test bw lines when header has not ended */
+ for (i = 0; strcmp(lines_fail[i], "end"); i++) {
+ tt_assert(measured_bw_line_parse(&mbwl, lines_fail[i], 0) == -1);
+ expect_log_msg_containing("Missing bw or node_id in bandwidth file line:");
+ mock_clean_saved_logs();
+ }
+
+ tt_assert(measured_bw_line_parse(&mbwl, line_pass, 0) == 0);
+
+ done:
+ teardown_capture_of_logs();
+}
+
+/* Test dirserv_read_measured_bandwidths with whole files. */
+static void
+test_dir_dirserv_read_measured_bandwidths(void *arg)
+{
+ (void)arg;
+ char *content = NULL;
+ time_t timestamp = time(NULL);
+ char *fname = tor_strdup(get_fname("V3BandwidthsFile"));
+
+ /* Test Torflow file only with timestamp*/
+ tor_asprintf(&content, "%ld", timestamp);
+ write_str_to_file(fname, content, 0);
+ tor_free(content);
+ tt_int_op(-1, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL));
+
+ /* Test Torflow file with timestamp followed by '\n' */
+ tor_asprintf(&content, "%ld\n", timestamp);
+ write_str_to_file(fname, content, 0);
+ tor_free(content);
+ tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL));
+
+ /* Test Torflow complete file*/
+ const char *torflow_relay_lines=
+ "node_id=$557365204145532d32353620696e73746561642e bw=1024 "
+ "nick=Test measured_at=1523911725 updated_at=1523911725 "
+ "pid_error=4.11374090719 pid_error_sum=4.11374090719 "
+ "pid_bw=57136645 pid_delta=2.12168374577 circ_fail=0.2 "
+ "scanner=/filepath\n";
+
+ tor_asprintf(&content, "%ld\n%s", timestamp, torflow_relay_lines);
+ write_str_to_file(fname, content, 0);
+ tor_free(content);
+ tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL));
+
+ /* Test Torflow complete file including v1.1.0 headers */
+ const char *v110_header_lines=
+ "version=1.1.0\n"
+ "software=sbws\n"
+ "software_version=0.1.0\n"
+ "generator_started=2018-05-08T16:13:25\n"
+ "earliest_bandwidth=2018-05-08T16:13:26\n"
+ "====\n";
+
+ tor_asprintf(&content, "%ld\n%s%s", timestamp, v110_header_lines,
+ torflow_relay_lines);
+ write_str_to_file(fname, content, 0);
+ tor_free(content);
+ tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL));
+
+ /* Test Torflow with additional headers afer a correct bw line */
+ tor_asprintf(&content, "%ld\n%s%s", timestamp, torflow_relay_lines,
+ v110_header_lines);
+ write_str_to_file(fname, content, 0);
+ tor_free(content);
+ tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL));
+
+ /* Test Torflow with additional headers afer a correct bw line and more
+ * bw lines after the headers. */
+ tor_asprintf(&content, "%ld\n%s%s%s", timestamp, torflow_relay_lines,
+ v110_header_lines, torflow_relay_lines);
+ write_str_to_file(fname, content, 0);
+ tor_free(content);
+ tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL));
+
+ /* Test sbws file */
+ const char *sbws_relay_lines=
+ "node_id=$68A483E05A2ABDCA6DA5A3EF8DB5177638A27F80 "
+ "master_key_ed25519=YaqV4vbvPYKucElk297eVdNArDz9HtIwUoIeo0+cVIpQ "
+ "bw=760 nick=Test rtt=380 time=2018-05-08T16:13:26\n";
+
+ tor_asprintf(&content, "%ld\n%s%s", timestamp, v110_header_lines,
+ sbws_relay_lines);
+ write_str_to_file(fname, content, 0);
+ tor_free(content);
+ tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL));
+
+ done:
+ tor_free(fname);
+}
+
#define MBWC_INIT_TIME 1000
/** Do the measured bandwidth cache unit test */
@@ -5849,9 +5978,11 @@ struct testcase_t dir_tests[] = {
DIR_LEGACY(versions),
DIR_LEGACY(fp_pairs),
DIR(split_fps, 0),
- DIR_LEGACY(dirserv_read_measured_bandwidths),
+ DIR_LEGACY(dirserv_read_measured_bandwidths_empty),
DIR_LEGACY(measured_bw_kb),
+ DIR_LEGACY(measured_bw_kb_line_is_after_headers),
DIR_LEGACY(measured_bw_kb_cache),
+ DIR_LEGACY(dirserv_read_measured_bandwidths),
DIR_LEGACY(param_voting),
DIR(param_voting_lookup, 0),
DIR_LEGACY(v3_networkstatus),