diff --git a/README.md b/README.md index a0b1ae2..110e9f9 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,8 @@ All tools return structured JSON (via Rizin's `j`-suffixed commands) in addition to a text rendering, so clients can parse results directly. **Session** -- `open_file` — open a binary/file or Rizin URI (`malloc://`, `hex://`, …) and run analysis +- `open_file` — open a binary/file or Rizin URI (`malloc://`, `hex://`, …) and run + analysis; `-` opens a session with no file - `close_file`, `session_info`, `analyze` (none/basic/full/experimental), `seek` - `command`, `command_json` — raw Rizin passthrough (see *Security* below) diff --git a/src/error.rs b/src/error.rs index 416f05b..9344c15 100644 --- a/src/error.rs +++ b/src/error.rs @@ -128,6 +128,15 @@ pub enum RizinError { /// A path that should exist was not found. #[error("file not found: {0}")] FileNotFound(String), + + /// Rizin could not open the target. + #[error("rizin cannot open '{target}'{}", fmt_detail(.detail))] + OpenFailed { + /// The path or URI. + target: String, + /// What Rizin said, possibly empty. + detail: String, + }, } /// Render diagnostics after the main error message. @@ -167,6 +176,7 @@ impl RizinError { | IllegalCommand { .. } | EmptyJson { .. } | FileNotFound(_) + | OpenFailed { .. } | NotPermitted(_) => ErrorData::invalid_params(self.to_string(), None), _ => ErrorData::internal_error(self.to_string(), None), } diff --git a/src/ffi.rs b/src/ffi.rs index f73c5e1..0a31575 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -226,8 +226,11 @@ impl Core { /// Open and load a target (path or Rizin URI) into the core. /// - /// Returns whether Rizin reported success. - pub fn file_open_load(&self, path: &str, write: bool) -> Result { + /// # Errors + /// [`RizinError::InvalidArgument`] for an interior NUL, and + /// [`RizinError::OpenFailed`], with what Rizin logged, if Rizin cannot open + /// the target. + pub fn file_open_load(&self, path: &str, write: bool) -> Result<(), RizinError> { let c = CString::new(path).map_err(|_| RizinError::InvalidArgument { arg: path.to_string(), reason: "path contains an interior NUL byte".into(), @@ -238,8 +241,17 @@ impl Core { RZ_PERM_R }; let _g = lock(); - let ok = unsafe { rz_core_file_open_load(self.ptr, c.as_ptr(), 0, perms, write) }; - Ok(ok) + let (ok, messages) = with_log_capture(|| unsafe { + rz_core_file_open_load(self.ptr, c.as_ptr(), 0, perms, write) + }); + if ok { + Ok(()) + } else { + Err(RizinError::OpenFailed { + target: path.to_string(), + detail: messages.join("\n"), + }) + } } } diff --git a/src/params.rs b/src/params.rs index 2f261fb..62953c9 100644 --- a/src/params.rs +++ b/src/params.rs @@ -17,7 +17,7 @@ use crate::session::AnalysisLevel; #[derive(Debug, Deserialize, JsonSchema)] pub struct OpenFileParams { /// Path to the binary/file to open, or a Rizin URI (e.g. `malloc://512`, - /// `hex://9090`). Use `-` for an empty session. + /// `hex://9090`). Use `-` for an empty session with no file open. pub path: String, /// Analysis depth to run immediately after opening. Defaults to `basic`. #[serde(default)] diff --git a/src/pipe.rs b/src/pipe.rs index 23ec8c3..61aeb19 100644 --- a/src/pipe.rs +++ b/src/pipe.rs @@ -69,15 +69,17 @@ impl RizinPipe { /// Spawn `rizin -q0 [extra_args...] ` and wait for it to become /// ready. /// - /// `target` may be a filesystem path or any Rizin URI (e.g. `malloc://512`) - /// or `-` for an empty session. + /// `target` may be a filesystem path or any Rizin URI (e.g. `malloc://512`), + /// or `None` for an empty session. (`rizin -` would read a file from stdin, + /// which is the command pipe.) /// /// # Errors - /// Returns an error if the process cannot be spawned, the startup handshake - /// fails, or Rizin does not become ready within `startup_timeout`. + /// Returns an error if the process cannot be spawned, Rizin cannot open the + /// target, the startup handshake fails, or Rizin does not become ready + /// within `startup_timeout`. pub async fn spawn( binary: &str, - target: &str, + target: Option<&str>, extra_args: &[String], startup_timeout: Duration, cmd_timeout: Duration, @@ -85,7 +87,7 @@ impl RizinPipe { let mut child = Command::new(binary) .arg("-q0") .args(extra_args) - .arg(target) + .args(target) .stdin(Stdio::piped()) .stdout(Stdio::piped()) // Read Rizin's diagnostics where that is supported; they are @@ -111,13 +113,22 @@ impl RizinPipe { .take() .ok_or_else(|| RizinError::Io("child process has no stdout".into()))?; let stderr = child.stderr.take(); + let target_owned = target.map(str::to_string); let (tx, rx) = mpsc::unbounded_channel::(); let (ready_tx, ready_rx) = oneshot::channel::>(); let join = std::thread::Builder::new() .name("rizin-pipe".into()) - .spawn(move || worker_loop(stdin, PipeIo::new(stdout, stderr), ready_tx, rx)) + .spawn(move || { + worker_loop( + stdin, + PipeIo::new(stdout, stderr), + target_owned, + ready_tx, + rx, + ) + }) .map_err(|e| RizinError::Io(format!("failed to spawn worker thread: {e}")))?; match tokio::time::timeout(startup_timeout, ready_rx).await { @@ -216,6 +227,7 @@ impl Drop for RizinPipe { fn worker_loop( mut stdin: ChildStdin, mut io: PipeIo, + target: Option, ready_tx: oneshot::Sender>, mut rx: mpsc::UnboundedReceiver, ) { @@ -223,13 +235,18 @@ fn worker_loop( match io.read_frame() { Ok(Some(_)) => {} Ok(None) => { + // Rizin exits before the handshake when it cannot open the target. let detail = io.take_stderr().join("\n"); - let reason = if detail.is_empty() { - "rizin exited before it was ready".to_string() - } else { - format!("rizin exited before it was ready: {detail}") + let err = match target { + Some(target) => RizinError::OpenFailed { target, detail }, + None if detail.is_empty() => { + RizinError::Handshake("rizin exited before it was ready".into()) + } + None => { + RizinError::Handshake(format!("rizin exited before it was ready: {detail}")) + } }; - let _ = ready_tx.send(Err(RizinError::Handshake(reason))); + let _ = ready_tx.send(Err(err)); return; } Err(e) => { diff --git a/src/rzcore.rs b/src/rzcore.rs index 9cb73f1..81a03fc 100644 --- a/src/rzcore.rs +++ b/src/rzcore.rs @@ -61,14 +61,14 @@ pub struct RizinCore { } impl RizinCore { - /// Create a core, open `path` (a filesystem path or Rizin URI), and get it - /// ready for commands. + /// Create a core, open `path` (a filesystem path or Rizin URI; `None` for + /// an empty session), and get it ready for commands. /// /// # Errors /// Returns an error if the core cannot be allocated, the target fails to /// open, or readiness is not reached within `startup_timeout`. pub async fn open( - path: &str, + path: Option<&str>, write: bool, startup_timeout: Duration, cmd_timeout: Duration, @@ -76,7 +76,7 @@ impl RizinCore { let (tx, rx) = mpsc::unbounded_channel::(); let (ready_tx, ready_rx) = oneshot::channel::>(); let (done_tx, done_rx) = std_mpsc::channel::<()>(); - let path_owned = path.to_string(); + let path_owned = path.map(str::to_string); std::thread::Builder::new() .name("rizin-core".into()) @@ -158,7 +158,7 @@ impl Drop for RizinCore { /// Worker thread: own the core for its entire lifetime, service commands. fn worker_loop( - path: String, + path: Option, write: bool, ready_tx: oneshot::Sender>, mut rx: mpsc::UnboundedReceiver, @@ -176,16 +176,11 @@ fn worker_loop( }; core.configure(); - match core.file_open_load(&path, write) { - Ok(true) => {} - Ok(false) => { - let _ = ready_tx.send(Err(RizinError::FileNotFound(path))); - return; - } - Err(e) => { - let _ = ready_tx.send(Err(e)); - return; - } + if let Some(path) = path + && let Err(e) = core.file_open_load(&path, write) + { + let _ = ready_tx.send(Err(e)); + return; } if ready_tx.send(Ok(())).is_err() { @@ -211,7 +206,7 @@ mod tests { // Open a tiny in-memory target so the session is live. `malloc://` // needs no file on disk. let core = RizinCore::open( - "malloc://64", + Some("malloc://64"), false, Duration::from_secs(30), Duration::from_secs(30), @@ -236,7 +231,7 @@ mod tests { // Drop) before the next is created. for _ in 0..5 { let core = RizinCore::open( - "malloc://16", + Some("malloc://16"), false, Duration::from_secs(30), Duration::from_secs(30), @@ -253,7 +248,7 @@ mod tests { #[tokio::test] async fn failure_carries_status_and_log_message() { let core = RizinCore::open( - "malloc://64", + Some("malloc://64"), true, Duration::from_secs(30), Duration::from_secs(30), diff --git a/src/session.rs b/src/session.rs index 3cd8095..18a7f65 100644 --- a/src/session.rs +++ b/src/session.rs @@ -18,6 +18,9 @@ use crate::pipe::RizinPipe; use crate::rzcore::RizinCore; use crate::status::CmdOutput; +/// The `open_file` path that opens a session without a file. +pub const EMPTY_SESSION: &str = "-"; + /// How much automatic analysis to run when opening a file. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "lowercase")] @@ -108,9 +111,9 @@ impl SessionManager { &self.config } - /// Open `path` (a filesystem path or Rizin URI), optionally enabling write - /// mode, and run the requested analysis level. Replaces any current - /// session (closing the previous backend). + /// Open `path` (a filesystem path or Rizin URI, or `-` for an empty + /// session), optionally enabling write mode, and run the requested analysis + /// level. Replaces any current session (closing the previous backend). /// /// # Errors /// Returns an error if write mode is requested but disabled, a plain path @@ -126,8 +129,18 @@ impl SessionManager { "write mode is disabled; start the server with --allow-write".into(), )); } - if !is_rizin_uri(path) && path != "-" && !Path::new(path).exists() { - return Err(RizinError::FileNotFound(path.to_string())); + if !is_rizin_uri(path) && path != EMPTY_SESSION { + if !Path::new(path).exists() { + return Err(RizinError::FileNotFound(path.to_string())); + } + // The `rizin` CLI refuses directories, but the library opens one as + // an endless file. + if Path::new(path).is_dir() { + return Err(RizinError::InvalidArgument { + arg: path.to_string(), + reason: "is a directory, not a file".into(), + }); + } } // Tear down any existing session first, so at most one backend is ever @@ -171,11 +184,12 @@ impl SessionManager { /// Construct the configured backend for `path`. async fn spawn_backend(&self, path: &str, write: bool) -> Result { + let target = (path != EMPTY_SESSION).then_some(path); match self.config.transport { #[cfg(feature = "api")] Transport::Api => Ok(Backend::Api( RizinCore::open( - path, + target, write, self.config.startup_timeout, self.config.command_timeout, @@ -196,7 +210,7 @@ impl SessionManager { Ok(Backend::Pipe( RizinPipe::spawn( &self.config.rizin_binary, - path, + target, &extra, self.config.startup_timeout, self.config.command_timeout, diff --git a/tests/tools.rs b/tests/tools.rs index b1e2be7..d9f8a75 100644 --- a/tests/tools.rs +++ b/tests/tools.rs @@ -275,3 +275,46 @@ async fn writes_are_verified() { assert_eq!(read("0x60", 4).await, "deadbeef", "{t:?}"); } } + +#[tokio::test] +async fn empty_session_and_open_failures() { + for t in transports() { + let s = Session::start(t, |_| {}).await; + // `-` opens a session without a file on both backends. + let info = s + .ok("open_file", json!({ "path": "-", "analysis": "none" })) + .await; + assert_eq!(info["file"], "-", "{t:?}"); + let r = s + .call("command", json!({ "command": "echo ready" })) + .await + .expect("command"); + assert_eq!(r.is_error, Some(false), "{t:?}"); + + // A target Rizin cannot open fails the same way on both backends, + // with Rizin's reason. + for target in ["nosuchscheme://x", "file:///nonexistent", "malloc://0"] { + let err = s + .call("open_file", json!({ "path": target, "analysis": "none" })) + .await + .expect_err(target); + assert_eq!(err.code.0, -32602, "{t:?}: {err:?}"); + assert!( + err.message + .starts_with(&format!("rizin cannot open '{target}'")), + "{t:?}: {}", + err.message + ); + assert!( + err.message.contains("Cannot open"), + "{t:?}: {}", + err.message + ); + } + let err = s + .call("open_file", json!({ "path": "/", "analysis": "none" })) + .await + .expect_err("a directory"); + assert!(err.message.contains("directory"), "{t:?}: {}", err.message); + } +}