From d252be2113d465b09ab10622a860f4d9ab622b04 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 13 Dec 2024 15:04:07 +0100 Subject: [PATCH] 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 7ea21d0ba..5ace77ecf 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 496606e6c..615a0ceca 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 f9eab5039..969a95b3e 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 a3478081a..fa22f271d 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 9e24407cd..14749e542 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 000000000..df033f285 --- /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 6e09d904d..e02df464a 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 5b4d85a5e..e337cbdec 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 21154c823..6daeec4cd 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 c5f6a9c2c..632aa30f2 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 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); - let processed_url = params.process_url(name, &pictrs_config.url); + if let Some(size) = params.max_size { + url = format!("{url}&thumbnail={size}",); + } + 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)); - } - - Ok(client_res.body(BodyStream::new(res.bytes_stream()))) -} - -#[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 28d144a99..94a807f78 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 1aef9f79b..9e561713c 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 ba509596e..94dd32264 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 56e3721d7..e7cb82883 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 e67c6d7d4..99ae86fbc 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 c28748775..ae060ec75 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,