bugfixes and note missing features

deal with content-length headers better when reading http
don't assume struct socks4_info is a packed struct
fail the socks handshake if destip is zero
flesh out conn_state_to_string() for dir conn
fix typo (bug) in connection_handle_read()
directory get is now called fetch, post is now upload
reopen logs on sighup


svn:r475

src/or/buffers.c

@@ -230,9 +230,8 @@ int fetch_from_buf_http(char *buf, int *buf_datalen,
     return -1;
   }
 
-#define CONTENT_LENGTH "Content-Length: "
+#define CONTENT_LENGTH "\r\nContent-Length: "
   i = find_on_inbuf(CONTENT_LENGTH, strlen(CONTENT_LENGTH), headers, headerlen);
-  /* This includes headers like Not-Content-Length. But close enough. */
   if(i > 0) {
     contentlen = atoi(headers+i);
     if(bodylen < contentlen) {
@@ -265,13 +264,13 @@ int fetch_from_buf_http(char *buf, int *buf_datalen,
  *   then pull the handshake off the buf, assign to addr_out and port_out,
  *   and return 1.
  * If it's invalid or too big, return -1.
- * Else it's not all there yet, change nothing return 0.
+ * Else it's not all there yet, change nothing and return 0.
  */
 int fetch_from_buf_socks(char *buf, int *buf_datalen,
                          char *addr_out, int max_addrlen,
                          uint16_t *port_out) {
-  socks4_t *socks4_info;
-  char tmpbuf[512];
+  socks4_t socks4_info;
+  char *tmpbuf=NULL;
   uint16_t port;
   enum {socks4, socks4a } socks_prot = socks4a;
   char *next, *startaddr;
@@ -279,38 +278,47 @@ int fetch_from_buf_socks(char *buf, int *buf_datalen,
   if(*buf_datalen < sizeof(socks4_t)) /* basic info available? */
     return 0; /* not yet */
 
-  socks4_info = (socks4_t *)buf;
+  /* an inlined socks4_unpack() */
+  socks4_info.version = *buf;
+  socks4_info.command = *(buf+1);
+  socks4_info.destport = *(uint16_t*)(buf+2); 
+  socks4_info.destip = *(uint32_t*)(buf+4);
 
-  if(socks4_info->version != 4) {
-    log_fn(LOG_NOTICE,"Unrecognized version %d.",socks4_info->version);
+  if(socks4_info.version != 4) {
+    log_fn(LOG_NOTICE,"Unrecognized version %d.",socks4_info.version);
     return -1;
   }
 
-  if(socks4_info->command != 1) { /* not a connect? we don't support it. */
-    log_fn(LOG_NOTICE,"command %d not '1'.",socks4_info->command);
+  if(socks4_info.command != 1) { /* not a connect? we don't support it. */
+    log_fn(LOG_NOTICE,"command %d not '1'.",socks4_info.command);
     return -1;
   }
 
-  port = ntohs(*(uint16_t*)&socks4_info->destport);
+  port = ntohs(socks4_info.destport);
   if(!port) {
     log_fn(LOG_NOTICE,"Port is zero.");
     return -1;
   }
 
-  if(socks4_info->destip[0] || socks4_info->destip[1] ||
-     socks4_info->destip[2] || !socks4_info->destip[3]) { /* not 0.0.0.x */
+  if(!socks4_info.destip) {
+    log_fn(LOG_NOTICE,"DestIP is zero.");
+    return -1;
+  }
+
+  if(socks4_info.destip >> 8) {
+    struct in_addr in;
     log_fn(LOG_NOTICE,"destip not in form 0.0.0.x.");
-    sprintf(tmpbuf, "%d.%d.%d.%d", socks4_info->destip[0],
-      socks4_info->destip[1], socks4_info->destip[2], socks4_info->destip[3]);
+    in.s_addr = htonl(socks4_info.destip);
+    tmpbuf = inet_ntoa(in);
     if(max_addrlen <= strlen(tmpbuf)) {
-      log_fn(LOG_DEBUG,"socks4-addr too long.");
+      log_fn(LOG_DEBUG,"socks4 addr too long.");
       return -1;
     }
     log_fn(LOG_DEBUG,"Successfully read destip (%s)", tmpbuf);
     socks_prot = socks4;
   }
 
-  next = memchr(buf+sizeof(socks4_t), 0, *buf_datalen);
+  next = memchr(buf+SOCKS4_NETWORK_LEN, 0, *buf_datalen);
   if(!next) {
     log_fn(LOG_DEBUG,"Username not here yet.");
     return 0;

src/or/connection.c

@@ -47,11 +47,14 @@ char *conn_state_to_string[][15] = {
     "waiting for OR connection",       /* 4 */
     "open" },                          /* 5 */
   { "ready" }, /* dir listener, 0 */
-  { "connecting",                      /* 0 */
-    "sending command",                 /* 1 */
-    "reading",                         /* 2 */
-    "awaiting command",                /* 3 */
-    "writing" },                       /* 4 */
+  { "connecting (fetch)",              /* 0 */
+    "connecting (upload)",             /* 1 */
+    "client sending fetch",            /* 2 */
+    "client sending upload",           /* 3 */
+    "client reading fetch",            /* 4 */
+    "client reading upload",           /* 5 */
+    "awaiting command",                /* 6 */
+    "writing" },                       /* 7 */
   { "idle",                /* dns worker, 0 */
     "busy" },                          /* 1 */
   { "idle",                /* cpu worker, 0 */
@@ -437,7 +440,8 @@ int connection_handle_read(connection_t *conn) {
 
   if(connection_read_to_buf(conn) < 0) {
     if(conn->type == CONN_TYPE_DIR && 
-      (conn->state == DIR_CONN_STATE_CONNECTING_GET || DIR_CONN_STATE_CONNECTING_POST)) {
+      (conn->state == DIR_CONN_STATE_CONNECTING_FETCH ||
+       conn->state == DIR_CONN_STATE_CONNECTING_UPLOAD)) {
        /* it's a directory server and connecting failed: forget about this router */
        /* XXX I suspect pollerr may make Windows not get to this point. :( */
        router_forget_router(conn->addr,conn->port); 

src/or/connection_edge.c

@@ -433,7 +433,7 @@ int connection_consider_sending_sendme(connection_t *conn, int edge_type) {
 
 static int connection_ap_handshake_process_socks(connection_t *conn) {
   circuit_t *circ;
-  char destaddr[200];
+  char destaddr[200]; /* XXX why 200? but not 256, because it won't fit in a cell */
   uint16_t destport;
 
   assert(conn);
@@ -511,16 +511,16 @@ static int connection_ap_handshake_send_begin(connection_t *ap_conn, circuit_t *
 }
 
 static int connection_ap_handshake_socks_reply(connection_t *conn, char result) {
-  socks4_t socks4_info; 
+  char buf[SOCKS4_NETWORK_LEN];
 
   assert(conn);
 
-  socks4_info.version = 0;
-  socks4_info.command = result;
-  socks4_info.destport[0] = socks4_info.destport[1] = 0;
-  socks4_info.destip[0] = socks4_info.destip[1] = socks4_info.destip[2] = socks4_info.destip[3] = 0;
+  /* an inlined socks4_pack() */
+  memset(buf,0,sizeof(buf));
+  buf[1] = result; /* command */
+  /* leave version, destport, destip zero */
 
-  if(connection_write_to_buf((char *)&socks4_info, sizeof(socks4_t), conn) < 0)
+  if(connection_write_to_buf(buf, sizeof(buf), conn) < 0)
     return -1;
   return connection_flush_buf(conn); /* try to flush it, in case we're about to close the conn */
 }

src/or/directory.c

@@ -18,8 +18,8 @@ static char the_directory[MAX_DIR_SIZE+1];
 static int directorylen=0;
 static int directory_dirty=1;
 
-static char getstring[] = "GET / HTTP/1.0\r\n\r\n";
-static char poststring[] = "POST / HTTP/1.0\r\n\r\n";
+static char fetchstring[] = "GET / HTTP/1.0\r\n\r\n";
+static char uploadstring[] = "POST / HTTP/1.0\r\n\r\n";
 static char answerstring[] = "HTTP/1.0 200 OK\r\n\r\n";
 
 /********* END VARIABLES ************/
@@ -35,10 +35,10 @@ void directory_initiate_command(routerinfo_t *router, int command) {
     return;
   }
 
-  if(command == DIR_CONN_STATE_CONNECTING_GET)
-    log_fn(LOG_DEBUG,"initiating directory get");
+  if(command == DIR_CONN_STATE_CONNECTING_FETCH)
+    log_fn(LOG_DEBUG,"initiating directory fetch");
   else
-    log_fn(LOG_DEBUG,"initiating directory post");
+    log_fn(LOG_DEBUG,"initiating directory upload");
 
   conn = connection_new(CONN_TYPE_DIR);
   if(!conn)
@@ -90,25 +90,25 @@ static int directory_send_command(connection_t *conn, int command) {
   assert(conn && conn->type == CONN_TYPE_DIR);
 
   switch(command) {
-    case DIR_CONN_STATE_CONNECTING_GET:
-      if(connection_write_to_buf(getstring, strlen(getstring), conn) < 0) {
-        log_fn(LOG_DEBUG,"Couldn't write get to buffer.");
+    case DIR_CONN_STATE_CONNECTING_FETCH:
+      if(connection_write_to_buf(fetchstring, strlen(fetchstring), conn) < 0) {
+        log_fn(LOG_DEBUG,"Couldn't write fetch to buffer.");
         return -1;
       }
-      conn->state = DIR_CONN_STATE_CLIENT_SENDING_GET;
+      conn->state = DIR_CONN_STATE_CLIENT_SENDING_FETCH;
       break;
-    case DIR_CONN_STATE_CONNECTING_POST:
+    case DIR_CONN_STATE_CONNECTING_UPLOAD:
       s = router_get_my_descriptor();
       if(!s) {
         log_fn(LOG_DEBUG,"Failed to get my descriptor.");
         return -1;
       }
-      if(connection_write_to_buf(poststring, strlen(poststring), conn) < 0 ||
+      if(connection_write_to_buf(uploadstring, strlen(uploadstring), conn) < 0 ||
          connection_write_to_buf(s, strlen(s), conn) < 0) {
         log_fn(LOG_DEBUG,"Couldn't write post/descriptor to buffer.");
         return -1;
       }
-      conn->state = DIR_CONN_STATE_CLIENT_SENDING_POST;
+      conn->state = DIR_CONN_STATE_CLIENT_SENDING_UPLOAD;
       break;
   }
   return 0;
@@ -139,18 +139,19 @@ int connection_dir_process_inbuf(connection_t *conn) {
 
   if(conn->inbuf_reached_eof) {
     switch(conn->state) {
-      case DIR_CONN_STATE_CLIENT_READING_GET:
+      case DIR_CONN_STATE_CLIENT_READING_FETCH:
         /* kill it, but first process the_directory and learn about new routers. */
         switch(fetch_from_buf_http(conn->inbuf,&conn->inbuf_datalen,
                                    NULL, 0, the_directory, MAX_DIR_SIZE)) {
           case -1: /* overflow */
-            log_fn(LOG_DEBUG,"'get' response too large. Failing.");
+            log_fn(LOG_DEBUG,"'fetch' response too large. Failing.");
             return -1;
           case 0:
-            log_fn(LOG_DEBUG,"'get' response not all here, but we're at eof. Closing.");
+            log_fn(LOG_DEBUG,"'fetch' response not all here, but we're at eof. Closing.");
             return -1;
           /* case 1, fall through */
         }
+        /* XXX check headers, at least make sure returned 2xx */
         directorylen = strlen(the_directory);
         log_fn(LOG_DEBUG,"Received directory (size %d):\n%s", directorylen, the_directory);
         if(directorylen == 0) {
@@ -167,9 +168,9 @@ int connection_dir_process_inbuf(connection_t *conn) {
           router_retry_connections();
         }
         return -1;
-      case DIR_CONN_STATE_CLIENT_READING_POST:
+      case DIR_CONN_STATE_CLIENT_READING_UPLOAD:
         /* XXX make sure there's a 200 OK on the buffer */
-        log_fn(LOG_DEBUG,"eof while reading post response. Finished.");
+        log_fn(LOG_DEBUG,"eof while reading upload response. Finished.");
         return -1;
       default:
         log_fn(LOG_DEBUG,"conn reached eof, not reading. Closing.");
@@ -205,6 +206,7 @@ static int directory_handle_command(connection_t *conn) {
 
   log_fn(LOG_DEBUG,"headers '%s', body '%s'.",headers,body);
   if(!strncasecmp(headers,"GET",3)) {
+    /* XXX should check url and http version */
 
     directory_rebuild(); /* rebuild it now, iff it's dirty */
 
@@ -224,6 +226,7 @@ static int directory_handle_command(connection_t *conn) {
   }
 
   if(!strncasecmp(headers,"POST",4)) {
+    /* XXX should check url and http version */
     log_fn(LOG_DEBUG,"Received POST command, body '%s'", body);
     if(connection_write_to_buf(answerstring, strlen(answerstring), conn) < 0) {
       log_fn(LOG_DEBUG,"Failed to write answerstring to outbuf.");
@@ -243,8 +246,8 @@ int connection_dir_finished_flushing(connection_t *conn) {
   assert(conn && conn->type == CONN_TYPE_DIR);
 
   switch(conn->state) {
-    case DIR_CONN_STATE_CONNECTING_GET:
-    case DIR_CONN_STATE_CONNECTING_POST:
+    case DIR_CONN_STATE_CONNECTING_FETCH:
+    case DIR_CONN_STATE_CONNECTING_UPLOAD:
       if (getsockopt(conn->s, SOL_SOCKET, SO_ERROR, (void*)&e, &len) < 0)  { /* not yet */
         if(!ERRNO_CONN_EINPROGRESS(errno)) {
           log_fn(LOG_DEBUG,"in-progress connect failed. Removing.");
@@ -260,14 +263,14 @@ int connection_dir_finished_flushing(connection_t *conn) {
           conn->address,conn->port);
 
       return directory_send_command(conn, conn->state);
-    case DIR_CONN_STATE_CLIENT_SENDING_GET:
-      log_fn(LOG_DEBUG,"client finished sending 'get' command.");
-      conn->state = DIR_CONN_STATE_CLIENT_READING_GET;
+    case DIR_CONN_STATE_CLIENT_SENDING_FETCH:
+      log_fn(LOG_DEBUG,"client finished sending fetch command.");
+      conn->state = DIR_CONN_STATE_CLIENT_READING_FETCH;
       connection_watch_events(conn, POLLIN);
       return 0;
-    case DIR_CONN_STATE_CLIENT_SENDING_POST:
-      log_fn(LOG_DEBUG,"client finished sending 'post' command.");
-      conn->state = DIR_CONN_STATE_CLIENT_READING_POST;
+    case DIR_CONN_STATE_CLIENT_SENDING_UPLOAD:
+      log_fn(LOG_DEBUG,"client finished sending upload command.");
+      conn->state = DIR_CONN_STATE_CLIENT_READING_UPLOAD;
       connection_watch_events(conn, POLLIN);
       return 0;
     case DIR_CONN_STATE_SERVER_WRITING:

src/or/main.c

@@ -25,7 +25,7 @@ static int nfds=0; /* number of connections currently active */
 
 #ifndef MS_WINDOWS /* do signal stuff only on unix */
 static int please_dumpstats=0; /* whether we should dump stats during the loop */
-static int please_fetch_directory=0; /* whether we should fetch a new directory */
+static int please_reset =0; /* whether we just got a sighup */
 static int please_reap_children=0; /* whether we should waitpid for exited children*/
 #endif /* signal stuff */
 
@@ -337,7 +337,7 @@ static int prepare_for_poll(void) {
         /* NOTE directory servers do not currently fetch directories.
          * Hope this doesn't bite us later.
          */
-        directory_initiate_command(router_pick_directory_server(), DIR_CONN_STATE_CONNECTING_GET);
+        directory_initiate_command(router_pick_directory_server(), DIR_CONN_STATE_CONNECTING_FETCH);
         time_to_fetch_directory = now.tv_sec + options.DirFetchPeriod;
       }
     }
@@ -488,15 +488,19 @@ static int do_main_loop(void) {
       dumpstats();
       please_dumpstats = 0;
     }
-    if(please_fetch_directory) {
+    if(please_reset) {
+      /* fetch a new directory */
       if(options.DirPort) {
         if(router_get_list_from_file(options.RouterFile) < 0) {
           log(LOG_ERR,"Error reloading router list. Continuing with old list.");
         }
       } else {
-        directory_initiate_command(router_pick_directory_server(), DIR_CONN_STATE_CONNECTING_GET);
+        directory_initiate_command(router_pick_directory_server(), DIR_CONN_STATE_CONNECTING_FETCH);
       }
-      please_fetch_directory = 0;
+
+      reset_logs(); /* close and reopen the log files */
+
+      please_reset = 0;
     }
     if(please_reap_children) {
       while(waitpid(-1,NULL,WNOHANG)) ; /* keep reaping until no more zombies */
@@ -546,7 +550,7 @@ static void catch(int the_signal) {
       log(LOG_NOTICE,"Catching signal %d, exiting cleanly.", the_signal);
       exit(0);
     case SIGHUP:
-      please_fetch_directory = 1;
+      please_reset = 1;
       break;
     case SIGUSR1:
       please_dumpstats = 1;

src/or/or.h

@@ -156,12 +156,12 @@
 #define _AP_CONN_STATE_MAX 5
 
 #define _DIR_CONN_STATE_MIN 0
-#define DIR_CONN_STATE_CONNECTING_GET 0
-#define DIR_CONN_STATE_CONNECTING_POST 1
-#define DIR_CONN_STATE_CLIENT_SENDING_GET 2
-#define DIR_CONN_STATE_CLIENT_SENDING_POST 3
-#define DIR_CONN_STATE_CLIENT_READING_GET 4
-#define DIR_CONN_STATE_CLIENT_READING_POST 5
+#define DIR_CONN_STATE_CONNECTING_FETCH 0
+#define DIR_CONN_STATE_CONNECTING_UPLOAD 1
+#define DIR_CONN_STATE_CLIENT_SENDING_FETCH 2
+#define DIR_CONN_STATE_CLIENT_SENDING_UPLOAD 3
+#define DIR_CONN_STATE_CLIENT_READING_FETCH 4
+#define DIR_CONN_STATE_CLIENT_READING_UPLOAD 5
 #define DIR_CONN_STATE_SERVER_COMMAND_WAIT 6
 #define DIR_CONN_STATE_SERVER_WRITING 7
 #define _DIR_CONN_STATE_MAX 7
@@ -219,12 +219,14 @@
 typedef struct {
    unsigned char version;     /* socks version number */
    unsigned char command;     /* command code */
-   unsigned char destport[2]; /* destination port, network order */
-   unsigned char destip[4];   /* destination address */
-   /* userid follows, terminated by a NULL */
-   /* dest host follows, terminated by a NULL */
+   uint16_t destport; /* destination port, network order */
+   uint32_t destip;   /* destination address, host order */
+   /* userid follows, terminated by a \0 */
+   /* dest host follows, terminated by a \0 */
 } socks4_t;
 
+#define SOCKS4_NETWORK_LEN 8
+
 typedef uint16_t aci_t;
 
 /* cell definition */