Merge branch 'cr-lf-in-log-file'

3 files changed, 62 insertions, 18 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 57cc9a58fb..ee33f43f71 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -55,6 +55,7 @@ Line wrap the file at 100 chars.                                              Th
 - Reduce RPC timeout to Mullvad API server.
 - Fix OpenVPN warning about usage of AES-256-CBC cipher.
 - Fix "Out of time" screen status icon position.
+- Fix log newline characters on Windows.
 
 
 ## [2018.1] - 2018-03-01
diff --git a/mullvad-daemon/src/bin/problem-report.rs b/mullvad-daemon/src/bin/problem-report.rs
index a5e8ac0577..666df33c3d 100644
--- a/mullvad-daemon/src/bin/problem-report.rs
+++ b/mullvad-daemon/src/bin/problem-report.rs
@@ -23,9 +23,8 @@ use std::borrow::Cow;
 use std::cmp::min;
 use std::collections::HashMap;
 use std::env;
-use std::fmt;
 use std::fs::File;
-use std::io::{self, Read, Seek, SeekFrom, Write};
+use std::io::{self, BufWriter, Read, Seek, SeekFrom, Write};
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
@@ -38,6 +37,23 @@ const REPORT_MAX_SIZE: usize = 2 * LOG_MAX_READ_BYTES + 16 * 1024;
 /// Field delimeter in generated problem report
 const LOG_DELIMITER: &'static str = "====================";
 
+/// Line separator character sequence
+#[cfg(not(windows))]
+const LINE_SEPARATOR: &str = "\n";
+
+#[cfg(windows)]
+const LINE_SEPARATOR: &str = "\r\n";
+
+/// Custom macro to write a line to an output formatter that uses platform-specific newline
+/// character sequences.
+macro_rules! write_line {
+    ($fmt:expr $(,)*) => { write!($fmt, "{}", LINE_SEPARATOR) };
+    ($fmt:expr, $pattern:expr $(, $arg:expr)* $(,)*) => {
+        write!($fmt, $pattern, $( $arg ),*)
+            .and_then(|_| write!($fmt, "{}", LINE_SEPARATOR))
+    };
+}
+
 error_chain!{
     errors {
         WriteReportError(path: PathBuf) {
@@ -156,8 +172,8 @@ fn collect_report(
 }
 
 fn send_problem_report(user_email: &str, user_message: &str, report_path: &Path) -> Result<()> {
-    let report_content = read_file_lossy(report_path, REPORT_MAX_SIZE)
-        .chain_err(|| ErrorKind::ReadLogError(report_path.to_path_buf()))?;
+    let report_content = normalize_newlines(read_file_lossy(report_path, REPORT_MAX_SIZE)
+        .chain_err(|| ErrorKind::ReadLogError(report_path.to_path_buf()))?);
     let metadata = collect_metadata();
     let mut rpc_manager = mullvad_rpc::MullvadRpcFactory::new();
     let mut rpc_client = mullvad_rpc::ProblemReportProxy::connect(&mut rpc_manager)
@@ -169,11 +185,11 @@ fn send_problem_report(user_email: &str, user_message: &str, report_path: &Path)
 }
 
 fn write_problem_report(path: &Path, problem_report: ProblemReport) -> io::Result<()> {
-    let mut file = File::create(path)?;
+    let file = File::create(path)?;
     let mut permissions = file.metadata()?.permissions();
     permissions.set_readonly(true);
     file.set_permissions(permissions)?;
-    file.write(problem_report.to_string().as_bytes())?;
+    problem_report.write_to(BufWriter::new(file))?;
     Ok(())
 }
 
@@ -250,21 +266,19 @@ impl ProblemReport {
         }
         out
     }
-}
 
-impl fmt::Display for ProblemReport {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        writeln!(fmt, "System information:")?;
+    fn write_to<W: Write>(&self, mut output: W) -> io::Result<()> {
+        write_line!(output, "System information:")?;
         for (key, value) in &self.metadata {
-            writeln!(fmt, "{}: {}", key, value)?;
+            write_line!(output, "{}: {}", key, value)?;
         }
-        writeln!(fmt, "")?;
+        write_line!(output)?;
         for &(ref label, ref content) in &self.logs {
-            writeln!(fmt, "{}", LOG_DELIMITER)?;
-            writeln!(fmt, "Log: {}", label)?;
-            writeln!(fmt, "{}", LOG_DELIMITER)?;
-            fmt.write_str(content)?;
-            writeln!(fmt)?;
+            write_line!(output, "{}", LOG_DELIMITER)?;
+            write_line!(output, "Log: {}", label)?;
+            write_line!(output, "{}", LOG_DELIMITER)?;
+            output.write_all(content.as_bytes())?;
+            write_line!(output)?;
         }
         Ok(())
     }
@@ -386,6 +400,16 @@ fn command_stdout_lossy(cmd: &str, args: &[&str]) -> Option<String> {
         .ok()
 }
 
+#[cfg(not(windows))]
+fn normalize_newlines(text: String) -> String {
+    text
+}
+
+#[cfg(windows)]
+fn normalize_newlines(text: String) -> String {
+    text.replace(LINE_SEPARATOR, "\n")
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/mullvad-daemon/src/logging.rs b/mullvad-daemon/src/logging.rs
index caac15d02a..b8cc898bc7 100644
--- a/mullvad-daemon/src/logging.rs
+++ b/mullvad-daemon/src/logging.rs
@@ -1,6 +1,7 @@
 extern crate fern;
 
 use self::fern::colors::{Color, ColoredLevelConfig};
+use self::fern::Output;
 use chrono;
 use log;
 
@@ -40,6 +41,12 @@ const COLORS: ColoredLevelConfig = ColoredLevelConfig {
     trace: Color::Black,
 };
 
+#[cfg(not(windows))]
+const LINE_SEPARATOR: &str = "\n";
+
+#[cfg(windows)]
+const LINE_SEPARATOR: &str = "\r\n";
+
 pub const DATE_TIME_FORMAT_STR: &str = "[%Y-%m-%d %H:%M:%S%.3f]";
 
 pub fn init_logger(
@@ -70,7 +77,7 @@ pub fn init_logger(
             .chain_err(|| ErrorKind::WriteFileError(log_file.to_path_buf()))?;
         let file_dispatcher = fern::Dispatch::new()
             .format(move |out, message, record| file_formatter.output_msg(out, message, record))
-            .chain(f);
+            .chain(Output::file(f, LINE_SEPARATOR));
         top_dispatcher = top_dispatcher.chain(file_dispatcher);
     }
     top_dispatcher.apply()?;
@@ -106,6 +113,8 @@ impl Formatter {
         message: &fmt::Arguments,
         record: &log::Record,
     ) {
+        let message = escape_newlines(format!("{}", message));
+
         out.finish(format_args!(
             "{}[{}][{}] {}",
             chrono::Local::now().format(self.get_timetsamp_fmt()),
@@ -115,3 +124,13 @@ impl Formatter {
         ))
     }
 }
+
+#[cfg(not(windows))]
+fn escape_newlines(text: String) -> String {
+    text
+}
+
+#[cfg(windows)]
+fn escape_newlines(text: String) -> String {
+    text.replace("\n", LINE_SEPARATOR)
+}