diff --git a/src/database/mod.rs b/src/database/mod.rs index df97315..77ac42e 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -27,6 +27,24 @@ impl SqliteUserDao { } } +#[cfg(test)] +pub mod test { + use diesel::{Connection, SqliteConnection}; + use diesel_migrations::{embed_migrations, EmbeddedMigrations, MigrationHarness}; + + const DB_MIGRATIONS: EmbeddedMigrations = embed_migrations!(); + + pub fn in_memory_db_connection() -> SqliteConnection { + let mut connection = SqliteConnection::establish(":memory:") + .expect("Unable to create in-memory db connection"); + connection + .run_pending_migrations(DB_MIGRATIONS) + .expect("Failure running DB migrations"); + + return connection; + } +} + impl UserDao for SqliteUserDao { // TODO: Should probably use Result here fn create_user(&mut self, user: &str, pass: &str) -> Option { diff --git a/src/files.rs b/src/files.rs index caa6ed3..92ff3e9 100644 --- a/src/files.rs +++ b/src/files.rs @@ -19,16 +19,17 @@ use crate::AppState; use crate::tags::TagDao; use path_absolutize::*; -pub async fn list_photos( +pub async fn list_photos( _: Claims, req: Query, app_state: web::Data, + file_system: web::Data, tag_dao: web::Data>, ) -> HttpResponse { let path = &req.path; - if let Some(path) = is_valid_full_path(&PathBuf::from(&app_state.base_path), path) { + + if let Ok(files) = file_system.get_files_for_path(path) { debug!("Valid path: {:?}", path); - let files = list_files(&path).unwrap_or_default(); let photos = files .iter() @@ -36,7 +37,7 @@ pub async fn list_photos( f.metadata().map_or_else( |e| { error!("Failed getting file metadata: {:?}", e); - false + f.extension().is_some() }, |md| md.is_file(), ) @@ -110,10 +111,13 @@ pub fn is_image_or_video(path: &Path) -> bool { || extension == "nef" } -pub fn is_valid_full_path + Debug>(base: &P, path: &str) -> Option { - debug!("Base: {:?}. Path: {}", base, path); +pub fn is_valid_full_path + Debug + AsRef>( + base: &P, + path: &P, +) -> Option { + debug!("Base: {:?}. Path: {:?}", base, path); - let path = PathBuf::from(path); + let path = PathBuf::from(&path); let mut path = if path.is_relative() { let mut full_path = PathBuf::new(); full_path.push(base); @@ -153,31 +157,95 @@ fn is_path_above_base_dir + Debug>( ) } +pub trait FileSystemAccess { + fn get_files_for_path(&self, path: &str) -> anyhow::Result>; +} + +pub struct RealFileSystem { + base_path: String, +} + +impl RealFileSystem { + pub(crate) fn new(base_path: String) -> RealFileSystem { + RealFileSystem { base_path } + } +} + +impl FileSystemAccess for RealFileSystem { + fn get_files_for_path(&self, path: &str) -> anyhow::Result> { + is_valid_full_path(&PathBuf::from(&self.base_path), &PathBuf::from(path)) + .map(|path| { + debug!("Valid path: {:?}", path); + list_files(&path).unwrap_or_default() + }) + .context("Invalid path") + } +} + #[cfg(test)] mod tests { + use crate::database::connect; + use crate::tags::SqliteTagDao; + use actix_web::web::Data; + use diesel::{Connection, SqliteConnection}; + use std::collections::HashMap; use std::env; + use std::ffi::OsStr; use std::fs::File; use super::*; + struct FakeFileSystem { + files: HashMap>, + err: bool, + } + + impl FakeFileSystem { + fn with_error() -> FakeFileSystem { + FakeFileSystem { + files: HashMap::new(), + err: true, + } + } + + fn new(files: HashMap>) -> FakeFileSystem { + FakeFileSystem { files, err: false } + } + } + + impl FileSystemAccess for FakeFileSystem { + fn get_files_for_path(&self, path: &str) -> anyhow::Result> { + if self.err { + Err(anyhow!("Error for test")) + } else { + if let Some(files) = self.files.get(path) { + Ok(files + .iter() + .map(|p| PathBuf::from(p)) + .collect::>()) + } else { + Ok(Vec::new()) + } + } + } + } + mod api { use super::*; use actix::Actor; - use actix_web::{ - web::{self, Query}, - HttpResponse, - }; + use actix_web::{web::Query, HttpResponse}; use crate::{ - data::{Claims, PhotosResponse, ThumbnailRequest}, + data::{Claims, PhotosResponse}, testhelpers::BodyReader, video::StreamActor, AppState, }; - use std::{fs, sync::Arc}; - use actix_web::web::Data; + use crate::database::test::in_memory_db_connection; use crate::tags::SqliteTagDao; + use actix_web::web::Data; + use std::{fs, sync::Arc}; fn setup() { let _ = env_logger::builder().is_test(true).try_init(); @@ -194,7 +262,7 @@ mod tests { let request: Query = Query::from_query("path=").unwrap(); - let mut temp_photo = std::env::temp_dir(); + let mut temp_photo = env::temp_dir(); let mut tmp = temp_photo.clone(); tmp.push("test-dir"); @@ -202,24 +270,25 @@ mod tests { temp_photo.push("photo.jpg"); - fs::File::create(temp_photo.clone()).unwrap(); + File::create(temp_photo.clone()).unwrap(); let response: HttpResponse = list_photos( claims, request, - web::Data::new(AppState::new( + Data::new(AppState::new( Arc::new(StreamActor {}.start()), String::from("/tmp"), String::from("/tmp/thumbs"), )), + Data::new(RealFileSystem::new(String::from("/tmp"))), Data::new(Mutex::new(SqliteTagDao::default())), ) - .await; + .await; let status = response.status(); + assert_eq!(status, 200); let body: PhotosResponse = serde_json::from_str(&response.read_to_str()).unwrap(); - assert_eq!(status, 200); assert!(body.photos.contains(&String::from("photo.jpg"))); assert!(body.dirs.contains(&String::from("test-dir"))); assert!(body @@ -246,28 +315,145 @@ mod tests { let response = list_photos( claims, request, - web::Data::new(AppState::new( + Data::new(AppState::new( Arc::new(StreamActor {}.start()), String::from("/tmp"), String::from("/tmp/thumbs"), )), + Data::new(RealFileSystem::new(String::from("./"))), Data::new(Mutex::new(SqliteTagDao::default())), ) - .await; + .await; assert_eq!(response.status(), 400); } + + #[actix_rt::test] + async fn get_files_with_tag_any_filter() { + setup(); + + let claims = Claims { + sub: String::from("1"), + exp: 12345, + }; + + let request: Query = Query::from_query("path=&tag_ids=1,3").unwrap(); + + let mut tag_dao = SqliteTagDao::new(in_memory_db_connection()); + + let tag1 = tag_dao.create_tag("tag1").unwrap(); + let tag2 = tag_dao.create_tag("tag2").unwrap(); + let tag3 = tag_dao.create_tag("tag3").unwrap(); + + &tag_dao.tag_file("test.jpg", tag1.id).unwrap(); + &tag_dao.tag_file("test.jpg", tag3.id).unwrap(); + + let mut files = HashMap::new(); + files.insert( + String::from(""), + vec![ + String::from("file1.txt"), + String::from("test.jpg"), + String::from("some-other.jpg"), + ], + ); + + let response: HttpResponse = list_photos( + claims, + request, + Data::new(AppState::new( + Arc::new(StreamActor {}.start()), + String::from(""), + String::from("/tmp/thumbs"), + )), + Data::new(FakeFileSystem::new(files)), + Data::new(Mutex::new(tag_dao)), + ) + .await; + + assert_eq!(200, response.status()); + + let body: PhotosResponse = serde_json::from_str(&response.read_to_str()).unwrap(); + assert_eq!(1, body.photos.len()); + assert!(body.photos.contains(&String::from("test.jpg"))); + } + + #[actix_rt::test] + async fn get_files_with_tag_all_filter() { + setup(); + + let claims = Claims { + sub: String::from("1"), + exp: 12345, + }; + + let mut tag_dao = SqliteTagDao::new(in_memory_db_connection()); + + let tag1 = tag_dao.create_tag("tag1").unwrap(); + let tag2 = tag_dao.create_tag("tag2").unwrap(); + let tag3 = tag_dao.create_tag("tag3").unwrap(); + + &tag_dao.tag_file("test.jpg", tag1.id).unwrap(); + &tag_dao.tag_file("test.jpg", tag3.id).unwrap(); + + // Should get filtered since it doesn't have tag3 + tag_dao.tag_file("some-other.jpg", tag1.id).unwrap(); + + let mut files = HashMap::new(); + files.insert( + String::from(""), + vec![ + String::from("file1.txt"), + String::from("test.jpg"), + String::from("some-other.jpg"), + ], + ); + + let request: Query = + Query::from_query("path=&tag_ids=1,3&tag_filter_mode=All").unwrap(); + + let response: HttpResponse = list_photos( + claims, + request, + Data::new(AppState::new( + Arc::new(StreamActor {}.start()), + String::from(""), + String::from("/tmp/thumbs"), + )), + Data::new(FakeFileSystem::new(files)), + Data::new(Mutex::new(tag_dao)), + ) + .await; + + assert_eq!(200, response.status()); + + let body: PhotosResponse = serde_json::from_str(&response.read_to_str()).unwrap(); + assert_eq!(1, body.photos.len()); + assert!(body.photos.contains(&String::from("test.jpg"))); + } } #[test] fn directory_traversal_test() { let base = env::temp_dir(); - assert_eq!(None, is_valid_full_path(&base, "../")); - assert_eq!(None, is_valid_full_path(&base, "..")); - assert_eq!(None, is_valid_full_path(&base, "fake/../../../")); - assert_eq!(None, is_valid_full_path(&base, "../../../etc/passwd")); - assert_eq!(None, is_valid_full_path(&base, "..//etc/passwd")); - assert_eq!(None, is_valid_full_path(&base, "../../etc/passwd")); + assert_eq!(None, is_valid_full_path(&base, &PathBuf::from("../"))); + assert_eq!(None, is_valid_full_path(&base, &PathBuf::from(".."))); + assert_eq!( + None, + is_valid_full_path(&base, &PathBuf::from("fake/../../../")) + ); + assert_eq!( + None, + is_valid_full_path(&base, &PathBuf::from("../../../etc/passwd")) + ); + assert_eq!( + None, + is_valid_full_path(&base, &PathBuf::from("..//etc/passwd")) + ); + assert_eq!( + None, + is_valid_full_path(&base, &PathBuf::from("../../etc/passwd")) + ); } #[test] @@ -277,7 +463,7 @@ mod tests { test_file.push("test.png"); File::create(test_file).unwrap(); - assert!(is_valid_full_path(&base, "test.png").is_some()); + assert!(is_valid_full_path(&base, &PathBuf::from("test.png")).is_some()); } #[test] @@ -288,7 +474,7 @@ mod tests { let mut test_file = PathBuf::from(&base); test_file.push(path); - assert_eq!(None, is_valid_full_path(&base, path)); + assert_eq!(None, is_valid_full_path(&base, &test_file)); } #[test] @@ -298,11 +484,11 @@ mod tests { test_file.push("test.png"); File::create(&test_file).unwrap(); - assert!(is_valid_full_path(&base, test_file.to_str().unwrap()).is_some()); + assert!(is_valid_full_path(&base, &test_file).is_some()); assert_eq!( Some(PathBuf::from("/tmp/test.png")), - is_valid_full_path(&base, "/tmp/test.png") + is_valid_full_path(&base, &PathBuf::from("/tmp/test.png")) ); } diff --git a/src/main.rs b/src/main.rs index 5b02c45..ad09092 100644 --- a/src/main.rs +++ b/src/main.rs @@ -35,7 +35,7 @@ use log::{debug, error, info}; use crate::auth::login; use crate::data::*; use crate::database::*; -use crate::files::{is_image_or_video, is_valid_full_path}; +use crate::files::{is_image_or_video, is_valid_full_path, RealFileSystem}; use crate::service::ServiceBuilder; use crate::state::AppState; use crate::tags::*; @@ -156,9 +156,10 @@ async fn upload_image( let path = file_path.unwrap_or_else(|| app_state.base_path.clone()); if !file_content.is_empty() { let full_path = PathBuf::from(&path).join(file_name.unwrap()); - if let Some(full_path) = - is_valid_full_path(&app_state.base_path, full_path.to_str().unwrap_or("")) - { + if let Some(full_path) = is_valid_full_path( + &app_state.base_path, + &full_path.to_str().unwrap().to_string(), + ) { if !full_path.is_file() && is_image_or_video(&full_path) { let mut file = File::create(full_path).unwrap(); file.write_all(&file_content).unwrap(); @@ -474,7 +475,10 @@ fn main() -> std::io::Result<()> { App::new() .wrap(middleware::Logger::default()) .service(web::resource("/login").route(web::post().to(login::))) - .service(web::resource("/photos").route(web::get().to(files::list_photos::))) + .service( + web::resource("/photos") + .route(web::get().to(files::list_photos::)), + ) .service(get_image) .service(upload_image) .service(generate_video) @@ -486,6 +490,7 @@ fn main() -> std::io::Result<()> { .service(get_file_metadata) .add_feature(add_tag_services::<_, SqliteTagDao>) .app_data(app_data.clone()) + .app_data::>(Data::new(RealFileSystem::new(app_data.base_path.clone()))) .app_data::>>(Data::new(Mutex::new(user_dao))) .app_data::>>>(Data::new(Mutex::new(Box::new( favorites_dao,