when we think a router is unreachable, pass the message back to the

server's logs, and make it a 'warn'.
also, fix a memory leak for rejected router descriptors.


svn:r4889

src/or/directory.c

@@ -1188,19 +1188,21 @@ directory_handle_command_post(connection_t *conn, char *headers,
 
   if (!strcmp(url,"/tor/")) { /* server descriptor post */
     const char *msg;
-    switch (dirserv_add_descriptor(body, &msg)) {
+    int r = dirserv_add_descriptor(body, &msg);
+    tor_assert(msg);
+    if (r > 0)
+      dirserv_get_directory(&cp, 0); /* rebuild and write to disk */
+    switch (r) {
       case -2:
       case -1:
-        /* malformed descriptor, or something wrong */
-        write_http_status_line(conn, 400, msg?msg:"Malformed or unacceptable server descriptor");
-        log_fn(LOG_NOTICE,"Rejected descriptor published by %s.", origin);
-        break;
-      case 0:
-        write_http_status_line(conn, 200, msg?msg:"Server descriptor okay, but not accepted.");
-        break;
       case 1:
-        dirserv_get_directory(&cp, 0); /* rebuild and write to disk */
-        write_http_status_line(conn, 200, msg?msg:"Server descriptor accepted");
+        log_fn(LOG_NOTICE,"Rejected descriptor published by %s.", origin);
+        /* malformed descriptor, or something wrong */
+        write_http_status_line(conn, 400, msg);
+        break;
+      case 0: /* accepted but discarded */
+      case 2: /* accepted */
+        write_http_status_line(conn, 200, msg);
         break;
     }
     goto done;

src/or/dirserv.c

@@ -319,18 +319,19 @@ dirserv_wants_to_reject_router(routerinfo_t *ri, int *verified,
     *msg = "Rejected: Address is not an IP, or IP is a private address.";
     return -1;
   }
-
   return 0;
 }
 
-/** Parse the server descriptor at desc and maybe insert it into the list of
- * server descriptors.  Set msg to a message that should be passed back to the
- * origin of this descriptor, or to NULL.
+/** 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.
  *
- * Return 1 if descriptor is well-formed and accepted;
+ * 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 *desc.
+ * -2 if we can't find a router descriptor in <b>desc</b>.
+ *
  */
 int
 dirserv_add_descriptor(const char *desc, const char **msg)
@@ -345,7 +346,7 @@ dirserv_add_descriptor(const char *desc, const char **msg)
   if (!ri) {
     log(LOG_WARN, "Couldn't parse descriptor");
     *msg = "Rejected: Couldn't parse server descriptor.";
-    return -1;
+    return -2;
   }
   if ((r = router_add_to_routerlist(ri, msg))<0) {
     return r == -1 ? 0 : -1;
@@ -358,7 +359,7 @@ dirserv_add_descriptor(const char *desc, const char **msg)
       *msg =  ri->is_verified ? "Verified server descriptor accepted" :
         "Unverified server descriptor accepted";
     }
-    return 1;
+    return r == 0 ? 2 : 1;
   }
 }
 
@@ -504,7 +505,7 @@ dirserv_thinks_router_is_reachable(routerinfo_t *router, time_t now)
 /** Return 1 if we're confident that there's a problem with
  * <b>router</b>'s reachability and its operator should be notified.
  */
-static int
+int
 dirserv_thinks_router_is_blatantly_unreachable(routerinfo_t *router, time_t now)
 {
   connection_t *conn;

src/or/or.h

@@ -1632,6 +1632,8 @@ const char *dirserv_get_nickname_by_digest(const char *digest);
 int dirserv_add_descriptor(const char *desc, const char **msg);
 char *dirserver_getinfo_unregistered(const char *question);
 void dirserv_free_descriptors(void);
+int dirserv_thinks_router_is_blatantly_unreachable(routerinfo_t *router,
+                                                   time_t now);
 int list_server_status(smartlist_t *routers, char **router_status_out);
 void dirserv_log_unreachable_servers(time_t now);
 int dirserv_dump_directory_to_string(char **dir_out,

src/or/router.c

@@ -329,7 +329,7 @@ init_keys(void)
       log_fn(LOG_ERR, "Error adding own fingerprint to approved set");
       return -1;
     }
-    if (dirserv_add_descriptor(tmp, &m) != 1) {
+    if (dirserv_add_descriptor(tmp, &m) < 0) {
       log(LOG_ERR, "Unable to add own descriptor to directory: %s",
           m?m:"<unknown error>");
       return -1;

src/or/routerlist.c

@@ -854,15 +854,18 @@ router_mark_as_down(const char *digest)
 /** Add <b>router</b> to the routerlist, if we don't already have it.  Replace
  * older entries (if any) with the same name.  Note: Callers should not hold
  * their pointers to <b>router</b> if this function fails; <b>router</b>
- * will either be inserted into the routerlist or freed.  Returns 0 if the
- * router was added; less than 0 if it was not.
+ * will either be inserted into the routerlist or freed.
  *
- * If we're returning an error, then assign to *<b>msg</b> a static string
- * describing the reason for refusing the routerinfo.
+ * Returns >= 0 if the router was added; less than 0 if it was not.
+ *
+ * If we're returning non-zero, then assign to *<b>msg</b> a static string
+ * describing the reason for not liking the routerinfo.
  *
  * If the return value is less than -1, there was a problem with the
- * routerinfo.  If the return value is equal to -1, then the routerinfo was
- * fine, but out-of-date.
+ * routerinfo. If the return value is equal to -1, then the routerinfo was
+ * fine, but out-of-date. If the return value is equal to 1, the
+ * routerinfo was accepted, but we should notify the generator of the
+ * descriptor using the message *<b>msg</b>.
  */
 int
 router_add_to_routerlist(routerinfo_t *router, const char **msg)
@@ -882,8 +885,11 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg)
   crypto_pk_get_digest(router->identity_pkey, id_digest);
 
   if (authdir) {
-    if (dirserv_wants_to_reject_router(router, &authdir_verified, msg))
+    if (dirserv_wants_to_reject_router(router, &authdir_verified, msg)) {
+      tor_assert(*msg);
+      routerinfo_free(router);
       return -2;
+    }
     router->is_verified = authdir_verified;
     if (tor_version_as_new_as(router->platform,"0.1.0.2-rc"))
       router->is_verified = 1;
@@ -903,9 +909,10 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg)
           old_router->is_running = router->is_running;
         }
         routerinfo_free(router);
-        if (msg) *msg = "Router descriptor was not new.";
+        *msg = "Router descriptor was not new.";
         return -1;
       } else {
+        int unreachable;
         log_fn(LOG_DEBUG, "Replacing entry for router '%s/%s' [%s]",
                router->nickname, old_router->nickname,
                hex_str(id_digest,DIGEST_LEN));
@@ -915,13 +922,15 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg)
           router->last_reachable = old_router->last_reachable;
           router->testing_since = old_router->testing_since;
         }
-        if (msg)
-          *msg = authdir_verified ? "Verified server updated":
-          "Unverified server updated. (Have you sent us your key finerprint?)";
+        unreachable = authdir &&
+          dirserv_thinks_router_is_blatantly_unreachable(router, time(NULL));
         routerinfo_free(old_router);
         smartlist_set(routerlist->routers, i, router);
         directory_set_dirty();
-        return 0;
+        *msg = unreachable ? "Dirserver believes your ORPort is unreachable" :
+               authdir_verified ? "Verified server updated" :
+               "Unverified server updated. (Have you sent us your key fingerprint?)";
+        return unreachable ? 1 : 0;
       }
     } else if (!strcasecmp(router->nickname, old_router->nickname)) {
       /* nicknames match, keys don't. */
@@ -947,7 +956,7 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg)
         log_fn(LOG_DEBUG, "Skipping unverified entry for verified router '%s'",
                router->nickname);
         routerinfo_free(router);
-        if (msg) *msg = "Already have verified router with same nickname and different key.";
+        *msg = "Already have verified router with same nickname and different key.";
         return -2;
       }
     }
@@ -1066,7 +1075,7 @@ router_load_routerlist_from_directory(const char *s,
     SMARTLIST_FOREACH(new_list->routers, routerinfo_t *, r,
     {
       const char *msg;
-      if (router_add_to_routerlist(r,&msg)==0)
+      if (router_add_to_routerlist(r,&msg)>=0)
         smartlist_add(changed, r);
     });
     smartlist_clear(new_list->routers);
@@ -1082,13 +1091,6 @@ router_load_routerlist_from_directory(const char *s,
     control_event_descriptors_changed(routerlist->routers);
   }
   router_normalize_routerlist(routerlist);
-#if 0
-  if (get_options()->AuthoritativeDir) {
-    /* Learn about the descriptors in the directory. */
-    dirserv_load_from_directory_string(s);
-//XXXRD
-  }
-#endif
   return 0;
 }
 

src/or/test.c

@@ -1294,9 +1294,9 @@ test_dir_format(void)
   r1.published_on = time(NULL);
   r2.published_on = time(NULL)-3*60*60;
   test_assert(router_dump_router_to_string(buf, 2048, &r1, pk2)>0);
-  test_eq(dirserv_add_descriptor(buf,&m), 1);
+  test_eq(dirserv_add_descriptor(buf,&m), 2);
   test_assert(router_dump_router_to_string(buf, 2048, &r2, pk1)>0);
-  test_eq(dirserv_add_descriptor(buf,&m), 1);
+  test_eq(dirserv_add_descriptor(buf,&m), 2);
   get_options()->Nickname = tor_strdup("DirServer");
   test_assert(!dirserv_dump_directory_to_string(&cp,pk3));
   test_assert(!router_parse_routerlist_from_directory(cp, &dir1, pk3, 1, 0));