Merge branch 'main' into asonix/per-upload-limits-and-operations

Cargo.lock

@@ -1819,7 +1819,7 @@ dependencies = [
 
 [[package]]
 name = "pict-rs"
-version = "0.5.9"
+version = "0.5.10"
 dependencies = [
  "actix-form-data",
  "actix-web",

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "pict-rs"
 description = "A simple image hosting service"
-version = "0.5.9"
+version = "0.5.10"
 authors = ["asonix <asonix@asonix.dog>"]
 license = "AGPL-3.0"
 readme = "README.md"

defaults.toml

@@ -4,6 +4,7 @@ read_only = false
 danger_dummy_mode = false
 max_file_count = 1
 temporary_directory = "/tmp"
+cleanup_temporary_directory = true
 
 [client]
 timeout = 30
@@ -46,7 +47,7 @@ proxy = "7d"
 [media.magick]
 max_width = 10000
 max_height = 10000
-max_area = 40000000
+max_area = 20000
 memory = 256
 map = 512
 disk = 1024

pict-rs.nix

@@ -11,7 +11,7 @@
 
 rustPlatform.buildRustPackage {
   pname = "pict-rs";
-  version = "0.5.9";
+  version = "0.5.10";
   src = ./.;
 
   cargoLock = {

pict-rs.toml

@@ -37,6 +37,11 @@ max_file_count = 1
 # default: The system's advertised temporary directory ("/tmp" on most linuxes)
 temporary_directory = "/tmp"
 
+## Optional: whether to delete the contents of $temporary_directory/pict-rs on launch
+# environment variable: PICTRS__SERVER__CLEANUP_TEMPORARY_DIRECTORY
+# default: true
+cleanup_temporary_directory = true
+
 ## Optional: path to server certificate to enable TLS
 # environment variable: PICTRS__SERVER__CERTIFICATE
 # default: empty

releases/0.5.10.md

@@ -0,0 +1,31 @@
+# pict-rs 0.5.10
+
+## Overview
+
+pict-rs 0.5.10 is a small release with changes to how pict-rs handles temporary files.
+
+### Changes
+
+- [Temporary File Cleanup](#temporary-file-cleanup)
+
+
+## Upgrade Notes
+
+There are no significant changes from 0.5.9. Upgrading should be as simple as pulling the new
+version.
+
+
+## Descriptions
+
+### Temporary File Cleanup
+
+pict-rs now nests its temporary files inside a `pict-rs` toplevel temporary folder. This is useful
+because pict-rs 0.5.10 introduces a new behavior: it will completely delete that folder and its
+contents on launch. If you are running multiple copies of pict-rs on the same host and they share
+your temporary folder, this might cause problems. In that scenario, this behavior can be disabled by
+setting `PICTRS__SERVER__CLEANUP_TEMPORARY_DIRECTORY=false` or passing
+`--no-cleanup-temporary-directory` on the commandline.
+
+This new behavior has been introduced in order to better clean up after crashes. If pict-rs is
+killed while processing media, maybe due to an OOM, it will leave files behind in the temporary
+directory. This can cause the temporary directory to grow, leading to memory or disk problems.

src/config/commandline.rs

@@ -55,6 +55,7 @@ impl Args {
                 address,
                 api_key,
                 temporary_directory,
+                no_cleanup_temporary_directory,
                 certificate,
                 private_key,
                 client_timeout,
@@ -122,6 +123,7 @@ impl Args {
                     danger_dummy_mode,
                     max_file_count,
                     temporary_directory,
+                    cleanup_temporary_directory: !no_cleanup_temporary_directory,
                     certificate,
                     private_key,
                 };
@@ -541,6 +543,7 @@ struct Server {
     max_file_count: Option<u32>,
     #[serde(skip_serializing_if = "Option::is_none")]
     temporary_directory: Option<PathBuf>,
+    cleanup_temporary_directory: bool,
     #[serde(skip_serializing_if = "Option::is_none")]
     certificate: Option<PathBuf>,
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -973,6 +976,10 @@ struct Run {
     #[arg(long)]
     temporary_directory: Option<PathBuf>,
 
+    /// Whether to attempt to clean files left behind from a previous run of pict-rs
+    #[arg(long)]
+    no_cleanup_temporary_directory: bool,
+
     /// The path to the TLS certificate. Both the certificate and the private_key must be specified
     /// to enable TLS
     #[arg(long)]

src/config/defaults.rs

@@ -24,6 +24,7 @@ struct ServerDefaults {
     danger_dummy_mode: bool,
     max_file_count: u32,
     temporary_directory: PathBuf,
+    cleanup_temporary_directory: bool,
 }
 
 #[derive(Clone, Debug, serde::Serialize)]
@@ -211,6 +212,7 @@ impl Default for ServerDefaults {
             danger_dummy_mode: false,
             max_file_count: 1,
             temporary_directory: std::env::temp_dir(),
+            cleanup_temporary_directory: true,
         }
     }
 }

src/config/file.rs

@@ -119,6 +119,8 @@ pub(crate) struct Server {
 
     pub(crate) temporary_directory: PathBuf,
 
+    pub(crate) cleanup_temporary_directory: bool,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     pub(crate) certificate: Option<PathBuf>,
 

src/lib.rs

@@ -2017,7 +2017,11 @@ impl PictRsConfiguration {
         // describe all the metrics pict-rs produces
         init_metrics::init_metrics();
 
-        let tmp_dir = TmpDir::init(&config.server.temporary_directory).await?;
+        let tmp_dir = TmpDir::init(
+            &config.server.temporary_directory,
+            config.server.cleanup_temporary_directory,
+        )
+        .await?;
         let policy_dir = magick::write_magick_policy(&config.media, &tmp_dir).await?;
 
         let client = build_client()?;

src/tmp_file.rs

@@ -16,9 +16,17 @@ pub(crate) struct TmpDir {
 }
 
 impl TmpDir {
-    pub(crate) async fn init<P: AsRef<Path>>(path: P) -> std::io::Result<Arc<Self>> {
-        let path = path.as_ref().join(Uuid::now_v7().to_string());
-        tokio::fs::create_dir(&path).await?;
+    pub(crate) async fn init<P: AsRef<Path>>(path: P, cleanup: bool) -> std::io::Result<Arc<Self>> {
+        let base_path = path.as_ref().join("pict-rs");
+
+        if cleanup && tokio::fs::metadata(&base_path).await.is_ok() {
+            tokio::fs::remove_dir_all(&base_path).await?;
+        }
+
+        let path = base_path.join(Uuid::now_v7().to_string());
+
+        tokio::fs::create_dir_all(&path).await?;
+
         Ok(Arc::new(TmpDir { path: Some(path) }))
     }
 
@@ -47,8 +55,13 @@ impl TmpDir {
     }
 
     pub(crate) async fn cleanup(self: Arc<Self>) -> std::io::Result<()> {
-        if let Some(path) = Arc::into_inner(self).and_then(|mut this| this.path.take()) {
-            tokio::fs::remove_dir_all(path).await?;
+        if let Some(mut path) = Arc::into_inner(self).and_then(|mut this| this.path.take()) {
+            tokio::fs::remove_dir_all(&path).await?;
+
+            if path.pop() {
+                // attempt to remove parent directory if it is empty
+                let _ = tokio::fs::remove_dir(path).await;
+            }
         }
 
         Ok(())
@@ -57,9 +70,13 @@ impl TmpDir {
 
 impl Drop for TmpDir {
     fn drop(&mut self) {
-        if let Some(path) = self.path.take() {
+        if let Some(mut path) = self.path.take() {
             tracing::warn!("TmpDir - Blocking remove of {path:?}");
-            std::fs::remove_dir_all(path).expect("Removed directory");
+            std::fs::remove_dir_all(&path).expect("Removed directory");
+            if path.pop() {
+                // attempt to remove parent directory if it is empty
+                let _ = std::fs::remove_dir(path);
+            }
         }
     }
 }