Fix router_compare_addr_to_exit_policy, and name its return codes. The bug was: "maybe reject,accept" should be "maybe", not "accept".

svn:r1093

3 files changed, 29 insertions, 25 deletions

diff --git a/src/or/or.h b/src/or/or.h
index 1e127d517c..1ec9ba6f69 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -817,6 +817,9 @@ routerinfo_t *router_get_entry_from_string(const char **s);
 int router_add_exit_policy_from_string(routerinfo_t *router, const char *s);
 int router_compare_addr_to_exit_policy(uint32_t addr, uint16_t port,
                                        struct exit_policy_t *policy);
+#define ADDR_POLICY_ACCEPTED 0
+#define ADDR_POLICY_REJECTED -1
+#define ADDR_POLICY_UNKNOWN 1
 int router_exit_policy_all_routers_reject(uint32_t addr, uint16_t port);
 int router_exit_policy_rejects_all(routerinfo_t *router);
 
diff --git a/src/or/router.c b/src/or/router.c
index 032039873f..076d148b77 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -286,19 +286,17 @@ static void router_add_exit_policy_from_config(routerinfo_t *router) {
   }
 }
 
-/* Return 0 if my exit policy says to allow connection to conn.
- * Else return -1.
+/* Return false if my exit policy says to allow connection to conn.
+ * Else return true.
  */
 int router_compare_to_my_exit_policy(connection_t *conn) {
   assert(desc_routerinfo);
   assert(conn->addr); /* make sure it's resolved to something. this
                          way we can't get a 'maybe' below. */
 
-  if (router_compare_addr_to_exit_policy(conn->addr, conn->port,
-                                         desc_routerinfo->exit_policy) == 0)
-    return 0;
-  else
-    return -1;
+  return router_compare_addr_to_exit_policy(conn->addr, conn->port, 
+                   desc_routerinfo->exit_policy) == ADDR_POLICY_ACCEPTED;
+    
 }
 
 const char *router_get_my_descriptor(void) {
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 1cac72de74..5f87f985e2 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -402,6 +402,7 @@ int router_compare_addr_to_exit_policy(uint32_t addr, uint16_t port,
                                        struct exit_policy_t *policy)
 {
   int maybe_reject = 0;
+  int maybe_accept = 0;
   int match = 0;
   struct in_addr in;
   struct exit_policy_t *tmpe;
@@ -413,10 +414,13 @@ int router_compare_addr_to_exit_policy(uint32_t addr, uint16_t port,
       if (tmpe->msk == 0 && (port >= tmpe->prt_min && port <= tmpe->prt_max)) {
         /* The exit policy is accept/reject *:port */
         match = 1;
-      } else if (port >= tmpe->prt_min && port <= tmpe->prt_max &&
-                 tmpe->policy_type == EXIT_POLICY_REJECT) {
-        /* The exit policy is reject ???:port */
-        maybe_reject = 1;
+      } else if (port >= tmpe->prt_min && port <= tmpe->prt_max)
+        if (tmpe->policy_type == EXIT_POLICY_REJECT) {
+          /* The exit policy is reject ???:port */
+          maybe_reject = 1;
+        } else {
+          /* The exit policy is acccept ???:port */
+          maybe_accept = 1;
       }
     } else {
       /* Address is known */
@@ -430,16 +434,17 @@ int router_compare_addr_to_exit_policy(uint32_t addr, uint16_t port,
       in.s_addr = htonl(addr);
       log_fn(LOG_INFO,"Address %s:%d matches exit policy '%s'",
              inet_ntoa(in), port, tmpe->string);
-      if(tmpe->policy_type == EXIT_POLICY_ACCEPT)
-        return 0;
-      else
-        return -1;
+      if(tmpe->policy_type == EXIT_POLICY_ACCEPT) {
+        /* If we already hit a clause that might trigger a 'reject', than we
+         * can't be sure of this certain 'accept'.*/
+        return maybe_reject ? ADDR_POLICY_UNKNOWN : ADDR_POLICY_ACCEPTED;
+      } else {
+        return maybe_accept ? ADDR_POLICY_UNKNOWN : ADDR_POLICY_REJECTED;
+      }
     }
   }
-  if (maybe_reject)
-    return 1;
-  else
-    return 0; /* accept all by default. */
+  /* accept all by default. */
+  return maybe_reject ? ADDR_POLICY_UNKNOWN : ADDR_POLICY_ACCEPTED;
 }
 
 /* return 1 if all running routers will reject addr:port, return 0 if
@@ -450,18 +455,16 @@ int router_exit_policy_all_routers_reject(uint32_t addr, uint16_t port) {
 
   for (i=0;i<routerlist->n_routers;i++) {
     router = routerlist->routers[i];
-    if (router->is_running && router_compare_addr_to_exit_policy(addr,
-        port, router->exit_policy) >= 0)
+    if (router->is_running && router_compare_addr_to_exit_policy(
+             addr, port, router->exit_policy) != ADDR_POLICY_REJECTED)
       return 0; /* this one could be ok. good enough. */
   }
   return 1; /* all will reject. */
 }
 
 int router_exit_policy_rejects_all(routerinfo_t *router) {
-  if (router_compare_addr_to_exit_policy(0, 0, router->exit_policy) < 0)
-    return 1; /* yes, rejects all */
-  else
-    return 0; /* no, might accept some */
+  return router_compare_addr_to_exit_policy(0, 0, router->exit_policy) 
+    == ADDR_POLICY_REJECTED;
 }
 
 /* Helper function: parse a directory from 's' and, when done, store the