From 5bca26c8f7e91aefb11cbbf02da44e9bb7a937de Mon Sep 17 00:00:00 2001 From: Tim Bruijnzeels Date: Wed, 6 Feb 2019 16:04:31 +0100 Subject: [PATCH] Fixing clippy warnings. Just a few ;) --- src/api/publication.rs | 7 ++-- src/api/publishers.rs | 4 +-- src/api/rrdp_data.rs | 18 ++++------ src/bin/pubc.rs | 2 +- src/bin/pubc_cms.rs | 6 ++-- src/krillc/data.rs | 8 +++-- src/krillc/options.rs | 3 +- src/krilld/auth.rs | 4 +-- src/krilld/config.rs | 8 ++--- src/krilld/endpoints.rs | 27 ++++++++------ src/krilld/http/server.rs | 36 +++++++++---------- src/krilld/http/ssl.rs | 22 ++++++------ src/krilld/krillserver.rs | 41 +++++++++++---------- src/krilld/pubd/mod.rs | 24 ++++++------- src/krilld/pubd/repo.rs | 12 +++---- src/krilld/pubd/rrdpd.rs | 38 ++++++++++---------- src/krilld/pubd/rsyncd.rs | 4 +-- src/pubc/apiclient.rs | 12 +++---- src/pubc/cmsclient.rs | 31 ++++++++-------- src/pubc/mod.rs | 6 ++-- src/remote/builder.rs | 23 ++++++------ src/remote/cmsproxy.rs | 18 +++++----- src/remote/id.rs | 2 +- src/remote/responder.rs | 16 ++++----- src/remote/rfc8181.rs | 27 ++++++-------- src/remote/rfc8183.rs | 14 ++++---- src/remote/sigmsg.rs | 2 +- src/storage/caching_ks.rs | 52 +++++++++++++-------------- src/storage/events.rs | 27 +++++++------- src/storage/keystore.rs | 32 ++++++++--------- src/util/file.rs | 2 +- src/util/httpclient.rs | 4 +-- src/util/softsigner.rs | 22 ++++++------ src/util/test.rs | 6 ++-- src/util/xml.rs | 23 ++++++------ tests/client_api_publish_at_server.rs | 4 +-- tests/client_cms_publish_at_server.rs | 4 +-- tests/manage_publishers.rs | 2 +- 38 files changed, 298 insertions(+), 295 deletions(-) diff --git a/src/api/publication.rs b/src/api/publication.rs index 5a7f7e9b..8836ec01 100644 --- a/src/api/publication.rs +++ b/src/api/publication.rs @@ -56,6 +56,7 @@ impl PublishDelta { //------------ PublishDeltaBuilder ------------------------------------------- +#[derive(Default)] pub struct PublishDeltaBuilder { publishes: Vec, updates: Vec, @@ -64,11 +65,7 @@ pub struct PublishDeltaBuilder { impl PublishDeltaBuilder { pub fn new() -> Self { - PublishDeltaBuilder { - publishes: vec![], - updates: vec![], - withdraws: vec![] - } + Self::default() } pub fn add_publish(&mut self, publish: Publish) { diff --git a/src/api/publishers.rs b/src/api/publishers.rs index e75b39eb..f8caffdd 100644 --- a/src/api/publishers.rs +++ b/src/api/publishers.rs @@ -36,7 +36,7 @@ impl CmsAuthData { impl CmsAuthData { pub fn new(tag: Option, id_cert: IdCert) -> Self { - let tag = tag.unwrap_or("".to_string()); + let tag = tag.unwrap_or_else(String::new); CmsAuthData { tag, id_cert } } } @@ -162,7 +162,7 @@ pub struct PublisherList<'a> { impl<'a> PublisherList<'a> { pub fn from( - publishers: &'a Vec>, + publishers: &'a[Arc], path_publishers: &'a str ) -> PublisherList<'a> { let publishers: Vec = publishers.iter().map(|p| diff --git a/src/api/rrdp_data.rs b/src/api/rrdp_data.rs index 34a9b907..6c7283a9 100644 --- a/src/api/rrdp_data.rs +++ b/src/api/rrdp_data.rs @@ -12,8 +12,8 @@ use crate::util::file::{self, RecursorError}; use crate::util::ext_serde; use util::sha256; -const VERSION: &'static str = "1"; -const NS: &'static str = "http://www.ripe.net/rpki/rrdp"; +const VERSION: &str = "1"; +const NS: &str = "http://www.ripe.net/rpki/rrdp"; #[derive(Clone, Debug, Deserialize, Eq, Serialize, PartialEq)] pub struct PublishedObject { @@ -78,7 +78,7 @@ impl Snapshot { Some(&a), |w| { for uri in self.objects.keys() { - let objects = self.objects.get(uri).unwrap(); + let objects = &self.objects[uri]; for cf in objects { let uri = cf.uri.to_string(); let a = [ ("uri", uri.as_ref()) ]; @@ -401,6 +401,7 @@ impl FileInfo { //------------ NotificationBuilder ------------------------------------------- +#[derive(Default)] pub struct NotificationBuilder { serial: Option, session_id: Option, @@ -410,12 +411,7 @@ pub struct NotificationBuilder { impl NotificationBuilder { pub fn new() -> Self { - NotificationBuilder { - serial: None, - session_id: None, - snapshot: None, - deltas: Vec::new() - } + Self::default() } pub fn with_serial(&mut self, serial: usize) { @@ -457,8 +453,8 @@ impl NotificationBuilder { let mut count = 0; self.deltas.retain(|d| { - count = count + 1; - total_deltas = total_deltas + d.size; + count += 1; + total_deltas += d.size; count <= 2 || total_deltas < size_snapshot }) diff --git a/src/bin/pubc.rs b/src/bin/pubc.rs index c752ce1a..9c6c5f0a 100644 --- a/src/bin/pubc.rs +++ b/src/bin/pubc.rs @@ -16,7 +16,7 @@ fn main() { let format = options.format().clone(); match apiclient::execute(options) { - Ok(res) => res.report(format), + Ok(res) => res.report(&format), Err(e) => { eprintln!("{}", e); ::std::process::exit(1); diff --git a/src/bin/pubc_cms.rs b/src/bin/pubc_cms.rs index 6802a8e3..3ebd99ac 100644 --- a/src/bin/pubc_cms.rs +++ b/src/bin/pubc_cms.rs @@ -28,7 +28,7 @@ fn main() { } }; - let client = match PubClient::new(config.state_dir()) { + let client = match PubClient::build(config.state_dir()) { Ok(client) => client, Err(e) => { eprintln!("{}", e); @@ -63,7 +63,7 @@ fn publisher_request( ) -> Result<(), Error> { let req = client.publisher_request()?; let mut file = file::create_file_with_path(&path)?; - file.write(&req.encode_vec())?; + file.write_all(&req.encode_vec())?; Ok(()) } @@ -74,7 +74,7 @@ fn process_response( let bytes = file::read(path)?; let res = RepositoryResponse::decode(bytes.as_ref())?; res.validate()?; - client.process_repo_response(res)?; + client.process_repo_response(&res)?; Ok(()) } diff --git a/src/krillc/data.rs b/src/krillc/data.rs index c80f84e7..29fc5a16 100644 --- a/src/krillc/data.rs +++ b/src/krillc/data.rs @@ -3,11 +3,13 @@ use rpki::uri; use crate::util::ext_serde; use remote::id::IdCert; +use std::str::FromStr; //------------ ApiResponse --------------------------------------------------- /// This type defines all supported responses for the api #[derive(Clone, Debug, Eq, PartialEq)] +#[allow(clippy::large_enum_variant)] pub enum ApiResponse { Health, PublisherDetails(PublisherDetails), @@ -59,8 +61,10 @@ pub enum ReportFormat { Xml } -impl ReportFormat { - pub fn from_str(s: &str) -> Result { +impl FromStr for ReportFormat { + type Err = ReportError; + + fn from_str(s: &str) -> Result { match s { "none" => Ok(ReportFormat::None), "json" => Ok(ReportFormat::Json), diff --git a/src/krillc/options.rs b/src/krillc/options.rs index 558be9e5..7983f4e7 100644 --- a/src/krillc/options.rs +++ b/src/krillc/options.rs @@ -1,4 +1,5 @@ use std::path::PathBuf; +use std::str::FromStr; use clap::{App, Arg, SubCommand}; use rpki::uri; use uuid::Uuid; @@ -210,7 +211,7 @@ impl Options { } if let Some(m) = m.subcommand_matches("response") { let handle = m.value_of("handle").unwrap(); - let file = m.value_of("out").map(|s| PathBuf::from(s)); + let file = m.value_of("out").map(PathBuf::from); let response = PublishersCommand::RepositoryResponseXml( handle.to_string(), file diff --git a/src/krilld/auth.rs b/src/krilld/auth.rs index c0715469..17de73a8 100644 --- a/src/krilld/auth.rs +++ b/src/krilld/auth.rs @@ -6,8 +6,8 @@ use actix_web::http::HeaderMap; use actix_web::middleware::{Middleware, Started}; use crate::krilld::krillserver::KrillServer; -const ADMIN_API_PATH: &'static str = "/api/"; -const PUBLICATION_API_PATH: &'static str = "/publication/"; +const ADMIN_API_PATH: &str = "/api/"; +const PUBLICATION_API_PATH: &str = "/publication/"; pub struct CheckAuthorisation; diff --git a/src/krilld/config.rs b/src/krilld/config.rs index 59678fae..2f2caece 100644 --- a/src/krilld/config.rs +++ b/src/krilld/config.rs @@ -13,7 +13,7 @@ use toml; use crate::krilld::http::ssl; use crate::util::ext_serde; -const SERVER_NAME: &'static str = "Krill"; +const SERVER_NAME: &str = "Krill"; //------------ ConfigDefaults ------------------------------------------------ @@ -199,7 +199,7 @@ impl Config { let config_file = matches.value_of("config") .unwrap_or("./defaults/krill.conf"); - let c = Self::read_config(config_file.as_ref())?; + let c = Self::read_config(config_file)?; c.init_logging()?; Ok(c) } @@ -212,7 +212,7 @@ impl Config { let c: Config = toml::from_slice(v.as_slice())?; if c.port < 1024 { - return Err(ConfigError::from_str("Port number must be >1024")) + return Err(ConfigError::other("Port number must be >1024")) } Ok(c) @@ -309,7 +309,7 @@ pub enum ConfigError { } impl ConfigError { - pub fn from_str(s: &str) -> ConfigError { + pub fn other(s: &str) -> ConfigError { ConfigError::Other(s.to_string()) } } diff --git a/src/krilld/endpoints.rs b/src/krilld/endpoints.rs index 9e259caf..8bb76b18 100644 --- a/src/krilld/endpoints.rs +++ b/src/krilld/endpoints.rs @@ -32,13 +32,13 @@ fn render_json(object: O) -> HttpResponse { .content_type("application/json") .body(enc) }, - Err(e) => server_error(Error::JsonError(e)) + Err(e) => server_error(&Error::JsonError(e)) } } /// Helper function to render server side errors. Also responsible for /// logging the errors. -fn server_error(error: Error) -> HttpResponse { +fn server_error(error: &Error) -> HttpResponse { error!("{}", error); error.error_response() } @@ -64,7 +64,7 @@ pub fn health(_r: &HttpRequest) -> HttpResponse { /// Returns a json structure with all publishers in it. pub fn publishers(req: &HttpRequest) -> HttpResponse { match ro_server(req).publishers() { - Err(e) => server_error(Error::ServerError(e)), + Err(e) => server_error(&Error::ServerError(e)), Ok(publishers) => { render_json( publishers::PublisherList::from(&publishers, "/api/v1/publishers") @@ -75,6 +75,7 @@ pub fn publishers(req: &HttpRequest) -> HttpResponse { /// Adds a publisher, expects that an RFC8183 section 5.2.3 Publisher /// Request XML is posted. +#[allow(clippy::needless_pass_by_value)] pub fn add_publisher( req: HttpRequest, pbl: publishers::Publisher @@ -82,12 +83,13 @@ pub fn add_publisher( let mut server = rw_server(&req); match server.add_publisher(pbl) { Ok(()) => api_ok(), - Err(e) => server_error(Error::ServerError(e)) + Err(e) => server_error(&Error::ServerError(e)) } } /// Removes a publisher. Should be idempotent! If if did not exist then /// that's just fine. +#[allow(clippy::needless_pass_by_value)] pub fn remove_publisher( req: HttpRequest, handle: PublisherHandle @@ -96,11 +98,12 @@ pub fn remove_publisher( Ok(()) => api_ok(), Err(krillserver::Error::PublisherStore( pubd::Error::UnknownPublisher(_))) => api_ok(), - Err(e) => server_error(Error::ServerError(e)) + Err(e) => server_error(&Error::ServerError(e)) } } /// Returns a json structure with publisher details +#[allow(clippy::needless_pass_by_value)] pub fn publisher_details( req: HttpRequest, handle: PublisherHandle @@ -116,12 +119,13 @@ pub fn publisher_details( server.service_base_uri()) ) }, - Err(e) => server_error(Error::ServerError(e)) + Err(e) => server_error(&Error::ServerError(e)) } } /// Shows the server's RFC8183 section 5.2.4 Repository Response XML /// file for a known publisher. +#[allow(clippy::needless_pass_by_value)] pub fn repository_response( req: HttpRequest, handle: PublisherHandle @@ -137,7 +141,7 @@ pub fn repository_response( api_not_found() }, Err(e) => { - server_error(Error::ServerError(e)) + server_error(&Error::ServerError(e)) } } } @@ -146,6 +150,7 @@ pub fn repository_response( //------------ Publication --------------------------------------------------- /// Processes an RFC8181 query and returns the appropriate response. +#[allow(clippy::needless_pass_by_value)] pub fn handle_rfc8181_request( req: HttpRequest, msg: SignedMessage, @@ -159,12 +164,13 @@ pub fn handle_rfc8181_request( .body(captured.into_bytes()) } Err(e) => { - server_error(Error::ServerError(e)) + server_error(&Error::ServerError(e)) } } } /// Processes a publishdelta request sent to the API. +#[allow(clippy::needless_pass_by_value)] pub fn handle_delta( req: HttpRequest, delta: publication::PublishDelta, @@ -172,18 +178,19 @@ pub fn handle_delta( ) -> HttpResponse { match rw_server(&req).handle_delta(delta, handle.as_ref()) { Ok(()) => api_ok(), - Err(e) => server_error(Error::ServerError(e)) + Err(e) => server_error(&Error::ServerError(e)) } } /// Processes a list request sent to the API. +#[allow(clippy::needless_pass_by_value)] pub fn handle_list( req: HttpRequest, handle: PublisherHandle ) -> HttpResponse { match ro_server(&req).handle_list(handle.as_ref()) { Ok(list) => render_json(list), - Err(e) => server_error(Error::ServerError(e)) + Err(e) => server_error(&Error::ServerError(e)) } } diff --git a/src/krilld/http/server.rs b/src/krilld/http/server.rs index c52282a8..7a5fa555 100644 --- a/src/krilld/http/server.rs +++ b/src/krilld/http/server.rs @@ -25,7 +25,7 @@ use crate::krilld::krillserver::KrillServer; use crate::remote::rfc8183; use crate::remote::sigmsg::SignedMessage; -const NOT_FOUND: &'static [u8] = include_bytes!("../../../ui/dev/html/404.html"); +const NOT_FOUND: &[u8] = include_bytes!("../../../ui/dev/html/404.html"); //------------ PubServerApp -------------------------------------------------- @@ -80,16 +80,13 @@ impl PubServerApp { }); use std::env; - match env::var("KRILL_DEV_MODE") { - Ok(_) => { - app = app.handler( - "/ui/dev", - fs::StaticFiles::new("./ui/dev") - .unwrap() - .show_files_listing() - ); - }, - _ => {} + if env::var("KRILL_DEV_MODE").is_ok() { + app = app.handler( + "/ui/dev", + fs::StaticFiles::new("./ui/dev") + .unwrap() + .show_files_listing() + ); } PubServerApp(with_statics(app)) @@ -97,7 +94,7 @@ impl PubServerApp { pub fn create_server(config: &Config) -> Arc> { let authorizer = Authorizer::new(&config.auth_token); - let pub_server = match KrillServer::new( + let pub_server = match KrillServer::build( &config.data_dir, &config.rsync_base, config.service_uri(), @@ -121,7 +118,7 @@ impl PubServerApp { server::new(move || PubServerApp::new(ps.clone())) .bind(config.socket_addr()) - .expect(&format!("Cannot bind to: {}", config.socket_addr())) + .unwrap_or_else(|_| panic!("Cannot bind to: {}", config.socket_addr())) .shutdown_timeout(0) .start(); } @@ -136,7 +133,7 @@ impl PubServerApp { match Self::https_builder(config) { Ok(https_builder) => { server.bind_ssl(config.socket_addr(), https_builder) - .expect(&format!("Cannot bind to: {}", config.socket_addr())) + .unwrap_or_else(|_| panic!("Cannot bind to: {}", config.socket_addr())) .shutdown_timeout(0) .run(); }, @@ -148,7 +145,7 @@ impl PubServerApp { } else { server.bind(config.socket_addr()) - .expect(&format!("Cannot bind to: {}", config.socket_addr())) + .unwrap_or_else(|_| panic!("Cannot bind to: {}", config.socket_addr())) .shutdown_timeout(0) .run(); } @@ -275,7 +272,7 @@ impl FromRequest for publishers::Publisher { .and_then(|bytes| { let p: publishers::Publisher = serde_json::from_reader(bytes.as_ref()) - .map_err(|e| Error::JsonError(e))?; + .map_err(Error::JsonError)?; Ok(p) }) ) @@ -327,8 +324,7 @@ impl FromRequest for publication::PublishDelta { .from_err() .and_then(|bytes| { let delta: publication::PublishDelta = - serde_json::from_reader(bytes.as_ref()) - .map_err(|e| Error::JsonError(e))?; + serde_json::from_reader(bytes.as_ref())?; Ok(delta) }) ) @@ -386,6 +382,10 @@ pub enum Error { Other(String), } +impl From for Error { + fn from(e: serde_json::Error) -> Self { Error::JsonError(e) } +} + impl error::Error for Error { fn description(&self) -> &str { "An error happened" diff --git a/src/krilld/http/ssl.rs b/src/krilld/http/ssl.rs index ee0268ee..7f7100a8 100644 --- a/src/krilld/http/ssl.rs +++ b/src/krilld/http/ssl.rs @@ -24,9 +24,9 @@ use crate::util::softsigner::SignerKeyId; use crate::remote::builder; const KEY_SIZE: u32 = 2048; -pub const HTTPS_SUB_DIR: &'static str = "ssl"; -pub const KEY_FILE: &'static str = "key.pem"; -pub const CERT_FILE: &'static str = "cert.pem"; +pub const HTTPS_SUB_DIR: &str = "ssl"; +pub const KEY_FILE: &str = "key.pem"; +pub const CERT_FILE: &str = "cert.pem"; /// Creates a new private key and certificate file if either is found to be /// missing in the base_path directory. @@ -38,7 +38,7 @@ pub fn create_key_cert_if_needed(data_dir: &PathBuf) -> Result<(), HttpsSignerEr let cert_file_path = file::file_path(&https_dir, CERT_FILE); if ! key_file_path.exists() || ! cert_file_path.exists() { - create_key_and_cert(https_dir) + create_key_and_cert(&https_dir) } else { Ok(()) } @@ -47,12 +47,12 @@ pub fn create_key_cert_if_needed(data_dir: &PathBuf) -> Result<(), HttpsSignerEr /// Creates a new private key and certificate to be used when serving HTTPS. /// Only call this in case there is no current key and certificate file /// present, or have your files ruthlessly overwritten! -fn create_key_and_cert(https_dir: PathBuf) -> Result<(), HttpsSignerError> { +fn create_key_and_cert(https_dir: &PathBuf) -> Result<(), HttpsSignerError> { if ! https_dir.exists() { file::create_dir(&https_dir)?; } - let mut signer = HttpsSigner::new()?; + let mut signer = HttpsSigner::build()?; signer.save_private_key(&https_dir)?; signer.save_certificate(&https_dir)?; @@ -69,7 +69,7 @@ struct HttpsSigner { } impl HttpsSigner { - fn new() -> Result { + fn build() -> Result { let rsa = Rsa::generate(KEY_SIZE)?; let private = PKey::from_rsa(rsa)?; Ok(HttpsSigner { private }) @@ -80,7 +80,7 @@ impl HttpsSigner { let mut pem_file = File::create(path)?; let pem = self.private.private_key_to_pem_pkcs8()?; - pem_file.write(&pem)?; + pem_file.write_all(&pem)?; Ok(()) } @@ -94,9 +94,9 @@ impl HttpsSigner { let der = cert.to_bytes(); let cert_pem = base64::encode(&der); - pem_file.write("-----BEGIN CERTIFICATE-----\n".as_ref())?; - pem_file.write(cert_pem.as_bytes())?; - pem_file.write("\n-----END CERTIFICATE-----\n".as_ref())?; + pem_file.write_all("-----BEGIN CERTIFICATE-----\n".as_ref())?; + pem_file.write_all(cert_pem.as_bytes())?; + pem_file.write_all("\n-----END CERTIFICATE-----\n".as_ref())?; Ok(()) } diff --git a/src/krilld/krillserver.rs b/src/krilld/krillserver.rs index f43b9a6f..d588d1e6 100644 --- a/src/krilld/krillserver.rs +++ b/src/krilld/krillserver.rs @@ -13,7 +13,7 @@ use crate::remote::rfc8183; use crate::remote::sigmsg::SignedMessage; /// # Naming things in the keystore. -const ACTOR: &'static str = "krill pubd"; +const ACTOR: &str = "krill pubd"; //------------ KrillServer --------------------------------------------------- @@ -55,16 +55,16 @@ pub struct KrillServer { impl KrillServer { /// Creates a new publication server. Note that state is preserved /// on disk in the work_dir provided. - pub fn new( + pub fn build( work_dir: &PathBuf, base_uri: &uri::Rsync, service_uri: uri::Http, rrdp_base_uri: &uri::Http, authorizer: Authorizer ) -> Result { - let cms_proxy = CmsProxy::new(work_dir)?; - let publisher_store = PublisherStore::new(work_dir, base_uri)?; - let repository = Repository::new(rrdp_base_uri, work_dir)?; + let cms_proxy = CmsProxy::build(work_dir)?; + let publisher_store = PublisherStore::build(work_dir, base_uri)?; + let repository = Repository::build(rrdp_base_uri, work_dir)?; Ok( KrillServer { @@ -152,7 +152,7 @@ impl KrillServer { name: impl AsRef ) -> Result>, Error> { self.publisher_store.publisher(name) - .map_err(|e| Error::PublisherStore(e)) + .map_err(Error::PublisherStore) } /// Returns a repository response for the given publisher. @@ -166,10 +166,10 @@ impl KrillServer { let rrdp_notification = self.repository.rrdp_notification_uri(); self.cms_proxy .repository_response( - publisher, + &publisher, self.service_base_uri(), rrdp_notification) - .map_err(|e| Error::CmsProxy(e)) + .map_err(Error::CmsProxy) } pub fn rrdp_base_path(&self) -> PathBuf { @@ -209,23 +209,26 @@ impl KrillServer { }; match self.cms_proxy.publish_request(sigmsg, id_cert) { - Err(e) => self.cms_proxy.wrap_error(e).map_err(|e| Error::CmsProxy(e)), + Err(e) => self.cms_proxy.wrap_error(&e).map_err(Error::CmsProxy), Ok(req) => { let reply = match req { publication::PublishRequest::List => { - self.handle_list(handle).map(|list| - publication::PublishReply::List(list)) - + self.handle_list(handle) + .map(publication::PublishReply::List) }, publication::PublishRequest::Delta(delta) => { - self.handle_delta(delta, handle).map(|_| - publication::PublishReply::Success) + self.handle_delta(delta, handle) + .map(|_| publication::PublishReply::Success) } }; match reply { - Ok(reply) => self.cms_proxy.wrap_publish_reply(reply).map_err(|e| Error::CmsProxy(e)), - Err(Error::Repository(e)) => self.cms_proxy.wrap_error(e).map_err(|e| Error::CmsProxy(e)), + Ok(reply) => { + self.cms_proxy.wrap_publish_reply(reply).map_err(Error::CmsProxy) + }, + Err(Error::Repository(e)) => { + self.cms_proxy.wrap_error(&e).map_err(Error::CmsProxy) + }, Err(e) => Err(e) } } @@ -234,6 +237,7 @@ impl KrillServer { /// Handles a publish delta request sent to the API, or.. through /// the CmsProxy. + #[allow(clippy::needless_pass_by_value)] pub fn handle_delta( &mut self, delta: publication::PublishDelta, @@ -241,8 +245,7 @@ impl KrillServer { ) -> Result<(), Error> { let publisher = self.publisher_store.get_publisher(handle)?; let base_uri = publisher.base_uri(); - self.repository.publish(&delta, base_uri) - .map_err(|e| Error::Repository(e)) + self.repository.publish(&delta, base_uri).map_err(Error::Repository) } /// Handles a list request sent to the API, or.. through the CmsProxy. @@ -252,7 +255,7 @@ impl KrillServer { ) -> Result { let publisher = self.publisher_store.get_publisher(handle)?; let base_uri = publisher.base_uri(); - self.repository.list(base_uri).map_err(|e| Error::Repository(e)) + self.repository.list(base_uri).map_err(Error::Repository) } } diff --git a/src/krilld/pubd/mod.rs b/src/krilld/pubd/mod.rs index 45031d74..bcb027f1 100644 --- a/src/krilld/pubd/mod.rs +++ b/src/krilld/pubd/mod.rs @@ -12,7 +12,7 @@ use crate::remote::rfc8183; use crate::storage::keystore::{self, Info, Key, KeyStore}; use crate::storage::caching_ks::CachingDiskKeyStore; -pub const RSYNC_FOLDER: &'static str = "rsync"; +pub const RSYNC_FOLDER: &str = "rsync"; //------------ PublisherStore ------------------------------------------------- @@ -28,7 +28,7 @@ pub struct PublisherStore { impl PublisherStore { - pub fn new( + pub fn build( work_dir: &PathBuf, base_uri: &uri::Rsync ) -> Result { @@ -40,14 +40,14 @@ impl PublisherStore { Ok( PublisherStore { - store: CachingDiskKeyStore::new(publisher_dir)?, + store: CachingDiskKeyStore::build(publisher_dir)?, base_uri: base_uri.clone() } ) } fn verify_handle(&self, handle: &str) -> Result<(), Error> { - if handle.contains("/") { + if handle.contains('/') { return Err(Error::ForwardSlashInHandle(handle.to_string())) } @@ -61,7 +61,7 @@ impl PublisherStore { fn verify_base_uri(&self, base_uri: &uri::Rsync) -> Result<(), Error> { let base_uri = base_uri.to_string(); if base_uri.starts_with(self.base_uri.to_string().as_str()) && - base_uri.ends_with("/") { + base_uri.ends_with('/') { Ok(()) } else { Err(Error::InvalidBaseUri) @@ -80,7 +80,7 @@ impl PublisherStore { self.verify_handle(pbl.handle())?; self.verify_base_uri(pbl.base_uri())?; - let key = Key::from_str(pbl.handle()); + let key = Key::new(pbl.handle()); let info = Info::now( actor, &format!("Added publisher: {}", pbl.handle()) @@ -104,7 +104,7 @@ impl PublisherStore { match self.publisher(name)? { None => Err(Error::UnknownPublisher(name.to_string())), Some(_p) => { - let key = Key::from_str(name); + let key = Key::new(name); let info = Info::now( actor, @@ -119,7 +119,7 @@ impl PublisherStore { /// Returns whether a publisher exists for this name. pub fn has_publisher(&self, handle: &str) -> bool { - let key = Key::from_str(handle); + let key = Key::new(handle); match self.store.version(&key) { Ok(Some(version)) => { if version > 0 { @@ -138,7 +138,7 @@ impl PublisherStore { &self, handle: impl AsRef ) -> Result>, Error> { - let key = Key::from_str(handle.as_ref()); + let key = Key::new(handle.as_ref()); self.store.get(&key).map_err(|e| { Error::KeyStoreError(e)}) } @@ -162,8 +162,8 @@ impl PublisherStore { pub fn publishers(&self) -> Result>, Error> { let mut res = Vec::new(); - for ref k in self.store.keys() { - if let Some(arc) = self.store.get(k)? { + for k in self.store.keys() { + if let Some(arc) = self.store.get(&k)? { res.push(arc); } } @@ -244,7 +244,7 @@ mod tests { fn test_publisher_store(dir: &PathBuf) -> PublisherStore { let uri = test::rsync_uri("rsync://host/module/"); - PublisherStore::new(dir, &uri).unwrap() + PublisherStore::build(dir, &uri).unwrap() } #[test] diff --git a/src/krilld/pubd/repo.rs b/src/krilld/pubd/repo.rs index ffba4718..13ba3267 100644 --- a/src/krilld/pubd/repo.rs +++ b/src/krilld/pubd/repo.rs @@ -20,13 +20,13 @@ pub struct Repository { /// # Construct /// impl Repository { - pub fn new( + pub fn build( rrdp_base_uri: &uri::Http, work_dir: &PathBuf ) -> Result { - let fs = rsyncd::FileStore::new(work_dir)?; - let rrdp = rrdpd::RrdpServer::new(rrdp_base_uri, work_dir)?; + let fs = rsyncd::FileStore::build(work_dir)?; + let rrdp = rrdpd::RrdpServer::build(rrdp_base_uri, work_dir)?; Ok( Repository { fs, rrdp } ) } } @@ -110,7 +110,7 @@ mod tests { fn should_publish() { test::test_with_tmp_dir(|d| { let rrdp_base_uri = test::http_uri("http://localhost:3000/repo/"); - let mut repo = Repository::new(&rrdp_base_uri, &d).unwrap() ; + let mut repo = Repository::build(&rrdp_base_uri, &d).unwrap() ; // Publish a file let rsync_for_alice = @@ -185,7 +185,7 @@ mod tests { #[test] fn should_store_list_withdraw_files() { test::test_with_tmp_dir(|d| { - let mut file_store = rsyncd::FileStore::new(&d).unwrap(); + let mut file_store = rsyncd::FileStore::build(&d).unwrap(); // Using a port here to make sure that it works in mapping // the rsync URI to and from disk. @@ -239,7 +239,7 @@ mod tests { #[test] fn should_not_allow_publishing_or_withdrawing_outside_of_base() { test::test_with_tmp_dir(|d| { - let mut file_store = rsyncd::FileStore::new(&d).unwrap(); + let mut file_store = rsyncd::FileStore::build(&d).unwrap(); // Using a port here to make sure that it works in mapping // the rsync URI to and from disk. diff --git a/src/krilld/pubd/rrdpd.rs b/src/krilld/pubd/rrdpd.rs index 827ea66a..425298bf 100644 --- a/src/krilld/pubd/rrdpd.rs +++ b/src/krilld/pubd/rrdpd.rs @@ -22,11 +22,11 @@ use api::rrdp_data::SnapshotRef; //const VERSION: &'static str = "1"; //const NS: &'static str = "http://www.ripe.net/rpki/rrdp"; -const RRDP_FOLDER: &'static str = "rrdp"; -const FS_FOLDER: &'static str = "rsync"; +const RRDP_FOLDER: &str = "rrdp"; +const FS_FOLDER: &str = "rsync"; -const VERSION: &'static str = "1"; -const NS: &'static str = "http://www.ripe.net/rpki/rrdp"; +const VERSION: &str = "1"; +const NS: &str = "http://www.ripe.net/rpki/rrdp"; @@ -55,12 +55,12 @@ impl RrdpServer { /// present, or initialise a new server with a random session_id, /// starting at serial 1, and including a snapshot for everything /// currently stored in the rsync file_store. - pub fn new( + pub fn build( base_uri: &uri::Http, work_dir: &PathBuf ) -> Result { - if ! base_uri.to_string().ends_with("/") { + if ! base_uri.to_string().ends_with('/') { return Err(Error::UriConfigError) } @@ -70,7 +70,7 @@ impl RrdpServer { fs::create_dir_all(&rrdp_store_dir)?; } - let store = CachingDiskKeyStore::new(rrdp_store_dir)?; + let store = CachingDiskKeyStore::build(rrdp_store_dir)?; let rrdp_base = file::sub_dir(work_dir, RRDP_FOLDER)?; let fs_base = file::sub_dir(work_dir, FS_FOLDER)?; @@ -85,27 +85,27 @@ impl RrdpServer { // const REL_NOTIFICATION: &'static str = "notification.xml"; fn key_notification() -> Key { - Key::from_str("notification") + Key::new("notification") } - fn key_snapshot(session: &String, serial: usize) -> Key { - Key::from_str(&format!("{}-{}-snapshot", session, serial)) + fn key_snapshot(session: &str, serial: usize) -> Key { + Key::new(&format!("{}-{}-snapshot", session, serial)) } pub fn get_notification( &self ) -> Result>, Error> { let key = Self::key_notification(); - self.store.get(&key).map_err(|e| Error::Keystore(e) ) + self.store.get(&key).map_err(Error::Keystore) } pub fn get_snapshot( &self, - session: &String, + session: &str, serial: usize ) -> Result>, Error> { let key = Self::key_snapshot(session, serial); - self.store.get(&key).map_err(|e| Error::Keystore(e) ) + self.store.get(&key).map_err(Error::Keystore ) } pub fn save_notification( @@ -117,12 +117,12 @@ impl RrdpServer { key, notification, Info::now("server", "notification") - ).map_err(|e| Error::Keystore(e)) + ).map_err(Error::Keystore) } pub fn save_snapshot( &mut self, - session: &String, + session: &str, serial: usize, snapshot: Snapshot ) -> Result<(), Error> { @@ -131,7 +131,7 @@ impl RrdpServer { key, snapshot, Info::now("server", "notification") - ).map_err(|e| Error::Keystore(e)) + ).map_err(Error::Keystore) } } @@ -209,7 +209,7 @@ impl RrdpServer { match objects.iter().position(|cur| {cur.uri() == u.uri()}) { None => return Err(Error::NoObjectPresent(u.uri().clone())), Some(pos) => { - if objects.get(pos).unwrap().hash() != u.hash() { + if objects[pos].hash() != u.hash() { return Err(Error::NoObjectMatchingHash) } else { objects.remove(pos); @@ -226,7 +226,7 @@ impl RrdpServer { match objects.iter().position(|cur| {cur.uri() == w.uri()}) { None => return Err(Error::NoObjectPresent(w.uri().clone())), Some(pos) => { - if objects.get(pos).unwrap().hash() != w.hash() { + if objects[pos].hash() != w.hash() { return Err(Error::NoObjectMatchingHash) } else { objects.remove(pos); @@ -288,7 +288,7 @@ impl RrdpServer { /// Saves the RFC8181 PublishQuery as an RFC8182 delta file. fn save_delta( &mut self, - session_id: &String, + session_id: &str, serial: usize, delta: &publication::PublishDelta ) -> Result diff --git a/src/krilld/pubd/rsyncd.rs b/src/krilld/pubd/rsyncd.rs index 1d1288f4..55cc75c7 100644 --- a/src/krilld/pubd/rsyncd.rs +++ b/src/krilld/pubd/rsyncd.rs @@ -21,7 +21,7 @@ pub struct FileStore { /// # Construct /// impl FileStore { - pub fn new(work_dir: &PathBuf) -> Result { + pub fn build(work_dir: &PathBuf) -> Result { let mut rsync_dir = PathBuf::from(work_dir); rsync_dir.push(RSYNC_FOLDER); if ! rsync_dir.is_dir() { @@ -56,7 +56,7 @@ impl FileStore { Ok(Vec::new()) } else { file::crawl_incl_rsync_base(&path, base_uri) - .map_err(|e| Error::RecursorError(e)) + .map_err(Error::RecursorError) } } diff --git a/src/pubc/apiclient.rs b/src/pubc/apiclient.rs index 1ee99121..1739e08a 100644 --- a/src/pubc/apiclient.rs +++ b/src/pubc/apiclient.rs @@ -21,7 +21,7 @@ impl Command { } pub fn sync(dir: &str, base_uri: &str) -> Result { let dir = PathBuf::from(dir); - if ! base_uri.ends_with("/") { + if ! base_uri.ends_with('/') { Err(Error::InvalidBaseUri) } else { let uri = uri::Rsync::from_str(base_uri)?; @@ -46,7 +46,7 @@ pub struct Connection { } impl Connection { - pub fn new( + pub fn build( server_uri: &str, handle: &str, token: &str @@ -142,7 +142,7 @@ impl Options { let server_uri = m.value_of("server").unwrap(); let handle = m.value_of("handle").unwrap(); let token = m.value_of("token").unwrap(); - Connection::new(server_uri, handle, token)? + Connection::build(server_uri, handle, token)? }; let command = { @@ -194,7 +194,7 @@ pub enum ApiResponse { } impl ApiResponse { - pub fn report(&self, format: Format) { + pub fn report(&self, format: &Format) { match format { Format::None => {}, // done, Format::Json => { @@ -229,7 +229,7 @@ pub fn execute(options: Options) -> Result { match cmd { Command::List => { - list_query(&connection).map(|l| ApiResponse::List(l)) + list_query(&connection).map(ApiResponse::List) }, Command::Sync(dir, rsync_uri) => { sync(&connection, &dir, &rsync_uri) @@ -261,7 +261,7 @@ fn sync( ) -> Result { let list_reply = list_query(connection)?; let delta = pubc::create_delta( - list_reply, + &list_reply, dir, base_rsync )?; diff --git a/src/pubc/cmsclient.rs b/src/pubc/cmsclient.rs index 76dc8477..ba925aeb 100644 --- a/src/pubc/cmsclient.rs +++ b/src/pubc/cmsclient.rs @@ -23,23 +23,23 @@ use pubc; /// # Some constants for naming resources in the keystore for clients. -const ACTOR: &'static str = "publication client"; +const ACTOR: &str = "publication client"; fn id_key() -> Key { - Key::from_str("my_id") + Key::new("my_id") } fn parent_key() -> Key { - Key::from_str("my_parent") + Key::new("my_parent") } fn repo_key() -> Key { - Key::from_str("my_repo") + Key::new("my_repo") } -const ID_MSG: &'static str = "initialised identity"; -const PARENT_MSG: &'static str ="updated parent info"; -const REPO_MSG: &'static str = "update repo info"; +const ID_MSG: &str = "initialised identity"; +const PARENT_MSG: &str ="updated parent info"; +const REPO_MSG: &str = "update repo info"; //------------ PubClient ----------------------------------------------------- @@ -68,9 +68,9 @@ pub struct PubClient { impl PubClient { /// Creates a new publication client - pub fn new(work_dir: &PathBuf) -> Result { - let store = CachingDiskKeyStore::new(work_dir.clone())?; - let signer = OpenSslSigner::new(work_dir)?; + pub fn build(work_dir: &PathBuf) -> Result { + let store = CachingDiskKeyStore::build(work_dir.clone())?; + let signer = OpenSslSigner::build(work_dir)?; Ok( PubClient { signer, @@ -129,7 +129,7 @@ impl PubClient { /// Process the publication server parent response. pub fn process_repo_response( &mut self, - response: rfc8183::RepositoryResponse + response: &rfc8183::RepositoryResponse ) -> Result<(), Error> { // Store parent info @@ -182,7 +182,7 @@ impl PubClient { let query = rfc8181::Message::list_query(); let signed_request = self.sign_request(query)?; - let reply = self.send_request(signed_request)?.as_reply()?; + let reply = self.send_request(signed_request)?.into_reply()?; match reply { rfc8181::ReplyMessage::ErrorReply(e) => Err(Error::ErrorReply(e)), @@ -198,12 +198,13 @@ impl PubClient { let repo = self.get_my_repo()?; let list_reply = self.get_server_list()?; - let delta = pubc::create_delta(list_reply, base_path, repo.sia_base())?; + let delta = pubc::create_delta(&list_reply, base_path, repo.sia_base + ())?; if ! delta.is_empty() { let msg = rfc8181::Message::publish_delta_query(delta); let sgn_msg = self.sign_request(msg)?; - let reply = self.send_request(sgn_msg)?.as_reply()?; + let reply = self.send_request(sgn_msg)?.into_reply()?; match reply { rfc8181::ReplyMessage::ErrorReply(e) => Err(Error::ErrorReply(e)), @@ -245,7 +246,7 @@ impl PubClient { ) -> Result { let id = self.get_my_id()?; - let builder = SignedMessageBuilder::new( + let builder = SignedMessageBuilder::create( id.key_id(), &mut self.signer, msg diff --git a/src/pubc/mod.rs b/src/pubc/mod.rs index ef0eb9a4..2ef0697b 100644 --- a/src/pubc/mod.rs +++ b/src/pubc/mod.rs @@ -7,7 +7,7 @@ use crate::api::publication; use crate::util::file; pub fn create_delta( - list_reply: publication::ListReply, + list_reply: &publication::ListReply, dir: &PathBuf, base_rsync: &uri::Rsync ) -> Result { @@ -17,7 +17,7 @@ pub fn create_delta( // loop through what the server has and find the ones to withdraw for p in list_reply.elements() { - if current.iter().find(|c| { c.uri() == p.uri() }).is_none() { + if current.iter().find(|c| c.uri() == p.uri()).is_none() { delta_builder.add_withdraw( publication::Withdraw::from_list_element(p) ); @@ -28,7 +28,7 @@ pub fn create_delta( // to be added to, which need to be updated at, or for which no change is // needed at the server. for f in current { - match list_reply.elements().iter().find(|pbl| { pbl.uri() == f.uri()}) { + match list_reply.elements().iter().find(|pbl| pbl.uri() == f.uri()) { None => delta_builder.add_publish(f.as_publish()), Some(pbl) => { if pbl.hash() != f.hash() { diff --git a/src/remote/builder.rs b/src/remote/builder.rs index 1406ae5e..6a00f4d5 100644 --- a/src/remote/builder.rs +++ b/src/remote/builder.rs @@ -31,6 +31,7 @@ use crate::remote::rfc8181::Message; //------------ TbsCertificate ------------------------------------------------ /// The supported extension types for our RPKI TbsCertificate +#[allow(clippy::large_enum_variant)] pub enum RpkiTbsExtension { ResourceExtensions(Extensions), IdExtensions(IdExtensions) @@ -267,14 +268,14 @@ pub struct SignedMessageBuilder { } impl SignedMessageBuilder { - pub fn new( + pub fn create( issuing_key: &S::KeyId, signer: &mut S, message: Message ) -> Result> { let content = OctetString::new(message.into_bytes()); - let signer_info = SignerInfoBuilder::new( + let signer_info = SignerInfoBuilder::create( signer, &content.to_bytes() )?; @@ -285,7 +286,7 @@ impl SignedMessageBuilder { signer )?; - let crl = CrlBuilder::new(issuing_key, signer)?; + let crl = CrlBuilder::create(issuing_key, signer)?; Ok( SignedMessageBuilder { @@ -464,7 +465,7 @@ impl SignedAttributes { // encoding, rather an EXPLICIT SET OF tag is used... let encode_in_set = encode::set(self.encode()).to_captured(Mode::Der); signer.sign_one_off(SignatureAlgorithm, encode_in_set.as_slice()) - .map_err(|e| Error::SignerError(e)) + .map_err(Error::SignerError) } } @@ -546,7 +547,7 @@ impl SignerInfoBuilder { /// /// A lot of this is pretty well restricted in RFCs 6488 amd 6492. We /// really only require some bits. - pub fn new( + pub fn create( signer: &mut S, message: &Bytes ) -> Result> { @@ -589,7 +590,7 @@ impl CrlBuilder { /// /// This will all be changed in future when we implement generating CRLs /// for the RPKI CA. - pub fn new( + pub fn create( issuing_key: &S::KeyId, signer: &mut S ) -> Result> @@ -701,7 +702,7 @@ pub mod tests { #[test] fn should_create_self_signed_ta_id_cert() { test::test_with_tmp_dir(|d| { - let mut s = OpenSslSigner::new(&d); + let mut s = OpenSslSigner::build(&d); let key_id = s.create_key(&PublicKeyAlgorithm::RsaEncryption).unwrap(); let id_cert = IdCertBuilder::new_ta_id_cert(&key_id, & mut s).unwrap(); @@ -712,11 +713,11 @@ pub mod tests { #[test] fn should_create_crl_for_protocol() { test::test_with_tmp_dir(|d| { - let mut s = OpenSslSigner::new(&d); + let mut s = OpenSslSigner::build(&d); let key_id = s.create_key(&PublicKeyAlgorithm::RsaEncryption).unwrap(); let key_info = s.get_key_info(&key_id).unwrap(); - let crl = CrlBuilder::new(&key_id, & mut s).unwrap(); + let crl = CrlBuilder::create(&key_id, & mut s).unwrap(); crl.validate(&key_info).unwrap(); }) } @@ -724,13 +725,13 @@ pub mod tests { #[test] fn should_create_signed_publication_message() { test::test_with_tmp_dir(|d| { - let mut s = OpenSslSigner::new(&d); + let mut s = OpenSslSigner::build(&d); let key_id = s.create_key(&PublicKeyAlgorithm::RsaEncryption).unwrap(); let id_cert = IdCertBuilder::new_ta_id_cert(&key_id, & mut s).unwrap(); let message = ListQuery::build_message(); - let builder = SignedMessageBuilder::new( + let builder = SignedMessageBuilder::create( &key_id, &mut s, message.clone() diff --git a/src/remote/cmsproxy.rs b/src/remote/cmsproxy.rs index dba665b9..eeb29061 100644 --- a/src/remote/cmsproxy.rs +++ b/src/remote/cmsproxy.rs @@ -33,7 +33,7 @@ pub struct CmsProxy { /// # Set up impl CmsProxy { - pub fn new( + pub fn build( work_dir: &PathBuf ) -> Result { let responder = Responder::init(work_dir)?; @@ -56,8 +56,8 @@ impl CmsProxy { debug!("Validating Signed Message"); msg.validate(id_cert)?; let msg = rfc8181::Message::from_signed_message(&msg)?; - let msg = msg.as_query()?; - Ok(msg.as_publish_request()) + let msg = msg.into_query()?; + Ok(msg.into_publish_request()) } /// Handles a PublishReply, and wraps it in an RFC8181 message @@ -75,13 +75,13 @@ impl CmsProxy { } }; - self.responder.sign_msg(msg).map_err(|e| Error::ResponderError(e)) + self.responder.sign_msg(msg).map_err(Error::ResponderError) } /// Converts an error to an RFC8181 response message pub fn wrap_error( &mut self, - error: impl ToReportErrorCode + error: &impl ToReportErrorCode ) -> Result { let mut error_builder = rfc8181::ErrorReply::build(); error_builder.add( @@ -92,13 +92,13 @@ impl CmsProxy { ); let msg = error_builder.build_message(); - self.responder.sign_msg(msg).map_err(|e| Error::ResponderError(e)) + self.responder.sign_msg(msg).map_err(Error::ResponderError) } /// Returns an RFC8183 Repository Response pub fn repository_response( &self, - publisher: Arc, + publisher: &Arc, base_service_uri: &uri::Http, rrdp_notification_uri: uri::Http ) -> Result { @@ -112,10 +112,10 @@ impl CmsProxy { self.responder .repository_response( - publisher, + &publisher, service_uri, rrdp_notification_uri) - .map_err(|e| Error::ResponderError(e)) + .map_err(Error::ResponderError) } diff --git a/src/remote/id.rs b/src/remote/id.rs index e07978fb..5f0a5360 100644 --- a/src/remote/id.rs +++ b/src/remote/id.rs @@ -422,7 +422,7 @@ impl IdCert { // 4.8.1. Basic Constraints: For a CA it must be present (RFC6487) // und the “cA” flag must be set (RFC5280). if let Some(ref ca) = self.extensions.basic_ca { - if ca.ca() == true { + if ca.ca() { return Ok(()) } } diff --git a/src/remote/responder.rs b/src/remote/responder.rs index 12e1253a..092759a7 100644 --- a/src/remote/responder.rs +++ b/src/remote/responder.rs @@ -18,13 +18,13 @@ use crate::util::softsigner::{self, OpenSslSigner}; /// # Naming things in the keystore. -const ACTOR: &'static str = "publication server"; +const ACTOR: &str = "publication server"; fn my_id_key() -> Key { - Key::from_str("my_id") + Key::new("my_id") } -const MY_ID_MSG: &'static str = "initialised identity"; +const MY_ID_MSG: &str = "initialised identity"; //------------ Responder ----------------------------------------------------- @@ -53,8 +53,8 @@ impl Responder { fs::create_dir_all(&responder_dir)?; } - let signer = OpenSslSigner::new(&responder_dir)?; - let store = CachingDiskKeyStore::new(responder_dir)?; + let signer = OpenSslSigner::build(&responder_dir)?; + let store = CachingDiskKeyStore::build(responder_dir)?; let mut responder = Responder { signer, @@ -95,7 +95,7 @@ impl Responder { impl Responder { pub fn repository_response( &self, - publisher: Arc, + publisher: &Arc, service_uri: uri::Http, rrdp_notification_uri: uri::Http ) -> Result { @@ -130,7 +130,7 @@ impl Responder { /// Creates an encoded SignedMessage for a contained Message. pub fn sign_msg(&mut self, msg: Message) -> Result { if let Some(id) = self.my_identity()? { - let builder = SignedMessageBuilder::new( + let builder = SignedMessageBuilder::create( id.key_id(), &mut self.signer, msg @@ -224,7 +224,7 @@ mod tests { let rrdp_uri = test::http_uri("http://host/rrdp/"); responder.repository_response( - publisher, + &publisher, service_uri, rrdp_uri ).unwrap(); diff --git a/src/remote/rfc8181.rs b/src/remote/rfc8181.rs index eabc745e..3d12d0bd 100644 --- a/src/remote/rfc8181.rs +++ b/src/remote/rfc8181.rs @@ -13,8 +13,8 @@ use crate::util::xml::{ XmlWriter }; -pub const VERSION: &'static str = "4"; -pub const NS: &'static str = "http://www.hactrn.net/uris/rpki/publication-spec/"; +pub const VERSION: &str = "4"; +pub const NS: &str = "http://www.hactrn.net/uris/rpki/publication-spec/"; //------------ Message ------------------------------------------------------- @@ -51,7 +51,7 @@ impl Message { Ok(Message::ReplyMessage(ReplyMessage::decode(r)?)) } _ => { - return Err(MessageError::UnknownMessageType) + Err(MessageError::UnknownMessageType) } } }) @@ -104,7 +104,7 @@ impl Message { /// Consumes this message and returns the contained query, or /// an error if you tried this on a reply. - pub fn as_query(self) -> Result { + pub fn into_query(self) -> Result { match self { Message::QueryMessage(q) => Ok(q), _ => Err(MessageError::WrongMessageType) @@ -113,7 +113,7 @@ impl Message { /// Consumes this message and returns the contained query, or /// an error if you tried this on a reply. - pub fn as_reply(self) -> Result { + pub fn into_reply(self) -> Result { match self { Message::ReplyMessage(r) => Ok(r), _ => Err(MessageError::WrongMessageType) @@ -207,7 +207,7 @@ impl QueryMessage { } /// Consumes this and returns this a PublishRequest for our (json) API - pub fn as_publish_request(self) -> publication::PublishRequest { + pub fn into_publish_request(self) -> publication::PublishRequest { match self { QueryMessage::ListQuery => publication::PublishRequest::List, QueryMessage::PublishDelta(d) => publication::PublishRequest::Delta(d) @@ -321,16 +321,11 @@ impl PublishDeltaXml { ) -> Result { let mut bld = publication::PublishDeltaBuilder::new(); - loop { - match Self::decode_opt(r)? { - Some(pde) => { - match pde { - PublishDeltaElement::Publish(p) => bld.add_publish(p), - PublishDeltaElement::Update(u) => bld.add_update(u), - PublishDeltaElement::Withdraw(w) => bld.add_withdraw(w) - } - }, - None => break + while let Some(pde) = Self::decode_opt(r)? { + match pde { + PublishDeltaElement::Publish(p) => bld.add_publish(p), + PublishDeltaElement::Update(u) => bld.add_update(u), + PublishDeltaElement::Withdraw(w) => bld.add_withdraw(w) } } diff --git a/src/remote/rfc8183.rs b/src/remote/rfc8183.rs index c7f40c51..d78ecf7f 100644 --- a/src/remote/rfc8183.rs +++ b/src/remote/rfc8183.rs @@ -16,8 +16,8 @@ use crate::util::xml::{AttributesError, XmlReader, XmlReaderErr, XmlWriter}; //------------ PublisherRequest ---------------------------------------------- -pub const VERSION: &'static str = "1"; -pub const NS: &'static str = "http://www.hactrn.net/uris/rpki/rpki-setup/"; +pub const VERSION: &str = "1"; +pub const NS: &str = "http://www.hactrn.net/uris/rpki/rpki-setup/"; /// Type representing a /// @@ -67,7 +67,7 @@ impl PublisherRequest { Ok(PublisherRequest{ tag: tag.map(Into::into), - publisher_handle: ph.into(), + publisher_handle: ph, id_cert: IdCert::decode(cert)? }) }) @@ -79,7 +79,8 @@ impl PublisherRequest { } pub fn validate_at(&self, now: Time) -> Result<(), PublisherRequestError> { - Ok(self.id_cert.validate_ta_at(now)?) + self.id_cert.validate_ta_at(now)?; + Ok(()) } /// Encodes a to a Vec @@ -281,7 +282,7 @@ impl RepositoryResponse { Ok(RepositoryResponse{ tag: tag.map(Into::into), - publisher_handle: publisher_handle.into(), + publisher_handle, id_cert: IdCert::decode(id_cert)?, service_uri, sia_base, @@ -300,7 +301,8 @@ impl RepositoryResponse { &self, now: Time ) -> Result<(), RepositoryResponseError> { - Ok(self.id_cert.validate_ta_at(now)?) + self.id_cert.validate_ta_at(now)?; + Ok(()) } /// Encodes the to a Vec diff --git a/src/remote/sigmsg.rs b/src/remote/sigmsg.rs index 81e1566b..6a04a047 100644 --- a/src/remote/sigmsg.rs +++ b/src/remote/sigmsg.rs @@ -161,7 +161,7 @@ impl SignedMessage { let msg = self.signer_info.signed_attrs().encode_verify(); ::ring::signature::verify( &::ring::signature::RSA_PKCS1_2048_8192_SHA256, - Input::from(self.id_cert.public_key().as_ref()), + Input::from(self.id_cert.public_key()), Input::from(&msg), Input::from(self.signer_info.signature().value().as_ref()) ).map_err(|_| ValidationError) diff --git a/src/storage/caching_ks.rs b/src/storage/caching_ks.rs index aaba0755..9d8069f6 100644 --- a/src/storage/caching_ks.rs +++ b/src/storage/caching_ks.rs @@ -41,7 +41,7 @@ pub struct CachingDiskKeyStore { /// # Creating impl CachingDiskKeyStore { - pub fn new(base_dir: PathBuf) -> Result { + pub fn build(base_dir: PathBuf) -> Result { if ! base_dir.is_dir() { Err(Error::Other("Invalid base_dir for DiskKeyStore".to_string())) } else { @@ -65,7 +65,7 @@ impl CachingDiskKeyStore { let v = Arc::new(value); let mut w = self.cache.write() - .map_err(|_| Error::from_str("Write Lock error"))?; + .map_err(|_| Error::other("Write Lock error"))?; if let Some(current) = w.get_mut(&key) { current.version += 1; current.value = v; @@ -87,14 +87,14 @@ impl CachingDiskKeyStore { key: &Key ) -> Result>, Error> { let r = self.cache.read().map_err( - |_| Error::from_str("Can't get read lock"))?; + |_| Error::other("Can't get read lock"))?; match r.get(key) { None => Ok(None), Some(ref v) => { if let Ok(res) = v.value_copy().downcast::() { Ok(Some(res)) } else { - Err(Error::from_str("Object has the wrong type!")) + Err(Error::other("Object has the wrong type!")) } } } @@ -103,7 +103,7 @@ impl CachingDiskKeyStore { /// Gets the version for the current value for a key, if any. fn cache_version(&self, key: &Key) -> Result, Error> { let r = self.cache.read().map_err( - |_| Error::from_str("Can't get read lock"))?; + |_| Error::other("Can't get read lock"))?; Ok(r.get(key).map(|c| { c.version })) } } @@ -113,8 +113,8 @@ impl CachingDiskKeyStore { impl CachingDiskKeyStore { /// Verifies that the base directory exists, or tries to create it. fn verify_or_create_dir(&self, key: &Key) -> Result<(), Error> { - if key.path().to_string_lossy().contains("/") { - return Err(Error::from_str("Key cannot contain subdir.")) + if key.path().to_string_lossy().contains('/') { + return Err(Error::other("Key cannot contain subdir.")) } let mut full_path = PathBuf::new(); @@ -123,10 +123,8 @@ impl CachingDiskKeyStore { if !full_path.exists() { fs::create_dir_all(full_path)?; - } else { - if ! full_path.is_dir() { - return Err(Error::from_str("Key is not a dir")); - } + } else if ! full_path.is_dir() { + return Err(Error::other("Key is not a dir")); } Ok(()) @@ -157,12 +155,12 @@ impl CachingDiskKeyStore { let value_key = self.key_for_value(&key, version); let mut f = File::create(self.full_path(value_key.path()))?; let v = serde_json::to_string(&value)?; - f.write(v.as_ref())?; + f.write_all(v.as_ref())?; let info_key = self.key_for_info(&key, version); let mut f = File::create(self.full_path(info_key.path()))?; let i = serde_json::to_string(&info)?; - f.write(i.as_ref())?; + f.write_all(i.as_ref())?; Ok(()) } @@ -204,22 +202,22 @@ impl CachingDiskKeyStore { /// Archives the current version for a key, by negating the version /// number. Note that versions can be 'revived' simply by storing a new /// value for a key. - fn disk_archive(&self, key: &Key, info: Info) -> Result<(), Error> { + fn disk_archive(&self, key: &Key, info: &Info) -> Result<(), Error> { if let Some(v) = self.disk_version(key)? { if v > 0 { let version_key = self.key_for_version(key); let mut f = File::create(self.full_path(version_key.path()))?; - write!(f, "{}", v * -1)?; + write!(f, "{}", -v)?; let arch_key = self.key_for_archive_info(&key, v); let mut f = File::create(self.full_path(arch_key.path()))?; let i = serde_json::to_string(&info)?; - f.write(i.as_ref())?; + f.write_all(i.as_ref())?; return Ok(()) } } - Err(Error::from_str("No version to archive")) + Err(Error::other("No version to archive")) } } @@ -250,7 +248,7 @@ impl KeyStore for CachingDiskKeyStore { let mut w = self.cache.write().unwrap(); w.remove_entry(key); // Don't care if it was actually cached. } - self.disk_archive(key, info) + self.disk_archive(key, &info) } fn get( @@ -358,8 +356,8 @@ mod tests { #[test] fn should_store_and_retrieve_from_caching_disk() { test::test_with_tmp_dir(|d| { - let mut store = CachingDiskKeyStore::new(PathBuf::from(d)).unwrap(); - let key = Key::from_str("key_name"); + let mut store = CachingDiskKeyStore::build(PathBuf::from(d)).unwrap(); + let key = Key::new("key_name"); let value = TestStruct::from_str("foo"); let info = Info::new(Utc::now(), "me", "A!"); @@ -375,8 +373,8 @@ mod tests { #[test] fn should_report_keys_from_caching_disk_store() { test::test_with_tmp_dir(|d| { - let mut store = CachingDiskKeyStore::new(PathBuf::from(d)).unwrap(); - let key = Key::from_str("key_name"); + let mut store = CachingDiskKeyStore::build(PathBuf::from(d)).unwrap(); + let key = Key::new("key_name"); let value = TestStruct::from_str("foo"); let info = Info::new(Utc::now(), "me", "A!"); store.store(key.clone(), value.clone(), info).unwrap(); @@ -391,16 +389,16 @@ mod tests { fn should_read_from_disk() { test::test_with_tmp_dir(|d| { // Store stuff in memory and on disk - let mut store = CachingDiskKeyStore::new(PathBuf::from(d.clone())) + let mut store = CachingDiskKeyStore::build(PathBuf::from(d.clone())) .unwrap(); - let key = Key::from_str("key_name"); + let key = Key::new("key_name"); let value = TestStruct::from_str("foo"); let info = Info::new(Utc::now(), "me", "A!"); store.store(key.clone(), value.clone(), info).unwrap(); // Initiate a new keystore pointing to the same dir, so it can // read values from there. - let store = CachingDiskKeyStore::new(PathBuf::from(d)).unwrap(); + let store = CachingDiskKeyStore::build(PathBuf::from(d)).unwrap(); let stored_keys: Vec = store.keys().collect(); assert!(stored_keys.contains(&key)); @@ -417,9 +415,9 @@ mod tests { fn should_archive() { test::test_with_tmp_dir(|d| { // Store stuff in memory and on disk - let mut store = CachingDiskKeyStore::new(PathBuf::from(d.clone())) + let mut store = CachingDiskKeyStore::build(PathBuf::from(d.clone())) .unwrap(); - let key = Key::from_str("key_name"); + let key = Key::new("key_name"); let value = TestStruct::from_str("foo"); let actor = "me"; let msg = "created"; diff --git a/src/storage/events.rs b/src/storage/events.rs index 83b838a9..027bd1c9 100644 --- a/src/storage/events.rs +++ b/src/storage/events.rs @@ -26,7 +26,7 @@ impl Storable for T { } pub struct AggregateId(String); impl AggregateId { - pub fn from_str(s: &str) -> Self { + pub fn new(s: &str) -> Self { AggregateId(s.to_string()) } } @@ -256,9 +256,8 @@ impl AggregateManager { if ! has_key { let init_key = S::key_for_event(id, 0); if let Some(init) = self.store.get::(&init_key) - .map_err(|e| AggMgrErr::KeyStoreError(e))? { - let mut agg = A::init(init) - .map_err(|e| AggMgrErr::AggregateError(e))?; + .map_err(AggMgrErr::KeyStoreError)? { + let mut agg = A::init(init).map_err(AggMgrErr::AggregateError)?; cache.insert(id.clone(), Arc::new(agg)); force = true; @@ -275,7 +274,7 @@ impl AggregateManager { let ver = agg.version(); let key = S::key_for_event(id, ver); if let Some(event) = self.store.get::(&key) - .map_err(|e| AggMgrErr::KeyStoreError(e))? { + .map_err(AggMgrErr::KeyStoreError)? { agg.apply(event) } else { break @@ -288,6 +287,7 @@ impl AggregateManager { /// Get a reference to the latest version of the aggregate. + #[allow(clippy::type_complexity)] pub fn get_latest( &self, id: &AggregateId @@ -297,6 +297,7 @@ impl AggregateManager { .map(|arc| AggregateRef { agg: arc.clone() } )) } + #[allow(clippy::type_complexity)] pub fn create( &self, id: &AggregateId, @@ -309,11 +310,9 @@ impl AggregateManager { Err(AggMgrErr::AggregateAlreadyExists) } else { let key = S::key_for_event(id, 0); - self.store.store(&key, &event) - .map_err(|e| AggMgrErr::KeyStoreError(e))?; + self.store.store(&key, &event).map_err(AggMgrErr::KeyStoreError)?; - let agg = A::init(event) - .map_err(|e| AggMgrErr::AggregateError(e))?; + let agg = A::init(event).map_err(AggMgrErr::AggregateError)?; cache.insert(id.clone(), Arc::new(agg)); Ok(()) @@ -322,6 +321,7 @@ impl AggregateManager { /// Apply a command to the latest aggregate, save the events and return /// the updated aggregate. + #[allow(clippy::type_complexity)] pub fn apply( &self, command: A::Command @@ -345,12 +345,11 @@ impl AggregateManager { } let events = agg.process_command(command) - .map_err(|e| AggMgrErr::AggregateError(e))?; + .map_err(AggMgrErr::AggregateError)?; for e in events { let key = S::key_for_event(&id, e.version()); - self.store.store(&key, &e) - .map_err(|e| AggMgrErr::KeyStoreError(e))?; + self.store.store(&key, &e).map_err(AggMgrErr::KeyStoreError)?; agg.apply(e); } @@ -434,7 +433,7 @@ impl KeyStore for DiskKeyStore { } else { let mut f = file::create_file_with_path(&self.file_path(key))?; let json = serde_json::to_string(value)?; - f.write(json.as_ref())?; + f.write_all(json.as_ref())?; Ok(()) } } @@ -631,7 +630,7 @@ mod tests { let storage = DiskKeyStore::new(d.clone()); let manager = PersonManager::new(storage); - let id_alice = AggregateId::from_str("alice"); + let id_alice = AggregateId::new("alice"); let alice_init = InitPersonEvent::init(&id_alice, "alice smith"); manager.create(&id_alice, alice_init).unwrap(); diff --git a/src/storage/keystore.rs b/src/storage/keystore.rs index 8448cd61..53446361 100644 --- a/src/storage/keystore.rs +++ b/src/storage/keystore.rs @@ -22,6 +22,17 @@ pub struct Key { } impl Key { + /// Creates an instance from a ui str. Will unwrap, and panic, if + /// unsafe characters are used. Use [`from_path`] for a method that + /// returns a Result instead, and see [`verify_path`] for restrictions. + /// + /// [`from_path`]: struct.Key.html#method.from_path + /// [`verify_path`]: struct.Key.html#method.verify_path + pub fn new(s: &str) -> Key { + let path = PathBuf::from(s); + Self::from_path(path).unwrap() + } + /// Creates a new key based on the path. /// /// Paths must not contain '/' so that they can be used as a single @@ -35,17 +46,6 @@ impl Key { Ok(Self { path }) } - /// Creates an instance from a ui str. Will unwrap, and panic, if - /// unsafe characters are used. Use [`from_path`] for a method that - /// returns a Result instead, and see [`verify_path`] for restrictions. - /// - /// [`from_path`]: struct.Key.html#method.from_path - /// [`verify_path`]: struct.Key.html#method.verify_path - pub fn from_str(s: &str) -> Key { - let path = PathBuf::from(s); - Self::from_path(path).unwrap() - } - /// Other than this the may contain any character allowed in a /// 'segment' in the 'hier-part' defined in RFC3896 only, i.e: /// @@ -63,20 +63,20 @@ impl Key { match path.to_str() { None => { return Err(InvalidKey) }, Some(s) => { - if ! s.bytes().all(|b| { + if ! s.bytes().all(|b| b.is_ascii_alphanumeric() || // ALPHA DIGIT b == b'-' || b == b'.' || b == b'_' || b == b'~' || b == b'%' || // Not checking against invalid % encoding (e.g. %%) b == b'!' || b == b'$' || b == b'&' || b == b'\'' || b == b'(' || b == b')' || b == b'*' || b == b'+' || b == b',' || b == b';' || b == b'=' - }) { + ) { return Err(InvalidKey) } } } - if path.components().all(|c| { c == Component::Normal("..".as_ref())}) + if path.components().all(|c| c == Component::Normal("..".as_ref())) || ! path.is_relative() { return Err(InvalidKey); @@ -242,7 +242,7 @@ pub trait KeyStore { None => Ok(1), Some(current) => { if current < 0 { - Ok(current * -1 + 1) + Ok(-current + 1) } else { Ok(current + 1) } @@ -270,7 +270,7 @@ pub enum Error { } impl Error { - pub fn from_str(s: &str) -> Self { + pub fn other(s: &str) -> Self { Error::Other(s.to_string()) } } diff --git a/src/util/file.rs b/src/util/file.rs index 4d37e6bc..1b9788cd 100644 --- a/src/util/file.rs +++ b/src/util/file.rs @@ -47,7 +47,7 @@ pub fn file_path(base_path: &PathBuf, file_name: &str) -> PathBuf { /// Saves a file, creating parent dirs as needed pub fn save(content: &Bytes, full_path: &PathBuf) -> Result<(), io::Error> { let mut f = create_file_with_path(full_path)?; - f.write(content)?; + f.write_all(content)?; trace!("Saved file: {}", full_path.to_string_lossy()); Ok(()) diff --git a/src/util/httpclient.rs b/src/util/httpclient.rs index 66c2ec2f..c53fd71b 100644 --- a/src/util/httpclient.rs +++ b/src/util/httpclient.rs @@ -12,7 +12,7 @@ use reqwest::header::{ use serde::Serialize; use serde::de::DeserializeOwned; -const JSON_CONTENT: &'static str = "application/json"; +const JSON_CONTENT: &str = "application/json"; /// Performs a GET request that expects a json response that can be @@ -137,7 +137,7 @@ fn client() -> Result { .gzip(true) .timeout(Duration::from_secs(30)) .build() - .map_err(|e| Error::RequestError(e)) + .map_err(Error::RequestError) } fn headers( diff --git a/src/util/softsigner.rs b/src/util/softsigner.rs index 983c02f0..6d30f86d 100644 --- a/src/util/softsigner.rs +++ b/src/util/softsigner.rs @@ -53,7 +53,7 @@ pub struct OpenSslSigner { } impl OpenSslSigner { - pub fn new(work_dir: &PathBuf) -> Result { + pub fn build(work_dir: &PathBuf) -> Result { let meta_data = fs::metadata(&work_dir)?; if meta_data.is_dir() { @@ -65,7 +65,7 @@ impl OpenSslSigner { Ok( OpenSslSigner { - store: CachingDiskKeyStore::new(keys_dir)?, + store: CachingDiskKeyStore::build(keys_dir)?, } ) } else { @@ -104,12 +104,12 @@ impl Signer for OpenSslSigner { _algorithm: PublicKeyFormat ) -> Result { - let kp = OpenSslKeyPair::new()?; + let kp = OpenSslKeyPair::build()?; let pk = &kp.subject_public_key_info()?; let hex_hash = hex::encode(pk.key_identifier().as_ref()); let key_id = SignerKeyId(hex_hash); - let store_key = Key::from_str(key_id.as_ref()); + let store_key = Key::new(key_id.as_ref()); let info = Info::now("openssl signer", "created key"); self.store.store(store_key, kp, info)?; @@ -121,7 +121,7 @@ impl Signer for OpenSslSigner { &self, key_id: &Self::KeyId ) -> Result> { - let store_key = Key::from_str(key_id.as_ref()); + let store_key = Key::new(key_id.as_ref()); let key_pair_option: Option> = self.store.get(&store_key) @@ -140,7 +140,7 @@ impl Signer for OpenSslSigner { key_id: &Self::KeyId ) -> Result<(), KeyError> { - let store_key = Key::from_str(key_id.as_ref()); + let store_key = Key::new(key_id.as_ref()); let info = Info::now("openssl signer", "archived key"); self.store.archive(&store_key, info).map_err(|e| { @@ -154,7 +154,7 @@ impl Signer for OpenSslSigner { _algorithm: SignatureAlgorithm, data: &D ) -> Result> { - let store_key = Key::from_str(key_id.as_ref()); + let store_key = Key::new(key_id.as_ref()); let key_pair_option: Option> = self.store.get(&store_key) .map_err(|e| { @@ -175,7 +175,7 @@ impl Signer for OpenSslSigner { _algorithm: SignatureAlgorithm, data: &D ) -> Result<(Signature, PublicKey), SignerError> { - let kp = OpenSslKeyPair::new()?; + let kp = OpenSslKeyPair::build()?; let signature = Self::sign_with_key( kp.pkey.as_ref(), @@ -234,7 +234,7 @@ impl<'de> Deserialize<'de> for OpenSslKeyPair { } impl OpenSslKeyPair { - fn new() -> Result { + fn build() -> Result { // Issues unwrapping this indicate a bug in the openssl library. // So, there is no way to recover. let rsa = Rsa::generate(2048)?; @@ -309,7 +309,7 @@ pub mod tests { #[test] fn should_return_subject_public_key_info() { test::test_with_tmp_dir(|d| { - let mut s = OpenSslSigner::new(&d).unwrap(); + let mut s = OpenSslSigner::build(&d).unwrap(); let ki = s.create_key(PublicKeyFormat).unwrap(); s.get_key_info(&ki).unwrap(); s.destroy_key(&ki).unwrap(); @@ -319,7 +319,7 @@ pub mod tests { #[test] fn should_serialize_and_deserialize_key() { - let key = OpenSslKeyPair::new().unwrap(); + let key = OpenSslKeyPair::build().unwrap(); let json = serde_json::to_string(&key).unwrap(); let key_des: OpenSslKeyPair = serde_json::from_str(json.as_str()).unwrap(); let json_from_des = serde_json::to_string(&key_des).unwrap(); diff --git a/src/util/test.rs b/src/util/test.rs index d1e800b0..decbacb6 100644 --- a/src/util/test.rs +++ b/src/util/test.rs @@ -49,7 +49,7 @@ pub fn http_uri(s: &str) -> uri::Http { pub fn as_bytes(s: &str) -> Bytes { Bytes::from(s) } pub fn new_id_cert(work_dir: &PathBuf) -> IdCert { - let mut s = OpenSslSigner::new(work_dir).unwrap(); + let mut s = OpenSslSigner::build(work_dir).unwrap(); let key_id = s.create_key(PublicKeyFormat).unwrap(); IdCertBuilder::new_ta_id_cert(&key_id, &mut s).unwrap() } @@ -70,7 +70,7 @@ pub fn save_file(base_dir: &PathBuf, file_name: &str, content: &[u8]) { let mut full_name = base_dir.clone(); full_name.push(PathBuf::from(file_name)); let mut f = File::create(full_name).unwrap(); - f.write(content).unwrap(); + f.write_all(content).unwrap(); } pub fn save_pr(base_dir: &PathBuf, file_name: &str, pr: &PublisherRequest) { @@ -78,5 +78,5 @@ pub fn save_pr(base_dir: &PathBuf, file_name: &str, pr: &PublisherRequest) { full_name.push(PathBuf::from(file_name)); let mut f = File::create(full_name).unwrap(); let xml = pr.encode_vec(); - f.write(xml.as_ref()).unwrap(); + f.write_all(xml.as_ref()).unwrap(); } diff --git a/src/util/xml.rs b/src/util/xml.rs index 6934e06a..c4a96e69 100644 --- a/src/util/xml.rs +++ b/src/util/xml.rs @@ -46,7 +46,7 @@ impl XmlReader { } /// Puts an XmlEvent back so that it can be retrieved by 'next' - fn cache(&mut self, e: XmlEvent) -> () { + fn cache(&mut self, e: XmlEvent) { self.cached_event = Some(e); } } @@ -61,8 +61,8 @@ impl XmlReader { /// Takes the next element and expects a start of document. fn start_document(&mut self) -> Result<(), XmlReaderErr> { match self.next() { - Ok(reader::XmlEvent::StartDocument {..}) => { Ok(())}, - _ => return Err(XmlReaderErr::ExpectedStartDocument) + Ok(reader::XmlEvent::StartDocument {..}) => Ok(()), + _ => Err(XmlReaderErr::ExpectedStartDocument) } } @@ -72,7 +72,7 @@ impl XmlReader { Ok(reader::XmlEvent::StartElement { name, attributes, ..}) => { Ok((Tag{name: name.local_name}, Attributes{attributes})) }, - _ => return Err(XmlReaderErr::ExpectedStart) + _ => Err(XmlReaderErr::ExpectedStart) } } @@ -212,7 +212,7 @@ impl XmlReader { Ok(reader::XmlEvent::Characters(chars)) => { Ok(chars) } - _ => return Err(XmlReaderErr::ExpectedCharacters) + _ => Err(XmlReaderErr::ExpectedCharacters) } } @@ -349,7 +349,7 @@ impl Attributes { /// Takes a required attribute by name pub fn take_req(&mut self, name: &str) -> Result { self.take_opt(name) - .ok_or(AttributesError::MissingAttribute(name.to_string())) + .ok_or_else(|| AttributesError::MissingAttribute(name.to_string())) } /// Takes a required hexencoded attribute and converts it to Bytes @@ -357,20 +357,19 @@ impl Attributes { -> Result { match hex::decode(self.take_req(name)?) { - Err(e) => return Err(AttributesError::HexError(e)), + Err(e) => Err(AttributesError::HexError(e)), Ok(b) => Ok(Bytes::from(b)) } } /// Verifies that there are no more attributes pub fn exhausted(&self) -> Result<(), AttributesError> { - if self.attributes.len() > 0 { - return Err(AttributesError::ExtraAttributes) + if self.attributes.is_empty() { + Ok(()) + } else { + Err(AttributesError::ExtraAttributes) } - Ok(()) } - - } diff --git a/tests/client_api_publish_at_server.rs b/tests/client_api_publish_at_server.rs index 4568b43c..02d20440 100644 --- a/tests/client_api_publish_at_server.rs +++ b/tests/client_api_publish_at_server.rs @@ -29,7 +29,7 @@ use std::collections::HashSet; use krill::util::httpclient; fn list(server_uri: &str, handle: &str, token: &str) -> apiclient::Options { - let conn = apiclient::Connection::new(server_uri, handle, token).unwrap(); + let conn = apiclient::Connection::build(server_uri, handle, token).unwrap(); let cmd = apiclient::Command::List; let fmt = apiclient::Format::Json; @@ -43,7 +43,7 @@ fn sync( syncdir: &PathBuf, base_uri: &str ) -> apiclient::Options { - let conn = apiclient::Connection::new(server_uri, handle, token).unwrap(); + let conn = apiclient::Connection::build(server_uri, handle, token).unwrap(); let cmd = apiclient::Command::sync(syncdir.to_str().unwrap(), base_uri).unwrap(); let fmt = apiclient::Format::Json; diff --git a/tests/client_cms_publish_at_server.rs b/tests/client_cms_publish_at_server.rs index c7a5fc04..9e5b6364 100644 --- a/tests/client_cms_publish_at_server.rs +++ b/tests/client_cms_publish_at_server.rs @@ -31,7 +31,7 @@ fn client_publish_at_server() { // Set up a client let client_dir = test::create_sub_dir(&d); - let mut client = PubClient::new(&client_dir).unwrap(); + let mut client = PubClient::build(&client_dir).unwrap(); client.init("alice").unwrap(); let pr = client.publisher_request().unwrap(); test::save_pr(&d, "alice.xml", &pr); @@ -85,7 +85,7 @@ fn client_publish_at_server() { ).unwrap(); repo_res.validate().unwrap(); - client.process_repo_response(repo_res).unwrap(); + client.process_repo_response(&repo_res).unwrap(); // List files at server let list = client.get_server_list().unwrap(); diff --git a/tests/manage_publishers.rs b/tests/manage_publishers.rs index 9567a99b..0478368d 100644 --- a/tests/manage_publishers.rs +++ b/tests/manage_publishers.rs @@ -56,7 +56,7 @@ fn manage_publishers() { // Set up a client "alice" { let client_dir = test::create_sub_dir(&d); - let mut client = PubClient::new(&client_dir).unwrap(); + let mut client = PubClient::build(&client_dir).unwrap(); client.init("alice").unwrap(); let pr = client.publisher_request().unwrap(); test::save_pr(&d, "alice.xml", &pr);