r15608@catbus: nickm | 2007-10-09 19:01:50 -0400

 Give better messages and return values from signature uploads and downlaods; also, log actual errors when we screw up.


svn:r11823

4 files changed, 27 insertions, 13 deletions

diff --git a/ChangeLog b/ChangeLog
index f04e55606..c5132eeaf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -42,6 +42,8 @@ Changes in version 0.2.0.8-alpha - 2007-??-??
     - When we get a valid consensus, recompute the voting schedule.
     - Base the valid-after time of a vote on the consensus voting schedule,
       not on our preferred schedule.
+    - Make the return values and messages from signature uploads and downloads
+      more sensible.
 
   o Minor bugfixes (performance):
     - Use a slightly simpler string hashing algorithm (copying Python's
diff --git a/src/or/directory.c b/src/or/directory.c
index bcb44ebe1..ea1f896d8 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -1465,6 +1465,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
     }
   }
   if (conn->_base.purpose == DIR_PURPOSE_FETCH_DETACHED_SIGNATURES) {
+    const char *msg = NULL;
     log_info(LD_DIR,"Got detached signatures (size %d) from server %s:%d",
              (int) body_len, conn->_base.address, conn->_base.port);
     if (status_code != 200) {
@@ -1476,7 +1477,10 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
       tor_free(body); tor_free(headers); tor_free(reason);
       return -1;
     }
-    dirvote_add_signatures(body);
+    if (dirvote_add_signatures(body, &msg)<0) {
+      log_warn(LD_DIR, "Problem adding detached signatures from %s:%d: %s",
+               conn->_base.address, conn->_base.port, msg?msg:"???");
+    }
   }
 
   if (conn->_base.purpose == DIR_PURPOSE_FETCH_SERVERDESC ||
@@ -2561,10 +2565,13 @@ directory_handle_command_post(dir_connection_t *conn, const char *headers,
 
   if (authdir_mode_v3(options) &&
       !strcmp(url,"/tor/post/consensus-signature")) { /* sigs on consensus. */
-    if (dirvote_add_signatures(body)>=0) {
-      write_http_status_line(conn, 200, "Signatures stored");
+    const char *msg = NULL;
+    if (dirvote_add_signatures(body, &msg)>=0) {
+      write_http_status_line(conn, 200, msg?msg:"Signatures stored");
     } else {
-      write_http_status_line(conn, 400, "Unable to store signatures");
+      log_warn(LD_DIR, "Unable to store signatures posted by %s: %s",
+               conn->_base.address, msg?msg:"???");
+      write_http_status_line(conn, 400, msg?msg:"Unable to store signatures");
     }
     goto done;
   }
diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index 40a964a3a..893f5c17a 100644
--- a/src/or/dirvote.c
+++ b/src/or/dirvote.c
@@ -812,11 +812,13 @@ networkstatus_check_consensus_signature(networkstatus_vote_t *consensus,
  * <b>src_voter_list</b> that should be added to <b>target.  (A signature
  * should be added if we have no signature for that voter in <b>target</b>
  * yet, or if we have no verifiable signature and the new signature is
- * verifiable.)   Return the number of signatures added or changed. */
+ * verifiable.)   Return the number of signatures added or changed, or
+ * -1 on error. */
 static int
 networkstatus_add_signatures_impl(networkstatus_vote_t *target,
                                   smartlist_t *src_voter_list)
 {
+  /*XXXX020 merge with the only function that calls it. */
   int r = 0;
   tor_assert(target);
   tor_assert(!target->is_vote);
@@ -862,7 +864,7 @@ networkstatus_add_signatures_impl(networkstatus_vote_t *target,
   return r;
 }
 
-/** As networkstatus_add_consensus_signature_impl, but takes new signatures
+/** As networkstatus_add_signature_impl, but takes new signatures
  * from the detached signatures document <b>sigs</b>. */
 int
 networkstatus_add_detached_signatures(networkstatus_vote_t *target,
@@ -1645,9 +1647,11 @@ dirvote_add_signatures_to_pending_consensus(
     }
     tor_free(pending_consensus_signatures);
     pending_consensus_signatures = new_detached;
+    *msg_out = "Signatures added";
+  } else {
+    *msg_out = "Digest mismatch when adding detached signatures";
   }
 
-  *msg_out = "ok";
   goto done;
  err:
   if (!msg_out)
@@ -1661,22 +1665,22 @@ dirvote_add_signatures_to_pending_consensus(
 /** Helper: we just got the <b>deteached_signatures_body</b> sent to us as
  * signatures on the currently pending consensus.  Add them to the pending
  * consensus (if we have one); otherwise queue them until we have a
- * consensus. */
+ * consensus.  Return negative on failure, nonnegative on success. */
 int
-dirvote_add_signatures(const char *detached_signatures_body)
+dirvote_add_signatures(const char *detached_signatures_body,
+                       const char **msg)
 {
-  /*XXXX020 return value is senseless. */
   if (pending_consensus) {
-    const char *msg=NULL;
     log_notice(LD_DIR, "Got a signature. Adding it to the pending consensus.");
     return dirvote_add_signatures_to_pending_consensus(
-                                         detached_signatures_body, &msg);
+                                     detached_signatures_body, msg);
   } else {
     log_notice(LD_DIR, "Got a signature. Queueing it for the next consensus.");
     if (!pending_consensus_signature_list)
       pending_consensus_signature_list = smartlist_create();
     smartlist_add(pending_consensus_signature_list,
                   tor_strdup(detached_signatures_body));
+    *msg = "Signature queued";
     return 0;
   }
 }
diff --git a/src/or/or.h b/src/or/or.h
index 25d3ace56..ad9fdf25e 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2939,7 +2939,8 @@ void dirvote_act(time_t now);
 struct pending_vote_t * dirvote_add_vote(const char *vote_body,
                                          const char **msg_out,
                                          int *status_out);
-int dirvote_add_signatures(const char *detached_signatures_body);
+int dirvote_add_signatures(const char *detached_signatures_body,
+                           const char **msg_out);
 
 /* Item access */
 const char *dirvote_get_pending_consensus(void);