Fixing proxy images (#4722)

* Adding an image_details table to store image dimensions. - Adds an image_details table, which stores the height, width, and content_type for local and remote images. - For LocalImages, this information already comes back with the upload. - For RemoteImages, it calls the pictrs details endpoint. - Fixed some issues with proxying non-image urls. - Fixes #3328 - Also fixes #4703 * Running sql format. * Running fmt. * Don't fetch metadata in background for local API requests. * Dont export remote_image table to typescript. * Cleaning up validate. * Dont proxy url. * Fixing tests, fixing issue with federated thumbnails. * Fix tests. * Updating corepack, fixing issue. * Refactoring image inserts to use transactions. * Use select exists again. * Fixing imports. * Fix test. * Removing pointless backgrounded metadata generation version. * Removing public pictrs details route. * Fixing clippy. * Fixing proxy image fetching. Fixes #4703 - This extracts only the proxy image fixes from #4704, leaving off thumbnails. * Fix test. * Addressing PR comments. * Address PR comments 2. --------- Co-authored-by: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com>
2024-11-25 05:50:19 +00:00 · 2024-05-22 04:28:47 -04:00 · 2024-05-22 04:28:47 -04:00 · 55f84dd38a
commit 55f84dd38a
parent 6b46a70535
17 changed files with 220 additions and 212 deletions
--- a/api_tests/package.json
+++ b/api_tests/package.json
@ -6,11 +6,11 @@
  "repository": "https://github.com/LemmyNet/lemmy",
  "author": "Dessalines",
  "license": "AGPL-3.0",
-  "packageManager": "pnpm@9.0.6",
+  "packageManager": "pnpm@9.1.1+sha256.9551e803dcb7a1839fdf5416153a844060c7bce013218ce823410532504ac10b",
  "scripts": {
    "lint": "tsc --noEmit && eslint --report-unused-disable-directives --ext .js,.ts,.tsx src && prettier --check 'src/**/*.ts'",
    "fix": "prettier --write src && eslint --fix src",
-    "api-test": "jest -i follow.spec.ts && jest -i post.spec.ts && jest -i comment.spec.ts && jest -i private_message.spec.ts && jest -i user.spec.ts && jest -i community.spec.ts && jest -i image.spec.ts",
+    "api-test": "jest -i follow.spec.ts && jest -i image.spec.ts && jest -i user.spec.ts && jest -i private_message.spec.ts && jest -i community.spec.ts &&  jest -i post.spec.ts && jest -i comment.spec.ts ",
    "api-test-follow": "jest -i follow.spec.ts",
    "api-test-comment": "jest -i comment.spec.ts",
    "api-test-post": "jest -i post.spec.ts",
--- a/api_tests/prepare-drone-federation-test.sh
+++ b/api_tests/prepare-drone-federation-test.sh
@ -15,7 +15,7 @@ export LEMMY_TEST_FAST_FEDERATION=1 # by default, the persistent federation queu

 # pictrs setup
 if [ ! -f "api_tests/pict-rs" ]; then
-  curl "https://git.asonix.dog/asonix/pict-rs/releases/download/v0.5.0-beta.2/pict-rs-linux-amd64" -o api_tests/pict-rs
+  curl "https://git.asonix.dog/asonix/pict-rs/releases/download/v0.5.13/pict-rs-linux-amd64" -o api_tests/pict-rs
  chmod +x api_tests/pict-rs
 fi
 ./api_tests/pict-rs \
--- a/api_tests/src/comment.spec.ts
+++ b/api_tests/src/comment.spec.ts
@ -45,7 +45,6 @@ let postOnAlphaRes: PostResponse;

 beforeAll(async () => {
  await setupLogins();
-  await unfollows();
  await Promise.all([followBeta(alpha), followBeta(gamma)]);
  betaCommunity = (await resolveBetaCommunity(alpha)).community;
  if (betaCommunity) {
--- a/api_tests/src/community.spec.ts
+++ b/api_tests/src/community.spec.ts
@ -380,8 +380,8 @@ test("User blocks instance, communities are hidden", async () => {
 test("Community follower count is federated", async () => {
  // Follow the beta community from alpha
  let community = await createCommunity(beta);
-  let community_id = community.community_view.community.actor_id;
-  let resolved = await resolveCommunity(alpha, community_id);
+  let communityActorId = community.community_view.community.actor_id;
+  let resolved = await resolveCommunity(alpha, communityActorId);
  if (!resolved.community) {
    throw "Missing beta community";
  }
@ -389,7 +389,7 @@ test("Community follower count is federated", async () => {
  await followCommunity(alpha, true, resolved.community.community.id);
  let followed = (
    await waitUntil(
-      () => resolveCommunity(alpha, community_id),
+      () => resolveCommunity(alpha, communityActorId),
      c => c.community?.subscribed === "Subscribed",
    )
  ).community;
@ -398,7 +398,7 @@ test("Community follower count is federated", async () => {
  expect(followed?.counts.subscribers).toBe(1);

  // Follow the community from gamma
-  resolved = await resolveCommunity(gamma, community_id);
+  resolved = await resolveCommunity(gamma, communityActorId);
  if (!resolved.community) {
    throw "Missing beta community";
  }
@ -406,7 +406,7 @@ test("Community follower count is federated", async () => {
  await followCommunity(gamma, true, resolved.community.community.id);
  followed = (
    await waitUntil(
-      () => resolveCommunity(gamma, community_id),
+      () => resolveCommunity(gamma, communityActorId),
      c => c.community?.subscribed === "Subscribed",
    )
  ).community;
@ -415,7 +415,7 @@ test("Community follower count is federated", async () => {
  expect(followed?.counts?.subscribers).toBe(2);

  // Follow the community from delta
-  resolved = await resolveCommunity(delta, community_id);
+  resolved = await resolveCommunity(delta, communityActorId);
  if (!resolved.community) {
    throw "Missing beta community";
  }
@ -423,7 +423,7 @@ test("Community follower count is federated", async () => {
  await followCommunity(delta, true, resolved.community.community.id);
  followed = (
    await waitUntil(
-      () => resolveCommunity(delta, community_id),
+      () => resolveCommunity(delta, communityActorId),
      c => c.community?.subscribed === "Subscribed",
    )
  ).community;
--- a/api_tests/src/image.spec.ts
+++ b/api_tests/src/image.spec.ts
@ -29,14 +29,16 @@ import {
  unfollows,
  getPost,
  waitUntil,
-  randomString,
  createPostWithThumbnail,
+  sampleImage,
 } from "./shared";
 const downloadFileSync = require("download-file-sync");

 beforeAll(setupLogins);

-afterAll(unfollows);
+afterAll(async () => {
+  await Promise.all([unfollows(), deleteAllImages(alpha)]);
+});

 test("Upload image and delete it", async () => {
  // Before running this test, you need to delete all previous images in the DB
@ -159,7 +161,6 @@ test("Purge post, linked image removed", async () => {
  expect(post.post_view.post.url).toBe(upload.url);

  // purge post
-
  const purgeForm: PurgePost = {
    post_id: post.post_view.post.id,
  };
@ -183,13 +184,13 @@ test("Images in remote post are proxied if setting enabled", async () => {
    gamma,
    community.community_view.community.id,
    upload.url,
-    "![](http://example.com/image2.png)",
+    `![](${sampleImage})`,
  );
  expect(post.post_view.post).toBeDefined();

  // remote image gets proxied after upload
  expect(
-    post.post_view.post.url?.startsWith(
+    post.post_view.post.thumbnail_url?.startsWith(
      "http://lemmy-gamma:8561/api/v3/image_proxy?url",
    ),
  ).toBeTruthy();
@ -202,14 +203,20 @@ test("Images in remote post are proxied if setting enabled", async () => {
  let epsilonPost = await resolvePost(epsilon, post.post_view.post);
  expect(epsilonPost.post).toBeDefined();

-  // remote image gets proxied after federation
+  // Fetch the post again, the metadata should be backgrounded now
+  // Wait for the metadata to get fetched, since this is backgrounded now
+  let epsilonPost2 = await waitUntil(
+    () => getPost(epsilon, epsilonPost.post!.post.id),
+    p => p.post_view.post.thumbnail_url != undefined,
+  );
+
  expect(
-    epsilonPost.post!.post.url?.startsWith(
+    epsilonPost2.post_view.post.thumbnail_url?.startsWith(
      "http://lemmy-epsilon:8581/api/v3/image_proxy?url",
    ),
  ).toBeTruthy();
  expect(
-    epsilonPost.post!.post.body?.startsWith(
+    epsilonPost2.post_view.post.body?.startsWith(
      "![](http://lemmy-epsilon:8581/api/v3/image_proxy?url",
    ),
  ).toBeTruthy();
@ -232,7 +239,7 @@ test("No image proxying if setting is disabled", async () => {
    alpha,
    community.community_view.community.id,
    upload.url,
-    "![](http://example.com/image2.png)",
+    `![](${sampleImage})`,
  );
  expect(post.post_view.post).toBeDefined();

@ -240,7 +247,7 @@ test("No image proxying if setting is disabled", async () => {
  expect(
    post.post_view.post.url?.startsWith("http://127.0.0.1:8551/pictrs/image/"),
  ).toBeTruthy();
-  expect(post.post_view.post.body).toBe("![](http://example.com/image2.png)");
+  expect(post.post_view.post.body).toBe(`![](${sampleImage})`);

  let betaPost = await waitForPost(
    beta,
@ -253,8 +260,7 @@ test("No image proxying if setting is disabled", async () => {
  expect(
    betaPost.post.url?.startsWith("http://127.0.0.1:8551/pictrs/image/"),
  ).toBeTruthy();
-  expect(betaPost.post.body).toBe("![](http://example.com/image2.png)");
-
+  expect(betaPost.post.body).toBe(`![](${sampleImage})`);
  // Make sure the alt text got federated
  expect(post.post_view.post.alt_text).toBe(betaPost.post.alt_text);
 });
--- a/api_tests/src/post.spec.ts
+++ b/api_tests/src/post.spec.ts
@ -48,7 +48,6 @@ beforeAll(async () => {
  await setupLogins();
  betaCommunity = (await resolveBetaCommunity(alpha)).community;
  expect(betaCommunity).toBeDefined();
-  await unfollows();
 });

 afterAll(unfollows);
@ -83,10 +82,7 @@ async function assertPostFederation(postOne: PostView, postTwo: PostView) {

 test("Create a post", async () => {
  // Setup some allowlists and blocklists
-  let editSiteForm: EditSite = {
-    allowed_instances: ["lemmy-beta"],
-  };
-  await delta.editSite(editSiteForm);
+  const editSiteForm: EditSite = {};

  editSiteForm.allowed_instances = [];
  editSiteForm.blocked_instances = ["lemmy-alpha"];
@ -749,7 +745,7 @@ test("Block post that contains banned URL", async () => {

  await epsilon.editSite(editSiteForm);

-  await delay(500);
+  await delay();

  if (!betaCommunity) {
    throw "Missing beta community";
--- a/api_tests/src/shared.ts
+++ b/api_tests/src/shared.ts
@ -81,6 +81,8 @@ import { ListingType } from "lemmy-js-client/dist/types/ListingType";

 export const fetchFunction = fetch;
 export const imageFetchLimit = 50;
+export const sampleImage =
+  "https://i.pinimg.com/originals/df/5f/5b/df5f5b1b174a2b4b6026cc6c8f9395c1.jpg";

 export let alphaUrl = "http://127.0.0.1:8541";
 export let betaUrl = "http://127.0.0.1:8551";
@ -180,6 +182,10 @@ export async function setupLogins() {
  ];
  await gamma.editSite(editSiteForm);

+  // Setup delta allowed instance
+  editSiteForm.allowed_instances = ["lemmy-beta"];
+  await delta.editSite(editSiteForm);
+
  // Create the main alpha/beta communities
  // Ignore thrown errors of duplicates
  try {
@ -693,8 +699,8 @@ export async function saveUserSettingsBio(
 export async function saveUserSettingsFederated(
  api: LemmyHttp,
 ): Promise<SuccessResponse> {
-  let avatar = "https://image.flaticon.com/icons/png/512/35/35896.png";
-  let banner = "https://image.flaticon.com/icons/png/512/36/35896.png";
+  let avatar = sampleImage;
+  let banner = sampleImage;
  let bio = "a changed bio";
  let form: SaveUserSettings = {
    show_nsfw: false,
@ -760,6 +766,7 @@ export async function unfollowRemotes(
  await Promise.all(
    remoteFollowed.map(cu => followCommunity(api, false, cu.community.id)),
  );
+
  let siteRes = await getSite(api);
  return siteRes;
 }
@ -889,14 +896,17 @@ export async function deleteAllImages(api: LemmyHttp) {
    limit: imageFetchLimit,
  });
  imagesRes.images;
-
-  for (const image of imagesRes.images) {
+  Promise.all(
+    imagesRes.images
+      .map(image => {
        const form: DeleteImage = {
          token: image.local_image.pictrs_delete_token,
          filename: image.local_image.pictrs_alias,
        };
-    await api.deleteImage(form);
-  }
+        return form;
+      })
+      .map(form => api.deleteImage(form)),
+  );
 }

 export async function unfollows() {
@ -907,6 +917,24 @@ export async function unfollows() {
    unfollowRemotes(delta),
    unfollowRemotes(epsilon),
  ]);
+  await Promise.all([
+    purgeAllPosts(alpha),
+    purgeAllPosts(beta),
+    purgeAllPosts(gamma),
+    purgeAllPosts(delta),
+    purgeAllPosts(epsilon),
+  ]);
+}
+
+export async function purgeAllPosts(api: LemmyHttp) {
+  // The best way to get all federated items, is to find the posts
+  let res = await api.getPosts({ type_: "All", limit: 50 });
+  await Promise.all(
+    Array.from(new Set(res.posts.map(p => p.post.id)))
+      .map(post_id => api.purgePost({ post_id }))
+      // Ignore errors
+      .map(p => p.catch(e => e)),
+  );
 }

 export function getCommentParentId(comment: Comment): number | undefined {
--- a/crates/api_common/src/request.rs
+++ b/crates/api_common/src/request.rs
@ -3,9 +3,10 @@ use crate::{
  lemmy_db_schema::traits::Crud,
  post::{LinkMetadata, OpenGraphData},
  send_activity::{ActivityChannel, SendActivityData},
-  utils::{local_site_opt_to_sensitive, proxy_image_link, proxy_image_link_opt_apub},
+  utils::{local_site_opt_to_sensitive, proxy_image_link},
 };
 use activitypub_federation::config::Data;
+use chrono::{DateTime, Utc};
 use encoding_rs::{Encoding, UTF_8};
 use lemmy_db_schema::{
  newtypes::DbUrl,
@ -18,14 +19,13 @@ use lemmy_db_schema::{
 use lemmy_utils::{
  error::{LemmyError, LemmyErrorType, LemmyResult},
  settings::structs::{PictrsImageMode, Settings},
-  spawn_try_task,
  REQWEST_TIMEOUT,
  VERSION,
 };
 use mime::Mime;
 use reqwest::{header::CONTENT_TYPE, Client, ClientBuilder};
 use reqwest_middleware::ClientWithMiddleware;
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use tracing::info;
 use url::Url;
 use urlencoding::encode;
@ -65,22 +65,20 @@ pub async fn fetch_link_metadata(url: &Url, context: &LemmyContext) -> LemmyResu
  })
 }

-/// Generate post thumbnail in background task, because some sites can be very slow to respond.
+/// Generates and saves a post thumbnail and metadata.
 ///
 /// Takes a callback to generate a send activity task, so that post can be federated with metadata.
 ///
 /// TODO: `federated_thumbnail` param can be removed once we federate full metadata and can
 ///       write it to db directly, without calling this function.
 ///       https://github.com/LemmyNet/lemmy/issues/4598
-pub fn generate_post_link_metadata(
+pub async fn generate_post_link_metadata(
  post: Post,
  custom_thumbnail: Option<Url>,
-  federated_thumbnail: Option<Url>,
  send_activity: impl FnOnce(Post) -> Option<SendActivityData> + Send + 'static,
  local_site: Option<LocalSite>,
  context: Data<LemmyContext>,
-) {
-  spawn_try_task(async move {
+) -> LemmyResult<()> {
  let metadata = match &post.url {
    Some(url) => fetch_link_metadata(url, &context).await.unwrap_or_default(),
    _ => Default::default(),
@ -95,38 +93,28 @@ pub fn generate_post_link_metadata(
  let allow_sensitive = local_site_opt_to_sensitive(&local_site);
  let allow_generate_thumbnail = allow_sensitive || !post.nsfw;

-    // Use custom thumbnail if available and its not an image post
-    let thumbnail_url = if !is_image_post && custom_thumbnail.is_some() {
-      custom_thumbnail
-    }
-    // Use federated thumbnail if available
-    else if federated_thumbnail.is_some() {
-      federated_thumbnail
-    }
-    // Generate local thumbnail if allowed
-    else if allow_generate_thumbnail {
-      match post
-        .url
-        .filter(|_| is_image_post)
-        .or(metadata.opengraph_data.image)
-      {
-        Some(url) => generate_pictrs_thumbnail(&url, &context).await.ok(),
-        None => None,
-      }
-    }
-    // Otherwise use opengraph preview image directly
-    else {
-      metadata.opengraph_data.image.map(Into::into)
+  let image_url = if is_image_post {
+    post.url
+  } else {
+    metadata.opengraph_data.image.clone()
  };

-    // Proxy the image fetch if necessary
-    let proxied_thumbnail_url = proxy_image_link_opt_apub(thumbnail_url, &context).await?;
+  let thumbnail_url = if let (false, Some(url)) = (is_image_post, custom_thumbnail) {
+    proxy_image_link(url, &context).await.ok()
+  } else if let (true, Some(url)) = (allow_generate_thumbnail, image_url) {
+    generate_pictrs_thumbnail(&url, &context)
+      .await
+      .ok()
+      .map(Into::into)
+  } else {
+    metadata.opengraph_data.image.clone()
+  };

  let form = PostUpdateForm {
    embed_title: Some(metadata.opengraph_data.title),
    embed_description: Some(metadata.opengraph_data.description),
    embed_video_url: Some(metadata.opengraph_data.embed_video_url),
-      thumbnail_url: Some(proxied_thumbnail_url),
+    thumbnail_url: Some(thumbnail_url),
    url_content_type: Some(metadata.content_type),
    ..Default::default()
  };
@ -135,25 +123,12 @@ pub fn generate_post_link_metadata(
    ActivityChannel::submit_activity(send_activity, &context).await?;
  }
  Ok(())
-  });
 }

 /// Extract site metadata from HTML Opengraph attributes.
 fn extract_opengraph_data(html_bytes: &[u8], url: &Url) -> LemmyResult<OpenGraphData> {
  let html = String::from_utf8_lossy(html_bytes);

-  // Make sure the first line is doctype html
-  let first_line = html
-    .trim_start()
-    .lines()
-    .next()
-    .ok_or(LemmyErrorType::NoLinesInHtml)?
-    .to_lowercase();
-
-  if !first_line.starts_with("<!doctype html") {
-    Err(LemmyErrorType::SiteMetadataPageIsNotDoctypeHtml)?
-  }
-
  let mut page = HTML::from_string(html.to_string(), None)?;

  // If the web page specifies that it isn't actually UTF-8, re-decode the received bytes with the
@ -201,19 +176,40 @@ fn extract_opengraph_data(html_bytes: &[u8], url: &Url) -> LemmyResult<OpenGraph
  })
 }

-#[derive(Deserialize, Debug)]
-struct PictrsResponse {
-  files: Vec<PictrsFile>,
-  msg: String,
+#[derive(Deserialize, Serialize, Debug)]
+pub struct PictrsResponse {
+  pub files: Option<Vec<PictrsFile>>,
+  pub msg: String,
 }

-#[derive(Deserialize, Debug)]
-struct PictrsFile {
-  file: String,
-  delete_token: String,
+#[derive(Deserialize, Serialize, Debug)]
+pub struct PictrsFile {
+  pub file: String,
+  pub delete_token: String,
+  pub details: PictrsFileDetails,
 }

-#[derive(Deserialize, Debug)]
+impl PictrsFile {
+  pub fn thumbnail_url(&self, protocol_and_hostname: &str) -> Result<Url, url::ParseError> {
+    Url::parse(&format!(
+      "{protocol_and_hostname}/pictrs/image/{}",
+      self.file
+    ))
+  }
+}
+
+/// Stores extra details about a Pictrs image.
+#[derive(Deserialize, Serialize, Debug)]
+pub struct PictrsFileDetails {
+  /// In pixels
+  pub width: u16,
+  /// In pixels
+  pub height: u16,
+  pub content_type: String,
+  pub created_at: DateTime<Utc>,
+}
+
+#[derive(Deserialize, Serialize, Debug)]
 struct PictrsPurgeResponse {
  msg: String,
 }
@ -295,33 +291,34 @@ async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> L
    encode(image_url.as_str())
  );

-  let response = context
+  let res = context
    .client()
    .get(&fetch_url)
    .timeout(REQWEST_TIMEOUT)
    .send()
+    .await?
+    .json::<PictrsResponse>()
    .await?;

-  let response: PictrsResponse = response.json().await?;
+  let files = res.files.unwrap_or_default();
+
+  let image = files
+    .first()
+    .ok_or(LemmyErrorType::PictrsResponseError(res.msg))?;

-  if response.msg == "ok" {
-    let thumbnail_url = Url::parse(&format!(
-      "{}/pictrs/image/{}",
-      context.settings().get_protocol_and_hostname(),
-      response.files.first().expect("missing pictrs file").file
-    ))?;
-    for uploaded_image in response.files {
  let form = LocalImageForm {
+    // This is none because its an internal request.
+    // IE, a local user shouldn't get to delete the thumbnails for their link posts
    local_user_id: None,
-        pictrs_alias: uploaded_image.file.to_string(),
-        pictrs_delete_token: uploaded_image.delete_token.to_string(),
+    pictrs_alias: image.file.clone(),
+    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)?;
+
  LocalImage::create(&mut context.pool(), &form).await?;
-    }
+
  Ok(thumbnail_url)
-  } else {
-    Err(LemmyErrorType::PictrsResponseError(response.msg))?
-  }
 }

 // TODO: get rid of this by reading content type from db
--- a/crates/api_common/src/utils.rs
+++ b/crates/api_common/src/utils.rs
@ -965,13 +965,10 @@ async fn proxy_image_link_internal(
  if link.domain() == Some(&context.settings().hostname) {
    Ok(link.into())
  } else if image_mode == PictrsImageMode::ProxyAllImages {
-    let proxied = format!(
-      "{}/api/v3/image_proxy?url={}",
-      context.settings().get_protocol_and_hostname(),
-      encode(link.as_str())
-    );
+    let proxied = build_proxied_image_url(&link, &context.settings().get_protocol_and_hostname())?;
+
    RemoteImage::create(&mut context.pool(), vec![link]).await?;
-    Ok(Url::parse(&proxied)?.into())
+    Ok(proxied.into())
  } else {
    Ok(link.into())
  }
@ -1025,6 +1022,17 @@ pub async fn proxy_image_link_opt_apub(
  }
 }

+fn build_proxied_image_url(
+  link: &Url,
+  protocol_and_hostname: &str,
+) -> Result<Url, url::ParseError> {
+  Url::parse(&format!(
+    "{}/api/v3/image_proxy?url={}",
+    protocol_and_hostname,
+    encode(link.as_str())
+  ))
+}
+
 #[cfg(test)]
 #[allow(clippy::unwrap_used)]
 #[allow(clippy::indexing_slicing)]
--- a/crates/api_crud/src/post/create.rs
+++ b/crates/api_crud/src/post/create.rs
@ -14,7 +14,6 @@ use lemmy_api_common::{
    local_site_to_slur_regex,
    mark_post_as_read,
    process_markdown_opt,
-    proxy_image_link_opt_apub,
    EndpointType,
  },
 };
@ -75,7 +74,6 @@ pub async fn create_post(
  is_url_blocked(&url, &url_blocklist)?;
  check_url_scheme(&url)?;
  check_url_scheme(&custom_thumbnail)?;
-  let url = proxy_image_link_opt_apub(url, &context).await?;

  check_community_user_action(
    &local_user_view.person,
@ -125,7 +123,7 @@ pub async fn create_post(

  let post_form = PostInsertForm::builder()
    .name(data.name.trim().to_string())
-    .url(url)
+    .url(url.map(Into::into))
    .body(body)
    .alt_text(data.alt_text.clone())
    .community_id(data.community_id)
@ -159,11 +157,11 @@ pub async fn create_post(
  generate_post_link_metadata(
    updated_post.clone(),
    custom_thumbnail,
-    None,
    |post| Some(SendActivityData::CreatePost(post)),
    Some(local_site),
    context.reset_request_count(),
-  );
+  )
+  .await?;

  // They like their own post by default
  let person_id = local_user_view.person.id;
--- a/crates/api_crud/src/post/update.rs
+++ b/crates/api_crud/src/post/update.rs
@ -11,7 +11,6 @@ use lemmy_api_common::{
    get_url_blocklist,
    local_site_to_slur_regex,
    process_markdown_opt,
-    proxy_image_link_opt_apub,
  },
 };
 use lemmy_db_schema::{
@ -86,11 +85,6 @@ pub async fn update_post(
    Err(LemmyErrorType::NoPostEditAllowed)?
  }

-  let url = match url {
-    Some(url) => Some(proxy_image_link_opt_apub(Some(url), &context).await?),
-    _ => Default::default(),
-  };
-
  let language_id = data.language_id;
  CommunityLanguage::is_allowed_community_language(
    &mut context.pool(),
@ -101,7 +95,7 @@ pub async fn update_post(

  let post_form = PostUpdateForm {
    name: data.name.clone(),
-    url,
+    url: Some(url.map(Into::into)),
    body: diesel_option_overwrite(body),
    alt_text: diesel_option_overwrite(data.alt_text.clone()),
    nsfw: data.nsfw,
@ -118,11 +112,11 @@ pub async fn update_post(
  generate_post_link_metadata(
    updated_post.clone(),
    custom_thumbnail,
-    None,
    |post| Some(SendActivityData::UpdatePost(post)),
    Some(local_site),
    context.reset_request_count(),
-  );
+  )
+  .await?;

  build_post_response(
    context.deref(),
--- a/crates/apub/src/objects/post.rs
+++ b/crates/apub/src/objects/post.rs
@ -45,6 +45,7 @@ use lemmy_db_schema::{
 use lemmy_db_views_actor::structs::CommunityModeratorView;
 use lemmy_utils::{
  error::{LemmyError, LemmyErrorType, LemmyResult},
+  spawn_try_task,
  utils::{markdown::markdown_to_html, slurs::check_slurs_opt, validation::check_url_scheme},
 };
 use std::ops::Deref;
@ -255,15 +256,13 @@ impl Object for ApubPost {

    let timestamp = page.updated.or(page.published).unwrap_or_else(naive_now);
    let post = Post::insert_apub(&mut context.pool(), timestamp, &form).await?;
+    let post_ = post.clone();
+    let context_ = context.reset_request_count();

-    generate_post_link_metadata(
-      post.clone(),
-      None,
-      page.image.map(|i| i.url),
-      |_| None,
-      local_site,
-      context.reset_request_count(),
-    );
+    // Generates a post thumbnail in background task, because some sites can be very slow to respond.
+    spawn_try_task(async move {
+      generate_post_link_metadata(post_, None, |_| None, local_site, context_).await
+    });

    Ok(post.into())
  }
--- a/crates/db_schema/src/source/images.rs
+++ b/crates/db_schema/src/source/images.rs
@ -22,7 +22,7 @@ use typed_builder::TypedBuilder;
  diesel(belongs_to(crate::source::local_user::LocalUser))
 )]
 #[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))]
-#[cfg_attr(feature = "full", diesel(primary_key(local_user_id)))]
+#[cfg_attr(feature = "full", diesel(primary_key(pictrs_alias)))]
 pub struct LocalImage {
  pub local_user_id: Option<LocalUserId>,
  pub pictrs_alias: String,
@ -41,9 +41,11 @@ pub struct LocalImageForm {

 /// Stores all images which are hosted on remote domains. When attempting to proxy an image, it
 /// is checked against this table to avoid Lemmy being used as a general purpose proxy.
-#[derive(PartialEq, Eq, Debug, Clone)]
-#[cfg_attr(feature = "full", derive(Queryable, Identifiable))]
+#[skip_serializing_none]
+#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)]
+#[cfg_attr(feature = "full", derive(Queryable, Selectable, Identifiable))]
 #[cfg_attr(feature = "full", diesel(table_name = remote_image))]
+#[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))]
 pub struct RemoteImage {
  pub id: i32,
  pub link: DbUrl,
--- a/crates/db_views/src/structs.rs
+++ b/crates/db_views/src/structs.rs
@ -216,6 +216,7 @@ pub struct VoteView {
  pub score: i16,
 }

+#[skip_serializing_none]
 #[derive(Debug, Serialize, Deserialize, Clone)]
 #[cfg_attr(feature = "full", derive(TS, Queryable))]
 #[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))]
--- a/crates/routes/src/images.rs
+++ b/crates/routes/src/images.rs
@ -1,18 +1,16 @@
 use actix_web::{
  body::BodyStream,
-  error,
  http::{
    header::{HeaderName, ACCEPT_ENCODING, HOST},
    StatusCode,
  },
  web,
  web::Query,
-  Error,
  HttpRequest,
  HttpResponse,
 };
 use futures::stream::{Stream, StreamExt};
-use lemmy_api_common::context::LemmyContext;
+use lemmy_api_common::{context::LemmyContext, request::PictrsResponse};
 use lemmy_db_schema::source::{
  images::{LocalImage, LocalImageForm, RemoteImage},
  local_site::LocalSite,
@ -21,7 +19,7 @@ use lemmy_db_views::structs::LocalUserView;
 use lemmy_utils::{error::LemmyResult, rate_limit::RateLimitCell, REQWEST_TIMEOUT};
 use reqwest::Body;
 use reqwest_middleware::{ClientWithMiddleware, RequestBuilder};
-use serde::{Deserialize, Serialize};
+use serde::Deserialize;
 use std::time::Duration;
 use url::Url;
 use urlencoding::decode;
@ -43,20 +41,8 @@ pub fn config(
    .service(web::resource("/pictrs/image/delete/{token}/{filename}").route(web::get().to(delete)));
 }

-#[derive(Debug, Serialize, Deserialize)]
-struct Image {
-  file: String,
-  delete_token: String,
-}
-
-#[derive(Debug, Serialize, Deserialize)]
-struct Images {
-  msg: String,
-  files: Option<Vec<Image>>,
-}
-
 #[derive(Deserialize)]
-struct PictrsParams {
+struct PictrsGetParams {
  format: Option<String>,
  thumbnail: Option<i32>,
 }
@ -92,7 +78,7 @@ async fn upload(
  local_user_view: LocalUserView,
  client: web::Data<ClientWithMiddleware>,
  context: web::Data<LemmyContext>,
-) -> Result<HttpResponse, Error> {
+) -> LemmyResult<HttpResponse> {
  // TODO: check rate limit here
  let pictrs_config = context.settings().pictrs_config()?;
  let image_url = format!("{}image", pictrs_config.url);
@ -106,21 +92,18 @@ async fn upload(
    .timeout(Duration::from_secs(pictrs_config.upload_timeout))
    .body(Body::wrap_stream(make_send(body)))
    .send()
-    .await
-    .map_err(error::ErrorBadRequest)?;
+    .await?;

  let status = res.status();
-  let images = res.json::<Images>().await.map_err(error::ErrorBadRequest)?;
+  let images = res.json::<PictrsResponse>().await?;
  if let Some(images) = &images.files {
-    for uploaded_image in images {
+    for image in images {
      let form = LocalImageForm {
        local_user_id: Some(local_user_view.local_user.id),
-        pictrs_alias: uploaded_image.file.to_string(),
-        pictrs_delete_token: uploaded_image.delete_token.to_string(),
+        pictrs_alias: image.file.to_string(),
+        pictrs_delete_token: image.delete_token.to_string(),
      };
-      LocalImage::create(&mut context.pool(), &form)
-        .await
-        .map_err(error::ErrorBadRequest)?;
+      LocalImage::create(&mut context.pool(), &form).await?;
    }
  }

@ -129,16 +112,14 @@ async fn upload(

 async fn full_res(
  filename: web::Path<String>,
-  web::Query(params): web::Query<PictrsParams>,
+  web::Query(params): web::Query<PictrsGetParams>,
  req: HttpRequest,
  client: web::Data<ClientWithMiddleware>,
  context: web::Data<LemmyContext>,
  local_user_view: Option<LocalUserView>,
-) -> Result<HttpResponse, Error> {
+) -> LemmyResult<HttpResponse> {
  // block access to images if instance is private and unauthorized, public
-  let local_site = LocalSite::read(&mut context.pool())
-    .await
-    .map_err(error::ErrorBadRequest)?;
+  let local_site = LocalSite::read(&mut context.pool()).await?;
  if local_site.private_instance && local_user_view.is_none() {
    return Ok(HttpResponse::Unauthorized().finish());
  }
@ -169,7 +150,7 @@ async fn image(
  url: String,
  req: HttpRequest,
  client: &ClientWithMiddleware,
-) -> Result<HttpResponse, Error> {
+) -> LemmyResult<HttpResponse> {
  let mut client_req = adapt_request(&req, client, url);

  if let Some(addr) = req.head().peer_addr {
@ -180,7 +161,7 @@ async fn image(
    client_req = client_req.header("X-Forwarded-For", addr.to_string());
  }

-  let res = client_req.send().await.map_err(error::ErrorBadRequest)?;
+  let res = client_req.send().await?;

  if res.status() == StatusCode::NOT_FOUND {
    return Ok(HttpResponse::NotFound().finish());
@ -202,7 +183,7 @@ async fn delete(
  context: web::Data<LemmyContext>,
  // require login
  _local_user_view: LocalUserView,
-) -> Result<HttpResponse, Error> {
+) -> LemmyResult<HttpResponse> {
  let (token, file) = components.into_inner();

  let pictrs_config = context.settings().pictrs_config()?;
@ -214,11 +195,9 @@ async fn delete(
    client_req = client_req.header("X-Forwarded-For", addr.to_string());
  }

-  let res = client_req.send().await.map_err(error::ErrorBadRequest)?;
+  let res = client_req.send().await?;

-  LocalImage::delete_by_alias(&mut context.pool(), &file)
-    .await
-    .map_err(error::ErrorBadRequest)?;
+  LocalImage::delete_by_alias(&mut context.pool(), &file).await?;

  Ok(HttpResponse::build(res.status()).body(BodyStream::new(res.bytes_stream())))
 }
@ -230,6 +209,8 @@ pub struct ImageProxyParams {

 pub async fn image_proxy(
  Query(params): Query<ImageProxyParams>,
+  req: HttpRequest,
+  client: web::Data<ClientWithMiddleware>,
  context: web::Data<LemmyContext>,
 ) -> LemmyResult<HttpResponse> {
  let url = Url::parse(&decode(&params.url)?)?;
@ -240,9 +221,8 @@ pub async fn image_proxy(

  let pictrs_config = context.settings().pictrs_config()?;
  let url = format!("{}image/original?proxy={}", pictrs_config.url, &params.url);
-  let image_response = context.client().get(url).send().await?;

-  Ok(HttpResponse::Ok().streaming(image_response.bytes_stream()))
+  image(url, req, &client).await
 }

 fn make_send<S>(mut stream: S) -> impl Stream<Item = S::Item> + Send + Unpin + 'static
--- a/docker/docker-compose.yml
+++ b/docker/docker-compose.yml
@ -75,7 +75,7 @@ services:
    init: true

  pictrs:
-    image: asonix/pictrs:0.5.0-rc.2
+    image: asonix/pictrs:0.5.13
    # this needs to match the pictrs url in lemmy.hjson
    hostname: pictrs
    # we can set options to pictrs like this, here we set max. image size and forced format for conversion
--- a/docker/federation/docker-compose.yml
+++ b/docker/federation/docker-compose.yml
@ -49,7 +49,7 @@ services:

  pictrs:
    restart: always
-    image: asonix/pictrs:0.5.0-rc.2
+    image: asonix/pictrs:0.5.13
    user: 991:991
    volumes:
      - ./volumes/pictrs_alpha:/mnt:Z