Merge pull request 'Improve testability and remove boxing' (#20) from feature/testing-improvements into master
All checks were successful
Core Repos/ImageApi/pipeline/head This commit looks good

Reviewed-on: #20
This commit was merged in pull request #20.
This commit is contained in:
2022-03-18 00:25:51 +00:00
5 changed files with 183 additions and 129 deletions

View File

@@ -1,4 +1,4 @@
use actix_web::{post, Responder};
use actix_web::Responder;
use actix_web::{
web::{self, Json},
HttpResponse,
@@ -12,10 +12,10 @@ use crate::{
database::UserDao,
};
#[post("/register")]
async fn register(
#[allow(dead_code)]
async fn register<D: UserDao>(
user: Json<CreateAccountRequest>,
user_dao: web::Data<Box<dyn UserDao>>,
user_dao: web::Data<D>,
) -> impl Responder {
if !user.username.is_empty() && user.password.len() > 5 && user.password == user.confirmation {
if user_dao.user_exists(&user.username) {
@@ -30,10 +30,7 @@ async fn register(
}
}
pub async fn login(
creds: Json<LoginRequest>,
user_dao: web::Data<Box<dyn UserDao>>,
) -> HttpResponse {
pub async fn login<D: UserDao>(creds: Json<LoginRequest>, user_dao: web::Data<D>) -> HttpResponse {
debug!("Logging in: {}", creds.username);
if let Some(user) = user_dao.get_user(&creds.username, &creds.password) {
@@ -74,7 +71,7 @@ mod tests {
password: "pass".to_string(),
});
let response = login(j, web::Data::new(Box::new(dao))).await;
let response = login::<TestUserDao>(j, web::Data::new(dao)).await;
assert_eq!(response.status(), 200);
}
@@ -89,7 +86,7 @@ mod tests {
password: "password".to_string(),
});
let response = login(j, web::Data::new(Box::new(dao))).await;
let response = login::<TestUserDao>(j, web::Data::new(dao)).await;
assert_eq!(response.status(), 200);
let response_text: String = response.read_to_str();
@@ -107,7 +104,7 @@ mod tests {
password: "password".to_string(),
});
let response = login(j, web::Data::new(Box::new(dao))).await;
let response = login::<TestUserDao>(j, web::Data::new(dao)).await;
assert_eq!(response.status(), 404);
}

View File

@@ -1,3 +1,4 @@
use std::fmt::Debug;
use std::fs::read_dir;
use std::io;
use std::path::{Path, PathBuf};
@@ -5,17 +6,25 @@ use std::path::{Path, PathBuf};
use ::anyhow;
use anyhow::{anyhow, Context};
use actix_web::{web::Query, HttpResponse};
use actix_web::{
web::{self, Query},
HttpResponse,
};
use log::{debug, error};
use crate::data::{Claims, PhotosResponse, ThumbnailRequest};
use crate::AppState;
use path_absolutize::*;
pub async fn list_photos(_: Claims, req: Query<ThumbnailRequest>) -> HttpResponse {
pub async fn list_photos(
_: Claims,
req: Query<ThumbnailRequest>,
app_state: web::Data<AppState>,
) -> HttpResponse {
let path = &req.path;
if let Some(path) = is_valid_path(path) {
if let Some(path) = is_valid_full_path(&PathBuf::from(&app_state.base_path), path) {
debug!("Valid path: {:?}", path);
let files = list_files(&path).unwrap_or_default();
@@ -31,9 +40,7 @@ pub async fn list_photos(_: Claims, req: Query<ThumbnailRequest>) -> HttpRespons
)
})
.map(|path: &PathBuf| {
let relative = path
.strip_prefix(dotenv::var("BASE_PATH").unwrap())
.unwrap();
let relative = path.strip_prefix(&app_state.base_path).unwrap();
relative.to_path_buf()
})
.map(|f| f.to_str().unwrap().to_string())
@@ -43,9 +50,7 @@ pub async fn list_photos(_: Claims, req: Query<ThumbnailRequest>) -> HttpRespons
.iter()
.filter(|&f| f.metadata().map_or(false, |md| md.is_dir()))
.map(|path: &PathBuf| {
let relative = path
.strip_prefix(dotenv::var("BASE_PATH").unwrap())
.unwrap();
let relative = path.strip_prefix(&app_state.base_path).unwrap();
relative.to_path_buf()
})
.map(|f| f.to_str().unwrap().to_string())
@@ -82,18 +87,13 @@ pub fn is_image_or_video(path: &Path) -> bool {
|| extension == "nef"
}
pub fn is_valid_path(path: &str) -> Option<PathBuf> {
let base = PathBuf::from(dotenv::var("BASE_PATH").unwrap());
is_valid_full_path(&base, path)
}
fn is_valid_full_path(base: &Path, path: &str) -> Option<PathBuf> {
pub fn is_valid_full_path<P: AsRef<Path> + Debug>(base: &P, path: &str) -> Option<PathBuf> {
debug!("Base: {:?}. Path: {}", base, path);
let path = PathBuf::from(path);
let mut path = if path.is_relative() {
let mut full_path = PathBuf::from(base);
let mut full_path = PathBuf::new();
full_path.push(base);
full_path.push(&path);
full_path
} else {
@@ -109,7 +109,10 @@ fn is_valid_full_path(base: &Path, path: &str) -> Option<PathBuf> {
}
}
fn is_path_above_base_dir(base: &Path, full_path: &mut PathBuf) -> anyhow::Result<PathBuf> {
fn is_path_above_base_dir<P: AsRef<Path> + Debug>(
base: P,
full_path: &mut PathBuf,
) -> anyhow::Result<PathBuf> {
full_path
.absolutize()
.with_context(|| format!("Unable to resolve absolute path: {:?}", full_path))
@@ -135,15 +138,21 @@ mod tests {
use super::*;
mod api {
use actix_web::{web::Query, HttpResponse};
use super::*;
use actix::Actor;
use actix_web::{
web::{self, Query},
HttpResponse,
};
use super::list_photos;
use crate::{
data::{Claims, PhotosResponse, ThumbnailRequest},
testhelpers::BodyReader,
video::StreamActor,
AppState,
};
use std::fs;
use std::{fs, sync::Arc};
fn setup() {
let _ = env_logger::builder().is_test(true).try_init();
@@ -160,7 +169,6 @@ mod tests {
let request: Query<ThumbnailRequest> = Query::from_query("path=").unwrap();
std::env::set_var("BASE_PATH", "/tmp");
let mut temp_photo = std::env::temp_dir();
let mut tmp = temp_photo.clone();
@@ -169,9 +177,18 @@ mod tests {
temp_photo.push("photo.jpg");
fs::File::create(temp_photo).unwrap();
fs::File::create(temp_photo.clone()).unwrap();
let response: HttpResponse = list_photos(claims, request).await;
let response: HttpResponse = list_photos(
claims,
request,
web::Data::new(AppState::new(
Arc::new(StreamActor {}.start()),
String::from("/tmp"),
String::from("/tmp/thumbs"),
)),
)
.await;
let status = response.status();
let body: PhotosResponse = serde_json::from_str(&response.read_to_str()).unwrap();
@@ -200,7 +217,16 @@ mod tests {
let request: Query<ThumbnailRequest> = Query::from_query("path=..").unwrap();
let response = list_photos(claims, request).await;
let response = list_photos(
claims,
request,
web::Data::new(AppState::new(
Arc::new(StreamActor {}.start()),
String::from("/tmp"),
String::from("/tmp/thumbs"),
)),
)
.await;
assert_eq!(response.status(), 400);
}
@@ -208,12 +234,13 @@ mod tests {
#[test]
fn directory_traversal_test() {
assert_eq!(None, is_valid_path("../"));
assert_eq!(None, is_valid_path(".."));
assert_eq!(None, is_valid_path("fake/../../../"));
assert_eq!(None, is_valid_path("../../../etc/passwd"));
assert_eq!(None, is_valid_path("..//etc/passwd"));
assert_eq!(None, is_valid_path("../../etc/passwd"));
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"));
}
#[test]

View File

@@ -7,7 +7,7 @@ use actix_web_prom::PrometheusMetricsBuilder;
use futures::stream::StreamExt;
use lazy_static::lazy_static;
use prometheus::{self, IntGauge};
use std::sync::{mpsc::channel, Arc};
use std::sync::mpsc::channel;
use std::{collections::HashMap, io::prelude::*};
use std::{env, fs::File};
use std::{
@@ -16,7 +16,6 @@ use std::{
};
use walkdir::{DirEntry, WalkDir};
use actix::prelude::*;
use actix_files::NamedFile;
use actix_multipart as mp;
use actix_web::{
@@ -32,13 +31,15 @@ use log::{debug, error, info};
use crate::auth::login;
use crate::data::*;
use crate::database::*;
use crate::files::{is_image_or_video, is_valid_path};
use crate::files::{is_image_or_video, is_valid_full_path};
use crate::state::AppState;
use crate::video::*;
mod auth;
mod data;
mod database;
mod files;
mod state;
mod video;
#[cfg(test)]
@@ -62,13 +63,15 @@ async fn get_image(
_claims: Claims,
request: HttpRequest,
req: web::Query<ThumbnailRequest>,
app_state: web::Data<AppState>,
) -> impl Responder {
if let Some(path) = is_valid_path(&req.path) {
if let Some(path) = is_valid_full_path(&app_state.base_path, &req.path) {
if req.size.is_some() {
let thumbs = dotenv::var("THUMBNAILS").unwrap();
let relative_path = path
.strip_prefix(dotenv::var("BASE_PATH").unwrap())
.expect("Error stripping prefix");
.strip_prefix(&app_state.base_path)
.expect("Error stripping base path prefix from thumbnail");
let thumbs = &app_state.thumbnail_path;
let thumb_path = Path::new(&thumbs).join(relative_path);
debug!("{:?}", thumb_path);
@@ -89,8 +92,12 @@ async fn get_image(
}
#[get("/image/metadata")]
async fn get_file_metadata(_: Claims, path: web::Query<ThumbnailRequest>) -> impl Responder {
match is_valid_path(&path.path)
async fn get_file_metadata(
_: Claims,
path: web::Query<ThumbnailRequest>,
app_state: web::Data<AppState>,
) -> impl Responder {
match is_valid_full_path(&app_state.base_path, &path.path)
.ok_or_else(|| ErrorKind::InvalidData.into())
.and_then(File::open)
.and_then(|file| file.metadata())
@@ -107,7 +114,11 @@ async fn get_file_metadata(_: Claims, path: web::Query<ThumbnailRequest>) -> imp
}
#[post("/image")]
async fn upload_image(_: Claims, mut payload: mp::Multipart) -> impl Responder {
async fn upload_image(
_: Claims,
mut payload: mp::Multipart,
app_state: web::Data<AppState>,
) -> impl Responder {
let mut file_content: BytesMut = BytesMut::new();
let mut file_name: Option<String> = None;
let mut file_path: Option<String> = None;
@@ -131,10 +142,12 @@ async fn upload_image(_: Claims, mut payload: mp::Multipart) -> impl Responder {
}
}
let path = file_path.unwrap_or_else(|| dotenv::var("BASE_PATH").unwrap());
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_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_or(""))
{
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();
@@ -155,7 +168,7 @@ async fn upload_image(_: Claims, mut payload: mp::Multipart) -> impl Responder {
#[post("/video/generate")]
async fn generate_video(
_claims: Claims,
data: web::Data<AppState>,
app_state: web::Data<AppState>,
body: web::Json<ThumbnailRequest>,
) -> impl Responder {
let filename = PathBuf::from(&body.path);
@@ -163,9 +176,10 @@ async fn generate_video(
if let Some(name) = filename.file_stem() {
let filename = name.to_str().expect("Filename should convert to string");
let playlist = format!("tmp/{}.m3u8", filename);
if let Some(path) = is_valid_path(&body.path) {
if let Some(path) = is_valid_full_path(&app_state.base_path, &body.path) {
if let Ok(child) = create_playlist(path.to_str().unwrap(), &playlist).await {
data.stream_manager
app_state
.stream_manager
.do_send(ProcessMessage(playlist.clone(), child));
}
} else {
@@ -184,12 +198,13 @@ async fn stream_video(
request: HttpRequest,
_: Claims,
path: web::Query<ThumbnailRequest>,
app_state: web::Data<AppState>,
) -> impl Responder {
let playlist = &path.path;
debug!("Playlist: {}", playlist);
// Extract video playlist dir to dotenv
if !playlist.starts_with("tmp") && is_valid_path(playlist) != None {
if !playlist.starts_with("tmp") && is_valid_full_path(&app_state.base_path, playlist) != None {
HttpResponse::BadRequest().finish()
} else if let Ok(file) = NamedFile::open(playlist) {
file.into_response(&request)
@@ -406,7 +421,56 @@ fn main() -> std::io::Result<()> {
env_logger::init();
create_thumbnails();
watch_files();
let system = actix::System::new();
system.block_on(async {
let app_data = web::Data::new(AppState::default());
let labels = HashMap::new();
let prometheus = PrometheusMetricsBuilder::new("api")
.const_labels(labels)
.build()
.expect("Unable to build prometheus metrics middleware");
prometheus
.registry
.register(Box::new(IMAGE_GAUGE.clone()))
.unwrap();
prometheus
.registry
.register(Box::new(VIDEO_GAUGE.clone()))
.unwrap();
HttpServer::new(move || {
let user_dao = SqliteUserDao::new();
let favorites_dao = SqliteFavoriteDao::new();
App::new()
.wrap(middleware::Logger::default())
.service(web::resource("/login").route(web::post().to(login::<SqliteUserDao>)))
.service(web::resource("/photos").route(web::get().to(files::list_photos)))
.service(get_image)
.service(upload_image)
.service(generate_video)
.service(stream_video)
.service(get_video_part)
.service(favorites)
.service(put_add_favorite)
.service(delete_favorite)
.service(get_file_metadata)
.app_data(app_data.clone())
.app_data::<Data<SqliteUserDao>>(Data::new(user_dao))
.app_data::<Data<Box<dyn FavoriteDao>>>(Data::new(Box::new(favorites_dao)))
.wrap(prometheus.clone())
})
.bind(dotenv::var("BIND_URL").unwrap())?
.bind("localhost:8088")?
.run()
.await
})
}
fn watch_files() {
std::thread::spawn(|| {
let (wtx, wrx) = channel();
let mut watcher = watcher(wtx, std::time::Duration::from_secs(10)).unwrap();
@@ -449,58 +513,4 @@ fn main() -> std::io::Result<()> {
}
}
});
let system = actix::System::new();
system.block_on(async {
let act = StreamActor {}.start();
let app_data = web::Data::new(AppState {
stream_manager: Arc::new(act),
});
let labels = HashMap::new();
let prometheus = PrometheusMetricsBuilder::new("api")
.const_labels(labels)
.build()
.expect("Unable to build prometheus metrics middleware");
prometheus
.registry
.register(Box::new(IMAGE_GAUGE.clone()))
.unwrap();
prometheus
.registry
.register(Box::new(VIDEO_GAUGE.clone()))
.unwrap();
HttpServer::new(move || {
let user_dao = SqliteUserDao::new();
let favorites_dao = SqliteFavoriteDao::new();
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(get_image)
.service(upload_image)
.service(generate_video)
.service(stream_video)
.service(get_video_part)
.service(favorites)
.service(put_add_favorite)
.service(delete_favorite)
.service(get_file_metadata)
.app_data(app_data.clone())
.app_data::<Data<Box<dyn UserDao>>>(Data::new(Box::new(user_dao)))
.app_data::<Data<Box<dyn FavoriteDao>>>(Data::new(Box::new(favorites_dao)))
.wrap(prometheus.clone())
})
.bind(dotenv::var("BIND_URL").unwrap())?
.bind("localhost:8088")?
.run()
.await
})
}
struct AppState {
stream_manager: Arc<Addr<StreamActor>>,
}

33
src/state.rs Normal file
View File

@@ -0,0 +1,33 @@
use crate::StreamActor;
use actix::{Actor, Addr};
use std::{env, sync::Arc};
pub struct AppState {
pub stream_manager: Arc<Addr<StreamActor>>,
pub base_path: String,
pub thumbnail_path: String,
}
impl AppState {
pub fn new(
stream_manager: Arc<Addr<StreamActor>>,
base_path: String,
thumbnail_path: String,
) -> Self {
Self {
stream_manager,
base_path,
thumbnail_path,
}
}
}
impl Default for AppState {
fn default() -> Self {
Self::new(
Arc::new(StreamActor {}.start()),
env::var("BASE_PATH").expect("BASE_PATH was not set in the env"),
env::var("THUMBNAILS").expect("THUMBNAILS was not set in the env"),
)
}
}

View File

@@ -1,6 +1,7 @@
use actix_web::body::MessageBody;
use actix_web::{body::BoxBody, HttpResponse};
use serde::de::DeserializeOwned;
use actix_web::{
body::{BoxBody, MessageBody},
HttpResponse,
};
use crate::database::{models::User, UserDao};
use std::cell::RefCell;
@@ -65,17 +66,3 @@ impl BodyReader for HttpResponse<BoxBody> {
std::str::from_utf8(&body).unwrap().to_string()
}
}
pub trait TypedBodyReader<T>
where
T: DeserializeOwned,
{
fn read_body(self) -> T;
}
impl<T: DeserializeOwned> TypedBodyReader<T> for HttpResponse<BoxBody> {
fn read_body(self) -> T {
let body = self.read_to_str();
serde_json::from_value(serde_json::Value::String(body.clone())).unwrap()
}
}