Fix panic safety issue in StderrForwarder (#1079)

src/command_helpers.rs

@@ -90,7 +90,6 @@ impl StderrForwarder {
         }
     }
 
-    #[allow(clippy::uninit_vec)]
     fn forward_available(&mut self) -> bool {
         if let Some((stderr, buffer)) = self.inner.as_mut() {
             loop {
@@ -104,7 +103,7 @@ impl StderrForwarder {
                 let to_reserve = if self.is_non_blocking && !self.bytes_available_failed {
                     match crate::parallel::stderr::bytes_available(stderr) {
                         #[cfg(windows)]
-                        Ok(0) => return false,
+                        Ok(0) => break false,
                         #[cfg(unix)]
                         Ok(0) => {
                             // On Unix, depending on the implementation, we may sometimes get 0 in a
@@ -120,7 +119,7 @@ impl StderrForwarder {
                                 write_warning(&buffer[..]);
                             }
                             self.inner = None;
-                            return true;
+                            break true;
                         }
                         #[cfg(unix)]
                         Err(_) => {
@@ -137,32 +136,21 @@ impl StderrForwarder {
                 };
                 buffer.reserve(to_reserve);
 
-                // SAFETY: 1) the length is set to the capacity, so we are never using memory beyond
-                // the underlying buffer and 2) we always call `truncate` below to set the len back
-                // to the initialized data.
-                unsafe {
-                    buffer.set_len(buffer.capacity());
-                }
-                match stderr.read(&mut buffer[old_data_end..]) {
+                // Safety: stderr.read only writes to the spare part of the buffer, it never reads from it
+                match stderr
+                    .read(unsafe { &mut *(buffer.spare_capacity_mut() as *mut _ as *mut [u8]) })
+                {
                     Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => {
                         // No data currently, yield back.
-                        buffer.truncate(old_data_end);
-                        return false;
+                        break false;
                     }
                     Err(err) if err.kind() == std::io::ErrorKind::Interrupted => {
                         // Interrupted, try again.
-                        buffer.truncate(old_data_end);
-                    }
-                    Ok(0) | Err(_) => {
-                        // End of stream: flush remaining data and bail.
-                        if old_data_end > 0 {
-                            write_warning(&buffer[..old_data_end]);
-                        }
-                        self.inner = None;
-                        return true;
+                        continue;
                     }
-                    Ok(bytes_read) => {
-                        buffer.truncate(old_data_end + bytes_read);
+                    Ok(bytes_read) if bytes_read != 0 => {
+                        // Safety: bytes_read bytes is written to spare part of the buffer
+                        unsafe { buffer.set_len(old_data_end + bytes_read) };
                         let mut consumed = 0;
                         for line in buffer.split_inclusive(|&b| b == b'\n') {
                             // Only forward complete lines, leave the rest in the buffer.
@@ -173,6 +161,19 @@ impl StderrForwarder {
                         }
                         buffer.drain(..consumed);
                     }
+                    res => {
+                        // End of stream: flush remaining data and bail.
+                        if old_data_end > 0 {
+                            write_warning(&buffer[..old_data_end]);
+                        }
+                        if let Err(err) = res {
+                            write_warning(
+                                format!("Failed to read from child stderr: {err}").as_bytes(),
+                            );
+                        }
+                        self.inner.take();
+                        break true;
+                    }
                 }
             }
         } else {