From 3554253d6bf2fffabe80f3ce09dd0d1d97b3fa66 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Thu, 15 Feb 2024 22:56:35 +0000 Subject: [PATCH 01/11] Use async for filesystem hits Except `Directory`, as there's no async-compatible `glob` implementation --- Cargo.toml | 2 +- src/file.rs | 17 ++--- src/paste.rs | 129 +++++++++++++++++++++----------------- src/server.rs | 168 +++++++++++++++++++++++++++----------------------- src/util.rs | 24 +++++--- 5 files changed, 191 insertions(+), 149 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 192272a1..9efad54b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,7 +44,7 @@ humantime-serde = "1.1.1" glob = "0.3.1" ring = "0.17.8" hotwatch = "0.5.0" -tokio = { version = "1.36.0", optional = true } +tokio = { version = "1.35.1", optional = true, 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 0b6261d3..4e43c043 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, @@ -183,8 +185,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) } @@ -233,9 +235,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() @@ -244,7 +253,9 @@ impl Paste { .to_string()); } } - 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. @@ -254,7 +265,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()))?; @@ -269,7 +280,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) } } @@ -302,15 +313,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), @@ -322,11 +333,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), @@ -338,11 +349,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), @@ -354,10 +365,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; @@ -365,10 +376,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 { @@ -380,15 +391,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), @@ -400,15 +411,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), @@ -420,22 +433,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; @@ -444,13 +460,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), @@ -461,20 +479,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"); @@ -498,14 +516,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 6fe1e41e..edaab6f0 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(); @@ -963,7 +977,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); @@ -1002,7 +1016,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(()) } @@ -1058,7 +1072,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(()) @@ -1113,7 +1127,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}")) @@ -1140,7 +1154,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, @@ -1154,8 +1168,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; @@ -1180,7 +1194,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(); @@ -1217,9 +1231,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(()) } @@ -1242,7 +1256,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, @@ -1272,7 +1286,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 c7b4a9ef01ee866934a140df19d78f6490dbbe92 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sun, 25 Feb 2024 15:13:14 +0000 Subject: [PATCH 02/11] 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 4e43c043..a30d8570 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; @@ -202,7 +201,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)?; @@ -217,8 +216,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() @@ -229,9 +226,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() { @@ -254,7 +248,7 @@ impl Paste { } } Ok(self - .store_file(file_name, expiry_date, None, &config) + .store_file(file_name, expiry_date, None, config) .await?) } @@ -505,9 +499,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 edaab6f0..429a8c65 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 97e919b1f4d19d85ae1337aa35a606689e2d882b Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Wed, 28 Feb 2024 20:01:25 +0000 Subject: [PATCH 03/11] 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 d48d1e6cdb2844b44ca403d562fd9f435caf9f59 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Tue, 5 Mar 2024 22:44:44 +0000 Subject: [PATCH 04/11] 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 2325899044891aca7086ffe49a59cbe1f7cdd95a Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Fri, 8 Mar 2024 14:44:55 +0000 Subject: [PATCH 05/11] 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 06/11] 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 a30d8570..83b2a311 100644 --- a/src/paste.rs +++ b/src/paste.rs @@ -293,7 +293,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 { @@ -317,6 +317,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, @@ -364,6 +372,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 { @@ -395,6 +411,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, @@ -439,16 +463,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, @@ -464,10 +495,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(), @@ -488,7 +535,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 829936762a329c02d93bcf648764490aa393a81c Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sat, 23 Mar 2024 11:40:06 +0000 Subject: [PATCH 07/11] Run tests in isolated directory This prevents tests from conflicting with eachother if they don't clean up after themselves correctly. --- Cargo.lock | 1 + Cargo.toml | 1 + src/paste.rs | 83 ++++++++++++++++++++------------------------------- src/server.rs | 49 ++++++++++++++++++------------ src/util.rs | 18 +++++------ 5 files changed, 73 insertions(+), 79 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2d6fff64..4561a695 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2229,6 +2229,7 @@ dependencies = [ "serde_regex", "shuttle-actix-web", "shuttle-runtime", + "tempfile", "tokio", "tracing", "tracing-subscriber", diff --git a/Cargo.toml b/Cargo.toml index 9efad54b..a55ab089 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 83b2a311..112c0987 100644 --- a/src/paste.rs +++ b/src/paste.rs @@ -287,15 +287,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), @@ -308,14 +309,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(()) } @@ -323,8 +319,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, @@ -336,10 +333,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), @@ -352,10 +349,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), @@ -368,9 +365,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(()) } @@ -378,8 +375,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 { @@ -387,9 +385,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 { @@ -402,14 +400,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(()) } @@ -417,8 +410,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, @@ -437,9 +431,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), @@ -459,9 +453,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(()) } @@ -470,7 +464,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( @@ -502,7 +496,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() @@ -542,7 +536,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( @@ -562,25 +556,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 429a8c65..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)?; + 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(()) } From b251b42d5b68fc3579cdb6b239fd7d707df26edc Mon Sep 17 00:00:00 2001 From: "Helmut K. C. Tessarek" Date: Wed, 3 Apr 2024 17:23:33 -0400 Subject: [PATCH 08/11] Update Cargo.toml --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 2886b69d..7b07f8b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,7 +44,7 @@ humantime-serde = "1.1.1" glob = "0.3.1" ring = "0.17.8" hotwatch = "0.5.0" -tokio = { version = "1.37.0", optional = true, features = ["fs"] } +tokio = { version = "1.37.0", features = ["fs"] } tracing = "0.1.40" tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } uts2ts = "0.4.1" From ad3d599dae16733fd2d687deaefa3dca659c1607 Mon Sep 17 00:00:00 2001 From: "Helmut K. C. Tessarek" Date: Wed, 3 Apr 2024 17:29:24 -0400 Subject: [PATCH 09/11] Update Cargo.toml --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 7b07f8b2..305d1ac5 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.5.1" } From dd7b93f4434597d9d2b8e96c9db544515e64b39f Mon Sep 17 00:00:00 2001 From: "Helmut K. C. Tessarek" Date: Wed, 3 Apr 2024 17:47:11 -0400 Subject: [PATCH 10/11] Update paste.rs --- src/paste.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/paste.rs b/src/paste.rs index 112c0987..962506bb 100644 --- a/src/paste.rs +++ b/src/paste.rs @@ -177,7 +177,7 @@ impl Paste { .unwrap_or_default() .to_string(); let file_path = util::glob_match_file(path.clone()) - .map_err(|_| IoError::new(IoErrorKind::Other, String::from("path is not valid")))?; + .await.map_err(|_| IoError::new(IoErrorKind::Other, String::from("path is not valid")))?; if file_path.is_file() && file_path.exists() { return Err(error::ErrorConflict("file already exists\n")); } From a7a5977dab95b15e8f816d7a73dda75ca88a7e21 Mon Sep 17 00:00:00 2001 From: "Helmut K. C. Tessarek" Date: Wed, 3 Apr 2024 17:52:41 -0400 Subject: [PATCH 11/11] fix: lints and formatting --- src/paste.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/paste.rs b/src/paste.rs index 962506bb..b602fb21 100644 --- a/src/paste.rs +++ b/src/paste.rs @@ -177,7 +177,8 @@ impl Paste { .unwrap_or_default() .to_string(); let file_path = util::glob_match_file(path.clone()) - .await.map_err(|_| IoError::new(IoErrorKind::Other, String::from("path is not valid")))?; + .await + .map_err(|_| IoError::new(IoErrorKind::Other, String::from("path is not valid")))?; if file_path.is_file() && file_path.exists() { return Err(error::ErrorConflict("file already exists\n")); } @@ -247,9 +248,7 @@ impl Paste { .to_string()); } } - Ok(self - .store_file(file_name, expiry_date, None, config) - .await?) + self.store_file(file_name, expiry_date, None, config).await } /// Writes an URL to a file in upload directory.