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 46210d22f3bbff729d4ca41c04b3bc23068df657 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Fri, 8 Mar 2024 14:44:55 +0000 Subject: [PATCH 5/7] Ensure config read locks are dropped when expected --- src/config.rs | 2 +- src/main.rs | 43 ++++++++++++++++++++++--------------------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/config.rs b/src/config.rs index 05821258..9ea181c5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -121,7 +121,7 @@ pub struct PasteConfig { } /// Default interval for cleanup -pub const DEFAULT_CLEANUP_INTERVAL: Duration = Duration::from_secs(30); +pub const DEFAULT_CLEANUP_INTERVAL: Duration = Duration::from_secs(60); const fn get_default_cleanup_interval() -> Duration { DEFAULT_CLEANUP_INTERVAL diff --git a/src/main.rs b/src/main.rs index e78f4540..2be9ca77 100644 --- a/src/main.rs +++ b/src/main.rs @@ -98,6 +98,8 @@ async fn setup(config_folder: &Path) -> IoResult<(Data>, Hotwatch if let (EventKind::Modify(ModifyKind::Data(_)), Some(path)) = (event.kind, event.paths.first()) { + info!("Reloading configuration"); + match Config::parse(path) { Ok(new_config) => { let mut locked_config = config_watcher_config.blocking_write(); @@ -118,31 +120,28 @@ async fn setup(config_folder: &Path) -> IoResult<(Data>, Hotwatch // Create a thread for cleaning up expired files. let expired_files_config = config_lock.clone(); + let mut cleanup_interval = DEFAULT_CLEANUP_INTERVAL; thread::spawn(move || loop { - 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 + // Additional context block to ensure the config lock is dropped { - if cleanup_config.enabled { - debug!("Running cleanup..."); - for file in util::get_expired_files(&upload_path) { - match fs::remove_file(&file) { - Ok(()) => info!("Removed expired file: {:?}", file), - Err(e) => error!("Cannot remove expired file: {}", e), + let locked_config = expired_files_config.blocking_read(); + let upload_path = locked_config.server.upload_path.clone(); + + if let Some(ref cleanup_config) = locked_config.paste.delete_expired_files { + if cleanup_config.enabled { + debug!("Running cleanup..."); + for file in util::get_expired_files(&upload_path) { + match fs::remove_file(&file) { + Ok(()) => info!("Removed expired file: {:?}", file), + Err(e) => error!("Cannot remove expired file: {}", e), + } } + cleanup_interval = cleanup_config.interval; } - thread::sleep(cleanup_config.interval); } - } else { - // Sleep for a bit when not configured to avoid a hot loop - thread::sleep(DEFAULT_CLEANUP_INTERVAL); } + + thread::sleep(cleanup_interval); }); Ok((config_lock, hotwatch)) @@ -154,7 +153,8 @@ async fn main() -> IoResult<()> { // Set up the application. let (config, _hotwatch) = setup(&PathBuf::new()).await?; - let server_config = config.read().await.server.clone(); + // Extra context block ensures the lock is stopped + let server_config = { config.read().await.server.clone() }; // Create an HTTP server. let mut http_server = HttpServer::new(move || { @@ -193,7 +193,8 @@ async fn actix_web() -> ShuttleActixWeb Date: Thu, 21 Mar 2024 17:23:21 +0300 Subject: [PATCH 6/7] refactor(tests): split tests for readability --- src/paste.rs | 81 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 72 insertions(+), 9 deletions(-) diff --git a/src/paste.rs b/src/paste.rs index 72ab3b3c..7ae5bb96 100644 --- a/src/paste.rs +++ b/src/paste.rs @@ -292,7 +292,7 @@ mod tests { #[actix_rt::test] #[allow(deprecated)] - async fn test_paste_data() -> Result<(), Error> { + async fn test_paste() -> Result<(), Error> { let mut config = Config::default(); config.server.upload_path = env::current_dir()?; config.paste.random_url = Some(RandomURLConfig { @@ -316,6 +316,14 @@ mod tests { ); fs::remove_file(file_name).await?; + Ok(()) + } + + #[actix_rt::test] + #[allow(deprecated)] + async fn test_paste_random() -> Result<(), Error> { + let mut config = Config::default(); + config.server.upload_path = env::current_dir()?; config.paste.random_url = Some(RandomURLConfig { length: Some(4), type_: RandomURLType::Alphanumeric, @@ -363,6 +371,14 @@ mod tests { assert!(file_name.ends_with(".tar.gz")); fs::remove_file(file_name).await?; + Ok(()) + } + + #[actix_rt::test] + #[allow(deprecated)] + async fn test_paste_with_extension() -> Result<(), Error> { + let mut config = Config::default(); + config.server.upload_path = env::current_dir()?; config.paste.default_extension = String::from("txt"); config.paste.random_url = None; let paste = Paste { @@ -394,6 +410,14 @@ mod tests { ); fs::remove_file(file_name).await?; + Ok(()) + } + + #[actix_rt::test] + #[allow(deprecated)] + async fn test_paste_filename_from_header() -> Result<(), Error> { + let mut config = Config::default(); + config.server.upload_path = env::current_dir()?; config.paste.random_url = Some(RandomURLConfig { length: Some(4), type_: RandomURLType::Alphanumeric, @@ -438,16 +462,23 @@ mod tests { assert_eq!("fn_from_header", 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?; - } + Ok(()) + } + #[actix_rt::test] + #[allow(deprecated)] + async fn test_paste_oneshot() -> Result<(), Error> { + let mut config = Config::default(); + config.server.upload_path = env::current_dir()?; config.paste.random_url = None; + + fs::create_dir_all( + PasteType::Oneshot + .get_path(&config.server.upload_path) + .expect("Bad upload path"), + ) + .await?; + let paste = Paste { data: vec![116, 101, 115, 116], type_: PasteType::Oneshot, @@ -463,10 +494,26 @@ mod tests { assert_eq!("test", fs::read_to_string(&file_path).await?); fs::remove_file(file_path).await?; + Ok(()) + } + + #[actix_rt::test] + #[allow(deprecated)] + async fn test_paste_url() -> Result<(), Error> { + let mut config = Config::default(); + config.server.upload_path = env::current_dir()?; config.paste.random_url = Some(RandomURLConfig { enabled: Some(true), ..RandomURLConfig::default() }); + + fs::create_dir_all( + PasteType::Url + .get_path(&config.server.upload_path) + .expect("Bad upload path"), + ) + .await?; + let url = String::from("https://orhun.dev/"); let paste = Paste { data: url.as_bytes().to_vec(), @@ -487,7 +534,23 @@ mod tests { }; assert!(paste.store_url(None, &config).await.is_err()); + Ok(()) + } + + #[actix_rt::test] + #[allow(deprecated)] + async fn test_paste_remote_url() -> Result<(), Error> { + let mut config = Config::default(); + config.server.upload_path = env::current_dir()?; config.server.max_content_length = Byte::from_str("30k").expect("cannot parse byte"); + + fs::create_dir_all( + PasteType::Url + .get_path(&config.server.upload_path) + .expect("Bad upload path"), + ) + .await?; + let url = String::from("https://upload.wikimedia.org/wikipedia/en/a/a9/Example.jpg"); let mut paste = Paste { data: url.as_bytes().to_vec(), From 18a1abf771e6681c029bc48aab301438d2ff6771 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sat, 23 Mar 2024 11:40:06 +0000 Subject: [PATCH 7/7] Run tests in isolated directory This prevents tests from conflicting with eachother if they don't clean up after themselves correctly. --- Cargo.lock | 16 +++++----- Cargo.toml | 1 + src/paste.rs | 83 ++++++++++++++++++++------------------------------- src/server.rs | 49 ++++++++++++++++++------------ src/util.rs | 18 +++++------ 5 files changed, 80 insertions(+), 87 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e6c470a2..3c2c01cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1525,9 +1525,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.150" +version = "0.2.153" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89d92a4743f9a61002fae18374ed11e7973f530cb3a3255fb354818118b2203c" +checksum = "9c198f91728a82281a64e1f4f9eeb25d82cb32a5de251c6bd1b5154d63a8e7bd" [[package]] name = "linked-hash-map" @@ -2354,9 +2354,9 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.26" +version = "0.38.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9470c4bf8246c8daf25f9598dca807fb6510347b1e1cfa55749113850c79d88a" +checksum = "65e04861e65f21776e67888bfbea442b3642beaa0138fdb1dd7a84a52dffdb89" dependencies = [ "bitflags 2.4.1", "errno", @@ -2440,6 +2440,7 @@ dependencies = [ "serde_regex", "shuttle-actix-web", "shuttle-runtime", + "tempfile", "tokio", "tracing", "tracing-subscriber", @@ -2895,15 +2896,14 @@ checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" [[package]] name = "tempfile" -version = "3.8.1" +version = "3.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ef1adac450ad7f4b3c28589471ade84f25f731a7a0fe30d71dfa9f60fd808e5" +checksum = "85b77fafb263dd9d05cbeac119526425676db3784113aa9295c88498cbf8bff1" dependencies = [ "cfg-if", "fastrand", - "redox_syscall 0.4.1", "rustix", - "windows-sys 0.48.0", + "windows-sys 0.52.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 5b0ae548..13db9c18 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,6 +65,7 @@ default-features = false [dev-dependencies] actix-rt = "2.9.0" +tempfile = "3.10.1" [profile.dev] opt-level = 0 diff --git a/src/paste.rs b/src/paste.rs index 7ae5bb96..b602fb21 100644 --- a/src/paste.rs +++ b/src/paste.rs @@ -286,15 +286,16 @@ mod tests { use actix_web::web::Data; use awc::ClientBuilder; use byte_unit::Byte; - use std::env; use std::str::FromStr; use std::time::Duration; + use tempfile::tempdir; #[actix_rt::test] #[allow(deprecated)] async fn test_paste() -> Result<(), Error> { + let temp_upload_path = tempdir()?; let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = temp_upload_path.path().to_path_buf(); config.paste.random_url = Some(RandomURLConfig { enabled: Some(true), words: Some(3), @@ -307,14 +308,9 @@ mod tests { type_: PasteType::File, }; 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).await?; + let file_path = temp_upload_path.path().join(file_name); + assert_eq!("ABC", fs::read_to_string(&file_path).await?); + assert_eq!(Some("txt"), file_path.extension().and_then(|v| v.to_str())); Ok(()) } @@ -322,8 +318,9 @@ mod tests { #[actix_rt::test] #[allow(deprecated)] async fn test_paste_random() -> Result<(), Error> { + let temp_upload_path = tempdir()?; let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = temp_upload_path.path().to_path_buf(); config.paste.random_url = Some(RandomURLConfig { length: Some(4), type_: RandomURLType::Alphanumeric, @@ -335,10 +332,10 @@ mod tests { type_: PasteType::File, }; let file_name = paste.store_file("foo.tar.gz", None, None, &config).await?; - assert_eq!("tessus", fs::read_to_string(&file_name).await?); + let file_path = temp_upload_path.path().join(&file_name); + assert_eq!("tessus", fs::read_to_string(&file_path).await?); assert!(file_name.ends_with(".tar.gz")); assert!(file_name.starts_with("foo.")); - fs::remove_file(file_name).await?; config.paste.random_url = Some(RandomURLConfig { length: Some(4), @@ -351,10 +348,10 @@ mod tests { type_: PasteType::File, }; let file_name = paste.store_file(".foo.tar.gz", None, None, &config).await?; - assert_eq!("tessus", fs::read_to_string(&file_name).await?); + let file_path = temp_upload_path.path().join(&file_name); + assert_eq!("tessus", fs::read_to_string(&file_path).await?); assert!(file_name.ends_with(".tar.gz")); assert!(file_name.starts_with(".foo.")); - fs::remove_file(file_name).await?; config.paste.random_url = Some(RandomURLConfig { length: Some(4), @@ -367,9 +364,9 @@ mod tests { type_: PasteType::File, }; let file_name = paste.store_file("foo.tar.gz", None, None, &config).await?; - assert_eq!("tessus", fs::read_to_string(&file_name).await?); + let file_path = temp_upload_path.path().join(&file_name); + assert_eq!("tessus", fs::read_to_string(&file_path).await?); assert!(file_name.ends_with(".tar.gz")); - fs::remove_file(file_name).await?; Ok(()) } @@ -377,8 +374,9 @@ mod tests { #[actix_rt::test] #[allow(deprecated)] async fn test_paste_with_extension() -> Result<(), Error> { + let temp_upload_path = tempdir()?; let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = temp_upload_path.path().to_path_buf(); config.paste.default_extension = String::from("txt"); config.paste.random_url = None; let paste = Paste { @@ -386,9 +384,9 @@ mod tests { type_: PasteType::File, }; let file_name = paste.store_file(".foo", None, None, &config).await?; - assert_eq!("xyz", fs::read_to_string(&file_name).await?); + let file_path = temp_upload_path.path().join(&file_name); + assert_eq!("xyz", fs::read_to_string(&file_path).await?); assert_eq!(".foo.txt", file_name); - fs::remove_file(file_name).await?; config.paste.default_extension = String::from("bin"); config.paste.random_url = Some(RandomURLConfig { @@ -401,14 +399,9 @@ mod tests { type_: PasteType::File, }; 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).await?; + let file_path = temp_upload_path.path().join(&file_name); + assert_eq!(Some("bin"), file_path.extension().and_then(|v| v.to_str())); + assert_eq!("xyz", fs::read_to_string(&file_path).await?); Ok(()) } @@ -416,8 +409,9 @@ mod tests { #[actix_rt::test] #[allow(deprecated)] async fn test_paste_filename_from_header() -> Result<(), Error> { + let temp_upload_path = tempdir()?; let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = temp_upload_path.path().to_path_buf(); config.paste.random_url = Some(RandomURLConfig { length: Some(4), type_: RandomURLType::Alphanumeric, @@ -436,9 +430,9 @@ mod tests { &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).await?; + let file_path = temp_upload_path.path().join(&file_name); + assert_eq!("tessus", fs::read_to_string(&file_path).await?); config.paste.random_url = Some(RandomURLConfig { length: Some(4), @@ -458,9 +452,9 @@ mod tests { &config, ) .await?; - assert_eq!("tessus", fs::read_to_string(&file_name).await?); + let file_path = temp_upload_path.path().join(&file_name); + assert_eq!("tessus", fs::read_to_string(&file_path).await?); assert_eq!("fn_from_header", file_name); - fs::remove_file(file_name).await?; Ok(()) } @@ -469,7 +463,7 @@ mod tests { #[allow(deprecated)] async fn test_paste_oneshot() -> Result<(), Error> { let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = tempdir()?.path().to_path_buf(); config.paste.random_url = None; fs::create_dir_all( @@ -501,7 +495,7 @@ mod tests { #[allow(deprecated)] async fn test_paste_url() -> Result<(), Error> { let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = tempdir()?.path().to_path_buf(); config.paste.random_url = Some(RandomURLConfig { enabled: Some(true), ..RandomURLConfig::default() @@ -541,7 +535,7 @@ mod tests { #[allow(deprecated)] async fn test_paste_remote_url() -> Result<(), Error> { let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = tempdir()?.path().to_path_buf(); config.server.max_content_length = Byte::from_str("30k").expect("cannot parse byte"); fs::create_dir_all( @@ -561,25 +555,12 @@ mod tests { .timeout(Duration::from_secs(30)) .finish(), ); - 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") - .join(file_name); + let _ = paste.store_remote_file(None, &client_data, &config).await?; + assert_eq!( "70ff72a2f7651b5fae3aa9834e03d2a2233c52036610562f7fa04e089e8198ed", util::sha256_digest(&*paste.data)? ); - 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 da35657c..db5b82df 100644 --- a/src/server.rs +++ b/src/server.rs @@ -398,6 +398,7 @@ mod tests { use std::str; use std::thread; use std::time::Duration; + use tempfile::tempdir; fn get_multipart_request(data: &str, name: &str, filename: &str) -> TestRequest { let multipart_data = format!( @@ -730,9 +731,10 @@ mod tests { #[actix_web::test] async fn test_delete_file() -> Result<(), Error> { + let temp_upload_path = tempdir()?; let mut config = Config::default(); config.server.delete_tokens = Some(["test".to_string()].into()); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = temp_upload_path.path().to_path_buf(); let app = test::init_service( App::new() @@ -759,8 +761,7 @@ mod tests { assert_eq!(StatusCode::OK, response.status()); assert_body(response.into_body(), "file deleted\n").await?; - let path = PathBuf::from(file_name); - assert!(!path.exists()); + assert!(!temp_upload_path.path().join(file_name).exists()); Ok(()) } @@ -768,7 +769,7 @@ mod tests { #[actix_web::test] async fn test_delete_file_without_token_in_config() -> Result<(), Error> { let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = tempdir()?.path().to_path_buf(); let app = test::init_service( App::new() @@ -793,8 +794,9 @@ mod tests { #[actix_web::test] async fn test_upload_file() -> Result<(), Error> { + let test_delete_file = tempdir()?; let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = test_delete_file.path().to_path_buf(); let app = test::init_service( App::new() @@ -825,7 +827,7 @@ mod tests { assert_eq!(StatusCode::OK, response.status()); assert_body(response.into_body(), ×tamp).await?; - fs::remove_file(file_name).await?; + fs::remove_file(test_delete_file.path().join(file_name)).await?; let serve_request = TestRequest::get() .uri(&format!("/{file_name}")) .to_request(); @@ -837,8 +839,9 @@ mod tests { #[actix_web::test] async fn test_upload_file_override_filename() -> Result<(), Error> { + let test_delete_file = tempdir()?; let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = test_delete_file.path().to_path_buf(); let app = test::init_service( App::new() @@ -875,7 +878,7 @@ mod tests { assert_eq!(StatusCode::OK, response.status()); assert_body(response.into_body(), ×tamp).await?; - fs::remove_file(header_filename).await?; + fs::remove_file(test_delete_file.path().join(header_filename)).await?; let serve_request = TestRequest::get() .uri(&format!("/{header_filename}")) .to_request(); @@ -887,8 +890,9 @@ mod tests { #[actix_web::test] async fn test_upload_same_filename() -> Result<(), Error> { + let temp_upload_dir = tempdir()?; let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = temp_upload_dir.path().to_path_buf(); let app = test::init_service( App::new() @@ -932,7 +936,7 @@ mod tests { assert_eq!(StatusCode::CONFLICT, response.status()); assert_body(response.into_body(), "file already exists\n").await?; - fs::remove_file(header_filename).await?; + fs::remove_file(temp_upload_dir.path().join(header_filename)).await?; Ok(()) } @@ -987,8 +991,9 @@ mod tests { #[actix_web::test] async fn test_upload_expiring_file() -> Result<(), Error> { + let temp_upload_path = tempdir()?; let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = temp_upload_path.path().to_path_buf(); let app = test::init_service( App::new() @@ -1032,9 +1037,14 @@ mod tests { let response = test::call_service(&app, serve_request).await; assert_eq!(StatusCode::NOT_FOUND, response.status()); - if let Some(glob_path) = glob(&format!("{file_name}.[0-9]*")) - .map_err(error::ErrorInternalServerError)? - .next() + if let Some(glob_path) = glob( + &temp_upload_path + .path() + .join(format!("{file_name}.[0-9]*")) + .to_string_lossy(), + ) + .map_err(error::ErrorInternalServerError)? + .next() { fs::remove_file(glob_path.map_err(error::ErrorInternalServerError)?).await?; } @@ -1044,8 +1054,9 @@ mod tests { #[actix_web::test] async fn test_upload_remote_file() -> Result<(), Error> { + let temp_upload_dir = tempdir()?; let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = temp_upload_dir.path().to_path_buf(); config.server.max_content_length = Byte::from_u128(30000).unwrap_or_default(); let app = test::init_service( @@ -1091,7 +1102,7 @@ mod tests { util::sha256_digest(&*body_bytes)? ); - fs::remove_file(file_name).await?; + fs::remove_file(temp_upload_dir.path().join(file_name)).await?; let serve_request = TestRequest::get() .uri(&format!("/{file_name}")) @@ -1105,7 +1116,7 @@ mod tests { #[actix_web::test] async fn test_upload_url() -> Result<(), Error> { let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = tempdir()?.path().to_path_buf(); let app = test::init_service( App::new() @@ -1145,7 +1156,7 @@ mod tests { #[actix_web::test] async fn test_upload_oneshot() -> Result<(), Error> { let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = tempdir()?.path().to_path_buf(); let app = test::init_service( App::new() @@ -1205,7 +1216,7 @@ mod tests { #[actix_web::test] async fn test_upload_oneshot_url() -> Result<(), Error> { let mut config = Config::default(); - config.server.upload_path = env::current_dir()?; + config.server.upload_path = tempdir()?.path().to_path_buf(); let oneshot_url_suffix = "oneshot_url"; diff --git a/src/util.rs b/src/util.rs index 1e491102..1752c214 100644 --- a/src/util.rs +++ b/src/util.rs @@ -143,6 +143,8 @@ mod tests { use std::env; use std::fs; use std::thread; + use tempfile::tempdir; + #[test] fn test_system_time() -> Result<(), ActixError> { let system_time = get_system_time()?.as_millis(); @@ -185,18 +187,16 @@ mod tests { #[test] fn test_get_expired_files() -> Result<(), ActixError> { - let current_dir = env::current_dir()?; + let test_temp_dir = tempdir()?; + let test_dir = test_temp_dir.path(); let expiration_time = get_system_time()?.as_millis() + 50; - let path = PathBuf::from(format!("expired.file2.{expiration_time}")); + let path = test_dir.join(format!("expired.file2.{expiration_time}")); fs::write(&path, String::new())?; - assert_eq!(Vec::::new(), get_expired_files(¤t_dir)); + assert_eq!(Vec::::new(), get_expired_files(test_dir)); thread::sleep(Duration::from_millis(75)); - assert_eq!( - vec![current_dir.join(&path)], - get_expired_files(¤t_dir) - ); - fs::remove_file(path)?; - assert_eq!(Vec::::new(), get_expired_files(¤t_dir)); + assert_eq!(vec![path.clone()], get_expired_files(test_dir)); + fs::remove_file(&path)?; + assert_eq!(Vec::::new(), get_expired_files(test_dir)); Ok(()) }