From ee7df0dc3558909ae432f92f65a898571c6e374a Mon Sep 17 00:00:00 2001 From: Dessalines Date: Sun, 5 May 2024 21:47:02 -0400 Subject: [PATCH] Adding an image_details table to store image dimensions. - Adds an image_details table, which stores the height, width, and content_type for local and remote images. - For LocalImages, this information already comes back with the upload. - For RemoteImages, it calls the pictrs details endpoint. - Fixed some issues with proxying non-image urls. - Fixes #3328 - Also fixes #4703 --- api_tests/prepare-drone-federation-test.sh | 2 +- api_tests/src/image.spec.ts | 1 - crates/api_common/src/request.rs | 204 ++++++++++++------ crates/api_common/src/utils.rs | 54 ++++- crates/api_crud/src/post/create.rs | 4 +- crates/api_crud/src/post/update.rs | 7 +- crates/db_schema/src/impls/images.rs | 32 ++- crates/db_schema/src/schema.rs | 14 +- crates/db_schema/src/source/images.rs | 38 +++- crates/db_views/src/post_view.rs | 4 + crates/db_views/src/structs.rs | 4 +- crates/routes/src/images.rs | 72 ++++--- docker/docker-compose.yml | 2 +- .../down.sql | 7 + .../up.sql | 15 ++ 15 files changed, 332 insertions(+), 128 deletions(-) create mode 100644 migrations/2024-05-05-162540_add_image_detail_table/down.sql create mode 100644 migrations/2024-05-05-162540_add_image_detail_table/up.sql diff --git a/api_tests/prepare-drone-federation-test.sh b/api_tests/prepare-drone-federation-test.sh index 147a553ab..6249348f9 100755 --- a/api_tests/prepare-drone-federation-test.sh +++ b/api_tests/prepare-drone-federation-test.sh @@ -15,7 +15,7 @@ export LEMMY_TEST_FAST_FEDERATION=1 # by default, the persistent federation queu # pictrs setup if [ ! -f "api_tests/pict-rs" ]; then - curl "https://git.asonix.dog/asonix/pict-rs/releases/download/v0.5.0-beta.2/pict-rs-linux-amd64" -o api_tests/pict-rs + curl "https://git.asonix.dog/asonix/pict-rs/releases/download/v0.5.13/pict-rs-linux-amd64" -o api_tests/pict-rs chmod +x api_tests/pict-rs fi ./api_tests/pict-rs \ diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index a1b3c3f3e..7326420ca 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -29,7 +29,6 @@ import { unfollows, getPost, waitUntil, - randomString, createPostWithThumbnail, } from "./shared"; const downloadFileSync = require("download-file-sync"); diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index a2720998b..929f26bac 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -3,14 +3,15 @@ use crate::{ lemmy_db_schema::traits::Crud, post::{LinkMetadata, OpenGraphData}, send_activity::{ActivityChannel, SendActivityData}, - utils::{local_site_opt_to_sensitive, proxy_image_link, proxy_image_link_opt_apub}, + utils::{local_site_opt_to_sensitive, proxy_image_link}, }; use activitypub_federation::config::Data; +use chrono::{DateTime, Utc}; use encoding_rs::{Encoding, UTF_8}; use lemmy_db_schema::{ newtypes::DbUrl, source::{ - images::{LocalImage, LocalImageForm}, + images::{ImageDetails, ImageDetailsForm, LocalImage, LocalImageForm}, local_site::LocalSite, post::{Post, PostUpdateForm}, }, @@ -25,7 +26,7 @@ use lemmy_utils::{ use mime::Mime; use reqwest::{header::CONTENT_TYPE, Client, ClientBuilder}; use reqwest_middleware::ClientWithMiddleware; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use tracing::info; use url::Url; use urlencoding::encode; @@ -95,38 +96,48 @@ pub fn generate_post_link_metadata( let allow_sensitive = local_site_opt_to_sensitive(&local_site); let allow_generate_thumbnail = allow_sensitive || !post.nsfw; - // Use custom thumbnail if available and its not an image post - let thumbnail_url = if !is_image_post && custom_thumbnail.is_some() { - custom_thumbnail - } - // Use federated thumbnail if available - else if federated_thumbnail.is_some() { - federated_thumbnail - } - // Generate local thumbnail if allowed - else if allow_generate_thumbnail { - match post - .url - .filter(|_| is_image_post) - .or(metadata.opengraph_data.image) - { - Some(url) => generate_pictrs_thumbnail(&url, &context).await.ok(), - None => None, + let thumbnail_url = if is_image_post { + if allow_generate_thumbnail { + match post.url { + Some(url) => generate_pictrs_thumbnail(&url, &context) + .await + .ok() + .map(Into::into), + None => None, + } + } else { + None + } + } else { + // Use custom thumbnail if available and its not an image post + if let Some(custom_thumbnail) = custom_thumbnail { + proxy_image_link(custom_thumbnail, &context).await.ok() + } + // Use federated thumbnail if available + else if let Some(federated_thumbnail) = federated_thumbnail { + proxy_image_link(federated_thumbnail, &context).await.ok() + } + // Generate local thumbnail if allowed + else if allow_generate_thumbnail { + match metadata.opengraph_data.image { + Some(url) => generate_pictrs_thumbnail(&url, &context) + .await + .ok() + .map(Into::into), + None => None, + } + } + // Otherwise use opengraph preview image directly + else { + metadata.opengraph_data.image } - } - // Otherwise use opengraph preview image directly - else { - metadata.opengraph_data.image.map(Into::into) }; - // Proxy the image fetch if necessary - let proxied_thumbnail_url = proxy_image_link_opt_apub(thumbnail_url, &context).await?; - let form = PostUpdateForm { embed_title: Some(metadata.opengraph_data.title), embed_description: Some(metadata.opengraph_data.description), embed_video_url: Some(metadata.opengraph_data.embed_video_url), - thumbnail_url: Some(proxied_thumbnail_url), + thumbnail_url: Some(thumbnail_url), url_content_type: Some(metadata.content_type), ..Default::default() }; @@ -142,17 +153,6 @@ pub fn generate_post_link_metadata( fn extract_opengraph_data(html_bytes: &[u8], url: &Url) -> LemmyResult { let html = String::from_utf8_lossy(html_bytes); - // Make sure the first line is doctype html - let first_line = html - .trim_start() - .lines() - .next() - .ok_or(LemmyErrorType::NoLinesInHtml)? - .to_lowercase(); - - if !first_line.starts_with(" LemmyResult, - msg: String, +#[derive(Deserialize, Serialize, Debug)] +pub struct PictrsResponse { + pub files: Option>, + pub msg: String, } -#[derive(Deserialize, Debug)] -struct PictrsFile { - file: String, - delete_token: String, +#[derive(Deserialize, Serialize, Debug)] +pub struct PictrsFile { + pub file: String, + pub delete_token: String, + pub details: PictrsFileDetails, } -#[derive(Deserialize, Debug)] +impl PictrsFile { + pub fn thumbnail_url(&self, protocol_and_hostname: &str) -> Result { + Url::parse(&format!( + "{protocol_and_hostname}/pictrs/image/{}", + self.file + )) + } +} + +#[derive(Deserialize, Serialize, Debug)] +pub struct PictrsFileDetails { + pub width: u16, + pub height: u16, + pub content_type: String, + pub created_at: DateTime, +} + +impl PictrsFileDetails { + /// Builds the image form. This should always use the thumbnail_url, + /// Because the post_view joins to it + pub fn build_image_details_form(&self, thumbnail_url: &Url) -> ImageDetailsForm { + ImageDetailsForm { + link: thumbnail_url.clone().into(), + width: self.width.into(), + height: self.height.into(), + content_type: self.content_type.clone(), + } + } +} + +#[derive(Deserialize, Serialize, Debug)] struct PictrsPurgeResponse { msg: String, } @@ -295,35 +326,78 @@ async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> L encode(image_url.as_str()) ); - let response = context + let res: PictrsResponse = context .client() .get(&fetch_url) .timeout(REQWEST_TIMEOUT) .send() + .await? + .json() .await?; - let response: PictrsResponse = response.json().await?; + if let Some(image) = res.files.unwrap_or_default().first() { + let form = LocalImageForm { + // This is none because its an internal request. + // IE, a local user shouldn't get to delete the thumbnails for their link posts + local_user_id: None, + pictrs_alias: image.file.clone(), + pictrs_delete_token: image.delete_token.clone(), + }; + LocalImage::create(&mut context.pool(), &form).await?; + + let protocol_and_hostname = context.settings().get_protocol_and_hostname(); + let thumbnail_url = image.thumbnail_url(&protocol_and_hostname)?; + + // Also store the details for the image + let details_form = image.details.build_image_details_form(&thumbnail_url); + ImageDetails::create(&mut context.pool(), &details_form).await?; - if response.msg == "ok" { - let thumbnail_url = Url::parse(&format!( - "{}/pictrs/image/{}", - context.settings().get_protocol_and_hostname(), - response.files.first().expect("missing pictrs file").file - ))?; - for uploaded_image in response.files { - let form = LocalImageForm { - local_user_id: None, - pictrs_alias: uploaded_image.file.to_string(), - pictrs_delete_token: uploaded_image.delete_token.to_string(), - }; - LocalImage::create(&mut context.pool(), &form).await?; - } Ok(thumbnail_url) } else { - Err(LemmyErrorType::PictrsResponseError(response.msg))? + Err(LemmyErrorType::PictrsResponseError(res.msg))? } } +/// Fetches the image details for pictrs proxied images +/// +/// We don't need to check for image mode, as that's already been done +#[tracing::instrument(skip_all)] +pub async fn fetch_pictrs_proxied_image_details( + image_url: &Url, + context: &LemmyContext, +) -> LemmyResult { + let pictrs_config = context.settings().pictrs_config()?; + + // Pictrs needs you to fetch the proxied image before you can fetch the details + let proxy_url = format!( + "{}image/original?proxy={}", + context.settings().pictrs_config()?.url, + encode(image_url.as_str()) + ); + + let res = context.client().get(&proxy_url).send().await?.status(); + if !res.is_success() { + Err(LemmyErrorType::NotAnImageType)? + } + + let details_url = format!( + "{}image/details/original?proxy={}", + pictrs_config.url, + encode(image_url.as_str()) + ); + + let res = context + .client() + .get(&details_url) + .timeout(REQWEST_TIMEOUT) + .send() + .await? + .json() + .await?; + + Ok(res) +} + // TODO: get rid of this by reading content type from db #[tracing::instrument(skip_all)] async fn is_image_content_type(client: &ClientWithMiddleware, url: &Url) -> LemmyResult<()> { diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 4ab587928..d0adb73ad 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -1,6 +1,10 @@ use crate::{ context::LemmyContext, - request::{delete_image_from_pictrs, purge_image_from_pictrs}, + request::{ + delete_image_from_pictrs, + fetch_pictrs_proxied_image_details, + purge_image_from_pictrs, + }, site::{FederatedInstances, InstanceWithFederationState}, }; use chrono::{DateTime, Days, Local, TimeZone, Utc}; @@ -12,7 +16,7 @@ use lemmy_db_schema::{ community::{Community, CommunityModerator, CommunityUpdateForm}, community_block::CommunityBlock, email_verification::{EmailVerification, EmailVerificationForm}, - images::RemoteImage, + images::{ImageDetails, RemoteImage}, instance::Instance, instance_block::InstanceBlock, local_site::LocalSite, @@ -931,7 +935,20 @@ pub async fn process_markdown( if context.settings().pictrs_config()?.image_mode() == PictrsImageMode::ProxyAllImages { let (text, links) = markdown_rewrite_image_links(text); - RemoteImage::create(&mut context.pool(), links).await?; + + // Create images and image detail rows + for link in links { + RemoteImage::create(&mut context.pool(), &link).await?; + + // Insert image details for the remote image + let details_res = fetch_pictrs_proxied_image_details(&link, context).await; + if let Ok(details) = details_res { + let proxied = + build_proxied_image_url(&link, &context.settings().get_protocol_and_hostname())?; + let details_form = details.build_image_details_form(&proxied); + ImageDetails::create(&mut context.pool(), &details_form).await?; + } + } Ok(text) } else { Ok(text) @@ -965,13 +982,19 @@ async fn proxy_image_link_internal( if link.domain() == Some(&context.settings().hostname) { Ok(link.into()) } else if image_mode == PictrsImageMode::ProxyAllImages { - let proxied = format!( - "{}/api/v3/image_proxy?url={}", - context.settings().get_protocol_and_hostname(), - encode(link.as_str()) - ); - RemoteImage::create(&mut context.pool(), vec![link]).await?; - Ok(Url::parse(&proxied)?.into()) + let proxied = build_proxied_image_url(&link, &context.settings().get_protocol_and_hostname())?; + + RemoteImage::create(&mut context.pool(), &link).await?; + + // This should fail softly, since pictrs might not even be running + let details_res = fetch_pictrs_proxied_image_details(&link, context).await; + + if let Ok(details) = details_res { + let details_form = details.build_image_details_form(&proxied); + ImageDetails::create(&mut context.pool(), &details_form).await?; + } + + Ok(proxied.into()) } else { Ok(link.into()) } @@ -1025,6 +1048,17 @@ pub async fn proxy_image_link_opt_apub( } } +fn build_proxied_image_url( + link: &Url, + protocol_and_hostname: &str, +) -> Result { + Url::parse(&format!( + "{}/api/v3/image_proxy?url={}", + protocol_and_hostname, + encode(link.as_str()) + )) +} + #[cfg(test)] #[allow(clippy::unwrap_used)] #[allow(clippy::indexing_slicing)] diff --git a/crates/api_crud/src/post/create.rs b/crates/api_crud/src/post/create.rs index fcd274c03..6e760bb3e 100644 --- a/crates/api_crud/src/post/create.rs +++ b/crates/api_crud/src/post/create.rs @@ -14,7 +14,6 @@ use lemmy_api_common::{ local_site_to_slur_regex, mark_post_as_read, process_markdown_opt, - proxy_image_link_opt_apub, EndpointType, }, }; @@ -75,7 +74,6 @@ pub async fn create_post( is_url_blocked(&url, &url_blocklist)?; check_url_scheme(&url)?; check_url_scheme(&custom_thumbnail)?; - let url = proxy_image_link_opt_apub(url, &context).await?; check_community_user_action( &local_user_view.person, @@ -125,7 +123,7 @@ pub async fn create_post( let post_form = PostInsertForm::builder() .name(data.name.trim().to_string()) - .url(url) + .url(url.map(Into::into)) .body(body) .alt_text(data.alt_text.clone()) .community_id(data.community_id) diff --git a/crates/api_crud/src/post/update.rs b/crates/api_crud/src/post/update.rs index 74e0c0d47..5fc28e883 100644 --- a/crates/api_crud/src/post/update.rs +++ b/crates/api_crud/src/post/update.rs @@ -11,7 +11,6 @@ use lemmy_api_common::{ get_url_blocklist, local_site_to_slur_regex, process_markdown_opt, - proxy_image_link_opt_apub, }, }; use lemmy_db_schema::{ @@ -86,10 +85,6 @@ pub async fn update_post( Err(LemmyErrorType::NoPostEditAllowed)? } - let url = match url { - Some(url) => Some(proxy_image_link_opt_apub(Some(url), &context).await?), - _ => Default::default(), - }; let language_id = data.language_id; CommunityLanguage::is_allowed_community_language( @@ -101,7 +96,7 @@ pub async fn update_post( let post_form = PostUpdateForm { name: data.name.clone(), - url, + url: Some(url.map(Into::into)), body: diesel_option_overwrite(body), alt_text: diesel_option_overwrite(data.alt_text.clone()), nsfw: data.nsfw, diff --git a/crates/db_schema/src/impls/images.rs b/crates/db_schema/src/impls/images.rs index 9589aeee3..8cc2aa795 100644 --- a/crates/db_schema/src/impls/images.rs +++ b/crates/db_schema/src/impls/images.rs @@ -1,7 +1,14 @@ use crate::{ newtypes::DbUrl, - schema::{local_image, remote_image}, - source::images::{LocalImage, LocalImageForm, RemoteImage, RemoteImageForm}, + schema::{image_details, local_image, remote_image}, + source::images::{ + ImageDetails, + ImageDetailsForm, + LocalImage, + LocalImageForm, + RemoteImage, + RemoteImageForm, + }, utils::{get_conn, DbPool}, }; use diesel::{ @@ -39,14 +46,13 @@ impl LocalImage { } impl RemoteImage { - pub async fn create(pool: &mut DbPool<'_>, links: Vec) -> Result { + pub async fn create(pool: &mut DbPool<'_>, link_: &Url) -> Result { let conn = &mut get_conn(pool).await?; - let forms = links - .into_iter() - .map(|url| RemoteImageForm { link: url.into() }) - .collect::>(); + let form = RemoteImageForm { + link: link_.clone().into(), + }; insert_into(remote_image::table) - .values(forms) + .values(form) .on_conflict_do_nothing() .execute(conn) .await @@ -67,3 +73,13 @@ impl RemoteImage { } } } + +impl ImageDetails { + pub async fn create(pool: &mut DbPool<'_>, form: &ImageDetailsForm) -> Result { + let conn = &mut get_conn(pool).await?; + insert_into(image_details::table) + .values(form) + .get_result::(conn) + .await + } +} diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index 50bdac751..73a1b109a 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -309,6 +309,16 @@ diesel::table! { } } +diesel::table! { + image_details (link) { + link -> Text, + width -> Int4, + height -> Int4, + content_type -> Text, + published -> Timestamptz, + } +} + diesel::table! { instance (id) { id -> Int4, @@ -849,8 +859,7 @@ diesel::table! { } diesel::table! { - remote_image (id) { - id -> Int4, + remote_image (link) { link -> Text, published -> Timestamptz, } @@ -1055,6 +1064,7 @@ diesel::allow_tables_to_appear_in_same_query!( federation_allowlist, federation_blocklist, federation_queue_state, + image_details, instance, instance_block, language, diff --git a/crates/db_schema/src/source/images.rs b/crates/db_schema/src/source/images.rs index da6be2a14..aa57b9494 100644 --- a/crates/db_schema/src/source/images.rs +++ b/crates/db_schema/src/source/images.rs @@ -1,6 +1,6 @@ use crate::newtypes::{DbUrl, LocalUserId}; #[cfg(feature = "full")] -use crate::schema::{local_image, remote_image}; +use crate::schema::{image_details, local_image, remote_image}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; @@ -22,7 +22,7 @@ use typed_builder::TypedBuilder; diesel(belongs_to(crate::source::local_user::LocalUser)) )] #[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))] -#[cfg_attr(feature = "full", diesel(primary_key(local_user_id)))] +#[cfg_attr(feature = "full", diesel(primary_key(pictrs_alias)))] pub struct LocalImage { pub local_user_id: Option, pub pictrs_alias: String, @@ -41,11 +41,14 @@ pub struct LocalImageForm { /// Stores all images which are hosted on remote domains. When attempting to proxy an image, it /// is checked against this table to avoid Lemmy being used as a general purpose proxy. -#[derive(PartialEq, Eq, Debug, Clone)] -#[cfg_attr(feature = "full", derive(Queryable, Identifiable))] +#[skip_serializing_none] +#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)] +#[cfg_attr(feature = "full", derive(Queryable, Selectable, Identifiable, TS))] +#[cfg_attr(feature = "full", ts(export))] #[cfg_attr(feature = "full", diesel(table_name = remote_image))] +#[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))] +#[cfg_attr(feature = "full", diesel(primary_key(link)))] pub struct RemoteImage { - pub id: i32, pub link: DbUrl, pub published: DateTime, } @@ -56,3 +59,28 @@ pub struct RemoteImage { pub struct RemoteImageForm { pub link: DbUrl, } + +#[skip_serializing_none] +#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)] +#[cfg_attr(feature = "full", derive(Queryable, Selectable, Identifiable, TS))] +#[cfg_attr(feature = "full", ts(export))] +#[cfg_attr(feature = "full", diesel(table_name = image_details))] +#[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))] +#[cfg_attr(feature = "full", diesel(primary_key(link)))] +pub struct ImageDetails { + pub link: DbUrl, + pub width: i32, + pub height: i32, + pub content_type: String, + pub published: DateTime, +} + +#[derive(Debug, Clone, TypedBuilder)] +#[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] +#[cfg_attr(feature = "full", diesel(table_name = image_details))] +pub struct ImageDetailsForm { + pub link: DbUrl, + pub width: i32, + pub height: i32, + pub content_type: String, +} diff --git a/crates/db_views/src/post_view.rs b/crates/db_views/src/post_view.rs index 9f3a29411..9513f286f 100644 --- a/crates/db_views/src/post_view.rs +++ b/crates/db_views/src/post_view.rs @@ -28,6 +28,7 @@ use lemmy_db_schema::{ community_follower, community_moderator, community_person_ban, + image_details, instance_block, local_user, local_user_language, @@ -237,10 +238,12 @@ fn queries<'a>() -> Queries< .inner_join(person::table) .inner_join(community::table) .inner_join(post::table) + .left_join(image_details::table.on(post::thumbnail_url.eq(image_details::link.nullable()))) .select(( post::all_columns, person::all_columns, community::all_columns, + image_details::all_columns.nullable(), is_creator_banned_from_community, is_local_user_banned_from_community_selection, creator_is_moderator, @@ -1666,6 +1669,7 @@ mod tests { public_key: inserted_person.public_key.clone(), last_refreshed_at: inserted_person.last_refreshed_at, }, + image_details: None, creator_banned_from_community: false, banned_from_community: false, creator_is_moderator: false, diff --git a/crates/db_views/src/structs.rs b/crates/db_views/src/structs.rs index 4311710ab..b3275dad0 100644 --- a/crates/db_views/src/structs.rs +++ b/crates/db_views/src/structs.rs @@ -8,7 +8,7 @@ use lemmy_db_schema::{ community::Community, custom_emoji::CustomEmoji, custom_emoji_keyword::CustomEmojiKeyword, - images::LocalImage, + images::{ImageDetails, LocalImage}, local_site::LocalSite, local_site_rate_limit::LocalSiteRateLimit, local_user::LocalUser, @@ -130,6 +130,7 @@ pub struct PostView { pub post: Post, pub creator: Person, pub community: Community, + pub image_details: Option, pub creator_banned_from_community: bool, pub banned_from_community: bool, pub creator_is_moderator: bool, @@ -216,6 +217,7 @@ pub struct VoteView { pub score: i16, } +#[skip_serializing_none] #[derive(Debug, Serialize, Deserialize, Clone)] #[cfg_attr(feature = "full", derive(TS, Queryable))] #[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))] diff --git a/crates/routes/src/images.rs b/crates/routes/src/images.rs index 671aa223e..ff0e1c051 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -1,18 +1,16 @@ use actix_web::{ body::BodyStream, - error, http::{ header::{HeaderName, ACCEPT_ENCODING, HOST}, StatusCode, }, web, web::Query, - Error, HttpRequest, HttpResponse, }; use futures::stream::{Stream, StreamExt}; -use lemmy_api_common::context::LemmyContext; +use lemmy_api_common::{context::LemmyContext, request::PictrsFileDetails}; use lemmy_db_schema::source::{ images::{LocalImage, LocalImageForm, RemoteImage}, local_site::LocalSite, @@ -40,6 +38,7 @@ pub fn config( ) // This has optional query params: /image/{filename}?format=jpg&thumbnail=256 .service(web::resource("/pictrs/image/{filename}").route(web::get().to(full_res))) + .service(web::resource("/pictrs/image/details/original").route(web::get().to(details))) .service(web::resource("/pictrs/image/delete/{token}/{filename}").route(web::get().to(delete))); } @@ -56,11 +55,17 @@ struct Images { } #[derive(Deserialize)] -struct PictrsParams { +struct PictrsGetParams { format: Option, thumbnail: Option, } +#[derive(Deserialize)] +struct PictrsDetailsParams { + alias: Option, + proxy: Option, +} + fn adapt_request( request: &HttpRequest, client: &ClientWithMiddleware, @@ -92,7 +97,7 @@ async fn upload( local_user_view: LocalUserView, client: web::Data, context: web::Data, -) -> Result { +) -> LemmyResult { // TODO: check rate limit here let pictrs_config = context.settings().pictrs_config()?; let image_url = format!("{}image", pictrs_config.url); @@ -106,11 +111,10 @@ async fn upload( .timeout(Duration::from_secs(pictrs_config.upload_timeout)) .body(Body::wrap_stream(make_send(body))) .send() - .await - .map_err(error::ErrorBadRequest)?; + .await?; let status = res.status(); - let images = res.json::().await.map_err(error::ErrorBadRequest)?; + let images = res.json::().await?; if let Some(images) = &images.files { for uploaded_image in images { let form = LocalImageForm { @@ -118,9 +122,7 @@ async fn upload( pictrs_alias: uploaded_image.file.to_string(), pictrs_delete_token: uploaded_image.delete_token.to_string(), }; - LocalImage::create(&mut context.pool(), &form) - .await - .map_err(error::ErrorBadRequest)?; + LocalImage::create(&mut context.pool(), &form).await?; } } @@ -129,16 +131,14 @@ async fn upload( async fn full_res( filename: web::Path, - web::Query(params): web::Query, + web::Query(params): web::Query, req: HttpRequest, client: web::Data, context: web::Data, local_user_view: Option, -) -> Result { +) -> LemmyResult { // block access to images if instance is private and unauthorized, public - let local_site = LocalSite::read(&mut context.pool()) - .await - .map_err(error::ErrorBadRequest)?; + let local_site = LocalSite::read(&mut context.pool()).await?; if local_site.private_instance && local_user_view.is_none() { return Ok(HttpResponse::Unauthorized().finish()); } @@ -169,7 +169,7 @@ async fn image( url: String, req: HttpRequest, client: &ClientWithMiddleware, -) -> Result { +) -> LemmyResult { let mut client_req = adapt_request(&req, client, url); if let Some(addr) = req.head().peer_addr { @@ -180,7 +180,7 @@ async fn image( client_req = client_req.header("X-Forwarded-For", addr.to_string()); } - let res = client_req.send().await.map_err(error::ErrorBadRequest)?; + let res = client_req.send().await?; if res.status() == StatusCode::NOT_FOUND { return Ok(HttpResponse::NotFound().finish()); @@ -202,7 +202,7 @@ async fn delete( context: web::Data, // require login _local_user_view: LocalUserView, -) -> Result { +) -> LemmyResult { let (token, file) = components.into_inner(); let pictrs_config = context.settings().pictrs_config()?; @@ -214,11 +214,9 @@ async fn delete( client_req = client_req.header("X-Forwarded-For", addr.to_string()); } - let res = client_req.send().await.map_err(error::ErrorBadRequest)?; + let res = client_req.send().await?; - LocalImage::delete_by_alias(&mut context.pool(), &file) - .await - .map_err(error::ErrorBadRequest)?; + LocalImage::delete_by_alias(&mut context.pool(), &file).await?; Ok(HttpResponse::build(res.status()).body(BodyStream::new(res.bytes_stream()))) } @@ -228,8 +226,33 @@ pub struct ImageProxyParams { url: String, } +async fn details( + web::Query(params): web::Query, + client: web::Data, + context: web::Data, +) -> LemmyResult { + let pictrs_config = context.settings().pictrs_config()?; + + let mut url = format!("{}image/details/original", pictrs_config.url); + if let Some(alias) = params.alias { + url = format!("{url}?alias={alias}"); + }; + + if let Some(proxy) = params.proxy { + url = format!("{url}?proxy={proxy}"); + }; + + let res = client.get(url).send().await?; + let status = res.status(); + let json = res.json::().await?; + + Ok(HttpResponse::build(status).json(json)) +} + pub async fn image_proxy( Query(params): Query, + req: HttpRequest, + client: web::Data, context: web::Data, ) -> LemmyResult { let url = Url::parse(&decode(¶ms.url)?)?; @@ -240,9 +263,8 @@ pub async fn image_proxy( let pictrs_config = context.settings().pictrs_config()?; let url = format!("{}image/original?proxy={}", pictrs_config.url, ¶ms.url); - let image_response = context.client().get(url).send().await?; - Ok(HttpResponse::Ok().streaming(image_response.bytes_stream())) + image(url, req, &client).await } fn make_send(mut stream: S) -> impl Stream + Send + Unpin + 'static diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 141368936..c690a5f48 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -75,7 +75,7 @@ services: init: true pictrs: - image: asonix/pictrs:0.5.0-rc.2 + image: asonix/pictrs:0.5.13 # this needs to match the pictrs url in lemmy.hjson hostname: pictrs # we can set options to pictrs like this, here we set max. image size and forced format for conversion diff --git a/migrations/2024-05-05-162540_add_image_detail_table/down.sql b/migrations/2024-05-05-162540_add_image_detail_table/down.sql new file mode 100644 index 000000000..819277c7f --- /dev/null +++ b/migrations/2024-05-05-162540_add_image_detail_table/down.sql @@ -0,0 +1,7 @@ +ALTER TABLE remote_image + ADD UNIQUE (link), + DROP CONSTRAINT remote_image_pkey, + ADD COLUMN id serial PRIMARY KEY; + +DROP TABLE image_details; + diff --git a/migrations/2024-05-05-162540_add_image_detail_table/up.sql b/migrations/2024-05-05-162540_add_image_detail_table/up.sql new file mode 100644 index 000000000..1dbada93e --- /dev/null +++ b/migrations/2024-05-05-162540_add_image_detail_table/up.sql @@ -0,0 +1,15 @@ +-- Drop the id column from the remote_image table, just use link +ALTER TABLE remote_image + DROP COLUMN id, + ADD PRIMARY KEY (link), + DROP CONSTRAINT remote_image_link_key; + +-- No good way to do references here unfortunately, unless we combine the images tables +-- The link should be the URL, not the pictrs_alias, to allow joining from post.thumbnail_url +CREATE TABLE image_details ( + link text PRIMARY KEY, + width integer NOT NULL, + height integer NOT NULL, + content_type text NOT NULL, + published timestamptz DEFAULT now() NOT NULL +);