address review

api_tests/src/image.spec.ts

@@ -129,7 +129,7 @@ test("Purge post, linked image removed", async () => {
   expect(content2).toBe("");
 });
 
-test("Images in remote post are proxied", async () => {
+test("Images in remote post are proxied if setting enabled", async () => {
   let user = await registerUser(beta, betaUrl);
   let community = await createCommunity(gamma);
 
@@ -137,7 +137,6 @@ test("Images in remote post are proxied", async () => {
     image: Buffer.from("test"),
   };
   const upload = await user.uploadImage(upload_form);
-  console.log(upload);
   let post = await createPost(
     gamma,
     community.community_view.community.id,
@@ -145,8 +144,8 @@ test("Images in remote post are proxied", async () => {
     "![](http://example.com/image2.png)",
   );
   expect(post.post_view.post).toBeDefined();
+
   // remote image gets proxied after upload
-  console.log(post.post_view.post);
   expect(
     post.post_view.post.url?.startsWith(
       "http://lemmy-gamma:8561/api/v3/image_proxy?url",
@@ -160,7 +159,7 @@ test("Images in remote post are proxied", async () => {
 
   let epsilonPost = await resolvePost(epsilon, post.post_view.post);
   expect(epsilonPost.post).toBeDefined();
-  console.log(epsilonPost.post);
+
   // remote image gets proxied after federation
   expect(
     epsilonPost.post!.post.url?.startsWith(
@@ -173,3 +172,35 @@ test("Images in remote post are proxied", async () => {
     ),
   ).toBeTruthy();
 });
+
+test("No image proxying if setting is disabled", async () => {
+  let user = await registerUser(beta, betaUrl);
+  let community = await createCommunity(alpha);
+
+  const upload_form: UploadImage = {
+    image: Buffer.from("test"),
+  };
+  const upload = await user.uploadImage(upload_form);
+  let post = await createPost(
+    alpha,
+    community.community_view.community.id,
+    upload.url,
+    "![](http://example.com/image2.png)",
+  );
+  expect(post.post_view.post).toBeDefined();
+
+  // remote image doesnt get proxied after upload
+  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)");
+
+  let gammaPost = await resolvePost(delta, post.post_view.post);
+  expect(gammaPost.post).toBeDefined();
+
+  // remote image doesnt get proxied after federation
+  expect(
+    gammaPost.post!.post.url?.startsWith("http://127.0.0.1:8551/pictrs/image/"),
+  ).toBeTruthy();
+  expect(gammaPost.post!.post.body).toBe("![](http://example.com/image2.png)");
+});

api_tests/src/post.spec.ts

@@ -39,7 +39,7 @@ import {
   loginUser,
 } from "./shared";
 import { PostView } from "lemmy-js-client/dist/types/PostView";
-import { ResolveObject } from "lemmy-js-client";
+import { EditSite, ResolveObject } from "lemmy-js-client";
 
 let betaCommunity: CommunityView | undefined;
 
@@ -72,6 +72,16 @@ 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);
+
+  editSiteForm.allowed_instances = [];
+  editSiteForm.blocked_instances = ["lemmy-alpha"];
+  await epsilon.editSite(editSiteForm);
+
   if (!betaCommunity) {
     throw "Missing beta community";
   }
@@ -104,6 +114,12 @@ test("Create a post", async () => {
   await expect(
     resolvePost(epsilon, postRes.post_view.post),
   ).rejects.toStrictEqual(Error("couldnt_find_object"));
+
+  // remove added allow/blocklists
+  editSiteForm.allowed_instances = [];
+  editSiteForm.blocked_instances = [];
+  await delta.editSite(editSiteForm);
+  await epsilon.editSite(editSiteForm);
 });
 
 test("Create a post in a non-existent community", async () => {

api_tests/src/shared.ts

@@ -177,13 +177,6 @@ export async function setupLogins() {
   ];
   await gamma.editSite(editSiteForm);
 
-  editSiteForm.allowed_instances = ["lemmy-beta"];
-  await delta.editSite(editSiteForm);
-
-  editSiteForm.allowed_instances = [];
-  editSiteForm.blocked_instances = ["lemmy-alpha"];
-  await epsilon.editSite(editSiteForm);
-
   // Create the main alpha/beta communities
   // Ignore thrown errors of duplicates
   try {
@@ -528,7 +521,7 @@ export async function likeComment(
 
 export async function createCommunity(
   api: LemmyHttp,
-  name_: string = randomString(5),
+  name_: string = randomString(10),
 ): Promise<CommunityResponse> {
   let description = "a sample description";
   let form: CreateCommunity = {

crates/api/src/local_user/save_settings.rs

@@ -34,11 +34,10 @@ pub async fn save_user_settings(
   let site_view = SiteView::read_local(&mut context.pool()).await?;
 
   let slur_regex = local_site_to_slur_regex(&site_view.local_site);
-  let bio = process_markdown_opt(&data.bio, &slur_regex, &context).await?;
+  let bio = diesel_option_overwrite(process_markdown_opt(&data.bio, &slur_regex, &context).await?);
 
   let avatar = proxy_image_link_opt_api(&data.avatar, &context).await?;
   let banner = proxy_image_link_opt_api(&data.banner, &context).await?;
-  let bio = diesel_option_overwrite(bio);
   let display_name = diesel_option_overwrite(data.display_name.clone());
   let matrix_user_id = diesel_option_overwrite(data.matrix_user_id.clone());
   let email_deref = data.email.as_deref().map(str::to_lowercase);

crates/api_common/src/request.rs

@@ -49,7 +49,7 @@ pub async fn fetch_link_metadata(
   // https://github.com/LemmyNet/lemmy/issues/1964
   let html_bytes = response.bytes().await.map_err(LemmyError::from)?.to_vec();
 
-  let mut metadata = extract_opengraph_data(&html_bytes, url)?;
+  let mut metadata = extract_opengraph_data(&html_bytes, url).unwrap_or_default();
 
   metadata.content_type = content_type.map(|c| c.to_string());
   if generate_thumbnail && is_image {
@@ -71,14 +71,13 @@ pub async fn fetch_link_metadata_opt(
   url: Option<&Url>,
   generate_thumbnail: bool,
   context: &LemmyContext,
-) -> Result<LinkMetadata, LemmyError> {
+) -> LinkMetadata {
-  let metadata = match &url {
+  match &url {
     Some(url) => fetch_link_metadata(url, generate_thumbnail, context)
       .await
       .unwrap_or_default(),
     _ => Default::default(),
-  };
+  }
-  Ok(metadata)
 }
 
 /// Extract site metadata from HTML Opengraph attributes.
@@ -300,6 +299,7 @@ mod tests {
     context::LemmyContext,
     request::{extract_opengraph_data, fetch_link_metadata},
   };
+  use lemmy_utils::settings::SETTINGS;
   use pretty_assertions::assert_eq;
   use serial_test::serial;
   use url::Url;

crates/api_common/src/utils.rs

@@ -846,9 +846,13 @@ pub async fn process_markdown(
   context: &LemmyContext,
 ) -> LemmyResult<String> {
   let text = remove_slurs(text, slur_regex);
-  let (text, links) = markdown_rewrite_image_links(text);
+  if context.settings().pictrs_config()?.image_proxy {
-  RemoteImage::create(&mut context.pool(), links).await?;
+    let (text, links) = markdown_rewrite_image_links(text);
-  Ok(text)
+    RemoteImage::create(&mut context.pool(), links).await?;
+    Ok(text)
+  } else {
+    Ok(text)
+  }
 }
 
 pub async fn process_markdown_opt(
@@ -862,6 +866,11 @@ pub async fn process_markdown_opt(
   }
 }
 
+/// Rewrite a link to go through `/api/v3/image_proxy` endpoint. This is only for remote urls and
+/// if image_proxy setting is enabled.
+///
+/// The parameter `image_proxy` is the config value of `pictrs.image_proxy`. Its necessary to pass
+/// as separate parameter so it can be changed in tests.
 pub(crate) async fn proxy_image_link(
   link: Url,
   image_proxy: bool,

crates/api_crud/src/post/create.rs

@@ -84,7 +84,7 @@ pub async fn create_post(
   }
 
   // Fetch post links and pictrs cached image
-  let metadata = fetch_link_metadata_opt(url.as_ref(), true, &context).await?;
+  let metadata = fetch_link_metadata_opt(url.as_ref(), true, &context).await;
   let url = proxy_image_link_opt_apub(url, &context).await?;
 
   // Only need to check if language is allowed in case user set it explicitly. When using default

crates/apub/src/objects/post.rs

@@ -224,7 +224,7 @@ impl Object for ApubPost {
       let do_generate_thumbnail = thumbnail_url.is_none() && allow_generate_thumbnail;
 
       // Generate local thumbnail only if no thumbnail was federated and 'sensitive' attributes allow it.
-      let metadata = fetch_link_metadata_opt(url.as_ref(), do_generate_thumbnail, context).await?;
+      let metadata = fetch_link_metadata_opt(url.as_ref(), do_generate_thumbnail, context).await;
       if let Some(thumbnail_url_) = metadata.thumbnail {
         thumbnail_url = Some(thumbnail_url_.into());
       }

crates/db_schema/src/source/images.rs

@@ -30,6 +30,8 @@ pub struct LocalImageForm {
   pub pictrs_delete_token: String,
 }
 
+/// 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.
 #[skip_serializing_none]
 #[derive(PartialEq, Eq, Debug, Clone)]
 #[cfg_attr(feature = "full", derive(Queryable, Identifiable))]

docker/lemmy.hjson

@@ -21,12 +21,8 @@
   pictrs: {
     url: "http://pictrs:8080/"
     # api_key: "API_KEY"
-<<<<<<< HEAD
-    cache_remote_images: true
     image_proxy: true
-=======
     cache_external_link_previews: true
->>>>>>> main
   }
 
   #opentelemetry_url: "http://otel:4137"