mirror of
https://github.com/rizinorg/rizin-mcp.git
synced 2026-11-07 06:46:00 -10:00
Open an empty session for "-" on both backends
* The in-process backend passed `-` to `rz_core_file_open_load()`, which
looked for a file named `-` and failed with "file not found: -". The
pipe backend ran `rizin -q0 -`, which reads a file from stdin, i.e.
from the command pipe, so it never became ready and the open failed at
the startup timeout.
* Treat `-` as "no target": create the core without opening a file, or
run `rizin -q0` without a target.
* When Rizin cannot open a target, both backends now return
`OpenFailed` ("rizin cannot open '<target>'" and Rizin's reason, logged
in process or read from stderr before rizin exits), instead of "file
not found" in process and a handshake error through the pipe.
* Refuse directories up front: the CLI does, but the library opens one as
an endless file.
This commit is contained in:
1 parent
b432844fa9
commit
686ee318fc
8 files changed
+135
-43
No files matched your search
@@ -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)
|
||||
|
||||
|
||||
@@ -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),
|
||||
}
|
||||
|
||||
+16
-4
@@ -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<bool, RizinError> {
|
||||
/// # 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"),
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+1
-1
@@ -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)]
|
||||
|
||||
+29
-12
@@ -69,15 +69,17 @@ impl RizinPipe {
|
||||
/// Spawn `rizin -q0 [extra_args...] <target>` 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::<Job>();
|
||||
let (ready_tx, ready_rx) = oneshot::channel::<Result<(), RizinError>>();
|
||||
|
||||
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<String>,
|
||||
ready_tx: oneshot::Sender<Result<(), RizinError>>,
|
||||
mut rx: mpsc::UnboundedReceiver<Job>,
|
||||
) {
|
||||
@@ -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) => {
|
||||
|
||||
+13
-18
@@ -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::<Job>();
|
||||
let (ready_tx, ready_rx) = oneshot::channel::<Result<(), RizinError>>();
|
||||
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<String>,
|
||||
write: bool,
|
||||
ready_tx: oneshot::Sender<Result<(), RizinError>>,
|
||||
mut rx: mpsc::UnboundedReceiver<Job>,
|
||||
@@ -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),
|
||||
|
||||
+21
-7
@@ -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<Backend, RizinError> {
|
||||
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,
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user