Strip '\r' characters when reading text files on Unix.

This patch ensures that we strip "\r" characters on both Windows as well
as Unix when we read text files. This should prevent the issue where
some Tor state files have been moved from a Windows machine, and thus
contains CRLF line ending, to a Unix machine where only \n is needed.

We add a test-case to ensure that we handle this properly on all our
platforms.

See: https://bugs.torproject.org/tpo/core/tor/33781

diff --git a/changes/bug33781 b/changes/bug33781
new file mode 100644 (file)
index 0000000..9f63ab0
--- /dev/null
+++ b/changes/bug33781
@@ -0,0 +1,7 @@
+  o Minor bugfixes (compatibility):
+    - Strip '\r' characters when reading text files on Unix platforms.
+      This should resolve an issue where a relay operator migrates a relay from
+      Windows to Unix, but does not change the line ending of Tor's various state
+      files to match the platform, the CRLF line endings from Windows ends up leaking
+      into other files such as the extra-info document. Fixes bug 33781; bugfix on
+      0.0.9pre5.

diff --git a/src/lib/fs/files.c b/src/lib/fs/files.c
index b98a51a28796d746d1e37e1db1807b7bd0a563ef..55d5f1bf8c51de8631a83400f9b2d45b181f33d0 100644 (file)
--- a/src/lib/fs/files.c
+++ b/src/lib/fs/files.c
@@ -685,7 +685,6 @@ read_file_to_str, (const char *filename, int flags, struct stat *stat_out))
   }
   string[r] = '\0'; /* NUL-terminate the result. */
 
-#if defined(_WIN32) || defined(__CYGWIN__)
   if (!bin && strchr(string, '\r')) {
     log_debug(LD_FS, "We didn't convert CRLF to LF as well as we hoped "
               "when reading %s. Coping.",
@@ -695,8 +694,7 @@ read_file_to_str, (const char *filename, int flags, struct stat *stat_out))
   }
   if (!bin) {
     statbuf.st_size = (size_t) r;
-  } else
-#endif /* defined(_WIN32) || defined(__CYGWIN__) */
+  } else {
     if (r != statbuf.st_size) {
       /* Unless we're using text mode on win32, we'd better have an exact
        * match for size. */
@@ -708,6 +706,7 @@ read_file_to_str, (const char *filename, int flags, struct stat *stat_out))
       errno = save_errno;
       return NULL;
     }
+  }
   close(fd);
   if (stat_out) {
     memcpy(stat_out, &statbuf, sizeof(struct stat));

diff --git a/src/test/test_util.c b/src/test/test_util.c
index 3a0eb151576a5fa13a6de33e23f310db21ad5653..06ea8191a2ca41dcca64fd3266f8b01e6a123d20 100644 (file)
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -173,6 +173,54 @@ test_util_read_file_eof_zero_bytes(void *arg)
   test_util_read_until_eof_impl("tor_test_fifo_empty", 0, 10000);
 }
 
+static void
+test_util_read_file_endlines(void *arg)
+{
+  (void)arg;
+
+  char *fname = NULL;
+  char *read_content = NULL;
+  int r = -1;
+
+  /* Write a file that contains both \n and \r\n as line ending. */
+  const char *file_content = "foo bar\n"
+                             "foo bar baz\r\n"
+                             "foo bar\r\n";
+
+  const char *expected_file_content = "foo bar\n"
+                                      "foo bar baz\n"
+                                      "foo bar\n";
+
+  fname = tor_strdup(get_fname("file_with_crlf_ending"));
+
+  r = write_bytes_to_file(fname, file_content, strlen(file_content), 1);
+  tt_int_op(r, OP_EQ, 0);
+
+  /* Read the file in text mode: we strip \r's from the files on both Windows
+   * and UNIX. */
+  read_content = read_file_to_str(fname, 0, NULL);
+
+  tt_ptr_op(read_content, OP_NE, NULL);
+  tt_int_op(strlen(read_content), OP_EQ, strlen(expected_file_content));
+  tt_str_op(read_content, OP_EQ, expected_file_content);
+
+  tor_free(read_content);
+
+  /* Read the file in binary mode: we should preserve the \r here. */
+  read_content = read_file_to_str(fname, RFTS_BIN, NULL);
+
+  tt_ptr_op(read_content, OP_NE, NULL);
+  tt_int_op(strlen(read_content), OP_EQ, strlen(file_content));
+  tt_str_op(read_content, OP_EQ, file_content);
+
+  tor_free(read_content);
+
+ done:
+  unlink(fname);
+  tor_free(fname);
+  tor_free(read_content);
+}
+
 /* Test the basic expected behaviour for write_chunks_to_file.
  * NOTE: This will need to be updated if we ever change the tempfile location
  * or extension */
@@ -6508,6 +6556,7 @@ struct testcase_t util_tests[] = {
   UTIL_TEST(read_file_eof_two_loops, 0),
   UTIL_TEST(read_file_eof_two_loops_b, 0),
   UTIL_TEST(read_file_eof_zero_bytes, 0),
+  UTIL_TEST(read_file_endlines, 0),
   UTIL_TEST(write_chunks_to_file, 0),
   UTIL_TEST(mathlog, 0),
   UTIL_TEST(fraction, 0),