Skip to content

Commit 5e161c6

Browse files
authored
Fixed unsoundness in StderrForwarder::forward_available (#1203)
1 parent 8df1156 commit 5e161c6

File tree

1 file changed

+16
-13
lines changed

1 file changed

+16
-13
lines changed

src/command_helpers.rs

+16-13
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ pub(crate) struct StderrForwarder {
9494
is_non_blocking: bool,
9595
#[cfg(feature = "parallel")]
9696
bytes_available_failed: bool,
97+
/// number of bytes buffered in inner
98+
bytes_buffered: usize,
9799
}
98100

99101
const MIN_BUFFER_CAPACITY: usize = 100;
@@ -105,6 +107,7 @@ impl StderrForwarder {
105107
.stderr
106108
.take()
107109
.map(|stderr| (stderr, Vec::with_capacity(MIN_BUFFER_CAPACITY))),
110+
bytes_buffered: 0,
108111
#[cfg(feature = "parallel")]
109112
is_non_blocking: false,
110113
#[cfg(feature = "parallel")]
@@ -115,8 +118,6 @@ impl StderrForwarder {
115118
fn forward_available(&mut self) -> bool {
116119
if let Some((stderr, buffer)) = self.inner.as_mut() {
117120
loop {
118-
let old_data_end = buffer.len();
119-
120121
// For non-blocking we check to see if there is data available, so we should try to
121122
// read at least that much. For blocking, always read at least the minimum amount.
122123
#[cfg(not(feature = "parallel"))]
@@ -158,12 +159,11 @@ impl StderrForwarder {
158159
} else {
159160
MIN_BUFFER_CAPACITY
160161
};
161-
buffer.reserve(to_reserve);
162+
if self.bytes_buffered + to_reserve > buffer.len() {
163+
buffer.resize(self.bytes_buffered + to_reserve, 0);
164+
}
162165

163-
// Safety: stderr.read only writes to the spare part of the buffer, it never reads from it
164-
match stderr
165-
.read(unsafe { &mut *(buffer.spare_capacity_mut() as *mut _ as *mut [u8]) })
166-
{
166+
match stderr.read(&mut buffer[self.bytes_buffered..]) {
167167
Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => {
168168
// No data currently, yield back.
169169
break false;
@@ -173,22 +173,25 @@ impl StderrForwarder {
173173
continue;
174174
}
175175
Ok(bytes_read) if bytes_read != 0 => {
176-
// Safety: bytes_read bytes is written to spare part of the buffer
177-
unsafe { buffer.set_len(old_data_end + bytes_read) };
176+
self.bytes_buffered += bytes_read;
178177
let mut consumed = 0;
179-
for line in buffer.split_inclusive(|&b| b == b'\n') {
178+
for line in buffer[..self.bytes_buffered].split_inclusive(|&b| b == b'\n') {
180179
// Only forward complete lines, leave the rest in the buffer.
181180
if let Some((b'\n', line)) = line.split_last() {
182181
consumed += line.len() + 1;
183182
write_warning(line);
184183
}
185184
}
186-
buffer.drain(..consumed);
185+
if consumed > 0 && consumed < self.bytes_buffered {
186+
// Remove the consumed bytes from buffer
187+
buffer.copy_within(consumed.., 0);
188+
}
189+
self.bytes_buffered -= consumed;
187190
}
188191
res => {
189192
// End of stream: flush remaining data and bail.
190-
if old_data_end > 0 {
191-
write_warning(&buffer[..old_data_end]);
193+
if self.bytes_buffered > 0 {
194+
write_warning(&buffer[..self.bytes_buffered]);
192195
}
193196
if let Err(err) = res {
194197
write_warning(

0 commit comments

Comments
 (0)