From f4609db5aa7207dbb5fb53c83c2a054c6ffbb749 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radek=20St=C4=99pie=C5=84?= Date: Thu, 21 Sep 2023 18:37:12 +0200 Subject: [PATCH 1/4] remove exif info during file uploads --- crates/kitsune-core/Cargo.toml | 2 + crates/kitsune-core/src/error.rs | 15 ++ crates/kitsune-core/src/service/attachment.rs | 176 +++++++++++++++++- 3 files changed, 186 insertions(+), 7 deletions(-) diff --git a/crates/kitsune-core/Cargo.toml b/crates/kitsune-core/Cargo.toml index 209836c79..ded7dc8ff 100644 --- a/crates/kitsune-core/Cargo.toml +++ b/crates/kitsune-core/Cargo.toml @@ -34,6 +34,7 @@ garde = { version = "0.15.0", default-features = false, features = [ globset = "0.4.13" hex-simd = "0.8.0" http = "0.2.9" +img-parts = "0.3.0" iso8601-timestamp = "0.2.11" just-retry = { path = "../../lib/just-retry" } kitsune-cache = { path = "../kitsune-cache" } @@ -90,4 +91,5 @@ kitsune-test = { path = "../kitsune-test" } pretty_assertions = "1.4.0" redis = "0.23.3" serial_test = "2.0.0" +tempfile = "3.8.0" tower = "0.4.13" diff --git a/crates/kitsune-core/src/error.rs b/crates/kitsune-core/src/error.rs index 5e33ee38b..bfe42475a 100644 --- a/crates/kitsune-core/src/error.rs +++ b/crates/kitsune-core/src/error.rs @@ -1,7 +1,10 @@ +use std::error::Error as ErrorTrait; + use kitsune_http_signatures::ring; use thiserror::Error; use tokio::sync::oneshot; +pub type BoxError = Box; pub type Result = std::result::Result; #[derive(Debug, Error)] @@ -46,6 +49,15 @@ pub enum FederationFilterError { UrlParse(#[from] url::ParseError), } +#[derive(Debug, Error)] +pub enum UploadError { + #[error(transparent)] + ImageProcessingError(#[from] img_parts::Error), + + #[error(transparent)] + StreamError(#[from] BoxError), +} + #[derive(Debug, Error)] pub enum Error { #[error(transparent)] @@ -120,6 +132,9 @@ pub enum Error { #[error(transparent)] TokioOneshot(#[from] oneshot::error::RecvError), + #[error(transparent)] + Upload(#[from] UploadError), + #[error(transparent)] UriInvalid(#[from] http::uri::InvalidUri), diff --git a/crates/kitsune-core/src/service/attachment.rs b/crates/kitsune-core/src/service/attachment.rs index 9dbf0afcc..b00833556 100644 --- a/crates/kitsune-core/src/service/attachment.rs +++ b/crates/kitsune-core/src/service/attachment.rs @@ -1,14 +1,15 @@ use super::url::UrlService; use crate::{ consts::{MAX_MEDIA_DESCRIPTION_LENGTH, USER_AGENT}, - error::{ApiError, Error, Result}, + error::{ApiError, Error, Result, UploadError}, }; -use bytes::Bytes; +use bytes::{Bytes, BytesMut}; use derive_builder::Builder; use diesel::{BoolExpressionMethods, ExpressionMethods, QueryDsl}; use diesel_async::RunQueryDsl; -use futures_util::{Stream, StreamExt, TryStreamExt}; +use futures_util::{pin_mut, stream, Stream, StreamExt, TryStreamExt}; use garde::Validate; +use img_parts::{DynImage, ImageEXIF}; use kitsune_db::{ model::media_attachment::{MediaAttachment, NewMediaAttachment, UpdateMediaAttachment}, schema::media_attachments, @@ -35,6 +36,10 @@ fn is_allowed_filetype(value: &str, _ctx: &()) -> garde::Result { Ok(()) } +fn is_image_type_with_supported_metadata(mime: &str) -> bool { + matches!(mime, "image/jpeg" | "image/png" | "image/webp") +} + #[derive(TypedBuilder, Validate)] pub struct Update { #[garde(skip)] @@ -179,10 +184,35 @@ impl AttachmentService { { upload.validate(&())?; - self.storage_backend - .put(&upload.path, upload.stream) - .await - .map_err(Error::Storage)?; + // remove exif info from image uploads + let upload_stream = if is_image_type_with_supported_metadata(&upload.content_type) { + let mut img_bytes = BytesMut::new(); + let stream = upload.stream; + pin_mut!(stream); + while let Some(chunk) = stream + .next() + .await + .transpose() + .map_err(UploadError::StreamError)? + { + img_bytes.extend_from_slice(&chunk); + } + let img_bytes = img_bytes.freeze(); + let final_bytes = DynImage::from_bytes(img_bytes) + .map_err(UploadError::ImageProcessingError)? + .ok_or(img_parts::Error::WrongSignature) + .map(|mut image| { + image.set_exif(None); + image.encoder().bytes() + }) + .map_err(UploadError::ImageProcessingError)?; + self.storage_backend + .put(&upload.path, stream::once(async { Ok(final_bytes) })) + } else { + self.storage_backend.put(&upload.path, upload.stream) + }; + + upload_stream.await.map_err(Error::Storage)?; let media_attachment = self .db_pool @@ -205,3 +235,135 @@ impl AttachmentService { Ok(media_attachment) } } + +#[cfg(test)] +mod test { + use std::convert::Infallible; + + use bytes::{Bytes, BytesMut}; + use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl}; + use futures_util::{future, pin_mut, stream, StreamExt}; + use http::{Request, Response}; + use hyper::Body; + use img_parts::{ + jpeg::{markers, JpegSegment}, + ImageEXIF, + }; + use iso8601_timestamp::Timestamp; + use kitsune_db::{ + model::{ + account::{ActorType, NewAccount}, + media_attachment::MediaAttachment, + }, + schema::accounts, + }; + use kitsune_http_client::Client; + use kitsune_storage::fs::Storage; + use kitsune_test::database_test; + use scoped_futures::ScopedFutureExt; + use speedy_uuid::Uuid; + use tempfile::TempDir; + use tower::service_fn; + + use crate::{ + error::Error, + service::{ + attachment::{AttachmentService, Upload}, + url::UrlService, + }, + }; + + #[tokio::test] + #[serial_test::serial] + async fn upload_jpeg() { + database_test(|db_pool| async move { + let client = Client::builder().service(service_fn(handle)); + + let account_id = db_pool + .with_connection(|db_conn| { + async move { Ok::<_, eyre::Report>(prepare_db(db_conn).await) }.scoped() + }) + .await + .unwrap(); + + let temp_dir = TempDir::new().unwrap(); + let storage = Storage::new(temp_dir.path().to_owned()); + let url_service = UrlService::builder() + .domain("example.com") + .scheme("http") + .build(); + + let attachment_service = AttachmentService::builder() + .client(client) + .db_pool(db_pool) + .url_service(url_service) + .storage_backend(storage) + .media_proxy_enabled(false) + .build(); + + let base = hex_simd::decode_to_vec("ffd8ffe000104a46494600010101004800480000ffdb004300030202020202030202020303030304060404040404080606050609080a0a090809090a0c0f0c0a0b0e0b09090d110d0e0f101011100a0c12131210130f101010ffc9000b080001000101011100ffcc000600101005ffda0008010100003f00d2cf20ffd9").unwrap(); + let mut jpeg = img_parts::jpeg::Jpeg::from_bytes(Bytes::from(base)).unwrap(); + let comment_segment = JpegSegment::new_with_contents(markers::APP1, Bytes::from("Exif\0\0Some info to be stripped")); + jpeg.segments_mut().insert(1, comment_segment); + assert!(jpeg.exif().is_some()); + + let upload = Upload::builder().content_type(String::from("image/jpeg")).path(String::from("test.jpeg")).stream(stream::once(future::ok(jpeg.encoder().bytes()))).account_id(account_id).build().unwrap(); + attachment_service.upload(upload).await.unwrap(); + + let attachment = MediaAttachment { id: Uuid::now_v7(), account_id, content_type: String::from("image/jpeg"), description: None, blurhash: None, file_path: Some(String::from("test.jpeg")), remote_url: None, created_at: Timestamp::now_utc(), updated_at: Timestamp::now_utc() }; + let download = attachment_service.stream_file(&attachment).await.unwrap(); + + let mut img_bytes = BytesMut::new(); + pin_mut!(download); + while let Some(chunk) = download.next().await.transpose().unwrap() { + img_bytes.extend_from_slice(&chunk); + } + let img_bytes = img_bytes.freeze(); + + let jpeg = img_parts::jpeg::Jpeg::from_bytes(img_bytes).unwrap(); + assert!(jpeg.exif().is_none()); + }) + .await; + } + + async fn handle(_req: Request) -> Result, Infallible> { + Ok::<_, Infallible>(Response::new(Body::from(""))) + } + + async fn prepare_db(db_conn: &mut AsyncPgConnection) -> Uuid { + // Create a local user `@alice` + db_conn + .transaction(|tx| { + async move { + let account_id = Uuid::now_v7(); + diesel::insert_into(accounts::table) + .values(NewAccount { + id: account_id, + display_name: None, + username: "alice", + locked: false, + note: None, + local: true, + domain: "example.com", + actor_type: ActorType::Person, + url: "https://example.com/users/alice", + featured_collection_url: None, + followers_url: None, + following_url: None, + inbox_url: None, + outbox_url: None, + shared_inbox_url: None, + public_key_id: "https://example.com/users/alice#main-key", + public_key: "", + created_at: None, + }) + .execute(tx) + .await?; + Ok::<_, Error>(account_id) + } + .scope_boxed() + }) + .await + .unwrap() + } +} From cd304940454d1c6355bf47094433270a56ffaca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radek=20St=C4=99pie=C5=84?= Date: Thu, 21 Sep 2023 19:58:43 +0200 Subject: [PATCH 2/4] add some space --- crates/kitsune-core/src/service/attachment.rs | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/crates/kitsune-core/src/service/attachment.rs b/crates/kitsune-core/src/service/attachment.rs index b00833556..d86457c5e 100644 --- a/crates/kitsune-core/src/service/attachment.rs +++ b/crates/kitsune-core/src/service/attachment.rs @@ -186,9 +186,10 @@ impl AttachmentService { // remove exif info from image uploads let upload_stream = if is_image_type_with_supported_metadata(&upload.content_type) { - let mut img_bytes = BytesMut::new(); let stream = upload.stream; pin_mut!(stream); + + let mut img_bytes = BytesMut::new(); while let Some(chunk) = stream .next() .await @@ -197,6 +198,7 @@ impl AttachmentService { { img_bytes.extend_from_slice(&chunk); } + let img_bytes = img_bytes.freeze(); let final_bytes = DynImage::from_bytes(img_bytes) .map_err(UploadError::ImageProcessingError)? @@ -206,6 +208,7 @@ impl AttachmentService { image.encoder().bytes() }) .map_err(UploadError::ImageProcessingError)?; + self.storage_backend .put(&upload.path, stream::once(async { Ok(final_bytes) })) } else { @@ -303,14 +306,32 @@ mod test { let base = hex_simd::decode_to_vec("ffd8ffe000104a46494600010101004800480000ffdb004300030202020202030202020303030304060404040404080606050609080a0a090809090a0c0f0c0a0b0e0b09090d110d0e0f101011100a0c12131210130f101010ffc9000b080001000101011100ffcc000600101005ffda0008010100003f00d2cf20ffd9").unwrap(); let mut jpeg = img_parts::jpeg::Jpeg::from_bytes(Bytes::from(base)).unwrap(); - let comment_segment = JpegSegment::new_with_contents(markers::APP1, Bytes::from("Exif\0\0Some info to be stripped")); + + let comment_segment = JpegSegment::new_with_contents( + markers::APP1, + Bytes::from("Exif\0\0Some info to be stripped") + ); jpeg.segments_mut().insert(1, comment_segment); assert!(jpeg.exif().is_some()); - let upload = Upload::builder().content_type(String::from("image/jpeg")).path(String::from("test.jpeg")).stream(stream::once(future::ok(jpeg.encoder().bytes()))).account_id(account_id).build().unwrap(); + let upload = Upload::builder() + .content_type(String::from("image/jpeg")) + .path(String::from("test.jpeg")) + .stream(stream::once(future::ok(jpeg.encoder().bytes()))) + .account_id(account_id).build().unwrap(); attachment_service.upload(upload).await.unwrap(); - let attachment = MediaAttachment { id: Uuid::now_v7(), account_id, content_type: String::from("image/jpeg"), description: None, blurhash: None, file_path: Some(String::from("test.jpeg")), remote_url: None, created_at: Timestamp::now_utc(), updated_at: Timestamp::now_utc() }; + let attachment = MediaAttachment { + id: Uuid::now_v7(), + account_id, + content_type: String::from("image/jpeg"), + description: None, + blurhash: None, + file_path: Some(String::from("test.jpeg")), + remote_url: None, + created_at: Timestamp::now_utc(), + updated_at: Timestamp::now_utc() + }; let download = attachment_service.stream_file(&attachment).await.unwrap(); let mut img_bytes = BytesMut::new(); From 4c70567580b758f57bc1ab3b92863ff9aed3b8ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radek=20St=C4=99pie=C5=84?= Date: Thu, 21 Sep 2023 20:31:59 +0200 Subject: [PATCH 3/4] formatting issue --- crates/kitsune-core/src/service/attachment.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/kitsune-core/src/service/attachment.rs b/crates/kitsune-core/src/service/attachment.rs index d86457c5e..a0a52f96a 100644 --- a/crates/kitsune-core/src/service/attachment.rs +++ b/crates/kitsune-core/src/service/attachment.rs @@ -198,7 +198,7 @@ impl AttachmentService { { img_bytes.extend_from_slice(&chunk); } - + let img_bytes = img_bytes.freeze(); let final_bytes = DynImage::from_bytes(img_bytes) .map_err(UploadError::ImageProcessingError)? @@ -208,7 +208,7 @@ impl AttachmentService { image.encoder().bytes() }) .map_err(UploadError::ImageProcessingError)?; - + self.storage_backend .put(&upload.path, stream::once(async { Ok(final_bytes) })) } else { @@ -306,7 +306,7 @@ mod test { let base = hex_simd::decode_to_vec("ffd8ffe000104a46494600010101004800480000ffdb004300030202020202030202020303030304060404040404080606050609080a0a090809090a0c0f0c0a0b0e0b09090d110d0e0f101011100a0c12131210130f101010ffc9000b080001000101011100ffcc000600101005ffda0008010100003f00d2cf20ffd9").unwrap(); let mut jpeg = img_parts::jpeg::Jpeg::from_bytes(Bytes::from(base)).unwrap(); - + let comment_segment = JpegSegment::new_with_contents( markers::APP1, Bytes::from("Exif\0\0Some info to be stripped") From 5f24b36a19aa970010e6823501b6f2b954b308ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radek=20St=C4=99pie=C5=84?= Date: Sat, 23 Sep 2023 15:23:16 +0200 Subject: [PATCH 4/4] add a comment about mime types --- crates/kitsune-core/src/service/attachment.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/kitsune-core/src/service/attachment.rs b/crates/kitsune-core/src/service/attachment.rs index a0a52f96a..643d41bb3 100644 --- a/crates/kitsune-core/src/service/attachment.rs +++ b/crates/kitsune-core/src/service/attachment.rs @@ -37,6 +37,7 @@ fn is_allowed_filetype(value: &str, _ctx: &()) -> garde::Result { } fn is_image_type_with_supported_metadata(mime: &str) -> bool { + // TODO: migrate the match to use the mime crate enums matches!(mime, "image/jpeg" | "image/png" | "image/webp") }