Moved uploaded banner and generated styles to a persistent data directory. Fixes #9

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/backend/appearance.rs

@@ -7,12 +7,13 @@ use crate::components::data_form::ForValidation;
 cfg_if! { if #[cfg(feature = "ssr")] {
 
     use std::fs;
+    use std::path::Path;
     use actix_web::{post, Responder};
     use actix_multipart::Multipart;
     use actix_session::Session;
     use actix_web::web::Redirect;
     use std::fs::File;
-    use std::io::{Write, Read};
+    use std::io::Write;
     use futures_util::{StreamExt, TryStreamExt};
     use crate::backend::data::User;
     use crate::error::AppError;
@@ -23,8 +24,44 @@ cfg_if! { if #[cfg(feature = "ssr")] {
     use regex::Regex;
     use rand::Rng;
     use rand::distributions::Alphanumeric;
+    use log::{info, warn};
 
-    pub async fn check_appearance(pool: &PgPool) -> Result<(), AppError> {
+    /// Style of the public page banner. The template is shipped with the application,
+    /// the copy in the data directory is the base for generated styles.
+    const BANNER_CSS: &str = "banner.css";
+    const BANNER_EXTENSIONS: [&str; 5] = ["jpg", "jpeg", "png", "gif", "webp"];
+
+    /// Turns the file name sent by the browser into a safe name inside the data directory.
+    /// Returns `None` when the file is not an image that can be used as a banner.
+    pub fn banner_file_name(name: &str) -> Option<String> {
+        // Only the last path segment, whatever separator the client used
+        let name = name.rsplit(['/', '\\']).next().unwrap_or_default();
+        let (stem, extension) = name.rsplit_once('.')?;
+
+        if stem.is_empty() || !BANNER_EXTENSIONS.contains(&extension.to_lowercase().as_str()) {
+            return None
+        }
+
+        let stem = stem.chars()
+            .map(|c| if c.is_ascii_alphanumeric() || c == '-' || c == '_' { c } else { '_' })
+            .collect::<String>();
+
+        Some(format!("{}.{}", stem, extension.to_lowercase()))
+    }
+
+    /// Creates the data directory and puts the banner style template into it, if it is not there yet
+    pub fn ensure_data_dir(data_dir: &Path, site_root: &Path) -> Result<(), AppError> {
+        fs::create_dir_all(data_dir)?;
+
+        if !data_dir.join(BANNER_CSS).exists() {
+            info!("Copying {} to data directory {}", BANNER_CSS, data_dir.display());
+            fs::copy(site_root.join(BANNER_CSS), data_dir.join(BANNER_CSS))?;
+        }
+
+        Ok(())
+    }
+
+    pub async fn check_appearance(pool: &PgPool, data_dir: &Path) -> Result<(), AppError> {
         let count: (i64,) = query_as("SELECT COUNT(id) FROM appearance")
             .fetch_one(pool)
             .await?;
@@ -37,7 +74,22 @@ cfg_if! { if #[cfg(feature = "ssr")] {
 
         let app = query_as::<_, Appearance>("SELECT * FROM appearance").fetch_one(pool).await?;
         if let None = app.css_name {
-            query("UPDATE appearance SET css_name = 'banner.css'").execute(pool).await?;
+            query("UPDATE appearance SET css_name = $1").bind(BANNER_CSS).execute(pool).await?;
+        }
+
+        // The generated style can be missing, e.g. when the data directory was not kept during an upgrade
+        let css_name = app.css_name.unwrap_or(BANNER_CSS.to_string());
+        if !data_dir.join(&css_name).exists() {
+            match app.banner {
+                Some(banner) if data_dir.join(&banner).exists() => {
+                    warn!("Banner style {} is missing, generating it again", css_name);
+                    modify_style(&banner, pool, data_dir).await?;
+                }
+                _ => {
+                    warn!("Banner style {} is missing, using default style", css_name);
+                    query("UPDATE appearance SET css_name = $1").bind(BANNER_CSS).execute(pool).await?;
+                }
+            }
         }
 
         Ok(())
@@ -52,10 +104,8 @@ cfg_if! { if #[cfg(feature = "ssr")] {
         Ok(())
     }
 
-    async fn modify_style(file_name: &str, pool: &PgPool) -> Result<(), AppError> {
-        let mut css_file = File::open("target/site/data/banner.css")?;
-        let mut css_str= String::new();
-        css_file.read_to_string(&mut css_str)?;
+    async fn modify_style(file_name: &str, pool: &PgPool, data_dir: &Path) -> Result<(), AppError> {
+        let mut css_str = fs::read_to_string(data_dir.join(BANNER_CSS))?;
 
         if css_str.contains("/*bg-img*/") {
             css_str = css_str.replace("/*bg-img*/", &format!("background-image: url('{}');", file_name));
@@ -64,12 +114,15 @@ cfg_if! { if #[cfg(feature = "ssr")] {
             css_str = re.replace(&css_str, &format!("background-image: url('{}')", file_name)).to_string();
         }
 
-        let old_css: (String,) = query_as("SELECT css_name FROM appearance")
+        let old_css: (Option<String>,) = query_as("SELECT css_name FROM appearance")
             .fetch_one(pool)
             .await?;
 
-        if old_css.0 != "banner.css" {
-            fs::remove_file(format!("target/site/data/{}", old_css.0))?;
+        if let Some(old_css) = old_css.0 {
+            if old_css != BANNER_CSS {
+                // The old style may be already gone
+                let _ = fs::remove_file(data_dir.join(old_css));
+            }
         }
 
         let s: String = rand::thread_rng()
@@ -78,7 +131,7 @@ cfg_if! { if #[cfg(feature = "ssr")] {
             .map(char::from)
             .collect();
         let css_name = format!("banner-{}.css", s);
-        let mut css_file = File::create(format!("target/site/data/{}", css_name))?;
+        let mut css_file = File::create(data_dir.join(&css_name))?;
         css_file.write_all(css_str.as_bytes())?;
         query("UPDATE appearance SET css_name = $1")
             .bind(css_name)
@@ -88,6 +141,30 @@ cfg_if! { if #[cfg(feature = "ssr")] {
         Ok(())
     }
 
+    async fn save_banner(field: &mut actix_multipart::Field, file_name: &str, app_data: &AppData) -> Result<(), AppError> {
+        let data_dir = app_data.data_dir();
+        let old_banner: (Option<String>,) = query_as("SELECT banner FROM appearance")
+            .fetch_one(app_data.db_pool())
+            .await?;
+
+        let mut file = File::create(data_dir.join(file_name))?;
+        while let Some(chunk) = field.next().await {
+            let chunk = chunk.map_err(|e| AppError::FileIOError(e.to_string()))?;
+            file.write_all(&chunk)?;
+        }
+
+        set_banner_name(file_name, app_data.db_pool()).await?;
+        modify_style(file_name, app_data.db_pool(), data_dir).await?;
+
+        if let Some(old_banner) = old_banner.0 {
+            if old_banner != file_name {
+                let _ = fs::remove_file(data_dir.join(old_banner));
+            }
+        }
+
+        Ok(())
+    }
+
     #[post("/upload")]
     pub async fn upload(mut data: Multipart, session: Session, app_data: Data<AppData>) -> impl Responder {
         let user = session.get::<User>("user").unwrap_or(None);
@@ -103,21 +180,23 @@ cfg_if! { if #[cfg(feature = "ssr")] {
         }
 
         if let Ok(Some(mut field)) = data.try_next().await {
-            let content_disp = field.content_disposition();
-            let file_name = content_disp.unwrap().get_filename().unwrap().to_string();
+            let sent_name = field.content_disposition()
+                .and_then(|cd| cd.get_filename())
+                .unwrap_or_default()
+                .to_string();
 
-            if file_name.is_empty() {
-                return Redirect::to("/admin/appearance").see_other();
+            match banner_file_name(&sent_name) {
+                Some(file_name) => {
+                    if let Err(e) = save_banner(&mut field, &file_name, &app_data).await {
+                        warn!("Banner {} not saved: {}", file_name, e);
+                    }
+                }
+                None => {
+                    if !sent_name.is_empty() {
+                        warn!("Uploaded file {:?} is not a supported image", sent_name);
+                    }
+                }
             }
-
-            let mut file = File::create(format!("target/site/data/{}", file_name)).unwrap();
-            let _name = field.name();
-            while let Some(chunk) = field.next().await {
-                let c = chunk.unwrap();
-                let _ = file.write_all(&c);
-            }
-            let _ = set_banner_name(&file_name, &app_data.db_pool).await;
-            let _ = modify_style(&file_name, &app_data.db_pool).await;
         }
 
         Redirect::to("/admin/appearance").see_other()
@@ -163,11 +242,13 @@ impl ForValidation for UpdateAppearance {
 pub async fn delete_banner() -> Result<ApiResponse<()>, ServerFnError> {
     use std::fs;
     use crate::perm_check;
+    use crate::backend::get_data_dir;
 
     perm_check!(is_admin);
 
     let appearance = get_appearance().await?;
     let pool = get_pool().await?;
+    let data_dir = get_data_dir().await?;
     query("UPDATE appearance SET banner = $1 WHERE id = $2")
         .bind(None::<String>)
         .bind(appearance.id())
@@ -175,8 +256,71 @@ pub async fn delete_banner() -> Result<ApiResponse<()>, ServerFnError> {
         .await?;
 
     if let Some(f) = appearance.banner {
-        fs::remove_file(format!("target/site/data/{}", f))?;
+        // The file may be already gone
+        let _ = fs::remove_file(data_dir.join(f));
     }
 
     Ok(ApiResponse::Data(()))
-}
+}
+
+#[cfg(all(test, feature = "ssr"))]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn banner_file_name_keeps_simple_names() {
+        assert_eq!(banner_file_name("banner1.jpg"), Some("banner1.jpg".to_string()));
+        assert_eq!(banner_file_name("my-banner_2.PNG"), Some("my-banner_2.png".to_string()));
+    }
+
+    #[test]
+    fn banner_file_name_strips_directories() {
+        assert_eq!(banner_file_name("../../etc/banner.jpg"), Some("banner.jpg".to_string()));
+        assert_eq!(banner_file_name("/etc/banner.jpg"), Some("banner.jpg".to_string()));
+        assert_eq!(banner_file_name("C:\\Users\\joe\\banner.jpg"), Some("banner.jpg".to_string()));
+        assert_eq!(banner_file_name("..\\..\\banner.jpg"), Some("banner.jpg".to_string()));
+    }
+
+    #[test]
+    fn banner_file_name_replaces_unsafe_characters() {
+        assert_eq!(banner_file_name("můj banner (1).jpg"), Some("m_j_banner__1_.jpg".to_string()));
+        assert_eq!(banner_file_name("a'b\"c.gif"), Some("a_b_c.gif".to_string()));
+        assert_eq!(banner_file_name("archive.tar.webp"), Some("archive_tar.webp".to_string()));
+    }
+
+    #[test]
+    fn banner_file_name_rejects_other_files() {
+        for name in ["", "banner", "banner.css", "config.toml", "index.html", "script.js", "image.svg", ".jpg", "..", "dir/"] {
+            assert_eq!(banner_file_name(name), None, "name: {:?}", name);
+        }
+    }
+
+    #[test]
+    fn ensure_data_dir_copies_template_once() {
+        let base = std::env::temp_dir().join(format!("rezervator-test-data-{}", std::process::id()));
+        let site_root = base.join("site");
+        let data_dir = base.join("data/nested");
+        fs::create_dir_all(&site_root).unwrap();
+        fs::write(site_root.join(BANNER_CSS), "template").unwrap();
+
+        ensure_data_dir(&data_dir, &site_root).unwrap();
+        assert_eq!(fs::read_to_string(data_dir.join(BANNER_CSS)).unwrap(), "template");
+
+        // An existing file is not overwritten
+        fs::write(data_dir.join(BANNER_CSS), "customized").unwrap();
+        ensure_data_dir(&data_dir, &site_root).unwrap();
+        assert_eq!(fs::read_to_string(data_dir.join(BANNER_CSS)).unwrap(), "customized");
+
+        fs::remove_dir_all(base).unwrap();
+    }
+
+    #[test]
+    fn ensure_data_dir_fails_without_template() {
+        let base = std::env::temp_dir().join(format!("rezervator-test-notemplate-{}", std::process::id()));
+        fs::create_dir_all(base.join("site")).unwrap();
+
+        assert!(ensure_data_dir(&base.join("data"), &base.join("site")).is_err());
+
+        fs::remove_dir_all(base).unwrap();
+    }
+}

src/backend/mod.rs

@@ -56,21 +56,29 @@ cfg_if!{
         use leptos_actix::extract;
         use leptos::prelude::ServerFnError;
         use crate::backend::mail::Mailer;
+        use std::path::PathBuf;
 
         #[derive(Clone)]
         pub struct AppData {
             db_pool: PgPool,
-            mailer: Mailer
+            mailer: Mailer,
+            data_dir: PathBuf
         }
 
         impl AppData {
-            pub fn new(db_pool: PgPool, mailer: Mailer) -> Self {
+            pub fn new(db_pool: PgPool, mailer: Mailer, data_dir: PathBuf) -> Self {
                 Self {
                     db_pool,
-                    mailer
+                    mailer,
+                    data_dir
                 }
             }
 
+            /// Directory for files created at runtime, served under `/data`
+            pub fn data_dir(&self) -> &PathBuf {
+                &self.data_dir
+            }
+
             pub fn db_pool(&self) -> &PgPool {
                 &self.db_pool
             }
@@ -85,6 +93,11 @@ cfg_if!{
             Ok(data.db_pool().clone())
         }
 
+        pub async fn get_data_dir() -> Result<PathBuf, ServerFnError> {
+            let data = extract::<Data<AppData>>().await?;
+            Ok(data.data_dir().clone())
+        }
+
         pub async fn get_mailing() -> Result<Mailer, ServerFnError> {
             let data = extract::<Data<AppData>>().await?;
             Ok(data.mailer().clone())

src/config.rs

@@ -3,7 +3,7 @@ use std::fs::read_to_string;
 #[cfg(feature = "ssr")]
 use std::net::SocketAddr;
 #[cfg(feature = "ssr")]
-use std::path::Path;
+use std::path::{Path, PathBuf};
 #[cfg(feature = "ssr")]
 use std::str::FromStr;
 #[cfg(feature = "ssr")]
@@ -122,6 +122,21 @@ impl Logging {
     }
 }
 
+/// Where files created at runtime (uploaded banner, generated CSS) are kept
+#[cfg(feature = "ssr")]
+#[derive(Deserialize, Default)]
+pub struct Storage {
+    data_dir: Option<String>
+}
+
+#[cfg(feature = "ssr")]
+impl Storage {
+    /// Directory served under `/data`. Defaults to `data` in the working directory.
+    pub fn data_dir(&self) -> PathBuf {
+        PathBuf::from(self.data_dir.as_deref().unwrap_or("data"))
+    }
+}
+
 #[cfg(feature = "ssr")]
 #[derive(Deserialize)]
 pub struct Configuration {
@@ -129,7 +144,9 @@ pub struct Configuration {
     network: Network,
     database: Database,
     mailing: Mailing,
-    logging: Logging
+    logging: Logging,
+    #[serde(default)]
+    storage: Storage
 }
 
 #[cfg(feature = "ssr")]
@@ -149,13 +166,16 @@ impl Configuration {
     pub fn logging(&self) -> &Logging {
         &self.logging
     }
+    pub fn storage(&self) -> &Storage {
+        &self.storage
+    }
 }
 
-/// The config file holds passwords and the session key, so it must never lie in the directory
-/// that is served as static files. Returns true when `config_path` is inside `site_root`.
+/// The config file holds passwords and the session key, so it must never lie in a directory
+/// that is served as static files. Returns true when `config_path` is inside `public_dir`.
 #[cfg(feature = "ssr")]
-pub fn is_publicly_served(config_path: &Path, site_root: &Path) -> bool {
-    match (config_path.canonicalize(), site_root.canonicalize()) {
+pub fn is_publicly_served(config_path: &Path, public_dir: &Path) -> bool {
+    match (config_path.canonicalize(), public_dir.canonicalize()) {
         (Ok(config), Ok(root)) => config.starts_with(root),
         _ => false
     }
@@ -224,6 +244,22 @@ mod tests {
         std::fs::remove_dir_all(base).unwrap();
     }
 
+    #[test]
+    fn data_dir_defaults_to_data() {
+        // The sample config has the storage section commented out
+        assert_eq!(sample().storage().data_dir(), PathBuf::from("data"));
+
+        let storage: Storage = toml::from_str("").unwrap();
+        assert_eq!(storage.data_dir(), PathBuf::from("data"));
+    }
+
+    #[test]
+    fn data_dir_can_be_configured() {
+        let storage: Storage = toml::from_str("data_dir = '/var/lib/rezervator'").unwrap();
+
+        assert_eq!(storage.data_dir(), PathBuf::from("/var/lib/rezervator"));
+    }
+
     #[test]
     fn sample_session_key_is_long_enough() {
         // actix_web::cookie::Key::from panics on keys shorter than 64 bytes

src/main.rs

@@ -4,7 +4,7 @@
 use actix_web::middleware::Logger;
 use leptos_captcha::spow::pow::Pow;
 use log::error;
-use rezervator::backend::appearance::check_appearance;
+use rezervator::backend::appearance::{check_appearance, ensure_data_dir};
 use rezervator::backend::company::check_company;
 use rezervator::backend::mail::{check_messages, Mailer};
 use rezervator::backend::user::create_admin;
@@ -56,13 +56,22 @@ async fn main() -> std::io::Result<()> {
 
     let conf = get_configuration(None).unwrap();
 
-    // The site root is served as static files, the config with passwords must not be there
-    if is_publicly_served(Path::new(cfg_path.trim()), Path::new(&*conf.leptos_options.site_root)) {
-        let msg = format!(
-            "Config file {} is inside the publicly served directory {}. Move it elsewhere and change the passwords and the session key in it.",
-            cfg_path.trim(), conf.leptos_options.site_root);
-        error!("{}", msg);
-        return Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, msg));
+    // Files created at runtime (banner, generated styles) live outside the site root, so they survive upgrades
+    let data_dir = srv_conf.storage().data_dir();
+    if let Err(e) = ensure_data_dir(&data_dir, Path::new(&*conf.leptos_options.site_root)) {
+        error!("Cannot prepare data directory {}: {}", data_dir.display(), e);
+        return Err(std::io::Error::other(e));
+    }
+
+    // The site root and the data directory are served as static files, the config with passwords must not be there
+    for public_dir in [Path::new(&*conf.leptos_options.site_root), data_dir.as_path()] {
+        if is_publicly_served(Path::new(cfg_path.trim()), public_dir) {
+            let msg = format!(
+                "Config file {} is inside the publicly served directory {}. Move it elsewhere and change the passwords and the session key in it.",
+                cfg_path.trim(), public_dir.display());
+            error!("{}", msg);
+            return Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, msg));
+        }
     }
 
     let key = Key::from(srv_conf.session().key());
@@ -84,7 +93,7 @@ async fn main() -> std::io::Result<()> {
     if let Err(e) = check_messages(&pool).await {
         error!("Error while checking messages: {:?}", e);
     }
-    if let Err(e) = check_appearance(&pool).await {
+    if let Err(e) = check_appearance(&pool, &data_dir).await {
         error!("Error while checking messages: {:?}", e);
     }
 
@@ -95,7 +104,7 @@ async fn main() -> std::io::Result<()> {
         let site_root = leptos_options.site_root.clone().to_string();
 
         App::new()
-            .app_data(Data::new(AppData::new(pool.clone(), mailer.clone())))
+            .app_data(Data::new(AppData::new(pool.clone(), mailer.clone(), data_dir.clone())))
             .wrap(Logger::default())
             .wrap(Authentication)
             .wrap(SessionMiddleware::new(
@@ -123,6 +132,8 @@ async fn main() -> std::io::Result<()> {
             })
             .app_data(Data::new(leptos_options.to_owned()))
             .service(appearance::upload)
+            // Runtime files first, the site root would shadow them
+            .service(Files::new("/data", data_dir.clone()))
             .service(Files::new("/", site_root))
             .wrap(middleware::Compress::default())
     })