From 6a43cbd6200518a25d34e6e4756517721f48e751 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Thu, 15 Feb 2024 22:56:35 +0000 Subject: [PATCH 1/7] Use async for filesystem hits Except `Directory`, as there's no async-compatible `glob` implementation --- Cargo.toml | 4 +- src/file.rs | 17 ++--- src/paste.rs | 129 +++++++++++++++++++++----------------- src/server.rs | 168 +++++++++++++++++++++++++++----------------------- src/util.rs | 24 +++++--- 5 files changed, 192 insertions(+), 150 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 571e11aa..5b0ae548 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ include = ["src/**/*", "Cargo.*", "LICENSE", "README.md", "CHANGELOG.md"] default = ["rustls"] openssl = ["actix-web/openssl", "awc/openssl"] rustls = ["actix-web/rustls-0_21", "awc/rustls-0_21"] -shuttle = ["dep:shuttle-actix-web", "dep:shuttle-runtime", "dep:tokio"] +shuttle = ["dep:shuttle-actix-web", "dep:shuttle-runtime"] [dependencies] actix-web = { version = "4.4.1" } @@ -44,7 +44,7 @@ humantime-serde = "1.1.1" glob = "0.3.1" ring = "0.17.7" hotwatch = "0.5.0" -tokio = { version = "1.35.1", optional = true } +tokio = { version = "1.35.1", features = ["fs"] } tracing = "0.1.40" tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } uts2ts = "0.4.1" diff --git a/src/file.rs b/src/file.rs index 0d45b19e..375f71f7 100644 --- a/src/file.rs +++ b/src/file.rs @@ -1,5 +1,4 @@ use crate::util; -use actix_web::{error, Error as ActixError}; use glob::glob; use std::convert::TryFrom; use std::fs::File as OsFile; @@ -21,12 +20,16 @@ pub struct Directory { } impl<'a> TryFrom<&'a Path> for Directory { - type Error = ActixError; + type Error = String; fn try_from(directory: &'a Path) -> Result { - let files = glob(directory.join("**").join("*").to_str().ok_or_else(|| { - error::ErrorInternalServerError("directory contains invalid characters") - })?) - .map_err(error::ErrorInternalServerError)? + let files = glob( + directory + .join("**") + .join("*") + .to_str() + .ok_or_else(|| String::from("directory contains invalid characters"))?, + ) + .map_err(|e| e.msg)? .filter_map(Result::ok) .filter(|path| !path.is_dir()) .filter_map(|path| match OsFile::open(&path) { @@ -58,7 +61,7 @@ mod tests { use std::ffi::OsString; #[test] - fn test_file_checksum() -> Result<(), ActixError> { + fn test_file_checksum() -> Result<(), String> { assert_eq!( Some(OsString::from("rustypaste_logo.png").as_ref()), Directory::try_from( diff --git a/src/paste.rs b/src/paste.rs index 7a1c27cf..2d55d01c 100644 --- a/src/paste.rs +++ b/src/paste.rs @@ -5,11 +5,13 @@ use crate::util; use actix_web::{error, Error}; use awc::Client; use std::convert::{TryFrom, TryInto}; -use std::fs::{self, File}; -use std::io::{Error as IoError, ErrorKind as IoErrorKind, Result as IoResult, Write}; +use std::io::{Error as IoError, ErrorKind as IoErrorKind, Result as IoResult}; use std::path::{Path, PathBuf}; use std::str; use std::sync::RwLock; +use tokio::fs::{self, File}; +use tokio::io::AsyncWriteExt; +use tokio::task::spawn_blocking; use url::Url; /// Type of the data to store. @@ -92,7 +94,7 @@ impl Paste { /// /// [`default_extension`]: crate::config::PasteConfig::default_extension /// [`random_url.enabled`]: crate::random::RandomURLConfig::enabled - pub fn store_file( + pub async fn store_file( &self, file_name: &str, expiry_date: Option, @@ -179,8 +181,8 @@ impl Paste { if let Some(timestamp) = expiry_date { path.set_file_name(format!("{file_name}.{timestamp}")); } - let mut buffer = File::create(&path)?; - buffer.write_all(&self.data)?; + let mut buffer = File::create(&path).await?; + buffer.write_all(&self.data).await?; Ok(file_name) } @@ -229,9 +231,16 @@ impl Paste { let bytes_checksum = util::sha256_digest(&*bytes)?; self.data = bytes; if !config.paste.duplicate_files.unwrap_or(true) && expiry_date.is_none() { - if let Some(file) = - Directory::try_from(config.server.upload_path.as_path())?.get_file(bytes_checksum) - { + let upload_path = config.server.upload_path.clone(); + + let directory = + match spawn_blocking(move || Directory::try_from(upload_path.as_path())).await { + Ok(Ok(d)) => d, + Ok(Err(e)) => return Err(error::ErrorInternalServerError(e)), + Err(e) => return Err(error::ErrorInternalServerError(e)), + }; + + if let Some(file) = directory.get_file(bytes_checksum) { return Ok(file .path .file_name() @@ -240,7 +249,9 @@ impl Paste { .to_string()); } } - Ok(self.store_file(file_name, expiry_date, None, &config)?) + Ok(self + .store_file(file_name, expiry_date, None, &config) + .await?) } /// Writes an URL to a file in upload directory. @@ -250,7 +261,7 @@ impl Paste { /// /// [`random_url.enabled`]: crate::random::RandomURLConfig::enabled #[allow(deprecated)] - pub fn store_url(&self, expiry_date: Option, config: &Config) -> IoResult { + pub async fn store_url(&self, expiry_date: Option, config: &Config) -> IoResult { let data = str::from_utf8(&self.data) .map_err(|e| IoError::new(IoErrorKind::Other, e.to_string()))?; let url = Url::parse(data).map_err(|e| IoError::new(IoErrorKind::Other, e.to_string()))?; @@ -265,7 +276,7 @@ impl Paste { if let Some(timestamp) = expiry_date { path.set_file_name(format!("{file_name}.{timestamp}")); } - fs::write(&path, url.to_string())?; + fs::write(&path, url.to_string()).await?; Ok(file_name) } } @@ -298,15 +309,15 @@ mod tests { data: vec![65, 66, 67], type_: PasteType::File, }; - let file_name = paste.store_file("test.txt", None, None, &config)?; - assert_eq!("ABC", fs::read_to_string(&file_name)?); + let file_name = paste.store_file("test.txt", None, None, &config).await?; + assert_eq!("ABC", fs::read_to_string(&file_name).await?); assert_eq!( Some("txt"), PathBuf::from(&file_name) .extension() .and_then(|v| v.to_str()) ); - fs::remove_file(file_name)?; + fs::remove_file(file_name).await?; config.paste.random_url = Some(RandomURLConfig { length: Some(4), @@ -318,11 +329,11 @@ mod tests { data: vec![116, 101, 115, 115, 117, 115], type_: PasteType::File, }; - let file_name = paste.store_file("foo.tar.gz", None, None, &config)?; - assert_eq!("tessus", fs::read_to_string(&file_name)?); + let file_name = paste.store_file("foo.tar.gz", None, None, &config).await?; + assert_eq!("tessus", fs::read_to_string(&file_name).await?); assert!(file_name.ends_with(".tar.gz")); assert!(file_name.starts_with("foo.")); - fs::remove_file(file_name)?; + fs::remove_file(file_name).await?; config.paste.random_url = Some(RandomURLConfig { length: Some(4), @@ -334,11 +345,11 @@ mod tests { data: vec![116, 101, 115, 115, 117, 115], type_: PasteType::File, }; - let file_name = paste.store_file(".foo.tar.gz", None, None, &config)?; - assert_eq!("tessus", fs::read_to_string(&file_name)?); + let file_name = paste.store_file(".foo.tar.gz", None, None, &config).await?; + assert_eq!("tessus", fs::read_to_string(&file_name).await?); assert!(file_name.ends_with(".tar.gz")); assert!(file_name.starts_with(".foo.")); - fs::remove_file(file_name)?; + fs::remove_file(file_name).await?; config.paste.random_url = Some(RandomURLConfig { length: Some(4), @@ -350,10 +361,10 @@ mod tests { data: vec![116, 101, 115, 115, 117, 115], type_: PasteType::File, }; - let file_name = paste.store_file("foo.tar.gz", None, None, &config)?; - assert_eq!("tessus", fs::read_to_string(&file_name)?); + let file_name = paste.store_file("foo.tar.gz", None, None, &config).await?; + assert_eq!("tessus", fs::read_to_string(&file_name).await?); assert!(file_name.ends_with(".tar.gz")); - fs::remove_file(file_name)?; + fs::remove_file(file_name).await?; config.paste.default_extension = String::from("txt"); config.paste.random_url = None; @@ -361,10 +372,10 @@ mod tests { data: vec![120, 121, 122], type_: PasteType::File, }; - let file_name = paste.store_file(".foo", None, None, &config)?; - assert_eq!("xyz", fs::read_to_string(&file_name)?); + let file_name = paste.store_file(".foo", None, None, &config).await?; + assert_eq!("xyz", fs::read_to_string(&file_name).await?); assert_eq!(".foo.txt", file_name); - fs::remove_file(file_name)?; + fs::remove_file(file_name).await?; config.paste.default_extension = String::from("bin"); config.paste.random_url = Some(RandomURLConfig { @@ -376,15 +387,15 @@ mod tests { data: vec![120, 121, 122], type_: PasteType::File, }; - let file_name = paste.store_file("random", None, None, &config)?; - assert_eq!("xyz", fs::read_to_string(&file_name)?); + let file_name = paste.store_file("random", None, None, &config).await?; + assert_eq!("xyz", fs::read_to_string(&file_name).await?); assert_eq!( Some("bin"), PathBuf::from(&file_name) .extension() .and_then(|v| v.to_str()) ); - fs::remove_file(file_name)?; + fs::remove_file(file_name).await?; config.paste.random_url = Some(RandomURLConfig { length: Some(4), @@ -396,15 +407,17 @@ mod tests { data: vec![116, 101, 115, 115, 117, 115], type_: PasteType::File, }; - let file_name = paste.store_file( - "filename.txt", - None, - Some("fn_from_header.txt".to_string()), - &config, - )?; - assert_eq!("tessus", fs::read_to_string(&file_name)?); + let file_name = paste + .store_file( + "filename.txt", + None, + Some("fn_from_header.txt".to_string()), + &config, + ) + .await?; + assert_eq!("tessus", fs::read_to_string(&file_name).await?); assert_eq!("fn_from_header.txt", file_name); - fs::remove_file(file_name)?; + fs::remove_file(file_name).await?; config.paste.random_url = Some(RandomURLConfig { length: Some(4), @@ -416,22 +429,25 @@ mod tests { data: vec![116, 101, 115, 115, 117, 115], type_: PasteType::File, }; - let file_name = paste.store_file( - "filename.txt", - None, - Some("fn_from_header".to_string()), - &config, - )?; - assert_eq!("tessus", fs::read_to_string(&file_name)?); + let file_name = paste + .store_file( + "filename.txt", + None, + Some("fn_from_header".to_string()), + &config, + ) + .await?; + assert_eq!("tessus", fs::read_to_string(&file_name).await?); assert_eq!("fn_from_header", file_name); - fs::remove_file(file_name)?; + fs::remove_file(file_name).await?; for paste_type in &[PasteType::Url, PasteType::Oneshot] { fs::create_dir_all( paste_type .get_path(&config.server.upload_path) .expect("Bad upload path"), - )?; + ) + .await?; } config.paste.random_url = None; @@ -440,13 +456,15 @@ mod tests { type_: PasteType::Oneshot, }; let expiry_date = util::get_system_time()?.as_millis() + 100; - let file_name = paste.store_file("test.file", Some(expiry_date), None, &config)?; + let file_name = paste + .store_file("test.file", Some(expiry_date), None, &config) + .await?; let file_path = PasteType::Oneshot .get_path(&config.server.upload_path) .expect("Bad upload path") .join(format!("{file_name}.{expiry_date}")); - assert_eq!("test", fs::read_to_string(&file_path)?); - fs::remove_file(file_path)?; + assert_eq!("test", fs::read_to_string(&file_path).await?); + fs::remove_file(file_path).await?; config.paste.random_url = Some(RandomURLConfig { enabled: Some(true), @@ -457,20 +475,20 @@ mod tests { data: url.as_bytes().to_vec(), type_: PasteType::Url, }; - let file_name = paste.store_url(None, &config)?; + let file_name = paste.store_url(None, &config).await?; let file_path = PasteType::Url .get_path(&config.server.upload_path) .expect("Bad upload path") .join(&file_name); - assert_eq!(url, fs::read_to_string(&file_path)?); - fs::remove_file(file_path)?; + assert_eq!(url, fs::read_to_string(&file_path).await?); + fs::remove_file(file_path).await?; let url = String::from("testurl.com"); let paste = Paste { data: url.as_bytes().to_vec(), type_: PasteType::Url, }; - assert!(paste.store_url(None, &config).is_err()); + assert!(paste.store_url(None, &config).await.is_err()); config.server.max_content_length = Byte::from_str("30k").expect("cannot parse byte"); let url = String::from("https://upload.wikimedia.org/wikipedia/en/a/a9/Example.jpg"); @@ -494,14 +512,15 @@ mod tests { "70ff72a2f7651b5fae3aa9834e03d2a2233c52036610562f7fa04e089e8198ed", util::sha256_digest(&*paste.data)? ); - fs::remove_file(file_path)?; + fs::remove_file(file_path).await?; for paste_type in &[PasteType::Url, PasteType::Oneshot] { fs::remove_dir( paste_type .get_path(&config.server.upload_path) .expect("Bad upload path"), - )?; + ) + .await?; } Ok(()) diff --git a/src/server.rs b/src/server.rs index 2abbf854..61932d2f 100644 --- a/src/server.rs +++ b/src/server.rs @@ -18,10 +18,10 @@ use mime::TEXT_PLAIN_UTF_8; use serde::{Deserialize, Serialize}; use std::convert::TryFrom; use std::env; -use std::fs; use std::path::PathBuf; use std::sync::RwLock; use std::time::Duration; +use tokio::fs; use uts2ts; /// Shows the landing page. @@ -53,7 +53,7 @@ async fn index(config: web::Data>) -> Result } if let Some(mut landing_page) = config.landing_page { if let Some(file) = landing_page.file { - landing_page.text = fs::read_to_string(file).ok(); + landing_page.text = fs::read_to_string(file).await.ok(); } match landing_page.text { Some(page) => Ok(HttpResponse::Ok() @@ -89,12 +89,13 @@ async fn serve( let config = config .read() .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; - let mut path = util::glob_match_file(safe_path_join(&config.server.upload_path, &*file)?)?; + let mut path = + util::glob_match_file(safe_path_join(&config.server.upload_path, &*file)?).await?; let mut paste_type = PasteType::File; if !path.exists() || path.is_dir() { for type_ in &[PasteType::Url, PasteType::Oneshot, PasteType::OneshotUrl] { let alt_path = safe_path_join(type_.get_path(&config.server.upload_path)?, &*file)?; - let alt_path = util::glob_match_file(alt_path)?; + let alt_path = util::glob_match_file(alt_path).await?; if alt_path.exists() || path.file_name().and_then(|v| v.to_str()) == Some(&type_.get_dir()) { @@ -128,21 +129,23 @@ async fn serve( file, util::get_system_time()?.as_millis() )), - )?; + ) + .await?; } Ok(response) } PasteType::Url => Ok(HttpResponse::Found() - .append_header(("Location", fs::read_to_string(&path)?)) + .append_header(("Location", fs::read_to_string(&path).await?)) .finish()), PasteType::OneshotUrl => { let resp = HttpResponse::Found() - .append_header(("Location", fs::read_to_string(&path)?)) + .append_header(("Location", fs::read_to_string(&path).await?)) .finish(); fs::rename( &path, path.with_file_name(format!("{}.{}", file, util::get_system_time()?.as_millis())), - )?; + ) + .await?; Ok(resp) } } @@ -158,11 +161,11 @@ async fn delete( let config = config .read() .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; - let path = util::glob_match_file(safe_path_join(&config.server.upload_path, &*file)?)?; + let path = util::glob_match_file(safe_path_join(&config.server.upload_path, &*file)?).await?; if !path.is_file() || !path.exists() { return Err(error::ErrorNotFound("file is not found or expired :(\n")); } - match fs::remove_file(path) { + match fs::remove_file(path).await { Ok(_) => info!("deleted file: {:?}", file.to_string()), Err(e) => { error!("cannot delete file: {}", e); @@ -250,7 +253,8 @@ async fn upload( let config = config .read() .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; - if let Some(file) = Directory::try_from(config.server.upload_path.as_path())? + if let Some(file) = Directory::try_from(config.server.upload_path.as_path()) + .map_err(error::ErrorInternalServerError)? .get_file(bytes_checksum) { urls.push(format!( @@ -273,12 +277,14 @@ async fn upload( let config = config .read() .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; - paste.store_file( - content.get_file_name()?, - expiry_date, - header_filename, - &config, - )? + paste + .store_file( + content.get_file_name()?, + expiry_date, + header_filename, + &config, + ) + .await? } PasteType::RemoteFile => { paste @@ -289,7 +295,7 @@ async fn upload( let config = config .read() .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; - paste.store_url(expiry_date, &config)? + paste.store_url(expiry_date, &config).await? } }; info!( @@ -334,49 +340,57 @@ async fn list(config: web::Data>) -> Result .read() .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))? .clone(); + if !config.server.expose_list.unwrap_or(false) { warn!("server is not configured to expose list endpoint"); - Err(error::ErrorNotFound(""))?; + return Err(error::ErrorNotFound("")); } - let entries: Vec = fs::read_dir(config.server.upload_path)? - .filter_map(|entry| { - entry.ok().and_then(|e| { - let metadata = match e.metadata() { - Ok(metadata) => { - if metadata.is_dir() { - return None; - } - metadata - } - Err(e) => { - error!("failed to read metadata: {e}"); - return None; - } - }; - let mut file_name = PathBuf::from(e.file_name()); - let expires_at_utc = if let Some(expiration) = file_name - .extension() - .and_then(|ext| ext.to_str()) - .and_then(|v| v.parse::().ok()) - { - file_name.set_extension(""); - if util::get_system_time().ok()? - > Duration::from_millis(expiration.try_into().ok()?) - { - return None; - } - Some(uts2ts::uts2ts(expiration / 1000).as_string()) - } else { - None - }; - Some(ListItem { - file_name, - file_size: metadata.len(), - expires_at_utc, - }) - }) - }) - .collect(); + + let mut entries: Vec = Vec::new(); + + let mut dir_contents = fs::read_dir(config.server.upload_path).await?; + + let system_time = util::get_system_time()?; + + while let Ok(Some(entry)) = dir_contents.next_entry().await { + let metadata = match entry.metadata().await { + Ok(metadata) if metadata.is_dir() => continue, + Ok(metadata) => metadata, + Err(e) => { + error!("failed to read metadata: {e}"); + continue; + } + }; + + let mut file_name = PathBuf::from(entry.file_name()); + + let expires_at_utc = match file_name + .extension() + .and_then(|ext| ext.to_str()) + .and_then(|v| v.parse::().ok()) + { + Some(expiration) if system_time > Duration::from_millis(expiration) => continue, + Some(expiration) => { + file_name.set_extension(""); + Some( + uts2ts::uts2ts( + (expiration / 1000) + .try_into() + .map_err(|_| error::ErrorInternalServerError("Invalid timestamp"))?, + ) + .as_string(), + ) + } + None => None, + }; + + entries.push(ListItem { + file_name, + file_size: metadata.len(), + expires_at_utc, + }); + } + Ok(HttpResponse::Ok().json(entries)) } @@ -523,7 +537,7 @@ mod tests { let response = test::call_service(&app, request).await; assert_eq!(StatusCode::OK, response.status()); assert_body(response.into_body(), "landing page from file").await?; - fs::remove_file(filename)?; + fs::remove_file(filename).await?; Ok(()) } @@ -626,7 +640,7 @@ mod tests { config.server.expose_list = Some(true); let test_upload_dir = "test_upload"; - fs::create_dir(test_upload_dir)?; + fs::create_dir(test_upload_dir).await?; config.server.upload_path = PathBuf::from(test_upload_dir); let app = test::init_service( @@ -657,7 +671,7 @@ mod tests { PathBuf::from(filename) ); - fs::remove_dir_all(test_upload_dir)?; + fs::remove_dir_all(test_upload_dir).await?; Ok(()) } @@ -668,7 +682,7 @@ mod tests { config.server.expose_list = Some(true); let test_upload_dir = "test_upload"; - fs::create_dir(test_upload_dir)?; + fs::create_dir(test_upload_dir).await?; config.server.upload_path = PathBuf::from(test_upload_dir); let app = test::init_service( @@ -702,7 +716,7 @@ mod tests { assert!(result.is_empty()); - fs::remove_dir_all(test_upload_dir)?; + fs::remove_dir_all(test_upload_dir).await?; Ok(()) } @@ -847,7 +861,7 @@ mod tests { assert_eq!(StatusCode::OK, response.status()); assert_body(response.into_body(), ×tamp).await?; - fs::remove_file(file_name)?; + fs::remove_file(file_name).await?; let serve_request = TestRequest::get() .uri(&format!("/{file_name}")) .to_request(); @@ -897,7 +911,7 @@ mod tests { assert_eq!(StatusCode::OK, response.status()); assert_body(response.into_body(), ×tamp).await?; - fs::remove_file(header_filename)?; + fs::remove_file(header_filename).await?; let serve_request = TestRequest::get() .uri(&format!("/{header_filename}")) .to_request(); @@ -911,7 +925,7 @@ mod tests { #[allow(deprecated)] async fn test_upload_duplicate_file() -> Result<(), Error> { let test_upload_dir = "test_upload"; - fs::create_dir(test_upload_dir)?; + fs::create_dir(test_upload_dir).await?; let mut config = Config::default(); config.server.upload_path = PathBuf::from(&test_upload_dir); @@ -950,7 +964,7 @@ mod tests { assert_eq!(first_body_bytes, second_body_bytes); - fs::remove_dir_all(test_upload_dir)?; + fs::remove_dir_all(test_upload_dir).await?; Ok(()) } @@ -1006,7 +1020,7 @@ mod tests { .map_err(error::ErrorInternalServerError)? .next() { - fs::remove_file(glob_path.map_err(error::ErrorInternalServerError)?)?; + fs::remove_file(glob_path.map_err(error::ErrorInternalServerError)?).await?; } Ok(()) @@ -1061,7 +1075,7 @@ mod tests { util::sha256_digest(&*body_bytes)? ); - fs::remove_file(file_name)?; + fs::remove_file(file_name).await?; let serve_request = TestRequest::get() .uri(&format!("/{file_name}")) @@ -1088,7 +1102,7 @@ mod tests { let url_upload_path = PasteType::Url .get_path(&config.server.upload_path) .expect("Bad upload path"); - fs::create_dir_all(&url_upload_path)?; + fs::create_dir_all(&url_upload_path).await?; let response = test::call_service( &app, @@ -1102,8 +1116,8 @@ mod tests { let response = test::call_service(&app, serve_request).await; assert_eq!(StatusCode::FOUND, response.status()); - fs::remove_file(url_upload_path.join("url"))?; - fs::remove_dir(url_upload_path)?; + fs::remove_file(url_upload_path.join("url")).await?; + fs::remove_dir(url_upload_path).await?; let serve_request = TestRequest::get().uri("/url").to_request(); let response = test::call_service(&app, serve_request).await; @@ -1128,7 +1142,7 @@ mod tests { let oneshot_upload_path = PasteType::Oneshot .get_path(&config.server.upload_path) .expect("Bad upload path"); - fs::create_dir_all(&oneshot_upload_path)?; + fs::create_dir_all(&oneshot_upload_path).await?; let file_name = "oneshot.txt"; let timestamp = util::get_system_time()?.as_secs().to_string(); @@ -1165,9 +1179,9 @@ mod tests { .map_err(error::ErrorInternalServerError)? .next() { - fs::remove_file(glob_path.map_err(error::ErrorInternalServerError)?)?; + fs::remove_file(glob_path.map_err(error::ErrorInternalServerError)?).await?; } - fs::remove_dir(oneshot_upload_path)?; + fs::remove_dir(oneshot_upload_path).await?; Ok(()) } @@ -1190,7 +1204,7 @@ mod tests { let url_upload_path = PasteType::OneshotUrl .get_path(&config.server.upload_path) .expect("Bad upload path"); - fs::create_dir_all(&url_upload_path)?; + fs::create_dir_all(&url_upload_path).await?; let response = test::call_service( &app, @@ -1220,7 +1234,7 @@ mod tests { assert_eq!(StatusCode::NOT_FOUND, response.status()); // Cleanup - fs::remove_dir_all(url_upload_path)?; + fs::remove_dir_all(url_upload_path).await?; Ok(()) } diff --git a/src/util.rs b/src/util.rs index 11bac079..1e491102 100644 --- a/src/util.rs +++ b/src/util.rs @@ -10,6 +10,7 @@ use std::io::{Error as IoError, ErrorKind as IoErrorKind, Result as IoResult}; use std::path::{Path, PathBuf}; use std::time::Duration; use std::time::{SystemTime, UNIX_EPOCH}; +use tokio::task::spawn_blocking; /// Regex for matching the timestamp extension of a path. pub static TIMESTAMP_EXTENSION_REGEX: Lazy = lazy_regex!(r#"\.[0-9]{10,}$"#); @@ -24,7 +25,7 @@ pub fn get_system_time() -> Result { /// Returns the first _unexpired_ path matched by a custom glob pattern. /// /// The file extension is accepted as a timestamp that points to the expiry date. -pub fn glob_match_file(mut path: PathBuf) -> Result { +pub async fn glob_match_file(mut path: PathBuf) -> Result { path = PathBuf::from( TIMESTAMP_EXTENSION_REGEX .replacen( @@ -36,10 +37,15 @@ pub fn glob_match_file(mut path: PathBuf) -> Result { ) .to_string(), ); - if let Some(glob_path) = glob(&format!("{}.[0-9]*", path.to_string_lossy())) - .map_err(error::ErrorInternalServerError)? - .last() - { + + let path_string = path.to_string_lossy().into_owned(); + let glob_match = match spawn_blocking(move || glob(&format!("{}.[0-9]*", path_string))).await { + Ok(Ok(m)) => m.last(), + Ok(Err(e)) => return Err(error::ErrorInternalServerError(e)), + Err(e) => return Err(error::ErrorInternalServerError(e)), + }; + + if let Some(glob_path) = glob_match { let glob_path = glob_path.map_err(error::ErrorInternalServerError)?; if let Some(extension) = glob_path .extension() @@ -145,19 +151,19 @@ mod tests { Ok(()) } - #[test] - fn test_glob_match() -> Result<(), ActixError> { + #[actix_rt::test] + async fn test_glob_match() -> Result<(), ActixError> { let path = PathBuf::from(format!( "expired.file1.{}", get_system_time()?.as_millis() + 50 )); fs::write(&path, String::new())?; - assert_eq!(path, glob_match_file(PathBuf::from("expired.file1"))?); + assert_eq!(path, glob_match_file(PathBuf::from("expired.file1")).await?); thread::sleep(Duration::from_millis(75)); assert_eq!( PathBuf::from("expired.file1"), - glob_match_file(PathBuf::from("expired.file1"))? + glob_match_file(PathBuf::from("expired.file1")).await? ); fs::remove_file(path)?; From 07fb759b5d49bd47b76e676a757cb774b217c1b8 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sun, 25 Feb 2024 15:13:14 +0000 Subject: [PATCH 2/7] Use async lock for config --- src/paste.rs | 14 +++---------- src/server.rs | 56 +++++++++------------------------------------------ 2 files changed, 13 insertions(+), 57 deletions(-) diff --git a/src/paste.rs b/src/paste.rs index 2d55d01c..c3376d47 100644 --- a/src/paste.rs +++ b/src/paste.rs @@ -8,7 +8,6 @@ use std::convert::{TryFrom, TryInto}; use std::io::{Error as IoError, ErrorKind as IoErrorKind, Result as IoResult}; use std::path::{Path, PathBuf}; use std::str; -use std::sync::RwLock; use tokio::fs::{self, File}; use tokio::io::AsyncWriteExt; use tokio::task::spawn_blocking; @@ -198,7 +197,7 @@ impl Paste { &mut self, expiry_date: Option, client: &Client, - config: &RwLock, + config: &Config, ) -> Result { let data = str::from_utf8(&self.data).map_err(error::ErrorBadRequest)?; let url = Url::parse(data).map_err(error::ErrorBadRequest)?; @@ -213,8 +212,6 @@ impl Paste { .await .map_err(error::ErrorInternalServerError)?; let payload_limit = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))? .server .max_content_length .try_into() @@ -225,9 +222,6 @@ impl Paste { .await .map_err(error::ErrorInternalServerError)? .to_vec(); - let config = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; let bytes_checksum = util::sha256_digest(&*bytes)?; self.data = bytes; if !config.paste.duplicate_files.unwrap_or(true) && expiry_date.is_none() { @@ -250,7 +244,7 @@ impl Paste { } } Ok(self - .store_file(file_name, expiry_date, None, &config) + .store_file(file_name, expiry_date, None, config) .await?) } @@ -501,9 +495,7 @@ mod tests { .timeout(Duration::from_secs(30)) .finish(), ); - let file_name = paste - .store_remote_file(None, &client_data, &RwLock::new(config.clone())) - .await?; + let file_name = paste.store_remote_file(None, &client_data, &config).await?; let file_path = PasteType::RemoteFile .get_path(&config.server.upload_path) .expect("Bad upload path") diff --git a/src/server.rs b/src/server.rs index 61932d2f..4719b148 100644 --- a/src/server.rs +++ b/src/server.rs @@ -19,19 +19,16 @@ use serde::{Deserialize, Serialize}; use std::convert::TryFrom; use std::env; use std::path::PathBuf; -use std::sync::RwLock; use std::time::Duration; use tokio::fs; +use tokio::sync::RwLock; use uts2ts; /// Shows the landing page. #[get("/")] #[allow(deprecated)] async fn index(config: web::Data>) -> Result { - let mut config = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))? - .clone(); + let mut config = config.read().await.clone(); let redirect = HttpResponse::Found() .append_header(("Location", env!("CARGO_PKG_HOMEPAGE"))) .finish(); @@ -86,9 +83,7 @@ async fn serve( options: Option>, config: web::Data>, ) -> Result { - let config = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; + let config = config.read().await; let mut path = util::glob_match_file(safe_path_join(&config.server.upload_path, &*file)?).await?; let mut paste_type = PasteType::File; @@ -158,9 +153,7 @@ async fn delete( file: web::Path, config: web::Data>, ) -> Result { - let config = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; + let config = config.read().await; let path = util::glob_match_file(safe_path_join(&config.server.upload_path, &*file)?).await?; if !path.is_file() || !path.exists() { return Err(error::ErrorNotFound("file is not found or expired :(\n")); @@ -179,9 +172,7 @@ async fn delete( #[get("/version")] #[actix_web_grants::protect("TokenType::Auth", ty = TokenType, error = unauthorized_error)] async fn version(config: web::Data>) -> Result { - let config = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; + let config = config.read().await; if !config.server.expose_version.unwrap_or(false) { warn!("server is not configured to expose version endpoint"); Err(error::ErrorNotFound(""))?; @@ -202,13 +193,8 @@ async fn upload( ) -> Result { let connection = request.connection_info().clone(); let host = connection.realip_remote_addr().unwrap_or("unknown host"); - let server_url = match config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))? - .server - .url - .clone() - { + let config = config.read().await; + let server_url = match config.server.url.clone() { Some(v) => v, None => { format!("{}://{}", connection.scheme(), connection.host(),) @@ -218,8 +204,6 @@ async fn upload( let mut expiry_date = header::parse_expiry_date(request.headers(), time)?; if expiry_date.is_none() { expiry_date = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))? .paste .default_expiry .and_then(|v| time.checked_add(v).map(|t| t.as_millis())); @@ -242,17 +226,9 @@ async fn upload( && paste_type != PasteType::RemoteFile && paste_type != PasteType::OneshotUrl && expiry_date.is_none() - && !config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))? - .paste - .duplicate_files - .unwrap_or(true) + && !config.paste.duplicate_files.unwrap_or(true) { let bytes_checksum = util::sha256_digest(&*bytes)?; - let config = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; if let Some(file) = Directory::try_from(config.server.upload_path.as_path()) .map_err(error::ErrorInternalServerError)? .get_file(bytes_checksum) @@ -274,9 +250,6 @@ async fn upload( }; let mut file_name = match paste.type_ { PasteType::File | PasteType::Oneshot => { - let config = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; paste .store_file( content.get_file_name()?, @@ -292,9 +265,6 @@ async fn upload( .await? } PasteType::Url | PasteType::OneshotUrl => { - let config = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; paste.store_url(expiry_date, &config).await? } }; @@ -306,9 +276,6 @@ async fn upload( .get_appropriate_unit(UnitType::Decimal), host ); - let config = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; if let Some(handle_spaces_config) = config.server.handle_spaces { file_name = handle_spaces_config.process_filename(&file_name); } @@ -336,10 +303,7 @@ pub struct ListItem { #[get("/list")] #[actix_web_grants::protect("TokenType::Auth", ty = TokenType, error = unauthorized_error)] async fn list(config: web::Data>) -> Result { - let config = config - .read() - .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))? - .clone(); + let config = config.read().await; if !config.server.expose_list.unwrap_or(false) { warn!("server is not configured to expose list endpoint"); @@ -348,7 +312,7 @@ async fn list(config: web::Data>) -> Result let mut entries: Vec = Vec::new(); - let mut dir_contents = fs::read_dir(config.server.upload_path).await?; + let mut dir_contents = fs::read_dir(&config.server.upload_path).await?; let system_time = util::get_system_time()?; From fc9e4934080ddeaced582d5c1700ef3f0adb0bcd Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Wed, 28 Feb 2024 20:01:25 +0000 Subject: [PATCH 3/7] Correctly get config when extracting tokens --- src/auth.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index af989cca..ebe03ca5 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -5,7 +5,7 @@ use actix_web::http::Method; use actix_web::middleware::ErrorHandlerResponse; use actix_web::{error, web, Error}; use std::collections::HashSet; -use std::sync::RwLock; +use tokio::sync::RwLock; /// Extracts the tokens from the authorization header by token type. /// @@ -14,8 +14,8 @@ pub(crate) async fn extract_tokens(req: &ServiceRequest) -> Result>>() .map(|cfg| cfg.read()) - .and_then(Result::ok) - .ok_or_else(|| error::ErrorInternalServerError("cannot acquire config"))?; + .ok_or_else(|| error::ErrorInternalServerError("cannot acquire config"))? + .await; let mut user_tokens = HashSet::with_capacity(2); From 3547c079e75a51f1c807fc69eea5d5eceea911bf Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Tue, 5 Mar 2024 22:44:44 +0000 Subject: [PATCH 4/7] Use correct `RwLock` when creating server This required a little refactor, but the code is now much cleaner, and correctly handles updating the config --- src/config.rs | 9 +++++- src/main.rs | 82 ++++++++++++++++++++++----------------------------- 2 files changed, 44 insertions(+), 47 deletions(-) diff --git a/src/config.rs b/src/config.rs index a7aac7e2..05821258 100644 --- a/src/config.rs +++ b/src/config.rs @@ -120,13 +120,20 @@ pub struct PasteConfig { pub delete_expired_files: Option, } +/// Default interval for cleanup +pub const DEFAULT_CLEANUP_INTERVAL: Duration = Duration::from_secs(30); + +const fn get_default_cleanup_interval() -> Duration { + DEFAULT_CLEANUP_INTERVAL +} + /// Cleanup configuration. #[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] pub struct CleanupConfig { /// Enable cleaning up. pub enabled: bool, /// Interval between clean-ups. - #[serde(default, with = "humantime_serde")] + #[serde(default = "get_default_cleanup_interval", with = "humantime_serde")] pub interval: Duration, } diff --git a/src/main.rs b/src/main.rs index 14705a7f..e78f4540 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,7 +5,7 @@ use actix_web::{App, HttpServer}; use awc::ClientBuilder; use hotwatch::notify::event::ModifyKind; use hotwatch::{Event, EventKind, Hotwatch}; -use rustypaste::config::{Config, ServerConfig}; +use rustypaste::config::{Config, DEFAULT_CLEANUP_INTERVAL}; use rustypaste::middleware::ContentLengthLimiter; use rustypaste::paste::PasteType; use rustypaste::server; @@ -15,9 +15,9 @@ use std::env; use std::fs; use std::io::Result as IoResult; use std::path::{Path, PathBuf}; -use std::sync::{mpsc, RwLock}; use std::thread; use std::time::Duration; +use tokio::sync::RwLock; #[cfg(not(feature = "shuttle"))] use tracing_subscriber::{ filter::LevelFilter, layer::SubscriberExt as _, util::SubscriberInitExt as _, EnvFilter, @@ -38,7 +38,7 @@ extern crate tracing; /// * initializes the logger /// * creates the necessary directories /// * spawns the threads -fn setup(config_folder: &Path) -> IoResult<(Data>, ServerConfig, Hotwatch)> { +async fn setup(config_folder: &Path) -> IoResult<(Data>, Hotwatch)> { // Load the .env file. dotenvy::dotenv().ok(); @@ -61,6 +61,7 @@ fn setup(config_folder: &Path) -> IoResult<(Data>, ServerConfig, } None => config_folder.join("config.toml"), }; + if !config_path.exists() { error!( "{} is not found, please provide a configuration file.", @@ -68,17 +69,15 @@ fn setup(config_folder: &Path) -> IoResult<(Data>, ServerConfig, ); std::process::exit(1); } + let config = Config::parse(&config_path).expect("failed to parse config"); trace!("{:#?}", config); config.warn_deprecation(); - let server_config = config.server.clone(); - let paste_config = RwLock::new(config.paste.clone()); - let (config_sender, config_receiver) = mpsc::channel::(); // Create necessary directories. - fs::create_dir_all(&server_config.upload_path)?; + fs::create_dir_all(&config.server.upload_path)?; for paste_type in &[PasteType::Url, PasteType::Oneshot, PasteType::OneshotUrl] { - fs::create_dir_all(paste_type.get_path(&server_config.upload_path)?)?; + fs::create_dir_all(paste_type.get_path(&config.server.upload_path)?)?; } // Set up a watcher for the configuration file changes. @@ -91,45 +90,44 @@ fn setup(config_folder: &Path) -> IoResult<(Data>, ServerConfig, ) .expect("failed to initialize configuration file watcher"); + let config_lock = Data::new(RwLock::new(config)); + // Hot-reload the configuration file. - let config = Data::new(RwLock::new(config)); - let cloned_config = Data::clone(&config); + let config_watcher_config = config_lock.clone(); let config_watcher = move |event: Event| { if let (EventKind::Modify(ModifyKind::Data(_)), Some(path)) = (event.kind, event.paths.first()) { match Config::parse(path) { - Ok(config) => match cloned_config.write() { - Ok(mut cloned_config) => { - *cloned_config = config.clone(); - info!("Configuration has been updated."); - if let Err(e) = config_sender.send(config) { - error!("Failed to send config for the cleanup routine: {}", e) - } - cloned_config.warn_deprecation(); - } - Err(e) => { - error!("Failed to acquire config: {}", e); - } - }, + Ok(new_config) => { + let mut locked_config = config_watcher_config.blocking_write(); + *locked_config = new_config; + info!("Configuration has been updated."); + locked_config.warn_deprecation(); + } Err(e) => { error!("Failed to update config: {}", e); } } } }; + hotwatch .watch(&config_path, config_watcher) .unwrap_or_else(|_| panic!("failed to watch {config_path:?}")); // Create a thread for cleaning up expired files. - let upload_path = server_config.upload_path.clone(); + let expired_files_config = config_lock.clone(); thread::spawn(move || loop { - let mut enabled = false; - if let Some(ref cleanup_config) = paste_config - .read() - .ok() - .and_then(|v| v.delete_expired_files.clone()) + let upload_path = expired_files_config + .blocking_read() + .server + .upload_path + .clone(); + if let Some(ref cleanup_config) = expired_files_config + .blocking_read() + .paste + .delete_expired_files { if cleanup_config.enabled { debug!("Running cleanup..."); @@ -141,32 +139,22 @@ fn setup(config_folder: &Path) -> IoResult<(Data>, ServerConfig, } thread::sleep(cleanup_config.interval); } - enabled = cleanup_config.enabled; - } - if let Some(new_config) = if enabled { - config_receiver.try_recv().ok() } else { - config_receiver.recv().ok() - } { - match paste_config.write() { - Ok(mut paste_config) => { - *paste_config = new_config.paste; - } - Err(e) => { - error!("Failed to update config for the cleanup routine: {}", e); - } - } + // Sleep for a bit when not configured to avoid a hot loop + thread::sleep(DEFAULT_CLEANUP_INTERVAL); } }); - Ok((config, server_config, hotwatch)) + Ok((config_lock, hotwatch)) } #[cfg(not(feature = "shuttle"))] #[actix_web::main] async fn main() -> IoResult<()> { // Set up the application. - let (config, server_config, _hotwatch) = setup(&PathBuf::new())?; + let (config, _hotwatch) = setup(&PathBuf::new()).await?; + + let server_config = config.read().await.server.clone(); // Create an HTTP server. let mut http_server = HttpServer::new(move || { @@ -203,7 +191,9 @@ async fn main() -> IoResult<()> { #[shuttle_runtime::main] async fn actix_web() -> ShuttleActixWeb { // Set up the application. - let (config, server_config, _hotwatch) = setup(Path::new("shuttle"))?; + let (config, _hotwatch) = setup(Path::new("shuttle"))?; + + let server_config = config.read().await.server.clone(); // Create the service. let service_config = move |cfg: &mut ServiceConfig| { From 46c6ed43d4ba9598384fe48be12866cc569a01b6 Mon Sep 17 00:00:00 2001 From: "Helmut K. C. Tessarek" Date: Thu, 7 Mar 2024 19:20:05 -0500 Subject: [PATCH 5/7] test: explicitly set config.paste.default_expiry --- src/server.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server.rs b/src/server.rs index 4719b148..04eca014 100644 --- a/src/server.rs +++ b/src/server.rs @@ -894,6 +894,7 @@ mod tests { let mut config = Config::default(); config.server.upload_path = PathBuf::from(&test_upload_dir); config.paste.duplicate_files = Some(false); + config.paste.default_expiry = None; config.paste.random_url = Some(RandomURLConfig { enabled: Some(true), type_: RandomURLType::Alphanumeric, From 43a9d50df354d87a6ebc6cdac61785d98f08f0eb Mon Sep 17 00:00:00 2001 From: "Helmut K. C. Tessarek" Date: Thu, 7 Mar 2024 19:29:23 -0500 Subject: [PATCH 6/7] ci: remove tarpulin --- .github/workflows/ci.yml | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ed03d218..ce83e25d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,19 +30,8 @@ jobs: uses: actions/checkout@v4 - name: Install Rust toolchain uses: dtolnay/rust-toolchain@stable - - name: Setup cargo-tarpaulin - uses: taiki-e/install-action@cargo-tarpaulin - name: Run tests - run: cargo tarpaulin --out xml --verbose -- --test-threads 1 - - name: Upload reports to codecov - uses: codecov/codecov-action@v3 - with: - name: code-coverage-report - file: cobertura.xml - flags: unit-tests - fail_ci_if_error: true - verbose: true - token: ${{ secrets.CODECOV_TOKEN }} + run: cargo test -- --test-threads 1 fixtures: strategy: From bc77b3ab92a75d79213d5e642ad267f71b974d73 Mon Sep 17 00:00:00 2001 From: "Helmut K. C. Tessarek" Date: Thu, 7 Mar 2024 19:45:10 -0500 Subject: [PATCH 7/7] test: remove explicitly set option again --- src/server.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/server.rs b/src/server.rs index 04eca014..4719b148 100644 --- a/src/server.rs +++ b/src/server.rs @@ -894,7 +894,6 @@ mod tests { let mut config = Config::default(); config.server.upload_path = PathBuf::from(&test_upload_dir); config.paste.duplicate_files = Some(false); - config.paste.default_expiry = None; config.paste.random_url = Some(RandomURLConfig { enabled: Some(true), type_: RandomURLType::Alphanumeric,