diff --git a/crates/radicle-remote-helper/src/fetch.rs b/crates/radicle-remote-helper/src/fetch.rs index e0caab57..4c6221ef 100644 --- a/crates/radicle-remote-helper/src/fetch.rs +++ b/crates/radicle-remote-helper/src/fetch.rs @@ -6,13 +6,13 @@ use thiserror::Error; use radicle::git; use crate::service::GitService; -use crate::{read_line, Verbosity}; +use crate::Verbosity; #[derive(Debug, Error)] pub enum Error { - /// Invalid command received. - #[error("invalid command `{0}`")] - InvalidCommand(String), + /// Protocol error. + #[error("protocol error: {0}")] + Protocol(#[from] crate::protocol::Error), /// I/O error. #[error("i/o error: {0}")] Io(#[from] io::Error), @@ -30,31 +30,33 @@ pub enum Error { stderr: String, stdout: String, }, + + /// Received an unexpected command after the first `fetch` command. + #[error("unexpected command after first `fetch`: {0:?}")] + UnexpectedCommand(crate::protocol::Command), } /// Run a git fetch command. pub(super) fn run( mut refs: Vec<(git::Oid, git::fmt::RefString)>, - stored: radicle::storage::git::Repository, + stored: &radicle::storage::git::Repository, git: &G, - stdin: &io::Stdin, + command_reader: &mut crate::protocol::LineReader, verbosity: Verbosity, ) -> Result<(), Error> { // Read all the `fetch` lines. - let mut line = String::new(); - loop { - let tokens = read_line(stdin, &mut line)?; - match tokens.as_slice() { - ["fetch", oid, refstr] => { - let oid = git::Oid::from_str(oid)?; - let refstr = git::fmt::RefString::try_from(*refstr)?; - + for line in command_reader.by_ref() { + match line?? { + crate::protocol::Line::Valid(crate::protocol::Command::Fetch { oid, refstr }) => { + let oid = git::Oid::from_str(&oid)?; + let refstr = git::fmt::RefString::try_from(refstr)?; refs.push((oid, refstr)); } - // An empty line means end of input. - [] => break, - // Once the first `fetch` command is received, we don't expect anything else. - _ => return Err(Error::InvalidCommand(line.trim().to_owned())), + crate::protocol::Line::Blank => { + // An empty line means end of input. + break; + } + crate::protocol::Line::Valid(command) => return Err(Error::UnexpectedCommand(command)), } } @@ -73,7 +75,7 @@ pub(super) fn run( // used in the working copy, this will always result in the object // missing. This seems to only be an issue with `libgit2`/`git2` // and not `git` itself. - let output = git.fetch_pack(working, &stored, oids, verbosity.into())?; + let output = git.fetch_pack(working, stored, oids, verbosity.into())?; if !output.status.success() { return Err(Error::FetchPackFailed { diff --git a/crates/radicle-remote-helper/src/main.rs b/crates/radicle-remote-helper/src/main.rs index c45f4393..43dc4848 100644 --- a/crates/radicle-remote-helper/src/main.rs +++ b/crates/radicle-remote-helper/src/main.rs @@ -18,13 +18,15 @@ mod fetch; mod list; +mod protocol; mod push; mod service; +use std::io::{self, BufRead, Write}; use std::path::PathBuf; use std::process; use std::str::FromStr; -use std::{env, fmt, io}; +use std::{env, fmt}; use thiserror::Error; @@ -36,6 +38,8 @@ use radicle::{cob, profile}; use radicle::{git, storage, Profile}; use radicle_cli::terminal as cli; +use crate::protocol::{Command, Line, LineReader}; + pub const VERSION: Version = Version { name: env!("CARGO_BIN_NAME"), commit: env!("GIT_HEAD"), @@ -85,9 +89,6 @@ pub enum Error { /// Remote repository not found (or empty). #[error("remote repository `{0}` not found")] RepositoryNotFound(PathBuf), - /// Invalid command received. - #[error("invalid command `{0}`")] - InvalidCommand(String), /// Invalid arguments received. #[error("invalid arguments: {0:?}")] InvalidArguments(Vec), @@ -121,6 +122,9 @@ pub enum Error { /// Invalid object ID. #[error("invalid oid: {0}")] InvalidOid(#[from] radicle::git::ParseOidError), + /// Protocol error. + #[error(transparent)] + Protocol(#[from] protocol::Error), } /// Models values for the `verbosity` option, see @@ -239,9 +243,7 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { let debug = radicle::profile::env::debug(); let stdin = io::stdin(); - let mut line = String::new(); - let mut opts = Options::default(); - let mut expected_refs = Vec::new(); + let stdout = io::stdout(); let git = service::RealGitService; let mut node = service::RealNodeSession::new(&profile); @@ -251,98 +253,152 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { } } - loop { - let tokens = read_line(&stdin, &mut line)?; + run_loop( + stdin.lock(), + stdout.lock(), + &git, + &mut node, + &stored, + &profile, + remote, + url, + ) +} + +#[allow(clippy::too_many_arguments)] +fn run_loop( + mut input: R, + mut output: W, + git: &G, + node: &mut N, + stored: &storage::git::Repository, + profile: &Profile, + remote: Option, + url: Url, +) -> Result<(), Error> { + let mut opts = Options::default(); + let mut expected_refs = Vec::new(); + let debug = radicle::profile::env::debug(); + + let mut command_reader = LineReader::new(&mut input); + + while let Some(line) = command_reader.next() { + let line = line??; if debug { - eprintln!("{}: {}", VERSION.name, &tokens.join(" ")); + eprintln!("{}: {:?}", VERSION.name, line); } - match tokens.as_slice() { - ["capabilities"] => { - println!("option"); - println!("push"); // Implies `list` command. - println!("fetch"); - println!(); + match line { + Line::Valid(Command::Capabilities) => { + writeln!(output, "option")?; + writeln!(output, "push")?; // Implies `list` command. + writeln!(output, "fetch")?; + writeln!(output)?; } - ["option", "verbosity", verbosity] => match verbosity.parse::() { - Ok(verbosity) => { - opts.verbosity = verbosity; - println!("ok"); + Line::Valid(Command::Option { key, value }) => match key.as_str() { + "verbosity" => { + if let Some(val) = value { + match val.parse::() { + Ok(verbosity) => { + opts.verbosity = verbosity; + writeln!(output, "ok")?; + } + Err(err) => { + writeln!(output, "error {err}")?; + } + } + } else { + writeln!(output, "error missing value for verbosity")?; + } } - Err(err) => { - println!("error {err}"); + "push-option" => { + if let Some(val) = value { + let args = val.split(' ').collect::>(); + // Nb. Git documentation says that we can print `error ` or `unsupported` + // for options that are not supported, but this results in Git saying that + // "push-option" itself is an unsupported option, which is not helpful or correct. + // Hence, we just exit with an error in this case. + push_option(&args, &mut opts)?; + writeln!(output, "ok")?; + } else { + writeln!(output, "error missing value for push-option")?; + } + } + "cas" => { + if let Some(val) = value { + expected_refs.push(val); + writeln!(output, "ok")?; + } else { + writeln!(output, "error missing value for cas")?; + } + } + "progress" => { + writeln!(output, "unsupported")?; + } + _ => { + writeln!(output, "unsupported")?; } }, - ["option", "push-option", args @ ..] => { - // Nb. Git documentation says that we can print `error ` or `unsupported` - // for options that are not supported, but this results in Git saying that - // "push-option" itself is an unsupported option, which is not helpful or correct. - // Hence, we just exit with an error in this case. - push_option(args, &mut opts)?; - println!("ok"); - } - ["option", "cas", refstr] => { - expected_refs.push((*refstr).to_owned()); - println!("ok"); - } - ["option", "progress", ..] | ["option", ..] => { - println!("unsupported"); - } - ["fetch", oid, refstr] => { - let oid = git::Oid::from_str(oid)?; - let refstr = git::fmt::RefString::try_from(*refstr)?; + Line::Valid(Command::Fetch { oid, refstr }) => { + let oid = git::Oid::from_str(&oid)?; + let refstr = git::fmt::RefString::try_from(refstr.as_str())?; - fetch::run(vec![(oid, refstr)], stored, &git, &stdin, opts.verbosity)?; - - // Nb. An empty line means we're done - println!(); - - return Ok(()); - } - ["push", refspec] => { - let output = push::run( - vec![refspec.to_string()], - remote, - url, - &stored, - &profile, - &stdin, - opts, - &expected_refs, - &git, - &mut node, + fetch::run( + vec![(oid, refstr)], + stored, + git, + &mut command_reader, + opts.verbosity, )?; - for line in output { - println!("{line}"); - } - println!(); + // Nb. An empty line means we're done + writeln!(output)?; return Ok(()); } - ["list"] => { - let refs = list::for_fetch(&url, &profile, &stored)?; - for line in refs { - println!("{line}"); + Line::Valid(Command::Push(refspec)) => { + let result = push::run( + vec![refspec], + remote.clone(), + url.clone(), + stored, + profile, + &mut command_reader, + opts.clone(), + &expected_refs, + git, + node, + )?; + + for line in result { + writeln!(output, "{line}")?; } - println!(); - } - ["list", "for-push"] => { - let refs = list::for_push(&profile, &stored)?; - for line in refs { - println!("{line}"); - } - println!(); - } - [] => { + writeln!(output)?; + return Ok(()); } - _ => { - return Err(Error::InvalidCommand(line.trim().to_owned())); + Line::Valid(Command::List) => { + let refs = list::for_fetch(&url, profile, stored)?; + for line in refs { + writeln!(output, "{line}")?; + } + writeln!(output)?; + } + Line::Valid(Command::ListForPush) => { + let refs = list::for_push(profile, stored)?; + for line in refs { + writeln!(output, "{line}")?; + } + writeln!(output)?; + } + Line::Blank => { + break; } } } + + Ok(()) } /// Parse a single push option. Returns `Ok` if it was successful. @@ -394,20 +450,6 @@ fn push_option(args: &[&str], opts: &mut Options) -> Result<(), Error> { Ok(()) } -/// Read one line from stdin, and split it into tokens. -pub(crate) fn read_line<'a>(stdin: &io::Stdin, line: &'a mut String) -> io::Result> { - line.clear(); - - let read = stdin.read_line(line)?; - if read == 0 { - return Ok(vec![]); - } - let line = line.trim(); - let tokens = line.split(' ').filter(|t| !t.is_empty()).collect(); - - Ok(tokens) -} - /// Write a hint to the user. pub(crate) fn hint(s: impl fmt::Display) { eprintln!("{}", cli::format::hint(format!("hint: {s}"))); diff --git a/crates/radicle-remote-helper/src/protocol.rs b/crates/radicle-remote-helper/src/protocol.rs new file mode 100644 index 00000000..26d648d2 --- /dev/null +++ b/crates/radicle-remote-helper/src/protocol.rs @@ -0,0 +1,230 @@ +use thiserror::Error; + +#[derive(Debug, Error)] +pub(super) enum Error { + #[error("invalid command `{0}`")] + InvalidCommand(String), +} + +#[derive(Debug, PartialEq, Eq)] +pub(super) enum Command { + Capabilities, + List, + ListForPush, + Fetch { oid: String, refstr: String }, + Push(String), + Option { key: String, value: Option }, +} + +#[derive(Debug, PartialEq, Eq)] +pub(super) enum Line { + Valid(Command), + Blank, +} + +impl Command { + pub(super) fn parse_line(line: &str) -> Result { + let line = line.trim(); + if line.is_empty() { + return Ok(Line::Blank); + } + + // Split the command verb from the rest of the line. + let (cmd, args) = line.split_once(' ').unwrap_or((line, "")); + let args = args.trim(); + + match cmd { + "capabilities" => Ok(Line::Valid(Command::Capabilities)), + "list" => { + if args == "for-push" { + Ok(Line::Valid(Command::ListForPush)) + } else if args.is_empty() { + Ok(Line::Valid(Command::List)) + } else { + Err(Error::InvalidCommand(line.to_owned())) + } + } + "fetch" => { + // fetch + // Use split_whitespace to handle multiple spaces between OID and Ref, + // which is permitted. + let mut parts = args.split_whitespace(); + let oid = parts + .next() + .ok_or_else(|| Error::InvalidCommand(line.to_owned()))?; + let refstr = parts + .next() + .ok_or_else(|| Error::InvalidCommand(line.to_owned()))?; + Ok(Line::Valid(Command::Fetch { + oid: oid.to_owned(), + refstr: refstr.to_owned(), + })) + } + "push" => Ok(Line::Valid(Command::Push(args.to_owned()))), + "option" => { + // option [value] + // Use split_once to preserve whitespace in the value. + let (key, val) = args.split_once(' ').unwrap_or((args, "")); + let value = if val.is_empty() { + None + } else { + Some(val.to_owned()) + }; + Ok(Line::Valid(Command::Option { + key: key.to_owned(), + value, + })) + } + _ => Err(Error::InvalidCommand(line.to_owned())), + } + } +} + +mod io { + use std::io::{self, prelude::*}; + + use super::*; + + pub(crate) struct LineReader { + inner: io::BufReader, + } + + impl LineReader { + pub(crate) fn new(reader: R) -> Self { + Self { + inner: io::BufReader::new(reader), + } + } + + pub(crate) fn read_line(&mut self) -> io::Result> { + let mut line = String::new(); + if self.inner.read_line(&mut line)? == 0 { + // EOF reached + return Ok(Ok(Line::Blank)); + } + Ok(Command::parse_line(&line)) + } + } + + impl Iterator for LineReader { + type Item = io::Result>; + + fn next(&mut self) -> Option { + match self.read_line() { + Ok(line) => Some(Ok(line)), + Err(e) => Some(Err(e)), + } + } + } +} + +pub(crate) use io::*; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_capabilities() { + assert_eq!( + Command::parse_line("capabilities").unwrap(), + Line::Valid(Command::Capabilities) + ); + } + + #[test] + fn test_list() { + assert_eq!( + Command::parse_line("list").unwrap(), + Line::Valid(Command::List) + ); + } + + #[test] + fn test_list_for_push() { + assert_eq!( + Command::parse_line("list for-push").unwrap(), + Line::Valid(Command::ListForPush) + ); + } + + #[test] + fn test_fetch() { + assert_eq!( + Command::parse_line("fetch oid ref").unwrap(), + Line::Valid(Command::Fetch { + oid: "oid".to_owned(), + refstr: "ref".to_owned() + }) + ); + } + + #[test] + fn test_fetch_whitespace() { + assert_eq!( + Command::parse_line("fetch oid ref").unwrap(), + Line::Valid(Command::Fetch { + oid: "oid".to_owned(), + refstr: "ref".to_owned() + }) + ); + } + + #[test] + fn test_push() { + assert_eq!( + Command::parse_line("push src:dst").unwrap(), + Line::Valid(Command::Push("src:dst".to_owned())) + ); + } + + #[test] + fn test_push_force() { + assert_eq!( + Command::parse_line("push +src:dst").unwrap(), + Line::Valid(Command::Push("+src:dst".to_owned())) + ); + } + + #[test] + fn test_push_delete() { + assert_eq!( + Command::parse_line("push :dst").unwrap(), + Line::Valid(Command::Push(":dst".to_owned())) + ); + } + + #[test] + fn test_option() { + assert_eq!( + Command::parse_line("option verbosity 2").unwrap(), + Line::Valid(Command::Option { + key: "verbosity".to_owned(), + value: Some("2".to_owned()) + }) + ); + } + + #[test] + fn test_option_whitespace_preservation() { + assert_eq!( + Command::parse_line("option patch.message Fix: whitespace").unwrap(), + Line::Valid(Command::Option { + key: "patch.message".to_owned(), + value: Some("Fix: whitespace".to_owned()) + }) + ); + } + + #[test] + fn test_empty() { + assert_eq!(Command::parse_line("").unwrap(), Line::Blank); + assert_eq!(Command::parse_line(" ").unwrap(), Line::Blank); + } + + #[test] + fn test_invalid() { + assert!(Command::parse_line("invalid command").is_err()); + assert!(Command::parse_line("list invalid").is_err()); + } +} diff --git a/crates/radicle-remote-helper/src/push.rs b/crates/radicle-remote-helper/src/push.rs index e35daf6b..f1bb5041 100644 --- a/crates/radicle-remote-helper/src/push.rs +++ b/crates/radicle-remote-helper/src/push.rs @@ -31,7 +31,7 @@ use radicle_cli::terminal as term; use crate::service::GitService; use crate::service::NodeSession; -use crate::{hint, read_line, warn, Options, Verbosity}; +use crate::{hint, warn, Options, Verbosity}; #[derive(Debug, Error)] pub enum Error { @@ -50,9 +50,9 @@ pub enum Error { /// Identity payload error. #[error("payload: {0}")] Payload(#[from] radicle::identity::doc::PayloadError), - /// Invalid command received. - #[error("invalid command `{0}`")] - InvalidCommand(String), + /// Protocol error. + #[error("protocol error: {0}")] + Protocol(#[from] crate::protocol::Error), /// I/O error. #[error("i/o error: {0}")] Io(#[from] io::Error), @@ -128,6 +128,13 @@ pub enum Error { stderr: String, stdout: String, }, + + /// Received an unexpected command after the first `push` command. + #[error("unexpected command after first `push`: {0:?}")] + UnexpectedCommand(crate::protocol::Command), + + #[error(transparent)] + CommandError(#[from] CommandError), } /// Push command. @@ -139,7 +146,7 @@ enum Command { } #[derive(Debug, thiserror::Error)] -enum CommandError { +pub(super) enum CommandError { #[error("expected refspec of the form `[]:`, got {rev}")] Empty { rev: String }, #[error("failed to parse destination reference ({rev}): {err}")] @@ -249,7 +256,7 @@ pub fn run( url: Url, stored: &storage::git::Repository, profile: &Profile, - stdin: &io::Stdin, + command_reader: &mut crate::protocol::LineReader, opts: Options, expected_refs: &[String], git: &impl GitService, @@ -267,7 +274,6 @@ pub fn run( .ok_or(Error::KeyMismatch(ns.into())) })?; let signer = profile.signer()?; - let mut line = String::new(); let mut ok = HashMap::new(); let hints = opts.hints || profile.hints(); let mut output = Vec::new(); @@ -275,16 +281,16 @@ pub fn run( assert_eq!(signer.public_key(), &nid); // Read all the `push` lines. - loop { - let tokens = read_line(stdin, &mut line)?; - match tokens.as_slice() { - ["push", spec] => { - specs.push(spec.to_string()); + for line in command_reader.by_ref() { + match line?? { + crate::protocol::Line::Blank => { + // An empty line means end of input. + break; } - // An empty line means end of input. - [] => break, - // Once the first `push` command is received, we don't expect anything else. - _ => return Err(Error::InvalidCommand(line.trim().to_owned())), + crate::protocol::Line::Valid(crate::protocol::Command::Push(spec)) => { + specs.push(spec); + } + crate::protocol::Line::Valid(command) => return Err(Error::UnexpectedCommand(command)), } } let delegates = stored.delegates()?; @@ -299,9 +305,7 @@ pub fn run( // For each refspec, push a ref or delete a ref. for spec in specs { - let Ok(cmd) = Command::parse(&spec, &working) else { - return Err(Error::InvalidCommand(format!("push {spec}"))); - }; + let cmd = Command::parse(&spec, &working)?; let result = match &cmd { Command::Delete(dst) => { // Delete refs.