From 0a0da1d9f36e6d5a7f738351d628fed9aa1497b1 Mon Sep 17 00:00:00 2001 From: Zeerooth Date: Sat, 23 Sep 2023 15:38:08 +0200 Subject: [PATCH] Remove exif info from image uploads (#352) * remove exif info during file uploads * add some space * formatting issue * add a comment about mime types --------- Co-authored-by: aumetra --- crates/kitsune-core/Cargo.toml | 2 + crates/kitsune-core/src/error.rs | 15 ++ crates/kitsune-core/src/service/attachment.rs | 198 +++++++++++++++++- 3 files changed, 208 insertions(+), 7 deletions(-) diff --git a/crates/kitsune-core/Cargo.toml b/crates/kitsune-core/Cargo.toml index 8704ffc06..5b5008bc7 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.12" 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..643d41bb3 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,11 @@ fn is_allowed_filetype(value: &str, _ctx: &()) -> garde::Result { Ok(()) } +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") +} + #[derive(TypedBuilder, Validate)] pub struct Update { #[garde(skip)] @@ -179,10 +185,38 @@ 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 stream = upload.stream; + pin_mut!(stream); + + let mut img_bytes = BytesMut::new(); + 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 +239,153 @@ 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() + } +}