remote_server: Remove unnecessary Box, prevent time-of-check time-of-use bug (#24730)

tidely created 11 months ago

The MultiWrite struct is defined in the function scope and is allowed to
have a concrete type, which means we can throw away the extra Box.
PathBuf::exists is known to be prone to invalid usage. It doesn't take
into account permissions errors and just returns false, additionally it
introduces a time-of-check time-of-use bug. While extremely unlikely,
why not fix it anyway.

Release Notes:

- remove unnecessary Box
- prevent time-of-check time-of-use bug

Change summary

crates/remote_server/src/unix.rs | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)

Detailed changes

crates/remote_server/src/unix.rs

@@ -59,7 +59,7 @@ fn init_logging_proxy() {
 
 fn init_logging_server(log_file_path: PathBuf) -> Result<Receiver<Vec<u8>>> {
     struct MultiWrite {
-        file: Box<dyn std::io::Write + Send + 'static>,
+        file: std::fs::File,
         channel: Sender<Vec<u8>>,
         buffer: Vec<u8>,
     }
@@ -80,14 +80,11 @@ fn init_logging_server(log_file_path: PathBuf) -> Result<Receiver<Vec<u8>>> {
         }
     }
 
-    let log_file = Box::new(if log_file_path.exists() {
-        std::fs::OpenOptions::new()
-            .append(true)
-            .open(&log_file_path)
-            .context("Failed to open log file in append mode")?
-    } else {
-        std::fs::File::create(&log_file_path).context("Failed to create log file")?
-    });
+    let log_file = std::fs::OpenOptions::new()
+        .create(true)
+        .append(true)
+        .open(&log_file_path)
+        .context("Failed to open log file in append mode")?;
 
     let (tx, rx) = smol::channel::unbounded();