Uniformize media path/URL handling and implement direct download from S3 backend

plume-models/src/config.rs

@@ -371,12 +371,10 @@ pub struct S3Config {
     pub path_style: bool,
     pub protocol: String,
 
-    // options below this comment are not used yet
-    // upload directly from user to S3, without going through Plume. Uses PostObject endpoint
-    pub direct_upload: bool,
     // download directly from s3 to user, wihout going through Plume. Require public read on bucket
     pub direct_download: bool,
-    // use this hostname for downloads, can be used with caching proxy in front of s3
+    // use this hostname for downloads, can be used with caching proxy in front of s3 (expected to
+    // be reachable through https)
     pub alias: Option<String>,
 }
 
@@ -434,13 +432,15 @@ fn get_s3_config() -> Option<S3Config> {
 
         let path_style = var("S3_PATH_STYLE").unwrap_or_else(|_| "false".to_owned());
         let path_style = string_to_bool(&path_style, "S3_PATH_STYLE");
-        let direct_upload = var("S3_DIRECT_UPLOAD").unwrap_or_else(|_| "false".to_owned());
-        let direct_upload = string_to_bool(&direct_upload, "S3_DIRECT_UPLOAD");
         let direct_download = var("S3_DIRECT_DOWNLOAD").unwrap_or_else(|_| "false".to_owned());
         let direct_download = string_to_bool(&direct_download, "S3_DIRECT_DOWNLOAD");
 
         let alias = var("S3_ALIAS_HOST").ok();
 
+        if direct_download && protocol == "http" && alias.is_none() {
+            panic!("S3 direct download is disabled because bucket is accessed through plain HTTP. Use HTTPS or set an alias hostname (S3_ALIAS_HOST).");
+        }
+
         Some(S3Config {
             bucket,
             access_key_id,
@@ -449,7 +449,6 @@ fn get_s3_config() -> Option<S3Config> {
             hostname,
             protocol,
             path_style,
-            direct_upload,
             direct_download,
             alias,
         })

plume-models/src/medias.rs

@@ -16,6 +16,9 @@ use std::{
 use tracing::warn;
 use url::Url;
 
+#[cfg(feature = "s3")]
+use crate::config::S3Config;
+
 const REMOTE_MEDIA_DIRECTORY: &str = "remote";
 
 #[derive(Clone, Identifiable, Queryable, AsChangeset)]
@@ -105,7 +108,7 @@ impl Media {
             .file_path
             .rsplit_once('.')
             .map(|x| x.1)
-            .expect("Media::category: extension error")
+            .unwrap_or("")
             .to_lowercase()
         {
             "png" | "jpg" | "jpeg" | "gif" | "svg" => MediaCategory::Image,
@@ -151,19 +154,83 @@ impl Media {
         })
     }
 
+    /// Returns full file path for medias stored in the local media directory.
+    pub fn local_path(&self) -> Option<PathBuf> {
+        if self.file_path.is_empty() {
+            return None;
+        }
+
+        if CONFIG.s3.is_some() {
+            #[cfg(feature="s3")]
+            unreachable!("Called Media::local_path() but media are stored on S3");
+            #[cfg(not(feature="s3"))]
+            unreachable!();
+        }
+
+        let relative_path = self
+            .file_path
+            .trim_start_matches(&CONFIG.media_directory)
+            .trim_start_matches(path::MAIN_SEPARATOR)
+            .trim_start_matches("static/media/");
+
+        Some(Path::new(&CONFIG.media_directory).join(relative_path))
+    }
+
+    /// Returns the relative URL to access this file, which is also the key at which
+    /// it is stored in the S3 bucket if we are using S3 storage.
+    /// Does not start with a '/', it is of the form "static/media/<...>"
+    pub fn relative_url(&self) -> Option<String> {
+        if self.file_path.is_empty() {
+            return None;
+        }
+
+        let relative_path = self
+            .file_path
+            .trim_start_matches(&CONFIG.media_directory)
+            .replace(path::MAIN_SEPARATOR, "/");
+
+        let relative_path = relative_path
+            .trim_start_matches('/')
+            .trim_start_matches("static/media/");
+
+        Some(format!("static/media/{}", relative_path))
+    }
+
+    /// Returns a public URL through which this media file can be accessed
     pub fn url(&self) -> Result<String> {
         if self.is_remote {
             Ok(self.remote_url.clone().unwrap_or_default())
         } else {
-            let file_path = self.file_path.replace(path::MAIN_SEPARATOR, "/").replacen(
+            let relative_url = self.relative_url().unwrap_or_default();
-                &CONFIG.media_directory,
+
-                "static/media",
+            #[cfg(feature="s3")]
-                1,
+            if CONFIG.s3.as_ref().map(|x| x.direct_download).unwrap_or(false) {
-            ); // "static/media" from plume::routs::plume_media_files()
+                let s3_url = match CONFIG.s3.as_ref().unwrap() {
+                    S3Config { alias: Some(alias), .. } => {
+                        format!("https://{}/{}", alias, relative_url)
+                    }
+                    S3Config { path_style: true, hostname, bucket, .. } => {
+                        format!("https://{}/{}/{}",
+                            hostname,
+                            bucket,
+                            relative_url
+                        )
+                    }
+                    S3Config { path_style: false, hostname, bucket, .. } => {
+                        format!("https://{}.{}/{}",
+                            bucket,
+                            hostname,
+                            relative_url
+                        )
+                    }
+                };
+                return Ok(s3_url);
+            }
+
             Ok(ap_url(&format!(
                 "{}/{}",
                 Instance::get_local()?.public_domain,
-                &file_path
+                relative_url
             )))
         }
     }
@@ -176,9 +243,9 @@ impl Media {
 
                 #[cfg(feature = "s3")]
                 CONFIG.s3.as_ref().unwrap().get_bucket()
-                    .delete_object_blocking(&self.file_path)?;
+                    .delete_object_blocking(&self.relative_url().ok_or(Error::NotFound)?)?;
             } else {
-                fs::remove_file(self.file_path.as_str())?;
+                fs::remove_file(self.local_path().ok_or(Error::NotFound)?)?;
             }
         }
         diesel::delete(self)
@@ -316,12 +383,9 @@ impl Media {
 }
 
 fn determine_mirror_file_path(url: &str) -> PathBuf {
-    let mut file_path = Path::new(&super::CONFIG.media_directory).join(REMOTE_MEDIA_DIRECTORY);
+    let mut file_path = Path::new(&CONFIG.media_directory).join(REMOTE_MEDIA_DIRECTORY);
-    Url::parse(url)
+    match Url::parse(url) {
-        .map(|url| {
+        Ok(url) if url.has_host() => {
-            if !url.has_host() {
-                return;
-            }
             file_path.push(url.host_str().unwrap());
             for segment in url.path_segments().expect("FIXME") {
                 file_path.push(segment);
@@ -329,16 +393,21 @@ fn determine_mirror_file_path(url: &str) -> PathBuf {
             // TODO: handle query
             // HINT: Use characters which must be percent-encoded in path as separator between path and query
             // HINT: handle extension
-        })
+        }
-        .unwrap_or_else(|err| {
+        other => {
-            warn!("Failed to parse url: {} {}", &url, err);
+            if let Err(err) = other {
+                warn!("Failed to parse url: {} {}", &url, err);
+            } else {
+                warn!("Error without a host: {}", &url);
+            }
             let ext = url
                 .rsplit('.')
                 .next()
                 .map(ToOwned::to_owned)
                 .unwrap_or_else(|| String::from("png"));
             file_path.push(format!("{}.{}", GUID::rand(), ext));
-        });
+        }
+    }
     file_path
 }