r12761@catbus: nickm | 2007-05-15 03:13:52 -0400

 Be a lot smarter when parsing lists of routers and extrainfos.


svn:r10193

5 files changed, 94 insertions, 111 deletions

diff --git a/src/or/control.c b/src/or/control.c
index 355bad6e2..3e73510ce 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -3032,7 +3032,7 @@ control_event_address_mapped(const char *from, const char *to, time_t expires)
  * been done with it, and also optionally give an explanation/reason. */
 int
 control_event_or_authdir_new_descriptor(const char *action,
-                                        const char *descriptor,
+                                        signed_descriptor_t *desc,
                                         const char *msg)
 {
   char firstline[1024];
@@ -3050,7 +3050,8 @@ control_event_or_authdir_new_descriptor(const char *action,
                msg ? msg : "");
 
   /* Escape the server descriptor properly */
-  esclen = write_escaped_data(descriptor, strlen(descriptor), 1, &esc);
+  esclen = write_escaped_data(desc->signed_descriptor_body,
+                              desc->signed_descriptor_len, 1, &esc);
 
   totallen = strlen(firstline) + esclen + 1;
   buf = tor_malloc(totallen);
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index 2ca3be266..d977bf5e7 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -51,6 +51,8 @@ static int dirserv_thinks_router_is_reachable(routerinfo_t *router,
                                               time_t now);
 static void clear_cached_dir(cached_dir_t *d);
 
+static int dirserv_add_extrainfo(extrainfo_t *ei, const char **msg);
+
 /************** Fingerprint handling code ************/
 
 #define FP_NAMED   1  /**< Listed in fingerprint file. */
@@ -521,84 +523,56 @@ dirserv_add_multiple_descriptors(const char *desc, const char **msg)
   int r=100; /* higher than any actual return value. */
   int r_tmp;
   const char *msg_out;
+  smartlist_t *list;
+  const char *s;
+  int n_parsed = 0;
+
+  s = desc;
+  list = smartlist_create();
+  if (!router_parse_list_from_string(&s, list, SAVED_NOWHERE, 0)) {
+    SMARTLIST_FOREACH(list, routerinfo_t *, ri, {
+        r_tmp = dirserv_add_descriptor(ri, &msg_out);
+        if (r_tmp < r) {
+          r = r_tmp;
+          *msg = msg_out;
+        }
+      });
+  }
+  n_parsed += smartlist_len(list);
+  smartlist_clear(list);
 
-  while (desc && *desc) {
-    const char *eos = strstr(desc, "\nrouter-signature");
-    const char *next = NULL;
-    if (eos) {
-      char *next_extra = strstr(eos, "\nextra-info");
-      char *next_routerinfo = strstr(eos, "\nrouter ");
-      if (next_extra)
-        next = next_extra;
-      if (!next || (next_routerinfo && next_routerinfo < next))
-        next = next_routerinfo;
-    }
-    if (next)
-      ++next;
-
-    r_tmp = dirserv_add_descriptor(desc, next, &msg_out);
-    desc = next;
-
-    if (r_tmp < r) {
-      r = r_tmp;
-      *msg = msg_out;
-    }
+  s = desc;
+  if (!router_parse_list_from_string(&s, list, SAVED_NOWHERE, 1)) {
+    SMARTLIST_FOREACH(list, extrainfo_t *, ei, {
+        r_tmp = dirserv_add_extrainfo(ei, &msg_out);
+        if (r_tmp < r) {
+          r = r_tmp;
+          *msg = msg_out;
+        }
+      });
   }
+  n_parsed += smartlist_len(list);
+  smartlist_free(list);
 
-  return r <= 2 ? r : -2;
+  return r <= 2 ? r : 2;
 }
 
-/** Parse the server descriptor at <b>desc</b> and maybe insert it into
- * the list of server descriptors. Set *<b>msg</b> to a message that
- * should be passed back to the origin of this descriptor.
+/** Parse the server descriptor at <b>desc</b> and maybe insert it into the
+ * list of server descriptors. Set *<b>msg</b> to a message that should be
+ * passed back to the origin of this descriptor. DOCDOC no longer parses.
  *
  * Return 2 if descriptor is well-formed and accepted;
  *  1 if well-formed and accepted but origin should hear *msg;
  *  0 if well-formed but redundant with one we already have;
  * -1 if it looks vaguely like a router descriptor but rejected;
- * -2 if we can't find a router descriptor in <b>desc</b>.
  */
 int
-dirserv_add_descriptor(const char *desc, const char *end, const char **msg)
+dirserv_add_descriptor(routerinfo_t *ri, const char **msg)
 {
   int r;
-  routerinfo_t *ri = NULL, *ri_old = NULL;
-  extrainfo_t *ei = NULL;
-  tor_assert(msg);
-  *msg = NULL;
-  desc = eat_whitespace(desc);
-
-  if (!strcmpstart(desc, "extra-info")) {
-    /* It's an extra-info thingie. */
-    routerlist_t *rl = router_get_routerlist();
-    ei = extrainfo_parse_entry_from_string(desc, end, 1, rl->identity_map);
-    if (!ei) {
-      log_warn(LD_DIRSERV, "Couldn't parse uploaded extra-info descriptor");
-      *msg = "Rejected: couldn't parse extra-info descriptor";
-      return -2;
-    }
-    ri = router_get_by_digest(ei->cache_info.identity_digest);
-    if (!ri) {
-      *msg = "No corresponding router descriptor for extra-info descriptor";
-      extrainfo_free(ei);
-      return -1;
-    }
-    if (routerinfo_incompatible_with_extrainfo(ri, ei)) {
-      *msg = "Router descriptor incompatible with extra-info descriptor";
-      extrainfo_free(ei);
-      return -1;
-    }
-    router_add_extrainfo_to_routerlist(ei, msg, 0, 0);
-    return 2;
-  }
+  routerinfo_t *ri_old;
+  signed_descriptor_t *desc = &ri->cache_info;
 
-  /* Check: is the descriptor syntactically valid? */
-  ri = router_parse_entry_from_string(desc, end, 1);
-  if (!ri) {
-    log_warn(LD_DIRSERV, "Couldn't parse uploaded server descriptor");
-    *msg = "Rejected: Couldn't parse server descriptor.";
-    return -2;
-  }
   /* Check whether this descriptor is semantically identical to the last one
    * from this server.  (We do this here and not in router_add_to_routerlist
    * because we want to be able to accept the newest router descriptor that
@@ -636,6 +610,29 @@ dirserv_add_descriptor(const char *desc, const char *end, const char **msg)
   }
 }
 
+/** DOCDOC */
+static int
+dirserv_add_extrainfo(extrainfo_t *ei, const char **msg)
+{
+  routerinfo_t *ri;
+  tor_assert(msg);
+  *msg = NULL;
+
+  ri = router_get_by_digest(ei->cache_info.identity_digest);
+  if (!ri) {
+    *msg = "No corresponding router descriptor for extra-info descriptor";
+    extrainfo_free(ei);
+    return -1;
+  }
+  if (routerinfo_incompatible_with_extrainfo(ri, ei)) {
+    *msg = "Router descriptor incompatible with extra-info descriptor";
+    extrainfo_free(ei);
+    return -1;
+  }
+  router_add_extrainfo_to_routerlist(ei, msg, 0, 0);
+  return 2;
+}
+
 /** Remove all descriptors whose nicknames or fingerprints no longer
  * are allowed by our fingerprint list. (Descriptors that used to be
  * good can become bad when we reload the fingerprint list.)
diff --git a/src/or/or.h b/src/or/or.h
index da39349a6..2725ce220 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2487,7 +2487,7 @@ int control_event_descriptors_changed(smartlist_t *routers);
 int control_event_address_mapped(const char *from, const char *to,
                                  time_t expires);
 int control_event_or_authdir_new_descriptor(const char *action,
-                                            const char *descriptor,
+                                            signed_descriptor_t *desc,
                                             const char *msg);
 int control_event_my_descriptor_changed(void);
 int control_event_networkstatus_changed(smartlist_t *statuses);
@@ -2572,8 +2572,7 @@ int dirserv_load_fingerprint_file(void);
 void dirserv_free_fingerprint_list(void);
 const char *dirserv_get_nickname_by_digest(const char *digest);
 int dirserv_add_multiple_descriptors(const char *desc, const char **msg);
-int dirserv_add_descriptor(const char *desc, const char *end,
-                           const char **msg);
+int dirserv_add_descriptor(routerinfo_t *ri, const char **msg);
 int getinfo_helper_dirserv_unregistered(control_connection_t *conn,
                                         const char *question, char **answer);
 void dirserv_free_descriptors(void);
diff --git a/src/or/router.c b/src/or/router.c
index 17470cc5f..8937e0427 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -328,7 +328,7 @@ init_keys(void)
       log_err(LD_GENERAL,"Error initializing descriptor.");
       return -1;
     }
-    if (dirserv_add_descriptor(mydesc, NULL, &m) < 0) {
+    if (dirserv_add_descriptor(router_get_my_routerinfo(), &m) < 0) {
       log_err(LD_GENERAL,"Unable to add own descriptor to directory: %s",
               m?m:"<unknown error>");
       return -1;
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 8c0ca7069..736a8b1c6 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -833,13 +833,14 @@ check_signature_token(const char *digest,
 int
 router_parse_list_from_string(const char **s, smartlist_t *dest,
                               saved_location_t saved_location,
-                              int is_extrainfo)
+                              int want_extrainfo)
 {
   routerinfo_t *router;
   extrainfo_t *extrainfo;
   signed_descriptor_t *signed_desc;
   void *elt;
-  const char *end, *cp, *start;
+  const char *end, *start;
+  int have_extrainfo;
 
   tor_assert(s);
   tor_assert(*s);
@@ -849,60 +850,46 @@ router_parse_list_from_string(const char **s, smartlist_t *dest,
   while (1) {
     *s = eat_whitespace(*s);
     /* Don't start parsing the rest of *s unless it contains a router. */
-    if (is_extrainfo) {
-      if (strcmpstart(*s, "extra-info")!=0)
-        break;
+    if (strcmpstart(*s, "extra-info ")==0) {
+      have_extrainfo = 1;
+    } else  if (strcmpstart(*s, "router ")==0) {
+      have_extrainfo = 0;
     } else {
-      if (strcmpstart(*s, "router ")!=0)
+      /* skip junk. */
+      const char *ei = strstr(*s, "\nextra-info ");
+      const char *ri = strstr(*s, "\nrouter ");
+      if (ri && (!ei || ri < ei)) {
+        have_extrainfo = 0;
+        *s = ri + 1;
+      } else if (ei) {
+        have_extrainfo = 1;
+        *s = ei + 1;
+      } else {
         break;
+      }
     }
-    /* XXXX020 this is hideously complicated.  Can't we just search for the
-     * first -----END SIGNATURE----- and be done with it?  Or the first
-     * -----END SIGNATURE----- after the first \nrouter-signature ?*/
-    if (is_extrainfo && (end = strstr(*s+1, "\nextra-info"))) {
-      cp = end;
-      end++;
-    } else if (!is_extrainfo && (end = strstr(*s+1, "\nrouter "))) {
-      cp = end;
-      end++;
-    } else if ((end = strstr(*s+1, "\ndirectory-signature"))) {
-      cp = end;
-      end++;
-    } else {
-      cp = end = *s+strlen(*s);
-    }
-
-    while (cp > *s && (!*cp || TOR_ISSPACE(*cp)))
-      --cp;
-    /* cp now points to the last non-space character in this descriptor. */
+    end = strstr(*s, "\nrouter-signature");
+    if (end)
+      end = strstr(end, "\n-----END SIGNATURE-----\n");
+    if (end)
+      end += strlen("\n-----END SIGNATURE-----\n");
 
-    while (cp > *s  && *cp != '\n')
-      --cp;
-    /* cp now points to the first \n before the last non-blank line in this
-     * descriptor */
-
-    if (strcmpstart(cp, "\n-----END SIGNATURE-----\n")) {
-      log_info(LD_DIR, "Ignoring truncated router descriptor.");
-      *s = end;
-      continue;
-    }
+    if (!end)
+      break;
 
-    if (is_extrainfo) {
+    if (have_extrainfo && want_extrainfo) {
       routerlist_t *rl = router_get_routerlist();
       extrainfo = extrainfo_parse_entry_from_string(*s, end,
                                        saved_location != SAVED_IN_CACHE,
                                        rl->identity_map);
       signed_desc = &extrainfo->cache_info;
       elt = extrainfo;
-    } else {
+    } else if (!have_extrainfo && !want_extrainfo) {
       router = router_parse_entry_from_string(*s, end,
                                           saved_location != SAVED_IN_CACHE);
       signed_desc = &router->cache_info;
       elt = router;
-    }
-
-    if (!elt) {
-      log_warn(LD_DIR, "Error reading router; skipping");
+    } else {
       *s = end;
       continue;
     }
@@ -2138,7 +2125,6 @@ tokenize_string(const char *start, const char *end, smartlist_t *out,
   s = &start;
   if (!end)
     end = start+strlen(start);
-  memset(counts, 0, sizeof(counts));
   for (i = 0; i < _NIL; ++i)
     counts[i] = 0;
   while (*s < end && (!tok || tok->tp != _EOF)) {