From 2074fed6642713bcdfdc76f379956e97663ab8d0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 28 Aug 2019 11:51:16 -0400 Subject: [PATCH 1/3] Routerset config parsing: represent empty sets as NULL. routerset_t has two representations of an empty routerset: NULL, and a set containing no elements. But some of our config code assumes that empty routersets are represented as NULL. So let's give it what it assumes. Fixes bug 31495. Bugfix on e16b90b88a76; but not in any released Tor. --- src/feature/nodelist/routerset.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/feature/nodelist/routerset.c b/src/feature/nodelist/routerset.c index 76777847ef..12965ad0d8 100644 --- a/src/feature/nodelist/routerset.c +++ b/src/feature/nodelist/routerset.c @@ -479,6 +479,10 @@ routerset_kv_parse(void *target, const config_line_t *line, char **errmsg, *errmsg = tor_strdup("Invalid router list."); return -1; } else { + if (routerset_is_empty(rs)) { + /* Represent empty sets as NULL. */ + routerset_free(rs); + } *p = rs; return 0; } @@ -507,8 +511,10 @@ routerset_copy(void *dest, const void *src, const void *params) routerset_t **output = (routerset_t**)dest; const routerset_t *input = *(routerset_t**)src; routerset_free(*output); // sets *output to NULL - *output = routerset_new(); - routerset_union(*output, input); + if (! routerset_is_empty(input)) { + *output = routerset_new(); + routerset_union(*output, input); + } return 0; } From f0c1f96adcc049c49f7784645a5b197c8b3e44af Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 28 Aug 2019 12:23:36 -0400 Subject: [PATCH 2/3] Document configuration type definition functions for routerset_t These functions are all used to implement the ROUTERSET_type_defn object, which maps strings to and from routerset_t configuration variables for the configuration module. --- src/feature/nodelist/routerset.c | 37 ++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/feature/nodelist/routerset.c b/src/feature/nodelist/routerset.c index 12965ad0d8..0b89167806 100644 --- a/src/feature/nodelist/routerset.c +++ b/src/feature/nodelist/routerset.c @@ -466,6 +466,13 @@ routerset_free_(routerset_t *routerset) tor_free(routerset); } +/** + * config helper: parse a routerset-typed variable. + * + * Takes as input as a single line in line; writes its results into a + * routerset_t** passed as target. On success return 0; on failure + * return -1 and store an error message into *errmsg. + **/ static int routerset_kv_parse(void *target, const config_line_t *line, char **errmsg, const void *params) @@ -488,6 +495,12 @@ routerset_kv_parse(void *target, const config_line_t *line, char **errmsg, } } +/** + * config helper: encode a routerset-typed variable. + * + * Return a newly allocated string containing the value of the + * routerset_t** passed as value. + */ static char * routerset_encode(const void *value, const void *params) { @@ -496,6 +509,11 @@ routerset_encode(const void *value, const void *params) return routerset_to_string(*p); } +/** + * config helper: free and clear a routerset-typed variable. + * + * Clear the routerset_t** passed as value. + */ static void routerset_clear(void *value, const void *params) { @@ -504,6 +522,13 @@ routerset_clear(void *value, const void *params) routerset_free(*p); // sets *p to NULL. } +/** + * config helper: copy a routerset-typed variable. + * + * Takes it input from a routerset_t** in src; writes its output to a + * routerset_t** in dest. Returns 0 on success, -1 on (impossible) + * failure. + **/ static int routerset_copy(void *dest, const void *src, const void *params) { @@ -518,6 +543,9 @@ routerset_copy(void *dest, const void *src, const void *params) return 0; } +/** + * Function table to implement a routerset_t-based configuration type. + **/ static const var_type_fns_t routerset_type_fns = { .kv_parse = routerset_kv_parse, .encode = routerset_encode, @@ -525,6 +553,15 @@ static const var_type_fns_t routerset_type_fns = { .copy = routerset_copy }; +/** + * Definition of a routerset_t-based configuration type. + * + * Values are mapped to and from strings using the format defined in + * routerset_parse(): nicknames, IP address patterns, and fingerprints--with + * optional space, separated by commas. + * + * Empty sets are represented as NULL. + **/ const var_type_def_t ROUTERSET_type_defn = { .name = "RouterList", .fns = &routerset_type_fns From 1ef084c5fcc5f9185f05a71e7ca09fd88cfb641c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 28 Aug 2019 12:28:17 -0400 Subject: [PATCH 3/3] test_confparse: verify that clearing a routerset sets it to NULL. --- src/test/test_confparse.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/test_confparse.c b/src/test/test_confparse.c index ec018f0c52..4df275fc36 100644 --- a/src/test/test_confparse.c +++ b/src/test/test_confparse.c @@ -592,6 +592,10 @@ test_confparse_reset(void *arg) config_reset_line(&test_fmt, tst, "interval", 1); tt_int_op(tst->interval, OP_EQ, 10); + tt_ptr_op(tst->routerset, OP_NE, NULL); + config_reset_line(&test_fmt, tst, "routerset", 0); + tt_ptr_op(tst->routerset, OP_EQ, NULL); + done: config_free(&test_fmt, tst); }