commit 594e5ce635481a2aca60bbb31b656e3970ad511b Author: Victor Nepveu victor.nepveu@imt-atlantique.net Date: Tue Nov 12 22:27:36 2019 +0100

Refactor routerstatus_has_changed function

- Check all fields that might change in a routerstatus - Document the refactoring

Signed-off-by: Victor Nepveu victor.nepveu@imt-atlantique.net --- changes/ticket20218 | 3 +++ src/feature/nodelist/fmt_routerstatus.c | 2 ++ src/feature/nodelist/networkstatus.c | 22 +++++++++++++++++----- src/feature/nodelist/routerstatus_st.h | 4 ++++ 4 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/changes/ticket20218 b/changes/ticket20218 new file mode 100644 index 000000000..d5fb2b2cf --- /dev/null +++ b/changes/ticket20218 @@ -0,0 +1,3 @@ + o Minor bugfixes (controller): + - In routerstatus_has_changed(), check all the fields that are output over the control port. + Fixes bug 20218; bugfix on 0.1.1.11-alpha diff --git a/src/feature/nodelist/fmt_routerstatus.c b/src/feature/nodelist/fmt_routerstatus.c index 8dde0088d..dc44a6ef9 100644 --- a/src/feature/nodelist/fmt_routerstatus.c +++ b/src/feature/nodelist/fmt_routerstatus.c @@ -124,6 +124,8 @@ routerstatus_format_entry(const routerstatus_t *rs, const char *version, if (format != NS_CONTROL_PORT) { /* Blow up more or less nicely if we didn't get anything or not the * thing we expected. + * This should be kept in sync with the function + * routerstatus_has_changed and the struct routerstatus_t */ if (!desc) { char id[HEX_DIGEST_LEN+1]; diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c index 786802047..df035ef1b 100644 --- a/src/feature/nodelist/networkstatus.c +++ b/src/feature/nodelist/networkstatus.c @@ -101,6 +101,7 @@ #include "feature/nodelist/routerlist_st.h" #include "feature/dirauth/vote_microdesc_hash_st.h" #include "feature/nodelist/vote_routerstatus_st.h" +#include "routerstatus_st.h"

#ifdef HAVE_UNISTD_H #include <unistd.h> @@ -1603,8 +1604,14 @@ networkstatus_consensus_has_ipv6(const or_options_t* options) } }

-/** Given two router status entries for the same router identity, return 1 if - * if the contents have changed between them. Otherwise, return 0. */ +/** Given two router status entries for the same router identity, return 1 + * if the contents have changed between them. Otherwise, return 0. + * It only checks for fields that are output by control port. + * This should be kept in sync with the struct routerstatus_t + * and the printing function routerstatus_format_entry in + * NS_CONTROL_PORT mode. + **/ + static int routerstatus_has_changed(const routerstatus_t *a, const routerstatus_t *b) { @@ -1625,9 +1632,14 @@ routerstatus_has_changed(const routerstatus_t *a, const routerstatus_t *b) a->is_valid != b->is_valid || a->is_possible_guard != b->is_possible_guard || a->is_bad_exit != b->is_bad_exit || - a->is_hs_dir != b->is_hs_dir; - // XXXX this function needs a huge refactoring; it has gotten out - // XXXX of sync with routerstatus_t, and it will do so again. + a->is_hs_dir != b->is_hs_dir || + a->is_staledesc != b->is_staledesc || + a->has_bandwidth != b->has_bandwidth || + a->published_on != b->published_on || + a->ipv6_orport != b->ipv6_orport || + a->is_v2_dir != b->is_v2_dir || + a->bandwidth_kb != b->bandwidth_kb || + tor_addr_compare(&a->ipv6_addr, &b->ipv6_addr, CMP_EXACT); }

/** Notify controllers of any router status entries that changed between diff --git a/src/feature/nodelist/routerstatus_st.h b/src/feature/nodelist/routerstatus_st.h index 270cb871a..eb6c3e5cf 100644 --- a/src/feature/nodelist/routerstatus_st.h +++ b/src/feature/nodelist/routerstatus_st.h @@ -17,6 +17,10 @@ /** Contents of a single router entry in a network status object. */ struct routerstatus_t { + /* This should be kept in sync with the function + * routerstatus_has_changed and the printing function + * routerstatus_format_entry in NS_CONTROL_PORT mode. + */ time_t published_on; /**< When was this router published? */ char nickname[MAX_NICKNAME_LEN+1]; /**< The nickname this router says it * has. */