r15412@catbus: nickm | 2007-09-27 12:04:24 -0400

 More annotated-store work: handle annotations in lists correctly.  Add ability to prepend annotations to a routerdesc (and to every rtouredesc in a list), while verifying that the routerdesc is not already annotated.


svn:r11665

doc/TODO

@@ -122,17 +122,30 @@ Things we'd like to do in 0.2.0.x:
         - use the bridges for dir fetches even when our dirport is open.
 R     - drop 'authority' queries if they're to our own identity key; accept
         them otherwise.
-N     - Design/implement the "local-status" or something like it, from the
+      X Design/implement the "local-status" or something like it, from the
         "Descriptor purposes: how to tell them apart" section of
         http://archives.seul.org/or/dev/May-2007/msg00008.html
-        - cache of bridges that we've learned about and use but aren't
-          manually listed in the torrc.
       o timeout and retry schedules for fetching bridge descriptors
       - give extend_info_t a router_purpose again
       o react faster to download networkstatuses after the first bridge
         descriptor arrives
       o be more robust to bridges being marked as down and leaving us
         stranded without any known "running" bridges.
+N     . Cache for bridge descriptors
+        . Annotated router store
+          o Accept annotations before routers
+          o Preserve and ignore unexpected annotations
+          o Mechanism to add annotations when we first add a descriptor
+          o Don't serve annotations
+          o Reject annotations that appear in things we've downloaded
+          - Name the router store something different: cached-descriptors?
+            - But load from cached-routers if no cached-descriptors is
+              found.
+            - Document this.
+        - Use annotations to denote router purpose
+          - Learn purpose from annotations
+          - Set annotations based on purpose
+          - Preserve routers with unrecognized purpose.
     - Bridges operators (rudimentary version)
       - Ability to act as dir cache without a dir port.
       o Bridges publish to bridge authorities

src/or/dirserv.c

@@ -536,11 +536,13 @@ dirserv_add_multiple_descriptors(const char *desc, uint8_t purpose,
 
   s = desc;
   list = smartlist_create();
-  if (!router_parse_list_from_string(&s, NULL, list, SAVED_NOWHERE, 0, 0)) {
+  if (!router_parse_list_from_string(&s, NULL, list, SAVED_NOWHERE, 0, 0,
+                                     NULL)) {
     SMARTLIST_FOREACH(list, routerinfo_t *, ri, {
         msg_out = NULL;
 
         /* Assign the purpose.
+         *
          * XXX020 Perhaps this should get pushed into
          * router_parse_list_from_string()? Also, tie it somehow into
          * router_load_single_router()? Lastly, does extrainfo_t want
@@ -561,7 +563,8 @@ dirserv_add_multiple_descriptors(const char *desc, uint8_t purpose,
   smartlist_clear(list);
 
   s = desc;
-  if (!router_parse_list_from_string(&s, NULL, list, SAVED_NOWHERE, 1, 0)) {
+  if (!router_parse_list_from_string(&s, NULL, list, SAVED_NOWHERE, 1, 0,
+                                     NULL)) {
     SMARTLIST_FOREACH(list, extrainfo_t *, ei, {
         msg_out = NULL;
 

src/or/or.h

@@ -3603,7 +3603,8 @@ int router_parse_list_from_string(const char **s, const char *eos,
                                   smartlist_t *dest,
                                   saved_location_t saved_location,
                                   int is_extrainfo,
-                                  int allow_annotations);
+                                  int allow_annotations,
+                                  const char *prepend_annotations);
 int router_parse_routerlist_from_directory(const char *s,
                                            routerlist_t **dest,
                                            crypto_pk_env_t *pkey,
@@ -3613,7 +3614,8 @@ int router_parse_runningrouters(const char *str);
 int router_parse_directory(const char *str);
 routerinfo_t *router_parse_entry_from_string(const char *s, const char *end,
                                              int cache_copy,
-                                             int allow_annotations);
+                                             int allow_annotations,
+                                             const char *prepend_annotations);
 extrainfo_t *extrainfo_parse_entry_from_string(const char *s, const char *end,
                          int cache_copy, struct digest_ri_map_t *routermap);
 addr_policy_t *router_parse_addr_policy_from_string(const char *s,

src/or/router.c

@@ -485,7 +485,7 @@ init_keys(void)
       return -1;
     }
     if (mydesc) {
-      ri = router_parse_entry_from_string(mydesc, NULL, 1, 0);
+      ri = router_parse_entry_from_string(mydesc, NULL, 1, 0, NULL);
       if (!ri) {
         log_err(LD_GENERAL,"Generated a routerinfo we couldn't parse.");
         return -1;
@@ -1632,7 +1632,7 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
     const char *cp;
     routerinfo_t *ri_tmp;
     cp = s_dup = tor_strdup(s);
-    ri_tmp = router_parse_entry_from_string(cp, NULL, 1, 0);
+    ri_tmp = router_parse_entry_from_string(cp, NULL, 1, 0, NULL);
     if (!ri_tmp) {
       log_err(LD_BUG,
               "We just generated a router descriptor we can't parse.");

src/or/routerlist.c

@@ -3106,7 +3106,7 @@ router_load_single_router(const char *s, uint8_t purpose, const char **msg)
   tor_assert(msg);
   *msg = NULL;
 
-  if (!(ri = router_parse_entry_from_string(s, NULL, 1, 0))) {
+  if (!(ri = router_parse_entry_from_string(s, NULL, 1, 0, NULL))) {
     log_warn(LD_DIR, "Error parsing router descriptor; dropping.");
     *msg = "Couldn't parse router descriptor.";
     return -1;
@@ -3167,7 +3167,7 @@ router_load_routers_from_string(const char *s, const char *eos,
   int allow_annotations = (saved_location != SAVED_NOWHERE);
 
   router_parse_list_from_string(&s, eos, routers, saved_location, 0,
-                                allow_annotations);
+                                allow_annotations, NULL);
 
   routers_update_status_from_networkstatus(routers, !from_cache);
 
@@ -3228,7 +3228,8 @@ router_load_extrainfo_from_string(const char *s, const char *eos,
   const char *msg;
   int from_cache = (saved_location != SAVED_NOWHERE);
 
-  router_parse_list_from_string(&s, eos, extrainfo_list, saved_location, 1, 0);
+  router_parse_list_from_string(&s, eos, extrainfo_list, saved_location, 1, 0,
+                                NULL);
 
   log_info(LD_DIR, "%d elements to add", smartlist_len(extrainfo_list));
 

src/or/routerparse.c

@@ -372,10 +372,13 @@ static void token_free(directory_token_t *tok);
 static smartlist_t *find_all_exitpolicy(smartlist_t *s);
 static directory_token_t *find_first_by_keyword(smartlist_t *s,
                                                 directory_keyword keyword);
+#define TS_ANNOTATIONS_OK 1
+#define TS_NOCHECK 2
+#define TS_NO_NEW_ANNOTATIONS 4
 static int tokenize_string(const char *start, const char *end,
                            smartlist_t *out,
                            token_rule_t *table,
-                           int allow_annotations);
+                           int flags);
 static directory_token_t *get_next_token(const char **s,
                                          const char *eos,
                                          token_rule_t *table);
@@ -869,7 +872,8 @@ router_parse_list_from_string(const char **s, const char *eos,
                               smartlist_t *dest,
                               saved_location_t saved_location,
                               int want_extrainfo,
-                              int allow_annotations)
+                              int allow_annotations,
+                              const char *prepend_annotations)
 {
   routerinfo_t *router;
   extrainfo_t *extrainfo;
@@ -893,15 +897,24 @@ router_parse_list_from_string(const char **s, const char *eos,
     if ((eos - *s) < 32) /* make sure it's long enough. */
       break;
 
-    /* Don't start parsing the rest of *s unless it contains a router. */
+    /* Don't start parsing the rest of *s unless it contains a router or
+     * extra-info. */
     if (strcmpstart(*s, "extra-info ")==0) {
       have_extrainfo = 1;
-    } else  if (strcmpstart(*s, "router ")==0) {
+    } else if (strcmpstart(*s, "router ")==0) {
       have_extrainfo = 0;
     } else {
       /* skip junk. */
-      const char *ei = tor_memstr(*s, eos-*s, "\nextra-info ");
-      const char *ri = tor_memstr(*s, eos-*s, "\nrouter ");
+      const char *annotation = NULL, *ei, *ri;
+      if (**s == '@') {
+        annotation = *s;
+      } else {
+        if ((annotation = tor_memstr(*s, eos-*s, "\n@")))
+          ++annotation;
+      }
+
+      ei = tor_memstr(*s, eos-*s, "\nextra-info ");
+      ri = tor_memstr(*s, eos-*s, "\nrouter ");
       if (ri && (!ei || ri < ei)) {
         have_extrainfo = 0;
         *s = ri + 1;
@@ -911,6 +924,8 @@ router_parse_list_from_string(const char **s, const char *eos,
       } else {
         break;
       }
+      if (annotation && annotation < *s)
+        *s = annotation;
     }
     end = tor_memstr(*s, eos-*s, "\nrouter-signature");
     if (end)
@@ -935,7 +950,8 @@ router_parse_list_from_string(const char **s, const char *eos,
     } else if (!have_extrainfo && !want_extrainfo) {
       router = router_parse_entry_from_string(*s, end,
                                               saved_location != SAVED_IN_CACHE,
-                                              allow_annotations);
+                                              allow_annotations,
+                                              prepend_annotations);
       if (router) {
         signed_desc = &router->cache_info;
         elt = router;
@@ -986,10 +1002,12 @@ dump_distinct_digest_count(int severity)
  * returns NULL.  If <b>cache_copy</b> is true, duplicate the contents of
  * s through end into the signed_descriptor_body of the resulting
  * routerinfo_t.
+ * DOCDOC annotations
  */
 routerinfo_t *
 router_parse_entry_from_string(const char *s, const char *end,
-                               int cache_copy, int allow_annotations)
+                               int cache_copy, int allow_annotations,
+                               const char *prepend_annotations)
 {
   routerinfo_t *router = NULL;
   char digest[128];
@@ -998,6 +1016,8 @@ router_parse_entry_from_string(const char *s, const char *end,
   struct in_addr in;
   const char *start_of_annotations, *cp;
 
+  tor_assert(!allow_annotations || !prepend_annotations);
+
   if (!end) {
     end = s + strlen(s);
   }
@@ -1006,6 +1026,15 @@ router_parse_entry_from_string(const char *s, const char *end,
   while (end > s+2 && *(end-1) == '\n' && *(end-2) == '\n')
     --end;
 
+  tokens = smartlist_create();
+  if (prepend_annotations) {
+    if (tokenize_string(prepend_annotations,NULL,tokens,
+                        routerdesc_token_table,TS_NOCHECK)) {
+      log_warn(LD_DIR, "Error tokenizing router descriptor.");
+      goto err;
+    }
+  }
+
   start_of_annotations = s;
   cp = tor_memstr(s, end-s, "\nrouter ");
   if (!cp) {
@@ -1021,10 +1050,17 @@ router_parse_entry_from_string(const char *s, const char *end,
     log_warn(LD_DIR, "Couldn't compute router hash.");
     return NULL;
   }
-  tokens = smartlist_create();
-  if (tokenize_string(s,end,tokens,routerdesc_token_table,allow_annotations)) {
-    log_warn(LD_DIR, "Error tokenizing router descriptor.");
-    goto err;
+  {
+    int flags = 0;
+    if (allow_annotations)
+      flags |= TS_ANNOTATIONS_OK;
+    if (prepend_annotations)
+      flags |= TS_ANNOTATIONS_OK|TS_NO_NEW_ANNOTATIONS;
+
+    if (tokenize_string(s,end,tokens,routerdesc_token_table, flags)) {
+      log_warn(LD_DIR, "Error tokenizing router descriptor.");
+      goto err;
+    }
   }
 
   if (smartlist_len(tokens) < 2) {
@@ -1041,10 +1077,21 @@ router_parse_entry_from_string(const char *s, const char *end,
 
   router = tor_malloc_zero(sizeof(routerinfo_t));
   router->routerlist_index = -1;
-  if (cache_copy)
-    router->cache_info.signed_descriptor_body = tor_strndup(s, end-s);
-  router->cache_info.signed_descriptor_len = s-start_of_annotations;
+  router->cache_info.annotations_len = s-start_of_annotations +
+    (prepend_annotations ? strlen(prepend_annotations) : 0) ;
   router->cache_info.signed_descriptor_len = end-s;
+  if (cache_copy) {
+    size_t len = router->cache_info.signed_descriptor_len +
+                router->cache_info.annotations_len;
+    char *cp =
+      router->cache_info.signed_descriptor_body = tor_malloc(len+1);
+    if (prepend_annotations) {
+      strlcpy(cp, prepend_annotations, len+1);
+      cp += strlen(prepend_annotations);
+    }
+    memcpy(cp, s, end-s);
+    cp[len] = '\0';
+  }
   memcpy(router->cache_info.signed_descriptor_digest, digest, DIGEST_LEN);
 
   router->nickname = tor_strdup(tok->args[0]);
@@ -1069,6 +1116,8 @@ router_parse_entry_from_string(const char *s, const char *end,
   router->bandwidthrate =
     tor_parse_long(tok->args[0],10,0,INT_MAX,NULL,NULL);
 
+  /* Set purpose XXXX020 NM NM*/
+
   if (!router->bandwidthrate) {
     log_warn(LD_DIR, "bandwidthrate %s unreadable or 0. Failing.",
              escaped(tok->args[0]));
@@ -2663,16 +2712,18 @@ get_next_token(const char **s, const char *eos, token_rule_t *table)
 
 /** Read all tokens from a string between <b>start</b> and <b>end</b>, and add
  * them to <b>out</b>.  Parse according to the token rules in <b>table</b>.
+ * Caller must free tokens in <b>out</b>.
  */
 static int
 tokenize_string(const char *start, const char *end, smartlist_t *out,
-                token_rule_t *table, int allow_annotations)
+                token_rule_t *table, int flags)
 {
   const char **s;
   directory_token_t *tok = NULL;
   int counts[_NIL];
   int i;
   int first_nonannotation;
+  int prev_len = smartlist_len(out);
 
   s = &start;
   if (!end)
@@ -2691,7 +2742,10 @@ tokenize_string(const char *start, const char *end, smartlist_t *out,
     *s = eat_whitespace_eos(*s, end);
   }
 
-  if (allow_annotations) {
+  if (flags & TS_NOCHECK)
+    return 0;
+
+  if ((flags & TS_ANNOTATIONS_OK)) {
     first_nonannotation = -1;
     for (i = 0; i < smartlist_len(out); ++i) {
       tok = smartlist_get(out, i);
@@ -2711,6 +2765,12 @@ tokenize_string(const char *start, const char *end, smartlist_t *out,
         return -1;
       }
     }
+    if ((flags & TS_NO_NEW_ANNOTATIONS)) {
+      if (first_nonannotation != prev_len) {
+        log_warn(LD_DIR, "parse error: Unexpectd annotations.");
+        return -1;
+      }
+    }
   } else {
     for (i=0;  i < smartlist_len(out); ++i) {
       tok = smartlist_get(out, i);

src/or/test.c

@@ -2189,7 +2189,7 @@ test_dir_format(void)
 
   test_assert(router_dump_router_to_string(buf, 2048, &r1, pk2)>0);
   cp = buf;
-  rp1 = router_parse_entry_from_string((const char*)cp,NULL,1,0);
+  rp1 = router_parse_entry_from_string((const char*)cp,NULL,1,0,NULL);
   test_assert(rp1);
   test_streq(rp1->address, r1.address);
   test_eq(rp1->or_port, r1.or_port);