Merge pull request #92 from freifunk-saar/misc

some cleanup and clippy

.github/workflows/ci.yml

@@ -3,15 +3,25 @@ name: CI
 on:
   pull_request:
   merge_group:
-  schedule:
-    - cron: '42 2 * * SAT' # At 2:42 UTC every Saturday.
 
 jobs:
   build:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
       - uses: dtolnay/rust-toolchain@stable
-
       - name: Build
-        run: RUSTFLAGS="-D warnings" cargo build --locked
+        run: cargo build --locked
+
+  check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: 1.75.0
+          components: rustfmt, clippy
+      - name: rustfmt
+        run: cargo fmt --check
+      - name: clippy
+        run: cargo clippy -- -D warnings

Cargo.toml

@@ -26,5 +26,5 @@ hex = "0.4.3"
 reqwest = { version = "0.11", features = ["json"] }
 chrono = { version = "0.4.2", features = ["serde"] }
 mail = { version = "0.7", features = ["smtp"] }
-futures = "0.1" # cannot use newer version due to mail
+futures = { version = "0.3",  features = ["compat"] }
 uuid = { version = "1.0", features = ["v4"] }

src/action.rs

@@ -18,7 +18,6 @@ use rocket::form::FromFormField;
 use rocket::FromForm;
 
 use anyhow::{bail, Result};
-use diesel;
 use diesel::prelude::*;
 use diesel::result::{DatabaseErrorKind, Error as DieselError};
 use ring::{error, hmac};
@@ -53,7 +52,7 @@ pub struct SignedAction {
 impl Action {
     fn compute_signature(&self, key: &hmac::Key) -> hmac::Tag {
         let buf = serialize_to_vec(self).expect("failed to encode Action");
-        hmac::sign(&key, buf.as_slice())
+        hmac::sign(key, buf.as_slice())
     }
 
     fn verify_signature(
@@ -62,7 +61,7 @@ impl Action {
         signature: &[u8],
     ) -> Result<(), error::Unspecified> {
         let buf = serialize_to_vec(self).expect("failed to encode Action");
-        hmac::verify(&key, buf.as_slice(), signature)
+        hmac::verify(key, buf.as_slice(), signature)
     }
 
     pub fn sign(self, key: &hmac::Key) -> SignedAction {
@@ -109,7 +108,7 @@ impl Action {
 impl SignedAction {
     pub fn verify(self, key: &hmac::Key) -> Result<Action, error::Unspecified> {
         // Using a match to make it really clear we don't return the action in case of failure
-        match self.action.verify_signature(key, &*self.signature) {
+        match self.action.verify_signature(key, &self.signature) {
             Ok(_) => Ok(self.action),
             Err(e) => Err(e),
         }

src/config.rs

@@ -63,10 +63,7 @@ pub struct Secrets {
 impl Secrets {
     /// Getters for default values
     pub fn get_smtp_host(&self) -> &str {
-        self.smtp_host
-            .as_ref()
-            .map(String::as_str)
-            .unwrap_or("localhost")
+        self.smtp_host.as_deref().unwrap_or("localhost")
     }
 }
 
@@ -115,7 +112,7 @@ impl Config {
     }
 }
 
-/// A request guard that makes the config available to all templates
+/// A request guard that makes the config available to all templates.
 pub struct Renderer<'a>(&'a Config);
 
 #[rocket::async_trait]

src/main.rs

@@ -14,13 +14,13 @@
 //  You should have received a copy of the GNU Affero General Public License
 //  along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
-mod util;
 mod action;
 mod config;
 mod cron;
 mod models;
 mod routes;
 mod schema;
+mod util;
 
 use rocket::launch;
 

src/models.rs

@@ -14,8 +14,8 @@
 //  You should have received a copy of the GNU Affero General Public License
 //  along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
-use serde::Serialize;
 use diesel::prelude::*;
+use serde::Serialize;
 
 use crate::schema::*;
 

src/routes.rs

@@ -17,8 +17,8 @@
 use std::collections::HashSet;
 
 use base64::Engine as _;
-use rocket::{form::Form, response::Debug, State};
-use rocket::{get, post, routes, uri};
+use rocket::{form::Form, response, State};
+use rocket::{get, post, routes, uri, Request};
 use rocket_dyn_templates::Template;
 
 use diesel::prelude::*;
@@ -33,11 +33,29 @@ use crate::models::*;
 use crate::util::{EmailAddress, EmailSender};
 use crate::DbConn;
 
-type Result<T> = std::result::Result<T, Debug<anyhow::Error>>;
-
 const BASE64_ENGINE: base64::engine::GeneralPurpose =
     base64::engine::general_purpose::URL_SAFE_NO_PAD;
 
+/// Custom error type to allow using `?` below.
+struct Error(anyhow::Error);
+
+impl<'r> response::Responder<'r, 'static> for Error {
+    fn respond_to(self, r: &'r Request<'_>) -> response::Result<'static> {
+        response::Debug(self.0).respond_to(r)
+    }
+}
+
+impl<T> From<T> for Error
+where
+    anyhow::Error: From<T>,
+{
+    fn from(value: T) -> Self {
+        Self(value.into())
+    }
+}
+
+type Result<T> = std::result::Result<T, Error>;
+
 #[get("/")]
 fn index(renderer: Renderer) -> Result<Template> {
     Ok(renderer.render("index", json!({}))?)
@@ -98,7 +116,7 @@ async fn prepare_action(
 
     // obtain bytes for signed action payload
     let signed_action = action.clone().sign(&config.secrets.action_signing_key);
-    let signed_action = serialize_to_vec(&signed_action).map_err(|e| Debug(e.into()))?;
+    let signed_action = serialize_to_vec(&signed_action)?;
     let signed_action = BASE64_ENGINE.encode(&signed_action);
 
     // compute some URLs
@@ -116,8 +134,7 @@ async fn prepare_action(
                 .first::<NodeQuery>(db)
                 .optional()
         })
-        .await
-        .map_err(|e| Debug(e.into()))?;
+        .await?;
     let node_name = match node {
         Some(node) => node.name,
         None if action.op == Operation::Remove =>
@@ -171,11 +188,8 @@ async fn run_action(
 ) -> Result<Template> {
     // Determine and verify action
     let action: Result<Action> = (|| {
-        let signed_action = BASE64_ENGINE
-            .decode(signed_action.as_str())
-            .map_err(|e| Debug(e.into()))?;
-        let signed_action: SignedAction =
-            deserialize_from_slice(signed_action.as_slice()).map_err(|e| Debug(e.into()))?;
+        let signed_action = BASE64_ENGINE.decode(signed_action.as_str())?;
+        let signed_action: SignedAction = deserialize_from_slice(signed_action.as_slice())?;
         Ok(signed_action
             .verify(&config.secrets.action_signing_key)
             .map_err(|_| anyhow::anyhow!("signature verification failed"))?)
@@ -207,17 +221,15 @@ async fn cron_route(
     renderer: Renderer<'_>,
     email_sender: EmailSender<'_>,
 ) -> Result<Template> {
-    Ok(
-        match cron::update_nodes(&db, &*config, email_sender).await? {
-            cron::UpdateResult::NotEnoughOnline(online) => renderer.render(
-                "cron_error",
-                json!({
-                    "not_enough_online": online,
-                }),
-            ),
-            cron::UpdateResult::AllOk => renderer.render("cron", json!({})),
-        }?,
-    )
+    Ok(match cron::update_nodes(&db, config, email_sender).await? {
+        cron::UpdateResult::NotEnoughOnline(online) => renderer.render(
+            "cron_error",
+            json!({
+                "not_enough_online": online,
+            }),
+        ),
+        cron::UpdateResult::AllOk => renderer.render("cron", json!({})),
+    }?)
 }
 
 pub fn routes() -> Vec<rocket::Route> {

src/schema.rs

@@ -15,7 +15,4 @@ diesel::table! {
     }
 }
 
-diesel::allow_tables_to_appear_in_same_query!(
-    monitors,
-    nodes,
-);
+diesel::allow_tables_to_appear_in_same_query!(monitors, nodes,);

src/util.rs

@@ -23,8 +23,8 @@ use rocket::{
 };
 use rocket_dyn_templates::Template;
 
-use anyhow::{bail, Result};
-use futures::Future;
+use anyhow::Result;
+use futures::compat::Future01CompatExt;
 use mail::{default_impl::simple_context, headers, smtp, Email, HeaderTryFrom, Mail};
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
@@ -51,16 +51,16 @@ pub mod hex_signing_key {
 pub struct EmailAddress(String);
 
 impl EmailAddress {
-    pub fn new(s: String) -> Result<EmailAddress> {
+    pub fn new<'e>(s: String) -> form::Result<'e, EmailAddress> {
         let email_parts: Vec<&str> = s.split('@').collect();
         if email_parts.len() != 2 {
-            bail!("Too many or two few @");
+            return Err(rocket::form::Error::validation("invalid credit card number").into());
         }
         if email_parts[0].is_empty() {
-            bail!("User part is empty");
+            return Err(rocket::form::Error::validation("User part is empty").into());
         }
         if email_parts[1].find('.').is_none() {
-            bail!("Domain part must contain .");
+            return Err(rocket::form::Error::validation("Domain part must contain .").into());
         }
         Ok(EmailAddress(s))
     }
@@ -69,7 +69,8 @@ impl EmailAddress {
 #[rocket::async_trait]
 impl<'r> FromFormField<'r> for EmailAddress {
     fn from_value(field: form::ValueField<'r>) -> form::Result<'r, Self> {
-        Ok(EmailAddress(String::from_value(field)?))
+        // `new` does address validation
+        EmailAddress::new(String::from_value(field)?)
     }
 }
 
@@ -132,7 +133,6 @@ impl<'r> EmailSender<'r> {
             self.config.template_vals(vals)?,
         )
         .unwrap();
-        //let email_text = self.responder_body(email_template).await?;
         let email_parts: Vec<&str> = email_text.splitn(3, '\n').collect();
         let (email_from, email_subject, email_body) =
             (email_parts[0], email_parts[1], email_parts[2]);
@@ -160,7 +160,8 @@ impl<'r> EmailSender<'r> {
             smtp::ConnectionConfig::builder_with_port(smtp_host.parse()?, 25)?.build()
         };
         Ok(smtp::send(mail.into(), config, self.mail_ctx.clone())
-            .wait()
+            .compat()
+            .await
             .map_err(EmailError::MailSend)?)
     }
 }