Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite images to use local proxy #4035

Merged
merged 55 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
c2584d3
Add markdown rule to add rel=nofollow for all links
Nutomic Oct 11, 2023
3be2a55
Add markdown image rule to add local image proxy (fixes #1036)
Nutomic Oct 11, 2023
44d8168
comments
Nutomic Oct 11, 2023
650e3a7
Merge branch 'main' into markdown-link-rule
Nutomic Oct 23, 2023
fc66bad
rewrite markdown image links working
Nutomic Oct 23, 2023
540560b
add comment
Nutomic Oct 24, 2023
aaf3833
perform markdown image processing in api/apub receivers
Nutomic Oct 24, 2023
89976b8
clippy
Nutomic Oct 24, 2023
ef79422
add db table to validate proxied links
Nutomic Oct 24, 2023
ae96d86
rewrite link fields for avatar, banner etc
Nutomic Oct 24, 2023
7306153
sql fmt
Nutomic Oct 24, 2023
aa49a1b
proxy links received over federation
Nutomic Oct 25, 2023
f057abf
Merge branch 'main' into markdown-link-rule
Nutomic Oct 25, 2023
986913d
add config option
Nutomic Oct 26, 2023
388eb42
undo post.url rewriting, move http route definition
Nutomic Oct 26, 2023
98b5746
add tests
Nutomic Oct 26, 2023
45f5448
proxy images through pictrs
Nutomic Oct 26, 2023
9b40d74
Merge branch 'main' into markdown-link-rule
Nutomic Oct 26, 2023
95025ad
testing
Nutomic Oct 26, 2023
bf20539
cleanup request.rs file
Nutomic Oct 27, 2023
c8c355d
more cleanup (fixes #2611)
Nutomic Oct 27, 2023
5507d2d
include url content type when sending post over apub (fixes #2611)
Nutomic Oct 27, 2023
97697aa
store post url content type in db
Nutomic Oct 27, 2023
ed3e2e0
should be media_type
Nutomic Oct 27, 2023
c2a763d
get rid of cache_remote_thumbnails setting, instead automatically
Nutomic Oct 30, 2023
7fbfa48
fix tests
Nutomic Oct 30, 2023
7068419
add setting disable_external_link_previews
Nutomic Nov 3, 2023
ecd8e3b
federate post url as image depending on mime type
Nutomic Nov 6, 2023
66229ab
Merge branch 'main' into markdown-link-rule
Nutomic Nov 6, 2023
d6b3d82
change setting again
Nutomic Nov 6, 2023
abe3ab2
machete
Nutomic Nov 6, 2023
a5dc167
invert
Nutomic Nov 6, 2023
6257469
Merge branch 'main' into markdown-link-rule
Nutomic Nov 7, 2023
289c55d
support custom emoji
Nutomic Nov 8, 2023
bf6b0a5
clippy
Nutomic Nov 9, 2023
6232fa0
Merge branch 'main' into markdown-link-rule
Nutomic Nov 23, 2023
11cf93b
update defaults
Nutomic Nov 23, 2023
3d698dd
Merge branch 'main' into markdown-link-rule
Nutomic Dec 21, 2023
becf54c
add image proxy test, fix test
Nutomic Dec 21, 2023
06257f9
fix test
Nutomic Dec 21, 2023
4c2fe13
clippy
Nutomic Dec 21, 2023
5f79a3c
revert accidental changes
Nutomic Dec 22, 2023
dc17cb1
Merge branch 'main' into markdown-link-rule
Nutomic Jan 4, 2024
d793d80
address review
Nutomic Jan 4, 2024
1f29e72
clippy
Nutomic Jan 5, 2024
3399917
Markdown link rule-dess (#4356)
dessalines Jan 8, 2024
a88f4d6
fix setting
Nutomic Jan 8, 2024
518af87
use enum for image proxy setting
Nutomic Jan 8, 2024
4fc857a
fix test configs
Nutomic Jan 8, 2024
7e5e455
Merge branch 'main' into markdown-link-rule
Nutomic Jan 10, 2024
81359e2
add config backwards compat
Nutomic Jan 10, 2024
7930b57
clippy
Nutomic Jan 10, 2024
2fd7edd
Merge branch 'main' into markdown-link-rule
Nutomic Jan 22, 2024
b600bc5
Merge branch 'main' into markdown-link-rule
Nutomic Jan 25, 2024
0a6a587
machete
Nutomic Jan 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ rustls = { version = "0.21.8", features = ["dangerous_configuration"] }
futures-util = "0.3.28"
tokio-postgres = "0.7.10"
tokio-postgres-rustls = "0.10.0"
urlencoding = "2.1.3"
enum-map = "2.7"

[dependencies]
Expand Down
8 changes: 7 additions & 1 deletion config/defaults.hjson
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,13 @@
# Set a custom pictrs API key. ( Required for deleting images )
api_key: "string"
# Cache remote images
cache_remote_images: true
cache_remote_thumbnails: true
# If enabled, all images from remote domains are rewritten to pass through `/api/v3/image_proxy`.
# This improves privacy as users don't expose their IP to untrusted servers, and decreases load
# on other servers. However it causes more load for the local server.
#
# Requires pict-rs 0.5
image_proxy: false
}
# Email sending configuration. All options except login/password are mandatory
email: {
Expand Down
18 changes: 13 additions & 5 deletions crates/api/src/local_user/save_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ use actix_web::web::{Data, Json};
use lemmy_api_common::{
context::LemmyContext,
person::SaveUserSettings,
utils::send_verification_email,
utils::{
local_site_to_slur_regex,
process_markdown_opt,
proxy_image_link_opt_api,
send_verification_email,
},
SuccessResponse,
};
use lemmy_db_schema::{
Expand All @@ -12,7 +17,7 @@ use lemmy_db_schema::{
person::{Person, PersonUpdateForm},
},
traits::Crud,
utils::{diesel_option_overwrite, diesel_option_overwrite_to_url},
utils::diesel_option_overwrite,
};
use lemmy_db_views::structs::{LocalUserView, SiteView};
use lemmy_utils::{
Expand All @@ -28,9 +33,12 @@ pub async fn save_user_settings(
) -> Result<Json<SuccessResponse>, LemmyError> {
let site_view = SiteView::read_local(&mut context.pool()).await?;

let avatar = diesel_option_overwrite_to_url(&data.avatar)?;
let banner = diesel_option_overwrite_to_url(&data.banner)?;
let bio = diesel_option_overwrite(data.bio.clone());
let slur_regex = local_site_to_slur_regex(&site_view.local_site);
let bio = 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);
dessalines marked this conversation as resolved.
Show resolved Hide resolved
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);
Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/site/purge/person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use lemmy_api_common::{
};
use lemmy_db_schema::{
source::{
image_upload::ImageUpload,
images::LocalImage,
moderator::{AdminPurgePerson, AdminPurgePersonForm},
person::Person,
},
Expand All @@ -31,7 +31,7 @@ pub async fn purge_person(

let local_user = LocalUserView::read_person(&mut context.pool(), person_id).await?;
let pictrs_uploads =
ImageUpload::get_all_by_local_user_id(&mut context.pool(), &local_user.local_user.id).await?;
LocalImage::get_all_by_local_user_id(&mut context.pool(), &local_user.local_user.id).await?;

for upload in pictrs_uploads {
delete_image_from_pictrs(&upload.pictrs_alias, &upload.pictrs_delete_token, &context)
Expand Down
11 changes: 6 additions & 5 deletions crates/api_common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ reqwest-middleware = { workspace = true, optional = true }
regex = { workspace = true }
rosetta-i18n = { workspace = true, optional = true }
percent-encoding = { workspace = true, optional = true }
webpage = { version = "1.6", default-features = false, features = [
"serde",
], optional = true }
encoding = { version = "0.2.33", optional = true }
anyhow = { workspace = true }
futures = { workspace = true, optional = true }
uuid = { workspace = true, optional = true }
Expand All @@ -65,10 +61,15 @@ reqwest = { workspace = true, optional = true }
ts-rs = { workspace = true, optional = true }
once_cell = { workspace = true, optional = true }
actix-web = { workspace = true, optional = true }
enum-map = { workspace = true }
urlencoding = { workspace = true }
webpage = { version = "1.6", default-features = false, features = [
"serde",
], optional = true }
encoding = { version = "0.2.33", optional = true }
jsonwebtoken = { version = "8.3.0", optional = true }
# necessary for wasmt compilation
getrandom = { version = "0.2.10", features = ["js"] }
enum-map = { workspace = true }

[dev-dependencies]
serial_test = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion crates/api_common/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ pub(crate) async fn fetch_pictrs(
let pictrs_config = settings.pictrs_config()?;
is_image_content_type(client, image_url).await?;

if pictrs_config.cache_remote_images {
if pictrs_config.cache_remote_thumbnails {
// fetch remote non-pictrs images for persistent thumbnail link
let fetch_url = format!(
"{}image/download?url={}",
Expand Down
72 changes: 69 additions & 3 deletions crates/api_common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use lemmy_db_schema::{
comment::{Comment, CommentUpdateForm},
community::{Community, CommunityModerator, CommunityUpdateForm},
email_verification::{EmailVerification, EmailVerificationForm},
images::RemoteImage,
instance::Instance,
local_site::LocalSite,
local_site_rate_limit::LocalSiteRateLimit,
Expand All @@ -23,7 +24,7 @@ use lemmy_db_schema::{
post::{Post, PostRead},
},
traits::Crud,
utils::DbPool,
utils::{diesel_option_overwrite_to_url, DbPool},
};
use lemmy_db_views::{comment_view::CommentQuery, structs::LocalUserView};
use lemmy_db_views_actor::structs::{
Expand All @@ -36,14 +37,18 @@ use lemmy_utils::{
error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult},
location_info,
rate_limit::{ActionType, BucketConfig},
settings::structs::Settings,
utils::slurs::build_slur_regex,
settings::{structs::Settings, SETTINGS},
utils::{
markdown::markdown_rewrite_image_links,
slurs::{build_slur_regex, remove_slurs},
},
};
use regex::Regex;
use rosetta_i18n::{Language, LanguageId};
use std::collections::HashSet;
use tracing::warn;
use url::{ParseError, Url};
use urlencoding::encode;

pub static AUTH_COOKIE_NAME: &str = "auth";

Expand Down Expand Up @@ -786,6 +791,67 @@ fn limit_expire_time(expires: DateTime<Utc>) -> LemmyResult<Option<DateTime<Utc>
}
}

pub async fn process_markdown(
text: &str,
slur_regex: &Option<Regex>,
context: &LemmyContext,
) -> LemmyResult<String> {
let text = remove_slurs(text, slur_regex);
let (text, links) = markdown_rewrite_image_links(text);
dessalines marked this conversation as resolved.
Show resolved Hide resolved
RemoteImage::create(&mut context.pool(), links).await?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't clear that RemoteImage::create() shouldn't run if the setting for cache_external_images is false, or proxy_images is true. Maybe should do a check on that setting before trying to create it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RemoteImage is only written if the image is getting proxied. The table only exists to check that this url is valid for proxying, so there is no need to create it otherwise.

Ok(text)
}

pub async fn process_markdown_opt(
text: &Option<String>,
slur_regex: &Option<Regex>,
context: &LemmyContext,
) -> LemmyResult<Option<String>> {
match text {
dessalines marked this conversation as resolved.
Show resolved Hide resolved
Some(t) => process_markdown(t, slur_regex, context).await.map(Some),
None => Ok(None),
}
}

pub async fn proxy_image_link(link: Url, context: &LemmyContext) -> LemmyResult<DbUrl> {
// Dont rewrite links pointing to local domain.
if link.domain() == Some(&SETTINGS.hostname) {
return Ok(link.into());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been trying to remove early returns wherever possible. So maybe

if link.domain() .... { 
  Ok(link.into()))
} else { 
...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats wrong with early returns? To me they are much clearer than unnecessary intendation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really help with logical control flow. A few months ago I went through a lot of these removing as many return statements as I could. Some functions had like 4 early returns, and it was difficult to follow the chain of ifs / matches.

}

let proxied = format!(
"{}/api/v3/image_proxy?url={}",
SETTINGS.get_protocol_and_hostname(),
encode(link.as_str())
);
RemoteImage::create(&mut context.pool(), vec![link]).await?;
Ok(Url::parse(&proxied)?.into())
}

pub async fn proxy_image_link_opt_api(
link: &Option<String>,
context: &LemmyContext,
) -> LemmyResult<Option<Option<DbUrl>>> {
let link = diesel_option_overwrite_to_url(link)?;
if let Some(l) = link {
proxy_image_link_opt_apub(l.map(Into::into), context)
.await
.map(Some)
} else {
Ok(link)
}
}
pub async fn proxy_image_link_opt_apub(
link: Option<Url>,
context: &LemmyContext,
) -> LemmyResult<Option<DbUrl>> {
if let Some(l) = link {
proxy_image_link(l.clone(), context).await.map(Some)
} else {
Ok(None)
}
}

#[cfg(test)]
mod tests {
#![allow(clippy::unwrap_used)]
Expand Down
13 changes: 4 additions & 9 deletions crates/api_crud/src/comment/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use lemmy_api_common::{
generate_local_apub_endpoint,
get_post,
local_site_to_slur_regex,
process_markdown,
EndpointType,
},
};
Expand All @@ -28,11 +29,7 @@ use lemmy_db_schema::{
use lemmy_db_views::structs::LocalUserView;
use lemmy_utils::{
error::{LemmyError, LemmyErrorExt, LemmyErrorType},
utils::{
mention::scrape_text_for_mentions,
slurs::remove_slurs,
validation::is_valid_body_field,
},
utils::{mention::scrape_text_for_mentions, validation::is_valid_body_field},
};

const MAX_COMMENT_DEPTH_LIMIT: usize = 100;
Expand All @@ -45,10 +42,8 @@ pub async fn create_comment(
) -> Result<Json<CommentResponse>, LemmyError> {
let local_site = LocalSite::read(&mut context.pool()).await?;

let content = remove_slurs(
&data.content.clone(),
&local_site_to_slur_regex(&local_site),
);
let slur_regex = local_site_to_slur_regex(&local_site);
let content = process_markdown(&data.content, &slur_regex, &context).await?;
is_valid_body_field(&Some(content.clone()), false)?;

// Check for a community ban
Expand Down
15 changes: 4 additions & 11 deletions crates/api_crud/src/comment/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use lemmy_api_common::{
comment::{CommentResponse, EditComment},
context::LemmyContext,
send_activity::{ActivityChannel, SendActivityData},
utils::{check_community_user_action, local_site_to_slur_regex},
utils::{check_community_user_action, local_site_to_slur_regex, process_markdown_opt},
};
use lemmy_db_schema::{
source::{
Expand All @@ -19,11 +19,7 @@ use lemmy_db_schema::{
use lemmy_db_views::structs::{CommentView, LocalUserView};
use lemmy_utils::{
error::{LemmyError, LemmyErrorExt, LemmyErrorType},
utils::{
mention::scrape_text_for_mentions,
slurs::remove_slurs,
validation::is_valid_body_field,
},
utils::{mention::scrape_text_for_mentions, validation::is_valid_body_field},
};

#[tracing::instrument(skip(context))]
Expand Down Expand Up @@ -57,11 +53,8 @@ pub async fn update_comment(
)
.await?;

// Update the Content
let content = data
.content
.as_ref()
.map(|c| remove_slurs(c, &local_site_to_slur_regex(&local_site)));
let slur_regex = local_site_to_slur_regex(&local_site);
let content = process_markdown_opt(&data.content, &slur_regex, &context).await?;
is_valid_body_field(&content, false)?;

let comment_id = data.comment_id;
Expand Down
19 changes: 11 additions & 8 deletions crates/api_crud/src/community/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use lemmy_api_common::{
generate_shared_inbox_url,
is_admin,
local_site_to_slur_regex,
process_markdown_opt,
proxy_image_link_opt_api,
EndpointType,
},
};
Expand All @@ -27,13 +29,12 @@ use lemmy_db_schema::{
},
},
traits::{ApubActor, Crud, Followable, Joinable},
utils::diesel_option_overwrite_to_url_create,
};
use lemmy_db_views::structs::{LocalUserView, SiteView};
use lemmy_utils::{
error::{LemmyError, LemmyErrorExt, LemmyErrorType},
utils::{
slurs::{check_slurs, check_slurs_opt},
slurs::check_slurs,
validation::{is_valid_actor_name, is_valid_body_field},
},
};
Expand All @@ -51,14 +52,16 @@ pub async fn create_community(
Err(LemmyErrorType::OnlyAdminsCanCreateCommunities)?
}

// Check to make sure the icon and banners are urls
let icon = diesel_option_overwrite_to_url_create(&data.icon)?;
let banner = diesel_option_overwrite_to_url_create(&data.banner)?;

let slur_regex = local_site_to_slur_regex(&local_site);
check_slurs(&data.name, &slur_regex)?;
check_slurs(&data.title, &slur_regex)?;
check_slurs_opt(&data.description, &slur_regex)?;
let description = process_markdown_opt(&data.description, &slur_regex, &context).await?;
let icon = proxy_image_link_opt_api(&data.icon, &context)
.await?
.unwrap();
let banner = proxy_image_link_opt_api(&data.banner, &context)
.await?
.unwrap();

is_valid_actor_name(&data.name, local_site.actor_name_max_length as usize)?;
is_valid_body_field(&data.description, false)?;
Expand All @@ -81,7 +84,7 @@ pub async fn create_community(
let community_form = CommunityInsertForm::builder()
.name(data.name.clone())
.title(data.title.clone())
.description(data.description.clone())
.description(description)
.icon(icon)
.banner(banner)
.nsfw(data.nsfw)
Expand Down
Loading