Skip to content

Commit

Permalink
Ensure termination of syslog writer (#857)
Browse files Browse the repository at this point in the history
Closing #856 

This particular code has been struck by lightning a couple of times now
and is pretty hard to reason about using formal reasoning (e.g. a loop
invariant like `self.cursor < LIMIT` would seen to have to hold, but it
doesn't really). This PR doesn't improve that situation (i.e. the formal
reasoning), but it does ensure termination.

As a minor side-fix, the code could also panic on line 54 if we hit the
middle of a UTF8 character in exactly the wrong position, that is fixed
as well.

I'll work on an implementation of this writer in a more declarative
style.
  • Loading branch information
japaric authored Aug 23, 2024
2 parents 2ac8cbb + 6b5151d commit 2f4baf9
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/log/syslog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,20 @@ impl Write for SysLogWriter {
if self.cursor + message.len() > LIMIT {
// floor_char_boundary is currently unstable
let mut truncate_boundary = LIMIT - self.cursor;
while !message.is_char_boundary(truncate_boundary) {
while truncate_boundary > 0 && !message.is_char_boundary(truncate_boundary) {
truncate_boundary -= 1;
}

// don't overzealously truncate log messages
truncate_boundary = message[..truncate_boundary]
.rfind(|c: char| c.is_ascii_whitespace())
.unwrap_or(truncate_boundary);

if truncate_boundary == 0 {
// we failed to find a "nice" cut off point, abruptly cut off the msg
truncate_boundary = LIMIT - self.cursor;
}

let left = &message[..truncate_boundary];
let right = &message[truncate_boundary..];

Expand Down
1 change: 1 addition & 0 deletions test-framework/e2e-tests/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![cfg(test)]

mod pty;
mod regression;
mod su;

type Error = Box<dyn std::error::Error>;
Expand Down
17 changes: 17 additions & 0 deletions test-framework/e2e-tests/src/regression.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
use sudo_test::{Command, Env, TextFile};

use crate::Result;

#[test]
fn syslog_writer_should_not_hang() -> Result<()> {
let env = Env(TextFile("ALL ALL=(ALL:ALL) NOPASSWD: ALL").chmod("644")).build()?;

let stdout = Command::new("sudo")
.args(["env", "CC=clang-18", "CXX=clang++-18", "FOO=\"........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................\"", "whoami"])
.output(&env)?
.stdout()?;

assert_eq!(stdout, "root");

Ok(())
}

0 comments on commit 2f4baf9

Please sign in to comment.