Commit fixes for several pending tor core tasks: document all DOCDOCed functions; time out uncontrolled unattached streams; feed reasons to SOCKS5 (refactoring connection_ap_handshake_socks_reply in the process); change DirFetchPeriod/StatusFetchPeriod to have a special "Be smart" value.

svn:r3769
2024-11-24 12:23:32 +01:00 · 2005-03-17 12:38:37 +00:00 · 2005-03-17 12:38:37 +00:00 · df9c8feac7
commit df9c8feac7
parent b5a7347db5
17 changed files with 208 additions and 98 deletions
--- a/doc/TODO
+++ b/doc/TODO
@ -38,7 +38,9 @@ N . Switch to libevent
         o Implement patch, if so.
         - Implement Tor side once patch is accepted.
       o Check return from event_set, event_add, event_del.
-       - Keep pushing to get a windows patch accepted.
+       o Keep pushing to get a windows patch accepted.
+       - After about 26 March, check back with Niels; he should be back
+         by then.

 Security:
   - Make sure logged info is "safe"ish.
@ -72,13 +74,13 @@ N  . Implement pending controller features.
    - EXTENDCIRCUIT
 R     - revised circ selection stuff.
      - Implement controller interface.
-    . ATTACHSTREAM
+    o ATTACHSTREAM
      o Make streams have an 'unattached and not-automatically-attachable'
        state. ("Controller managed.")
      o Add support to put new streams into this state rather than try to
        attach them automatically.  ("Hidden" config option.)
      o Implement 'attach stream X to circuit Y' logic.
-      - Time out never-attached streams.
+      o Time out never-attached streams.
      o If we never get a CONNECTED back, we should put the stream back in
        CONTROLLER_WAIT, not in CIRCUIT_WAIT.
    - Tests for new controller features
@ -101,9 +103,9 @@ R o HTTPS proxy for OR CONNECT stuff. (For outgoing SSL connections to
      (Turns out, if package_raw_inbuf fails, it *can't* send an end cell.)
    o Check for any place where we can close an edge connection without
      sending an end; see if we should send an end.
-N . Feed end reason back into SOCK5 as reasonable.
+  o Feed end reason back into SOCK5 as reasonable.
 R o cache .foo.exit names better, or differently, or not.
-N - make !advertised_server_mode() ORs fetch dirs less often.
+  o make !advertised_server_mode() ORs fetch dirs less often.
 N - Clean up NT service code even more.  Document it. Enable it by default.
    Make sure it works.

--- a/doc/control-spec.txt
+++ b/doc/control-spec.txt
@ -346,8 +346,6 @@ the message.

 3.15 ATTACHSTREAM (Type 0x000E)

-  [Proposal; not finalized]
-
  Sent from the client to the server.  The message body contains two fields:
      Stream ID [4 octets]
      Circuit ID [4 octets]
@ -367,8 +365,6 @@ the message.

 3.16 POSTDESCRIPTOR (Type 0x000F)

-  [Proposal; not finalized]
-
  Sent from the client to the server.  The message body contains one field:
      Descriptor [NUL-terminated string]

@ -383,10 +379,8 @@ the message.

 3.17 FRAGMENTHEADER (Type 0x0010)

-  [Proposal; not finalized]
-
  Sent in either direction.  Used to encapsulate messages longer than 65535
-  bytes long.
+  bytes in length.

      Underlying type [2 bytes]
      Total Length    [4 bytes]
@ -403,10 +397,12 @@ the message.

 3.18 FRAGMENT (Type 0x0011)

-  [Proposal; not finalized]
-
      Data           [Entire message]

+  See FRAGMENTHEADER for more information
+
+3.19 
+
 4. Implementation notes

 4.1. There are four ways we could authenticate, for now:
--- a/doc/tor.1.in
+++ b/doc/tor.1.in
@ -114,11 +114,14 @@ security.
 \fBDirFetchPeriod \fR\fIN\fR \fBseconds\fR|\fBminutes\fR|\fBhours\fR|\fBdays\fR|\fBweeks\fP
 Every time the specified period elapses, Tor downloads a directory.
 A directory contains a signed list of all known servers as well as
-their current liveness status.  (Default: 1 hour)
+their current liveness status. A value of "0 seconds" tells Tor to choose an
+appropriate default. (Default: 1 hour for clients, 20 minutes for servers.)
 .TP
-\fBStatusFetchPeriod \fR\fIN\fR \fBseconds\fR|\fBminutes\fR|\fBhours\fR|\fBdays\fR|\fBweeks\fP
-Every time the specified period elapses, Tor downloads signed status
-information about the current state of known servers.  (Default: 20 minutes.)
+\fBStatusFetchPeriod \fR\fIN\fR \fBseconds\fR|\fBminutes\fR|\fBhours\fR|\fBdays\fR|\fBweeks\fP Every time the
+specified period elapses, Tor downloads signed status information about the
+current state of known servers.  A value of "0 seconds" tells Tor to choose
+an appropriate default. (Default: 30 minutes for clients, 15 minutes for
+servers.)  (Default: 20 minutes.)
 .TP
 \fBRendPostPeriod \fR\fIN\fR \fBseconds\fR|\fBminutes\fR|\fBhours\fR|\fBdays\fR|\fBweeks\fP
 Every time the specified period elapses, Tor uploads any rendezvous
--- a/src/common/log.c
+++ b/src/common/log.c
@ -55,6 +55,9 @@ static void delete_log(logfile_t *victim);
 static void close_log(logfile_t *victim);
 static int reset_log(logfile_t *lf);

+/** Helper: Write the standard prefix for log lines to a
+ * <b>buf_len</b> character buffer in <b>buf</b>.
+ */
 static INLINE size_t
 _log_prefix(char *buf, size_t buf_len, int severity)
 {
@ -290,7 +293,8 @@ static void delete_log(logfile_t *victim) {
  tor_free(victim);
 }

-/** DOCDOC */
+/** Helper: release system resources (but not memory) held by a single
+ * logfile_t. */
 static void close_log(logfile_t *victim)
 {
  if (victim->needs_close && victim->file) {
@ -304,7 +308,9 @@ static void close_log(logfile_t *victim)
  }
 }

-/** DOCDOC */
+/** Helper: reset a single logfile_t.  For a file log, this involves
+ * closing and reopening the log, and maybe writing the version.  For
+ * other log types, do nothing. */
 static int reset_log(logfile_t *lf)
 {
  if (lf->needs_close) {
@ -342,6 +348,11 @@ void add_temp_log(void)
  logfiles->is_temporary = 1;
 }

+/**
+ * Add a log handler to send messages of severity between
+ * <b>logLevelmin</b> and <b>logLevelMax</b> to the function
+ * <b>cb</b>.
+ */
 int add_callback_log(int loglevelMin, int loglevelMax, log_callback cb)
 {
  logfile_t *lf;
@ -437,11 +448,13 @@ int parse_log_level(const char *level) {
  return -1;
 }

+/** Return the string equivalent of a given log level. */
 const char *log_level_to_string(int level)
 {
  return sev_to_string(level);
 }

+/** Return the least severe log level that any current log is interested in. */
 int get_min_log_level(void)
 {
  logfile_t *lf;
--- a/src/common/util.c
+++ b/src/common/util.c
@ -1288,6 +1288,10 @@ parse_addr_and_port_range(const char *s, uint32_t *addr_out,
  return -1;
 }

+/** Given an IPv4 address <b>in</b> (in network order, as usual),
+ * write it as a string into the <b>buf_len</b>-byte buffer in
+ * <b>buf</b>.
+ */
 int
 tor_inet_ntoa(struct in_addr *in, char *buf, size_t buf_len)
 {
@ -1299,7 +1303,8 @@ tor_inet_ntoa(struct in_addr *in, char *buf, size_t buf_len)
                      (int)(uint8_t)((a    )&0xff));
 }

-/* DOCDOC */
+/* Return true iff <b>name</b> looks like it might be a hostname or IP
+ * address of some kind. */
 int
 is_plausible_address(const char *name)
 {
@ -1437,4 +1442,3 @@ void write_pidfile(char *filename) {
  }
 #endif
 }
-
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@ -183,7 +183,8 @@ circuit_free_cpath_node(crypt_path_t *victim) {
  tor_free(victim);
 }

-/** DOCDOC **/
+/** Return the circuit whose global ID is <b>id</b>, or NULL if no
+ * such circuit exists. */
 circuit_t *
 circuit_get_by_global_id(uint32_t id)
 {
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@ -975,7 +975,9 @@ consider_recording_trackhost(connection_t *conn, circuit_t *circ) {
                      time(NULL) + options->TrackHostExitsExpire);
 }

-/* DOCDOC. Return as for connection_ap_handshake_attach_chosen_circuit. */
+/** Attempt to attach the connection <b>conn</b> to <b>circ</b>, and
+ * send a begin or resolve cell as appropriate.  Return values for
+ * connection_ap_handshake_attach_chosen_circuit. */
 int
 connection_ap_handshake_attach_chosen_circuit(connection_t *conn,
                                              circuit_t *circ)
--- a/src/or/config.c
+++ b/src/or/config.c
@ -108,9 +108,8 @@ static config_var_t config_vars[] = {
  VAR("DirAllowPrivateAddresses",BOOL, DirAllowPrivateAddresses, NULL),
  VAR("DirPort",             UINT,     DirPort,              "0"),
  VAR("DirBindAddress",      LINELIST, DirBindAddress,       NULL),
-/* XXX we'd like dirfetchperiod to be higher for people with dirport not
- * set, but low for people with dirport set. how to have two defaults? */
-  VAR("DirFetchPeriod",      INTERVAL, DirFetchPeriod,       "1 hour"),
+  /** DOCDOC **/
+  VAR("DirFetchPeriod",      INTERVAL, DirFetchPeriod,       "0 seconds"),
  VAR("DirPostPeriod",       INTERVAL, DirPostPeriod,        "20 minutes"),
  VAR("RendPostPeriod",      INTERVAL, RendPostPeriod,       "20 minutes"),
  VAR("DirPolicy",           LINELIST, DirPolicy,            NULL),
@ -169,9 +168,8 @@ static config_var_t config_vars[] = {
  VAR("SocksPort",           UINT,     SocksPort,            "9050"),
  VAR("SocksBindAddress",    LINELIST, SocksBindAddress,     NULL),
  VAR("SocksPolicy",         LINELIST, SocksPolicy,          NULL),
-/* XXX as with dirfetchperiod, we want this to be 15 minutes for people
- * with a dirport open, but higher for people without a dirport open. */
-  VAR("StatusFetchPeriod",   INTERVAL, StatusFetchPeriod,    "15 minutes"),
+  /** DOCDOC */
+  VAR("StatusFetchPeriod",   INTERVAL, StatusFetchPeriod,    "0 seconds"),
  VAR("SysLog",              LINELIST_S, OldLogOptions,      NULL),
  OBSOLETE("TrafficShaping"),
  VAR("User",                STRING,   User,                 NULL),
@ -1375,11 +1373,13 @@ options_validate(or_options_t *options)
 #define MAX_CACHE_DIR_FETCH_PERIOD 3600
 #define MAX_CACHE_STATUS_FETCH_PERIOD 900

-  if (options->DirFetchPeriod < MIN_DIR_FETCH_PERIOD) {
+  if (options->DirFetchPeriod &&
+      options->DirFetchPeriod < MIN_DIR_FETCH_PERIOD) {
    log(LOG_WARN, "DirFetchPeriod option must be at least %d seconds. Clipping.", MIN_DIR_FETCH_PERIOD);
    options->DirFetchPeriod = MIN_DIR_FETCH_PERIOD;
  }
-  if (options->StatusFetchPeriod < MIN_STATUS_FETCH_PERIOD) {
+  if (options->StatusFetchPeriod &&
+      options->StatusFetchPeriod < MIN_STATUS_FETCH_PERIOD) {
    log(LOG_WARN, "StatusFetchPeriod option must be at least %d seconds. Clipping.", MIN_STATUS_FETCH_PERIOD);
    options->StatusFetchPeriod = MIN_STATUS_FETCH_PERIOD;
  }
--- a/src/or/connection.c
+++ b/src/or/connection.c
@ -283,8 +283,7 @@ void connection_about_to_close_connection(connection_t *conn)
        conn->hold_open_until_flushed = 1;
        /* XXX this socks_reply never gets sent, since conn
         * gets removed right after this function finishes. */
-        connection_ap_handshake_socks_reply(conn, NULL, 0, -1);
-        conn->socks_request->has_finished = 1;
+        connection_ap_handshake_socks_reply(conn, NULL, 0, SOCKS5_GENERAL_ERROR);
      } else {
        control_event_stream_status(conn, STREAM_EVENT_CLOSED);
      }
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@ -162,7 +162,10 @@ connection_edge_end(connection_t *conn, char reason, crypt_path_t *cpath_layer)
  return 0;
 }

-/** DOCDOC **/
+/** An error has just occured on an operation on an edge connection
+ * <b>conn</b>.  Extract the errno; convert it to an end reason, and send
+ * an appropriate relay end cell to <b>cpath_layer</b>.
+ **/
 int
 connection_edge_end_errno(connection_t *conn, crypt_path_t *cpath_layer)
 {
@ -265,7 +268,15 @@ void connection_ap_expire_beginning(void) {
    conn = carray[i];
    if (conn->type != CONN_TYPE_AP)
      continue;
-    if (conn->state != AP_CONN_STATE_RESOLVE_WAIT &&
+    if (conn->state == AP_CONN_STATE_CONTROLLER_WAIT) {
+      if (now - conn->timestamp_lastread >= 120) {
+        log_fn(LOG_NOTICE, "Closing unattached stream.");
+        connection_mark_for_close(conn);
+      }
+      continue;
+    }
+
+    else if (conn->state != AP_CONN_STATE_RESOLVE_WAIT &&
        conn->state != AP_CONN_STATE_CONNECT_WAIT)
      continue;
    if (now - conn->timestamp_lastread < 15)
@ -338,17 +349,21 @@ void connection_ap_attach_pending(void)
  }
 }

-/** DOCDOC
- * -1 on err, 1 on success, 0 on not-yet-sure.
+/** The AP connection <b>conn</b> has just failed while attaching or
+ * sending a BEGIN or resolving on <b>circ</b>, but another circuit
+ * might work.  Detach the circuit, and either reattach it, launch a
+ * new circuit, tell the controller, or give up as a appropriate.
+ *
+ * Returns -1 on err, 1 on success, 0 on not-yet-sure.
 */
 int
 connection_ap_detach_retriable(connection_t *conn, circuit_t *circ)
 {
  control_event_stream_status(conn, STREAM_EVENT_FAILED_RETRIABLE);
+  conn->timestamp_lastread = time(NULL);
  if (get_options()->ManageConnections) {
    conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
    circuit_detach_stream(circ,conn);
-    /* Muck with timestamps? */
    return connection_ap_handshake_attach_circuit(conn);
  } else {
    conn->state = AP_CONN_STATE_CONTROLLER_WAIT;
@ -678,7 +693,15 @@ addressmap_get_virtual_address(int type)
  }
 }

-/* DOCDOC */
+/** A controller has requested that we map some address of type
+ * <b>type</b> to the address <b>new_address</b>.  Choose an address
+ * that is unlikely to be used, and map it, and return it in a newly
+ * allocated string.  If another address of the same type is already
+ * mapped to <b>new_address</b>, try to return a copy of that address.
+ *
+ * The string in <b>new_address</b> may be freed, or inserted into a map
+ * as appropriate.
+ **/
 char *
 addressmap_register_virtual_address(int type, char *new_address)
 {
@ -720,7 +743,7 @@ address_is_invalid_destination(const char *address) {
  return 0;
 }

-/* Iterate over all address mapings which have exipry times between
+/** Iterate over all address mapings which have exipry times between
 * min_expires and max_expires, inclusive.  If sl is provided, add an
 * "old-addr new-addr" string to sl for each mapping.  If sl is NULL,
 * remove the mappings.
@ -779,19 +802,23 @@ static int connection_ap_handshake_process_socks(connection_t *conn) {
  log_fn(LOG_DEBUG,"entered.");

  sockshere = fetch_from_buf_socks(conn->inbuf, socks);
-  if (sockshere == -1 || sockshere == 0) {
-    if (socks->replylen) { /* we should send reply back */
-      log_fn(LOG_DEBUG,"reply is already set for us. Using it.");
-      connection_ap_handshake_socks_reply(conn, socks->reply, socks->replylen, 0);
-      socks->replylen = 0; /* zero it out so we can do another round of negotiation */
-    } else if (sockshere == -1) { /* send normal reject */
-      log_fn(LOG_WARN,"Fetching socks handshake failed. Closing.");
-      connection_ap_handshake_socks_reply(conn, NULL, 0, -1);
+  if (sockshere == 0) {
+    if (socks->replylen) {
+      connection_write_to_buf(socks->reply, socks->replylen, conn);
    } else {
      log_fn(LOG_DEBUG,"socks handshake not all here yet.");
    }
-    if (sockshere == -1)
-      socks->has_finished = 1;
+    return 0;
+  } else if (sockshere == -1) {
+    if (socks->replylen) { /* we should send reply back */
+      log_fn(LOG_DEBUG,"reply is already set for us. Using it.");
+      connection_ap_handshake_socks_reply(conn, socks->reply, socks->replylen,
+                                          SOCKS5_GENERAL_ERROR);
+      socks->replylen = 0; /* zero it out so we can do another round of negotiation */
+    } else {
+      log_fn(LOG_WARN,"Fetching socks handshake failed. Closing.");
+      connection_ap_handshake_socks_reply(conn, NULL, 0, SOCKS5_GENERAL_ERROR);
+    }
    return sockshere;
  } /* else socks handshake is done, continue processing */

@ -1080,7 +1107,11 @@ int connection_ap_make_bridge(char *address, uint16_t port) {
  return fd[1];
 }

-/* DOCDOC */
+/** Send an answer to an AP connection that has requested a DNS lookup
+ * via SOCKS.  The type should be one of RESOLVED_TYPE_(IPV4|IPV6) or
+ * -1 for unreachable; the answer should be in the format specified
+ * in the socks extensions document.
+ **/
 void connection_ap_handshake_socks_resolved(connection_t *conn,
                                            int answer_type,
                                            size_t answer_len,
@ -1133,14 +1164,14 @@ void connection_ap_handshake_socks_resolved(connection_t *conn,
  }
  connection_ap_handshake_socks_reply(conn, buf, replylen,
          (answer_type == RESOLVED_TYPE_IPV4 ||
-                                      answer_type == RESOLVED_TYPE_IPV6) ? 1 : -1);
+           answer_type == RESOLVED_TYPE_IPV6) ?
+                               SOCKS5_SUCCEEDED : SOCKS5_HOST_UNREACHABLE);
  conn->socks_request->has_finished = 1;
 }

 /** Send a socks reply to stream <b>conn</b>, using the appropriate
- * socks version, etc.
- *
- * Status can be 1 (succeeded), -1 (failed), or 0 (not sure yet).
+ * socks version, etc, and mark <b>conn</b> as completed with SOCKS
+ * handshaking.
 *
 * If <b>reply</b> is defined, then write <b>replylen</b> bytes of it
 * to conn and return, else reply based on <b>status</b>.
@ -1148,32 +1179,34 @@ void connection_ap_handshake_socks_resolved(connection_t *conn,
 * If <b>reply</b> is undefined, <b>status</b> can't be 0.
 */
 void connection_ap_handshake_socks_reply(connection_t *conn, char *reply,
-                                         size_t replylen, int status) {
+                                         size_t replylen,
+                                         socks5_reply_status_t status) {
  char buf[256];

-  if (status) /* it's either 1 or -1 */
  control_event_stream_status(conn,
-                       status==1 ? STREAM_EVENT_SUCCEEDED : STREAM_EVENT_FAILED);
+     status==SOCKS5_SUCCEEDED ? STREAM_EVENT_SUCCEEDED : STREAM_EVENT_FAILED);

-  if (replylen) { /* we already have a reply in mind */
-    connection_write_to_buf(reply, replylen, conn);
+  tor_assert(conn->socks_request);
+  if (conn->socks_request->has_finished) {
+    log_fn(LOG_WARN, "Harmless bug: duplicate calls to connection_ap_handshake_socks_reply.");
+    return;
+  }
+  if (replylen) { /* we already have a reply in mind */
+    connection_write_to_buf(reply, replylen, conn);
+    conn->socks_request->has_finished = 1;
    return;
  }
-  tor_assert(conn->socks_request);
-  tor_assert(status == 1 || status == -1);
  if (conn->socks_request->socks_version == 4) {
    memset(buf,0,SOCKS4_NETWORK_LEN);
 #define SOCKS4_GRANTED          90
 #define SOCKS4_REJECT           91
-    buf[1] = (status==1 ? SOCKS4_GRANTED : SOCKS4_REJECT);
+    buf[1] = (status==SOCKS5_SUCCEEDED ? SOCKS4_GRANTED : SOCKS4_REJECT);
    /* leave version, destport, destip zero */
    connection_write_to_buf(buf, SOCKS4_NETWORK_LEN, conn);
  }
  if (conn->socks_request->socks_version == 5) {
    buf[0] = 5; /* version 5 */
-#define SOCKS5_SUCCESS          0
-#define SOCKS5_GENERIC_ERROR    1
-    buf[1] = status==1 ? SOCKS5_SUCCESS : SOCKS5_GENERIC_ERROR;
+    buf[1] = (char)status;
    buf[2] = 0;
    buf[3] = 1; /* ipv4 addr */
    memset(buf+4,0,6); /* Set external addr/port to 0.
@ -1182,6 +1215,7 @@ void connection_ap_handshake_socks_reply(connection_t *conn, char *reply,
  }
  /* If socks_version isn't 4 or 5, don't send anything.
   * This can happen in the case of AP bridges. */
+  conn->socks_request->has_finished = 1;
  return;
 }

--- a/src/or/control.c
+++ b/src/or/control.c
@ -926,7 +926,10 @@ control_event_logmsg(int severity, const char *msg)
  send_control_event(EVENT_WARNING, (uint32_t)(len+1), msg);
 }

-/** DOCDOC */
+/** Called whenever we receive new router descriptors: tell any
+ * interested control connections.  <b>routers</b> is a list of
+ * DIGEST_LEN-byte identity digests.
+ */
 int control_event_descriptors_changed(smartlist_t *routers)
 {
  size_t len;
--- a/src/or/main.c
+++ b/src/or/main.c
@ -195,7 +195,7 @@ static void connection_unlink(connection_t *conn, int remove) {
  connection_free(conn);
 }

-/** DOCDOC **/
+/** Schedule <b>conn</b> to be closed. **/
 void
 add_connection_to_closeable_list(connection_t *conn)
 {
@ -318,7 +318,7 @@ void connection_start_writing(connection_t *conn) {
           (int)conn->s);
 }

-/** DOCDOC */
+/** Close all connections that have been schedule to get closed */
 static void
 close_closeable_connections(void)
 {
@ -337,7 +337,8 @@ close_closeable_connections(void)
  }
 }

-/** DOCDOC */
+/** Libevent callback: this gets invoked when (connection_t*)<b>conn</b> has
+ * some data to read. */
 static void
 conn_read_callback(int fd, short event, void *_conn)
 {
@ -371,6 +372,8 @@ conn_read_callback(int fd, short event, void *_conn)
    close_closeable_connections();
 }

+/** Libevent callback: this gets invoked when (connection_t*)<b>conn</b> has
+ * some data to write. */
 static void conn_write_callback(int fd, short events, void *_conn)
 {
  connection_t *conn = _conn;
@ -512,7 +515,6 @@ static void conn_write(int i) {
 *    - Otherwise, remove the connection from connection_array and from
 *      all other lists, close it, and free it.
 * Returns 1 if the connection was closed, 0 otherwise.
- * DOCDOC closeable_list
 */
 static int conn_close_if_marked(int i) {
  connection_t *conn;
@ -580,6 +582,35 @@ void directory_all_unreachable(time_t now) {
  }
 }

+INLINE int
+get_dir_fetch_period(or_options_t *options)
+{
+  if (options->DirFetchPeriod)
+    /* Value from config file. */
+    return options->DirFetchPeriod;
+  else if (options->DirPort)
+    /* Default for directory server */
+    return 20*60;
+  else
+    /* Default for average user. */
+    return 40*60;
+}
+
+INLINE int
+get_status_fetch_period(or_options_t *options)
+{
+  if (options->StatusFetchPeriod)
+    /* Value from config file. */
+    return options->StatusFetchPeriod;
+  else if (options->DirPort)
+    /* Default for directory server */
+    return 15*60;
+  else
+    /* Default for average user. */
+    return 30*60;
+}
+
+
 /** This function is called whenever we successfully pull down a directory.
 * If <b>identity_digest</b> is defined, it contains the digest of the
 * router that just gave us this directory. */
@ -593,13 +624,13 @@ void directory_has_arrived(time_t now, char *identity_digest) {
   * after the directory we had when we started.
   */
  if (!time_to_fetch_directory)
-    time_to_fetch_directory = now + options->DirFetchPeriod;
+    time_to_fetch_directory = now + get_dir_fetch_period(options);

  if (!time_to_force_upload_descriptor)
    time_to_force_upload_descriptor = now + options->DirPostPeriod;

  if (!time_to_fetch_running_routers)
-    time_to_fetch_running_routers = now + options->StatusFetchPeriod;
+    time_to_fetch_running_routers = now + get_status_fetch_period(options);

  if (identity_digest) { /* if this is us, then our dirport is reachable */
    routerinfo_t *router = router_get_by_digest(identity_digest);
@ -720,6 +751,7 @@ static void run_scheduled_events(time_t now) {
   * new running-routers list, and/or force-uploading our descriptor
   * (if we've passed our internal checks). */
  if (time_to_fetch_directory < now) {
+    time_t next_status_fetch;
    /* purge obsolete entries */
    routerlist_remove_old_routers(ROUTER_MAX_AGE);

@ -734,9 +766,10 @@ static void run_scheduled_events(time_t now) {
    }

    directory_get_from_dirserver(DIR_PURPOSE_FETCH_DIR, NULL, 1);
-    time_to_fetch_directory = now + options->DirFetchPeriod;
-    if (time_to_fetch_running_routers < now + options->StatusFetchPeriod) {
-      time_to_fetch_running_routers = now + options->StatusFetchPeriod;
+    time_to_fetch_directory = now + get_dir_fetch_period(options);
+    next_status_fetch = now + get_status_fetch_period(options);
+    if (time_to_fetch_running_routers < next_status_fetch) {
+      time_to_fetch_running_routers = next_status_fetch;
    }

    /* Also, take this chance to remove old information from rephist. */
@ -747,7 +780,7 @@ static void run_scheduled_events(time_t now) {
    if (!authdir_mode(options)) {
      directory_get_from_dirserver(DIR_PURPOSE_FETCH_RUNNING_LIST, NULL, 1);
    }
-    time_to_fetch_running_routers = now + options->StatusFetchPeriod;
+    time_to_fetch_running_routers = now + get_status_fetch_period(options);
  }

  if (time_to_force_upload_descriptor < now) {
@ -816,7 +849,7 @@ static void run_scheduled_events(time_t now) {
  close_closeable_connections();
 }

-/** DOCDOC */
+/** Libevent callback: invoked once every second. */
 static void second_elapsed_callback(int fd, short event, void *args)
 {
  static struct event *timeout_event = NULL;
--- a/src/or/or.h
+++ b/src/or/or.h
@ -700,8 +700,7 @@ typedef struct {
   */
  time_t published_on;
  time_t running_routers_updated_on;
-  /** DOCDOC
-   */
+  /** What is the most recently received running_routers structure? */
  running_routers_t *running_routers;
  /** Which router is claimed to have signed it? */
  char *signing_router;
@ -1308,7 +1307,8 @@ int connection_ap_handshake_send_resolve(connection_t *ap_conn, circuit_t *circ)

 int connection_ap_make_bridge(char *address, uint16_t port);
 void connection_ap_handshake_socks_reply(connection_t *conn, char *reply,
-                                         size_t replylen, int status);
+                                         size_t replylen,
+                                         socks5_reply_status_t status);
 void connection_ap_handshake_socks_resolved(connection_t *conn,
                                            int answer_type,
                                            size_t answer_len,
--- a/src/or/relay.c
+++ b/src/or/relay.c
@ -481,8 +481,11 @@ connection_edge_end_reason_str(char *payload, uint16_t length) {
  }
 }

-socks5_reply_status_t
-connection_edge_end_reason_sock5_response(char *payload, uint16_t length) {
+/** Translate the <b>payload</b> of length <b>length</b>, which
+ * came from a relay 'end' cell, into an appropriate SOCKS5 reply code.
+ */
+static socks5_reply_status_t
+connection_edge_end_reason_socks5_response(char *payload, uint16_t length) {
  if (length < 1) {
    log_fn(LOG_WARN,"End cell arrived with length 0. Should be at least 1.");
    return SOCKS5_GENERAL_ERROR;
@ -641,8 +644,6 @@ connection_edge_process_relay_cell_not_open(
        circuit_log_path(LOG_INFO,circ);
        tor_assert(circ->timestamp_dirty);
        circ->timestamp_dirty -= get_options()->MaxCircuitDirtiness;
-        /* make sure not to expire/retry the stream quite yet */
-        conn->timestamp_lastread = time(NULL);

        if (connection_ap_detach_retriable(conn, circ) >= 0)
          return 0;
@ -685,8 +686,7 @@ connection_edge_process_relay_cell_not_open(
                                conn->chosen_exit_name);
    }
    circuit_log_path(LOG_INFO,circ);
-    connection_ap_handshake_socks_reply(conn, NULL, 0, 1);
-    conn->socks_request->has_finished = 1;
+    connection_ap_handshake_socks_reply(conn, NULL, 0, SOCKS5_SUCCEEDED);
    /* handle anything that might have queued */
    if (connection_edge_package_raw_inbuf(conn, 1) < 0) {
      /* (We already sent an end cell if possible) */
@ -821,7 +821,12 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
             connection_edge_end_reason_str(cell->payload+RELAY_HEADER_SIZE,
                                            rh.length),
             conn->stream_id, (int)conn->stream_size);
-
+      if (conn->socks_request && !conn->socks_request->has_finished) {
+        socks5_reply_status_t status =
+          connection_edge_end_reason_socks5_response(
+                              cell->payload+RELAY_HEADER_SIZE, rh.length);
+        connection_ap_handshake_socks_reply(conn, NULL, 0, status);
+      }
 #ifdef HALF_OPEN
      conn->done_sending = 1;
      shutdown(conn->s, 1); /* XXX check return; refactor NM */
--- a/src/or/router.c
+++ b/src/or/router.c
@ -57,6 +57,9 @@ crypto_pk_env_t *get_previous_onion_key(void) {
  return lastonionkey;
 }

+/** Store a copy of the current onion key into *<b>key</b>, and a copy
+ * of the most recent onion key into *<b>last</b>.
+ */
 void dup_onion_keys(crypto_pk_env_t **key, crypto_pk_env_t **last)
 {
  tor_assert(key);
@ -516,6 +519,8 @@ void router_retry_connections(void) {
  }
 }

+/** Return true iff this OR should try to keep connections open to all
+ * other ORs. */
 int router_is_clique_mode(routerinfo_t *router) {
  if (router_digest_is_trusted_dir(router->identity_digest))
    return 1;
@ -533,8 +538,9 @@ static int desc_is_dirty = 1;
 /** Boolean: do we need to regenerate the above? */
 static int desc_needs_upload = 0;

-/** OR only: try to upload our signed descriptor to all the directory servers
- * we know about. DOCDOC force
+/** OR only: If <b>force</b> is true, or we haven't uploaded this
+ * descriptor successfully yet, try to upload our signed descriptor to
+ * all the directory servers we know about.
 */
 void router_upload_dir_desc_to_dirservers(int force) {
  const char *s;
@ -630,8 +636,9 @@ const char *router_get_my_descriptor(void) {
  return desc_routerinfo->signed_descriptor;
 }

-/** Rebuild a fresh routerinfo and signed server descriptor for this
- * OR.  Return 0 on success, -1 on error. DOCDOC force
+/** If <b>force</b> is true, or our descriptor is out-of-date, rebuild
+ * a fresh routerinfo and signed server descriptor for this OR.
+ * Return 0 on success, -1 on error.
 */
 int router_rebuild_descriptor(int force) {
  routerinfo_t *ri;
@ -694,7 +701,7 @@ int router_rebuild_descriptor(int force) {
  return 0;
 }

-/** DOCDOC */
+/** Call when the current descriptor is out of date. */
 void
 mark_my_descriptor_dirty(void)
 {
@ -919,6 +926,7 @@ int router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
  return written+1;
 }

+/** Return true iff <b>s</b> is a legally valid server nickname. */
 int is_legal_nickname(const char *s)
 {
  size_t len;
@ -927,6 +935,8 @@ int is_legal_nickname(const char *s)
  return len > 0 && len <= MAX_NICKNAME_LEN &&
    strspn(s,LEGAL_NICKNAME_CHARACTERS)==len;
 }
+/** Return true iff <b>s</b> is a legally valid server nickname or
+ * hex-encoded identity-key digest. */
 int is_legal_nickname_or_hexdigest(const char *s)
 {
  size_t len;
@ -938,6 +948,7 @@ int is_legal_nickname_or_hexdigest(const char *s)
  return len == HEX_DIGEST_LEN+1 && strspn(s+1,HEX_CHARACTERS)==len-1;
 }

+/** Release all resources held in router keys. */
 void router_free_all_keys(void)
 {
  if (onionkey)
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@ -1192,7 +1192,8 @@ void running_routers_free(running_routers_t *rr)
  tor_free(rr);
 }

-/** DOCDOC*/
+/** We've just got a running routers list in <b>rr</b>; update the
+ * status of the routers in <b>list</b>, and cache <b>rr</b> */
 void
 routerlist_set_runningrouters(routerlist_t *list, running_routers_t *rr)
 {
@ -1389,6 +1390,7 @@ add_trusted_dir_server(const char *address, uint16_t port, const char *digest)
  smartlist_add(trusted_dir_servers, ent);
 }

+/** Remove all members from the list of trusted dir servers. */
 void clear_trusted_dir_servers(void)
 {
  if (trusted_dir_servers) {
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@ -541,7 +541,9 @@ router_parse_routerlist_from_directory(const char *str,
  return r;
 }

-/* DOCDOC */
+/** Read a signed router status statement from <b>str</b>.  On
+ * success, return it, and cache the original string if
+ * <b>write_to_cache</b> is set.  Otherwise, return NULL.  */
 running_routers_t *
 router_parse_runningrouters(const char *str, int write_to_cache)
 {