[or-cvs] r11299: Make controllers accept LF as well as CRLF. Update spec to r (in tor/trunk: . doc doc/spec src/or)

nickm at seul.org nickm at seul.org
Wed Aug 29 19:02:34 UTC 2007


Author: nickm
Date: 2007-08-29 15:02:33 -0400 (Wed, 29 Aug 2007)
New Revision: 11299

Modified:
   tor/trunk/
   tor/trunk/ChangeLog
   tor/trunk/doc/TODO
   tor/trunk/doc/spec/control-spec.txt
   tor/trunk/src/or/buffers.c
   tor/trunk/src/or/connection_edge.c
   tor/trunk/src/or/control.c
   tor/trunk/src/or/or.h
   tor/trunk/src/or/test.c
Log:
 r14830 at catbus:  nickm | 2007-08-29 13:50:10 -0400
 Make controllers accept LF as well as CRLF.  Update spec to reflect this.  Remove now-dead code.  Make controller warning about v0 protocol more accurate.



Property changes on: tor/trunk
___________________________________________________________________
 svk:merge ticket from /tor/trunk [r14830] on 8246c3cf-6607-4228-993b-4d95d33730f1

Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog	2007-08-29 17:22:00 UTC (rev 11298)
+++ tor/trunk/ChangeLog	2007-08-29 19:02:33 UTC (rev 11299)
@@ -9,6 +9,10 @@
       temporarily runs an old version of Tor and then switches back to a new
       one, she doesn't automatically lose her guards.
 
+  o Minor features (controller):
+    - Accept LF instead of CRLF on controller, since some software has a
+      hard time generating real Internet newlines.
+
   o Minor bugfixes:
     - When generating information telling us how to extend to a given
       router, do not try to include the nickname if it is absent.  Fixes

Modified: tor/trunk/doc/TODO
===================================================================
--- tor/trunk/doc/TODO	2007-08-29 17:22:00 UTC (rev 11298)
+++ tor/trunk/doc/TODO	2007-08-29 19:02:33 UTC (rev 11299)
@@ -144,7 +144,10 @@
       - Write a proposal; make this part of 105.
     - Audit how much RAM we're using for buffers and cell pools; try to
       trim down a lot.
-    - Accept \n as end of lines in the control protocol in addition to \r\n.
+    o Accept \n as end of lines in the control protocol in addition to \r\n.
+      o Use fetch_from_buf_line_lf in control.c instead of fetch_from_buf_line.
+      o Fix up read escaped_data to accept LF instead of CRLF, and to
+        always translate_newlines (since that's the only way it's called).
     - Base relative control socket paths on datadir.
     - We should ship with a list of stable dir mirrors -- they're not
       trusted like the authorities, but they'll provide more robustness

Modified: tor/trunk/doc/spec/control-spec.txt
===================================================================
--- tor/trunk/doc/spec/control-spec.txt	2007-08-29 17:22:00 UTC (rev 11298)
+++ tor/trunk/doc/spec/control-spec.txt	2007-08-29 19:02:33 UTC (rev 11299)
@@ -52,6 +52,10 @@
   There are explicitly no limits on line length.  All 8-bit characters are
   permitted unless explicitly disallowed.
 
+  Wherever CRLF is specified to be accepted from the controller, Tor MAY also
+  accept LF.  Tor, however, MUST NOT generate LF instead of CRLF.
+  Controllers SHOULD always send CRLF.
+
 2.2. Commands from controller to Tor
 
     Command = Keyword Arguments CRLF / "+" Keyword Arguments CRLF Data

Modified: tor/trunk/src/or/buffers.c
===================================================================
--- tor/trunk/src/or/buffers.c	2007-08-29 17:22:00 UTC (rev 11298)
+++ tor/trunk/src/or/buffers.c	2007-08-29 19:02:33 UTC (rev 11299)
@@ -1462,52 +1462,6 @@
   return memchr(buf->mem, c, len_rest);
 }
 
-/** Helper: return a pointer to the first CRLF after cp on <b>buf</b>. Return
- * NULL if no CRLF is found. */
-static char *
-find_crlf_on_buf(buf_t *buf, char *cp)
-{
-  char *next;
-  while (1) {
-    size_t remaining = buf->datalen - _buf_offset(buf,cp);
-    cp = find_char_on_buf(buf, cp, remaining, '\r');
-    if (!cp)
-      return NULL;
-    next = _wrap_ptr(buf, cp+1);
-    if (next == _buf_end(buf))
-      return NULL;
-    if (*next == '\n')
-      return cp;
-    cp = next;
-  }
-}
-
-/** Try to read a single CRLF-terminated line from <b>buf</b>, and write it,
- * NUL-terminated, into the *<b>data_len</b> byte buffer at <b>data_out</b>.
- * Set *<b>data_len</b> to the number of bytes in the line, not counting the
- * terminating NUL.  Return 1 if we read a whole line, return 0 if we don't
- * have a whole line yet, and return -1 if we we need to grow the buffer.
- */
-int
-fetch_from_buf_line(buf_t *buf, char *data_out, size_t *data_len)
-{
-  char *eol;
-  size_t sz;
-  /* Look for a CRLF. */
-  if (!(eol = find_crlf_on_buf(buf, buf->cur))) {
-    return 0;
-  }
-  sz = _buf_offset(buf, eol);
-  if (sz+3 > *data_len) {
-    *data_len = sz+3;
-    return -1;
-  }
-  fetch_from_buf(data_out, sz+2, buf);
-  data_out[sz+2] = '\0';
-  *data_len = sz+2;
-  return 1;
-}
-
 /** Try to read a single LF-terminated line from <b>buf</b>, and write it,
  * NUL-terminated, into the *<b>data_len</b> byte buffer at <b>data_out</b>.
  * Set *<b>data_len</b> to the number of bytes in the line, not counting the
@@ -1516,7 +1470,7 @@
  *<b>data_len</b>.
  */
 int
-fetch_from_buf_line_lf(buf_t *buf, char *data_out, size_t *data_len)
+fetch_from_buf_line(buf_t *buf, char *data_out, size_t *data_len)
 {
   char *cp;
   size_t sz;

Modified: tor/trunk/src/or/connection_edge.c
===================================================================
--- tor/trunk/src/or/connection_edge.c	2007-08-29 17:22:00 UTC (rev 11298)
+++ tor/trunk/src/or/connection_edge.c	2007-08-29 19:02:33 UTC (rev 11299)
@@ -1725,7 +1725,7 @@
 
   /* look for LF-terminated "[DEST ip_addr port]"
    * where ip_addr is a dotted-quad and port is in string form */
-  err = fetch_from_buf_line_lf(conn->_base.inbuf, tmp_buf, &tlen);
+  err = fetch_from_buf_line(conn->_base.inbuf, tmp_buf, &tlen);
   if (err == 0)
     return 0;
   if (err < 0) {

Modified: tor/trunk/src/or/control.c
===================================================================
--- tor/trunk/src/or/control.c	2007-08-29 17:22:00 UTC (rev 11298)
+++ tor/trunk/src/or/control.c	2007-08-29 19:02:33 UTC (rev 11299)
@@ -272,16 +272,14 @@
 }
 
 /** Given a <b>len</b>-character string in <b>data</b>, made of lines
- * terminated by CRLF, allocate a new string in *<b>out</b>, and copy
- * the contents of <b>data</b> into *<b>out</b>, adding a period
- * before any period that that appears at the start of a line, and
- * adding a period-CRLF line at the end. If <b>translate_newlines</b>
- * is true, replace all LF characters sequences with CRLF.  Return the
- * number of bytes in *<b>out</b>.
+ * terminated by CRLF, allocate a new string in *<b>out</b>, and copy the
+ * contents of <b>data</b> into *<b>out</b>, adding a period before any period
+ * that that appears at the start of a line, and adding a period-CRLF line at
+ * the end. Replace all LF characters sequences with CRLF.  Return the number
+ * of bytes in *<b>out</b>.
  */
 /* static */ size_t
-write_escaped_data(const char *data, size_t len, int translate_newlines,
-                   char **out)
+write_escaped_data(const char *data, size_t len, char **out)
 {
   size_t sz_out = len+8;
   char *outp;
@@ -297,7 +295,7 @@
   start_of_line = 1;
   while (data < end) {
     if (*data == '\n') {
-      if (translate_newlines && data > start && data[-1] != '\r')
+      if (data > start && data[-1] != '\r')
         *outp++ = '\r';
       start_of_line = 1;
     } else if (*data == '.') {
@@ -325,12 +323,11 @@
 /** Given a <b>len</b>-character string in <b>data</b>, made of lines
  * terminated by CRLF, allocate a new string in *<b>out</b>, and copy
  * the contents of <b>data</b> into *<b>out</b>, removing any period
- * that appears at the start of a line.  If <b>translate_newlines</b>
- * is true, replace all CRLF sequences with LF.  Return the number of
+ * that appears at the start of a line, and replacing all CRLF sequences
+ * with LF.   Return the number of
  * bytes in *<b>out</b>. */
 /* static */ size_t
-read_escaped_data(const char *data, size_t len, int translate_newlines,
-                  char **out)
+read_escaped_data(const char *data, size_t len, char **out)
 {
   char *outp;
   const char *next;
@@ -341,28 +338,26 @@
   end = data+len;
 
   while (data < end) {
+    /* we're at the start of a line. */
     if (*data == '.')
       ++data;
-    if (translate_newlines)
-      next = tor_memmem(data, end-data, "\r\n", 2);
-    else
-      next = tor_memmem(data, end-data, "\r\n.", 3);
+    next = memchr(data, '\n', end-data);
     if (next) {
-      memcpy(outp, data, next-data);
-      outp += (next-data);
-      data = next+2;
+      size_t n_to_copy = next-data;
+      /* Don't copy a CR that precedes this LF. */
+      if (n_to_copy && *(next-1) == '\r')
+        --n_to_copy;
+      memcpy(outp, data, n_to_copy);
+      outp += n_to_copy;
+      data = next+1; /* This will point at the start of the next line,
+                      * or the end of the string, or a period. */
     } else {
       memcpy(outp, data, end-data);
       outp += (end-data);
       *outp = '\0';
       return outp - *out;
     }
-    if (translate_newlines) {
-      *outp++ = '\n';
-    } else {
-      *outp++ = '\r';
-      *outp++ = '\n';
-    }
+    *outp++ = '\n';
   }
 
   *outp = '\0';
@@ -1818,7 +1813,7 @@
     } else {
       char *esc = NULL;
       size_t esc_len;
-      esc_len = write_escaped_data(v, strlen(v), 1, &esc);
+      esc_len = write_escaped_data(v, strlen(v), &esc);
       connection_printf_to_buf(conn, "250+%s=\r\n", k);
       connection_write_to_buf(esc, esc_len, TO_CONN(conn));
       tor_free(esc);
@@ -2002,6 +1997,8 @@
 handle_control_setpurpose(control_connection_t *conn, int for_circuits,
                           uint32_t len, const char *body)
 {
+  /* XXXX020 this should maybe be two functions; almost no code is acutally
+     shared. */
   origin_circuit_t *circ = NULL;
   routerinfo_t *ri = NULL;
   uint8_t new_purpose;
@@ -2170,7 +2167,7 @@
   }
   SMARTLIST_FOREACH(args, char *, arg, tor_free(arg));
   smartlist_free(args);
-  read_escaped_data(cp, len-(cp-body), 1, &desc);
+  read_escaped_data(cp, len-(cp-body), &desc);
 
   switch (router_load_single_router(desc, purpose, &msg)) {
   case -1:
@@ -2530,8 +2527,8 @@
     char buf[128];
     set_uint16(buf+2, htons(0x0000)); /* type == error */
     set_uint16(buf+4, htons(0x0001)); /* code == internal error */
-    strlcpy(buf+6, "The v0 control protocol is not supported by Tor 0.2.0.x "
-            "and later; use Tor 0.1.2.x or upgrade your controller",
+    strlcpy(buf+6, "The v0 control protocol is not supported by Tor 0.1.2.17 "
+            "and later; upgrade your controller.",
             sizeof(buf)-6);
     body_len = 2+strlen(buf+6)+2; /* code, msg, nul. */
     set_uint16(buf+0, htons(body_len));
@@ -2571,11 +2568,17 @@
     if (last_idx == 0 && *conn->incoming_cmd != '+')
       /* One line command, didn't start with '+'. */
       break;
+    /* XXXX this code duplication is kind of dumb. */
     if (last_idx+3 == conn->incoming_cmd_cur_len &&
         !memcmp(conn->incoming_cmd + last_idx, ".\r\n", 3)) {
       /* Just appended ".\r\n"; we're done. Remove it. */
       conn->incoming_cmd_cur_len -= 3;
       break;
+    } else if (last_idx+2 == conn->incoming_cmd_cur_len &&
+               !memcmp(conn->incoming_cmd + last_idx, ".\n", 2)) {
+      /* Just appended ".\n"; we're done. Remove it. */
+      conn->incoming_cmd_cur_len -= 2;
+      break;
     }
     /* Otherwise, read another line. */
   }
@@ -3309,7 +3312,7 @@
                msg ? msg : "");
 
   /* Escape the server descriptor properly */
-  esclen = write_escaped_data(desc, desclen, 1, &esc);
+  esclen = write_escaped_data(desc, desclen, &esc);
 
   totallen = strlen(firstline) + esclen + 1;
   buf = tor_malloc(totallen);
@@ -3345,7 +3348,7 @@
     });
 
   s = smartlist_join_strings(strs, "", 0, NULL);
-  write_escaped_data(s, strlen(s), 1, &esc);
+  write_escaped_data(s, strlen(s), &esc);
   SMARTLIST_FOREACH(strs, char *, cp, tor_free(cp));
   smartlist_free(strs);
   tor_free(s);

Modified: tor/trunk/src/or/or.h
===================================================================
--- tor/trunk/src/or/or.h	2007-08-29 17:22:00 UTC (rev 11298)
+++ tor/trunk/src/or/or.h	2007-08-29 19:02:33 UTC (rev 11299)
@@ -2241,7 +2241,6 @@
 int fetch_from_buf_socks(buf_t *buf, socks_request_t *req,
                          int log_sockstype, int safe_socks);
 int fetch_from_buf_line(buf_t *buf, char *data_out, size_t *data_len);
-int fetch_from_buf_line_lf(buf_t *buf, char *data_out, size_t *data_len);
 
 int peek_buf_has_control0_command(buf_t *buf);
 
@@ -2720,10 +2719,8 @@
 
 #ifdef CONTROL_PRIVATE
 /* Used only by control.c and test.c */
-size_t write_escaped_data(const char *data, size_t len,
-                          int translate_newlines, char **out);
-size_t read_escaped_data(const char *data, size_t len,
-                         int translate_newlines,  char **out);
+size_t write_escaped_data(const char *data, size_t len, char **out);
+size_t read_escaped_data(const char *data, size_t len, char **out);
 #endif
 
 /********************************* cpuworker.c *****************************/

Modified: tor/trunk/src/or/test.c
===================================================================
--- tor/trunk/src/or/test.c	2007-08-29 17:22:00 UTC (rev 11298)
+++ tor/trunk/src/or/test.c	2007-08-29 19:02:33 UTC (rev 11299)
@@ -2000,7 +2000,7 @@
     "..This is a test\r\nof the emergency \nbroadcast\r\n..system.\r\nZ.\r\n";
   size_t sz;
 
-  sz = read_escaped_data(inp, strlen(inp), 1, &out);
+  sz = read_escaped_data(inp, strlen(inp), &out);
   test_streq(out,
              ".This is a test\nof the emergency \nbroadcast\n.system.\nZ.\n");
   test_eq(sz, strlen(out));



More information about the tor-commits mailing list