Skip to content

Commit

Permalink
fixup! plugin: deny unwrap call
Browse files Browse the repository at this point in the history
  • Loading branch information
vincenzopalazzo committed Mar 24, 2024
1 parent 12505dc commit 9070a58
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 8 deletions.
11 changes: 10 additions & 1 deletion plugin/src/commands/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,30 @@ impl<T: Clone> RPCCommand<T> for ManifestRPC {
#[derive(Clone)]
/// Type to define the init method and its attributes, used in plugin
pub struct InitRPC<T: 'static + Clone> {
#[allow(clippy::type_complexity)]
pub(crate) on_init: Option<Arc<dyn Fn(&mut Plugin<T>) -> Value>>,
}

impl<T: Clone> InitRPC<T> {
fn parse_option(&self, plugin: &mut Plugin<T>, options: &HashMap<String, serde_json::Value>) {
for option_name in options.keys() {
// SAFETY: We are iterating over the key this never None
#[allow(clippy::unwrap_used)]
let option = options.get(option_name).unwrap();
plugin.option.get_mut(option_name).unwrap().value = Some(option.to_owned());
// SAFETY: we put them into it so it is safe to unwrap.
// If we panic this mean that there is a bug
#[allow(clippy::unwrap_used)]
let opt = plugin.option.get_mut(option_name).unwrap();
opt.value = Some(option.to_owned());
}
}
}

impl<T: Clone> RPCCommand<T> for InitRPC<T> {
fn call<'c>(&self, plugin: &mut Plugin<T>, request: Value) -> Result<Value, PluginError> {
let mut response = init_payload();
// SAFETY: Shouwl be valid json so should be safe to unwrap
#[allow(clippy::unwrap_used)]
let init: InitConf = serde_json::from_value(request.to_owned()).unwrap();
plugin.configuration = Some(init.configuration);
self.parse_option(plugin, &init.options);
Expand Down
2 changes: 1 addition & 1 deletion plugin/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::errors::PluginError;
/// in contrast, it is more complex but the plugin_macros package will help to simplify the API.
pub trait RPCCommand<T: Clone>: RPCCommandClone<T> {
/// call is a generic method that it is used to simulate the callback.
fn call<'c>(
fn call(
&self,
_: &mut Plugin<T>,
_: serde_json::Value,
Expand Down
1 change: 1 addition & 0 deletions plugin/src/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ impl AsyncIO {
Ok(())
}

#[allow(clippy::wrong_self_convention)]
pub fn into_loop<F>(&mut self, mut async_callback: F) -> io::Result<()>
where
F: FnMut(String) -> Option<String>,
Expand Down
15 changes: 15 additions & 0 deletions plugin/src/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ impl<'a, T: 'a + Clone> Plugin<T> {
params: payload,
};
crate::poll_loop!({
// SAFETY: it is valid json and if we panic there is a buf somewhere
#[allow(clippy::unwrap_used)]
writer.write_all(serde_json::to_string(&request).unwrap().as_bytes())
});
crate::poll_loop!({ writer.flush() });
Expand All @@ -142,8 +144,12 @@ impl<'a, T: 'a + Clone> Plugin<T> {
) -> &mut Self {
let def_val = match opt_type {
"flag" | "bool" => {
// FIXME: remove unwrap and return the error
#[allow(clippy::unwrap_used)]
def_val.map(|val| serde_json::json!(val.parse::<bool>().unwrap()))
}
// FIXME: remove unwrap and return the error
#[allow(clippy::unwrap_used)]
"int" => def_val.map(|val| serde_json::json!(val.parse::<i64>().unwrap())),
"string" => def_val.map(|val| serde_json::json!(val)),
_ => unreachable!("{opt_type} not supported"),
Expand Down Expand Up @@ -212,6 +218,9 @@ impl<'a, T: 'a + Clone> Plugin<T> {
}

fn handle_notification(&'a mut self, name: &str, params: serde_json::Value) {
// SAFETY: we register the notification and if we do not have inside the map
// this is a bug.
#[allow(clippy::unwrap_used)]
let notification = self.rpc_notification.get(name).unwrap().clone();
notification.call_void(self, &params);
}
Expand Down Expand Up @@ -253,6 +262,8 @@ impl<'a, T: 'a + Clone> Plugin<T> {
match result {
Ok(json_resp) => response["result"] = json_resp.to_owned(),
Err(json_err) => {
// SAFETY: should be valud json
#[allow(clippy::unwrap_used)]
let err_resp = serde_json::to_value(json_err).unwrap();
response["error"] = err_resp;
}
Expand Down Expand Up @@ -281,6 +292,8 @@ impl<'a, T: 'a + Clone> Plugin<T> {
asyncio.into_loop(|buffer| {
#[cfg(feature = "log")]
log::info!("looping around the string: {buffer}");
// SAFETY: should be valud json
#[allow(clippy::unwrap_used)]
let request: Request<serde_json::Value> = serde_json::from_str(&buffer).unwrap();
if let Some(id) = request.id {
// when the id is specified this is a RPC or Hook, so we need to return a response
Expand All @@ -293,6 +306,8 @@ impl<'a, T: 'a + Clone> Plugin<T> {
request.method,
rpc_response
);
// SAFETY: should be valud json
#[allow(clippy::unwrap_used)]
Some(serde_json::to_string(&rpc_response).unwrap())
} else {
// in case of the id is None, we are receiving the notification, so the server is not
Expand Down
5 changes: 1 addition & 4 deletions plugin_macros/src/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ use kproc_parser::kproc_macros::KTokenStream;
use kproc_parser::proc_macro::{TokenStream, TokenTree};
use kproc_parser::{build_error, check, trace};

#[derive(Debug)]
#[derive(Default)]
#[derive(Debug, Default)]
pub struct PluginDeclaration {
pub state: Option<String>,
pub dynamic: Option<TokenTree>,
Expand Down Expand Up @@ -59,8 +58,6 @@ impl std::fmt::Display for PluginDeclaration {
}
}



/// proc macro syntax is something like this
///
/// ```ignore
Expand Down
4 changes: 2 additions & 2 deletions tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ pub mod fixtures {
pub fn lightningd() -> cln::Node {
init();
let pwd = std::env::var("PWD").unwrap();

async_run!(cln::Node::with_params(&format!("--developer --plugin={pwd}/target/debug/examples/foo_plugin --plugin={pwd}/target/debug/examples/macros_ex"), "regtest")).unwrap()
}

#[fixture]
pub fn lightningd_second() -> cln::Node {
init();

async_run!(cln::Node::with_params("--developer", "regtest")).unwrap()
}
}

0 comments on commit 9070a58

Please sign in to comment.