From 1b4a8b02d17fb1f79fc49b3da4e8d8608b9aee45 Mon Sep 17 00:00:00 2001 From: soenkehahn Date: Tue, 19 Mar 2019 11:57:56 -0400 Subject: [PATCH 1/7] use PathBuf for unmocked_commands --- src/protocol/mod.rs | 10 +++++----- src/protocol_checker/mod.rs | 6 +++--- src/recorder/hole_recorder.rs | 4 ++-- src/recorder/mod.rs | 15 ++++++++------- src/recorder/protocol_result.rs | 12 ++++++------ 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/protocol/mod.rs b/src/protocol/mod.rs index 9900d88..1270133 100644 --- a/src/protocol/mod.rs +++ b/src/protocol/mod.rs @@ -349,7 +349,7 @@ impl Protocol { #[derive(Debug, PartialEq)] pub struct Protocols { pub protocols: Vec, - pub unmocked_commands: Vec>, + pub unmocked_commands: Vec, pub interpreter: Option>, } @@ -381,7 +381,7 @@ impl Protocols { if let Ok(unmocked_commands) = object.expect_field("unmockedCommands") { for unmocked_command in unmocked_commands.expect_array()? { self.unmocked_commands - .push(unmocked_command.expect_str()?.as_bytes().to_vec()); + .push(PathBuf::from(unmocked_command.expect_str()?)); } } Ok(()) @@ -454,7 +454,7 @@ impl Protocols { self.unmocked_commands .iter() .map(|unmocked_command| { - Ok(Yaml::String(String::from_utf8(unmocked_command.to_vec())?)) + Ok(Yaml::String(path_to_string(unmocked_command)?.to_string())) }) .collect::>>()?, ), @@ -833,7 +833,7 @@ mod load { " )? .unmocked_commands - .map(|command| String::from_utf8(command).unwrap()), + .map(|path| path.to_string_lossy().to_string()), vec!["foo"] ); Ok(()) @@ -1071,7 +1071,7 @@ mod serialize { #[test] fn includes_unmocked_commands() -> R<()> { let mut protocols = Protocols::new(vec![Protocol::new(vec![])]); - protocols.unmocked_commands = vec![b"sed".to_vec()]; + protocols.unmocked_commands = vec![PathBuf::from("sed")]; roundtrip(protocols) } } diff --git a/src/protocol_checker/mod.rs b/src/protocol_checker/mod.rs index 3061c2c..e4ce643 100644 --- a/src/protocol_checker/mod.rs +++ b/src/protocol_checker/mod.rs @@ -19,7 +19,7 @@ use std::path::PathBuf; pub struct ProtocolChecker { context: Context, pub protocol: Protocol, - pub unmocked_commands: Vec>, + pub unmocked_commands: Vec, pub result: CheckerResult, temporary_executables: Vec, } @@ -28,7 +28,7 @@ impl ProtocolChecker { pub fn new( context: &Context, protocol: Protocol, - unmocked_commands: &[Vec], + unmocked_commands: &[PathBuf], ) -> ProtocolChecker { ProtocolChecker { context: context.clone(), @@ -103,7 +103,7 @@ impl SyscallMock for ProtocolChecker { if !self .unmocked_commands .iter() - .any(|c| protocol::compare_executables(&c, &executable)) + .any(|c| protocol::compare_executables(&c.as_os_str().as_bytes(), &executable)) { let mock_executable_path = self.handle_step(protocol::Command { executable, diff --git a/src/recorder/hole_recorder.rs b/src/recorder/hole_recorder.rs index 203cbbc..ff364ca 100644 --- a/src/recorder/hole_recorder.rs +++ b/src/recorder/hole_recorder.rs @@ -9,7 +9,7 @@ use crate::tracer::SyscallMock; use crate::{ExitCode, R}; use libc::user_regs_struct; use nix::unistd::Pid; -use std::path::Path; +use std::path::{Path, PathBuf}; pub enum HoleRecorder { Checker { @@ -24,7 +24,7 @@ pub enum HoleRecorder { impl HoleRecorder { pub fn new( context: &Context, - unmocked_commands: &[Vec], + unmocked_commands: &[PathBuf], protocol: Protocol, ) -> HoleRecorder { HoleRecorder::Checker { diff --git a/src/recorder/mod.rs b/src/recorder/mod.rs index 36fa06e..96eecda 100644 --- a/src/recorder/mod.rs +++ b/src/recorder/mod.rs @@ -8,11 +8,13 @@ use crate::tracer::SyscallMock; use crate::R; use libc::user_regs_struct; use nix::unistd::Pid; +use std::os::unix::ffi::OsStrExt; +use std::path::PathBuf; pub struct Recorder { protocol: Protocol, command: Option, - unmocked_commands: Vec>, + unmocked_commands: Vec, } impl Recorder { @@ -24,7 +26,7 @@ impl Recorder { } } - pub fn new(protocol: Protocol, unmocked_commands: &[Vec]) -> Recorder { + pub fn new(protocol: Protocol, unmocked_commands: &[PathBuf]) -> Recorder { Recorder { protocol, command: None, @@ -43,11 +45,10 @@ impl SyscallMock for Recorder { executable: Vec, arguments: Vec>, ) -> R<()> { - if !self - .unmocked_commands - .iter() - .any(|unmocked_command| compare_executables(&unmocked_command, &executable)) - { + let is_unmocked_command = self.unmocked_commands.iter().any(|unmocked_command| { + compare_executables(unmocked_command.as_os_str().as_bytes(), &executable) + }); + if !is_unmocked_command { self.command = Some(Command { executable, arguments, diff --git a/src/recorder/protocol_result.rs b/src/recorder/protocol_result.rs index 5e662b0..aee75bd 100644 --- a/src/recorder/protocol_result.rs +++ b/src/recorder/protocol_result.rs @@ -9,7 +9,7 @@ use crate::tracer::stdio_redirecting::CaptureStderr; use crate::tracer::Tracer; use crate::{ExitCode, R}; use std::fs::OpenOptions; -use std::path::Path; +use std::path::{Path, PathBuf}; #[derive(Debug, PartialEq)] pub enum ProtocolResult { @@ -44,7 +44,7 @@ impl ProtocolResult { interpreter: &Option>, program: &Path, protocols: Vec, - unmocked_commands: &[Vec], + unmocked_commands: &[PathBuf], ) -> R> { let mut results = vec![]; for protocol in protocols.into_iter() { @@ -52,7 +52,7 @@ impl ProtocolResult { context, &interpreter, program, - &unmocked_commands, + unmocked_commands, protocol, )?); } @@ -62,7 +62,7 @@ impl ProtocolResult { pub fn handle_results( context: &Context, protocols_file: &Path, - unmocked_commands: Vec>, + unmocked_commands: Vec, results: &[ProtocolResult], ) -> R { let checker_results = CheckerResults( @@ -85,7 +85,7 @@ impl ProtocolResult { fn handle_recorded( context: &Context, protocols_file: &Path, - unmocked_commands: Vec>, + unmocked_commands: Vec, results: &[ProtocolResult], checker_results: &CheckerResults, ) -> R<()> { @@ -117,7 +117,7 @@ fn run_against_protocol( context: &Context, interpreter: &Option>, program: &Path, - unmocked_commands: &[Vec], + unmocked_commands: &[PathBuf], protocol: Protocol, ) -> R { macro_rules! run_against_mock { From 18d7c1c2cb375c8e3ad573f7bd29ed68ef241a34 Mon Sep 17 00:00:00 2001 From: soenkehahn Date: Tue, 19 Mar 2019 12:15:00 -0400 Subject: [PATCH 2/7] use PathBuf for interpreter --- src/protocol/mod.rs | 9 +++++---- src/recorder/protocol_result.rs | 4 ++-- src/tracer/mod.rs | 21 +++++++++------------ 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/protocol/mod.rs b/src/protocol/mod.rs index 1270133..fe0aa38 100644 --- a/src/protocol/mod.rs +++ b/src/protocol/mod.rs @@ -350,7 +350,7 @@ impl Protocol { pub struct Protocols { pub protocols: Vec, pub unmocked_commands: Vec, - pub interpreter: Option>, + pub interpreter: Option, } impl Protocols { @@ -372,7 +372,7 @@ impl Protocols { fn add_interpreter(&mut self, object: &Hash) -> R<()> { if let Ok(interpreter) = object.expect_field("interpreter") { - self.interpreter = Some(interpreter.expect_str()?.as_bytes().to_vec()); + self.interpreter = Some(PathBuf::from(interpreter.expect_str()?)); } Ok(()) } @@ -857,8 +857,9 @@ mod load { ", )? .interpreter - .unwrap(), - b"/bin/bash", + .unwrap() + .to_string_lossy(), + "/bin/bash", ); Ok(()) } diff --git a/src/recorder/protocol_result.rs b/src/recorder/protocol_result.rs index aee75bd..ceca89e 100644 --- a/src/recorder/protocol_result.rs +++ b/src/recorder/protocol_result.rs @@ -41,7 +41,7 @@ impl ProtocolResult { pub fn collect_results( context: &Context, - interpreter: &Option>, + interpreter: &Option, program: &Path, protocols: Vec, unmocked_commands: &[PathBuf], @@ -115,7 +115,7 @@ impl ProtocolResult { fn run_against_protocol( context: &Context, - interpreter: &Option>, + interpreter: &Option, program: &Path, unmocked_commands: &[PathBuf], protocol: Protocol, diff --git a/src/tracer/mod.rs b/src/tracer/mod.rs index 33cb73d..3ec3619 100644 --- a/src/tracer/mod.rs +++ b/src/tracer/mod.rs @@ -21,7 +21,7 @@ use std::ffi::CString; use std::fs; use std::os::unix::ffi::OsStrExt; use std::panic; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::str; use stdio_redirecting::{CaptureStderr, Redirector}; use syscall::Syscall; @@ -80,7 +80,7 @@ impl Tracer { } fn execve_params( - interpreter: &Option>, + interpreter: &Option, program: &Path, args: Vec, env: HashMap, @@ -97,12 +97,9 @@ impl Tracer { } Ok(match interpreter { Some(interpreter) => { - c_args.push_front(CString::new(interpreter.clone())?); - ( - CString::new(interpreter.clone())?, - c_args.into_iter().collect(), - c_env, - ) + let c_interpreter = CString::new(interpreter.as_os_str().as_bytes())?; + c_args.push_front(c_interpreter.clone()); + (c_interpreter, c_args.into_iter().collect(), c_env) } None => (c_executable.clone(), c_args.into_iter().collect(), c_env), }) @@ -110,11 +107,11 @@ impl Tracer { fn format_execve_error( error: nix::Error, - interpreter: &Option>, + interpreter: &Option, program: &Path, ) -> String { let (program, interpreter) = if let Some(interpreter) = interpreter { - (program, String::from_utf8_lossy(&interpreter).to_string()) + (program, interpreter.to_string_lossy().into_owned()) } else { ( program, @@ -131,7 +128,7 @@ impl Tracer { } fn execve( - interpreter: &Option>, + interpreter: &Option, program: &Path, args: Vec, env: HashMap, @@ -144,7 +141,7 @@ impl Tracer { pub fn run_against_mock( context: &Context, - interpreter: &Option>, + interpreter: &Option, program: &Path, args: Vec, env: HashMap, From 3873ee2800313205f0b6e8a165884296b6995998 Mon Sep 17 00:00:00 2001 From: soenkehahn Date: Tue, 19 Mar 2019 12:52:29 -0400 Subject: [PATCH 3/7] use PathBuf for mocked_files --- src/protocol/mod.rs | 7 +++---- src/protocol_checker/mod.rs | 4 ++-- src/tracer/mod.rs | 9 +++++++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/protocol/mod.rs b/src/protocol/mod.rs index fe0aa38..f6fbed7 100644 --- a/src/protocol/mod.rs +++ b/src/protocol/mod.rs @@ -187,7 +187,7 @@ pub struct Protocol { pub cwd: Option>, pub stderr: Option>, pub exitcode: Option, - pub mocked_files: Vec>, + pub mocked_files: Vec, } impl Protocol { @@ -287,8 +287,7 @@ impl Protocol { fn add_mocked_files(&mut self, object: &Hash) -> R<()> { if let Ok(paths) = object.expect_field("mockedFiles") { for path in paths.expect_array()?.iter() { - self.mocked_files - .push(path.expect_str()?.as_bytes().to_owned()); + self.mocked_files.push(PathBuf::from(path.expect_str()?)); } } Ok(()) @@ -897,7 +896,7 @@ mod load { " )? .mocked_files - .map(|path| String::from_utf8_lossy(&path).into_owned()), + .map(|path| path.to_string_lossy().to_string()), vec![("/foo")] ); Ok(()) diff --git a/src/protocol_checker/mod.rs b/src/protocol_checker/mod.rs index e4ce643..b0b889d 100644 --- a/src/protocol_checker/mod.rs +++ b/src/protocol_checker/mod.rs @@ -130,10 +130,10 @@ impl SyscallMock for ProtocolChecker { Ok(()) } - fn handle_stat_exit(&self, pid: Pid, registers: &user_regs_struct, filename: Vec) -> R<()> { + fn handle_stat_exit(&self, pid: Pid, registers: &user_regs_struct, filename: PathBuf) -> R<()> { if self.protocol.mocked_files.contains(&filename) { let statbuf_ptr = registers.rsi; - let mock_mode = if filename.ends_with(b"/") { + let mock_mode = if filename.as_os_str().as_bytes().ends_with(b"/") { libc::S_IFDIR } else { libc::S_IFREG diff --git a/src/tracer/mod.rs b/src/tracer/mod.rs index 3ec3619..413724b 100644 --- a/src/tracer/mod.rs +++ b/src/tracer/mod.rs @@ -18,8 +18,10 @@ use nix::unistd::{execve, fork, getpid, ForkResult, Pid}; use std::collections::HashMap; use std::collections::VecDeque; use std::ffi::CString; +use std::ffi::OsString; use std::fs; use std::os::unix::ffi::OsStrExt; +use std::os::unix::ffi::OsStringExt; use std::panic; use std::path::{Path, PathBuf}; use std::str; @@ -52,7 +54,7 @@ pub trait SyscallMock { &self, _pid: Pid, _registers: &user_regs_struct, - _filename: Vec, + _filename: PathBuf, ) -> R<()> { Ok(()) } @@ -249,7 +251,10 @@ impl Tracer { syscall_mock.handle_getcwd_exit(pid, registers)? } (Syscall::Stat, SyscallStop::Exit) => { - let filename = tracee_memory::peek_string(pid, registers.rdi)?; + let filename = PathBuf::from(OsString::from_vec(tracee_memory::peek_string( + pid, + registers.rdi, + )?)); syscall_mock.handle_stat_exit(pid, registers, filename)? } _ => {} From d9d3c09c0a1a55e829b93fba0e90114241ad344f Mon Sep 17 00:00:00 2001 From: soenkehahn Date: Tue, 19 Mar 2019 13:09:56 -0400 Subject: [PATCH 4/7] use PathBuf for cwd --- src/protocol/mod.rs | 6 +++--- src/protocol_checker/mod.rs | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/protocol/mod.rs b/src/protocol/mod.rs index f6fbed7..09cd33e 100644 --- a/src/protocol/mod.rs +++ b/src/protocol/mod.rs @@ -184,7 +184,7 @@ pub struct Protocol { pub ends_with_hole: bool, pub arguments: Vec, pub env: HashMap, - pub cwd: Option>, + pub cwd: Option, pub stderr: Option>, pub exitcode: Option, pub mocked_files: Vec, @@ -265,7 +265,7 @@ impl Protocol { cwd ))?; } - self.cwd = Some(cwd.as_bytes().to_vec()); + self.cwd = Some(PathBuf::from(cwd)); } Ok(()) } @@ -741,7 +741,7 @@ mod load { " )? .cwd, - Some(b"/foo".to_vec()) + Some(PathBuf::from("/foo")) ); Ok(()) } diff --git a/src/protocol_checker/mod.rs b/src/protocol_checker/mod.rs index b0b889d..d989c44 100644 --- a/src/protocol_checker/mod.rs +++ b/src/protocol_checker/mod.rs @@ -120,6 +120,7 @@ impl SyscallMock for ProtocolChecker { fn handle_getcwd_exit(&self, pid: Pid, registers: &user_regs_struct) -> R<()> { if let Some(mock_cwd) = &self.protocol.cwd { + let mock_cwd = mock_cwd.as_os_str().as_bytes(); let buffer_ptr = registers.rdi; let max_size = registers.rsi; tracee_memory::poke_string(pid, buffer_ptr, mock_cwd, max_size)?; From 52f9257f855d06b2869632041a058dc1c50e4702 Mon Sep 17 00:00:00 2001 From: soenkehahn Date: Tue, 19 Mar 2019 13:28:55 -0400 Subject: [PATCH 5/7] use PathBuf for Command.executable --- src/protocol/command.rs | 46 +++++++++++++++++++---------------- src/protocol/mod.rs | 14 +++++------ src/protocol_checker/mod.rs | 14 ++++++----- src/recorder/hole_recorder.rs | 2 +- src/recorder/mod.rs | 7 ++++-- src/tracer/mod.rs | 7 ++++-- 6 files changed, 51 insertions(+), 39 deletions(-) diff --git a/src/protocol/command.rs b/src/protocol/command.rs index e161bd2..bf16713 100644 --- a/src/protocol/command.rs +++ b/src/protocol/command.rs @@ -1,11 +1,13 @@ use super::argument_parser::Parser; use super::executable_path; use crate::R; +use std::os::unix::ffi::OsStrExt; +use std::path::PathBuf; use std::str; #[derive(PartialEq, Eq, Debug, Clone)] pub struct Command { - pub executable: Vec, + pub executable: PathBuf, pub arguments: Vec>, } @@ -19,8 +21,10 @@ impl Command { } pub fn compare(&self, other: &Command) -> bool { - executable_path::compare_executables(&self.executable, &other.executable) - && self.arguments == other.arguments + executable_path::compare_executables( + &self.executable.as_os_str().as_bytes(), + &other.executable.as_os_str().as_bytes(), + ) && self.arguments == other.arguments } fn escape(word: String) -> String { @@ -45,8 +49,10 @@ impl Command { } pub fn format(&self) -> String { - let executable = - String::from_utf8_lossy(&executable_path::canonicalize(&self.executable)).into_owned(); + let executable = String::from_utf8_lossy(&executable_path::canonicalize( + &self.executable.as_os_str().as_bytes(), + )) + .into_owned(); if self.arguments.is_empty() { executable.to_string() } else { @@ -59,13 +65,11 @@ impl Command { } pub fn new(command: &str) -> R { - let mut words = Parser::parse_arguments(command)? - .into_iter() - .map(|word| word.into_bytes()); + let mut words = Parser::parse_arguments(command)?.into_iter(); match words.next() { Some(executable) => Ok(Command { - executable, - arguments: words.collect(), + executable: PathBuf::from(executable), + arguments: words.map(|word| word.into_bytes()).collect(), }), None => Err(format!( "expected: space-separated command and arguments ({:?})", @@ -89,7 +93,7 @@ mod command { assert_eq!( Command::new("foo bar")?, Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![b"bar".to_vec()] } ); @@ -101,7 +105,7 @@ mod command { assert_eq!( Command::new(r#"foo "bar baz""#)?, Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![b"bar baz".to_vec()] } ); @@ -113,14 +117,14 @@ mod command { assert_eq!( Command::new(r#"foo\" bar baz"#)?, Command { - executable: b"foo\"".to_vec(), + executable: PathBuf::from("foo\""), arguments: vec![b"bar", b"baz"].map(|arg| arg.to_vec()) } ); assert_eq!( Command::new(r#"foo\" "bar baz""#)?, Command { - executable: b"foo\"".to_vec(), + executable: PathBuf::from("foo\""), arguments: vec![b"bar baz".to_vec()] } ); @@ -132,7 +136,7 @@ mod command { assert_eq!( Command::new(r#"foo "bar\" baz""#)?, Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![b"bar\" baz".to_vec()] } ); @@ -171,7 +175,7 @@ mod command { assert_eq!( Command::new("foo bar")?, Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![b"bar".to_vec()] } ); @@ -183,7 +187,7 @@ mod command { assert_eq!( Command::new(" foo bar")?, Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![b"bar".to_vec()] } ); @@ -195,7 +199,7 @@ mod command { assert_eq!( Command::new("foo bar ")?, Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![b"bar".to_vec()] } ); @@ -210,7 +214,7 @@ mod command { assert_eq!( Command::new(r#"foo "bar\nbaz""#)?, Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![b"bar\nbaz".to_vec()] } ); @@ -222,7 +226,7 @@ mod command { assert_eq!( Command::new(r#"foo bar\ baz"#)?, Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![b"bar baz".to_vec()] } ); @@ -234,7 +238,7 @@ mod command { assert_eq!( Command::new(r#"foo bar\\baz"#)?, Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![br"bar\baz".to_vec()] } ); diff --git a/src/protocol/mod.rs b/src/protocol/mod.rs index 09cd33e..02eb4c6 100644 --- a/src/protocol/mod.rs +++ b/src/protocol/mod.rs @@ -98,7 +98,7 @@ mod parse_step { assert_eq!( test_parse_step(r#""foo""#)?, Step::new(Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![], }), ); @@ -110,7 +110,7 @@ mod parse_step { assert_eq!( test_parse_step(r#""foo bar""#)?.command, Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![b"bar".to_vec()], }, ); @@ -122,7 +122,7 @@ mod parse_step { assert_eq!( test_parse_step(r#"{command: "foo"}"#)?, Step::new(Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![], }), ); @@ -134,7 +134,7 @@ mod parse_step { assert_eq!( test_parse_step(r#"{command: "foo bar"}"#)?.command, Command { - executable: b"foo".to_vec(), + executable: PathBuf::from("foo"), arguments: vec![b"bar".to_vec()], }, ); @@ -508,7 +508,7 @@ mod load { ", )?, Protocol::new(vec![Step::new(Command { - executable: b"/bin/true".to_vec(), + executable: PathBuf::from("/bin/true"), arguments: vec![], })]), ); @@ -535,7 +535,7 @@ mod load { )? .steps .map(|step| step.command.executable), - vec![b"/bin/true".to_vec(), b"/bin/false".to_vec()], + vec![PathBuf::from("/bin/true"), PathBuf::from("/bin/false")], ); Ok(()) } @@ -566,7 +566,7 @@ mod load { " )?, Protocol::new(vec![Step::new(Command { - executable: b"/bin/true".to_vec(), + executable: PathBuf::from("/bin/true"), arguments: vec![], })]), ); diff --git a/src/protocol_checker/mod.rs b/src/protocol_checker/mod.rs index d989c44..9459c29 100644 --- a/src/protocol_checker/mod.rs +++ b/src/protocol_checker/mod.rs @@ -97,14 +97,16 @@ impl SyscallMock for ProtocolChecker { &mut self, pid: Pid, registers: &user_regs_struct, - executable: Vec, + executable: PathBuf, arguments: Vec>, ) -> R<()> { - if !self - .unmocked_commands - .iter() - .any(|c| protocol::compare_executables(&c.as_os_str().as_bytes(), &executable)) - { + let is_unmocked_command = self.unmocked_commands.iter().any(|unmocked_command| { + protocol::compare_executables( + &unmocked_command.as_os_str().as_bytes(), + &executable.as_os_str().as_bytes(), + ) + }); + if !is_unmocked_command { let mock_executable_path = self.handle_step(protocol::Command { executable, arguments, diff --git a/src/recorder/hole_recorder.rs b/src/recorder/hole_recorder.rs index ff364ca..0d01dc1 100644 --- a/src/recorder/hole_recorder.rs +++ b/src/recorder/hole_recorder.rs @@ -41,7 +41,7 @@ impl SyscallMock for HoleRecorder { &mut self, pid: Pid, registers: &user_regs_struct, - executable: Vec, + executable: PathBuf, arguments: Vec>, ) -> R<()> { match self { diff --git a/src/recorder/mod.rs b/src/recorder/mod.rs index 96eecda..a9eefc8 100644 --- a/src/recorder/mod.rs +++ b/src/recorder/mod.rs @@ -42,11 +42,14 @@ impl SyscallMock for Recorder { &mut self, _pid: Pid, _registers: &user_regs_struct, - executable: Vec, + executable: PathBuf, arguments: Vec>, ) -> R<()> { let is_unmocked_command = self.unmocked_commands.iter().any(|unmocked_command| { - compare_executables(unmocked_command.as_os_str().as_bytes(), &executable) + compare_executables( + unmocked_command.as_os_str().as_bytes(), + executable.as_os_str().as_bytes(), + ) }); if !is_unmocked_command { self.command = Some(Command { diff --git a/src/tracer/mod.rs b/src/tracer/mod.rs index 413724b..f3bc665 100644 --- a/src/tracer/mod.rs +++ b/src/tracer/mod.rs @@ -36,7 +36,7 @@ pub trait SyscallMock { &mut self, _pid: Pid, _registers: &user_regs_struct, - _executable: Vec, + _executable: PathBuf, _arguments: Vec>, ) -> R<()> { Ok(()) @@ -242,7 +242,10 @@ impl Tracer { match (&syscall, syscall_stop) { (Syscall::Execve, SyscallStop::Enter) => { if self.tracee_pid != pid { - let executable = tracee_memory::peek_string(pid, registers.rdi)?; + let executable = PathBuf::from(OsString::from_vec(tracee_memory::peek_string( + pid, + registers.rdi, + )?)); let arguments = tracee_memory::peek_string_array(pid, registers.rsi)?; syscall_mock.handle_execve_enter(pid, registers, executable, arguments)?; } From a9ddea94de0febaf9457f6e59cf596c19ed115ba Mon Sep 17 00:00:00 2001 From: soenkehahn Date: Tue, 19 Mar 2019 13:51:18 -0400 Subject: [PATCH 6/7] use OsString for Command.arguments --- src/protocol/command.rs | 31 ++++++++++++++++--------------- src/protocol/mod.rs | 13 +++++-------- src/protocol_checker/mod.rs | 3 ++- src/recorder/hole_recorder.rs | 3 ++- src/recorder/mod.rs | 3 ++- src/tracer/mod.rs | 7 +++++-- 6 files changed, 32 insertions(+), 28 deletions(-) diff --git a/src/protocol/command.rs b/src/protocol/command.rs index bf16713..bea5dac 100644 --- a/src/protocol/command.rs +++ b/src/protocol/command.rs @@ -1,6 +1,7 @@ use super::argument_parser::Parser; use super::executable_path; use crate::R; +use std::ffi::OsString; use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; use std::str; @@ -8,7 +9,7 @@ use std::str; #[derive(PartialEq, Eq, Debug, Clone)] pub struct Command { pub executable: PathBuf, - pub arguments: Vec>, + pub arguments: Vec, } impl Command { @@ -39,10 +40,10 @@ impl Command { word.chars().map(escape_char).collect::>().join("") } - pub fn format_arguments(arguments: Vec>) -> String { + pub fn format_arguments(arguments: Vec) -> String { arguments .into_iter() - .map(|argument| Command::escape(String::from_utf8_lossy(&argument).to_string())) + .map(|argument| Command::escape(argument.to_string_lossy().into_owned())) .map(Command::add_quotes_if_needed) .collect::>() .join(" ") @@ -69,7 +70,7 @@ impl Command { match words.next() { Some(executable) => Ok(Command { executable: PathBuf::from(executable), - arguments: words.map(|word| word.into_bytes()).collect(), + arguments: words.map(OsString::from).collect(), }), None => Err(format!( "expected: space-separated command and arguments ({:?})", @@ -94,7 +95,7 @@ mod command { Command::new("foo bar")?, Command { executable: PathBuf::from("foo"), - arguments: vec![b"bar".to_vec()] + arguments: vec![OsString::from("bar")] } ); Ok(()) @@ -106,7 +107,7 @@ mod command { Command::new(r#"foo "bar baz""#)?, Command { executable: PathBuf::from("foo"), - arguments: vec![b"bar baz".to_vec()] + arguments: vec![OsString::from("bar baz")] } ); Ok(()) @@ -118,14 +119,14 @@ mod command { Command::new(r#"foo\" bar baz"#)?, Command { executable: PathBuf::from("foo\""), - arguments: vec![b"bar", b"baz"].map(|arg| arg.to_vec()) + arguments: vec!["bar", "baz"].map(OsString::from) } ); assert_eq!( Command::new(r#"foo\" "bar baz""#)?, Command { executable: PathBuf::from("foo\""), - arguments: vec![b"bar baz".to_vec()] + arguments: vec![OsString::from("bar baz")] } ); Ok(()) @@ -137,7 +138,7 @@ mod command { Command::new(r#"foo "bar\" baz""#)?, Command { executable: PathBuf::from("foo"), - arguments: vec![b"bar\" baz".to_vec()] + arguments: vec![OsString::from("bar\" baz")] } ); Ok(()) @@ -176,7 +177,7 @@ mod command { Command::new("foo bar")?, Command { executable: PathBuf::from("foo"), - arguments: vec![b"bar".to_vec()] + arguments: vec![OsString::from("bar")] } ); Ok(()) @@ -188,7 +189,7 @@ mod command { Command::new(" foo bar")?, Command { executable: PathBuf::from("foo"), - arguments: vec![b"bar".to_vec()] + arguments: vec![OsString::from("bar")] } ); Ok(()) @@ -200,7 +201,7 @@ mod command { Command::new("foo bar ")?, Command { executable: PathBuf::from("foo"), - arguments: vec![b"bar".to_vec()] + arguments: vec![OsString::from("bar")] } ); Ok(()) @@ -215,7 +216,7 @@ mod command { Command::new(r#"foo "bar\nbaz""#)?, Command { executable: PathBuf::from("foo"), - arguments: vec![b"bar\nbaz".to_vec()] + arguments: vec![OsString::from("bar\nbaz")] } ); Ok(()) @@ -227,7 +228,7 @@ mod command { Command::new(r#"foo bar\ baz"#)?, Command { executable: PathBuf::from("foo"), - arguments: vec![b"bar baz".to_vec()] + arguments: vec![OsString::from("bar baz")] } ); Ok(()) @@ -239,7 +240,7 @@ mod command { Command::new(r#"foo bar\\baz"#)?, Command { executable: PathBuf::from("foo"), - arguments: vec![br"bar\baz".to_vec()] + arguments: vec![OsString::from(r"bar\baz")] } ); Ok(()) diff --git a/src/protocol/mod.rs b/src/protocol/mod.rs index 02eb4c6..99011f7 100644 --- a/src/protocol/mod.rs +++ b/src/protocol/mod.rs @@ -13,6 +13,7 @@ use crate::R; pub use command::Command; use linked_hash_map::LinkedHashMap; use std::collections::{HashMap, VecDeque}; +use std::ffi::OsString; use std::fs; use std::path::{Path, PathBuf}; use yaml_rust::{yaml::Hash, Yaml, YamlLoader}; @@ -111,7 +112,7 @@ mod parse_step { test_parse_step(r#""foo bar""#)?.command, Command { executable: PathBuf::from("foo"), - arguments: vec![b"bar".to_vec()], + arguments: vec![OsString::from("bar")], }, ); Ok(()) @@ -135,7 +136,7 @@ mod parse_step { test_parse_step(r#"{command: "foo bar"}"#)?.command, Command { executable: PathBuf::from("foo"), - arguments: vec![b"bar".to_vec()], + arguments: vec![OsString::from("bar")], }, ); Ok(()) @@ -317,11 +318,7 @@ impl Protocol { fn serialize(&self) -> Yaml { let mut protocol = LinkedHashMap::new(); if !self.arguments.is_empty() { - let arguments = self - .arguments - .iter() - .map(|arg| arg.as_bytes().to_vec()) - .collect(); + let arguments = self.arguments.iter().map(OsString::from).collect(); protocol.insert( Yaml::from_str("arguments"), Yaml::String(Command::format_arguments(arguments)), @@ -551,7 +548,7 @@ mod load { )? .steps .map(|step| step.command.arguments), - vec![vec![b"foo".to_vec(), b"bar".to_vec()]], + vec![vec![OsString::from("foo"), OsString::from("bar")]], ); Ok(()) } diff --git a/src/protocol_checker/mod.rs b/src/protocol_checker/mod.rs index 9459c29..1fe8c5c 100644 --- a/src/protocol_checker/mod.rs +++ b/src/protocol_checker/mod.rs @@ -12,6 +12,7 @@ use checker_result::CheckerResult; use libc::{c_ulonglong, user_regs_struct}; use nix::sys::ptrace; use nix::unistd::Pid; +use std::ffi::OsString; use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; @@ -98,7 +99,7 @@ impl SyscallMock for ProtocolChecker { pid: Pid, registers: &user_regs_struct, executable: PathBuf, - arguments: Vec>, + arguments: Vec, ) -> R<()> { let is_unmocked_command = self.unmocked_commands.iter().any(|unmocked_command| { protocol::compare_executables( diff --git a/src/recorder/hole_recorder.rs b/src/recorder/hole_recorder.rs index 0d01dc1..3321be7 100644 --- a/src/recorder/hole_recorder.rs +++ b/src/recorder/hole_recorder.rs @@ -9,6 +9,7 @@ use crate::tracer::SyscallMock; use crate::{ExitCode, R}; use libc::user_regs_struct; use nix::unistd::Pid; +use std::ffi::OsString; use std::path::{Path, PathBuf}; pub enum HoleRecorder { @@ -42,7 +43,7 @@ impl SyscallMock for HoleRecorder { pid: Pid, registers: &user_regs_struct, executable: PathBuf, - arguments: Vec>, + arguments: Vec, ) -> R<()> { match self { HoleRecorder::Checker { diff --git a/src/recorder/mod.rs b/src/recorder/mod.rs index a9eefc8..9aa4764 100644 --- a/src/recorder/mod.rs +++ b/src/recorder/mod.rs @@ -8,6 +8,7 @@ use crate::tracer::SyscallMock; use crate::R; use libc::user_regs_struct; use nix::unistd::Pid; +use std::ffi::OsString; use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; @@ -43,7 +44,7 @@ impl SyscallMock for Recorder { _pid: Pid, _registers: &user_regs_struct, executable: PathBuf, - arguments: Vec>, + arguments: Vec, ) -> R<()> { let is_unmocked_command = self.unmocked_commands.iter().any(|unmocked_command| { compare_executables( diff --git a/src/tracer/mod.rs b/src/tracer/mod.rs index f3bc665..8628b2e 100644 --- a/src/tracer/mod.rs +++ b/src/tracer/mod.rs @@ -37,7 +37,7 @@ pub trait SyscallMock { _pid: Pid, _registers: &user_regs_struct, _executable: PathBuf, - _arguments: Vec>, + _arguments: Vec, ) -> R<()> { Ok(()) } @@ -246,7 +246,10 @@ impl Tracer { pid, registers.rdi, )?)); - let arguments = tracee_memory::peek_string_array(pid, registers.rsi)?; + let arguments = tracee_memory::peek_string_array(pid, registers.rsi)? + .into_iter() + .map(OsString::from_vec) + .collect(); syscall_mock.handle_execve_enter(pid, registers, executable, arguments)?; } } From d99e9078766648e208a0a2724703afc7bccc4fd6 Mon Sep 17 00:00:00 2001 From: soenkehahn Date: Tue, 19 Mar 2019 14:10:36 -0400 Subject: [PATCH 7/7] refactor executable_path --- src/protocol/command.rs | 16 ++++------ src/protocol/executable_path.rs | 54 +++++++++++++++------------------ src/protocol_checker/mod.rs | 10 +++--- src/recorder/mod.rs | 11 +++---- 4 files changed, 39 insertions(+), 52 deletions(-) diff --git a/src/protocol/command.rs b/src/protocol/command.rs index bea5dac..aa93446 100644 --- a/src/protocol/command.rs +++ b/src/protocol/command.rs @@ -2,7 +2,6 @@ use super::argument_parser::Parser; use super::executable_path; use crate::R; use std::ffi::OsString; -use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; use std::str; @@ -22,10 +21,8 @@ impl Command { } pub fn compare(&self, other: &Command) -> bool { - executable_path::compare_executables( - &self.executable.as_os_str().as_bytes(), - &other.executable.as_os_str().as_bytes(), - ) && self.arguments == other.arguments + executable_path::compare_executables(&self.executable, &other.executable) + && self.arguments == other.arguments } fn escape(word: String) -> String { @@ -50,12 +47,11 @@ impl Command { } pub fn format(&self) -> String { - let executable = String::from_utf8_lossy(&executable_path::canonicalize( - &self.executable.as_os_str().as_bytes(), - )) - .into_owned(); + let executable = executable_path::canonicalize(&self.executable) + .to_string_lossy() + .into_owned(); if self.arguments.is_empty() { - executable.to_string() + executable } else { format!( "{} {}", diff --git a/src/protocol/executable_path.rs b/src/protocol/executable_path.rs index 3ecf543..b344c20 100644 --- a/src/protocol/executable_path.rs +++ b/src/protocol/executable_path.rs @@ -1,9 +1,7 @@ use quale::which; -use std::ffi::OsStr; -use std::os::unix::ffi::OsStrExt; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; -pub fn compare_executables(a: &[u8], b: &[u8]) -> bool { +pub fn compare_executables(a: &Path, b: &Path) -> bool { canonicalize(a) == canonicalize(b) } @@ -14,15 +12,15 @@ mod compare_executables { #[test] fn returns_true_if_executables_are_identical() -> R<()> { - let executable = b"./bin/myexec"; + let executable = Path::new("./bin/myexec"); assert!(compare_executables(executable, executable)); Ok(()) } #[test] fn returns_false_if_executables_are_distinct() -> R<()> { - let a = b"./bin/myexec"; - let b = b"./bin/myotherexec"; + let a = Path::new("./bin/myexec"); + let b = Path::new("./bin/myotherexec"); assert!(!compare_executables(a, b)); Ok(()) } @@ -30,30 +28,28 @@ mod compare_executables { #[test] fn returns_true_if_executables_match_after_lookup_in_path() -> R<()> { let path = which("cp").unwrap(); - let cp_long = path.as_os_str().as_bytes(); - let cp_short = b"cp"; - assert!(compare_executables(cp_long, cp_short)); + let cp_long = path; + let cp_short = Path::new("cp"); + assert!(compare_executables(&cp_long, cp_short)); Ok(()) } } -pub fn canonicalize(executable: &[u8]) -> Vec { - let path = PathBuf::from(OsStr::from_bytes(executable)); - let file_name = match path.file_name() { - None => return executable.to_vec(), +pub fn canonicalize(executable: &Path) -> PathBuf { + let file_name = match executable.file_name() { + None => return executable.into(), Some(f) => f, }; match which(file_name) { Some(resolved) => { - if resolved == path { - file_name.as_bytes() + if resolved == executable { + PathBuf::from(file_name) } else { - executable + executable.into() } } - None => executable, + None => executable.into(), } - .to_vec() } #[cfg(test)] @@ -66,40 +62,40 @@ mod canonicalize { fn shortens_absolute_executable_paths_if_found_in_path() -> R<()> { let executable = "cp"; let resolved = which(executable).unwrap(); - let file_name = canonicalize(resolved.as_os_str().as_bytes()); - assert_eq!(String::from_utf8(file_name)?, "cp"); + let file_name = canonicalize(&resolved); + assert_eq!(file_name, PathBuf::from("cp")); Ok(()) } #[test] fn does_not_shorten_executable_that_is_not_in_path() -> R<()> { - let executable = b"/foo/doesnotexist"; + let executable = Path::new("/foo/doesnotexist"); let file_name = canonicalize(executable); - assert_eq!(String::from_utf8(file_name)?, "/foo/doesnotexist"); + assert_eq!(file_name, PathBuf::from("/foo/doesnotexist")); Ok(()) } #[test] fn does_not_shorten_executable_that_is_not_in_path_but_has_same_name_as_one_that_is() -> R<()> { - let executable = b"/not/in/path/ls"; + let executable = Path::new("/not/in/path/ls"); let file_name = canonicalize(executable); - assert_eq!(String::from_utf8(file_name)?, "/not/in/path/ls"); + assert_eq!(file_name, PathBuf::from("/not/in/path/ls")); Ok(()) } #[test] fn does_not_shorten_relative_path() -> R<()> { - let executable = b"./foo"; + let executable = Path::new("./foo"); let file_name = canonicalize(executable); - assert_eq!(String::from_utf8(file_name)?, "./foo"); + assert_eq!(file_name, PathBuf::from("./foo")); Ok(()) } #[test] fn does_not_modify_short_forms_if_found_in_path() -> R<()> { - let executable = b"ls"; + let executable = Path::new("ls"); let file_name = canonicalize(executable); - assert_eq!(String::from_utf8(file_name)?, "ls"); + assert_eq!(file_name, PathBuf::from("ls")); Ok(()) } } diff --git a/src/protocol_checker/mod.rs b/src/protocol_checker/mod.rs index 1fe8c5c..38704f5 100644 --- a/src/protocol_checker/mod.rs +++ b/src/protocol_checker/mod.rs @@ -101,12 +101,10 @@ impl SyscallMock for ProtocolChecker { executable: PathBuf, arguments: Vec, ) -> R<()> { - let is_unmocked_command = self.unmocked_commands.iter().any(|unmocked_command| { - protocol::compare_executables( - &unmocked_command.as_os_str().as_bytes(), - &executable.as_os_str().as_bytes(), - ) - }); + let is_unmocked_command = self + .unmocked_commands + .iter() + .any(|unmocked_command| protocol::compare_executables(unmocked_command, &executable)); if !is_unmocked_command { let mock_executable_path = self.handle_step(protocol::Command { executable, diff --git a/src/recorder/mod.rs b/src/recorder/mod.rs index 9aa4764..ce83ac9 100644 --- a/src/recorder/mod.rs +++ b/src/recorder/mod.rs @@ -9,7 +9,6 @@ use crate::R; use libc::user_regs_struct; use nix::unistd::Pid; use std::ffi::OsString; -use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; pub struct Recorder { @@ -46,12 +45,10 @@ impl SyscallMock for Recorder { executable: PathBuf, arguments: Vec, ) -> R<()> { - let is_unmocked_command = self.unmocked_commands.iter().any(|unmocked_command| { - compare_executables( - unmocked_command.as_os_str().as_bytes(), - executable.as_os_str().as_bytes(), - ) - }); + let is_unmocked_command = self + .unmocked_commands + .iter() + .any(|unmocked_command| compare_executables(unmocked_command, &executable)); if !is_unmocked_command { self.command = Some(Command { executable,