From ebe9162af9fac09b7800e3b1b4a24534e4d2aad3 Mon Sep 17 00:00:00 2001 From: Yip Rui Fung Date: Sat, 9 Jul 2022 01:19:00 +0800 Subject: [PATCH 1/5] Add option to make file uploads use move_copy_to instead of persist_to This is to support scenarios where the attachments and sends folder are to be stored on a separate device from the tmp_folder (i.e. fuse-mounted S3 storage), due to having the tmp_dir on the same device being undesirable. Example being fuse-mounted S3 storage with the reasoning that because S3 basically requires a copy+delete operations to rename files, it's inefficient to rename files on device, if it's even allowed. --- src/api/core/ciphers.rs | 6 +++++- src/api/core/sends.rs | 7 ++++++- src/config.rs | 2 ++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index d70dd906..32c4d7a6 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -998,7 +998,11 @@ async fn save_attachment( attachment.save(&conn).await.expect("Error saving attachment"); } - data.data.persist_to(file_path).await?; + if CONFIG.uploads_use_copy() { + data.data.move_copy_to(file_path).await?; + } else { + data.data.persist_to(file_path).await?; + } nt.send_cipher_update(UpdateType::CipherUpdate, &cipher, &cipher.update_users_revision(&conn).await).await; diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs index 9c53e936..e8c5c3a6 100644 --- a/src/api/core/sends.rs +++ b/src/api/core/sends.rs @@ -225,7 +225,12 @@ async fn post_send_file(data: Form>, headers: Headers, conn: DbCo let folder_path = tokio::fs::canonicalize(&CONFIG.sends_folder()).await?.join(&send.uuid); let file_path = folder_path.join(&file_id); tokio::fs::create_dir_all(&folder_path).await?; - data.persist_to(&file_path).await?; + + if CONFIG.uploads_use_copy() { + data.move_copy_to(&file_path).await?; + } else { + data.persist_to(&file_path).await?; + } let mut data_value: Value = serde_json::from_str(&send.data)?; if let Some(o) = data_value.as_object_mut() { diff --git a/src/config.rs b/src/config.rs index 09240c36..a52abd4a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -343,6 +343,8 @@ make_config! { rsa_key_filename: String, false, auto, |c| format!("{}/{}", c.data_folder, "rsa_key"); /// Web vault folder web_vault_folder: String, false, def, "web-vault/".to_string(); + /// Uploading files uses move_copy_to instead of persist_to to support cross-device scenarios where having tmp_folder on the same drive is undesirable. i.e. fuse-mounted S3. + uploads_use_copy: bool, false, def, false; }, ws { /// Enable websocket notifications From 5c38b2c4eb67d74b33023227e4644b5f646703f1 Mon Sep 17 00:00:00 2001 From: Yip Rui Fung Date: Sat, 9 Jul 2022 08:53:00 +0800 Subject: [PATCH 2/5] Remove option and use unwrap_or_else to fall back to copy behavior. --- src/api/core/ciphers.rs | 10 ++++------ src/api/core/sends.rs | 9 ++++----- src/config.rs | 2 -- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 32c4d7a6..0da7a0c2 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -17,7 +17,7 @@ use crate::{ CONFIG, }; -use futures::{stream, stream::StreamExt}; +use futures::{stream, stream::StreamExt, TryFutureExt}; pub fn routes() -> Vec { // Note that many routes have an `admin` variant; this seems to be @@ -998,11 +998,9 @@ async fn save_attachment( attachment.save(&conn).await.expect("Error saving attachment"); } - if CONFIG.uploads_use_copy() { - data.data.move_copy_to(file_path).await?; - } else { - data.data.persist_to(file_path).await?; - } + data.data.persist_to(&file_path) + .unwrap_or_else(data.data.move_copy_to(&file_path)) + .await?; nt.send_cipher_update(UpdateType::CipherUpdate, &cipher, &cipher.update_users_revision(&conn).await).await; diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs index e8c5c3a6..90b75b55 100644 --- a/src/api/core/sends.rs +++ b/src/api/core/sends.rs @@ -1,6 +1,7 @@ use std::path::Path; use chrono::{DateTime, Duration, Utc}; +use futures::TryFutureExt; use rocket::form::Form; use rocket::fs::NamedFile; use rocket::fs::TempFile; @@ -226,11 +227,9 @@ async fn post_send_file(data: Form>, headers: Headers, conn: DbCo let file_path = folder_path.join(&file_id); tokio::fs::create_dir_all(&folder_path).await?; - if CONFIG.uploads_use_copy() { - data.move_copy_to(&file_path).await?; - } else { - data.persist_to(&file_path).await?; - } + data.persist_to(&file_path) + .unwrap_or_else(data.move_copy_to(&file_path)) + .await?; let mut data_value: Value = serde_json::from_str(&send.data)?; if let Some(o) = data_value.as_object_mut() { diff --git a/src/config.rs b/src/config.rs index a52abd4a..09240c36 100644 --- a/src/config.rs +++ b/src/config.rs @@ -343,8 +343,6 @@ make_config! { rsa_key_filename: String, false, auto, |c| format!("{}/{}", c.data_folder, "rsa_key"); /// Web vault folder web_vault_folder: String, false, def, "web-vault/".to_string(); - /// Uploading files uses move_copy_to instead of persist_to to support cross-device scenarios where having tmp_folder on the same drive is undesirable. i.e. fuse-mounted S3. - uploads_use_copy: bool, false, def, false; }, ws { /// Enable websocket notifications From 31595888ea0ae46eb93e49fc6876bbad368ab9d1 Mon Sep 17 00:00:00 2001 From: Yip Rui Fung Date: Sat, 9 Jul 2022 10:33:27 +0800 Subject: [PATCH 3/5] Use match to avoid ownership issues on the TempFile / file_path variables in closures. --- src/api/core/ciphers.rs | 28 +++++++++++++++------------- src/api/core/sends.rs | 13 +++++++------ 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 0da7a0c2..18334ea6 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -1,23 +1,24 @@ use std::collections::{HashMap, HashSet}; use chrono::{NaiveDateTime, Utc}; -use rocket::fs::TempFile; -use rocket::serde::json::Json; +use futures::{stream, stream::StreamExt}; use rocket::{ form::{Form, FromForm}, Route, }; +use rocket::fs::TempFile; +use rocket::serde::json::Json; use serde_json::Value; use crate::{ api::{self, EmptyResult, JsonResult, JsonUpcase, Notify, PasswordData, UpdateType}, auth::Headers, - crypto, - db::{models::*, DbConn, DbPool}, CONFIG, + crypto, + db::{DbConn, DbPool, models::*}, }; -use futures::{stream, stream::StreamExt, TryFutureExt}; +use super::folders::FolderData; pub fn routes() -> Vec { // Note that many routes have an `admin` variant; this seems to be @@ -212,7 +213,8 @@ pub struct CipherData { Card = 3, Identity = 4 */ - pub Type: i32, // TODO: Change this to NumberOrString + pub Type: i32, + // TODO: Change this to NumberOrString pub Name: String, Notes: Option, Fields: Option, @@ -230,7 +232,8 @@ pub struct CipherData { // These are used during key rotation #[serde(rename = "Attachments")] - _Attachments: Option, // Unused, contains map of {id: filename} + _Attachments: Option, + // Unused, contains map of {id: filename} Attachments2: Option>, // The revision datetime (in ISO 8601 format) of the client's local copy @@ -470,8 +473,6 @@ pub async fn update_cipher_from_data( Ok(()) } -use super::folders::FolderData; - #[derive(Deserialize)] #[allow(non_snake_case)] struct ImportData { @@ -804,7 +805,7 @@ async fn share_cipher_by_uuid( nt, UpdateType::CipherUpdate, ) - .await?; + .await?; Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, None, conn).await)) } @@ -998,9 +999,10 @@ async fn save_attachment( attachment.save(&conn).await.expect("Error saving attachment"); } - data.data.persist_to(&file_path) - .unwrap_or_else(data.data.move_copy_to(&file_path)) - .await?; + match data.data.persist_to(&file_path).await { + Ok(_result) => {} + Err(_error) => data.data.move_copy_to(&file_path).await? + } nt.send_cipher_update(UpdateType::CipherUpdate, &cipher, &cipher.update_users_revision(&conn).await).await; diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs index 90b75b55..8b8ccfb1 100644 --- a/src/api/core/sends.rs +++ b/src/api/core/sends.rs @@ -1,7 +1,6 @@ use std::path::Path; use chrono::{DateTime, Duration, Utc}; -use futures::TryFutureExt; use rocket::form::Form; use rocket::fs::NamedFile; use rocket::fs::TempFile; @@ -11,9 +10,9 @@ use serde_json::Value; use crate::{ api::{ApiResult, EmptyResult, JsonResult, JsonUpcase, Notify, NumberOrString, UpdateType}, auth::{ClientIp, Headers, Host}, - db::{models::*, DbConn, DbPool}, - util::SafeString, CONFIG, + db::{DbConn, DbPool, models::*}, + util::SafeString, }; const SEND_INACCESSIBLE_MSG: &str = "Send does not exist or is no longer available"; @@ -227,9 +226,11 @@ async fn post_send_file(data: Form>, headers: Headers, conn: DbCo let file_path = folder_path.join(&file_id); tokio::fs::create_dir_all(&folder_path).await?; - data.persist_to(&file_path) - .unwrap_or_else(data.move_copy_to(&file_path)) - .await?; + + match data.persist_to(&file_path).await { + Ok(_result) => {} + Err(_error) => data.move_copy_to(&file_path).await? + } let mut data_value: Value = serde_json::from_str(&send.data)?; if let Some(o) = data_value.as_object_mut() { From 84bcac0112af6570563f7f174849f98b23e5284b Mon Sep 17 00:00:00 2001 From: Yip Rui Fung Date: Sat, 9 Jul 2022 10:49:51 +0800 Subject: [PATCH 4/5] Apply rustfmt. Because apparently CLion's default formatting is not the same as rustfmt for some reason. --- src/api/core/ciphers.rs | 12 ++++++------ src/api/core/sends.rs | 7 +++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 18334ea6..7459bba0 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -2,20 +2,20 @@ use std::collections::{HashMap, HashSet}; use chrono::{NaiveDateTime, Utc}; use futures::{stream, stream::StreamExt}; +use rocket::fs::TempFile; +use rocket::serde::json::Json; use rocket::{ form::{Form, FromForm}, Route, }; -use rocket::fs::TempFile; -use rocket::serde::json::Json; use serde_json::Value; use crate::{ api::{self, EmptyResult, JsonResult, JsonUpcase, Notify, PasswordData, UpdateType}, auth::Headers, - CONFIG, crypto, - db::{DbConn, DbPool, models::*}, + db::{models::*, DbConn, DbPool}, + CONFIG, }; use super::folders::FolderData; @@ -805,7 +805,7 @@ async fn share_cipher_by_uuid( nt, UpdateType::CipherUpdate, ) - .await?; + .await?; Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, None, conn).await)) } @@ -1001,7 +1001,7 @@ async fn save_attachment( match data.data.persist_to(&file_path).await { Ok(_result) => {} - Err(_error) => data.data.move_copy_to(&file_path).await? + Err(_error) => data.data.move_copy_to(&file_path).await?, } nt.send_cipher_update(UpdateType::CipherUpdate, &cipher, &cipher.update_users_revision(&conn).await).await; diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs index 8b8ccfb1..b5ba15d0 100644 --- a/src/api/core/sends.rs +++ b/src/api/core/sends.rs @@ -10,9 +10,9 @@ use serde_json::Value; use crate::{ api::{ApiResult, EmptyResult, JsonResult, JsonUpcase, Notify, NumberOrString, UpdateType}, auth::{ClientIp, Headers, Host}, - CONFIG, - db::{DbConn, DbPool, models::*}, + db::{models::*, DbConn, DbPool}, util::SafeString, + CONFIG, }; const SEND_INACCESSIBLE_MSG: &str = "Send does not exist or is no longer available"; @@ -226,10 +226,9 @@ async fn post_send_file(data: Form>, headers: Headers, conn: DbCo let file_path = folder_path.join(&file_id); tokio::fs::create_dir_all(&folder_path).await?; - match data.persist_to(&file_path).await { Ok(_result) => {} - Err(_error) => data.move_copy_to(&file_path).await? + Err(_error) => data.move_copy_to(&file_path).await?, } let mut data_value: Value = serde_json::from_str(&send.data)?; From bf623eed7fc6d7184f2b794461d0d37a03731d5d Mon Sep 17 00:00:00 2001 From: Yip Rui Fung Date: Sat, 9 Jul 2022 11:41:53 +0800 Subject: [PATCH 5/5] Use if let instead of a match with empty block. --- src/api/core/ciphers.rs | 5 ++--- src/api/core/sends.rs | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 7459bba0..137242b0 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -999,9 +999,8 @@ async fn save_attachment( attachment.save(&conn).await.expect("Error saving attachment"); } - match data.data.persist_to(&file_path).await { - Ok(_result) => {} - Err(_error) => data.data.move_copy_to(&file_path).await?, + if let Err(_err) = data.data.persist_to(&file_path).await { + data.data.move_copy_to(file_path).await? } nt.send_cipher_update(UpdateType::CipherUpdate, &cipher, &cipher.update_users_revision(&conn).await).await; diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs index b5ba15d0..4012fd82 100644 --- a/src/api/core/sends.rs +++ b/src/api/core/sends.rs @@ -226,9 +226,8 @@ async fn post_send_file(data: Form>, headers: Headers, conn: DbCo let file_path = folder_path.join(&file_id); tokio::fs::create_dir_all(&folder_path).await?; - match data.persist_to(&file_path).await { - Ok(_result) => {} - Err(_error) => data.move_copy_to(&file_path).await?, + if let Err(_err) = data.persist_to(&file_path).await { + data.move_copy_to(file_path).await? } let mut data_value: Value = serde_json::from_str(&send.data)?;