Fixes from clippy

This commit is contained in:
Brian Picciano 2024-01-10 10:42:48 +01:00
parent 9c6e3c6240
commit dbb8e442c8
20 changed files with 79 additions and 84 deletions

View File

@ -196,9 +196,11 @@
(buildSystem: targetSystem: let (buildSystem: targetSystem: let
pkgs = mkPkgs buildSystem null; pkgs = mkPkgs buildSystem null;
toolchain = mkToolchain buildSystem targetSystem; toolchain = mkToolchain buildSystem targetSystem;
# make clippy available directly
clippy = pkgs.writeShellScriptBin "clippy" ''exec cargo clippy "$@"'';
in in
pkgs.mkShell ({ pkgs.mkShell ({
inputsFrom = []; # extra packages for dev packages = [ clippy ]; # extra packages for dev
} // (buildEnv buildSystem targetSystem)) } // (buildEnv buildSystem targetSystem))
); );
}; };

View File

@ -18,11 +18,11 @@ pub enum GetHttp01ChallengeKeyError {
#[mockall::automock] #[mockall::automock]
pub trait Manager { pub trait Manager {
fn sync_domain<'mgr>( fn sync_domain(
&'mgr self, &self,
domain: domain::Name, domain: domain::Name,
external_config: Option<domain::ConfigExternalDomain>, external_config: Option<domain::ConfigExternalDomain>,
) -> util::BoxFuture<'mgr, unexpected::Result<()>>; ) -> util::BoxFuture<'_, unexpected::Result<()>>;
fn get_http01_challenge_key(&self, token: &str) -> Result<String, GetHttp01ChallengeKeyError>; fn get_http01_challenge_key(&self, token: &str) -> Result<String, GetHttp01ChallengeKeyError>;
@ -32,17 +32,19 @@ pub trait Manager {
) -> unexpected::Result<Option<(PrivateKey, CertificateChain)>>; ) -> unexpected::Result<Option<(PrivateKey, CertificateChain)>>;
} }
type BoxExternalStoreFn = Box<
dyn Fn(
&domain::ConfigExternalDomain,
) -> unexpected::Result<Box<dyn acme::store::Store + Send + Sync>>
+ Send
+ Sync,
>;
pub struct ManagerImpl { pub struct ManagerImpl {
store: Box<dyn acme::store::Store + Send + Sync>, store: Box<dyn acme::store::Store + Send + Sync>,
token_store: Box<dyn token::Store + Send + Sync>, token_store: Box<dyn token::Store + Send + Sync>,
account: sync::Arc<acme2::Account>, account: sync::Arc<acme2::Account>,
external_store_fn: Box< external_store_fn: BoxExternalStoreFn,
dyn Fn(
&domain::ConfigExternalDomain,
) -> unexpected::Result<Box<dyn acme::store::Store + Send + Sync>>
+ Send
+ Sync,
>,
} }
impl ManagerImpl { impl ManagerImpl {
@ -112,11 +114,11 @@ impl ManagerImpl {
} }
impl Manager for ManagerImpl { impl Manager for ManagerImpl {
fn sync_domain<'mgr>( fn sync_domain(
&'mgr self, &self,
domain: domain::Name, domain: domain::Name,
external_config: Option<domain::ConfigExternalDomain>, external_config: Option<domain::ConfigExternalDomain>,
) -> util::BoxFuture<'mgr, Result<(), unexpected::Error>> { ) -> util::BoxFuture<'_, Result<(), unexpected::Error>> {
Box::pin(async move { Box::pin(async move {
let external_store; let external_store;
let store = match external_config { let store = match external_config {

View File

@ -10,7 +10,7 @@ pub struct DirectFSStore {
} }
impl DirectFSStore { impl DirectFSStore {
pub fn new( key_file_path: &path::Path, cert_file_path: &path::Path,) -> Self { pub fn new(key_file_path: &path::Path, cert_file_path: &path::Path) -> Self {
Self { Self {
key_file_path: key_file_path.into(), key_file_path: key_file_path.into(),
cert_file_path: cert_file_path.into(), cert_file_path: cert_file_path.into(),
@ -53,13 +53,10 @@ impl super::Store for DirectFSStore {
) )
})?; })?;
if key.is_none() != certs.is_none() {
}
match (key, certs) { match (key, certs) {
(None, None) => Ok(None), (None, None) => Ok(None),
(Some(key), Some(certs)) => Ok(Some((key, certs))), (Some(key), Some(certs)) => Ok(Some((key, certs))),
_ => _ =>
Err(unexpected::Error::from(format!( Err(unexpected::Error::from(format!(
"private key file {} and cert file {} are in inconsistent state, one exists but the other doesn't", "private key file {} and cert file {} are in inconsistent state, one exists but the other doesn't",
&self.key_file_path.display(), &self.key_file_path.display(),

View File

@ -73,6 +73,5 @@ impl super::Store for JSONFSStore {
))) )))
} }
.map_unexpected_while(|| format!("path is {}", path.display())) .map_unexpected_while(|| format!("path is {}", path.display()))
.map_err(|err| err.into())
} }
} }

View File

@ -22,15 +22,13 @@ fn addr_from_url(
if let Some(path_and_query) = parsed.path_and_query() { if let Some(path_and_query) = parsed.path_and_query() {
let path_and_query = path_and_query.as_str(); let path_and_query = path_and_query.as_str();
if path_and_query != "" && path_and_query != "/" { if !path_and_query.is_empty() && path_and_query != "/" {
return Err(unexpected::Error::from( return Err(unexpected::Error::from("path must be empty"));
format!("path must be empty").as_str(),
));
} }
} }
match parsed.authority() { match parsed.authority() {
None => Err(unexpected::Error::from(format!("host is missing").as_str())), None => Err(unexpected::Error::from("host is missing")),
Some(authority) => { Some(authority) => {
let port = authority.port().map(|p| p.as_u16()).unwrap_or(default_port); let port = authority.port().map(|p| p.as_u16()).unwrap_or(default_port);
Ok(format!("{}:{port}", authority.host())) Ok(format!("{}:{port}", authority.host()))
@ -90,15 +88,9 @@ pub struct HttpRequestHeader {
value: String, value: String,
} }
#[derive(Clone)] #[derive(Clone, Default)]
pub struct HttpRequestHeaders(pub http::header::HeaderMap); pub struct HttpRequestHeaders(pub http::header::HeaderMap);
impl Default for HttpRequestHeaders {
fn default() -> Self {
Self(http::header::HeaderMap::default())
}
}
impl TryFrom<HttpRequestHeaders> for Vec<HttpRequestHeader> { impl TryFrom<HttpRequestHeaders> for Vec<HttpRequestHeader> {
type Error = http::header::ToStrError; type Error = http::header::ToStrError;

View File

@ -86,6 +86,6 @@ impl Store for FSStore {
serde_json::to_writer(file, &stored).or_unexpected_while("writing cert to file")?; serde_json::to_writer(file, &stored).or_unexpected_while("writing cert to file")?;
return Ok(()); Ok(())
} }
} }

View File

@ -124,17 +124,17 @@ pub struct ManagedDomain {
pub trait Manager: Sync + Send { pub trait Manager: Sync + Send {
fn get_settings(&self, domain: &domain::Name) -> Result<GetSettingsResult, GetSettingsError>; fn get_settings(&self, domain: &domain::Name) -> Result<GetSettingsResult, GetSettingsError>;
fn get_file<'store>( fn get_file(
&'store self, &self,
domain: &domain::Name, domain: &domain::Name,
path: &str, path: &str,
) -> Result<util::BoxByteStream, GetFileError>; ) -> Result<util::BoxByteStream, GetFileError>;
fn sync_with_settings<'mgr>( fn sync_with_settings(
&'mgr self, &self,
domain: domain::Name, domain: domain::Name,
settings: domain::Settings, settings: domain::Settings,
) -> util::BoxFuture<'mgr, Result<(), SyncWithSettingsError>>; ) -> util::BoxFuture<'_, Result<(), SyncWithSettingsError>>;
fn get_acme_http01_challenge_key( fn get_acme_http01_challenge_key(
&self, &self,
@ -206,7 +206,11 @@ impl ManagerImpl {
fn sync_domain_gemini_cert(&self, domain: &domain::Name) -> unexpected::Result<()> { fn sync_domain_gemini_cert(&self, domain: &domain::Name) -> unexpected::Result<()> {
if let Some(ref gemini_store) = self.gemini_store { if let Some(ref gemini_store) = self.gemini_store {
log::info!("Syncing gemini certificate for domain {domain}"); log::info!("Syncing gemini certificate for domain {domain}");
if let Some(_) = gemini_store.get_certificate(domain).or_unexpected()? { if gemini_store
.get_certificate(domain)
.or_unexpected()?
.is_some()
{
return Ok(()); return Ok(());
} }
@ -332,8 +336,8 @@ impl Manager for ManagerImpl {
Ok(GetSettingsResult::Stored(self.domain_store.get(domain)?)) Ok(GetSettingsResult::Stored(self.domain_store.get(domain)?))
} }
fn get_file<'store>( fn get_file(
&'store self, &self,
domain: &domain::Name, domain: &domain::Name,
path: &str, path: &str,
) -> Result<util::BoxByteStream, GetFileError> { ) -> Result<util::BoxByteStream, GetFileError> {
@ -364,11 +368,11 @@ impl Manager for ManagerImpl {
Ok(f) Ok(f)
} }
fn sync_with_settings<'mgr>( fn sync_with_settings(
&'mgr self, &self,
domain: domain::Name, domain: domain::Name,
settings: domain::Settings, settings: domain::Settings,
) -> util::BoxFuture<'mgr, Result<(), SyncWithSettingsError>> { ) -> util::BoxFuture<'_, Result<(), SyncWithSettingsError>> {
Box::pin(async move { Box::pin(async move {
let is_interface = Some(&domain) == self.config.interface_domain.as_ref(); let is_interface = Some(&domain) == self.config.interface_domain.as_ref();
let is_builtin = self.config.builtin_domains.contains_key(&domain); let is_builtin = self.config.builtin_domains.contains_key(&domain);
@ -440,8 +444,8 @@ impl Manager for ManagerImpl {
self.config self.config
.proxied_domains .proxied_domains
.iter() .keys()
.map(|(domain, _)| ManagedDomain { .map(|domain| ManagedDomain {
domain: domain.clone(), domain: domain.clone(),
public: false, public: false,
}) })
@ -456,8 +460,8 @@ impl Manager for ManagerImpl {
self.config self.config
.external_domains .external_domains
.iter() .keys()
.map(|(domain, _)| ManagedDomain { .map(|domain| ManagedDomain {
domain: domain.clone(), domain: domain.clone(),
public: false, public: false,
}) })

View File

@ -24,11 +24,11 @@ impl Settings {
Ok(h.finalize().encode_hex::<String>()) Ok(h.finalize().encode_hex::<String>())
} }
fn add_path_prefix<'path, 'prefix>( fn add_path_prefix<'path>(
path: borrow::Cow<'path, str>, path: borrow::Cow<'path, str>,
prefix: &'prefix str, prefix: &str,
) -> borrow::Cow<'path, str> { ) -> borrow::Cow<'path, str> {
if prefix.len() == 0 { if prefix.is_empty() {
return path; return path;
} }

View File

@ -81,9 +81,9 @@ impl Store for FSStore {
"couldn't convert os string to &str", "couldn't convert os string to &str",
))?; ))?;
Ok(domain domain
.parse() .parse()
.map_unexpected_while(|| format!("parsing {domain} as domain name"))?) .map_unexpected_while(|| format!("parsing {domain} as domain name"))
}, },
) )
.try_collect() .try_collect()

View File

@ -71,10 +71,9 @@ impl Certificate {
let cert = builder.build(); let cert = builder.build();
Ok(cert cert.as_ref()
.as_ref()
.try_into() .try_into()
.or_unexpected_while("converting to cert")?) .or_unexpected_while("converting to cert")
} }
} }

View File

@ -53,7 +53,7 @@ async fn main() {
}) })
}; };
// inteface_cname is a CNAME record which points to the interface domain of the service. // interface_cname is a CNAME record which points to the interface domain of the service.
// Since the interface domain _must_ point to the service (otherwise it wouldn't work) it's // Since the interface domain _must_ point to the service (otherwise it wouldn't work) it's
// reasonable to assume that a CNAME on any domain would suffice to point that domain to // reasonable to assume that a CNAME on any domain would suffice to point that domain to
// the service. // the service.
@ -74,7 +74,7 @@ async fn main() {
} }
} }
if config.domain.external_domains.len() > 0 && config.service.http.https_addr.is_none() { if !config.domain.external_domains.is_empty() && config.service.http.https_addr.is_none() {
panic!("https must be enabled to use external_domains") panic!("https must be enabled to use external_domains")
} }

View File

@ -68,11 +68,7 @@ impl Store for sync::Arc<sync::Mutex<MockStore>> {
self.lock().unwrap().all_descrs() self.lock().unwrap().all_descrs()
} }
fn get_file<'store>( fn get_file(&self, descr: &Descr, path: &str) -> Result<util::BoxByteStream, GetFileError> {
&'store self,
descr: &Descr,
path: &str,
) -> Result<util::BoxByteStream, GetFileError> {
self.lock().unwrap().get_file(descr, path) self.lock().unwrap().get_file(descr, path)
} }
} }

View File

@ -127,7 +127,7 @@ impl FSStore {
.map_err(|e| match e { .map_err(|e| match e {
// it's not expected that the branch name is invalid at this point, it must have // it's not expected that the branch name is invalid at this point, it must have
// existed for sync to have been successful. // existed for sync to have been successful.
CreateRepoSnapshotError::InvalidBranchName => e.into_unexpected().into(), CreateRepoSnapshotError::InvalidBranchName => e.into_unexpected(),
CreateRepoSnapshotError::Unexpected(e) => e, CreateRepoSnapshotError::Unexpected(e) => e,
})?; })?;
@ -293,8 +293,8 @@ impl super::Store for FSStore {
).try_collect() ).try_collect()
} }
fn get_file<'store>( fn get_file(
&'store self, &self,
descr: &origin::Descr, descr: &origin::Descr,
path: &str, path: &str,
) -> Result<util::BoxByteStream, origin::GetFileError> { ) -> Result<util::BoxByteStream, origin::GetFileError> {

View File

@ -41,8 +41,8 @@ where
Ok(res) Ok(res)
} }
fn get_file<'store>( fn get_file(
&'store self, &self,
descr: &origin::Descr, descr: &origin::Descr,
path: &str, path: &str,
) -> Result<util::BoxByteStream, origin::GetFileError> { ) -> Result<util::BoxByteStream, origin::GetFileError> {

View File

@ -6,11 +6,8 @@ pub use config::*;
use std::borrow; use std::borrow;
fn append_index_to_path<'path, 'index>( fn append_index_to_path<'path>(path: &'path str, index: &str) -> borrow::Cow<'path, str> {
path: &'path str, if path.is_empty() {
index: &'index str,
) -> borrow::Cow<'path, str> {
if path.len() == 0 {
let mut path = String::with_capacity(1 + index.len()); let mut path = String::with_capacity(1 + index.len());
path.push('/'); path.push('/');
path.push_str(index); path.push_str(index);

View File

@ -128,7 +128,7 @@ impl Service {
if self.config.http.https_addr.is_some() { if self.config.http.https_addr.is_some() {
return "https"; return "https";
} }
return "http"; "http"
} }
fn render_error_page(&self, status_code: u16, e: &str) -> Response<Body> { fn render_error_page(&self, status_code: u16, e: &str) -> Response<Body> {
@ -212,7 +212,7 @@ impl Service {
match self.domain_manager.get_file(&domain, &path) { match self.domain_manager.get_file(&domain, &path) {
Ok(f) => self.serve(200, &path, Body::wrap_stream(f)), Ok(f) => self.serve(200, &path, Body::wrap_stream(f)),
Err(domain::manager::GetFileError::DomainNotFound) => { Err(domain::manager::GetFileError::DomainNotFound) => {
return self.render_error_page(404, "Unknown domain name") self.render_error_page(404, "Unknown domain name")
} }
Err(domain::manager::GetFileError::FileNotFound) => { Err(domain::manager::GetFileError::FileNotFound) => {
self.render_error_page(404, "File not found") self.render_error_page(404, "File not found")
@ -267,7 +267,7 @@ impl Service {
let settings = match self.domain_manager.get_settings(&args.domain) { let settings = match self.domain_manager.get_settings(&args.domain) {
Ok(domain::manager::GetSettingsResult::Stored(settings)) => Some(settings), Ok(domain::manager::GetSettingsResult::Stored(settings)) => Some(settings),
Ok(domain::manager::GetSettingsResult::Builtin(config)) => { Ok(domain::manager::GetSettingsResult::Builtin(config)) => {
config.public.then(|| config.settings) config.public.then_some(config.settings)
} }
Ok(domain::manager::GetSettingsResult::Proxied(_)) => None, Ok(domain::manager::GetSettingsResult::Proxied(_)) => None,
Ok(domain::manager::GetSettingsResult::Interface) => None, Ok(domain::manager::GetSettingsResult::Interface) => None,
@ -318,10 +318,11 @@ impl Service {
} }
}; };
let dns_records_have_cname = self.config.dns_records.iter().any(|r| match r { let dns_records_have_cname = self
service::ConfigDNSRecord::CNAME { .. } => true, .config
_ => false, .dns_records
}); .iter()
.any(|r| matches!(r, service::ConfigDNSRecord::CNAME { .. }));
let url_encoded_domain_settings = match settings.try_into() { let url_encoded_domain_settings = match settings.try_into() {
Ok(s) => s, Ok(s) => s,

View File

@ -9,7 +9,7 @@ pub async fn serve_http_request(
req_is_https: bool, req_is_https: bool,
) -> unexpected::Result<hyper::Response<hyper::Body>> { ) -> unexpected::Result<hyper::Response<hyper::Body>> {
for (name, value) in headers { for (name, value) in headers {
if value == "" { if value.is_empty() {
req.headers_mut().remove(name); req.headers_mut().remove(name);
continue; continue;
} }

View File

@ -12,7 +12,7 @@ pub async fn listen_http(
service: sync::Arc<service::http::Service>, service: sync::Arc<service::http::Service>,
canceller: CancellationToken, canceller: CancellationToken,
) -> Result<(), unexpected::Error> { ) -> Result<(), unexpected::Error> {
let addr = service.config.http.http_addr.clone(); let addr = service.config.http.http_addr;
// only used for logging // only used for logging
let listen_host = service let listen_host = service
@ -51,7 +51,7 @@ pub async fn listen_https(
canceller: CancellationToken, canceller: CancellationToken,
) -> Result<(), unexpected::Error> { ) -> Result<(), unexpected::Error> {
let cert_resolver = service.cert_resolver.clone(); let cert_resolver = service.cert_resolver.clone();
let addr = service.config.http.https_addr.unwrap().clone(); let addr = service.config.http.https_addr.unwrap();
// only used for logging // only used for logging
let listen_host = service let listen_host = service

View File

@ -20,6 +20,12 @@ impl MemStore {
} }
} }
impl Default for MemStore {
fn default() -> Self {
Self::new()
}
}
impl Store for MemStore { impl Store for MemStore {
fn get(&self, key: &str) -> unexpected::Result<Option<String>> { fn get(&self, key: &str) -> unexpected::Result<Option<String>> {
Ok(self.m.lock().unwrap().get(key).map(|s| s.to_string())) Ok(self.m.lock().unwrap().get(key).map(|s| s.to_string()))

View File

@ -23,7 +23,7 @@ pub fn parse_file<T: std::str::FromStr>(
path: &path::Path, path: &path::Path,
) -> Result<Option<T>, ParseFileError<T::Err>> { ) -> Result<Option<T>, ParseFileError<T::Err>> {
match fs::read_to_string(path) { match fs::read_to_string(path) {
Ok(s) => Ok(Some(s.parse().map_err(|e| ParseFileError::FromStr(e))?)), Ok(s) => Ok(Some(s.parse().map_err(ParseFileError::FromStr)?)),
Err(err) => match err.kind() { Err(err) => match err.kind() {
io::ErrorKind::NotFound => Ok(None), io::ErrorKind::NotFound => Ok(None),
_ => Err(ParseFileError::IO(err)), _ => Err(ParseFileError::IO(err)),