r8838@totoro: nickm | 2006-10-02 15:24:39 -0400

 Partial implementation of revised nickname syntax for controllers.  Implement ability to look up routers by "verbose" nicknames; add a per-v1-control-connection flag to turn the feature on in events.  Needs testing, spec, ability to actually turn on the flag, double-checking that we wont overflow any nickname buffers, and changelog.


svn:r8582

6 files changed, 226 insertions, 46 deletions

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index f2eda22dca..6df6748257 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -159,6 +159,49 @@ circuit_list_path(origin_circuit_t *circ, int verbose)
   return s;
 }
 
+/* DOCDOC long names only */
+char *
+circuit_list_path_for_controller(origin_circuit_t *circ)
+{
+  smartlist_t *elements = smartlist_create();
+  crypt_path_t *hop;
+  char *elt, *s;
+  routerinfo_t *ri;
+
+  hop = circ->cpath;
+  do {
+    if (!hop)
+      break;
+    if (hop->state != CPATH_STATE_OPEN)
+      break;
+    if (!hop->extend_info)
+      break;
+    elt = tor_malloc(MAX_VERBOSE_NICKNAME_LEN+1);
+    if ((ri = router_get_by_digest(hop->extend_info->identity_digest))) {
+      router_get_verbose_nickname(elt, ri);
+    } else if (hop->extend_info->nickname &&
+               is_legal_nickname(hop->extend_info->nickname)) {
+      elt[0] = '$';
+      base16_encode(elt+1, HEX_DIGEST_LEN+1,
+                    hop->extend_info->identity_digest, DIGEST_LEN);
+      elt[HEX_DIGEST_LEN+1]= '~';
+      strlcpy(elt+HEX_DIGEST_LEN+2,
+              hop->extend_info->nickname, MAX_NICKNAME_LEN+1);
+    } else {
+      elt[0] = '$';
+      base16_encode(elt+1, HEX_DIGEST_LEN+1,
+                    hop->extend_info->identity_digest, DIGEST_LEN);
+    }
+    smartlist_add(elements, elt);
+    hop = hop->next;
+  } while (hop != circ->cpath);
+
+  s = smartlist_join_strings(elements, ",", 0, NULL);
+  SMARTLIST_FOREACH(elements, char*, cp, tor_free(cp));
+  smartlist_free(elements);
+  return s;
+}
+
 /** Log, at severity <b>severity</b>, the nicknames of each router in
  * circ's cpath. Also log the length of the cpath, and the intended
  * exit point.
diff --git a/src/or/control.c b/src/or/control.c
index 1794b366b5..e37f951408 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -111,7 +111,8 @@ static const char * CONTROL0_COMMANDS[_CONTROL0_CMD_MAX_RECOGNIZED+1] = {
  * list to find out.
  **/
 static uint32_t global_event_mask0 = 0;
-static uint32_t global_event_mask1 = 0;
+static uint32_t global_event_mask1long = 0;
+static uint32_t global_event_mask1short = 0;
 
 /** True iff we have disabled log messages from being sent to the controller */
 static int disable_log_messages = 0;
@@ -119,9 +120,13 @@ static int disable_log_messages = 0;
 /** Macro: true if any control connection is interested in events of type
  * <b>e</b>. */
 #define EVENT_IS_INTERESTING0(e) (global_event_mask0 & (1<<(e)))
-#define EVENT_IS_INTERESTING1(e) (global_event_mask1 & (1<<(e)))
-#define EVENT_IS_INTERESTING(e) \
-  ((global_event_mask0|global_event_mask1) & (1<<(e)))
+#define EVENT_IS_INTERESTING1(e) \
+  ((global_event_mask1long|global_event_mask1short) & (1<<(e)))
+#define EVENT_IS_INTERESTING1L(e) (global_event_mask1long & (1<<(e)))
+#define EVENT_IS_INTERESTING1S(e) (global_event_mask1short & (1<<(e)))
+#define EVENT_IS_INTERESTING(e)                                         \
+  ((global_event_mask0|global_event_mask1short|global_event_mask1long)  \
+   & (1<<(e)))
 
 /** If we're using cookie-type authentication, how long should our cookies be?
  */
@@ -132,6 +137,10 @@ static int disable_log_messages = 0;
 static int authentication_cookie_is_set = 0;
 static char authentication_cookie[AUTHENTICATION_COOKIE_LEN];
 
+typedef enum {
+  SHORT_NAMES,LONG_NAMES,ALL_NAMES
+} name_type_t;
+
 static void connection_printf_to_buf(control_connection_t *conn,
                                      const char *format, ...)
   CHECK_PRINTF(2,3);
@@ -148,8 +157,9 @@ static void send_control0_error(control_connection_t *conn, uint16_t error,
                                const char *message);
 static void send_control0_event(uint16_t event, uint32_t len,
                                 const char *body);
-static void send_control1_event(uint16_t event, const char *format, ...)
-  CHECK_PRINTF(2,3);
+static void send_control1_event(uint16_t event, name_type_t which,
+                                const char *format, ...)
+  CHECK_PRINTF(3,4);
 static int handle_control_setconf(control_connection_t *conn, uint32_t len,
                                   char *body);
 static int handle_control_resetconf(control_connection_t *conn, uint32_t len,
@@ -238,7 +248,8 @@ control_update_global_event_mask(void)
   connection_t **conns;
   int n_conns, i;
   global_event_mask0 = 0;
-  global_event_mask1 = 0;
+  global_event_mask1short = 0;
+  global_event_mask1long = 0;
   get_connection_array(&conns, &n_conns);
   for (i = 0; i < n_conns; ++i) {
     if (conns[i]->type == CONN_TYPE_CONTROL &&
@@ -246,8 +257,10 @@ control_update_global_event_mask(void)
       control_connection_t *conn = TO_CONTROL_CONN(conns[i]);
       if (STATE_IS_V0(conn->_base.state))
         global_event_mask0 |= conn->event_mask;
+      else if (conn->use_long_names)
+        global_event_mask1long |= conn->event_mask;
       else
-        global_event_mask1 |= conn->event_mask;
+        global_event_mask1short |= conn->event_mask;
     }
   }
 
@@ -587,7 +600,7 @@ send_control0_event(uint16_t event, uint32_t len, const char *body)
 /* Send an event to all v1 controllers that are listening for code
  * <b>event</b>.  The event's body is given by <b>msg</b>. */
 static void
-send_control1_event_string(uint16_t event, const char *msg)
+send_control1_event_string(uint16_t event, name_type_t which, const char *msg)
 {
   connection_t **conns;
   int n_conns, i;
@@ -600,6 +613,13 @@ send_control1_event_string(uint16_t event, const char *msg)
         !conns[i]->marked_for_close &&
         conns[i]->state == CONTROL_CONN_STATE_OPEN_V1) {
       control_connection_t *control_conn = TO_CONTROL_CONN(conns[i]);
+      if (which == SHORT_NAMES) {
+        if (control_conn->use_long_names)
+          continue;
+      } else if (which == LONG_NAMES) {
+        if (! control_conn->use_long_names)
+          continue;
+      }
       if (control_conn->event_mask & (1<<event)) {
         connection_write_to_buf(msg, strlen(msg), TO_CONN(control_conn));
         if (event == EVENT_ERR_MSG)
@@ -616,7 +636,7 @@ send_control1_event_string(uint16_t event, const char *msg)
  * Currently the length of the message is limited to 1024 (including the
  * ending \n\r\0. */
 static void
-send_control1_event(uint16_t event, const char *format, ...)
+send_control1_event(uint16_t event, name_type_t which, const char *format, ...)
 {
 #define SEND_CONTROL1_EVENT_BUFFERSIZE 1024
   int r;
@@ -636,7 +656,7 @@ send_control1_event(uint16_t event, const char *format, ...)
     buf[SEND_CONTROL1_EVENT_BUFFERSIZE-3] = '\r';
   }
 
-  send_control1_event_string(event, buf);
+  send_control1_event_string(event, which, buf);
 }
 
 /** Given a text circuit <b>id</b>, return the corresponding circuit. */
@@ -2661,12 +2681,14 @@ connection_control_process_inbuf(control_connection_t *conn)
 int
 control_event_circuit_status(origin_circuit_t *circ, circuit_status_event_t tp)
 {
-  char *path, *msg;
+  char *path=NULL, *msg;
   if (!EVENT_IS_INTERESTING(EVENT_CIRCUIT_STATUS))
     return 0;
   tor_assert(circ);
 
-  path = circuit_list_path(circ,0);
+  if (EVENT_IS_INTERESTING0(EVENT_CIRCUIT_STATUS) ||
+      EVENT_IS_INTERESTING1S(EVENT_CIRCUIT_STATUS))
+    path = circuit_list_path(circ,0);
   if (EVENT_IS_INTERESTING0(EVENT_CIRCUIT_STATUS)) {
     size_t path_len = strlen(path);
     msg = tor_malloc(1+4+path_len+1); /* event, circid, path, NUL. */
@@ -2690,10 +2712,20 @@ control_event_circuit_status(origin_circuit_t *circ, circuit_status_event_t tp)
         log_warn(LD_BUG, "Unrecognized status code %d", (int)tp);
         return 0;
       }
-    send_control1_event(EVENT_CIRCUIT_STATUS,
-                        "650 CIRC %lu %s %s\r\n",
-                        (unsigned long)circ->global_identifier,
-                        status, path);
+    if (EVENT_IS_INTERESTING1S(EVENT_CIRCUIT_STATUS)) {
+      send_control1_event(EVENT_CIRCUIT_STATUS, SHORT_NAMES,
+                          "650 CIRC %lu %s %s\r\n",
+                          (unsigned long)circ->global_identifier,
+                          status, path);
+    }
+    if (EVENT_IS_INTERESTING1L(EVENT_CIRCUIT_STATUS)) {
+      char *vpath = circuit_list_path_for_controller(circ);
+      send_control1_event(EVENT_CIRCUIT_STATUS, LONG_NAMES,
+                          "650 CIRC %lu %s %s\r\n",
+                          (unsigned long)circ->global_identifier,
+                          status, vpath);
+      tor_free(vpath);
+    }
   }
   tor_free(path);
 
@@ -2766,7 +2798,7 @@ control_event_stream_status(edge_connection_t *conn, stream_status_event_t tp)
     circ = circuit_get_by_edge_conn(conn);
     if (circ && CIRCUIT_IS_ORIGIN(circ))
       origin_circ = TO_ORIGIN_CIRCUIT(circ);
-    send_control1_event(EVENT_STREAM_STATUS,
+    send_control1_event(EVENT_STREAM_STATUS, ALL_NAMES,
                         "650 STREAM %lu %s %lu %s\r\n",
                         (unsigned long)conn->global_identifier, status,
                         origin_circ?
@@ -2813,7 +2845,7 @@ control_event_or_conn_status(or_connection_t *conn,or_conn_status_event_t tp)
         log_warn(LD_BUG, "Unrecognized status code %d", (int)tp);
         return 0;
       }
-    send_control1_event(EVENT_OR_CONN_STATUS,
+    send_control1_event(EVENT_OR_CONN_STATUS, ALL_NAMES/*XXXX NM*/,
                         "650 ORCONN %s %s\r\n",
                         name, status);
   }
@@ -2833,7 +2865,7 @@ control_event_bandwidth_used(uint32_t n_read, uint32_t n_written)
     send_control0_event(EVENT_BANDWIDTH_USED, 8, buf);
   }
   if (EVENT_IS_INTERESTING1(EVENT_BANDWIDTH_USED)) {
-    send_control1_event(EVENT_BANDWIDTH_USED,
+    send_control1_event(EVENT_BANDWIDTH_USED, ALL_NAMES,
                         "650 BW %lu %lu\r\n",
                         (unsigned long)n_read,
                         (unsigned long)n_written);
@@ -2907,7 +2939,7 @@ control_event_logmsg(int severity, unsigned int domain, const char *msg)
       default: s = "UnknownLogSeverity"; break;
     }
     ++disable_log_messages;
-    send_control1_event(event, "650 %s %s\r\n", s, b?b:msg);
+    send_control1_event(event, ALL_NAMES, "650 %s %s\r\n", s, b?b:msg);
     --disable_log_messages;
     tor_free(b);
   }
@@ -2922,34 +2954,55 @@ control_event_descriptors_changed(smartlist_t *routers)
 {
   size_t len;
   char *msg;
-  smartlist_t *identities;
+  smartlist_t *identities = NULL;
   char buf[HEX_DIGEST_LEN+1];
 
   if (!EVENT_IS_INTERESTING(EVENT_NEW_DESC))
     return 0;
-  identities = smartlist_create();
-  SMARTLIST_FOREACH(routers, routerinfo_t *, r,
-  {
-    base16_encode(buf,sizeof(buf),r->cache_info.identity_digest,DIGEST_LEN);
-    smartlist_add(identities, tor_strdup(buf));
-  });
+  if (EVENT_IS_INTERESTING0(EVENT_NEW_DESC) ||
+      EVENT_IS_INTERESTING1S(EVENT_NEW_DESC)) {
+    identities = smartlist_create();
+    SMARTLIST_FOREACH(routers, routerinfo_t *, r,
+    {
+      base16_encode(buf,sizeof(buf),r->cache_info.identity_digest,DIGEST_LEN);
+      smartlist_add(identities, tor_strdup(buf));
+    });
+  }
   if (EVENT_IS_INTERESTING0(EVENT_NEW_DESC)) {
     msg = smartlist_join_strings(identities, ",", 0, &len);
     send_control0_event(EVENT_NEW_DESC, len+1, msg);
     tor_free(msg);
   }
-  if (EVENT_IS_INTERESTING1(EVENT_NEW_DESC)) {
+  if (EVENT_IS_INTERESTING1S(EVENT_NEW_DESC)) {
     char *ids = smartlist_join_strings(identities, " ", 0, &len);
     size_t len = strlen(ids)+32;
     msg = tor_malloc(len);
     tor_snprintf(msg, len, "650 NEWDESC %s\r\n", ids);
-    send_control1_event_string(EVENT_NEW_DESC, msg);
+    send_control1_event_string(EVENT_NEW_DESC, SHORT_NAMES, msg);
     tor_free(ids);
     tor_free(msg);
   }
-  SMARTLIST_FOREACH(identities, char *, cp, tor_free(cp));
-  smartlist_free(identities);
-
+  if (EVENT_IS_INTERESTING1L(EVENT_NEW_DESC)) {
+    smartlist_t *names;
+    names = smartlist_create();
+    SMARTLIST_FOREACH(routers, routerinfo_t *, ri, {
+        char *b = tor_malloc(MAX_VERBOSE_NICKNAME_LEN+1);
+        smartlist_add(names, b);
+      });
+    char *ids = smartlist_join_strings(names, " ", 0, &len);
+    size_t len = strlen(ids)+32;
+    msg = tor_malloc(len);
+    tor_snprintf(msg, len, "650 NEWDESC %s\r\n", ids);
+    send_control1_event_string(EVENT_NEW_DESC, LONG_NAMES, msg);
+    tor_free(ids);
+    tor_free(msg);
+    SMARTLIST_FOREACH(names, char *, cp, tor_free(cp));
+    smartlist_free(names);
+  }
+  if (identities) {
+    SMARTLIST_FOREACH(identities, char *, cp, tor_free(cp));
+    smartlist_free(identities);
+  }
   return 0;
 }
 
@@ -2962,12 +3015,13 @@ control_event_address_mapped(const char *from, const char *to, time_t expires)
     return 0;
 
   if (expires < 3)
-    send_control1_event(EVENT_ADDRMAP,
+    send_control1_event(EVENT_ADDRMAP, ALL_NAMES,
                         "650 ADDRMAP %s %s NEVER\r\n", from, to);
   else {
     char buf[ISO_TIME_LEN+1];
     format_local_iso_time(buf,expires);
-    send_control1_event(EVENT_ADDRMAP, "650 ADDRMAP %s %s \"%s\"\r\n",
+    send_control1_event(EVENT_ADDRMAP, ALL_NAMES,
+                        "650 ADDRMAP %s %s \"%s\"\r\n",
                         from, to, buf);
   }
 
@@ -3005,7 +3059,7 @@ control_event_or_authdir_new_descriptor(const char *action,
   buf = tor_malloc(totallen);
   strlcpy(buf, firstline, totallen);
   strlcpy(buf+strlen(firstline), esc, totallen);
-  send_control1_event_string(EVENT_AUTHDIR_NEWDESCS, buf);
+  send_control1_event_string(EVENT_AUTHDIR_NEWDESCS, ALL_NAMES, buf);
 
   tor_free(esc);
   tor_free(buf);
@@ -3018,7 +3072,7 @@ control_event_or_authdir_new_descriptor(const char *action,
 int
 control_event_my_descriptor_changed(void)
 {
-  send_control1_event(EVENT_DESCCHANGED, "650 DESCCHANGED\r\n");
+  send_control1_event(EVENT_DESCCHANGED, ALL_NAMES, "650 DESCCHANGED\r\n");
   return 0;
 }
 
diff --git a/src/or/or.h b/src/or/or.h
index f04fcca455..3cdadd3cfc 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -157,6 +157,9 @@
 #define MAX_NICKNAME_LEN 19
 /* Hex digest plus dollar sign. */
 #define MAX_HEX_NICKNAME_LEN (HEX_DIGEST_LEN+1)
+/* $Hexdigest=nickname */
+#define MAX_VERBOSE_NICKNAME_LEN (1+HEX_DIGEST_LEN+1+MAX_NICKNAME_LEN)
+
 /** Maximum size, in bytes, for resized buffers. */
 #define MAX_BUF_SIZE ((1<<24)-1)
 #define MAX_DIR_SIZE MAX_BUF_SIZE
@@ -783,6 +786,9 @@ typedef struct control_connection_t {
 
   uint32_t event_mask; /**< Bitfield: which events does this controller
                         * care about? */
+  unsigned int use_long_names:1; /**< True if we should use long nicknames
+                                  * on this (v1) connection. Only settable
+                                  * via v1 controllers. */
   uint32_t incoming_cmd_len;
   uint32_t incoming_cmd_cur_len;
   char *incoming_cmd;
@@ -1680,6 +1686,7 @@ void assert_buf_ok(buf_t *buf);
 /********************************* circuitbuild.c **********************/
 
 char *circuit_list_path(origin_circuit_t *circ, int verbose);
+char *circuit_list_path_for_controller(origin_circuit_t *circ);
 void circuit_log_path(int severity, unsigned int domain,
                       origin_circuit_t *circ);
 void circuit_rep_hist_note_result(origin_circuit_t *circ);
@@ -2504,6 +2511,7 @@ int router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
 int is_legal_nickname(const char *s);
 int is_legal_nickname_or_hexdigest(const char *s);
 int is_legal_hexdigest(const char *s);
+void router_get_verbose_nickname(char *buf, routerinfo_t *router);
 void router_reset_warnings(void);
 void router_free_all(void);
 
@@ -2547,6 +2555,7 @@ typedef enum {
   V1_AUTHORITY, V2_AUTHORITY, HIDSERV_AUTHORITY,
 } authority_type_t;
 routerstatus_t *router_pick_trusteddirserver(authority_type_t type,
+
                                              int requireother,
                                              int fascistfirewall,
                                              int retry_if_no_servers);
diff --git a/src/or/router.c b/src/or/router.c
index b139440f53..47897669da 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1306,8 +1306,28 @@ is_legal_hexdigest(const char *s)
   tor_assert(s);
   if (s[0] == '$') s++;
   len = strlen(s);
-  return (len == HEX_DIGEST_LEN &&
-          strspn(s,HEX_CHARACTERS)==len);
+  if (len > HEX_DIGEST_LEN) {
+    if (s[HEX_DIGEST_LEN] == '=' ||
+        s[HEX_DIGEST_LEN] == '~') {
+      if (!is_legal_nickname(s+HEX_DIGEST_LEN+1))
+        return 0;
+    } else {
+      return 0;
+    }
+  }
+  return (len >= HEX_DIGEST_LEN &&
+          strspn(s,HEX_CHARACTERS)==HEX_DIGEST_LEN);
+}
+
+/** DOCDOC buf must have MAX_VERBOSE_NICKNAME_LEN+1 bytes. */
+void
+router_get_verbose_nickname(char *buf, routerinfo_t *router)
+{
+  buf[0] = '$';
+  base16_encode(buf+1, HEX_DIGEST_LEN+1, router->cache_info.identity_digest,
+                DIGEST_LEN);
+  buf[1+HEX_DIGEST_LEN] = router->is_named ? '=' : '~';
+  strlcpy(buf+1+HEX_DIGEST_LEN+1, router->nickname, MAX_NICKNAME_LEN+1);
 }
 
 /** Forget that we have issued any router-related warnings, so that we'll
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index b36fa82d53..022341ee95 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1033,12 +1033,24 @@ static INLINE int
 router_hex_digest_matches(routerinfo_t *router, const char *hexdigest)
 {
   char digest[DIGEST_LEN];
+  size_t len;
   tor_assert(hexdigest);
   if (hexdigest[0] == '$')
     ++hexdigest;
 
-  if (strlen(hexdigest) != HEX_DIGEST_LEN ||
-      base16_decode(digest, DIGEST_LEN, hexdigest, HEX_DIGEST_LEN)<0)
+  len = strlen(hexdigest);
+  if (len < HEX_DIGEST_LEN)
+    return 0;
+  else if (len > HEX_DIGEST_LEN &&
+           (hexdigest[HEX_DIGEST_LEN] == '=' ||
+            hexdigest[HEX_DIGEST_LEN] == '~')) {
+    if (strcasecmp(hexdigest+HEX_DIGEST_LEN+1, router->nickname))
+      return 0;
+    if (hexdigest[HEX_DIGEST_LEN] == '=' && !router->is_named)
+      return 0;
+  }
+
+  if (base16_decode(digest, DIGEST_LEN, hexdigest, HEX_DIGEST_LEN)<0)
     return 0;
   return (!memcmp(digest, router->cache_info.identity_digest, DIGEST_LEN));
 }
@@ -1079,7 +1091,7 @@ router_get_by_nickname(const char *nickname, int warn_if_unnamed)
       !strcasecmp(nickname, get_options()->Nickname))
     return router_get_my_routerinfo();
 
-  maybedigest = (strlen(nickname) == HEX_DIGEST_LEN) &&
+  maybedigest = (strlen(nickname) >= HEX_DIGEST_LEN) &&
     (base16_decode(digest,DIGEST_LEN,nickname,HEX_DIGEST_LEN) == 0);
 
   if (named_server_map &&
@@ -1096,7 +1108,10 @@ router_get_by_nickname(const char *nickname, int warn_if_unnamed)
     } else if (maybedigest &&
                !memcmp(digest, router->cache_info.identity_digest, DIGEST_LEN)
                ) {
-      return router;
+      if (router_hex_digest_matches(router, nickname))
+        return router;
+      else
+        best_match = router; // XXXX NM not exactly right.
     }
   });
 
@@ -1190,17 +1205,35 @@ routerinfo_t *
 router_get_by_hexdigest(const char *hexdigest)
 {
   char digest[DIGEST_LEN];
+  size_t len;
+  routerinfo_t *ri;
 
   tor_assert(hexdigest);
   if (!routerlist)
     return NULL;
   if (hexdigest[0]=='$')
     ++hexdigest;
-  if (strlen(hexdigest) != HEX_DIGEST_LEN ||
+  len = strlen(hexdigest);
+  if (len < HEX_DIGEST_LEN ||
       base16_decode(digest,DIGEST_LEN,hexdigest,HEX_DIGEST_LEN) < 0)
     return NULL;
 
-  return router_get_by_digest(digest);
+  ri = router_get_by_digest(digest);
+
+  if (len > HEX_DIGEST_LEN) {
+    if (hexdigest[HEX_DIGEST_LEN] == '=') {
+      if (strcasecmp(ri->nickname, hexdigest+HEX_DIGEST_LEN+1) ||
+          !ri->is_named)
+        return NULL;
+    } else if (hexdigest[HEX_DIGEST_LEN] == '~') {
+      if (strcasecmp(ri->nickname, hexdigest+HEX_DIGEST_LEN+1))
+        return NULL;
+    } else {
+      return NULL;
+    }
+  }
+
+  return ri;
 }
 
 /** Return the router in our routerlist whose 20-byte key digest
diff --git a/src/or/test.c b/src/or/test.c
index 4f9d79ccf5..147c5c209a 100644
--- a/src/or/test.c
+++ b/src/or/test.c
@@ -1269,19 +1269,40 @@ test_dir_format(void)
   test_assert( is_legal_nickname("a"));
   test_assert(!is_legal_nickname(""));
   test_assert(!is_legal_nickname("abcdefghijklmnopqrst")); /* 20 chars */
-  test_assert(!is_legal_nickname("abcdefghijklmnopqrst")); /* 20 chars */
   test_assert(!is_legal_nickname("hyphen-")); /* bad char */
   test_assert( is_legal_nickname("abcdefghijklmnopqrs")); /* 19 chars */
   test_assert(!is_legal_nickname("$AAAAAAAA01234AAAAAAAAAAAAAAAAAAAAAAAAAAA"));
   /* valid */
   test_assert( is_legal_nickname_or_hexdigest(
                                  "$AAAAAAAA01234AAAAAAAAAAAAAAAAAAAAAAAAAAA"));
+  test_assert( is_legal_nickname_or_hexdigest(
+                         "$AAAAAAAA01234AAAAAAAAAAAAAAAAAAAAAAAAAAA=fred"));
+  test_assert( is_legal_nickname_or_hexdigest(
+                         "$AAAAAAAA01234AAAAAAAAAAAAAAAAAAAAAAAAAAA~fred"));
   /* too short */
   test_assert(!is_legal_nickname_or_hexdigest(
                                  "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"));
   /* illegal char */
   test_assert(!is_legal_nickname_or_hexdigest(
                                  "$AAAAAAzAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"));
+  /* hex part too long */
+  test_assert(!is_legal_nickname_or_hexdigest(
+                         "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"));
+  test_assert(!is_legal_nickname_or_hexdigest(
+                         "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=fred"));
+  /* Bad nickname */
+  test_assert(!is_legal_nickname_or_hexdigest(
+                         "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="));
+  test_assert(!is_legal_nickname_or_hexdigest(
+                         "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~"));
+  test_assert(!is_legal_nickname_or_hexdigest(
+                       "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~hyphen-"));
+  test_assert(!is_legal_nickname_or_hexdigest(
+                       "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~"
+                       "abcdefghijklmnoppqrst"));
+  /* Bad extra char. */
+  test_assert(!is_legal_nickname_or_hexdigest(
+                         "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA!"));
   test_assert(is_legal_nickname_or_hexdigest("xyzzy"));
   test_assert(is_legal_nickname_or_hexdigest("abcdefghijklmnopqrs"));
   test_assert(!is_legal_nickname_or_hexdigest("abcdefghijklmnopqrst"));