relay: Report the entire content of a stats file

It turns out that 9 years ago, we stopped appending data into stats file and
rather overwrite everytime we have new stats (see commit
a6a127c833eace1100aca7ab8ad118862bb8a8b9)

The load_stats_file() function was still thinking that we could have the same
line many times in the file which turns out to be false since 9 years ago.
However, that did not cause problem until IPv6 connection stats came along
which introduced a new line in conn-stats: "ipv6-conn-bi-direct ...".

Before, that file contained a single line starting with the tag
"conn-bi-direct".  That very tag appears also in the IPv6 tag (see above) so
the load_stats_file() function would consider that the IPv6 line as the last
tag to be appeneded to the file and fail to report the line above (for IPv4).
It would actually truncate the IPv6 line and report it (removing the "ipv6-"
part).

In other words, "conn-bi-direct" was not reported and instead
"ipv6-conn-bi-direct" was used without the "ipv6-" part.

This commit refactors the entire function so that now it looks for a
"timestamp tag" to validate and then if everything is fine, returns the entire
content of the file. The refactor simplifies the function, adds logging in
case of failures and modernize it in terms of coding standard.

Unit tests are also added that makes sure the loaded content matches the
entire file if timestamp validation passes.

Fixes #40226

Signed-off-by: David Goulet <dgoulet@torproject.org>

1 files changed, 65 insertions, 0 deletions

diff --git a/src/test/test_stats.c b/src/test/test_stats.c
index b6849b0b6d..154acd4f9f 100644
--- a/src/test/test_stats.c
+++ b/src/test/test_stats.c
@@ -31,6 +31,7 @@
 #define MAINLOOP_PRIVATE
 #define STATEFILE_PRIVATE
 #define BWHIST_PRIVATE
+#define ROUTER_PRIVATE
 
 #include "core/or/or.h"
 #include "lib/err/backtrace.h"
@@ -45,6 +46,7 @@
 #include "app/config/statefile.h"
 #include "feature/stats/bwhist.h"
 #include "feature/stats/bw_array_st.h"
+#include "feature/relay/router.h"
 
 /** Run unit tests for some stats code. */
 static void
@@ -493,6 +495,68 @@ test_get_bandwidth_lines(void *arg)
   bwhist_free_all();
 }
 
+static void
+test_load_stats_file(void *arg)
+{
+  int ret;
+  char *content = NULL, *read_file_content = NULL;
+  const char *fname = NULL;
+
+  (void) arg;
+
+  /* Load conn-stats. */
+  fname = get_fname("conn-stats");
+  tt_assert(fname);
+  read_file_content = tor_strdup(
+    "conn-bi-direct 2020-12-13 15:48:53 (86400 s) 12,34,56,78\n"
+    "ipv6-conn-bi-direct 2020-12-14 15:48:53 (86400 s) 21,43,65,87\n");
+  write_str_to_file(fname, read_file_content, 0);
+  ret = load_stats_file("conn-stats", "conn-bi-direct", 1607874000, &content);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_str_op(read_file_content, OP_EQ, content);
+
+  /* Load hidserv-stats. */
+  fname = get_fname("hidserv-stats");
+  tt_assert(fname);
+  tor_free(read_file_content);
+  read_file_content = tor_strdup(
+    "hidserv-stats-end 2020-12-13 15:48:53 (86400 s)\n"
+    "hidserv-rend-relayed-cells 48754891 delta_f=2048 epsilon=0.30 "
+      "bin_size=1024\n"
+    "hidserv-dir-onions-seen 53 delta_f=8 epsilon=0.30 bin_size=8\n");
+  write_str_to_file(fname, read_file_content, 0);
+  tor_free(content);
+  ret = load_stats_file("hidserv-stats", "hidserv-stats-end", 1607874000,
+                        &content);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_str_op(read_file_content, OP_EQ, content);
+
+  /* Load dirreq-stats. */
+  fname = get_fname("dirreq-stats");
+  tt_assert(fname);
+  tor_free(read_file_content);
+  read_file_content = tor_strdup(
+    "dirreq-stats-end 2020-12-13 15:48:53 (86400 s)\n"
+    "dirreq-v3-ips ru=1728,us=1144,de=696,ir=432,gb=328,fr=304,in=296,ua=232\n"
+    "dirreq-v3-reqs ru=3616,us=3576,de=1896,fr=800,gb=632,ir=616\n"
+    "dirreq-v3-resp ok=18472,not-enough-sigs=0,unavailable=0,not-found=0,"
+      "not-modified=3136,busy=0\n"
+    "dirreq-v3-direct-dl complete=0,timeout=0,running=0\n"
+    "dirreq-v3-tunneled-dl complete=18124,timeout=348,running=4,min=257,"
+      "d1=133653,d2=221050,q1=261242,d3=300622,d4=399758,md=539051,d6=721322,"
+      "d7=959866,q3=1103363,d8=1302035,d9=2046125,max=113404000\n");
+  write_str_to_file(fname, read_file_content, 0);
+  tor_free(content);
+  ret = load_stats_file("dirreq-stats", "dirreq-stats-end", 1607874000,
+                        &content);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_str_op(read_file_content, OP_EQ, content);
+
+ done:
+  tor_free(read_file_content);
+  tor_free(content);
+}
+
 #define ENT(name)                                                       \
   { #name, test_ ## name , 0, NULL, NULL }
 #define FORK(name)                                                      \
@@ -506,6 +570,7 @@ struct testcase_t stats_tests[] = {
   FORK(add_obs),
   FORK(fill_bandwidth_history),
   FORK(get_bandwidth_lines),
+  FORK(load_stats_file),
 
   END_OF_TESTCASES
 };