From 56ab67f80402752052d0a222c52b4dc3df44cffb Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Thu, 12 Dec 2024 15:59:07 +0100 Subject: [PATCH 01/16] Split image endpoints into API v3 and v4 --- crates/routes/src/images.rs | 64 ++--- src/api_routes_v3.rs | 490 ++++++++++++++++++------------------ src/api_routes_v4.rs | 21 +- 3 files changed, 286 insertions(+), 289 deletions(-) diff --git a/crates/routes/src/images.rs b/crates/routes/src/images.rs index 50897b95dd..dc981f6177 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -18,27 +18,13 @@ use lemmy_db_schema::source::{ local_site::LocalSite, }; use lemmy_db_views::structs::LocalUserView; -use lemmy_utils::{error::LemmyResult, rate_limit::RateLimitCell, REQWEST_TIMEOUT}; +use lemmy_utils::{error::LemmyResult, REQWEST_TIMEOUT}; use reqwest::Body; -use reqwest_middleware::{ClientWithMiddleware, RequestBuilder}; +use reqwest_middleware::RequestBuilder; use serde::Deserialize; use std::time::Duration; use url::Url; -pub fn config(cfg: &mut ServiceConfig, client: ClientWithMiddleware, rate_limit: &RateLimitCell) { - cfg - .app_data(Data::new(client)) - .service( - resource("/pictrs/image") - .wrap(rate_limit.image()) - .route(post().to(upload)), - ) - // This has optional query params: /image/{filename}?format=jpg&thumbnail=256 - .service(resource("/pictrs/image/{filename}").route(get().to(full_res))) - .service(resource("/pictrs/image/delete/{token}/{filename}").route(get().to(delete))) - .service(resource("/pictrs/healthz").route(get().to(healthz))); -} - trait ProcessUrl { /// If thumbnail or format is given, this uses the pictrs process endpoint. /// Otherwise, it uses the normal pictrs url (IE image/original). @@ -46,7 +32,7 @@ trait ProcessUrl { } #[derive(Deserialize, Clone)] -struct PictrsGetParams { +pub struct PictrsGetParams { format: Option, thumbnail: Option, } @@ -99,15 +85,12 @@ impl ProcessUrl for ImageProxyParams { } } } -fn adapt_request( - request: &HttpRequest, - client: &ClientWithMiddleware, - url: String, -) -> RequestBuilder { +fn adapt_request(request: &HttpRequest, context: &LemmyContext, url: String) -> RequestBuilder { // remove accept-encoding header so that pictrs doesn't compress the response const INVALID_HEADERS: &[HeaderName] = &[ACCEPT_ENCODING, HOST]; - let client_request = client + let client_request = context + .client() .request(convert_method(request.method()), url) .timeout(REQWEST_TIMEOUT); @@ -124,19 +107,17 @@ fn adapt_request( }) } -async fn upload( +pub async fn upload_image( req: HttpRequest, body: Payload, // require login local_user_view: LocalUserView, - client: Data, context: Data, ) -> LemmyResult { - // TODO: check rate limit here let pictrs_config = context.settings().pictrs_config()?; let image_url = format!("{}image", pictrs_config.url); - let mut client_req = adapt_request(&req, &client, image_url); + let mut client_req = adapt_request(&req, &context, image_url); if let Some(addr) = req.head().peer_addr { client_req = client_req.header("X-Forwarded-For", addr.to_string()) @@ -169,11 +150,10 @@ async fn upload( Ok(HttpResponse::build(convert_status(status)).json(images)) } -async fn full_res( +pub async fn get_full_res_image( filename: Path, Query(params): Query, req: HttpRequest, - client: Data, context: Data, local_user_view: Option, ) -> LemmyResult { @@ -189,15 +169,11 @@ async fn full_res( let processed_url = params.process_url(name, &pictrs_config.url); - image(processed_url, req, &client).await + image(processed_url, req, &context).await } -async fn image( - url: String, - req: HttpRequest, - client: &ClientWithMiddleware, -) -> LemmyResult { - let mut client_req = adapt_request(&req, client, url); +async fn image(url: String, req: HttpRequest, context: &LemmyContext) -> LemmyResult { + let mut client_req = adapt_request(&req, context, url); if let Some(addr) = req.head().peer_addr { client_req = client_req.header("X-Forwarded-For", addr.to_string()); @@ -222,10 +198,9 @@ async fn image( Ok(client_res.body(BodyStream::new(res.bytes_stream()))) } -async fn delete( +pub async fn delete_image( components: Path<(String, String)>, req: HttpRequest, - client: Data, context: Data, // require login _local_user_view: LocalUserView, @@ -235,7 +210,7 @@ async fn delete( let pictrs_config = context.settings().pictrs_config()?; let url = format!("{}image/delete/{}/{}", pictrs_config.url, &token, &file); - let mut client_req = adapt_request(&req, &client, url); + let mut client_req = adapt_request(&req, &context, url); if let Some(addr) = req.head().peer_addr { client_req = client_req.header("X-Forwarded-For", addr.to_string()); @@ -248,15 +223,11 @@ async fn delete( Ok(HttpResponse::build(convert_status(res.status())).body(BodyStream::new(res.bytes_stream()))) } -async fn healthz( - req: HttpRequest, - client: Data, - context: Data, -) -> LemmyResult { +pub async fn pictrs_healthz(req: HttpRequest, context: Data) -> LemmyResult { let pictrs_config = context.settings().pictrs_config()?; let url = format!("{}healthz", pictrs_config.url); - let mut client_req = adapt_request(&req, &client, url); + let mut client_req = adapt_request(&req, &context, url); if let Some(addr) = req.head().peer_addr { client_req = client_req.header("X-Forwarded-For", addr.to_string()); @@ -270,7 +241,6 @@ async fn healthz( pub async fn image_proxy( Query(params): Query, req: HttpRequest, - client: Data, context: Data, ) -> LemmyResult, HttpResponse>> { let url = Url::parse(¶ms.url)?; @@ -291,7 +261,7 @@ pub async fn image_proxy( Ok(Either::Left(Redirect::to(url.to_string()).respond_to(&req))) } else { // Proxy the image data through Lemmy - Ok(Either::Right(image(processed_url, req, &client).await?)) + Ok(Either::Right(image(processed_url, req, &context).await?)) } } diff --git a/src/api_routes_v3.rs b/src/api_routes_v3.rs index eefaf5b871..56dde6db95 100644 --- a/src/api_routes_v3.rs +++ b/src/api_routes_v3.rs @@ -134,252 +134,262 @@ use lemmy_apub::api::{ search::search, user_settings_backup::{export_settings, import_settings}, }; -use lemmy_routes::images::image_proxy; +use lemmy_routes::images::{delete_image, get_full_res_image, image_proxy, pictrs_healthz, upload_image}; use lemmy_utils::rate_limit::RateLimitCell; // Deprecated, use api v4 instead. // When removing api v3, we also need to rewrite all links in database with // `/api/v3/image_proxy` to use `/api/v4/image_proxy` instead. pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { - cfg.service( - scope("/api/v3") - .route("/image_proxy", get().to(image_proxy)) - // Site - .service( - scope("/site") - .wrap(rate_limit.message()) - .route("", get().to(get_site_v3)) - // Admin Actions - .route("", post().to(create_site)) - .route("", put().to(update_site)) - .route("/block", post().to(user_block_instance)), - ) - .service( - resource("/modlog") - .wrap(rate_limit.message()) - .route(get().to(get_mod_log)), - ) - .service( - resource("/search") - .wrap(rate_limit.search()) - .route(get().to(search)), - ) - .service( - resource("/resolve_object") - .wrap(rate_limit.message()) - .route(get().to(resolve_object)), - ) - // Community - .service( - resource("/community") - .guard(guard::Post()) - .wrap(rate_limit.register()) - .route(post().to(create_community)), - ) - .service( - scope("/community") - .wrap(rate_limit.message()) - .route("", get().to(get_community)) - .route("", put().to(update_community)) - .route("/hide", put().to(hide_community)) - .route("/list", get().to(list_communities)) - .route("/follow", post().to(follow_community)) - .route("/block", post().to(user_block_community)) - .route("/delete", post().to(delete_community)) - // Mod Actions - .route("/remove", post().to(remove_community)) - .route("/transfer", post().to(transfer_community)) - .route("/ban_user", post().to(ban_from_community)) - .route("/mod", post().to(add_mod_to_community)), - ) - .service( - scope("/federated_instances") - .wrap(rate_limit.message()) - .route("", get().to(get_federated_instances)), - ) - // Post - .service( - // Handle POST to /post separately to add the post() rate limitter - resource("/post") - .guard(guard::Post()) - .wrap(rate_limit.post()) - .route(post().to(create_post)), - ) - .service( - scope("/post") - .wrap(rate_limit.message()) - .route("", get().to(get_post)) - .route("", put().to(update_post)) - .route("/delete", post().to(delete_post)) - .route("/remove", post().to(remove_post)) - .route("/mark_as_read", post().to(mark_post_as_read)) - .route("/hide", post().to(hide_post)) - .route("/lock", post().to(lock_post)) - .route("/feature", post().to(feature_post)) - .route("/list", get().to(list_posts)) - .route("/like", post().to(like_post)) - .route("/like/list", get().to(list_post_likes)) - .route("/save", put().to(save_post)) - .route("/report", post().to(create_post_report)) - .route("/report/resolve", put().to(resolve_post_report)) - .route("/report/list", get().to(list_post_reports)) - .route("/site_metadata", get().to(get_link_metadata)), - ) - // Comment - .service( - // Handle POST to /comment separately to add the comment() rate limitter - resource("/comment") - .guard(guard::Post()) - .wrap(rate_limit.comment()) - .route(post().to(create_comment)), - ) - .service( - scope("/comment") - .wrap(rate_limit.message()) - .route("", get().to(get_comment)) - .route("", put().to(update_comment)) - .route("/delete", post().to(delete_comment)) - .route("/remove", post().to(remove_comment)) - .route("/mark_as_read", post().to(mark_reply_as_read)) - .route("/distinguish", post().to(distinguish_comment)) - .route("/like", post().to(like_comment)) - .route("/like/list", get().to(list_comment_likes)) - .route("/save", put().to(save_comment)) - .route("/list", get().to(list_comments)) - .route("/report", post().to(create_comment_report)) - .route("/report/resolve", put().to(resolve_comment_report)) - .route("/report/list", get().to(list_comment_reports)), - ) - // Private Message - .service( - scope("/private_message") - .wrap(rate_limit.message()) - .route("/list", get().to(get_private_message)) - .route("", post().to(create_private_message)) - .route("", put().to(update_private_message)) - .route("/delete", post().to(delete_private_message)) - .route("/mark_as_read", post().to(mark_pm_as_read)) - .route("/report", post().to(create_pm_report)) - .route("/report/resolve", put().to(resolve_pm_report)) - .route("/report/list", get().to(list_pm_reports)), - ) - // User - .service( - // Account action, I don't like that it's in /user maybe /accounts - // Handle /user/register separately to add the register() rate limiter - resource("/user/register") - .guard(guard::Post()) - .wrap(rate_limit.register()) - .route(post().to(register)), - ) - // User - .service( - // Handle /user/login separately to add the register() rate limiter - // TODO: pretty annoying way to apply rate limits for register and login, we should - // group them under a common path so that rate limit is only applied once (eg under - // /account). - resource("/user/login") - .guard(guard::Post()) - .wrap(rate_limit.register()) - .route(post().to(login)), - ) - .service( - resource("/user/password_reset") - .wrap(rate_limit.register()) - .route(post().to(reset_password)), - ) - .service( - // Handle captcha separately - resource("/user/get_captcha") - .wrap(rate_limit.post()) - .route(get().to(get_captcha)), - ) - .service( - resource("/user/export_settings") - .wrap(rate_limit.import_user_settings()) - .route(get().to(export_settings)), - ) - .service( - resource("/user/import_settings") - .wrap(rate_limit.import_user_settings()) - .route(post().to(import_settings)), - ) - // TODO, all the current account related actions under /user need to get moved here eventually - .service( - scope("/account") - .wrap(rate_limit.message()) - .route("/list_media", get().to(list_media)), - ) - // User actions - .service( - scope("/user") - .wrap(rate_limit.message()) - .route("", get().to(read_person)) - .route("/mention", get().to(list_mentions)) - .route( - "/mention/mark_as_read", - post().to(mark_person_mention_as_read), - ) - .route("/replies", get().to(list_replies)) - // Admin action. I don't like that it's in /user - .route("/ban", post().to(ban_from_site)) - .route("/banned", get().to(list_banned_users)) - .route("/block", post().to(user_block_person)) - // TODO Account actions. I don't like that they're in /user maybe /accounts - .route("/logout", post().to(logout)) - .route("/delete_account", post().to(delete_account)) - .route("/password_change", post().to(change_password_after_reset)) - // TODO mark_all_as_read feels off being in this section as well - .route("/mark_all_as_read", post().to(mark_all_notifications_read)) - .route("/save_user_settings", put().to(save_user_settings)) - .route("/change_password", put().to(change_password)) - .route("/report_count", get().to(report_count)) - .route("/unread_count", get().to(unread_count)) - .route("/verify_email", post().to(verify_email)) - .route("/leave_admin", post().to(leave_admin)) - .route("/totp/generate", post().to(generate_totp_secret)) - .route("/totp/update", post().to(update_totp)) - .route("/list_logins", get().to(list_logins)) - .route("/validate_auth", get().to(validate_auth)), - ) - // Admin Actions - .service( - scope("/admin") - .wrap(rate_limit.message()) - .route("/add", post().to(add_admin)) - .route( - "/registration_application/count", - get().to(get_unread_registration_application_count), - ) - .route( - "/registration_application/list", - get().to(list_registration_applications), - ) - .route( - "/registration_application/approve", - put().to(approve_registration_application), - ) - .route( - "/registration_application", - get().to(get_registration_application), - ) - .route("/list_all_media", get().to(list_all_media)) - .service( - scope("/purge") - .route("/person", post().to(purge_person)) - .route("/community", post().to(purge_community)) - .route("/post", post().to(purge_post)) - .route("/comment", post().to(purge_comment)), - ), - ) - .service( - scope("/custom_emoji") - .wrap(rate_limit.message()) - .route("", post().to(create_custom_emoji)) - .route("", put().to(update_custom_emoji)) - .route("/delete", post().to(delete_custom_emoji)), - ), - ); + cfg + .service( + resource("/pictrs/image") + .wrap(rate_limit.image()) + .route(post().to(upload_image)), + ) + .service(resource("/pictrs/image/{filename}").route(get().to(get_full_res_image))) + .service(resource("/pictrs/image/delete/{token}/{filename}").route(get().to(delete_image))) + .service(resource("/pictrs/healthz").route(get().to(pictrs_healthz))) + .service( + scope("/api/v3") + .route("/image_proxy", get().to(image_proxy)) + // Site + .service( + scope("/site") + .wrap(rate_limit.message()) + .route("", get().to(get_site_v3)) + // Admin Actions + .route("", post().to(create_site)) + .route("", put().to(update_site)) + .route("/block", post().to(user_block_instance)), + ) + .service( + resource("/modlog") + .wrap(rate_limit.message()) + .route(get().to(get_mod_log)), + ) + .service( + resource("/search") + .wrap(rate_limit.search()) + .route(get().to(search)), + ) + .service( + resource("/resolve_object") + .wrap(rate_limit.message()) + .route(get().to(resolve_object)), + ) + // Community + .service( + resource("/community") + .guard(guard::Post()) + .wrap(rate_limit.register()) + .route(post().to(create_community)), + ) + .service( + scope("/community") + .wrap(rate_limit.message()) + .route("", get().to(get_community)) + .route("", put().to(update_community)) + .route("/hide", put().to(hide_community)) + .route("/list", get().to(list_communities)) + .route("/follow", post().to(follow_community)) + .route("/block", post().to(user_block_community)) + .route("/delete", post().to(delete_community)) + // Mod Actions + .route("/remove", post().to(remove_community)) + .route("/transfer", post().to(transfer_community)) + .route("/ban_user", post().to(ban_from_community)) + .route("/mod", post().to(add_mod_to_community)), + ) + .service( + scope("/federated_instances") + .wrap(rate_limit.message()) + .route("", get().to(get_federated_instances)), + ) + // Post + .service( + // Handle POST to /post separately to add the post() rate limitter + resource("/post") + .guard(guard::Post()) + .wrap(rate_limit.post()) + .route(post().to(create_post)), + ) + .service( + scope("/post") + .wrap(rate_limit.message()) + .route("", get().to(get_post)) + .route("", put().to(update_post)) + .route("/delete", post().to(delete_post)) + .route("/remove", post().to(remove_post)) + .route("/mark_as_read", post().to(mark_post_as_read)) + .route("/hide", post().to(hide_post)) + .route("/lock", post().to(lock_post)) + .route("/feature", post().to(feature_post)) + .route("/list", get().to(list_posts)) + .route("/like", post().to(like_post)) + .route("/like/list", get().to(list_post_likes)) + .route("/save", put().to(save_post)) + .route("/report", post().to(create_post_report)) + .route("/report/resolve", put().to(resolve_post_report)) + .route("/report/list", get().to(list_post_reports)) + .route("/site_metadata", get().to(get_link_metadata)), + ) + // Comment + .service( + // Handle POST to /comment separately to add the comment() rate limitter + resource("/comment") + .guard(guard::Post()) + .wrap(rate_limit.comment()) + .route(post().to(create_comment)), + ) + .service( + scope("/comment") + .wrap(rate_limit.message()) + .route("", get().to(get_comment)) + .route("", put().to(update_comment)) + .route("/delete", post().to(delete_comment)) + .route("/remove", post().to(remove_comment)) + .route("/mark_as_read", post().to(mark_reply_as_read)) + .route("/distinguish", post().to(distinguish_comment)) + .route("/like", post().to(like_comment)) + .route("/like/list", get().to(list_comment_likes)) + .route("/save", put().to(save_comment)) + .route("/list", get().to(list_comments)) + .route("/report", post().to(create_comment_report)) + .route("/report/resolve", put().to(resolve_comment_report)) + .route("/report/list", get().to(list_comment_reports)), + ) + // Private Message + .service( + scope("/private_message") + .wrap(rate_limit.message()) + .route("/list", get().to(get_private_message)) + .route("", post().to(create_private_message)) + .route("", put().to(update_private_message)) + .route("/delete", post().to(delete_private_message)) + .route("/mark_as_read", post().to(mark_pm_as_read)) + .route("/report", post().to(create_pm_report)) + .route("/report/resolve", put().to(resolve_pm_report)) + .route("/report/list", get().to(list_pm_reports)), + ) + // User + .service( + // Account action, I don't like that it's in /user maybe /accounts + // Handle /user/register separately to add the register() rate limiter + resource("/user/register") + .guard(guard::Post()) + .wrap(rate_limit.register()) + .route(post().to(register)), + ) + // User + .service( + // Handle /user/login separately to add the register() rate limiter + // TODO: pretty annoying way to apply rate limits for register and login, we should + // group them under a common path so that rate limit is only applied once (eg under + // /account). + resource("/user/login") + .guard(guard::Post()) + .wrap(rate_limit.register()) + .route(post().to(login)), + ) + .service( + resource("/user/password_reset") + .wrap(rate_limit.register()) + .route(post().to(reset_password)), + ) + .service( + // Handle captcha separately + resource("/user/get_captcha") + .wrap(rate_limit.post()) + .route(get().to(get_captcha)), + ) + .service( + resource("/user/export_settings") + .wrap(rate_limit.import_user_settings()) + .route(get().to(export_settings)), + ) + .service( + resource("/user/import_settings") + .wrap(rate_limit.import_user_settings()) + .route(post().to(import_settings)), + ) + // TODO, all the current account related actions under /user need to get moved here + // eventually + .service( + scope("/account") + .wrap(rate_limit.message()) + .route("/list_media", get().to(list_media)), + ) + // User actions + .service( + scope("/user") + .wrap(rate_limit.message()) + .route("", get().to(read_person)) + .route("/mention", get().to(list_mentions)) + .route( + "/mention/mark_as_read", + post().to(mark_person_mention_as_read), + ) + .route("/replies", get().to(list_replies)) + // Admin action. I don't like that it's in /user + .route("/ban", post().to(ban_from_site)) + .route("/banned", get().to(list_banned_users)) + .route("/block", post().to(user_block_person)) + // TODO Account actions. I don't like that they're in /user maybe /accounts + .route("/logout", post().to(logout)) + .route("/delete_account", post().to(delete_account)) + .route("/password_change", post().to(change_password_after_reset)) + // TODO mark_all_as_read feels off being in this section as well + .route("/mark_all_as_read", post().to(mark_all_notifications_read)) + .route("/save_user_settings", put().to(save_user_settings)) + .route("/change_password", put().to(change_password)) + .route("/report_count", get().to(report_count)) + .route("/unread_count", get().to(unread_count)) + .route("/verify_email", post().to(verify_email)) + .route("/leave_admin", post().to(leave_admin)) + .route("/totp/generate", post().to(generate_totp_secret)) + .route("/totp/update", post().to(update_totp)) + .route("/list_logins", get().to(list_logins)) + .route("/validate_auth", get().to(validate_auth)), + ) + // Admin Actions + .service( + scope("/admin") + .wrap(rate_limit.message()) + .route("/add", post().to(add_admin)) + .route( + "/registration_application/count", + get().to(get_unread_registration_application_count), + ) + .route( + "/registration_application/list", + get().to(list_registration_applications), + ) + .route( + "/registration_application/approve", + put().to(approve_registration_application), + ) + .route( + "/registration_application", + get().to(get_registration_application), + ) + .route("/list_all_media", get().to(list_all_media)) + .service( + scope("/purge") + .route("/person", post().to(purge_person)) + .route("/community", post().to(purge_community)) + .route("/post", post().to(purge_post)) + .route("/comment", post().to(purge_comment)), + ), + ) + .service( + scope("/custom_emoji") + .wrap(rate_limit.message()) + .route("", post().to(create_custom_emoji)) + .route("", put().to(update_custom_emoji)) + .route("/delete", post().to(delete_custom_emoji)), + ), + ); cfg.service( scope("/sitemap.xml") .wrap(rate_limit.message()) diff --git a/src/api_routes_v4.rs b/src/api_routes_v4.rs index a9f71c9da9..ee88274c94 100644 --- a/src/api_routes_v4.rs +++ b/src/api_routes_v4.rs @@ -159,7 +159,13 @@ use lemmy_apub::api::{ search::search, user_settings_backup::{export_settings, import_settings}, }; -use lemmy_routes::images::image_proxy; +use lemmy_routes::images::{ + delete_image, + get_full_res_image, + image_proxy, + pictrs_healthz, + upload_image, +}; use lemmy_utils::rate_limit::RateLimitCell; pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { @@ -387,6 +393,17 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .wrap(rate_limit.register()) .route("/authenticate", post().to(authenticate_with_oauth)), ) - .route("/sitemap.xml", get().to(get_sitemap)), + .route("/sitemap.xml", get().to(get_sitemap)) + .service( + scope("/image") + .service( + resource("") + .wrap(rate_limit.image()) + .route(post().to(upload_image)), + ) + .route("/{filename}", get().to(get_full_res_image)) + .route("{token}/{filename}", delete().to(delete_image)) + .route("/healthz", get().to(pictrs_healthz)), + ), ); } From 05843fb7b5d6d1808b2473274eb84bc503d47641 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Thu, 12 Dec 2024 16:06:51 +0100 Subject: [PATCH 02/16] Move into subfolders --- .../routes/src/{images.rs => images/mod.rs} | 68 ++----------------- crates/routes/src/images/utils.rs | 63 +++++++++++++++++ .../utils/src/utils/markdown/image_links.rs | 12 ++-- src/api_routes_v3.rs | 2 +- src/api_routes_v4.rs | 2 +- 5 files changed, 78 insertions(+), 69 deletions(-) rename crates/routes/src/{images.rs => images/mod.rs} (82%) create mode 100644 crates/routes/src/images/utils.rs diff --git a/crates/routes/src/images.rs b/crates/routes/src/images/mod.rs similarity index 82% rename from crates/routes/src/images.rs rename to crates/routes/src/images/mod.rs index dc981f6177..0f98b330e3 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images/mod.rs @@ -2,7 +2,6 @@ use actix_web::{ body::{BodyStream, BoxBody}, http::{ header::{HeaderName, ACCEPT_ENCODING, HOST}, - Method, StatusCode, }, web::*, @@ -10,8 +9,6 @@ use actix_web::{ HttpResponse, Responder, }; -use futures::stream::{Stream, StreamExt}; -use http::HeaderValue; use lemmy_api_common::{context::LemmyContext, request::PictrsResponse}; use lemmy_db_schema::source::{ images::{LocalImage, LocalImageForm, RemoteImage}, @@ -24,6 +21,9 @@ use reqwest_middleware::RequestBuilder; use serde::Deserialize; use std::time::Duration; use url::Url; +use utils::{convert_header, convert_method, convert_status, make_send}; + +mod utils; trait ProcessUrl { /// If thumbnail or format is given, this uses the pictrs process endpoint. @@ -223,7 +223,10 @@ pub async fn delete_image( Ok(HttpResponse::build(convert_status(res.status())).body(BodyStream::new(res.bytes_stream()))) } -pub async fn pictrs_healthz(req: HttpRequest, context: Data) -> LemmyResult { +pub async fn pictrs_healthz( + req: HttpRequest, + context: Data, +) -> LemmyResult { let pictrs_config = context.settings().pictrs_config()?; let url = format!("{}healthz", pictrs_config.url); @@ -264,60 +267,3 @@ pub async fn image_proxy( Ok(Either::Right(image(processed_url, req, &context).await?)) } } - -fn make_send(mut stream: S) -> impl Stream + Send + Unpin + 'static -where - S: Stream + Unpin + 'static, - S::Item: Send, -{ - // NOTE: the 8 here is arbitrary - let (tx, rx) = tokio::sync::mpsc::channel(8); - - // NOTE: spawning stream into a new task can potentially hit this bug: - // - https://github.com/actix/actix-web/issues/1679 - // - // Since 4.0.0-beta.2 this issue is incredibly less frequent. I have not personally reproduced it. - // That said, it is still technically possible to encounter. - actix_web::rt::spawn(async move { - while let Some(res) = stream.next().await { - if tx.send(res).await.is_err() { - break; - } - } - }); - - SendStream { rx } -} - -struct SendStream { - rx: tokio::sync::mpsc::Receiver, -} - -impl Stream for SendStream -where - T: Send, -{ - type Item = T; - - fn poll_next( - mut self: std::pin::Pin<&mut Self>, - cx: &mut std::task::Context<'_>, - ) -> std::task::Poll> { - std::pin::Pin::new(&mut self.rx).poll_recv(cx) - } -} - -// TODO: remove these conversions after actix-web upgrades to http 1.0 -#[allow(clippy::expect_used)] -fn convert_status(status: http::StatusCode) -> StatusCode { - StatusCode::from_u16(status.as_u16()).expect("status can be converted") -} - -#[allow(clippy::expect_used)] -fn convert_method(method: &Method) -> http::Method { - http::Method::from_bytes(method.as_str().as_bytes()).expect("method can be converted") -} - -fn convert_header<'a>(name: &'a http::HeaderName, value: &'a HeaderValue) -> (&'a str, &'a [u8]) { - (name.as_str(), value.as_bytes()) -} diff --git a/crates/routes/src/images/utils.rs b/crates/routes/src/images/utils.rs new file mode 100644 index 0000000000..c951b4b014 --- /dev/null +++ b/crates/routes/src/images/utils.rs @@ -0,0 +1,63 @@ +use actix_web::http::{Method, StatusCode}; +use futures::stream::{Stream, StreamExt}; +use http::HeaderValue; + +pub(super) fn make_send(mut stream: S) -> impl Stream + Send + Unpin + 'static +where + S: Stream + Unpin + 'static, + S::Item: Send, +{ + // NOTE: the 8 here is arbitrary + let (tx, rx) = tokio::sync::mpsc::channel(8); + + // NOTE: spawning stream into a new task can potentially hit this bug: + // - https://github.com/actix/actix-web/issues/1679 + // + // Since 4.0.0-beta.2 this issue is incredibly less frequent. I have not personally reproduced it. + // That said, it is still technically possible to encounter. + actix_web::rt::spawn(async move { + while let Some(res) = stream.next().await { + if tx.send(res).await.is_err() { + break; + } + } + }); + + SendStream { rx } +} + +pub(super) struct SendStream { + rx: tokio::sync::mpsc::Receiver, +} + +impl Stream for SendStream +where + T: Send, +{ + type Item = T; + + fn poll_next( + mut self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + std::pin::Pin::new(&mut self.rx).poll_recv(cx) + } +} + +// TODO: remove these conversions after actix-web upgrades to http 1.0 +#[allow(clippy::expect_used)] +pub(super) fn convert_status(status: http::StatusCode) -> StatusCode { + StatusCode::from_u16(status.as_u16()).expect("status can be converted") +} + +#[allow(clippy::expect_used)] +pub(super) fn convert_method(method: &Method) -> http::Method { + http::Method::from_bytes(method.as_str().as_bytes()).expect("method can be converted") +} + +pub(super) fn convert_header<'a>( + name: &'a http::HeaderName, + value: &'a HeaderValue, +) -> (&'a str, &'a [u8]) { + (name.as_str(), value.as_bytes()) +} diff --git a/crates/utils/src/utils/markdown/image_links.rs b/crates/utils/src/utils/markdown/image_links.rs index 0990b1bc70..2185bdcad2 100644 --- a/crates/utils/src/utils/markdown/image_links.rs +++ b/crates/utils/src/utils/markdown/image_links.rs @@ -18,7 +18,7 @@ pub fn markdown_rewrite_image_links(mut src: String) -> (String, Vec) { // If link points to remote domain, replace with proxied link if parsed.domain() != Some(&SETTINGS.hostname) { let mut proxied = format!( - "{}/api/v4/image_proxy?url={}", + "{}/api/v4/image/proxy?url={}", SETTINGS.get_protocol_and_hostname(), encode(url), ); @@ -115,7 +115,7 @@ mod tests { ( "remote image proxied", "![link](http://example.com/image.jpg)", - "![link](https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)", + "![link](https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)", ), ( "local image unproxied", @@ -125,7 +125,7 @@ mod tests { ( "multiple image links", "![link](http://example.com/image1.jpg) ![link](http://example.com/image2.jpg)", - "![link](https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Fexample.com%2Fimage1.jpg) ![link](https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Fexample.com%2Fimage2.jpg)", + "![link](https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Fexample.com%2Fimage1.jpg) ![link](https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Fexample.com%2Fimage2.jpg)", ), ( "empty link handled", @@ -135,7 +135,7 @@ mod tests { ( "empty label handled", "![](http://example.com/image.jpg)", - "![](https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)" + "![](https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)" ), ( "invalid image link removed", @@ -145,12 +145,12 @@ mod tests { ( "label with nested markdown handled", "![a *b* c](http://example.com/image.jpg)", - "![a *b* c](https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)" + "![a *b* c](https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)" ), ( "custom emoji support", r#"![party-blob](https://www.hexbear.net/pictrs/image/83405746-0620-4728-9358-5f51b040ffee.gif "emoji party-blob")"#, - r#"![party-blob](https://lemmy-alpha/api/v4/image_proxy?url=https%3A%2F%2Fwww.hexbear.net%2Fpictrs%2Fimage%2F83405746-0620-4728-9358-5f51b040ffee.gif "emoji party-blob")"# + r#"![party-blob](https://lemmy-alpha/api/v4/image/proxy?url=https%3A%2F%2Fwww.hexbear.net%2Fpictrs%2Fimage%2F83405746-0620-4728-9358-5f51b040ffee.gif "emoji party-blob")"# ) ]; diff --git a/src/api_routes_v3.rs b/src/api_routes_v3.rs index 56dde6db95..452271dd1f 100644 --- a/src/api_routes_v3.rs +++ b/src/api_routes_v3.rs @@ -139,7 +139,7 @@ use lemmy_utils::rate_limit::RateLimitCell; // Deprecated, use api v4 instead. // When removing api v3, we also need to rewrite all links in database with -// `/api/v3/image_proxy` to use `/api/v4/image_proxy` instead. +// `/api/v3/image_proxy` to use `/api/v4/image/proxy` instead. pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { cfg .service( diff --git a/src/api_routes_v4.rs b/src/api_routes_v4.rs index ee88274c94..48f4031d4b 100644 --- a/src/api_routes_v4.rs +++ b/src/api_routes_v4.rs @@ -172,7 +172,6 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { cfg.service( scope("/api/v4") .wrap(rate_limit.message()) - .route("/image_proxy", get().to(image_proxy)) // Site .service( scope("/site") @@ -401,6 +400,7 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .wrap(rate_limit.image()) .route(post().to(upload_image)), ) + .route("/proxy", get().to(image_proxy)) .route("/{filename}", get().to(get_full_res_image)) .route("{token}/{filename}", delete().to(delete_image)) .route("/healthz", get().to(pictrs_healthz)), From cfa866a53427ac15d1f70fe10a8855f7321fd34f Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 13 Dec 2024 11:53:43 +0100 Subject: [PATCH 03/16] Upload avatar endpoint and other changes --- Cargo.lock | 1 + api_tests/run-federation-test.sh | 2 +- crates/api/src/local_user/save_settings.rs | 5 - crates/api_common/src/person.rs | 3 - crates/api_common/src/request.rs | 9 +- crates/routes/Cargo.toml | 1 + crates/routes/src/images/mod.rs | 219 ++++++--------------- crates/routes/src/images/person.rs | 36 ++++ crates/routes/src/images/utils.rs | 183 ++++++++++++++++- crates/utils/src/error.rs | 2 +- src/api_routes_v3.rs | 8 +- src/api_routes_v4.rs | 12 +- src/lib.rs | 12 +- 13 files changed, 301 insertions(+), 192 deletions(-) create mode 100644 crates/routes/src/images/person.rs diff --git a/Cargo.lock b/Cargo.lock index eebb1ce1a7..0b19fe6f6a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2773,6 +2773,7 @@ dependencies = [ "lemmy_utils", "reqwest 0.12.8", "reqwest-middleware", + "reqwest-tracing", "rss", "serde", "tokio", diff --git a/api_tests/run-federation-test.sh b/api_tests/run-federation-test.sh index 969a95b3e7..f9eab50395 100755 --- a/api_tests/run-federation-test.sh +++ b/api_tests/run-federation-test.sh @@ -11,7 +11,7 @@ killall -s1 lemmy_server || true popd pnpm i -pnpm api-test || true +pnpm api-test-image || true killall -s1 lemmy_server || true killall -s1 pict-rs || true diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs index 992fea1635..bcf1b02b6e 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -46,10 +46,6 @@ pub async fn save_user_settings( .as_deref(), ); - let avatar = diesel_url_update(data.avatar.as_deref())?; - replace_image(&avatar, &local_user_view.person.avatar, &context).await?; - let avatar = proxy_image_link_opt_api(avatar, &context).await?; - let banner = diesel_url_update(data.banner.as_deref())?; replace_image(&banner, &local_user_view.person.banner, &context).await?; let banner = proxy_image_link_opt_api(banner, &context).await?; @@ -108,7 +104,6 @@ pub async fn save_user_settings( bio, matrix_user_id, bot_account: data.bot_account, - avatar, banner, ..Default::default() }; diff --git a/crates/api_common/src/person.rs b/crates/api_common/src/person.rs index b95cf5e774..9c8a1d087b 100644 --- a/crates/api_common/src/person.rs +++ b/crates/api_common/src/person.rs @@ -114,9 +114,6 @@ pub struct SaveUserSettings { /// The language of the lemmy interface #[cfg_attr(feature = "full", ts(optional))] pub interface_language: Option, - /// A URL for your avatar. - #[cfg_attr(feature = "full", ts(optional))] - pub avatar: Option, /// A URL for your banner. #[cfg_attr(feature = "full", ts(optional))] pub banner: Option, diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index c6f86b8067..5b4d85a5e8 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -250,7 +250,8 @@ fn extract_opengraph_data(html_bytes: &[u8], url: &Url) -> LemmyResult>, + #[serde(default)] + pub files: Vec, pub msg: String, } @@ -388,9 +389,8 @@ async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> L .json::() .await?; - let files = res.files.unwrap_or_default(); - - let image = files + let image = res + .files .first() .ok_or(LemmyErrorType::PictrsResponseError(res.msg))?; @@ -467,6 +467,7 @@ async fn is_image_content_type(client: &ClientWithMiddleware, url: &Url) -> Lemm } /// When adding a new avatar, banner or similar image, delete the old one. +/// TODO: remove this function pub async fn replace_image( new_image: &Option>, old_image: &Option, diff --git a/crates/routes/Cargo.toml b/crates/routes/Cargo.toml index 91c3ed6830..3bd85d0e5f 100644 --- a/crates/routes/Cargo.toml +++ b/crates/routes/Cargo.toml @@ -33,4 +33,5 @@ url = { workspace = true } tracing = { workspace = true } tokio = { workspace = true } http.workspace = true +reqwest-tracing = { workspace = true } rss = "2.0.10" diff --git a/crates/routes/src/images/mod.rs b/crates/routes/src/images/mod.rs index 0f98b330e3..c5f6a9c2c5 100644 --- a/crates/routes/src/images/mod.rs +++ b/crates/routes/src/images/mod.rs @@ -1,112 +1,33 @@ use actix_web::{ body::{BodyStream, BoxBody}, - http::{ - header::{HeaderName, ACCEPT_ENCODING, HOST}, - StatusCode, - }, + http::StatusCode, web::*, HttpRequest, HttpResponse, Responder, }; -use lemmy_api_common::{context::LemmyContext, request::PictrsResponse}; +use lemmy_api_common::{context::LemmyContext, SuccessResponse}; use lemmy_db_schema::source::{ - images::{LocalImage, LocalImageForm, RemoteImage}, + images::{LocalImage, RemoteImage}, local_site::LocalSite, }; use lemmy_db_views::structs::LocalUserView; -use lemmy_utils::{error::LemmyResult, REQWEST_TIMEOUT}; -use reqwest::Body; -use reqwest_middleware::RequestBuilder; +use lemmy_utils::error::LemmyResult; use serde::Deserialize; -use std::time::Duration; use url::Url; -use utils::{convert_header, convert_method, convert_status, make_send}; +use utils::{ + adapt_request, + convert_header, + do_upload_image, + PictrsGetParams, + ProcessUrl, + UploadType, + PICTRS_CLIENT, +}; +pub mod person; mod utils; -trait ProcessUrl { - /// If thumbnail or format is given, this uses the pictrs process endpoint. - /// Otherwise, it uses the normal pictrs url (IE image/original). - fn process_url(&self, image_url: &str, pictrs_url: &Url) -> String; -} - -#[derive(Deserialize, Clone)] -pub struct PictrsGetParams { - format: Option, - thumbnail: Option, -} - -impl ProcessUrl for PictrsGetParams { - fn process_url(&self, src: &str, pictrs_url: &Url) -> String { - if self.format.is_none() && self.thumbnail.is_none() { - format!("{}image/original/{}", pictrs_url, src) - } else { - // Take file type from name, or jpg if nothing is given - let format = self - .clone() - .format - .unwrap_or_else(|| src.split('.').last().unwrap_or("jpg").to_string()); - - let mut url = format!("{}image/process.{}?src={}", pictrs_url, format, src); - - if let Some(size) = self.thumbnail { - url = format!("{url}&thumbnail={size}",); - } - url - } - } -} - -#[derive(Deserialize, Clone)] -pub struct ImageProxyParams { - url: String, - format: Option, - thumbnail: Option, -} - -impl ProcessUrl for ImageProxyParams { - fn process_url(&self, proxy_url: &str, pictrs_url: &Url) -> String { - if self.format.is_none() && self.thumbnail.is_none() { - format!("{}image/original?proxy={}", pictrs_url, proxy_url) - } else { - // Take file type from name, or jpg if nothing is given - let format = self - .clone() - .format - .unwrap_or_else(|| proxy_url.split('.').last().unwrap_or("jpg").to_string()); - - let mut url = format!("{}image/process.{}?proxy={}", pictrs_url, format, proxy_url); - - if let Some(size) = self.thumbnail { - url = format!("{url}&thumbnail={size}",); - } - url - } - } -} -fn adapt_request(request: &HttpRequest, context: &LemmyContext, url: String) -> RequestBuilder { - // remove accept-encoding header so that pictrs doesn't compress the response - const INVALID_HEADERS: &[HeaderName] = &[ACCEPT_ENCODING, HOST]; - - let client_request = context - .client() - .request(convert_method(request.method()), url) - .timeout(REQWEST_TIMEOUT); - - request - .headers() - .iter() - .fold(client_request, |client_req, (key, value)| { - if INVALID_HEADERS.contains(key) { - client_req - } else { - // TODO: remove as_str and as_bytes conversions after actix-web upgrades to http 1.0 - client_req.header(key.as_str(), value.as_bytes()) - } - }) -} - pub async fn upload_image( req: HttpRequest, body: Payload, @@ -114,40 +35,9 @@ pub async fn upload_image( local_user_view: LocalUserView, context: Data, ) -> LemmyResult { - let pictrs_config = context.settings().pictrs_config()?; - let image_url = format!("{}image", pictrs_config.url); - - let mut client_req = adapt_request(&req, &context, image_url); - - if let Some(addr) = req.head().peer_addr { - client_req = client_req.header("X-Forwarded-For", addr.to_string()) - }; - let res = client_req - .timeout(Duration::from_secs(pictrs_config.upload_timeout)) - .body(Body::wrap_stream(make_send(body))) - .send() - .await?; - - let status = res.status(); - let images = res.json::().await?; - if let Some(images) = &images.files { - for image in images { - let form = LocalImageForm { - local_user_id: Some(local_user_view.local_user.id), - pictrs_alias: image.file.to_string(), - pictrs_delete_token: image.delete_token.to_string(), - }; - - let protocol_and_hostname = context.settings().get_protocol_and_hostname(); - let thumbnail_url = image.thumbnail_url(&protocol_and_hostname)?; + let image = do_upload_image(req, body, UploadType::Other, &local_user_view, &context).await?; - // Also store the details for the image - let details_form = image.details.build_image_details_form(&thumbnail_url); - LocalImage::create(&mut context.pool(), &form, &details_form).await?; - } - } - - Ok(HttpResponse::build(convert_status(status)).json(images)) + Ok(HttpResponse::Ok().json(image)) } pub async fn get_full_res_image( @@ -169,11 +59,11 @@ pub async fn get_full_res_image( let processed_url = params.process_url(name, &pictrs_config.url); - image(processed_url, req, &context).await + image(processed_url, req).await } -async fn image(url: String, req: HttpRequest, context: &LemmyContext) -> LemmyResult { - let mut client_req = adapt_request(&req, context, url); +async fn image(url: String, req: HttpRequest) -> LemmyResult { + let mut client_req = adapt_request(&req, url); if let Some(addr) = req.head().peer_addr { client_req = client_req.header("X-Forwarded-For", addr.to_string()); @@ -198,47 +88,66 @@ async fn image(url: String, req: HttpRequest, context: &LemmyContext) -> LemmyRe Ok(client_res.body(BodyStream::new(res.bytes_stream()))) } +#[derive(Deserialize, Clone)] +pub struct DeleteImageParams { + file: String, + token: String, +} + pub async fn delete_image( - components: Path<(String, String)>, - req: HttpRequest, + data: Json, context: Data, // require login _local_user_view: LocalUserView, -) -> LemmyResult { - let (token, file) = components.into_inner(); - +) -> LemmyResult { let pictrs_config = context.settings().pictrs_config()?; - let url = format!("{}image/delete/{}/{}", pictrs_config.url, &token, &file); - - let mut client_req = adapt_request(&req, &context, url); - - if let Some(addr) = req.head().peer_addr { - client_req = client_req.header("X-Forwarded-For", addr.to_string()); - } + let url = format!( + "{}image/delete/{}/{}", + pictrs_config.url, &data.token, &data.file + ); - let res = client_req.send().await?; + PICTRS_CLIENT.delete(url).send().await?.error_for_status()?; - LocalImage::delete_by_alias(&mut context.pool(), &file).await?; + LocalImage::delete_by_alias(&mut context.pool(), &data.file).await?; - Ok(HttpResponse::build(convert_status(res.status())).body(BodyStream::new(res.bytes_stream()))) + Ok(SuccessResponse::default()) } -pub async fn pictrs_healthz( - req: HttpRequest, - context: Data, -) -> LemmyResult { +pub async fn pictrs_healthz(context: Data) -> LemmyResult { let pictrs_config = context.settings().pictrs_config()?; let url = format!("{}healthz", pictrs_config.url); - let mut client_req = adapt_request(&req, &context, url); + PICTRS_CLIENT.get(url).send().await?.error_for_status()?; - if let Some(addr) = req.head().peer_addr { - client_req = client_req.header("X-Forwarded-For", addr.to_string()); - } + Ok(SuccessResponse::default()) +} - let res = client_req.send().await?; +#[derive(Deserialize, Clone)] +pub struct ImageProxyParams { + url: String, + format: Option, + thumbnail: Option, +} - Ok(HttpResponse::build(convert_status(res.status())).body(BodyStream::new(res.bytes_stream()))) +impl ProcessUrl for ImageProxyParams { + fn process_url(&self, proxy_url: &str, pictrs_url: &Url) -> String { + if self.format.is_none() && self.thumbnail.is_none() { + format!("{}image/original?proxy={}", pictrs_url, proxy_url) + } else { + // Take file type from name, or jpg if nothing is given + let format = self + .clone() + .format + .unwrap_or_else(|| proxy_url.split('.').last().unwrap_or("jpg").to_string()); + + let mut url = format!("{}image/process.{}?proxy={}", pictrs_url, format, proxy_url); + + if let Some(size) = self.thumbnail { + url = format!("{url}&thumbnail={size}",); + } + url + } + } } pub async fn image_proxy( @@ -264,6 +173,6 @@ pub async fn image_proxy( Ok(Either::Left(Redirect::to(url.to_string()).respond_to(&req))) } else { // Proxy the image data through Lemmy - Ok(Either::Right(image(processed_url, req, &context).await?)) + Ok(Either::Right(image(processed_url, req).await?)) } } diff --git a/crates/routes/src/images/person.rs b/crates/routes/src/images/person.rs new file mode 100644 index 0000000000..edac1e41b2 --- /dev/null +++ b/crates/routes/src/images/person.rs @@ -0,0 +1,36 @@ +use super::utils::{delete_old_image, do_upload_image, UploadType}; +use actix_web::{self, web::*, HttpRequest}; +use lemmy_api_common::{context::LemmyContext, SuccessResponse}; +use lemmy_db_schema::{ + source::person::{Person, PersonUpdateForm}, + traits::Crud, +}; +use lemmy_db_views::structs::LocalUserView; +use lemmy_utils::error::LemmyResult; +use url::Url; + +pub async fn upload_avatar( + req: HttpRequest, + body: Payload, + local_user_view: LocalUserView, + context: Data, +) -> LemmyResult> { + let image = do_upload_image(req, body, UploadType::Avatar, &local_user_view, &context).await?; + + delete_old_image(&local_user_view.person.avatar, &context).await?; + + let avatar = format!( + "{}/api/v4/image/{}", + context.settings().get_protocol_and_hostname(), + image.file + ); + let avatar = Some(Some(Url::parse(&avatar)?.into())); + let person_form = PersonUpdateForm { + avatar, + ..Default::default() + }; + + Person::update(&mut context.pool(), local_user_view.person.id, &person_form).await?; + + Ok(Json(SuccessResponse::default())) +} diff --git a/crates/routes/src/images/utils.rs b/crates/routes/src/images/utils.rs index c951b4b014..28d144a99b 100644 --- a/crates/routes/src/images/utils.rs +++ b/crates/routes/src/images/utils.rs @@ -1,6 +1,97 @@ -use actix_web::http::{Method, StatusCode}; +use actix_web::{ + http::{ + header::{HeaderName, ACCEPT_ENCODING, HOST}, + Method, + StatusCode, + }, + web::{Data, Payload}, + HttpRequest, +}; use futures::stream::{Stream, StreamExt}; use http::HeaderValue; +use lemmy_api_common::{ + context::LemmyContext, + request::{client_builder, delete_image_from_pictrs, PictrsFile, PictrsResponse}, + LemmyErrorType, +}; +use lemmy_db_schema::{ + newtypes::DbUrl, + source::images::{LocalImage, LocalImageForm}, +}; +use lemmy_db_views::structs::LocalUserView; +use lemmy_utils::{error::LemmyResult, settings::SETTINGS, REQWEST_TIMEOUT}; +use reqwest::Body; +use reqwest_middleware::{ClientBuilder, ClientWithMiddleware, RequestBuilder}; +use reqwest_tracing::TracingMiddleware; +use serde::Deserialize; +use std::{sync::LazyLock, time::Duration}; +use url::Url; + +// Pictrs cannot use proxy +pub(super) static PICTRS_CLIENT: LazyLock = LazyLock::new(|| { + ClientBuilder::new( + client_builder(&SETTINGS) + .no_proxy() + .build() + .expect("build pictrs client"), + ) + .with(TracingMiddleware::default()) + .build() +}); + +#[derive(Deserialize, Clone)] +pub struct PictrsGetParams { + format: Option, + thumbnail: Option, +} + +pub(super) trait ProcessUrl { + /// If thumbnail or format is given, this uses the pictrs process endpoint. + /// Otherwise, it uses the normal pictrs url (IE image/original). + fn process_url(&self, image_url: &str, pictrs_url: &Url) -> String; +} + +impl ProcessUrl for PictrsGetParams { + fn process_url(&self, src: &str, pictrs_url: &Url) -> String { + if self.format.is_none() && self.thumbnail.is_none() { + format!("{}image/original/{}", pictrs_url, src) + } else { + // Take file type from name, or jpg if nothing is given + let format = self + .clone() + .format + .unwrap_or_else(|| src.split('.').last().unwrap_or("jpg").to_string()); + + let mut url = format!("{}image/process.{}?src={}", pictrs_url, format, src); + + if let Some(size) = self.thumbnail { + url = format!("{url}&thumbnail={size}",); + } + url + } + } +} + +pub(super) fn adapt_request(request: &HttpRequest, url: String) -> RequestBuilder { + // remove accept-encoding header so that pictrs doesn't compress the response + const INVALID_HEADERS: &[HeaderName] = &[ACCEPT_ENCODING, HOST]; + + let client_request = PICTRS_CLIENT + .request(convert_method(request.method()), url) + .timeout(REQWEST_TIMEOUT); + + request + .headers() + .iter() + .fold(client_request, |client_req, (key, value)| { + if INVALID_HEADERS.contains(key) { + client_req + } else { + // TODO: remove as_str and as_bytes conversions after actix-web upgrades to http 1.0 + client_req.header(key.as_str(), value.as_bytes()) + } + }) +} pub(super) fn make_send(mut stream: S) -> impl Stream + Send + Unpin + 'static where @@ -45,11 +136,6 @@ where } // TODO: remove these conversions after actix-web upgrades to http 1.0 -#[allow(clippy::expect_used)] -pub(super) fn convert_status(status: http::StatusCode) -> StatusCode { - StatusCode::from_u16(status.as_u16()).expect("status can be converted") -} - #[allow(clippy::expect_used)] pub(super) fn convert_method(method: &Method) -> http::Method { http::Method::from_bytes(method.as_str().as_bytes()).expect("method can be converted") @@ -61,3 +147,88 @@ pub(super) fn convert_header<'a>( ) -> (&'a str, &'a [u8]) { (name.as_str(), value.as_bytes()) } + +pub(super) enum UploadType { + Avatar, + Other, +} + +pub(super) async fn do_upload_image( + req: HttpRequest, + body: Payload, + upload_type: UploadType, + local_user_view: &LocalUserView, + context: &Data, +) -> LemmyResult { + let pictrs_config = context.settings().pictrs_config()?; + let image_url = format!("{}image", pictrs_config.url); + + let mut client_req = adapt_request(&req, image_url); + + client_req = match upload_type { + UploadType::Avatar => { + let max_size = context + .settings() + .pictrs_config()? + .max_thumbnail_size + .to_string(); + client_req.query(&[ + ("max_width", max_size.as_ref()), + ("max_height", max_size.as_ref()), + ("allow_animation", "false"), + ("allow_video", "false"), + ]) + } + _ => client_req, + }; + if let Some(addr) = req.head().peer_addr { + client_req = client_req.header("X-Forwarded-For", addr.to_string()) + }; + let res = client_req + .timeout(Duration::from_secs(pictrs_config.upload_timeout)) + .body(Body::wrap_stream(make_send(body))) + .send() + .await? + .error_for_status()?; + + let mut images = res.json::().await?; + for image in &images.files { + // Pictrs allows uploading multiple images in a single request. Lemmy doesnt need this, + // but still a user may upload multiple and so we need to store all links in db for + // to allow deletion via web ui. + let form = LocalImageForm { + local_user_id: Some(local_user_view.local_user.id), + pictrs_alias: image.file.to_string(), + pictrs_delete_token: image.delete_token.to_string(), + }; + + 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); + LocalImage::create(&mut context.pool(), &form, &details_form).await?; + } + let image = images + .files + .pop() + .ok_or(LemmyErrorType::InvalidImageUpload)?; + + Ok(image) +} + +/// When adding a new avatar, banner or similar image, delete the old one. +pub(super) async fn delete_old_image( + old_image: &Option, + context: &Data, +) -> LemmyResult<()> { + if let Some(old_image) = old_image { + let image = LocalImage::delete_by_url(&mut context.pool(), old_image) + .await + .ok(); + if let Some(image) = image { + delete_image_from_pictrs(&image.pictrs_alias, &image.pictrs_delete_token, context).await?; + } + } + Ok(()) +} diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index f45bc271f6..d2605608e2 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -23,7 +23,6 @@ pub enum LemmyErrorType { CouldntUpdateComment, CouldntUpdatePrivateMessage, CannotLeaveAdmin, - // TODO: also remove the translations of unused errors PictrsResponseError(String), PictrsPurgeResponseError(String), ImageUrlMissingPathSegments, @@ -31,6 +30,7 @@ pub enum LemmyErrorType { PictrsApiKeyNotProvided, NoContentTypeHeader, NotAnImageType, + InvalidImageUpload, NotAModOrAdmin, NotTopMod, NotLoggedIn, diff --git a/src/api_routes_v3.rs b/src/api_routes_v3.rs index 452271dd1f..56e3721d7c 100644 --- a/src/api_routes_v3.rs +++ b/src/api_routes_v3.rs @@ -134,7 +134,13 @@ use lemmy_apub::api::{ search::search, user_settings_backup::{export_settings, import_settings}, }; -use lemmy_routes::images::{delete_image, get_full_res_image, image_proxy, pictrs_healthz, upload_image}; +use lemmy_routes::images::{ + delete_image, + get_full_res_image, + image_proxy, + pictrs_healthz, + upload_image, +}; use lemmy_utils::rate_limit::RateLimitCell; // Deprecated, use api v4 instead. diff --git a/src/api_routes_v4.rs b/src/api_routes_v4.rs index 48f4031d4b..e67c6d7d4f 100644 --- a/src/api_routes_v4.rs +++ b/src/api_routes_v4.rs @@ -160,11 +160,7 @@ use lemmy_apub::api::{ user_settings_backup::{export_settings, import_settings}, }; use lemmy_routes::images::{ - delete_image, - get_full_res_image, - image_proxy, - pictrs_healthz, - upload_image, + delete_image, get_full_res_image, image_proxy, person::upload_avatar, pictrs_healthz, upload_image }; use lemmy_utils::rate_limit::RateLimitCell; @@ -293,7 +289,8 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .route("/change_password", put().to(change_password)) .route("/totp/generate", post().to(generate_totp_secret)) .route("/totp/update", post().to(update_totp)) - .route("/verify_email", post().to(verify_email)), + .route("/verify_email", post().to(verify_email)) + .route("/avatar", post().to(upload_avatar)), ) .route("/account/settings/save", put().to(save_user_settings)) .service( @@ -401,7 +398,8 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .route(post().to(upload_image)), ) .route("/proxy", get().to(image_proxy)) - .route("/{filename}", get().to(get_full_res_image)) + .route("/image/{filename}", get().to(get_full_res_image)) + // TODO: params are a bit strange like this .route("{token}/{filename}", delete().to(delete_image)) .route("/healthz", get().to(pictrs_healthz)), ), diff --git a/src/lib.rs b/src/lib.rs index 5586b61592..c287487755 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -285,17 +285,12 @@ fn create_http_server( .build() .map_err(|e| LemmyErrorType::Unknown(format!("Should always be buildable: {e}")))?; - let context: LemmyContext = federation_config.deref().clone(); - let rate_limit_cell = federation_config.rate_limit_cell().clone(); - - // Pictrs cannot use proxy - let pictrs_client = ClientBuilder::new(client_builder(&SETTINGS).no_proxy().build()?) - .with(TracingMiddleware::default()) - .build(); - // Create Http server let bind = (settings.bind, settings.port); let server = HttpServer::new(move || { + let context: LemmyContext = federation_config.deref().clone(); + let rate_limit_cell = federation_config.rate_limit_cell().clone(); + let cors_config = cors_config(&settings); let app = App::new() .wrap(middleware::Logger::new( @@ -328,7 +323,6 @@ fn create_http_server( } }) .configure(feeds::config) - .configure(|cfg| images::config(cfg, pictrs_client.clone(), &rate_limit_cell)) .configure(nodeinfo::config) }) .disable_signals() From d252be2113d465b09ab10622a860f4d9ab622b04 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 13 Dec 2024 15:04:07 +0100 Subject: [PATCH 04/16] Various other changes fixes #1772 fixes #4001 --- api_tests/package.json | 2 +- api_tests/pnpm-lock.yaml | 10 +- api_tests/run-federation-test.sh | 2 +- api_tests/src/image.spec.ts | 84 +++++++-------- config/defaults.hjson | 2 +- crates/api_common/src/image.rs | 42 ++++++++ crates/api_common/src/lib.rs | 1 + crates/api_common/src/request.rs | 12 ++- crates/api_common/src/utils.rs | 4 +- crates/routes/src/images/mod.rs | 142 +++++++++---------------- crates/routes/src/images/utils.rs | 78 +++++++------- crates/utils/src/settings/structs.rs | 2 +- crates/utils/src/utils/markdown/mod.rs | 10 +- src/api_routes_v3.rs | 12 +-- src/api_routes_v4.rs | 20 ++-- src/lib.rs | 2 +- 16 files changed, 208 insertions(+), 217 deletions(-) create mode 100644 crates/api_common/src/image.rs diff --git a/api_tests/package.json b/api_tests/package.json index 7ea21d0ba0..5ace77ecfe 100644 --- a/api_tests/package.json +++ b/api_tests/package.json @@ -28,7 +28,7 @@ "eslint": "^9.14.0", "eslint-plugin-prettier": "^5.1.3", "jest": "^29.5.0", - "lemmy-js-client": "0.20.0-api-v4.16", + "lemmy-js-client": "0.20.0-image-api-rework.3", "prettier": "^3.2.5", "ts-jest": "^29.1.0", "typescript": "^5.5.4", diff --git a/api_tests/pnpm-lock.yaml b/api_tests/pnpm-lock.yaml index 496606e6c2..615a0ceca2 100644 --- a/api_tests/pnpm-lock.yaml +++ b/api_tests/pnpm-lock.yaml @@ -30,8 +30,8 @@ importers: specifier: ^29.5.0 version: 29.7.0(@types/node@22.9.0) lemmy-js-client: - specifier: 0.20.0-api-v4.16 - version: 0.20.0-api-v4.16 + specifier: 0.20.0-image-api-rework.3 + version: 0.20.0-image-api-rework.3 prettier: specifier: ^3.2.5 version: 3.3.3 @@ -1167,8 +1167,8 @@ packages: resolution: {integrity: sha512-eTIzlVOSUR+JxdDFepEYcBMtZ9Qqdef+rnzWdRZuMbOywu5tO2w2N7rqjoANZ5k9vywhL6Br1VRjUIgTQx4E8w==} engines: {node: '>=6'} - lemmy-js-client@0.20.0-api-v4.16: - resolution: {integrity: sha512-9Wn7b8YT2KnEA286+RV1B3mLmecAynvAERoC0ZZiccfSgkEvd3rG9A5X9ejiPqp+JzDZJeisO57+Ut4QHr5oTw==} + lemmy-js-client@0.20.0-image-api-rework.3: + resolution: {integrity: sha512-SB20z+WD2S821q05OxzI2Lkwq1BWBNWM6Xd1l1bqKL310CRSAG4lln26+j8bjWxMgl/fYTqre8KG6l1YDiV3+Q==} leven@3.1.0: resolution: {integrity: sha512-qsda+H8jTaUaN/x5vzW2rzc+8Rw4TAQ/4KjB46IwK5VH+IlVeeeje/EoZRpiXvIqjFgK84QffqPztGI3VBLG1A==} @@ -3077,7 +3077,7 @@ snapshots: kleur@3.0.3: {} - lemmy-js-client@0.20.0-api-v4.16: {} + lemmy-js-client@0.20.0-image-api-rework.3: {} leven@3.1.0: {} diff --git a/api_tests/run-federation-test.sh b/api_tests/run-federation-test.sh index f9eab50395..969a95b3e7 100755 --- a/api_tests/run-federation-test.sh +++ b/api_tests/run-federation-test.sh @@ -11,7 +11,7 @@ killall -s1 lemmy_server || true popd pnpm i -pnpm api-test-image || true +pnpm api-test || true killall -s1 lemmy_server || true killall -s1 pict-rs || true diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index a3478081ad..fa22f271d1 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -2,9 +2,9 @@ jest.setTimeout(120000); import { UploadImage, - DeleteImage, PurgePerson, PurgePost, + DeleteImageParams, } from "lemmy-js-client"; import { alpha, @@ -54,13 +54,12 @@ test("Upload image and delete it", async () => { image: Buffer.from("test"), }; const upload = await alphaImage.uploadImage(upload_form); - expect(upload.files![0].file).toBeDefined(); - expect(upload.files![0].delete_token).toBeDefined(); - expect(upload.url).toBeDefined(); - expect(upload.delete_url).toBeDefined(); + expect(upload.image_url).toBeDefined(); + expect(upload.filename).toBeDefined(); + expect(upload.delete_token).toBeDefined(); // ensure that image download is working. theres probably a better way to do this - const response = await fetch(upload.url ?? ""); + const response = await fetch(upload.image_url ?? ""); const content = await response.text(); expect(content.length).toBeGreaterThan(0); @@ -77,26 +76,21 @@ test("Upload image and delete it", async () => { const previousThumbnails = 1; expect(listAllMediaRes.images.length).toBe(previousThumbnails); - // The deleteUrl is a combination of the endpoint, delete token, and alias - let firstImage = listMediaRes.images[0]; - let deleteUrl = `${alphaUrl}/pictrs/image/delete/${firstImage.local_image.pictrs_delete_token}/${firstImage.local_image.pictrs_alias}`; - expect(deleteUrl).toBe(upload.delete_url); - // Make sure the uploader is correct - expect(firstImage.person.actor_id).toBe( + expect(listMediaRes.images[0].person.actor_id).toBe( `http://lemmy-alpha:8541/u/lemmy_alpha`, ); // delete image - const delete_form: DeleteImage = { - token: upload.files![0].delete_token, - filename: upload.files![0].file, + const delete_form: DeleteImageParams = { + token: upload.delete_token, + filename: upload.filename, }; const delete_ = await alphaImage.deleteImage(delete_form); - expect(delete_).toBe(true); + expect(delete_.success).toBe(true); // ensure that image is deleted - const response2 = await fetch(upload.url ?? ""); + const response2 = await fetch(upload.image_url ?? ""); const content2 = await response2.text(); expect(content2).toBe(""); @@ -119,13 +113,12 @@ test("Purge user, uploaded image removed", async () => { image: Buffer.from("test"), }; const upload = await user.uploadImage(upload_form); - expect(upload.files![0].file).toBeDefined(); - expect(upload.files![0].delete_token).toBeDefined(); - expect(upload.url).toBeDefined(); - expect(upload.delete_url).toBeDefined(); + expect(upload.filename).toBeDefined(); + expect(upload.delete_token).toBeDefined(); + expect(upload.image_url).toBeDefined(); // ensure that image download is working. theres probably a better way to do this - const response = await fetch(upload.url ?? ""); + const response = await fetch(upload.image_url ?? ""); const content = await response.text(); expect(content.length).toBeGreaterThan(0); @@ -138,7 +131,7 @@ test("Purge user, uploaded image removed", async () => { expect(delete_.success).toBe(true); // ensure that image is deleted - const response2 = await fetch(upload.url ?? ""); + const response2 = await fetch(upload.image_url ?? ""); const content2 = await response2.text(); expect(content2).toBe(""); }); @@ -151,13 +144,12 @@ test("Purge post, linked image removed", async () => { image: Buffer.from("test"), }; const upload = await user.uploadImage(upload_form); - expect(upload.files![0].file).toBeDefined(); - expect(upload.files![0].delete_token).toBeDefined(); - expect(upload.url).toBeDefined(); - expect(upload.delete_url).toBeDefined(); + expect(upload.filename).toBeDefined(); + expect(upload.delete_token).toBeDefined(); + expect(upload.image_url).toBeDefined(); // ensure that image download is working. theres probably a better way to do this - const response = await fetch(upload.url ?? ""); + const response = await fetch(upload.image_url ?? ""); const content = await response.text(); expect(content.length).toBeGreaterThan(0); @@ -165,9 +157,9 @@ test("Purge post, linked image removed", async () => { let post = await createPost( user, community.community!.community.id, - upload.url, + upload.image_url, ); - expect(post.post_view.post.url).toBe(upload.url); + expect(post.post_view.post.url).toBe(upload.image_url); expect(post.post_view.image_details).toBeDefined(); // purge post @@ -178,7 +170,7 @@ test("Purge post, linked image removed", async () => { expect(delete_.success).toBe(true); // ensure that image is deleted - const response2 = await fetch(upload.url ?? ""); + const response2 = await fetch(upload.image_url ?? ""); const content2 = await response2.text(); expect(content2).toBe(""); }); @@ -200,11 +192,11 @@ test("Images in remote image post are proxied if setting enabled", async () => { // remote image gets proxied after upload expect( post.thumbnail_url?.startsWith( - "http://lemmy-gamma:8561/api/v4/image_proxy?url", + "http://lemmy-gamma:8561/api/v4/image/proxy?url", ), ).toBeTruthy(); expect( - post.body?.startsWith("![](http://lemmy-gamma:8561/api/v4/image_proxy?url"), + post.body?.startsWith("![](http://lemmy-gamma:8561/api/v4/image/proxy?url"), ).toBeTruthy(); // Make sure that it ends with jpg, to be sure its an image @@ -223,12 +215,12 @@ test("Images in remote image post are proxied if setting enabled", async () => { expect( epsilonPost.thumbnail_url?.startsWith( - "http://lemmy-epsilon:8581/api/v4/image_proxy?url", + "http://lemmy-epsilon:8581/api/v4/image/proxy?url", ), ).toBeTruthy(); expect( epsilonPost.body?.startsWith( - "![](http://lemmy-epsilon:8581/api/v4/image_proxy?url", + "![](http://lemmy-epsilon:8581/api/v4/image/proxy?url", ), ).toBeTruthy(); @@ -250,7 +242,7 @@ test("Thumbnail of remote image link is proxied if setting enabled", async () => // remote image gets proxied after upload expect( post.thumbnail_url?.startsWith( - "http://lemmy-gamma:8561/api/v4/image_proxy?url", + "http://lemmy-gamma:8561/api/v4/image/proxy?url", ), ).toBeTruthy(); @@ -268,7 +260,7 @@ test("Thumbnail of remote image link is proxied if setting enabled", async () => expect( epsilonPost.thumbnail_url?.startsWith( - "http://lemmy-epsilon:8581/api/v4/image_proxy?url", + "http://lemmy-epsilon:8581/api/v4/image/proxy?url", ), ).toBeTruthy(); @@ -292,14 +284,14 @@ test("No image proxying if setting is disabled", async () => { let post = await createPost( alpha, community.community_view.community.id, - upload.url, + upload.image_url, `![](${sampleImage})`, ); expect(post.post_view.post).toBeDefined(); // remote image doesn't get proxied after upload expect( - post.post_view.post.url?.startsWith("http://127.0.0.1:8551/pictrs/image/"), + post.post_view.post.url?.startsWith("http://lemmy-beta:8551/api/v4/image/"), ).toBeTruthy(); expect(post.post_view.post.body).toBe(`![](${sampleImage})`); @@ -312,7 +304,7 @@ test("No image proxying if setting is disabled", async () => { // remote image doesn't get proxied after federation expect( - betaPost.post.url?.startsWith("http://127.0.0.1:8551/pictrs/image/"), + betaPost.post.url?.startsWith("http://lemmy-beta:8551/api/v4/image/"), ).toBeTruthy(); expect(betaPost.post.body).toBe(`![](${sampleImage})`); // Make sure the alt text got federated @@ -334,7 +326,7 @@ test("Make regular post, and give it a custom thumbnail", async () => { alphaImage, community.community_view.community.id, wikipediaUrl, - upload1.url!, + upload1.image_url!, ); // Wait for the metadata to get fetched, since this is backgrounded now @@ -344,7 +336,7 @@ test("Make regular post, and give it a custom thumbnail", async () => { ); expect(post.post_view.post.url).toBe(wikipediaUrl); // Make sure it uses custom thumbnail - expect(post.post_view.post.thumbnail_url).toBe(upload1.url); + expect(post.post_view.post.thumbnail_url).toBe(upload1.image_url); }); test("Create an image post, and make sure a custom thumbnail doesn't overwrite it", async () => { @@ -363,14 +355,14 @@ test("Create an image post, and make sure a custom thumbnail doesn't overwrite i let post = await createPostWithThumbnail( alphaImage, community.community_view.community.id, - upload1.url!, - upload2.url!, + upload1.image_url!, + upload2.image_url!, ); post = await waitUntil( () => getPost(alphaImage, post.post_view.post.id), p => p.post_view.post.thumbnail_url != undefined, ); - expect(post.post_view.post.url).toBe(upload1.url); + expect(post.post_view.post.url).toBe(upload1.image_url); // Make sure the custom thumbnail is ignored - expect(post.post_view.post.thumbnail_url == upload2.url).toBe(false); + expect(post.post_view.post.thumbnail_url == upload2.image_url).toBe(false); }); diff --git a/config/defaults.hjson b/config/defaults.hjson index 9e24407cd0..14749e542e 100644 --- a/config/defaults.hjson +++ b/config/defaults.hjson @@ -44,7 +44,7 @@ # or # If enabled, all images from remote domains are rewritten to pass through - # `/api/v4/image_proxy`, including embedded images in markdown. Images are stored temporarily + # `/api/v4/image/proxy`, including embedded images in markdown. Images are stored temporarily # in pict-rs for caching. This improves privacy as users don't expose their IP to untrusted # servers, and decreases load on other servers. However it increases bandwidth use for the # local server. diff --git a/crates/api_common/src/image.rs b/crates/api_common/src/image.rs new file mode 100644 index 0000000000..df033f2858 --- /dev/null +++ b/crates/api_common/src/image.rs @@ -0,0 +1,42 @@ +use serde::{Deserialize, Serialize}; +use serde_with::skip_serializing_none; +#[cfg(feature = "full")] +use ts_rs::TS; +use url::Url; + +#[skip_serializing_none] +#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", ts(export))] +pub struct ImageGetParams { + pub file_type: Option, + pub max_size: Option, +} + +#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", ts(export))] +pub struct DeleteImageParams { + pub filename: String, + pub token: String, +} + +#[skip_serializing_none] +#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", ts(export))] +pub struct ImageProxyParams { + pub url: String, + pub file_type: Option, + pub max_size: Option, +} + +#[skip_serializing_none] +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", ts(export))] +pub struct UploadImageResponse { + pub image_url: Url, + pub filename: String, + pub delete_token: String, +} diff --git a/crates/api_common/src/lib.rs b/crates/api_common/src/lib.rs index 6e09d904d6..e02df464ad 100644 --- a/crates/api_common/src/lib.rs +++ b/crates/api_common/src/lib.rs @@ -7,6 +7,7 @@ pub mod community; #[cfg(feature = "full")] pub mod context; pub mod custom_emoji; +pub mod image; pub mod oauth_provider; pub mod person; pub mod post; diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index 5b4d85a5e8..e337cbdec9 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -263,9 +263,15 @@ pub struct PictrsFile { } impl PictrsFile { - pub fn thumbnail_url(&self, protocol_and_hostname: &str) -> Result { + pub fn image_url(&self, protocol_and_hostname: &str) -> Result { Url::parse(&format!( - "{protocol_and_hostname}/pictrs/image/{}", + "{protocol_and_hostname}/api/v4/image/{}", + self.file + )) + } + pub fn delete_url(&self, protocol_and_hostname: &str) -> Result { + Url::parse(&format!( + "{protocol_and_hostname}/api/v4/image/{}", self.file )) } @@ -402,7 +408,7 @@ async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> L pictrs_delete_token: image.delete_token.clone(), }; let protocol_and_hostname = context.settings().get_protocol_and_hostname(); - let thumbnail_url = image.thumbnail_url(&protocol_and_hostname)?; + let thumbnail_url = image.image_url(&protocol_and_hostname)?; // Also store the details for the image let details_form = image.details.build_image_details_form(&thumbnail_url); diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 21154c823c..6daeec4cd1 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -1177,7 +1177,7 @@ fn build_proxied_image_url( protocol_and_hostname: &str, ) -> Result { Url::parse(&format!( - "{}/api/v4/image_proxy?url={}", + "{}/api/v4/image/proxy?url={}", protocol_and_hostname, encode(link.as_str()) )) @@ -1256,7 +1256,7 @@ mod tests { ) .await?; assert_eq!( - "https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Flemmy-beta%2Fimage.png", + "https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Flemmy-beta%2Fimage.png", proxied.as_str() ); diff --git a/crates/routes/src/images/mod.rs b/crates/routes/src/images/mod.rs index c5f6a9c2c5..632aa30f20 100644 --- a/crates/routes/src/images/mod.rs +++ b/crates/routes/src/images/mod.rs @@ -1,29 +1,17 @@ -use actix_web::{ - body::{BodyStream, BoxBody}, - http::StatusCode, - web::*, - HttpRequest, - HttpResponse, - Responder, +use actix_web::{body::BoxBody, web::*, HttpRequest, HttpResponse, Responder}; +use lemmy_api_common::{ + context::LemmyContext, + image::{DeleteImageParams, ImageGetParams, ImageProxyParams, UploadImageResponse}, + SuccessResponse, }; -use lemmy_api_common::{context::LemmyContext, SuccessResponse}; use lemmy_db_schema::source::{ images::{LocalImage, RemoteImage}, local_site::LocalSite, }; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::LemmyResult; -use serde::Deserialize; use url::Url; -use utils::{ - adapt_request, - convert_header, - do_upload_image, - PictrsGetParams, - ProcessUrl, - UploadType, - PICTRS_CLIENT, -}; +use utils::{do_get_image, do_upload_image, file_type, UploadType, PICTRS_CLIENT}; pub mod person; mod utils; @@ -31,18 +19,22 @@ mod utils; pub async fn upload_image( req: HttpRequest, body: Payload, - // require login local_user_view: LocalUserView, context: Data, -) -> LemmyResult { +) -> LemmyResult> { let image = do_upload_image(req, body, UploadType::Other, &local_user_view, &context).await?; - Ok(HttpResponse::Ok().json(image)) + let image_url = image.image_url(&context.settings().get_protocol_and_hostname())?; + Ok(Json(UploadImageResponse { + image_url, + filename: image.file, + delete_token: image.delete_token, + })) } -pub async fn get_full_res_image( +pub async fn get_image( filename: Path, - Query(params): Query, + Query(params): Query, req: HttpRequest, context: Data, local_user_view: Option, @@ -55,43 +47,20 @@ pub async fn get_full_res_image( let name = &filename.into_inner(); // If there are no query params, the URL is original - let pictrs_config = context.settings().pictrs_config()?; - - let processed_url = params.process_url(name, &pictrs_config.url); - - image(processed_url, req).await -} - -async fn image(url: String, req: HttpRequest) -> LemmyResult { - let mut client_req = adapt_request(&req, url); - - if let Some(addr) = req.head().peer_addr { - client_req = client_req.header("X-Forwarded-For", addr.to_string()); - } - - if let Some(addr) = req.head().peer_addr { - client_req = client_req.header("X-Forwarded-For", addr.to_string()); - } - - let res = client_req.send().await?; - - if res.status() == http::StatusCode::NOT_FOUND { - return Ok(HttpResponse::NotFound().finish()); - } - - let mut client_res = HttpResponse::build(StatusCode::from_u16(res.status().as_u16())?); - - for (name, value) in res.headers().iter().filter(|(h, _)| *h != "connection") { - client_res.insert_header(convert_header(name, value)); - } + let pictrs_url = context.settings().pictrs_config()?.url; + let processed_url = if params.file_type.is_none() && params.max_size.is_none() { + format!("{}image/original/{}", pictrs_url, name) + } else { + let file_type = file_type(params.file_type, name); + let mut url = format!("{}image/process.{}?src={}", pictrs_url, file_type, name); - Ok(client_res.body(BodyStream::new(res.bytes_stream()))) -} + if let Some(size) = params.max_size { + url = format!("{url}&thumbnail={size}",); + } + url + }; -#[derive(Deserialize, Clone)] -pub struct DeleteImageParams { - file: String, - token: String, + do_get_image(processed_url, req).await } pub async fn delete_image( @@ -99,55 +68,27 @@ pub async fn delete_image( context: Data, // require login _local_user_view: LocalUserView, -) -> LemmyResult { +) -> LemmyResult> { let pictrs_config = context.settings().pictrs_config()?; let url = format!( "{}image/delete/{}/{}", - pictrs_config.url, &data.token, &data.file + pictrs_config.url, &data.token, &data.filename ); PICTRS_CLIENT.delete(url).send().await?.error_for_status()?; - LocalImage::delete_by_alias(&mut context.pool(), &data.file).await?; + LocalImage::delete_by_alias(&mut context.pool(), &data.filename).await?; - Ok(SuccessResponse::default()) + Ok(Json(SuccessResponse::default())) } -pub async fn pictrs_healthz(context: Data) -> LemmyResult { +pub async fn pictrs_health(context: Data) -> LemmyResult> { let pictrs_config = context.settings().pictrs_config()?; let url = format!("{}healthz", pictrs_config.url); PICTRS_CLIENT.get(url).send().await?.error_for_status()?; - Ok(SuccessResponse::default()) -} - -#[derive(Deserialize, Clone)] -pub struct ImageProxyParams { - url: String, - format: Option, - thumbnail: Option, -} - -impl ProcessUrl for ImageProxyParams { - fn process_url(&self, proxy_url: &str, pictrs_url: &Url) -> String { - if self.format.is_none() && self.thumbnail.is_none() { - format!("{}image/original?proxy={}", pictrs_url, proxy_url) - } else { - // Take file type from name, or jpg if nothing is given - let format = self - .clone() - .format - .unwrap_or_else(|| proxy_url.split('.').last().unwrap_or("jpg").to_string()); - - let mut url = format!("{}image/process.{}?proxy={}", pictrs_url, format, proxy_url); - - if let Some(size) = self.thumbnail { - url = format!("{url}&thumbnail={size}",); - } - url - } - } + Ok(Json(SuccessResponse::default())) } pub async fn image_proxy( @@ -162,7 +103,20 @@ pub async fn image_proxy( RemoteImage::validate(&mut context.pool(), url.clone().into()).await?; let pictrs_config = context.settings().pictrs_config()?; - let processed_url = params.process_url(¶ms.url, &pictrs_config.url); + let processed_url = if params.file_type.is_none() && params.max_size.is_none() { + format!("{}image/original?proxy={}", pictrs_config.url, params.url) + } else { + let file_type = file_type(params.file_type, url.as_str()); + let mut url = format!( + "{}image/process.{}?proxy={}", + pictrs_config.url, file_type, url + ); + + if let Some(size) = params.max_size { + url = format!("{url}&thumbnail={size}",); + } + url + }; let bypass_proxy = pictrs_config .proxy_bypass_domains @@ -173,6 +127,6 @@ pub async fn image_proxy( Ok(Either::Left(Redirect::to(url.to_string()).respond_to(&req))) } else { // Proxy the image data through Lemmy - Ok(Either::Right(image(processed_url, req).await?)) + Ok(Either::Right(do_get_image(processed_url, req).await?)) } } diff --git a/crates/routes/src/images/utils.rs b/crates/routes/src/images/utils.rs index 28d144a99b..94a807f787 100644 --- a/crates/routes/src/images/utils.rs +++ b/crates/routes/src/images/utils.rs @@ -1,4 +1,5 @@ use actix_web::{ + body::BodyStream, http::{ header::{HeaderName, ACCEPT_ENCODING, HOST}, Method, @@ -6,6 +7,7 @@ use actix_web::{ }, web::{Data, Payload}, HttpRequest, + HttpResponse, }; use futures::stream::{Stream, StreamExt}; use http::HeaderValue; @@ -23,7 +25,6 @@ use lemmy_utils::{error::LemmyResult, settings::SETTINGS, REQWEST_TIMEOUT}; use reqwest::Body; use reqwest_middleware::{ClientBuilder, ClientWithMiddleware, RequestBuilder}; use reqwest_tracing::TracingMiddleware; -use serde::Deserialize; use std::{sync::LazyLock, time::Duration}; use url::Url; @@ -39,40 +40,7 @@ pub(super) static PICTRS_CLIENT: LazyLock = LazyLock::new( .build() }); -#[derive(Deserialize, Clone)] -pub struct PictrsGetParams { - format: Option, - thumbnail: Option, -} - -pub(super) trait ProcessUrl { - /// If thumbnail or format is given, this uses the pictrs process endpoint. - /// Otherwise, it uses the normal pictrs url (IE image/original). - fn process_url(&self, image_url: &str, pictrs_url: &Url) -> String; -} - -impl ProcessUrl for PictrsGetParams { - fn process_url(&self, src: &str, pictrs_url: &Url) -> String { - if self.format.is_none() && self.thumbnail.is_none() { - format!("{}image/original/{}", pictrs_url, src) - } else { - // Take file type from name, or jpg if nothing is given - let format = self - .clone() - .format - .unwrap_or_else(|| src.split('.').last().unwrap_or("jpg").to_string()); - - let mut url = format!("{}image/process.{}?src={}", pictrs_url, format, src); - - if let Some(size) = self.thumbnail { - url = format!("{url}&thumbnail={size}",); - } - url - } - } -} - -pub(super) fn adapt_request(request: &HttpRequest, url: String) -> RequestBuilder { +fn adapt_request(request: &HttpRequest, url: String) -> RequestBuilder { // remove accept-encoding header so that pictrs doesn't compress the response const INVALID_HEADERS: &[HeaderName] = &[ACCEPT_ENCODING, HOST]; @@ -141,10 +109,7 @@ pub(super) fn convert_method(method: &Method) -> http::Method { http::Method::from_bytes(method.as_str().as_bytes()).expect("method can be converted") } -pub(super) fn convert_header<'a>( - name: &'a http::HeaderName, - value: &'a HeaderValue, -) -> (&'a str, &'a [u8]) { +fn convert_header<'a>(name: &'a http::HeaderName, value: &'a HeaderValue) -> (&'a str, &'a [u8]) { (name.as_str(), value.as_bytes()) } @@ -203,7 +168,7 @@ pub(super) async fn do_upload_image( }; let protocol_and_hostname = context.settings().get_protocol_and_hostname(); - let thumbnail_url = image.thumbnail_url(&protocol_and_hostname)?; + let thumbnail_url = image.image_url(&protocol_and_hostname)?; // Also store the details for the image let details_form = image.details.build_image_details_form(&thumbnail_url); @@ -217,6 +182,32 @@ pub(super) async fn do_upload_image( Ok(image) } +pub(super) async fn do_get_image(url: String, req: HttpRequest) -> LemmyResult { + let mut client_req = adapt_request(&req, url); + + if let Some(addr) = req.head().peer_addr { + client_req = client_req.header("X-Forwarded-For", addr.to_string()); + } + + if let Some(addr) = req.head().peer_addr { + client_req = client_req.header("X-Forwarded-For", addr.to_string()); + } + + let res = client_req.send().await?; + + if res.status() == http::StatusCode::NOT_FOUND { + return Ok(HttpResponse::NotFound().finish()); + } + + let mut client_res = HttpResponse::build(StatusCode::from_u16(res.status().as_u16())?); + + for (name, value) in res.headers().iter().filter(|(h, _)| *h != "connection") { + client_res.insert_header(convert_header(name, value)); + } + + Ok(client_res.body(BodyStream::new(res.bytes_stream()))) +} + /// When adding a new avatar, banner or similar image, delete the old one. pub(super) async fn delete_old_image( old_image: &Option, @@ -232,3 +223,10 @@ pub(super) async fn delete_old_image( } Ok(()) } + +/// Take file type from param, name, or use jpg if nothing is given +pub(super) fn file_type(file_type: Option, name: &str) -> String { + file_type + .clone() + .unwrap_or_else(|| name.split('.').last().unwrap_or("jpg").to_string()) +} diff --git a/crates/utils/src/settings/structs.rs b/crates/utils/src/settings/structs.rs index 1aef9f79ba..9e561713c5 100644 --- a/crates/utils/src/settings/structs.rs +++ b/crates/utils/src/settings/structs.rs @@ -120,7 +120,7 @@ pub enum PictrsImageMode { #[default] StoreLinkPreviews, /// If enabled, all images from remote domains are rewritten to pass through - /// `/api/v4/image_proxy`, including embedded images in markdown. Images are stored temporarily + /// `/api/v4/image/proxy`, including embedded images in markdown. Images are stored temporarily /// in pict-rs for caching. This improves privacy as users don't expose their IP to untrusted /// servers, and decreases load on other servers. However it increases bandwidth use for the /// local server. diff --git a/crates/utils/src/utils/markdown/mod.rs b/crates/utils/src/utils/markdown/mod.rs index ba509596ef..94dd322641 100644 --- a/crates/utils/src/utils/markdown/mod.rs +++ b/crates/utils/src/utils/markdown/mod.rs @@ -141,7 +141,7 @@ mod tests { ( "remote image proxied", "![link](http://example.com/image.jpg)", - "![link](https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)", + "![link](https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)", ), ( "local image unproxied", @@ -151,7 +151,7 @@ mod tests { ( "multiple image links", "![link](http://example.com/image1.jpg) ![link](http://example.com/image2.jpg)", - "![link](https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Fexample.com%2Fimage1.jpg) ![link](https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Fexample.com%2Fimage2.jpg)", + "![link](https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Fexample.com%2Fimage1.jpg) ![link](https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Fexample.com%2Fimage2.jpg)", ), ( "empty link handled", @@ -161,7 +161,7 @@ mod tests { ( "empty label handled", "![](http://example.com/image.jpg)", - "![](https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)" + "![](https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)" ), ( "invalid image link removed", @@ -171,12 +171,12 @@ mod tests { ( "label with nested markdown handled", "![a *b* c](http://example.com/image.jpg)", - "![a *b* c](https://lemmy-alpha/api/v4/image_proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)" + "![a *b* c](https://lemmy-alpha/api/v4/image/proxy?url=http%3A%2F%2Fexample.com%2Fimage.jpg)" ), ( "custom emoji support", r#"![party-blob](https://www.hexbear.net/pictrs/image/83405746-0620-4728-9358-5f51b040ffee.gif "emoji party-blob")"#, - r#"![party-blob](https://lemmy-alpha/api/v4/image_proxy?url=https%3A%2F%2Fwww.hexbear.net%2Fpictrs%2Fimage%2F83405746-0620-4728-9358-5f51b040ffee.gif "emoji party-blob")"# + r#"![party-blob](https://lemmy-alpha/api/v4/image/proxy?url=https%3A%2F%2Fwww.hexbear.net%2Fpictrs%2Fimage%2F83405746-0620-4728-9358-5f51b040ffee.gif "emoji party-blob")"# ) ]; diff --git a/src/api_routes_v3.rs b/src/api_routes_v3.rs index 56e3721d7c..e7cb828838 100644 --- a/src/api_routes_v3.rs +++ b/src/api_routes_v3.rs @@ -134,13 +134,7 @@ use lemmy_apub::api::{ search::search, user_settings_backup::{export_settings, import_settings}, }; -use lemmy_routes::images::{ - delete_image, - get_full_res_image, - image_proxy, - pictrs_healthz, - upload_image, -}; +use lemmy_routes::images::{delete_image, get_image, image_proxy, pictrs_health, upload_image}; use lemmy_utils::rate_limit::RateLimitCell; // Deprecated, use api v4 instead. @@ -153,9 +147,9 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .wrap(rate_limit.image()) .route(post().to(upload_image)), ) - .service(resource("/pictrs/image/{filename}").route(get().to(get_full_res_image))) + .service(resource("/pictrs/image/{filename}").route(get().to(get_image))) .service(resource("/pictrs/image/delete/{token}/{filename}").route(get().to(delete_image))) - .service(resource("/pictrs/healthz").route(get().to(pictrs_healthz))) + .service(resource("/pictrs/healthz").route(get().to(pictrs_health))) .service( scope("/api/v3") .route("/image_proxy", get().to(image_proxy)) diff --git a/src/api_routes_v4.rs b/src/api_routes_v4.rs index e67c6d7d4f..99ae86fbc6 100644 --- a/src/api_routes_v4.rs +++ b/src/api_routes_v4.rs @@ -160,7 +160,12 @@ use lemmy_apub::api::{ user_settings_backup::{export_settings, import_settings}, }; use lemmy_routes::images::{ - delete_image, get_full_res_image, image_proxy, person::upload_avatar, pictrs_healthz, upload_image + delete_image, + get_image, + image_proxy, + person::upload_avatar, + pictrs_health, + upload_image, }; use lemmy_utils::rate_limit::RateLimitCell; @@ -289,8 +294,7 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .route("/change_password", put().to(change_password)) .route("/totp/generate", post().to(generate_totp_secret)) .route("/totp/update", post().to(update_totp)) - .route("/verify_email", post().to(verify_email)) - .route("/avatar", post().to(upload_avatar)), + .route("/verify_email", post().to(verify_email)), ) .route("/account/settings/save", put().to(save_user_settings)) .service( @@ -318,6 +322,7 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .route("/unread_count", get().to(unread_count)) .route("/list_logins", get().to(list_logins)) .route("/validate_auth", get().to(validate_auth)) + .route("/avatar", post().to(upload_avatar)) .service( scope("/block") .route("/person", post().to(user_block_person)) @@ -395,13 +400,12 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .service( resource("") .wrap(rate_limit.image()) - .route(post().to(upload_image)), + .route(post().to(upload_image)) + .route(delete().to(delete_image)), ) .route("/proxy", get().to(image_proxy)) - .route("/image/{filename}", get().to(get_full_res_image)) - // TODO: params are a bit strange like this - .route("{token}/{filename}", delete().to(delete_image)) - .route("/healthz", get().to(pictrs_healthz)), + .route("/{filename}", get().to(get_image)) + .route("/health", get().to(pictrs_health)), ), ); } diff --git a/src/lib.rs b/src/lib.rs index c287487755..ae060ec75e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -36,7 +36,7 @@ use lemmy_apub::{ }; use lemmy_db_schema::{source::secret::Secret, utils::build_db_pool}; use lemmy_federate::{Opts, SendManager}; -use lemmy_routes::{feeds, images, nodeinfo, webfinger}; +use lemmy_routes::{feeds, nodeinfo, webfinger}; use lemmy_utils::{ error::{LemmyErrorType, LemmyResult}, rate_limit::RateLimitCell, From e8157789260a7ae65db83429c40c9442a720e551 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 13 Dec 2024 15:13:27 +0100 Subject: [PATCH 05/16] clippy --- crates/routes/src/images/utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/routes/src/images/utils.rs b/crates/routes/src/images/utils.rs index 94a807f787..dcc4188272 100644 --- a/crates/routes/src/images/utils.rs +++ b/crates/routes/src/images/utils.rs @@ -26,9 +26,9 @@ use reqwest::Body; use reqwest_middleware::{ClientBuilder, ClientWithMiddleware, RequestBuilder}; use reqwest_tracing::TracingMiddleware; use std::{sync::LazyLock, time::Duration}; -use url::Url; // Pictrs cannot use proxy +#[allow(clippy::expect_used)] pub(super) static PICTRS_CLIENT: LazyLock = LazyLock::new(|| { ClientBuilder::new( client_builder(&SETTINGS) From 60ba7af2a15248a3bc595d186ca4d7a5568ecd80 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 13 Dec 2024 15:23:05 +0100 Subject: [PATCH 06/16] config options --- config/defaults.hjson | 4 ++++ crates/routes/src/images/utils.rs | 11 ++++++----- crates/utils/src/settings/structs.rs | 12 ++++++++++++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/config/defaults.hjson b/config/defaults.hjson index 14749e542e..c1cb8d06b4 100644 --- a/config/defaults.hjson +++ b/config/defaults.hjson @@ -64,6 +64,10 @@ upload_timeout: 30 # Resize post thumbnails to this maximum width/height. max_thumbnail_size: 512 + # Maximum size for user avatar, community icon and site icon. + max_avatar_size: 512 + # Maximum size for user, community and site banner. + max_banner_size: 512 } # Email sending configuration. All options except login/password are mandatory email: { diff --git a/crates/routes/src/images/utils.rs b/crates/routes/src/images/utils.rs index dcc4188272..e4b2432014 100644 --- a/crates/routes/src/images/utils.rs +++ b/crates/routes/src/images/utils.rs @@ -61,7 +61,7 @@ fn adapt_request(request: &HttpRequest, url: String) -> RequestBuilder { }) } -pub(super) fn make_send(mut stream: S) -> impl Stream + Send + Unpin + 'static +fn make_send(mut stream: S) -> impl Stream + Send + Unpin + 'static where S: Stream + Unpin + 'static, S::Item: Send, @@ -85,7 +85,7 @@ where SendStream { rx } } -pub(super) struct SendStream { +struct SendStream { rx: tokio::sync::mpsc::Receiver, } @@ -135,15 +135,16 @@ pub(super) async fn do_upload_image( let max_size = context .settings() .pictrs_config()? - .max_thumbnail_size + .max_avatar_size .to_string(); client_req.query(&[ - ("max_width", max_size.as_ref()), - ("max_height", max_size.as_ref()), + ("resize", max_size.as_ref()), ("allow_animation", "false"), ("allow_video", "false"), ]) } + // TODO: same as above but using `max_banner_size` + // UploadType::Banner => {} _ => client_req, }; if let Some(addr) = req.head().peer_addr { diff --git a/crates/utils/src/settings/structs.rs b/crates/utils/src/settings/structs.rs index 9e561713c5..3048843936 100644 --- a/crates/utils/src/settings/structs.rs +++ b/crates/utils/src/settings/structs.rs @@ -104,6 +104,18 @@ pub struct PictrsConfig { /// Resize post thumbnails to this maximum width/height. #[default(512)] pub max_thumbnail_size: u32, + + /// Maximum size for user avatar, community icon and site icon. + #[default(512)] + pub max_avatar_size: u32, + + /// Maximum size for user, community and site banner. + /// + /// TODO: Unfortunately pictrs can only resize images to fit in a*a square, no rectangle. + /// Otherwise we have to use crop, or use max_width/max_height which throws error + /// if image is larger. + #[default(512)] + pub max_banner_size: u32, } #[derive(Debug, Deserialize, Serialize, Clone, SmartDefault, Document, PartialEq)] From 7c771d2113581449c0166ac5608ea6be56bafd40 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 13 Dec 2024 15:28:16 +0100 Subject: [PATCH 07/16] fix ts bindings --- config/defaults.hjson | 4 ++++ crates/api_common/src/image.rs | 4 ++++ crates/utils/src/settings/structs.rs | 4 ++-- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/config/defaults.hjson b/config/defaults.hjson index c1cb8d06b4..aaf2a94d84 100644 --- a/config/defaults.hjson +++ b/config/defaults.hjson @@ -67,6 +67,10 @@ # Maximum size for user avatar, community icon and site icon. max_avatar_size: 512 # Maximum size for user, community and site banner. + # + # TODO: Unfortunately pictrs can only resize images to fit in a*a square, no rectangle. + # Otherwise we have to use crop, or use max_width/max_height which throws error + # if image is larger. max_banner_size: 512 } # Email sending configuration. All options except login/password are mandatory diff --git a/crates/api_common/src/image.rs b/crates/api_common/src/image.rs index df033f2858..c28832921a 100644 --- a/crates/api_common/src/image.rs +++ b/crates/api_common/src/image.rs @@ -9,7 +9,9 @@ use url::Url; #[cfg_attr(feature = "full", derive(TS))] #[cfg_attr(feature = "full", ts(export))] pub struct ImageGetParams { + #[cfg_attr(feature = "full", ts(optional))] pub file_type: Option, + #[cfg_attr(feature = "full", ts(optional))] pub max_size: Option, } @@ -27,7 +29,9 @@ pub struct DeleteImageParams { #[cfg_attr(feature = "full", ts(export))] pub struct ImageProxyParams { pub url: String, + #[cfg_attr(feature = "full", ts(optional))] pub file_type: Option, + #[cfg_attr(feature = "full", ts(optional))] pub max_size: Option, } diff --git a/crates/utils/src/settings/structs.rs b/crates/utils/src/settings/structs.rs index 3048843936..28710e8ca9 100644 --- a/crates/utils/src/settings/structs.rs +++ b/crates/utils/src/settings/structs.rs @@ -110,8 +110,8 @@ pub struct PictrsConfig { pub max_avatar_size: u32, /// Maximum size for user, community and site banner. - /// - /// TODO: Unfortunately pictrs can only resize images to fit in a*a square, no rectangle. + /// + /// TODO: Unfortunately pictrs can only resize images to fit in a*a square, no rectangle. /// Otherwise we have to use crop, or use max_width/max_height which throws error /// if image is larger. #[default(512)] From 8ae4b405d0648465b378d64471073c8e35e74fcd Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 17 Dec 2024 11:29:43 +0100 Subject: [PATCH 08/16] fix api tests --- api_tests/package.json | 2 +- api_tests/pnpm-lock.yaml | 10 +++++----- api_tests/src/community.spec.ts | 3 +-- api_tests/src/follow.spec.ts | 1 - api_tests/src/image.spec.ts | 1 - api_tests/src/post.spec.ts | 1 - api_tests/src/shared.ts | 11 +++++------ api_tests/src/user.spec.ts | 25 +++---------------------- 8 files changed, 15 insertions(+), 39 deletions(-) diff --git a/api_tests/package.json b/api_tests/package.json index 5ace77ecfe..f4c6dd2850 100644 --- a/api_tests/package.json +++ b/api_tests/package.json @@ -28,7 +28,7 @@ "eslint": "^9.14.0", "eslint-plugin-prettier": "^5.1.3", "jest": "^29.5.0", - "lemmy-js-client": "0.20.0-image-api-rework.3", + "lemmy-js-client": "0.20.0-image-api-rework.5", "prettier": "^3.2.5", "ts-jest": "^29.1.0", "typescript": "^5.5.4", diff --git a/api_tests/pnpm-lock.yaml b/api_tests/pnpm-lock.yaml index 615a0ceca2..509b5bb5e3 100644 --- a/api_tests/pnpm-lock.yaml +++ b/api_tests/pnpm-lock.yaml @@ -30,8 +30,8 @@ importers: specifier: ^29.5.0 version: 29.7.0(@types/node@22.9.0) lemmy-js-client: - specifier: 0.20.0-image-api-rework.3 - version: 0.20.0-image-api-rework.3 + specifier: 0.20.0-image-api-rework.5 + version: 0.20.0-image-api-rework.5 prettier: specifier: ^3.2.5 version: 3.3.3 @@ -1167,8 +1167,8 @@ packages: resolution: {integrity: sha512-eTIzlVOSUR+JxdDFepEYcBMtZ9Qqdef+rnzWdRZuMbOywu5tO2w2N7rqjoANZ5k9vywhL6Br1VRjUIgTQx4E8w==} engines: {node: '>=6'} - lemmy-js-client@0.20.0-image-api-rework.3: - resolution: {integrity: sha512-SB20z+WD2S821q05OxzI2Lkwq1BWBNWM6Xd1l1bqKL310CRSAG4lln26+j8bjWxMgl/fYTqre8KG6l1YDiV3+Q==} + lemmy-js-client@0.20.0-image-api-rework.5: + resolution: {integrity: sha512-fMbgLefWy7B1QtX6c7Y/CMAbV70tKpT5eDKsdnCtKhV1eB73pATKNPaFtrMuAYu9h13M6LDlry6mX7CeWuJb6Q==} leven@3.1.0: resolution: {integrity: sha512-qsda+H8jTaUaN/x5vzW2rzc+8Rw4TAQ/4KjB46IwK5VH+IlVeeeje/EoZRpiXvIqjFgK84QffqPztGI3VBLG1A==} @@ -3077,7 +3077,7 @@ snapshots: kleur@3.0.3: {} - lemmy-js-client@0.20.0-image-api-rework.3: {} + lemmy-js-client@0.20.0-image-api-rework.5: {} leven@3.1.0: {} diff --git a/api_tests/src/community.spec.ts b/api_tests/src/community.spec.ts index 2bb0920881..a0b9d41d25 100644 --- a/api_tests/src/community.spec.ts +++ b/api_tests/src/community.spec.ts @@ -16,7 +16,6 @@ import { followCommunity, banPersonFromCommunity, resolvePerson, - getSite, createPost, getPost, resolvePost, @@ -36,7 +35,7 @@ import { userBlockInstance, } from "./shared"; import { AdminAllowInstanceParams } from "lemmy-js-client/dist/types/AdminAllowInstanceParams"; -import { EditCommunity, EditSite, GetPosts } from "lemmy-js-client"; +import { EditCommunity, GetPosts } from "lemmy-js-client"; beforeAll(setupLogins); afterAll(unfollows); diff --git a/api_tests/src/follow.spec.ts b/api_tests/src/follow.spec.ts index 936ce26065..c447e14cd7 100644 --- a/api_tests/src/follow.spec.ts +++ b/api_tests/src/follow.spec.ts @@ -5,7 +5,6 @@ import { setupLogins, resolveBetaCommunity, followCommunity, - getSite, waitUntil, beta, betaUrl, diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index fa22f271d1..d25ab7f4c3 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -18,7 +18,6 @@ import { epsilon, followCommunity, gamma, - getSite, imageFetchLimit, registerUser, resolveBetaCommunity, diff --git a/api_tests/src/post.spec.ts b/api_tests/src/post.spec.ts index 4158bbdc7b..c7193c573b 100644 --- a/api_tests/src/post.spec.ts +++ b/api_tests/src/post.spec.ts @@ -30,7 +30,6 @@ import { listPostReports, randomString, registerUser, - getSite, unfollows, resolveCommunity, waitUntil, diff --git a/api_tests/src/shared.ts b/api_tests/src/shared.ts index 1ed13d9cff..79614cb704 100644 --- a/api_tests/src/shared.ts +++ b/api_tests/src/shared.ts @@ -1,12 +1,11 @@ import { - AdminBlockInstanceParams, ApproveCommunityPendingFollower, BlockCommunity, BlockCommunityResponse, CommunityId, CommunityVisibility, CreatePrivateMessageReport, - DeleteImage, + DeleteImageParams, EditCommunity, GetCommunityPendingFollowsCountResponse, GetReplies, @@ -210,7 +209,9 @@ async function allowInstance(api: LemmyHttp, instance: string) { // Ignore errors from duplicate allows (because setup gets called for each test file) try { await api.adminAllowInstance(params); - } catch {} + } catch { + //console.log("Failed to allow instance"); + } } export async function createPost( @@ -715,7 +716,6 @@ export async function saveUserSettingsBio( export async function saveUserSettingsFederated( api: LemmyHttp, ): Promise { - let avatar = sampleImage; let banner = sampleImage; let bio = "a changed bio"; let form: SaveUserSettings = { @@ -724,7 +724,6 @@ export async function saveUserSettingsFederated( default_post_sort_type: "Hot", default_listing_type: "All", interface_language: "", - avatar, banner, display_name: "user321", show_avatars: false, @@ -947,7 +946,7 @@ export async function deleteAllImages(api: LemmyHttp) { Promise.all( imagesRes.images .map(image => { - const form: DeleteImage = { + const form: DeleteImageParams = { token: image.local_image.pictrs_delete_token, filename: image.local_image.pictrs_alias, }; diff --git a/api_tests/src/user.spec.ts b/api_tests/src/user.spec.ts index f7f80aecb2..7a25e61403 100644 --- a/api_tests/src/user.spec.ts +++ b/api_tests/src/user.spec.ts @@ -21,7 +21,6 @@ import { fetchFunction, alphaImage, unfollows, - saveUserSettingsBio, getMyUser, getPersonDetails, } from "./shared"; @@ -182,42 +181,24 @@ test("Set a new avatar, old avatar is deleted", async () => { const upload_form1: UploadImage = { image: Buffer.from("test1"), }; - const upload1 = await alphaImage.uploadImage(upload_form1); - expect(upload1.url).toBeDefined(); - - let form1 = { - avatar: upload1.url, - }; - await saveUserSettings(alpha, form1); + await alpha.userUploadAvatar(upload_form1); const listMediaRes1 = await alphaImage.listMedia(); expect(listMediaRes1.images.length).toBe(1); const upload_form2: UploadImage = { image: Buffer.from("test2"), }; - const upload2 = await alphaImage.uploadImage(upload_form2); - expect(upload2.url).toBeDefined(); - - let form2 = { - avatar: upload2.url, - }; - await saveUserSettings(alpha, form2); + await alpha.userUploadAvatar(upload_form2); // make sure only the new avatar is kept const listMediaRes2 = await alphaImage.listMedia(); expect(listMediaRes2.images.length).toBe(1); // Upload that same form2 avatar, make sure it isn't replaced / deleted - await saveUserSettings(alpha, form2); + await alpha.userUploadAvatar(upload_form2); // make sure only the new avatar is kept const listMediaRes3 = await alphaImage.listMedia(); expect(listMediaRes3.images.length).toBe(1); - // Now try to save a user settings, with the icon missing, - // and make sure it doesn't clear the data, or delete the image - await saveUserSettingsBio(alpha); - let my_user = await getMyUser(alpha); - expect(my_user.local_user_view.person.avatar).toBe(upload2.url); - // make sure only the new avatar is kept const listMediaRes4 = await alphaImage.listMedia(); expect(listMediaRes4.images.length).toBe(1); From b0d4bdb8ff0d9e4bb9e7c3f4132a264259e7b470 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 17 Dec 2024 16:10:55 +0100 Subject: [PATCH 09/16] Add option to disable image upload (fixes #1118) --- crates/api_common/src/request.rs | 10 +++++----- crates/api_common/src/site.rs | 3 +++ crates/api_common/src/utils.rs | 4 ++-- crates/api_crud/src/site/read.rs | 1 + crates/routes/src/images/mod.rs | 13 +++++++++---- crates/routes/src/images/utils.rs | 4 ++-- crates/utils/src/error.rs | 1 + crates/utils/src/settings/mod.rs | 2 +- crates/utils/src/settings/structs.rs | 5 +++++ 9 files changed, 29 insertions(+), 14 deletions(-) diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index e337cbdec9..a03e6598f3 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -320,7 +320,7 @@ pub async fn purge_image_from_pictrs(image_url: &Url, context: &LemmyContext) -> .next_back() .ok_or(LemmyErrorType::ImageUrlMissingLastPathSegment)?; - let pictrs_config = context.settings().pictrs_config()?; + let pictrs_config = context.settings().pictrs()?; let purge_url = format!("{}internal/purge?alias={}", pictrs_config.url, alias); let pictrs_api_key = pictrs_config @@ -348,7 +348,7 @@ pub async fn delete_image_from_pictrs( delete_token: &str, context: &LemmyContext, ) -> LemmyResult<()> { - let pictrs_config = context.settings().pictrs_config()?; + let pictrs_config = context.settings().pictrs()?; let url = format!( "{}image/delete/{}/{}", pictrs_config.url, &delete_token, &alias @@ -366,7 +366,7 @@ pub async fn delete_image_from_pictrs( /// Retrieves the image with local pict-rs and generates a thumbnail. Returns the thumbnail url. #[tracing::instrument(skip_all)] async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> LemmyResult { - let pictrs_config = context.settings().pictrs_config()?; + let pictrs_config = context.settings().pictrs()?; match pictrs_config.image_mode() { PictrsImageMode::None => return Ok(image_url.clone()), @@ -382,7 +382,7 @@ async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> L "{}image/download?url={}&resize={}", pictrs_config.url, encode(image_url.as_str()), - context.settings().pictrs_config()?.max_thumbnail_size + context.settings().pictrs()?.max_thumbnail_size ); let res = context @@ -425,7 +425,7 @@ pub async fn fetch_pictrs_proxied_image_details( image_url: &Url, context: &LemmyContext, ) -> LemmyResult { - let pictrs_url = context.settings().pictrs_config()?.url; + let pictrs_url = context.settings().pictrs()?.url; let encoded_image_url = encode(image_url.as_str()); // Pictrs needs you to fetch the proxied image before you can fetch the details diff --git a/crates/api_common/src/site.rs b/crates/api_common/src/site.rs index 7f1000b14e..0b1fb2200e 100644 --- a/crates/api_common/src/site.rs +++ b/crates/api_common/src/site.rs @@ -455,6 +455,9 @@ pub struct GetSiteResponse { #[cfg_attr(feature = "full", ts(optional))] pub admin_oauth_providers: Option>, pub blocked_urls: Vec, + // If true then uploads for post images or markdown images are disabled. Only avatars, icons and + // banners can be set. + pub image_upload_disabled: bool, } #[skip_serializing_none] diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 6daeec4cd1..3daa341070 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -1060,7 +1060,7 @@ pub async fn process_markdown( markdown_check_for_blocked_urls(&text, url_blocklist)?; - if context.settings().pictrs_config()?.image_mode() == PictrsImageMode::ProxyAllImages { + if context.settings().pictrs()?.image_mode() == PictrsImageMode::ProxyAllImages { let (text, links) = markdown_rewrite_image_links(text); RemoteImage::create(&mut context.pool(), links.clone()).await?; @@ -1130,7 +1130,7 @@ async fn proxy_image_link_internal( pub async fn proxy_image_link(link: Url, context: &LemmyContext) -> LemmyResult { proxy_image_link_internal( link, - context.settings().pictrs_config()?.image_mode(), + context.settings().pictrs()?.image_mode(), context, ) .await diff --git a/crates/api_crud/src/site/read.rs b/crates/api_crud/src/site/read.rs index 220fe1bd53..b618c0fa2a 100644 --- a/crates/api_crud/src/site/read.rs +++ b/crates/api_crud/src/site/read.rs @@ -69,5 +69,6 @@ async fn read_site(context: &LemmyContext) -> LemmyResult { tagline, oauth_providers: Some(oauth_providers), admin_oauth_providers: Some(admin_oauth_providers), + image_upload_disabled: context.settings().pictrs()?.disable_image_upload, }) } diff --git a/crates/routes/src/images/mod.rs b/crates/routes/src/images/mod.rs index 632aa30f20..8e5a547345 100644 --- a/crates/routes/src/images/mod.rs +++ b/crates/routes/src/images/mod.rs @@ -2,6 +2,7 @@ use actix_web::{body::BoxBody, web::*, HttpRequest, HttpResponse, Responder}; use lemmy_api_common::{ context::LemmyContext, image::{DeleteImageParams, ImageGetParams, ImageProxyParams, UploadImageResponse}, + LemmyErrorType, SuccessResponse, }; use lemmy_db_schema::source::{ @@ -22,6 +23,10 @@ pub async fn upload_image( local_user_view: LocalUserView, context: Data, ) -> LemmyResult> { + if context.settings().pictrs()?.disable_image_upload { + return Err(LemmyErrorType::ImageUploadDisabled.into()); + } + let image = do_upload_image(req, body, UploadType::Other, &local_user_view, &context).await?; let image_url = image.image_url(&context.settings().get_protocol_and_hostname())?; @@ -47,7 +52,7 @@ pub async fn get_image( let name = &filename.into_inner(); // If there are no query params, the URL is original - let pictrs_url = context.settings().pictrs_config()?.url; + let pictrs_url = context.settings().pictrs()?.url; let processed_url = if params.file_type.is_none() && params.max_size.is_none() { format!("{}image/original/{}", pictrs_url, name) } else { @@ -69,7 +74,7 @@ pub async fn delete_image( // require login _local_user_view: LocalUserView, ) -> LemmyResult> { - let pictrs_config = context.settings().pictrs_config()?; + let pictrs_config = context.settings().pictrs()?; let url = format!( "{}image/delete/{}/{}", pictrs_config.url, &data.token, &data.filename @@ -83,7 +88,7 @@ pub async fn delete_image( } pub async fn pictrs_health(context: Data) -> LemmyResult> { - let pictrs_config = context.settings().pictrs_config()?; + let pictrs_config = context.settings().pictrs()?; let url = format!("{}healthz", pictrs_config.url); PICTRS_CLIENT.get(url).send().await?.error_for_status()?; @@ -102,7 +107,7 @@ pub async fn image_proxy( // for arbitrary purposes. RemoteImage::validate(&mut context.pool(), url.clone().into()).await?; - let pictrs_config = context.settings().pictrs_config()?; + let pictrs_config = context.settings().pictrs()?; let processed_url = if params.file_type.is_none() && params.max_size.is_none() { format!("{}image/original?proxy={}", pictrs_config.url, params.url) } else { diff --git a/crates/routes/src/images/utils.rs b/crates/routes/src/images/utils.rs index e4b2432014..50e0b8fd40 100644 --- a/crates/routes/src/images/utils.rs +++ b/crates/routes/src/images/utils.rs @@ -125,7 +125,7 @@ pub(super) async fn do_upload_image( local_user_view: &LocalUserView, context: &Data, ) -> LemmyResult { - let pictrs_config = context.settings().pictrs_config()?; + let pictrs_config = context.settings().pictrs()?; let image_url = format!("{}image", pictrs_config.url); let mut client_req = adapt_request(&req, image_url); @@ -134,7 +134,7 @@ pub(super) async fn do_upload_image( UploadType::Avatar => { let max_size = context .settings() - .pictrs_config()? + .pictrs()? .max_avatar_size .to_string(); client_req.query(&[ diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index d2605608e2..d2a40b4ec5 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -31,6 +31,7 @@ pub enum LemmyErrorType { NoContentTypeHeader, NotAnImageType, InvalidImageUpload, + ImageUploadDisabled, NotAModOrAdmin, NotTopMod, NotLoggedIn, diff --git a/crates/utils/src/settings/mod.rs b/crates/utils/src/settings/mod.rs index 72d986d2da..6ef535ab95 100644 --- a/crates/utils/src/settings/mod.rs +++ b/crates/utils/src/settings/mod.rs @@ -97,7 +97,7 @@ impl Settings { WEBFINGER_REGEX.clone() } - pub fn pictrs_config(&self) -> LemmyResult { + pub fn pictrs(&self) -> LemmyResult { self .pictrs .clone() diff --git a/crates/utils/src/settings/structs.rs b/crates/utils/src/settings/structs.rs index 28710e8ca9..041980d2e5 100644 --- a/crates/utils/src/settings/structs.rs +++ b/crates/utils/src/settings/structs.rs @@ -116,6 +116,11 @@ pub struct PictrsConfig { /// if image is larger. #[default(512)] pub max_banner_size: u32, + + /// Prevent users from uploading images for posts or embedding in markdown. Avatars, icons and + /// banners can still be uploaded. + #[default(false)] + pub disable_image_upload: bool, } #[derive(Debug, Deserialize, Serialize, Clone, SmartDefault, Document, PartialEq)] From 55b8aced9d7c8b1245bc6f835d82639b0566fc6f Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 17 Dec 2024 16:23:43 +0100 Subject: [PATCH 10/16] split files into upload, download --- config/defaults.hjson | 3 + crates/api/src/site/leave_admin.rs | 1 + crates/api_common/src/utils.rs | 7 +- crates/api_crud/src/site/read.rs | 2 +- crates/routes/src/images/download.rs | 113 +++++++++++++++++++++++ crates/routes/src/images/mod.rs | 111 ++-------------------- crates/routes/src/images/person.rs | 36 -------- crates/routes/src/images/upload.rs | 132 +++++++++++++++++++++++++++ crates/routes/src/images/utils.rs | 122 ++----------------------- crates/utils/src/settings/structs.rs | 4 +- src/api_routes_v3.rs | 7 +- src/api_routes_v4.rs | 6 +- 12 files changed, 277 insertions(+), 267 deletions(-) create mode 100644 crates/routes/src/images/download.rs delete mode 100644 crates/routes/src/images/person.rs create mode 100644 crates/routes/src/images/upload.rs diff --git a/config/defaults.hjson b/config/defaults.hjson index aaf2a94d84..d6c24cce81 100644 --- a/config/defaults.hjson +++ b/config/defaults.hjson @@ -72,6 +72,9 @@ # Otherwise we have to use crop, or use max_width/max_height which throws error # if image is larger. max_banner_size: 512 + # Prevent users from uploading images for posts or embedding in markdown. Avatars, icons and + # banners can still be uploaded. + image_upload_disabled: false } # Email sending configuration. All options except login/password are mandatory email: { diff --git a/crates/api/src/site/leave_admin.rs b/crates/api/src/site/leave_admin.rs index fde258dd2e..042009d243 100644 --- a/crates/api/src/site/leave_admin.rs +++ b/crates/api/src/site/leave_admin.rs @@ -76,5 +76,6 @@ pub async fn leave_admin( blocked_urls, tagline, my_user: None, + image_upload_disabled: context.settings().pictrs()?.image_upload_disabled, })) } diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 3daa341070..27b7ad5311 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -1128,12 +1128,7 @@ async fn proxy_image_link_internal( /// Rewrite a link to go through `/api/v4/image_proxy` endpoint. This is only for remote urls and /// if image_proxy setting is enabled. pub async fn proxy_image_link(link: Url, context: &LemmyContext) -> LemmyResult { - proxy_image_link_internal( - link, - context.settings().pictrs()?.image_mode(), - context, - ) - .await + proxy_image_link_internal(link, context.settings().pictrs()?.image_mode(), context).await } pub async fn proxy_image_link_opt_api( diff --git a/crates/api_crud/src/site/read.rs b/crates/api_crud/src/site/read.rs index b618c0fa2a..64d2237a0f 100644 --- a/crates/api_crud/src/site/read.rs +++ b/crates/api_crud/src/site/read.rs @@ -69,6 +69,6 @@ async fn read_site(context: &LemmyContext) -> LemmyResult { tagline, oauth_providers: Some(oauth_providers), admin_oauth_providers: Some(admin_oauth_providers), - image_upload_disabled: context.settings().pictrs()?.disable_image_upload, + image_upload_disabled: context.settings().pictrs()?.image_upload_disabled, }) } diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs new file mode 100644 index 0000000000..b54b72f1ca --- /dev/null +++ b/crates/routes/src/images/download.rs @@ -0,0 +1,113 @@ +use super::utils::{adapt_request, convert_header, file_type}; +use actix_web::{ + body::{BodyStream, BoxBody}, + http::StatusCode, + web::{Data, *}, + HttpRequest, + HttpResponse, + Responder, +}; +use lemmy_api_common::{ + context::LemmyContext, + image::{ImageGetParams, ImageProxyParams}, +}; +use lemmy_db_schema::source::{images::RemoteImage, local_site::LocalSite}; +use lemmy_db_views::structs::LocalUserView; +use lemmy_utils::error::LemmyResult; +use url::Url; + +pub async fn get_image( + filename: Path, + Query(params): Query, + req: HttpRequest, + context: Data, + local_user_view: Option, +) -> LemmyResult { + // block access to images if instance is private and unauthorized, public + let local_site = LocalSite::read(&mut context.pool()).await?; + if local_site.private_instance && local_user_view.is_none() { + return Ok(HttpResponse::Unauthorized().finish()); + } + let name = &filename.into_inner(); + + // If there are no query params, the URL is original + let pictrs_url = context.settings().pictrs()?.url; + let processed_url = if params.file_type.is_none() && params.max_size.is_none() { + format!("{}image/original/{}", pictrs_url, name) + } else { + let file_type = file_type(params.file_type, name); + let mut url = format!("{}image/process.{}?src={}", pictrs_url, file_type, name); + + if let Some(size) = params.max_size { + url = format!("{url}&thumbnail={size}",); + } + url + }; + + do_get_image(processed_url, req).await +} +pub async fn image_proxy( + Query(params): Query, + req: HttpRequest, + context: Data, +) -> LemmyResult, HttpResponse>> { + let url = Url::parse(¶ms.url)?; + + // Check that url corresponds to a federated image so that this can't be abused as a proxy + // for arbitrary purposes. + RemoteImage::validate(&mut context.pool(), url.clone().into()).await?; + + let pictrs_config = context.settings().pictrs()?; + let processed_url = if params.file_type.is_none() && params.max_size.is_none() { + format!("{}image/original?proxy={}", pictrs_config.url, params.url) + } else { + let file_type = file_type(params.file_type, url.as_str()); + let mut url = format!( + "{}image/process.{}?proxy={}", + pictrs_config.url, file_type, url + ); + + if let Some(size) = params.max_size { + url = format!("{url}&thumbnail={size}",); + } + url + }; + + let bypass_proxy = pictrs_config + .proxy_bypass_domains + .iter() + .any(|s| url.domain().is_some_and(|d| d == s)); + if bypass_proxy { + // Bypass proxy and redirect user to original image + Ok(Either::Left(Redirect::to(url.to_string()).respond_to(&req))) + } else { + // Proxy the image data through Lemmy + Ok(Either::Right(do_get_image(processed_url, req).await?)) + } +} + +pub(super) async fn do_get_image(url: String, req: HttpRequest) -> LemmyResult { + let mut client_req = adapt_request(&req, url); + + if let Some(addr) = req.head().peer_addr { + client_req = client_req.header("X-Forwarded-For", addr.to_string()); + } + + if let Some(addr) = req.head().peer_addr { + client_req = client_req.header("X-Forwarded-For", addr.to_string()); + } + + let res = client_req.send().await?; + + if res.status() == http::StatusCode::NOT_FOUND { + return Ok(HttpResponse::NotFound().finish()); + } + + let mut client_res = HttpResponse::build(StatusCode::from_u16(res.status().as_u16())?); + + for (name, value) in res.headers().iter().filter(|(h, _)| *h != "connection") { + client_res.insert_header(convert_header(name, value)); + } + + Ok(client_res.body(BodyStream::new(res.bytes_stream()))) +} diff --git a/crates/routes/src/images/mod.rs b/crates/routes/src/images/mod.rs index 8e5a547345..52d3d89df3 100644 --- a/crates/routes/src/images/mod.rs +++ b/crates/routes/src/images/mod.rs @@ -1,73 +1,14 @@ -use actix_web::{body::BoxBody, web::*, HttpRequest, HttpResponse, Responder}; -use lemmy_api_common::{ - context::LemmyContext, - image::{DeleteImageParams, ImageGetParams, ImageProxyParams, UploadImageResponse}, - LemmyErrorType, - SuccessResponse, -}; -use lemmy_db_schema::source::{ - images::{LocalImage, RemoteImage}, - local_site::LocalSite, -}; +use actix_web::web::*; +use lemmy_api_common::{context::LemmyContext, image::DeleteImageParams, SuccessResponse}; +use lemmy_db_schema::source::images::LocalImage; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::LemmyResult; -use url::Url; -use utils::{do_get_image, do_upload_image, file_type, UploadType, PICTRS_CLIENT}; +use utils::PICTRS_CLIENT; -pub mod person; +pub mod download; +pub mod upload; mod utils; -pub async fn upload_image( - req: HttpRequest, - body: Payload, - local_user_view: LocalUserView, - context: Data, -) -> LemmyResult> { - if context.settings().pictrs()?.disable_image_upload { - return Err(LemmyErrorType::ImageUploadDisabled.into()); - } - - let image = do_upload_image(req, body, UploadType::Other, &local_user_view, &context).await?; - - let image_url = image.image_url(&context.settings().get_protocol_and_hostname())?; - Ok(Json(UploadImageResponse { - image_url, - filename: image.file, - delete_token: image.delete_token, - })) -} - -pub async fn get_image( - filename: Path, - Query(params): Query, - req: HttpRequest, - context: Data, - local_user_view: Option, -) -> LemmyResult { - // block access to images if instance is private and unauthorized, public - let local_site = LocalSite::read(&mut context.pool()).await?; - if local_site.private_instance && local_user_view.is_none() { - return Ok(HttpResponse::Unauthorized().finish()); - } - let name = &filename.into_inner(); - - // If there are no query params, the URL is original - let pictrs_url = context.settings().pictrs()?.url; - let processed_url = if params.file_type.is_none() && params.max_size.is_none() { - format!("{}image/original/{}", pictrs_url, name) - } else { - let file_type = file_type(params.file_type, name); - let mut url = format!("{}image/process.{}?src={}", pictrs_url, file_type, name); - - if let Some(size) = params.max_size { - url = format!("{url}&thumbnail={size}",); - } - url - }; - - do_get_image(processed_url, req).await -} - pub async fn delete_image( data: Json, context: Data, @@ -95,43 +36,3 @@ pub async fn pictrs_health(context: Data) -> LemmyResult, - req: HttpRequest, - context: Data, -) -> LemmyResult, HttpResponse>> { - let url = Url::parse(¶ms.url)?; - - // Check that url corresponds to a federated image so that this can't be abused as a proxy - // for arbitrary purposes. - RemoteImage::validate(&mut context.pool(), url.clone().into()).await?; - - let pictrs_config = context.settings().pictrs()?; - let processed_url = if params.file_type.is_none() && params.max_size.is_none() { - format!("{}image/original?proxy={}", pictrs_config.url, params.url) - } else { - let file_type = file_type(params.file_type, url.as_str()); - let mut url = format!( - "{}image/process.{}?proxy={}", - pictrs_config.url, file_type, url - ); - - if let Some(size) = params.max_size { - url = format!("{url}&thumbnail={size}",); - } - url - }; - - let bypass_proxy = pictrs_config - .proxy_bypass_domains - .iter() - .any(|s| url.domain().is_some_and(|d| d == s)); - if bypass_proxy { - // Bypass proxy and redirect user to original image - Ok(Either::Left(Redirect::to(url.to_string()).respond_to(&req))) - } else { - // Proxy the image data through Lemmy - Ok(Either::Right(do_get_image(processed_url, req).await?)) - } -} diff --git a/crates/routes/src/images/person.rs b/crates/routes/src/images/person.rs deleted file mode 100644 index edac1e41b2..0000000000 --- a/crates/routes/src/images/person.rs +++ /dev/null @@ -1,36 +0,0 @@ -use super::utils::{delete_old_image, do_upload_image, UploadType}; -use actix_web::{self, web::*, HttpRequest}; -use lemmy_api_common::{context::LemmyContext, SuccessResponse}; -use lemmy_db_schema::{ - source::person::{Person, PersonUpdateForm}, - traits::Crud, -}; -use lemmy_db_views::structs::LocalUserView; -use lemmy_utils::error::LemmyResult; -use url::Url; - -pub async fn upload_avatar( - req: HttpRequest, - body: Payload, - local_user_view: LocalUserView, - context: Data, -) -> LemmyResult> { - let image = do_upload_image(req, body, UploadType::Avatar, &local_user_view, &context).await?; - - delete_old_image(&local_user_view.person.avatar, &context).await?; - - let avatar = format!( - "{}/api/v4/image/{}", - context.settings().get_protocol_and_hostname(), - image.file - ); - let avatar = Some(Some(Url::parse(&avatar)?.into())); - let person_form = PersonUpdateForm { - avatar, - ..Default::default() - }; - - Person::update(&mut context.pool(), local_user_view.person.id, &person_form).await?; - - Ok(Json(SuccessResponse::default())) -} diff --git a/crates/routes/src/images/upload.rs b/crates/routes/src/images/upload.rs new file mode 100644 index 0000000000..c009aae263 --- /dev/null +++ b/crates/routes/src/images/upload.rs @@ -0,0 +1,132 @@ +use super::utils::{adapt_request, delete_old_image, make_send}; +use actix_web::{self, web::*, HttpRequest}; +use lemmy_api_common::{ + context::LemmyContext, + image::UploadImageResponse, + request::{PictrsFile, PictrsResponse}, + LemmyErrorType, + SuccessResponse, +}; +use lemmy_db_schema::{ + source::{ + images::{LocalImage, LocalImageForm}, + person::{Person, PersonUpdateForm}, + }, + traits::Crud, +}; +use lemmy_db_views::structs::LocalUserView; +use lemmy_utils::error::LemmyResult; +use reqwest::Body; +use std::time::Duration; +use url::Url; + +pub async fn upload_image( + req: HttpRequest, + body: Payload, + local_user_view: LocalUserView, + context: Data, +) -> LemmyResult> { + if context.settings().pictrs()?.image_upload_disabled { + return Err(LemmyErrorType::ImageUploadDisabled.into()); + } + + let image = do_upload_image(req, body, UploadType::Other, &local_user_view, &context).await?; + + let image_url = image.image_url(&context.settings().get_protocol_and_hostname())?; + Ok(Json(UploadImageResponse { + image_url, + filename: image.file, + delete_token: image.delete_token, + })) +} + +pub async fn upload_avatar( + req: HttpRequest, + body: Payload, + local_user_view: LocalUserView, + context: Data, +) -> LemmyResult> { + let image = do_upload_image(req, body, UploadType::Avatar, &local_user_view, &context).await?; + + delete_old_image(&local_user_view.person.avatar, &context).await?; + + let avatar = format!( + "{}/api/v4/image/{}", + context.settings().get_protocol_and_hostname(), + image.file + ); + let avatar = Some(Some(Url::parse(&avatar)?.into())); + let person_form = PersonUpdateForm { + avatar, + ..Default::default() + }; + + Person::update(&mut context.pool(), local_user_view.person.id, &person_form).await?; + + Ok(Json(SuccessResponse::default())) +} +pub enum UploadType { + Avatar, + Other, +} + +pub async fn do_upload_image( + req: HttpRequest, + body: Payload, + upload_type: UploadType, + local_user_view: &LocalUserView, + context: &Data, +) -> LemmyResult { + let pictrs_config = context.settings().pictrs()?; + let image_url = format!("{}image", pictrs_config.url); + + let mut client_req = adapt_request(&req, image_url); + + client_req = match upload_type { + UploadType::Avatar => { + let max_size = context.settings().pictrs()?.max_avatar_size.to_string(); + client_req.query(&[ + ("resize", max_size.as_ref()), + ("allow_animation", "false"), + ("allow_video", "false"), + ]) + } + // TODO: same as above but using `max_banner_size` + // UploadType::Banner => {} + _ => client_req, + }; + if let Some(addr) = req.head().peer_addr { + client_req = client_req.header("X-Forwarded-For", addr.to_string()) + }; + let res = client_req + .timeout(Duration::from_secs(pictrs_config.upload_timeout)) + .body(Body::wrap_stream(make_send(body))) + .send() + .await? + .error_for_status()?; + + let mut images = res.json::().await?; + for image in &images.files { + // Pictrs allows uploading multiple images in a single request. Lemmy doesnt need this, + // but still a user may upload multiple and so we need to store all links in db for + // to allow deletion via web ui. + let form = LocalImageForm { + local_user_id: Some(local_user_view.local_user.id), + pictrs_alias: image.file.to_string(), + pictrs_delete_token: image.delete_token.to_string(), + }; + + let protocol_and_hostname = context.settings().get_protocol_and_hostname(); + let thumbnail_url = image.image_url(&protocol_and_hostname)?; + + // Also store the details for the image + let details_form = image.details.build_image_details_form(&thumbnail_url); + LocalImage::create(&mut context.pool(), &form, &details_form).await?; + } + let image = images + .files + .pop() + .ok_or(LemmyErrorType::InvalidImageUpload)?; + + Ok(image) +} diff --git a/crates/routes/src/images/utils.rs b/crates/routes/src/images/utils.rs index 50e0b8fd40..040734a8bc 100644 --- a/crates/routes/src/images/utils.rs +++ b/crates/routes/src/images/utils.rs @@ -1,31 +1,22 @@ use actix_web::{ - body::BodyStream, http::{ header::{HeaderName, ACCEPT_ENCODING, HOST}, Method, - StatusCode, }, - web::{Data, Payload}, + web::Data, HttpRequest, - HttpResponse, }; use futures::stream::{Stream, StreamExt}; use http::HeaderValue; use lemmy_api_common::{ context::LemmyContext, - request::{client_builder, delete_image_from_pictrs, PictrsFile, PictrsResponse}, - LemmyErrorType, + request::{client_builder, delete_image_from_pictrs}, }; -use lemmy_db_schema::{ - newtypes::DbUrl, - source::images::{LocalImage, LocalImageForm}, -}; -use lemmy_db_views::structs::LocalUserView; +use lemmy_db_schema::{newtypes::DbUrl, source::images::LocalImage}; use lemmy_utils::{error::LemmyResult, settings::SETTINGS, REQWEST_TIMEOUT}; -use reqwest::Body; use reqwest_middleware::{ClientBuilder, ClientWithMiddleware, RequestBuilder}; use reqwest_tracing::TracingMiddleware; -use std::{sync::LazyLock, time::Duration}; +use std::sync::LazyLock; // Pictrs cannot use proxy #[allow(clippy::expect_used)] @@ -40,7 +31,7 @@ pub(super) static PICTRS_CLIENT: LazyLock = LazyLock::new( .build() }); -fn adapt_request(request: &HttpRequest, url: String) -> RequestBuilder { +pub(super) fn adapt_request(request: &HttpRequest, url: String) -> RequestBuilder { // remove accept-encoding header so that pictrs doesn't compress the response const INVALID_HEADERS: &[HeaderName] = &[ACCEPT_ENCODING, HOST]; @@ -61,7 +52,7 @@ fn adapt_request(request: &HttpRequest, url: String) -> RequestBuilder { }) } -fn make_send(mut stream: S) -> impl Stream + Send + Unpin + 'static +pub(super) fn make_send(mut stream: S) -> impl Stream + Send + Unpin + 'static where S: Stream + Unpin + 'static, S::Item: Send, @@ -109,106 +100,13 @@ pub(super) fn convert_method(method: &Method) -> http::Method { http::Method::from_bytes(method.as_str().as_bytes()).expect("method can be converted") } -fn convert_header<'a>(name: &'a http::HeaderName, value: &'a HeaderValue) -> (&'a str, &'a [u8]) { +pub(super) fn convert_header<'a>( + name: &'a http::HeaderName, + value: &'a HeaderValue, +) -> (&'a str, &'a [u8]) { (name.as_str(), value.as_bytes()) } -pub(super) enum UploadType { - Avatar, - Other, -} - -pub(super) async fn do_upload_image( - req: HttpRequest, - body: Payload, - upload_type: UploadType, - local_user_view: &LocalUserView, - context: &Data, -) -> LemmyResult { - let pictrs_config = context.settings().pictrs()?; - let image_url = format!("{}image", pictrs_config.url); - - let mut client_req = adapt_request(&req, image_url); - - client_req = match upload_type { - UploadType::Avatar => { - let max_size = context - .settings() - .pictrs()? - .max_avatar_size - .to_string(); - client_req.query(&[ - ("resize", max_size.as_ref()), - ("allow_animation", "false"), - ("allow_video", "false"), - ]) - } - // TODO: same as above but using `max_banner_size` - // UploadType::Banner => {} - _ => client_req, - }; - if let Some(addr) = req.head().peer_addr { - client_req = client_req.header("X-Forwarded-For", addr.to_string()) - }; - let res = client_req - .timeout(Duration::from_secs(pictrs_config.upload_timeout)) - .body(Body::wrap_stream(make_send(body))) - .send() - .await? - .error_for_status()?; - - let mut images = res.json::().await?; - for image in &images.files { - // Pictrs allows uploading multiple images in a single request. Lemmy doesnt need this, - // but still a user may upload multiple and so we need to store all links in db for - // to allow deletion via web ui. - let form = LocalImageForm { - local_user_id: Some(local_user_view.local_user.id), - pictrs_alias: image.file.to_string(), - pictrs_delete_token: image.delete_token.to_string(), - }; - - let protocol_and_hostname = context.settings().get_protocol_and_hostname(); - let thumbnail_url = image.image_url(&protocol_and_hostname)?; - - // Also store the details for the image - let details_form = image.details.build_image_details_form(&thumbnail_url); - LocalImage::create(&mut context.pool(), &form, &details_form).await?; - } - let image = images - .files - .pop() - .ok_or(LemmyErrorType::InvalidImageUpload)?; - - Ok(image) -} - -pub(super) async fn do_get_image(url: String, req: HttpRequest) -> LemmyResult { - let mut client_req = adapt_request(&req, url); - - if let Some(addr) = req.head().peer_addr { - client_req = client_req.header("X-Forwarded-For", addr.to_string()); - } - - if let Some(addr) = req.head().peer_addr { - client_req = client_req.header("X-Forwarded-For", addr.to_string()); - } - - let res = client_req.send().await?; - - if res.status() == http::StatusCode::NOT_FOUND { - return Ok(HttpResponse::NotFound().finish()); - } - - let mut client_res = HttpResponse::build(StatusCode::from_u16(res.status().as_u16())?); - - for (name, value) in res.headers().iter().filter(|(h, _)| *h != "connection") { - client_res.insert_header(convert_header(name, value)); - } - - Ok(client_res.body(BodyStream::new(res.bytes_stream()))) -} - /// When adding a new avatar, banner or similar image, delete the old one. pub(super) async fn delete_old_image( old_image: &Option, diff --git a/crates/utils/src/settings/structs.rs b/crates/utils/src/settings/structs.rs index 041980d2e5..b9c38f2036 100644 --- a/crates/utils/src/settings/structs.rs +++ b/crates/utils/src/settings/structs.rs @@ -116,11 +116,11 @@ pub struct PictrsConfig { /// if image is larger. #[default(512)] pub max_banner_size: u32, - + /// Prevent users from uploading images for posts or embedding in markdown. Avatars, icons and /// banners can still be uploaded. #[default(false)] - pub disable_image_upload: bool, + pub image_upload_disabled: bool, } #[derive(Debug, Deserialize, Serialize, Clone, SmartDefault, Document, PartialEq)] diff --git a/src/api_routes_v3.rs b/src/api_routes_v3.rs index e7cb828838..59b7de610f 100644 --- a/src/api_routes_v3.rs +++ b/src/api_routes_v3.rs @@ -134,7 +134,12 @@ use lemmy_apub::api::{ search::search, user_settings_backup::{export_settings, import_settings}, }; -use lemmy_routes::images::{delete_image, get_image, image_proxy, pictrs_health, upload_image}; +use lemmy_routes::images::{ + delete_image, + download::{get_image, image_proxy}, + pictrs_health, + upload::upload_image, +}; use lemmy_utils::rate_limit::RateLimitCell; // Deprecated, use api v4 instead. diff --git a/src/api_routes_v4.rs b/src/api_routes_v4.rs index 99ae86fbc6..4bfe06a6a6 100644 --- a/src/api_routes_v4.rs +++ b/src/api_routes_v4.rs @@ -161,11 +161,9 @@ use lemmy_apub::api::{ }; use lemmy_routes::images::{ delete_image, - get_image, - image_proxy, - person::upload_avatar, + download::{get_image, image_proxy}, pictrs_health, - upload_image, + upload::{upload_avatar, upload_image}, }; use lemmy_utils::rate_limit::RateLimitCell; From 460ccc8e5a37159b54f736b2c330815ea2658301 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Wed, 18 Dec 2024 11:11:31 +0100 Subject: [PATCH 11/16] move sitemap to top level, not in api --- src/api_routes_v3.rs | 6 ------ src/api_routes_v4.rs | 2 -- src/lib.rs | 8 +++++++- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/api_routes_v3.rs b/src/api_routes_v3.rs index 59b7de610f..738299cdac 100644 --- a/src/api_routes_v3.rs +++ b/src/api_routes_v3.rs @@ -87,7 +87,6 @@ use lemmy_api::{ unread_count::get_unread_registration_application_count, }, }, - sitemap::get_sitemap, }; use lemmy_api_crud::{ comment::{ @@ -395,9 +394,4 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .route("/delete", post().to(delete_custom_emoji)), ), ); - cfg.service( - scope("/sitemap.xml") - .wrap(rate_limit.message()) - .route("", get().to(get_sitemap)), - ); } diff --git a/src/api_routes_v4.rs b/src/api_routes_v4.rs index 4bfe06a6a6..f8dabd0622 100644 --- a/src/api_routes_v4.rs +++ b/src/api_routes_v4.rs @@ -96,7 +96,6 @@ use lemmy_api::{ unread_count::get_unread_registration_application_count, }, }, - sitemap::get_sitemap, }; use lemmy_api_crud::{ comment::{ @@ -392,7 +391,6 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .wrap(rate_limit.register()) .route("/authenticate", post().to(authenticate_with_oauth)), ) - .route("/sitemap.xml", get().to(get_sitemap)) .service( scope("/image") .service( diff --git a/src/lib.rs b/src/lib.rs index ae060ec75e..03dc783007 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,13 +11,14 @@ use actix_cors::Cors; use actix_web::{ dev::{ServerHandle, ServiceResponse}, middleware::{self, Condition, ErrorHandlerResponse, ErrorHandlers}, - web::Data, + web::{get, scope, Data}, App, HttpResponse, HttpServer, }; use actix_web_prom::PrometheusMetricsBuilder; use clap::Parser; +use lemmy_api::sitemap::get_sitemap; use lemmy_api_common::{ context::LemmyContext, lemmy_db_views::structs::SiteView, @@ -324,6 +325,11 @@ fn create_http_server( }) .configure(feeds::config) .configure(nodeinfo::config) + .service( + scope("/sitemap.xml") + .wrap(rate_limit_cell.message()) + .route("", get().to(get_sitemap)), + ) }) .disable_signals() .bind(bind)? From 5f0619534e2edefe87e598206e940488d19cd285 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Wed, 18 Dec 2024 11:24:30 +0100 Subject: [PATCH 12/16] simplify code --- api_tests/run-federation-test.sh | 2 +- crates/routes/src/images/upload.rs | 51 ++++++++++++---------------- crates/utils/src/settings/structs.rs | 1 + src/api_routes_v4.rs | 4 +-- 4 files changed, 26 insertions(+), 32 deletions(-) diff --git a/api_tests/run-federation-test.sh b/api_tests/run-federation-test.sh index 969a95b3e7..f9eab50395 100755 --- a/api_tests/run-federation-test.sh +++ b/api_tests/run-federation-test.sh @@ -11,7 +11,7 @@ killall -s1 lemmy_server || true popd pnpm i -pnpm api-test || true +pnpm api-test-image || true killall -s1 lemmy_server || true killall -s1 pict-rs || true diff --git a/crates/routes/src/images/upload.rs b/crates/routes/src/images/upload.rs index c009aae263..735a18d154 100644 --- a/crates/routes/src/images/upload.rs +++ b/crates/routes/src/images/upload.rs @@ -3,7 +3,7 @@ use actix_web::{self, web::*, HttpRequest}; use lemmy_api_common::{ context::LemmyContext, image::UploadImageResponse, - request::{PictrsFile, PictrsResponse}, + request::PictrsResponse, LemmyErrorType, SuccessResponse, }; @@ -18,7 +18,12 @@ use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::LemmyResult; use reqwest::Body; use std::time::Duration; -use url::Url; +use UploadType::*; + +pub enum UploadType { + Avatar, + Other, +} pub async fn upload_image( req: HttpRequest, @@ -30,45 +35,28 @@ pub async fn upload_image( return Err(LemmyErrorType::ImageUploadDisabled.into()); } - let image = do_upload_image(req, body, UploadType::Other, &local_user_view, &context).await?; - - let image_url = image.image_url(&context.settings().get_protocol_and_hostname())?; - Ok(Json(UploadImageResponse { - image_url, - filename: image.file, - delete_token: image.delete_token, - })) + Ok(Json( + do_upload_image(req, body, Other, &local_user_view, &context).await?, + )) } -pub async fn upload_avatar( +pub async fn upload_user_avatar( req: HttpRequest, body: Payload, local_user_view: LocalUserView, context: Data, ) -> LemmyResult> { - let image = do_upload_image(req, body, UploadType::Avatar, &local_user_view, &context).await?; - + let image = do_upload_image(req, body, Avatar, &local_user_view, &context).await?; delete_old_image(&local_user_view.person.avatar, &context).await?; - let avatar = format!( - "{}/api/v4/image/{}", - context.settings().get_protocol_and_hostname(), - image.file - ); - let avatar = Some(Some(Url::parse(&avatar)?.into())); let person_form = PersonUpdateForm { - avatar, + avatar: Some(Some(image.image_url.into())), ..Default::default() }; - Person::update(&mut context.pool(), local_user_view.person.id, &person_form).await?; Ok(Json(SuccessResponse::default())) } -pub enum UploadType { - Avatar, - Other, -} pub async fn do_upload_image( req: HttpRequest, @@ -76,14 +64,14 @@ pub async fn do_upload_image( upload_type: UploadType, local_user_view: &LocalUserView, context: &Data, -) -> LemmyResult { +) -> LemmyResult { let pictrs_config = context.settings().pictrs()?; let image_url = format!("{}image", pictrs_config.url); let mut client_req = adapt_request(&req, image_url); client_req = match upload_type { - UploadType::Avatar => { + Avatar => { let max_size = context.settings().pictrs()?.max_avatar_size.to_string(); client_req.query(&[ ("resize", max_size.as_ref()), @@ -92,7 +80,7 @@ pub async fn do_upload_image( ]) } // TODO: same as above but using `max_banner_size` - // UploadType::Banner => {} + // Banner => {} _ => client_req, }; if let Some(addr) = req.head().peer_addr { @@ -128,5 +116,10 @@ pub async fn do_upload_image( .pop() .ok_or(LemmyErrorType::InvalidImageUpload)?; - Ok(image) + let url = image.image_url(&context.settings().get_protocol_and_hostname())?; + Ok(UploadImageResponse { + image_url: url, + filename: image.file, + delete_token: image.delete_token, + }) } diff --git a/crates/utils/src/settings/structs.rs b/crates/utils/src/settings/structs.rs index b9c38f2036..153c4567e1 100644 --- a/crates/utils/src/settings/structs.rs +++ b/crates/utils/src/settings/structs.rs @@ -114,6 +114,7 @@ pub struct PictrsConfig { /// TODO: Unfortunately pictrs can only resize images to fit in a*a square, no rectangle. /// Otherwise we have to use crop, or use max_width/max_height which throws error /// if image is larger. + /// TODO: whats a sensible default here? #[default(512)] pub max_banner_size: u32, diff --git a/src/api_routes_v4.rs b/src/api_routes_v4.rs index f8dabd0622..90f751b558 100644 --- a/src/api_routes_v4.rs +++ b/src/api_routes_v4.rs @@ -162,7 +162,7 @@ use lemmy_routes::images::{ delete_image, download::{get_image, image_proxy}, pictrs_health, - upload::{upload_avatar, upload_image}, + upload::{upload_image, upload_user_avatar}, }; use lemmy_utils::rate_limit::RateLimitCell; @@ -319,7 +319,7 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .route("/unread_count", get().to(unread_count)) .route("/list_logins", get().to(list_logins)) .route("/validate_auth", get().to(validate_auth)) - .route("/avatar", post().to(upload_avatar)) + .route("/avatar", post().to(upload_user_avatar)) .service( scope("/block") .route("/person", post().to(user_block_person)) From 5f49b2aaec2af04dcc5acf8ada63ce5ee571401d Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Wed, 18 Dec 2024 11:26:59 +0100 Subject: [PATCH 13/16] add upload user banner --- crates/api/src/local_user/save_settings.rs | 9 +-------- crates/api_common/src/person.rs | 3 --- crates/routes/src/images/upload.rs | 19 +++++++++++++++++++ src/api_routes_v4.rs | 3 ++- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs index bcf1b02b6e..930f6bf483 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -3,12 +3,10 @@ use actix_web::web::Json; use lemmy_api_common::{ context::LemmyContext, person::SaveUserSettings, - request::replace_image, utils::{ get_url_blocklist, local_site_to_slur_regex, process_markdown_opt, - proxy_image_link_opt_api, send_verification_email, }, SuccessResponse, @@ -21,7 +19,7 @@ use lemmy_db_schema::{ person::{Person, PersonUpdateForm}, }, traits::Crud, - utils::{diesel_string_update, diesel_url_update}, + utils::diesel_string_update, }; use lemmy_db_views::structs::{LocalUserView, SiteView}; use lemmy_utils::{ @@ -46,10 +44,6 @@ pub async fn save_user_settings( .as_deref(), ); - let banner = diesel_url_update(data.banner.as_deref())?; - replace_image(&banner, &local_user_view.person.banner, &context).await?; - let banner = proxy_image_link_opt_api(banner, &context).await?; - let display_name = diesel_string_update(data.display_name.as_deref()); let matrix_user_id = diesel_string_update(data.matrix_user_id.as_deref()); let email_deref = data.email.as_deref().map(str::to_lowercase); @@ -104,7 +98,6 @@ pub async fn save_user_settings( bio, matrix_user_id, bot_account: data.bot_account, - banner, ..Default::default() }; diff --git a/crates/api_common/src/person.rs b/crates/api_common/src/person.rs index 9c8a1d087b..5e79023776 100644 --- a/crates/api_common/src/person.rs +++ b/crates/api_common/src/person.rs @@ -114,9 +114,6 @@ pub struct SaveUserSettings { /// The language of the lemmy interface #[cfg_attr(feature = "full", ts(optional))] pub interface_language: Option, - /// A URL for your banner. - #[cfg_attr(feature = "full", ts(optional))] - pub banner: Option, /// Your display name, which can contain strange characters, and does not need to be unique. #[cfg_attr(feature = "full", ts(optional))] pub display_name: Option, diff --git a/crates/routes/src/images/upload.rs b/crates/routes/src/images/upload.rs index 735a18d154..6b24940b95 100644 --- a/crates/routes/src/images/upload.rs +++ b/crates/routes/src/images/upload.rs @@ -22,6 +22,7 @@ use UploadType::*; pub enum UploadType { Avatar, + Banner, Other, } @@ -58,6 +59,24 @@ pub async fn upload_user_avatar( Ok(Json(SuccessResponse::default())) } +pub async fn upload_user_banner( + req: HttpRequest, + body: Payload, + local_user_view: LocalUserView, + context: Data, +) -> LemmyResult> { + let image = do_upload_image(req, body, Banner, &local_user_view, &context).await?; + delete_old_image(&local_user_view.person.banner, &context).await?; + + let person_form = PersonUpdateForm { + banner: Some(Some(image.image_url.into())), + ..Default::default() + }; + Person::update(&mut context.pool(), local_user_view.person.id, &person_form).await?; + + Ok(Json(SuccessResponse::default())) +} + pub async fn do_upload_image( req: HttpRequest, body: Payload, diff --git a/src/api_routes_v4.rs b/src/api_routes_v4.rs index 90f751b558..f725b66ca5 100644 --- a/src/api_routes_v4.rs +++ b/src/api_routes_v4.rs @@ -162,7 +162,7 @@ use lemmy_routes::images::{ delete_image, download::{get_image, image_proxy}, pictrs_health, - upload::{upload_image, upload_user_avatar}, + upload::{upload_image, upload_user_avatar, upload_user_banner}, }; use lemmy_utils::rate_limit::RateLimitCell; @@ -320,6 +320,7 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .route("/list_logins", get().to(list_logins)) .route("/validate_auth", get().to(validate_auth)) .route("/avatar", post().to(upload_user_avatar)) + .route("/banner", post().to(upload_user_banner)) .service( scope("/block") .route("/person", post().to(user_block_person)) From a6f7e76bec2da12b52515b085ea3667d3de3b8bf Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Wed, 18 Dec 2024 11:39:19 +0100 Subject: [PATCH 14/16] community icon/banner --- crates/api_common/src/image.rs | 8 +++ crates/api_crud/src/community/create.rs | 10 ---- crates/routes/src/images/upload.rs | 74 +++++++++++++++++++++---- src/api_routes_v4.rs | 10 +++- 4 files changed, 80 insertions(+), 22 deletions(-) diff --git a/crates/api_common/src/image.rs b/crates/api_common/src/image.rs index c28832921a..e93b367f60 100644 --- a/crates/api_common/src/image.rs +++ b/crates/api_common/src/image.rs @@ -1,3 +1,4 @@ +use lemmy_db_schema::newtypes::CommunityId; use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; #[cfg(feature = "full")] @@ -44,3 +45,10 @@ pub struct UploadImageResponse { pub filename: String, pub delete_token: String, } + +/// Parameter for setting community icon or banner. Can't use POST data here as it already contains +/// the image data. +#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)] +pub struct CommunityIdQuery { + pub id: CommunityId, +} diff --git a/crates/api_crud/src/community/create.rs b/crates/api_crud/src/community/create.rs index c81157950d..0437396cc7 100644 --- a/crates/api_crud/src/community/create.rs +++ b/crates/api_crud/src/community/create.rs @@ -13,7 +13,6 @@ use lemmy_api_common::{ is_admin, local_site_to_slur_regex, process_markdown_opt, - proxy_image_link_api, EndpointType, }, }; @@ -31,7 +30,6 @@ use lemmy_db_schema::{ }, }, traits::{ApubActor, Crud, Followable, Joinable}, - utils::diesel_url_create, }; use lemmy_db_views::structs::{LocalUserView, SiteView}; use lemmy_utils::{ @@ -76,12 +74,6 @@ pub async fn create_community( check_slurs(desc, &slur_regex)?; } - let icon = diesel_url_create(data.icon.as_deref())?; - let icon = proxy_image_link_api(icon, &context).await?; - - let banner = diesel_url_create(data.banner.as_deref())?; - let banner = proxy_image_link_api(banner, &context).await?; - is_valid_actor_name(&data.name, local_site.actor_name_max_length as usize)?; if let Some(desc) = &data.description { @@ -108,8 +100,6 @@ pub async fn create_community( let community_form = CommunityInsertForm { sidebar, description, - icon, - banner, nsfw: data.nsfw, actor_id: Some(community_actor_id.clone()), private_key: Some(keypair.private_key), diff --git a/crates/routes/src/images/upload.rs b/crates/routes/src/images/upload.rs index 6b24940b95..8774216411 100644 --- a/crates/routes/src/images/upload.rs +++ b/crates/routes/src/images/upload.rs @@ -2,13 +2,15 @@ use super::utils::{adapt_request, delete_old_image, make_send}; use actix_web::{self, web::*, HttpRequest}; use lemmy_api_common::{ context::LemmyContext, - image::UploadImageResponse, + image::{CommunityIdQuery, UploadImageResponse}, request::PictrsResponse, + utils::is_mod_or_admin, LemmyErrorType, SuccessResponse, }; use lemmy_db_schema::{ source::{ + community::{Community, CommunityUpdateForm}, images::{LocalImage, LocalImageForm}, person::{Person, PersonUpdateForm}, }, @@ -50,11 +52,11 @@ pub async fn upload_user_avatar( let image = do_upload_image(req, body, Avatar, &local_user_view, &context).await?; delete_old_image(&local_user_view.person.avatar, &context).await?; - let person_form = PersonUpdateForm { + let form = PersonUpdateForm { avatar: Some(Some(image.image_url.into())), ..Default::default() }; - Person::update(&mut context.pool(), local_user_view.person.id, &person_form).await?; + Person::update(&mut context.pool(), local_user_view.person.id, &form).await?; Ok(Json(SuccessResponse::default())) } @@ -68,11 +70,55 @@ pub async fn upload_user_banner( let image = do_upload_image(req, body, Banner, &local_user_view, &context).await?; delete_old_image(&local_user_view.person.banner, &context).await?; - let person_form = PersonUpdateForm { + let form = PersonUpdateForm { banner: Some(Some(image.image_url.into())), ..Default::default() }; - Person::update(&mut context.pool(), local_user_view.person.id, &person_form).await?; + Person::update(&mut context.pool(), local_user_view.person.id, &form).await?; + + Ok(Json(SuccessResponse::default())) +} + +pub async fn upload_community_icon( + req: HttpRequest, + query: Query, + body: Payload, + local_user_view: LocalUserView, + context: Data, +) -> LemmyResult> { + let community: Community = Community::read(&mut context.pool(), query.id).await?; + is_mod_or_admin(&mut context.pool(), &local_user_view.person, community.id).await?; + + let image = do_upload_image(req, body, Avatar, &local_user_view, &context).await?; + delete_old_image(&community.icon, &context).await?; + + let form = CommunityUpdateForm { + icon: Some(Some(image.image_url.into())), + ..Default::default() + }; + Community::update(&mut context.pool(), community.id, &form).await?; + + Ok(Json(SuccessResponse::default())) +} + +pub async fn upload_community_banner( + req: HttpRequest, + query: Query, + body: Payload, + local_user_view: LocalUserView, + context: Data, +) -> LemmyResult> { + let community: Community = Community::read(&mut context.pool(), query.id).await?; + is_mod_or_admin(&mut context.pool(), &local_user_view.person, community.id).await?; + + let image = do_upload_image(req, body, Banner, &local_user_view, &context).await?; + delete_old_image(&community.icon, &context).await?; + + let form = CommunityUpdateForm { + banner: Some(Some(image.image_url.into())), + ..Default::default() + }; + Community::update(&mut context.pool(), community.id, &form).await?; Ok(Json(SuccessResponse::default())) } @@ -84,29 +130,35 @@ pub async fn do_upload_image( local_user_view: &LocalUserView, context: &Data, ) -> LemmyResult { - let pictrs_config = context.settings().pictrs()?; - let image_url = format!("{}image", pictrs_config.url); + let pictrs = context.settings().pictrs()?; + let image_url = format!("{}image", pictrs.url); let mut client_req = adapt_request(&req, image_url); client_req = match upload_type { Avatar => { - let max_size = context.settings().pictrs()?.max_avatar_size.to_string(); + let max_size = pictrs.max_avatar_size.to_string(); + client_req.query(&[ + ("resize", max_size.as_ref()), + ("allow_animation", "false"), + ("allow_video", "false"), + ]) + } + Banner => { + let max_size = pictrs.max_banner_size.to_string(); client_req.query(&[ ("resize", max_size.as_ref()), ("allow_animation", "false"), ("allow_video", "false"), ]) } - // TODO: same as above but using `max_banner_size` - // Banner => {} _ => client_req, }; if let Some(addr) = req.head().peer_addr { client_req = client_req.header("X-Forwarded-For", addr.to_string()) }; let res = client_req - .timeout(Duration::from_secs(pictrs_config.upload_timeout)) + .timeout(Duration::from_secs(pictrs.upload_timeout)) .body(Body::wrap_stream(make_send(body))) .send() .await? diff --git a/src/api_routes_v4.rs b/src/api_routes_v4.rs index f725b66ca5..c7f0dd3548 100644 --- a/src/api_routes_v4.rs +++ b/src/api_routes_v4.rs @@ -162,7 +162,13 @@ use lemmy_routes::images::{ delete_image, download::{get_image, image_proxy}, pictrs_health, - upload::{upload_image, upload_user_avatar, upload_user_banner}, + upload::{ + upload_community_banner, + upload_community_icon, + upload_image, + upload_user_avatar, + upload_user_banner, + }, }; use lemmy_utils::rate_limit::RateLimitCell; @@ -205,6 +211,8 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { .route("/transfer", post().to(transfer_community)) .route("/ban_user", post().to(ban_from_community)) .route("/mod", post().to(add_mod_to_community)) + .route("/icon", post().to(upload_community_icon)) + .route("/banner", post().to(upload_community_banner)) .service( scope("/pending_follows") .route("/count", get().to(get_pending_follows_count)) From ada8f1ba8e7948e97877762ac8dc5452739a9e93 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Wed, 18 Dec 2024 11:52:17 +0100 Subject: [PATCH 15/16] site icon/banner --- api_tests/run-federation-test.sh | 2 +- crates/api_common/src/community.rs | 6 ---- crates/api_common/src/request.rs | 35 +++--------------- crates/api_common/src/site.rs | 10 ------ crates/api_common/src/utils.rs | 25 ------------- crates/api_crud/src/community/update.rs | 14 +------- crates/api_crud/src/site/create.rs | 11 +----- crates/api_crud/src/site/update.rs | 14 +------- crates/routes/src/images/upload.rs | 47 +++++++++++++++++++++++-- src/api_routes_v4.rs | 6 +++- 10 files changed, 58 insertions(+), 112 deletions(-) diff --git a/api_tests/run-federation-test.sh b/api_tests/run-federation-test.sh index f9eab50395..969a95b3e7 100755 --- a/api_tests/run-federation-test.sh +++ b/api_tests/run-federation-test.sh @@ -11,7 +11,7 @@ killall -s1 lemmy_server || true popd pnpm i -pnpm api-test-image || true +pnpm api-test || true killall -s1 lemmy_server || true killall -s1 pict-rs || true diff --git a/crates/api_common/src/community.rs b/crates/api_common/src/community.rs index 898767b345..9105c6f843 100644 --- a/crates/api_common/src/community.rs +++ b/crates/api_common/src/community.rs @@ -177,12 +177,6 @@ pub struct EditCommunity { /// A shorter, one line description of your community. #[cfg_attr(feature = "full", ts(optional))] pub description: Option, - /// An icon URL. - #[cfg_attr(feature = "full", ts(optional))] - pub icon: Option, - /// A banner URL. - #[cfg_attr(feature = "full", ts(optional))] - pub banner: Option, /// Whether its an NSFW community. #[cfg_attr(feature = "full", ts(optional))] pub nsfw: Option, diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index a03e6598f3..1691b93de6 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -9,13 +9,10 @@ use activitypub_federation::config::Data; use chrono::{DateTime, Utc}; use encoding_rs::{Encoding, UTF_8}; use futures::StreamExt; -use lemmy_db_schema::{ - newtypes::DbUrl, - source::{ - images::{ImageDetailsForm, LocalImage, LocalImageForm}, - post::{Post, PostUpdateForm}, - site::Site, - }, +use lemmy_db_schema::source::{ + images::{ImageDetailsForm, LocalImage, LocalImageForm}, + post::{Post, PostUpdateForm}, + site::Site, }; use lemmy_utils::{ error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult}, @@ -472,30 +469,6 @@ async fn is_image_content_type(client: &ClientWithMiddleware, url: &Url) -> Lemm } } -/// When adding a new avatar, banner or similar image, delete the old one. -/// TODO: remove this function -pub async fn replace_image( - new_image: &Option>, - old_image: &Option, - context: &Data, -) -> LemmyResult<()> { - if let (Some(Some(new_image)), Some(old_image)) = (new_image, old_image) { - // Note: Oftentimes front ends will include the current image in the form. - // In this case, deleting `old_image` would also be deletion of `new_image`, - // so the deletion must be skipped for the image to be kept. - if new_image != old_image { - // Ignore errors because image may be stored externally. - let image = LocalImage::delete_by_url(&mut context.pool(), old_image) - .await - .ok(); - if let Some(image) = image { - delete_image_from_pictrs(&image.pictrs_alias, &image.pictrs_delete_token, context).await?; - } - } - } - Ok(()) -} - #[cfg(test)] mod tests { diff --git a/crates/api_common/src/site.rs b/crates/api_common/src/site.rs index 0b1fb2200e..7ccf06ef5f 100644 --- a/crates/api_common/src/site.rs +++ b/crates/api_common/src/site.rs @@ -201,10 +201,6 @@ pub struct CreateSite { #[cfg_attr(feature = "full", ts(optional))] pub description: Option, #[cfg_attr(feature = "full", ts(optional))] - pub icon: Option, - #[cfg_attr(feature = "full", ts(optional))] - pub banner: Option, - #[cfg_attr(feature = "full", ts(optional))] pub enable_nsfw: Option, #[cfg_attr(feature = "full", ts(optional))] pub community_creation_admin_only: Option, @@ -298,12 +294,6 @@ pub struct EditSite { /// A shorter, one line description of your site. #[cfg_attr(feature = "full", ts(optional))] pub description: Option, - /// A url for your site's icon. - #[cfg_attr(feature = "full", ts(optional))] - pub icon: Option, - /// A url for your site's banner. - #[cfg_attr(feature = "full", ts(optional))] - pub banner: Option, /// Whether to enable NSFW. #[cfg_attr(feature = "full", ts(optional))] pub enable_nsfw: Option, diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 27b7ad5311..8b73fb6e82 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -1131,31 +1131,6 @@ pub async fn proxy_image_link(link: Url, context: &LemmyContext) -> LemmyResult< proxy_image_link_internal(link, context.settings().pictrs()?.image_mode(), context).await } -pub async fn proxy_image_link_opt_api( - link: Option>, - context: &LemmyContext, -) -> LemmyResult>> { - if let Some(Some(link)) = link { - proxy_image_link(link.into(), context) - .await - .map(Some) - .map(Some) - } else { - Ok(link) - } -} - -pub async fn proxy_image_link_api( - link: Option, - context: &LemmyContext, -) -> LemmyResult> { - if let Some(link) = link { - proxy_image_link(link.into(), context).await.map(Some) - } else { - Ok(link) - } -} - pub async fn proxy_image_link_opt_apub( link: Option, context: &LemmyContext, diff --git a/crates/api_crud/src/community/update.rs b/crates/api_crud/src/community/update.rs index d9c062c531..944f5bade5 100644 --- a/crates/api_crud/src/community/update.rs +++ b/crates/api_crud/src/community/update.rs @@ -6,14 +6,12 @@ use lemmy_api_common::{ build_response::build_community_response, community::{CommunityResponse, EditCommunity}, context::LemmyContext, - request::replace_image, send_activity::{ActivityChannel, SendActivityData}, utils::{ check_community_mod_action, get_url_blocklist, local_site_to_slur_regex, process_markdown_opt, - proxy_image_link_opt_api, }, }; use lemmy_db_schema::{ @@ -23,7 +21,7 @@ use lemmy_db_schema::{ local_site::LocalSite, }, traits::Crud, - utils::{diesel_string_update, diesel_url_update}, + utils::diesel_string_update, }; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::{ @@ -58,14 +56,6 @@ pub async fn update_community( let old_community = Community::read(&mut context.pool(), data.community_id).await?; - let icon = diesel_url_update(data.icon.as_deref())?; - replace_image(&icon, &old_community.icon, &context).await?; - let icon = proxy_image_link_opt_api(icon, &context).await?; - - let banner = diesel_url_update(data.banner.as_deref())?; - replace_image(&banner, &old_community.banner, &context).await?; - let banner = proxy_image_link_opt_api(banner, &context).await?; - // Verify its a mod (only mods can edit it) check_community_mod_action( &local_user_view.person, @@ -91,8 +81,6 @@ pub async fn update_community( title: data.title.clone(), sidebar, description, - icon, - banner, nsfw: data.nsfw, posting_restricted_to_mods: data.posting_restricted_to_mods, visibility: data.visibility, diff --git a/crates/api_crud/src/site/create.rs b/crates/api_crud/src/site/create.rs index c8140cc280..34965742d9 100644 --- a/crates/api_crud/src/site/create.rs +++ b/crates/api_crud/src/site/create.rs @@ -13,7 +13,6 @@ use lemmy_api_common::{ local_site_rate_limit_to_rate_limit_config, local_site_to_slur_regex, process_markdown_opt, - proxy_image_link_api, }, }; use lemmy_db_schema::{ @@ -24,7 +23,7 @@ use lemmy_db_schema::{ site::{Site, SiteUpdateForm}, }, traits::Crud, - utils::{diesel_string_update, diesel_url_create}, + utils::diesel_string_update, }; use lemmy_db_views::structs::{LocalUserView, SiteView}; use lemmy_utils::{ @@ -63,18 +62,10 @@ pub async fn create_site( let url_blocklist = get_url_blocklist(&context).await?; let sidebar = process_markdown_opt(&data.sidebar, &slur_regex, &url_blocklist, &context).await?; - let icon = diesel_url_create(data.icon.as_deref())?; - let icon = proxy_image_link_api(icon, &context).await?; - - let banner = diesel_url_create(data.banner.as_deref())?; - let banner = proxy_image_link_api(banner, &context).await?; - let site_form = SiteUpdateForm { name: Some(data.name.clone()), sidebar: diesel_string_update(sidebar.as_deref()), description: diesel_string_update(data.description.as_deref()), - icon: Some(icon), - banner: Some(banner), actor_id: Some(actor_id), last_refreshed_at: Some(Utc::now()), inbox_url, diff --git a/crates/api_crud/src/site/update.rs b/crates/api_crud/src/site/update.rs index d2585ea43b..40b51208ea 100644 --- a/crates/api_crud/src/site/update.rs +++ b/crates/api_crud/src/site/update.rs @@ -5,7 +5,6 @@ use actix_web::web::Json; use chrono::Utc; use lemmy_api_common::{ context::LemmyContext, - request::replace_image, site::{EditSite, SiteResponse}, utils::{ get_url_blocklist, @@ -13,7 +12,6 @@ use lemmy_api_common::{ local_site_rate_limit_to_rate_limit_config, local_site_to_slur_regex, process_markdown_opt, - proxy_image_link_opt_api, }, }; use lemmy_db_schema::{ @@ -26,7 +24,7 @@ use lemmy_db_schema::{ site::{Site, SiteUpdateForm}, }, traits::Crud, - utils::{diesel_string_update, diesel_url_update}, + utils::diesel_string_update, RegistrationMode, }; use lemmy_db_views::structs::{LocalUserView, SiteView}; @@ -72,20 +70,10 @@ pub async fn update_site( .as_deref(), ); - let icon = diesel_url_update(data.icon.as_deref())?; - replace_image(&icon, &site.icon, &context).await?; - let icon = proxy_image_link_opt_api(icon, &context).await?; - - let banner = diesel_url_update(data.banner.as_deref())?; - replace_image(&banner, &site.banner, &context).await?; - let banner = proxy_image_link_opt_api(banner, &context).await?; - let site_form = SiteUpdateForm { name: data.name.clone(), sidebar, description: diesel_string_update(data.description.as_deref()), - icon, - banner, content_warning: diesel_string_update(data.content_warning.as_deref()), updated: Some(Some(Utc::now())), ..Default::default() diff --git a/crates/routes/src/images/upload.rs b/crates/routes/src/images/upload.rs index 8774216411..9fd20b3ef0 100644 --- a/crates/routes/src/images/upload.rs +++ b/crates/routes/src/images/upload.rs @@ -4,7 +4,7 @@ use lemmy_api_common::{ context::LemmyContext, image::{CommunityIdQuery, UploadImageResponse}, request::PictrsResponse, - utils::is_mod_or_admin, + utils::{is_admin, is_mod_or_admin}, LemmyErrorType, SuccessResponse, }; @@ -13,6 +13,7 @@ use lemmy_db_schema::{ community::{Community, CommunityUpdateForm}, images::{LocalImage, LocalImageForm}, person::{Person, PersonUpdateForm}, + site::{Site, SiteUpdateForm}, }, traits::Crud, }; @@ -112,7 +113,7 @@ pub async fn upload_community_banner( is_mod_or_admin(&mut context.pool(), &local_user_view.person, community.id).await?; let image = do_upload_image(req, body, Banner, &local_user_view, &context).await?; - delete_old_image(&community.icon, &context).await?; + delete_old_image(&community.banner, &context).await?; let form = CommunityUpdateForm { banner: Some(Some(image.image_url.into())), @@ -123,6 +124,48 @@ pub async fn upload_community_banner( Ok(Json(SuccessResponse::default())) } +pub async fn upload_site_icon( + req: HttpRequest, + body: Payload, + local_user_view: LocalUserView, + context: Data, +) -> LemmyResult> { + is_admin(&local_user_view)?; + let site = Site::read_local(&mut context.pool()).await?; + + let image = do_upload_image(req, body, Avatar, &local_user_view, &context).await?; + delete_old_image(&site.icon, &context).await?; + + let form = SiteUpdateForm { + icon: Some(Some(image.image_url.into())), + ..Default::default() + }; + Site::update(&mut context.pool(), site.id, &form).await?; + + Ok(Json(SuccessResponse::default())) +} + +pub async fn upload_site_banner( + req: HttpRequest, + body: Payload, + local_user_view: LocalUserView, + context: Data, +) -> LemmyResult> { + is_admin(&local_user_view)?; + let site = Site::read_local(&mut context.pool()).await?; + + let image = do_upload_image(req, body, Banner, &local_user_view, &context).await?; + delete_old_image(&site.banner, &context).await?; + + let form = SiteUpdateForm { + banner: Some(Some(image.image_url.into())), + ..Default::default() + }; + Site::update(&mut context.pool(), site.id, &form).await?; + + Ok(Json(SuccessResponse::default())) +} + pub async fn do_upload_image( req: HttpRequest, body: Payload, diff --git a/src/api_routes_v4.rs b/src/api_routes_v4.rs index c7f0dd3548..86b35312aa 100644 --- a/src/api_routes_v4.rs +++ b/src/api_routes_v4.rs @@ -166,6 +166,8 @@ use lemmy_routes::images::{ upload_community_banner, upload_community_icon, upload_image, + upload_site_banner, + upload_site_icon, upload_user_avatar, upload_user_banner, }, @@ -181,7 +183,9 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) { scope("/site") .route("", get().to(get_site_v4)) .route("", post().to(create_site)) - .route("", put().to(update_site)), + .route("", put().to(update_site)) + .route("/icon", post().to(upload_site_icon)) + .route("/banner", post().to(upload_site_banner)), ) .route("/modlog", get().to(get_mod_log)) .service( From 4951aa0fd171554321940997942c95b510bb1887 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Wed, 18 Dec 2024 11:55:37 +0100 Subject: [PATCH 16/16] update js client --- api_tests/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_tests/package.json b/api_tests/package.json index f4c6dd2850..4939d8c82f 100644 --- a/api_tests/package.json +++ b/api_tests/package.json @@ -28,7 +28,7 @@ "eslint": "^9.14.0", "eslint-plugin-prettier": "^5.1.3", "jest": "^29.5.0", - "lemmy-js-client": "0.20.0-image-api-rework.5", + "lemmy-js-client": "0.20.0-image-api-rework.6", "prettier": "^3.2.5", "ts-jest": "^29.1.0", "typescript": "^5.5.4",