commit 80d3887360548b28fe2bd06501f0d51d0a1ba4f0 Author: Nick Mathewson nickm@torproject.org Date: Tue Aug 22 19:04:31 2017 -0400
Refactor node lookup APIs to take flags
Right now there's a single warn_if_unnamed flag for router_get_consensus_status_by_nickname() and node_get_by_nickname(), that is nearly always 1. I've turned it into an 'unsigned' bitfield, and inverted its sense. I've added the flags argument to node_get_by_hex_id() too, though it does nothing there right now.
I've removed the router_get_consensus_status_by_nickname() function, since it was only used in once place.
This patch changes the warning behavior of GETINFO ns/name/<name>, since all other name lookups from the controller currently warn.
Later I'm going to add more flags, for ed25519 support. --- src/or/addressmap.c | 2 +- src/or/circuituse.c | 6 +++--- src/or/connection_edge.c | 9 +++++---- src/or/control.c | 16 ++++++++-------- src/or/networkstatus.c | 18 ++---------------- src/or/networkstatus.h | 3 --- src/or/nodelist.c | 20 ++++++++++++-------- src/or/nodelist.h | 8 ++++++-- src/or/rendservice.c | 2 +- src/or/router.c | 2 +- src/or/routerset.c | 2 +- src/test/test_routerset.c | 18 +++++++++--------- 12 files changed, 49 insertions(+), 57 deletions(-)
diff --git a/src/or/addressmap.c b/src/or/addressmap.c index c92af3825..f278564e8 100644 --- a/src/or/addressmap.c +++ b/src/or/addressmap.c @@ -214,7 +214,7 @@ addressmap_clear_excluded_trackexithosts(const or_options_t *options) dot--; if (*dot == '.') dot++; nodename = tor_strndup(dot, len-5-(dot-target));; - node = node_get_by_nickname(nodename, 0); + node = node_get_by_nickname(nodename, NNF_NO_WARN_UNNAMED); tor_free(nodename); if (!node || (allow_nodes && !routerset_contains_node(allow_nodes, node)) || diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 21cc9c540..240e25293 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -2119,7 +2119,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn, } else { /* XXXX Duplicates checks in connection_ap_handshake_attach_circuit: * refactor into a single function. */ - const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 1); + const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 0); int opt = conn->chosen_exit_optional; if (node && !connection_ap_can_use_exit(conn, node)) { log_fn(opt ? LOG_INFO : LOG_WARN, LD_APP, @@ -2199,7 +2199,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn, if (conn->chosen_exit_name) { const node_t *r; int opt = conn->chosen_exit_optional; - r = node_get_by_nickname(conn->chosen_exit_name, 1); + r = node_get_by_nickname(conn->chosen_exit_name, 0); if (r && node_has_descriptor(r)) { /* We might want to connect to an IPv6 bridge for loading descriptors so we use the preferred address rather than @@ -2598,7 +2598,7 @@ connection_ap_handshake_attach_circuit(entry_connection_t *conn) * open to that exit. See what exit we meant, and whether we can use it. */ if (conn->chosen_exit_name) { - const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 1); + const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 0); int opt = conn->chosen_exit_optional; if (!node && !want_onehop) { /* We ran into this warning when trying to extend a circuit to a diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 12ddc7e82..c42adbf73 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1074,7 +1074,8 @@ circuit_discard_optional_exit_enclaves(extend_info_t *info) if (!entry_conn->chosen_exit_optional && !entry_conn->chosen_exit_retries) continue; - r1 = node_get_by_nickname(entry_conn->chosen_exit_name, 0); + r1 = node_get_by_nickname(entry_conn->chosen_exit_name, + NNF_NO_WARN_UNNAMED); r2 = node_get_by_id(info->identity_digest); if (!r1 || !r2 || r1 != r2) continue; @@ -1508,7 +1509,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, if (s[1] != '\0') { /* Looks like a real .exit one. */ conn->chosen_exit_name = tor_strdup(s+1); - node = node_get_by_nickname(conn->chosen_exit_name, 1); + node = node_get_by_nickname(conn->chosen_exit_name, 0);
if (exit_source == ADDRMAPSRC_TRACKEXIT) { /* We 5 tries before it expires the addressmap */ @@ -1529,7 +1530,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, * form that means (foo's address).foo.exit. */
conn->chosen_exit_name = tor_strdup(socks->address); - node = node_get_by_nickname(conn->chosen_exit_name, 1); + node = node_get_by_nickname(conn->chosen_exit_name, 0); if (node) { *socks->address = 0; node_get_address_string(node, socks->address, sizeof(socks->address)); @@ -3630,7 +3631,7 @@ connection_ap_can_use_exit(const entry_connection_t *conn, */ if (conn->chosen_exit_name) { const node_t *chosen_exit = - node_get_by_nickname(conn->chosen_exit_name, 1); + node_get_by_nickname(conn->chosen_exit_name, 0); if (!chosen_exit || tor_memneq(chosen_exit->identity, exit_node->identity, DIGEST_LEN)) { /* doesn't match */ diff --git a/src/or/control.c b/src/or/control.c index 724d4b35c..889436c67 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -1886,7 +1886,7 @@ getinfo_helper_dir(control_connection_t *control_conn, (void) control_conn; if (!strcmpstart(question, "desc/id/")) { const routerinfo_t *ri = NULL; - const node_t *node = node_get_by_hex_id(question+strlen("desc/id/")); + const node_t *node = node_get_by_hex_id(question+strlen("desc/id/"), 0); if (node) ri = node->ri; if (ri) { @@ -1905,7 +1905,7 @@ getinfo_helper_dir(control_connection_t *control_conn, /* XXX Setting 'warn_if_unnamed' here is a bit silly -- the * warning goes to the user, not to the controller. */ const node_t *node = - node_get_by_nickname(question+strlen("desc/name/"), 1); + node_get_by_nickname(question+strlen("desc/name/"), 0); if (node) ri = node->ri; if (ri) { @@ -1991,7 +1991,7 @@ getinfo_helper_dir(control_connection_t *control_conn, return -1; } } else if (!strcmpstart(question, "md/id/")) { - const node_t *node = node_get_by_hex_id(question+strlen("md/id/")); + const node_t *node = node_get_by_hex_id(question+strlen("md/id/"), 0); const microdesc_t *md = NULL; if (node) md = node->md; if (md && md->body) { @@ -2000,7 +2000,7 @@ getinfo_helper_dir(control_connection_t *control_conn, } else if (!strcmpstart(question, "md/name/")) { /* XXX Setting 'warn_if_unnamed' here is a bit silly -- the * warning goes to the user, not to the controller. */ - const node_t *node = node_get_by_nickname(question+strlen("md/name/"), 1); + const node_t *node = node_get_by_nickname(question+strlen("md/name/"), 0); /* XXXX duplicated code */ const microdesc_t *md = NULL; if (node) md = node->md; @@ -2013,7 +2013,7 @@ getinfo_helper_dir(control_connection_t *control_conn, } else if (!strcmpstart(question, "desc-annotations/id/")) { const routerinfo_t *ri = NULL; const node_t *node = - node_get_by_hex_id(question+strlen("desc-annotations/id/")); + node_get_by_hex_id(question+strlen("desc-annotations/id/"), 0); if (node) ri = node->ri; if (ri) { @@ -3394,7 +3394,7 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len,
nodes = smartlist_new(); SMARTLIST_FOREACH_BEGIN(router_nicknames, const char *, n) { - const node_t *node = node_get_by_nickname(n, 1); + const node_t *node = node_get_by_nickname(n, 0); if (!node) { connection_printf_to_buf(conn, "552 No such router "%s"\r\n", n); goto done; @@ -4158,7 +4158,7 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len, const char *server;
server = arg + strlen(opt_server); - node = node_get_by_hex_id(server); + node = node_get_by_hex_id(server, 0); if (!node) { connection_printf_to_buf(conn, "552 Server "%s" not found\r\n", server); @@ -4239,7 +4239,7 @@ handle_control_hspost(control_connection_t *conn, SMARTLIST_FOREACH_BEGIN(args, const char *, arg) { if (!strcasecmpstart(arg, opt_server)) { const char *server = arg + strlen(opt_server); - const node_t *node = node_get_by_hex_id(server); + const node_t *node = node_get_by_hex_id(server, 0);
if (!node || !node->rs) { connection_printf_to_buf(conn, "552 Server "%s" not found\r\n", diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 69bff55cf..6263be36c 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -794,21 +794,6 @@ router_get_consensus_status_by_id(const char *digest) return router_get_mutable_consensus_status_by_id(digest); }
-/** Given a nickname (possibly verbose, possibly a hexadecimal digest), return - * the corresponding routerstatus_t, or NULL if none exists. Warn the - * user if <b>warn_if_unnamed</b> is set, and they have specified a router by - * nickname, but the Named flag isn't set for that router. */ -const routerstatus_t * -router_get_consensus_status_by_nickname(const char *nickname, - int warn_if_unnamed) -{ - const node_t *node = node_get_by_nickname(nickname, warn_if_unnamed); - if (node) - return node->rs; - else - return NULL; -} - /** Return the identity digest that's mapped to officially by * <b>nickname</b>. */ const char * @@ -2555,7 +2540,8 @@ getinfo_helper_networkstatus(control_connection_t *conn, } status = router_get_consensus_status_by_id(d); } else if (!strcmpstart(question, "ns/name/")) { - status = router_get_consensus_status_by_nickname(question+8, 0); + const node_t *n = node_get_by_nickname(question+8, 0); + status = n ? n->rs : NULL; } else if (!strcmpstart(question, "ns/purpose/")) { *answer = networkstatus_getinfo_by_purpose(question+11, time(NULL)); return *answer ? 0 : -1; diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h index f9320747d..145be37d1 100644 --- a/src/or/networkstatus.h +++ b/src/or/networkstatus.h @@ -62,9 +62,6 @@ const routerstatus_t *router_get_consensus_status_by_descriptor_digest( MOCK_DECL(routerstatus_t *, router_get_mutable_consensus_status_by_descriptor_digest, (networkstatus_t *consensus, const char *digest)); -const routerstatus_t *router_get_consensus_status_by_nickname( - const char *nickname, - int warn_if_unnamed); const char *networkstatus_get_router_digest_by_nickname(const char *nickname); int networkstatus_nickname_is_unnamed(const char *nickname); int we_want_to_fetch_flavor(const or_options_t *options, int flavor); diff --git a/src/or/nodelist.c b/src/or/nodelist.c index b1cb8d6b5..f7cae9281 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -762,14 +762,16 @@ nodelist_get_list,(void)) /** Given a hex-encoded nickname of the format DIGEST, $DIGEST, $DIGEST=name, * or $DIGEST~name, return the node with the matching identity digest and * nickname (if any). Return NULL if no such node exists, or if <b>hex_id</b> - * is not well-formed. */ + * is not well-formed. DOCDOC flags */ const node_t * -node_get_by_hex_id(const char *hex_id) +node_get_by_hex_id(const char *hex_id, unsigned flags) { char digest_buf[DIGEST_LEN]; char nn_buf[MAX_NICKNAME_LEN+1]; char nn_char='\0';
+ (void) flags; // XXXX + if (hex_digest_nickname_decode(hex_id, digest_buf, &nn_char, nn_buf)==0) { const node_t *node = node_get_by_id(digest_buf); if (!node) @@ -785,19 +787,21 @@ node_get_by_hex_id(const char *hex_id) }
/** Given a nickname (possibly verbose, possibly a hexadecimal digest), return - * the corresponding node_t, or NULL if none exists. Warn the user if - * <b>warn_if_unnamed</b> is set, and they have specified a router by - * nickname, but the Named flag isn't set for that router. */ + * the corresponding node_t, or NULL if none exists. Warn the user if they + * have specified a router by nickname, unless the NNF_NO_WARN_UNNAMED bit is + * set in <b>flags</b>. */ MOCK_IMPL(const node_t *, -node_get_by_nickname,(const char *nickname, int warn_if_unnamed)) +node_get_by_nickname,(const char *nickname, unsigned flags)) { + const int warn_if_unnamed = !(flags & NNF_NO_WARN_UNNAMED); + if (!the_nodelist) return NULL;
/* Handle these cases: DIGEST, $DIGEST, $DIGEST=name, $DIGEST~name. */ { const node_t *node; - if ((node = node_get_by_hex_id(nickname)) != NULL) + if ((node = node_get_by_hex_id(nickname, flags)) != NULL) return node; }
@@ -1719,7 +1723,7 @@ nodelist_add_node_and_family(smartlist_t *sl, const node_t *node) SMARTLIST_FOREACH_BEGIN(declared_family, const char *, name) { const node_t *node2; const smartlist_t *family2; - if (!(node2 = node_get_by_nickname(name, 0))) + if (!(node2 = node_get_by_nickname(name, NNF_NO_WARN_UNNAMED))) continue; if (!(family2 = node_get_declared_family(node2))) continue; diff --git a/src/or/nodelist.h b/src/or/nodelist.h index c5805c55f..79ff720c9 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -21,7 +21,11 @@ MOCK_DECL(const node_t *, node_get_by_id, (const char *identity_digest)); node_t *node_get_mutable_by_ed25519_id(const ed25519_public_key_t *ed_id); MOCK_DECL(const node_t *, node_get_by_ed25519_id, (const ed25519_public_key_t *ed_id)); -const node_t *node_get_by_hex_id(const char *identity_digest); + +#define NNF_NO_WARN_UNNAMED (1u<<0) + +const node_t *node_get_by_hex_id(const char *identity_digest, + unsigned flags); node_t *nodelist_set_routerinfo(routerinfo_t *ri, routerinfo_t **ri_old_out); node_t *nodelist_add_microdesc(microdesc_t *md); void nodelist_set_consensus(networkstatus_t *ns); @@ -35,7 +39,7 @@ void nodelist_free_all(void); void nodelist_assert_ok(void);
MOCK_DECL(const node_t *, node_get_by_nickname, - (const char *nickname, int warn_if_unnamed)); + (const char *nickname, unsigned flags)); void node_get_verbose_nickname(const node_t *node, char *verbose_name_out); void node_get_verbose_nickname_by_id(const char *id_digest, diff --git a/src/or/rendservice.c b/src/or/rendservice.c index a205b00c6..925373883 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -2112,7 +2112,7 @@ find_rp_for_intro(const rend_intro_cell_t *intro, if (intro->version == 0 || intro->version == 1) { rp_nickname = (const char *)(intro->u.v0_v1.rp);
- node = node_get_by_nickname(rp_nickname, 0); + node = node_get_by_nickname(rp_nickname, NNF_NO_WARN_UNNAMED); if (!node) { if (err_msg_out) { tor_asprintf(&err_msg, diff --git a/src/or/router.c b/src/or/router.c index 7fad57265..8629168c8 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2279,7 +2279,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) if (!strcasecmp(name, options->Nickname)) continue; /* Don't list ourself, that's redundant */ else - member = node_get_by_nickname(name, 1); + member = node_get_by_nickname(name, 0); if (!member) { int is_legal = is_legal_nickname_or_hexdigest(name); if (!smartlist_contains_string(warned_nonexistent_family, name) && diff --git a/src/or/routerset.c b/src/or/routerset.c index 4906c6a51..54e26ef94 100644 --- a/src/or/routerset.c +++ b/src/or/routerset.c @@ -362,7 +362,7 @@ routerset_get_all_nodes(smartlist_t *out, const routerset_t *routerset, /* No routers are specified by type; all are given by name or digest. * we can do a lookup in O(len(routerset)). */ SMARTLIST_FOREACH(routerset->list, const char *, name, { - const node_t *node = node_get_by_nickname(name, 1); + const node_t *node = node_get_by_nickname(name, 0); if (node) { if (!running_only || node->is_running) if (!routerset_contains_node(excludeset, node)) diff --git a/src/test/test_routerset.c b/src/test/test_routerset.c index 7efd042ed..c9c69911d 100644 --- a/src/test/test_routerset.c +++ b/src/test/test_routerset.c @@ -1602,7 +1602,7 @@ NS(test_main)(void *arg) */
NS_DECL(const node_t *, node_get_by_nickname, - (const char *nickname, int warn_if_unused)); + (const char *nickname, unsigned flags)); static const char *NS(mock_nickname);
static void @@ -1632,11 +1632,11 @@ NS(test_main)(void *arg) }
const node_t * -NS(node_get_by_nickname)(const char *nickname, int warn_if_unused) +NS(node_get_by_nickname)(const char *nickname, unsigned flags) { CALLED(node_get_by_nickname)++; tt_str_op(nickname, OP_EQ, NS(mock_nickname)); - tt_int_op(warn_if_unused, OP_EQ, 1); + tt_uint_op(flags, OP_EQ, 0);
done: return NULL; @@ -1651,7 +1651,7 @@ NS(node_get_by_nickname)(const char *nickname, int warn_if_unused) */
NS_DECL(const node_t *, node_get_by_nickname, - (const char *nickname, int warn_if_unused)); + (const char *nickname, unsigned flags)); static const char *NS(mock_nickname); static node_t NS(mock_node);
@@ -1683,11 +1683,11 @@ NS(test_main)(void *arg) }
const node_t * -NS(node_get_by_nickname)(const char *nickname, int warn_if_unused) +NS(node_get_by_nickname)(const char *nickname, unsigned flags) { CALLED(node_get_by_nickname)++; tt_str_op(nickname, OP_EQ, NS(mock_nickname)); - tt_int_op(warn_if_unused, OP_EQ, 1); + tt_int_op(flags, OP_EQ, 0);
done: return &NS(mock_node); @@ -1701,7 +1701,7 @@ NS(node_get_by_nickname)(const char *nickname, int warn_if_unused) */
NS_DECL(const node_t *, node_get_by_nickname, - (const char *nickname, int warn_if_unused)); + (const char *nickname, unsigned flags)); static char *NS(mock_nickname); static node_t NS(mock_node);
@@ -1735,11 +1735,11 @@ NS(test_main)(void *arg) }
const node_t * -NS(node_get_by_nickname)(const char *nickname, int warn_if_unused) +NS(node_get_by_nickname)(const char *nickname, unsigned flags) { CALLED(node_get_by_nickname)++; tt_str_op(nickname, OP_EQ, NS(mock_nickname)); - tt_int_op(warn_if_unused, OP_EQ, 1); + tt_int_op(flags, OP_EQ, 0);
done: return &NS(mock_node);