From e78ba38e94cdc6f31b6922a71eae05cbd8a8e435 Mon Sep 17 00:00:00 2001 From: Andrew Yoon Date: Tue, 2 Mar 2021 07:41:48 -0500 Subject: [PATCH] Use URL type in most outstanding struct fields (#1468) * Use URL type in most outstanding struct fields This fixes all known remaining cases where url fields are stored as plain strings, with the exception of form fields where empty strings are used as sentinels (see `diesel_option_overwrite_to_url`). Tested for regressions in the federated docker setup attempting to exercise all changed fields, including through apub federation. Fixes #1385 * Add migration to fix blank-string post.url values to be null This also then fixes #602 * Address review feedback - Fixed some unwraps and err message formatting - Bumped the `url` library to 2.2.1 to fix a bug with serde error messages - Add unit tests for the two diesel option override functions - Fix migration teardown by adding a no-op * Rename lemmy_db_queries::Url to lemmy_db_queries::DbUrl * fix compile error * box PostOrComment variants --- Cargo.lock | 4 +- Cargo.toml | 2 +- crates/api/Cargo.toml | 2 +- crates/api/src/community.rs | 17 ++--- crates/api/src/lib.rs | 9 --- crates/api/src/post.rs | 17 +++-- crates/api/src/site.rs | 16 +++-- crates/api/src/user.rs | 10 +-- crates/api_structs/Cargo.toml | 2 +- crates/api_structs/src/post.rs | 5 +- crates/api_structs/src/site.rs | 5 +- crates/apub/Cargo.toml | 2 +- crates/apub/src/http/mod.rs | 6 +- crates/apub/src/inbox/mod.rs | 2 +- .../apub/src/inbox/receive_for_community.rs | 36 +++++------ crates/apub/src/lib.rs | 45 +++++++------- crates/apub/src/objects/community.rs | 8 +-- crates/apub/src/objects/mod.rs | 4 +- crates/apub/src/objects/post.rs | 32 +++++----- crates/apub/src/objects/user.rs | 12 ++-- crates/db_queries/Cargo.toml | 2 +- crates/db_queries/src/lib.rs | 50 ++++++++++++++- crates/db_queries/src/source/activity.rs | 29 +++++---- crates/db_queries/src/source/comment.rs | 8 +-- crates/db_queries/src/source/community.rs | 11 ++-- crates/db_queries/src/source/post.rs | 8 +-- .../db_queries/src/source/private_message.rs | 8 +-- crates/db_queries/src/source/user.rs | 4 +- crates/db_schema/Cargo.toml | 2 +- crates/db_schema/src/lib.rs | 29 ++++----- crates/db_schema/src/source/activity.rs | 6 +- crates/db_schema/src/source/comment.rs | 8 +-- crates/db_schema/src/source/community.rs | 32 +++++----- crates/db_schema/src/source/post.rs | 14 ++--- crates/db_schema/src/source/post_report.rs | 6 +- .../db_schema/src/source/private_message.rs | 6 +- crates/db_schema/src/source/site.rs | 10 +-- crates/db_schema/src/source/user.rs | 62 +++++++++---------- crates/db_views/Cargo.toml | 2 +- crates/routes/Cargo.toml | 2 +- crates/utils/Cargo.toml | 2 +- crates/utils/src/request.rs | 35 ++++++----- .../down.sql | 4 ++ .../up.sql | 1 + 44 files changed, 312 insertions(+), 265 deletions(-) create mode 100644 migrations/2021-02-28-162616_clean_empty_post_urls/down.sql create mode 100644 migrations/2021-02-28-162616_clean_empty_post_urls/up.sql diff --git a/Cargo.lock b/Cargo.lock index 69b22f95..959aec10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3655,9 +3655,9 @@ checksum = "a156c684c91ea7d62626509bce3cb4e1d9ed5c4d978f7b4352658f96a4c26b4a" [[package]] name = "url" -version = "2.2.0" +version = "2.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5909f2b0817350449ed73e8bcd81c8c3c8d9a7a5d8acba4b27db277f1868976e" +checksum = "9ccd964113622c8e9322cfac19eb1004a07e636c545f325da085d5cdde6f1f8b" dependencies = [ "form_urlencoded", "idna", diff --git a/Cargo.toml b/Cargo.toml index f76128cb..e7d92fdf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,7 +45,7 @@ actix-web = { version = "3.3.2", default-features = false, features = ["rustls"] log = "0.4.14" env_logger = "0.8.2" strum = "0.20.0" -url = { version = "2.2.0", features = ["serde"] } +url = { version = "2.2.1", features = ["serde"] } openssl = "0.10.32" http-signature-normalization-actix = { version = "0.4.1", default-features = false, features = ["sha-2"] } tokio = "0.3.6" diff --git a/crates/api/Cargo.toml b/crates/api/Cargo.toml index 5ab33958..ea3cd625 100644 --- a/crates/api/Cargo.toml +++ b/crates/api/Cargo.toml @@ -32,7 +32,7 @@ rand = "0.8.3" strum = "0.20.0" strum_macros = "0.20.1" lazy_static = "1.4.0" -url = { version = "2.2.0", features = ["serde"] } +url = { version = "2.2.1", features = ["serde"] } openssl = "0.10.32" http = "0.2.3" http-signature-normalization-actix = { version = "0.4.1", default-features = false, features = ["sha-2"] } diff --git a/crates/api/src/community.rs b/crates/api/src/community.rs index 4bfbaeb4..cee5d371 100644 --- a/crates/api/src/community.rs +++ b/crates/api/src/community.rs @@ -1,6 +1,5 @@ use crate::{ check_community_ban, - check_optional_url, get_user_from_jwt, get_user_from_jwt_opt, is_admin, @@ -19,7 +18,7 @@ use lemmy_apub::{ EndpointType, }; use lemmy_db_queries::{ - diesel_option_overwrite, + diesel_option_overwrite_to_url, source::{ comment::Comment_, community::{CommunityModerator_, Community_}, @@ -155,11 +154,8 @@ impl Perform for CreateCommunity { } // Check to make sure the icon and banners are urls - let icon = diesel_option_overwrite(&data.icon); - let banner = diesel_option_overwrite(&data.banner); - - check_optional_url(&icon)?; - check_optional_url(&banner)?; + let icon = diesel_option_overwrite_to_url(&data.icon)?; + let banner = diesel_option_overwrite_to_url(&data.banner)?; // When you create a community, make sure the user becomes a moderator and a follower let keypair = generate_actor_keypair()?; @@ -260,11 +256,8 @@ impl Perform for EditCommunity { }) .await??; - let icon = diesel_option_overwrite(&data.icon); - let banner = diesel_option_overwrite(&data.banner); - - check_optional_url(&icon)?; - check_optional_url(&banner)?; + let icon = diesel_option_overwrite_to_url(&data.icon)?; + let banner = diesel_option_overwrite_to_url(&data.banner)?; let community_form = CommunityForm { name: read_community.name, diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 6f4ea8f4..54d11c1e 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -186,15 +186,6 @@ pub(crate) async fn collect_moderated_communities( } } -pub(crate) fn check_optional_url(item: &Option>) -> Result<(), LemmyError> { - if let Some(Some(item)) = &item { - if Url::parse(item).is_err() { - return Err(ApiError::err("invalid_url").into()); - } - } - Ok(()) -} - pub(crate) async fn build_federated_instances( pool: &DbPool, ) -> Result, LemmyError> { diff --git a/crates/api/src/post.rs b/crates/api/src/post.rs index e905cd5c..202ea30b 100644 --- a/crates/api/src/post.rs +++ b/crates/api/src/post.rs @@ -1,7 +1,6 @@ use crate::{ check_community_ban, check_downvotes_enabled, - check_optional_url, collect_moderated_communities, get_user_from_jwt, get_user_from_jwt_opt, @@ -72,15 +71,14 @@ impl Perform for CreatePost { check_community_ban(user.id, data.community_id, context.pool()).await?; - check_optional_url(&Some(data.url.to_owned()))?; - // Fetch Iframely and pictrs cached image + let data_url = data.url.as_ref(); let (iframely_title, iframely_description, iframely_html, pictrs_thumbnail) = - fetch_iframely_and_pictrs_data(context.client(), data.url.to_owned()).await; + fetch_iframely_and_pictrs_data(context.client(), data_url).await; let post_form = PostForm { name: data.name.trim().to_owned(), - url: data.url.to_owned(), + url: data_url.map(|u| u.to_owned().into()), body: data.body.to_owned(), community_id: data.community_id, creator_id: user.id, @@ -93,7 +91,7 @@ impl Perform for CreatePost { embed_title: iframely_title, embed_description: iframely_description, embed_html: iframely_html, - thumbnail_url: pictrs_thumbnail, + thumbnail_url: pictrs_thumbnail.map(|u| u.into()), ap_id: None, local: true, published: None, @@ -385,12 +383,13 @@ impl Perform for EditPost { } // Fetch Iframely and Pictrs cached image + let data_url = data.url.as_ref(); let (iframely_title, iframely_description, iframely_html, pictrs_thumbnail) = - fetch_iframely_and_pictrs_data(context.client(), data.url.to_owned()).await; + fetch_iframely_and_pictrs_data(context.client(), data_url).await; let post_form = PostForm { name: data.name.trim().to_owned(), - url: data.url.to_owned(), + url: data_url.map(|u| u.to_owned().into()), body: data.body.to_owned(), nsfw: data.nsfw, creator_id: orig_post.creator_id.to_owned(), @@ -403,7 +402,7 @@ impl Perform for EditPost { embed_title: iframely_title, embed_description: iframely_description, embed_html: iframely_html, - thumbnail_url: pictrs_thumbnail, + thumbnail_url: pictrs_thumbnail.map(|u| u.into()), ap_id: Some(orig_post.ap_id), local: orig_post.local, published: None, diff --git a/crates/api/src/site.rs b/crates/api/src/site.rs index c4792f36..af9d22c4 100644 --- a/crates/api/src/site.rs +++ b/crates/api/src/site.rs @@ -11,7 +11,13 @@ use actix_web::web::Data; use anyhow::Context; use lemmy_api_structs::{blocking, site::*, user::Register}; use lemmy_apub::fetcher::search::search_by_apub_id; -use lemmy_db_queries::{diesel_option_overwrite, source::site::Site_, Crud, SearchType, SortType}; +use lemmy_db_queries::{ + diesel_option_overwrite_to_url, + source::site::Site_, + Crud, + SearchType, + SortType, +}; use lemmy_db_schema::{ naive_now, source::{ @@ -157,8 +163,8 @@ impl Perform for CreateSite { let site_form = SiteForm { name: data.name.to_owned(), description: data.description.to_owned(), - icon: Some(data.icon.to_owned()), - banner: Some(data.banner.to_owned()), + icon: Some(data.icon.to_owned().map(|url| url.into())), + banner: Some(data.banner.to_owned().map(|url| url.into())), creator_id: user.id, enable_downvotes: data.enable_downvotes, open_registration: data.open_registration, @@ -196,8 +202,8 @@ impl Perform for EditSite { let found_site = blocking(context.pool(), move |conn| Site::read_simple(conn)).await??; - let icon = diesel_option_overwrite(&data.icon); - let banner = diesel_option_overwrite(&data.banner); + let icon = diesel_option_overwrite_to_url(&data.icon)?; + let banner = diesel_option_overwrite_to_url(&data.banner)?; let site_form = SiteForm { name: data.name.to_owned(), diff --git a/crates/api/src/user.rs b/crates/api/src/user.rs index 6d37581e..903c00e7 100644 --- a/crates/api/src/user.rs +++ b/crates/api/src/user.rs @@ -1,6 +1,5 @@ use crate::{ captcha_espeak_wav_base64, - check_optional_url, collect_moderated_communities, get_user_from_jwt, get_user_from_jwt_opt, @@ -23,6 +22,7 @@ use lemmy_apub::{ }; use lemmy_db_queries::{ diesel_option_overwrite, + diesel_option_overwrite_to_url, source::{ comment::Comment_, community::Community_, @@ -366,17 +366,13 @@ impl Perform for SaveUserSettings { let data: &SaveUserSettings = &self; let user = get_user_from_jwt(&data.auth, context.pool()).await?; - let avatar = diesel_option_overwrite(&data.avatar); - let banner = diesel_option_overwrite(&data.banner); + let avatar = diesel_option_overwrite_to_url(&data.avatar)?; + let banner = diesel_option_overwrite_to_url(&data.banner)?; let email = diesel_option_overwrite(&data.email); let bio = diesel_option_overwrite(&data.bio); let preferred_username = diesel_option_overwrite(&data.preferred_username); let matrix_user_id = diesel_option_overwrite(&data.matrix_user_id); - // Check to make sure the avatar and banners are urls - check_optional_url(&avatar)?; - check_optional_url(&banner)?; - if let Some(Some(bio)) = &bio { if bio.chars().count() > 300 { return Err(ApiError::err("bio_length_overflow").into()); diff --git a/crates/api_structs/Cargo.toml b/crates/api_structs/Cargo.toml index fc91e759..242383e6 100644 --- a/crates/api_structs/Cargo.toml +++ b/crates/api_structs/Cargo.toml @@ -21,4 +21,4 @@ diesel = "1.4.5" actix-web = "3.3.2" chrono = { version = "0.4.19", features = ["serde"] } serde_json = { version = "1.0.61", features = ["preserve_order"] } -url = "2.2.0" +url = "2.2.1" diff --git a/crates/api_structs/src/post.rs b/crates/api_structs/src/post.rs index 4e2011e9..82be6617 100644 --- a/crates/api_structs/src/post.rs +++ b/crates/api_structs/src/post.rs @@ -8,11 +8,12 @@ use lemmy_db_views_actor::{ community_view::CommunityView, }; use serde::{Deserialize, Serialize}; +use url::Url; #[derive(Deserialize, Debug)] pub struct CreatePost { pub name: String, - pub url: Option, + pub url: Option, pub body: Option, pub nsfw: bool, pub community_id: i32, @@ -66,7 +67,7 @@ pub struct CreatePostLike { pub struct EditPost { pub post_id: i32, pub name: String, - pub url: Option, + pub url: Option, pub body: Option, pub nsfw: bool, pub auth: String, diff --git a/crates/api_structs/src/site.rs b/crates/api_structs/src/site.rs index edee17a8..9f69e63b 100644 --- a/crates/api_structs/src/site.rs +++ b/crates/api_structs/src/site.rs @@ -13,6 +13,7 @@ use lemmy_db_views_moderator::{ mod_sticky_post_view::ModStickyPostView, }; use serde::{Deserialize, Serialize}; +use url::Url; #[derive(Deserialize, Debug)] pub struct Search { @@ -60,8 +61,8 @@ pub struct GetModlogResponse { pub struct CreateSite { pub name: String, pub description: Option, - pub icon: Option, - pub banner: Option, + pub icon: Option, + pub banner: Option, pub enable_downvotes: bool, pub open_registration: bool, pub enable_nsfw: bool, diff --git a/crates/apub/Cargo.toml b/crates/apub/Cargo.toml index d294cbd9..b0538af6 100644 --- a/crates/apub/Cargo.toml +++ b/crates/apub/Cargo.toml @@ -32,7 +32,7 @@ rand = "0.8.3" strum = "0.20.0" strum_macros = "0.20.1" lazy_static = "1.4.0" -url = { version = "2.2.0", features = ["serde"] } +url = { version = "2.2.1", features = ["serde"] } percent-encoding = "2.1.0" openssl = "0.10.32" http = "0.2.3" diff --git a/crates/apub/src/http/mod.rs b/crates/apub/src/http/mod.rs index f117b6ea..6bf4bbde 100644 --- a/crates/apub/src/http/mod.rs +++ b/crates/apub/src/http/mod.rs @@ -7,6 +7,7 @@ use lemmy_db_schema::source::activity::Activity; use lemmy_utils::{settings::structs::Settings, LemmyError}; use lemmy_websocket::LemmyContext; use serde::{Deserialize, Serialize}; +use url::Url; pub mod comment; pub mod community; @@ -46,12 +47,13 @@ pub async fn get_activity( context: web::Data, ) -> Result, LemmyError> { let settings = Settings::get(); - let activity_id = format!( + let activity_id = Url::parse(&format!( "{}/activities/{}/{}", settings.get_protocol_and_hostname(), info.type_, info.id - ); + ))? + .into(); let activity = blocking(context.pool(), move |conn| { Activity::read_from_apub_id(&conn, &activity_id) }) diff --git a/crates/apub/src/inbox/mod.rs b/crates/apub/src/inbox/mod.rs index 2adefabe..21585aa6 100644 --- a/crates/apub/src/inbox/mod.rs +++ b/crates/apub/src/inbox/mod.rs @@ -45,7 +45,7 @@ pub(crate) async fn is_activity_already_known( pool: &DbPool, activity_id: &Url, ) -> Result { - let activity_id = activity_id.to_string(); + let activity_id = activity_id.to_owned().into(); let existing = blocking(pool, move |conn| { Activity::read_from_apub_id(&conn, &activity_id) }) diff --git a/crates/apub/src/inbox/receive_for_community.rs b/crates/apub/src/inbox/receive_for_community.rs index 9f8a6d6c..a3ffbf11 100644 --- a/crates/apub/src/inbox/receive_for_community.rs +++ b/crates/apub/src/inbox/receive_for_community.rs @@ -120,9 +120,9 @@ pub(in crate::inbox) async fn receive_like_for_community( .as_single_xsd_any_uri() .context(location_info!())?; match fetch_post_or_comment_by_id(&object_id, context, request_counter).await? { - PostOrComment::Post(post) => receive_like_post(like, post, context, request_counter).await, + PostOrComment::Post(post) => receive_like_post(like, *post, context, request_counter).await, PostOrComment::Comment(comment) => { - receive_like_comment(like, comment, context, request_counter).await + receive_like_comment(like, *comment, context, request_counter).await } } } @@ -152,10 +152,10 @@ pub(in crate::inbox) async fn receive_dislike_for_community( .context(location_info!())?; match fetch_post_or_comment_by_id(&object_id, context, request_counter).await? { PostOrComment::Post(post) => { - receive_dislike_post(dislike, post, context, request_counter).await + receive_dislike_post(dislike, *post, context, request_counter).await } PostOrComment::Comment(comment) => { - receive_dislike_comment(dislike, comment, context, request_counter).await + receive_dislike_comment(dislike, *comment, context, request_counter).await } } } @@ -177,8 +177,8 @@ pub(in crate::inbox) async fn receive_delete_for_community( .context(location_info!())?; match find_post_or_comment_by_id(context, object).await { - Ok(PostOrComment::Post(p)) => receive_delete_post(context, p).await, - Ok(PostOrComment::Comment(c)) => receive_delete_comment(context, c).await, + Ok(PostOrComment::Post(p)) => receive_delete_post(context, *p).await, + Ok(PostOrComment::Comment(c)) => receive_delete_comment(context, *c).await, // if we dont have the object, no need to do anything Err(_) => Ok(()), } @@ -215,8 +215,8 @@ pub(in crate::inbox) async fn receive_remove_for_community( remove.id(community_id.domain().context(location_info!())?)?; match find_post_or_comment_by_id(context, object).await { - Ok(PostOrComment::Post(p)) => receive_remove_post(context, remove, p).await, - Ok(PostOrComment::Comment(c)) => receive_remove_comment(context, remove, c).await, + Ok(PostOrComment::Post(p)) => receive_remove_post(context, remove, *p).await, + Ok(PostOrComment::Comment(c)) => receive_remove_comment(context, remove, *c).await, // if we dont have the object, no need to do anything Err(_) => Ok(()), } @@ -276,8 +276,8 @@ pub(in crate::inbox) async fn receive_undo_delete_for_community( .single_xsd_any_uri() .context(location_info!())?; match find_post_or_comment_by_id(context, object).await { - Ok(PostOrComment::Post(p)) => receive_undo_delete_post(context, p).await, - Ok(PostOrComment::Comment(c)) => receive_undo_delete_comment(context, c).await, + Ok(PostOrComment::Post(p)) => receive_undo_delete_post(context, *p).await, + Ok(PostOrComment::Comment(c)) => receive_undo_delete_comment(context, *c).await, // if we dont have the object, no need to do anything Err(_) => Ok(()), } @@ -300,8 +300,8 @@ pub(in crate::inbox) async fn receive_undo_remove_for_community( .single_xsd_any_uri() .context(location_info!())?; match find_post_or_comment_by_id(context, object).await { - Ok(PostOrComment::Post(p)) => receive_undo_remove_post(context, p).await, - Ok(PostOrComment::Comment(c)) => receive_undo_remove_comment(context, c).await, + Ok(PostOrComment::Post(p)) => receive_undo_remove_post(context, *p).await, + Ok(PostOrComment::Comment(c)) => receive_undo_remove_comment(context, *c).await, // if we dont have the object, no need to do anything Err(_) => Ok(()), } @@ -325,10 +325,10 @@ pub(in crate::inbox) async fn receive_undo_like_for_community( .context(location_info!())?; match fetch_post_or_comment_by_id(&object_id, context, request_counter).await? { PostOrComment::Post(post) => { - receive_undo_like_post(&like, post, context, request_counter).await + receive_undo_like_post(&like, *post, context, request_counter).await } PostOrComment::Comment(comment) => { - receive_undo_like_comment(&like, comment, context, request_counter).await + receive_undo_like_comment(&like, *comment, context, request_counter).await } } } @@ -351,10 +351,10 @@ pub(in crate::inbox) async fn receive_undo_dislike_for_community( .context(location_info!())?; match fetch_post_or_comment_by_id(&object_id, context, request_counter).await? { PostOrComment::Post(post) => { - receive_undo_dislike_post(&dislike, post, context, request_counter).await + receive_undo_dislike_post(&dislike, *post, context, request_counter).await } PostOrComment::Comment(comment) => { - receive_undo_dislike_comment(&dislike, comment, context, request_counter).await + receive_undo_dislike_comment(&dislike, *comment, context, request_counter).await } } } @@ -365,11 +365,11 @@ async fn fetch_post_or_comment_by_id( request_counter: &mut i32, ) -> Result { if let Ok(post) = get_or_fetch_and_insert_post(apub_id, context, request_counter).await { - return Ok(PostOrComment::Post(post)); + return Ok(PostOrComment::Post(Box::new(post))); } if let Ok(comment) = get_or_fetch_and_insert_comment(apub_id, context, request_counter).await { - return Ok(PostOrComment::Comment(comment)); + return Ok(PostOrComment::Comment(Box::new(comment))); } Err(NotFound.into()) diff --git a/crates/apub/src/lib.rs b/crates/apub/src/lib.rs index 5c0b267a..850ef503 100644 --- a/crates/apub/src/lib.rs +++ b/crates/apub/src/lib.rs @@ -26,13 +26,16 @@ use anyhow::{anyhow, Context}; use diesel::NotFound; use lemmy_api_structs::blocking; use lemmy_db_queries::{source::activity::Activity_, ApubObject, DbPool}; -use lemmy_db_schema::source::{ - activity::Activity, - comment::Comment, - community::Community, - post::Post, - private_message::PrivateMessage, - user::User_, +use lemmy_db_schema::{ + source::{ + activity::Activity, + comment::Comment, + community::Community, + post::Post, + private_message::PrivateMessage, + user::User_, + }, + DbUrl, }; use lemmy_utils::{location_info, settings::structs::Settings, LemmyError}; use lemmy_websocket::LemmyContext; @@ -216,7 +219,7 @@ pub enum EndpointType { pub fn generate_apub_endpoint( endpoint_type: EndpointType, name: &str, -) -> Result { +) -> Result { let point = match endpoint_type { EndpointType::Community => "c", EndpointType::User => "u", @@ -236,21 +239,15 @@ pub fn generate_apub_endpoint( ) } -pub fn generate_followers_url( - actor_id: &lemmy_db_schema::Url, -) -> Result { +pub fn generate_followers_url(actor_id: &DbUrl) -> Result { Ok(Url::parse(&format!("{}/followers", actor_id))?.into()) } -pub fn generate_inbox_url( - actor_id: &lemmy_db_schema::Url, -) -> Result { +pub fn generate_inbox_url(actor_id: &DbUrl) -> Result { Ok(Url::parse(&format!("{}/inbox", actor_id))?.into()) } -pub fn generate_shared_inbox_url( - actor_id: &lemmy_db_schema::Url, -) -> Result { +pub fn generate_shared_inbox_url(actor_id: &DbUrl) -> Result { let actor_id = actor_id.clone().into_inner(); let url = format!( "{}://{}{}/inbox", @@ -277,7 +274,7 @@ pub(crate) async fn insert_activity( where T: Serialize + std::fmt::Debug + Send + 'static, { - let ap_id = ap_id.to_string(); + let ap_id = ap_id.to_owned().into(); blocking(pool, move |conn| { Activity::insert(conn, ap_id, &activity, local, sensitive) }) @@ -286,8 +283,8 @@ where } pub(crate) enum PostOrComment { - Comment(Comment), - Post(Post), + Comment(Box), + Post(Box), } /// Tries to find a post or comment in the local database, without any network requests. @@ -303,7 +300,7 @@ pub(crate) async fn find_post_or_comment_by_id( }) .await?; if let Ok(p) = post { - return Ok(PostOrComment::Post(p)); + return Ok(PostOrComment::Post(Box::new(p))); } let ap_id = apub_id.clone(); @@ -312,7 +309,7 @@ pub(crate) async fn find_post_or_comment_by_id( }) .await?; if let Ok(c) = comment { - return Ok(PostOrComment::Comment(c)); + return Ok(PostOrComment::Comment(Box::new(c))); } Err(NotFound.into()) @@ -333,8 +330,8 @@ pub(crate) async fn find_object_by_id( let ap_id = apub_id.clone(); if let Ok(pc) = find_post_or_comment_by_id(context, ap_id.to_owned()).await { return Ok(match pc { - PostOrComment::Post(p) => Object::Post(p), - PostOrComment::Comment(c) => Object::Comment(c), + PostOrComment::Post(p) => Object::Post(*p), + PostOrComment::Comment(c) => Object::Comment(*c), }); } diff --git a/crates/apub/src/objects/community.rs b/crates/apub/src/objects/community.rs index f516768b..200497b7 100644 --- a/crates/apub/src/objects/community.rs +++ b/crates/apub/src/objects/community.rs @@ -73,13 +73,13 @@ impl ToApub for Community { if let Some(icon_url) = &self.icon { let mut image = Image::new(); - image.set_url(Url::parse(icon_url)?); + image.set_url::(icon_url.to_owned().into()); group.set_icon(image.into_any_base()?); } if let Some(banner_url) = &self.banner { let mut image = Image::new(); - image.set_url(Url::parse(banner_url)?); + image.set_url::(banner_url.to_owned().into()); group.set_image(image.into_any_base()?); } @@ -173,7 +173,7 @@ impl FromApubToForm for CommunityForm { .url() .context(location_info!())? .as_single_xsd_any_uri() - .map(|u| u.to_string()), + .map(|u| u.to_owned().into()), ), None => None, }; @@ -185,7 +185,7 @@ impl FromApubToForm for CommunityForm { .url() .context(location_info!())? .as_single_xsd_any_uri() - .map(|u| u.to_string()), + .map(|u| u.to_owned().into()), ), None => None, }; diff --git a/crates/apub/src/objects/mod.rs b/crates/apub/src/objects/mod.rs index 5667e100..6b59e577 100644 --- a/crates/apub/src/objects/mod.rs +++ b/crates/apub/src/objects/mod.rs @@ -14,7 +14,7 @@ use chrono::NaiveDateTime; use diesel::result::Error::NotFound; use lemmy_api_structs::blocking; use lemmy_db_queries::{ApubObject, Crud, DbPool}; -use lemmy_db_schema::source::community::Community; +use lemmy_db_schema::{source::community::Community, DbUrl}; use lemmy_utils::{ location_info, settings::structs::Settings, @@ -96,7 +96,7 @@ where pub(in crate::objects) fn check_object_domain( apub: &T, expected_domain: Url, -) -> Result +) -> Result where T: Base + AsBase, { diff --git a/crates/apub/src/objects/post.rs b/crates/apub/src/objects/post.rs index 841b3806..b066e6f8 100644 --- a/crates/apub/src/objects/post.rs +++ b/crates/apub/src/objects/post.rs @@ -24,10 +24,13 @@ use activitystreams_ext::Ext1; use anyhow::Context; use lemmy_api_structs::blocking; use lemmy_db_queries::{Crud, DbPool}; -use lemmy_db_schema::source::{ - community::Community, - post::{Post, PostForm}, - user::User_, +use lemmy_db_schema::{ + self, + source::{ + community::Community, + post::{Post, PostForm}, + user::User_, + }, }; use lemmy_utils::{ location_info, @@ -70,16 +73,13 @@ impl ToApub for Post { set_content_and_source(&mut page, &body)?; } - // TODO: hacky code because we get self.url == Some("") - // https://github.com/LemmyNet/lemmy/issues/602 - let url = self.url.as_ref().filter(|u| !u.is_empty()); - if let Some(u) = url { - page.set_url(Url::parse(u)?); + if let Some(url) = &self.url { + page.set_url::(url.to_owned().into()); } if let Some(thumbnail_url) = &self.thumbnail_url { let mut image = Image::new(); - image.set_url(Url::parse(thumbnail_url)?); + image.set_url::(thumbnail_url.to_owned().into()); page.set_image(image.into_any_base()?); } @@ -146,7 +146,7 @@ impl FromApubToForm for PostForm { let community = get_to_community(page, context, request_counter).await?; - let thumbnail_url = match &page.inner.image() { + let thumbnail_url: Option = match &page.inner.image() { Some(any_image) => Image::from_any_base( any_image .to_owned() @@ -158,7 +158,7 @@ impl FromApubToForm for PostForm { .url() .context(location_info!())? .as_single_xsd_any_uri() - .map(|u| u.to_string()), + .map(|url| url.to_owned()), None => None, }; let url = page @@ -166,11 +166,11 @@ impl FromApubToForm for PostForm { .url() .map(|u| u.as_single_xsd_any_uri()) .flatten() - .map(|s| s.to_string()); + .map(|u| u.to_owned()); let (iframely_title, iframely_description, iframely_html, pictrs_thumbnail) = if let Some(url) = &url { - fetch_iframely_and_pictrs_data(context.client(), Some(url.to_owned())).await + fetch_iframely_and_pictrs_data(context.client(), Some(url)).await } else { (None, None, None, thumbnail_url) }; @@ -192,7 +192,7 @@ impl FromApubToForm for PostForm { let body_slurs_removed = body.map(|b| remove_slurs(&b)); Ok(PostForm { name, - url, + url: url.map(|u| u.into()), body: body_slurs_removed, creator_id: creator.id, community_id: community.id, @@ -214,7 +214,7 @@ impl FromApubToForm for PostForm { embed_title: iframely_title, embed_description: iframely_description, embed_html: iframely_html, - thumbnail_url: pictrs_thumbnail, + thumbnail_url: pictrs_thumbnail.map(|u| u.into()), ap_id: Some(check_object_domain(page, expected_domain)?), local: false, }) diff --git a/crates/apub/src/objects/user.rs b/crates/apub/src/objects/user.rs index c979e3ee..83f75e49 100644 --- a/crates/apub/src/objects/user.rs +++ b/crates/apub/src/objects/user.rs @@ -50,13 +50,13 @@ impl ToApub for User_ { if let Some(avatar_url) = &self.avatar { let mut image = Image::new(); - image.set_url(Url::parse(avatar_url)?); + image.set_url::(avatar_url.to_owned().into()); person.set_icon(image.into_any_base()?); } if let Some(banner_url) = &self.banner { let mut image = Image::new(); - image.set_url(Url::parse(banner_url)?); + image.set_url::(banner_url.to_owned().into()); person.set_image(image.into_any_base()?); } @@ -126,7 +126,7 @@ impl FromApubToForm for UserForm { .url() .context(location_info!())? .as_single_xsd_any_uri() - .map(|u| u.to_string()), + .map(|url| url.to_owned()), ), None => None, }; @@ -139,7 +139,7 @@ impl FromApubToForm for UserForm { .url() .context(location_info!())? .as_single_xsd_any_uri() - .map(|u| u.to_string()), + .map(|url| url.to_owned()), ), None => None, }; @@ -174,8 +174,8 @@ impl FromApubToForm for UserForm { admin: false, banned: None, email: None, - avatar, - banner, + avatar: avatar.map(|o| o.map(|i| i.into())), + banner: banner.map(|o| o.map(|i| i.into())), published: person.inner.published().map(|u| u.to_owned().naive_local()), updated: person.updated().map(|u| u.to_owned().naive_local()), show_nsfw: false, diff --git a/crates/db_queries/Cargo.toml b/crates/db_queries/Cargo.toml index 0e489e22..c950eea9 100644 --- a/crates/db_queries/Cargo.toml +++ b/crates/db_queries/Cargo.toml @@ -20,7 +20,7 @@ strum = "0.20.0" strum_macros = "0.20.1" log = "0.4.14" sha2 = "0.9.3" -url = { version = "2.2.0", features = ["serde"] } +url = { version = "2.2.1", features = ["serde"] } lazy_static = "1.4.0" regex = "1.4.3" bcrypt = "0.9.0" diff --git a/crates/db_queries/src/lib.rs b/crates/db_queries/src/lib.rs index 5667b426..c36bc34a 100644 --- a/crates/db_queries/src/lib.rs +++ b/crates/db_queries/src/lib.rs @@ -13,10 +13,12 @@ extern crate diesel_migrations; extern crate serial_test; use diesel::{result::Error, *}; -use lemmy_db_schema::Url; +use lemmy_db_schema::DbUrl; +use lemmy_utils::ApiError; use regex::Regex; use serde::{Deserialize, Serialize}; use std::{env, env::VarError}; +use url::Url; pub mod aggregates; pub mod source; @@ -112,7 +114,7 @@ pub trait Reportable { } pub trait ApubObject { - fn read_from_apub_id(conn: &PgConnection, object_id: &Url) -> Result + fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result where Self: Sized; fn upsert(conn: &PgConnection, user_form: &T) -> Result @@ -219,6 +221,20 @@ pub fn diesel_option_overwrite(opt: &Option) -> Option> { } } +pub fn diesel_option_overwrite_to_url( + opt: &Option, +) -> Result>, ApiError> { + match opt.as_ref().map(|s| s.as_str()) { + // An empty string is an erase + Some("") => Ok(Some(None)), + Some(str_url) => match Url::parse(str_url) { + Ok(url) => Ok(Some(Some(url.into()))), + Err(_) => Err(ApiError::err("invalid_url")), + }, + None => Ok(None), + } +} + embed_migrations!(); pub fn establish_unpooled_connection() -> PgConnection { @@ -250,7 +266,7 @@ pub mod functions { #[cfg(test)] mod tests { - use super::fuzzy_search; + use super::{fuzzy_search, *}; use crate::is_email_regex; #[test] @@ -264,4 +280,32 @@ mod tests { assert!(is_email_regex("gush@gmail.com")); assert!(!is_email_regex("nada_neutho")); } + + #[test] + fn test_diesel_option_overwrite() { + assert_eq!(diesel_option_overwrite(&None), None); + assert_eq!(diesel_option_overwrite(&Some("".to_string())), Some(None)); + assert_eq!( + diesel_option_overwrite(&Some("test".to_string())), + Some(Some("test".to_string())) + ); + } + + #[test] + fn test_diesel_option_overwrite_to_url() { + assert!(matches!(diesel_option_overwrite_to_url(&None), Ok(None))); + assert!(matches!( + diesel_option_overwrite_to_url(&Some("".to_string())), + Ok(Some(None)) + )); + assert!(matches!( + diesel_option_overwrite_to_url(&Some("invalid_url".to_string())), + Err(_) + )); + let example_url = "https://example.com"; + assert!(matches!( + diesel_option_overwrite_to_url(&Some(example_url.to_string())), + Ok(Some(Some(url))) if url == Url::parse(&example_url).unwrap().into() + )); + } } diff --git a/crates/db_queries/src/source/activity.rs b/crates/db_queries/src/source/activity.rs index cf946d67..56b904e8 100644 --- a/crates/db_queries/src/source/activity.rs +++ b/crates/db_queries/src/source/activity.rs @@ -1,6 +1,6 @@ use crate::Crud; use diesel::{dsl::*, result::Error, sql_types::Text, *}; -use lemmy_db_schema::{source::activity::*, Url}; +use lemmy_db_schema::{source::activity::*, DbUrl}; use log::debug; use serde::Serialize; use serde_json::Value; @@ -41,7 +41,7 @@ impl Crud for Activity { pub trait Activity_ { fn insert( conn: &PgConnection, - ap_id: String, + ap_id: DbUrl, data: &T, local: bool, sensitive: bool, @@ -49,20 +49,20 @@ pub trait Activity_ { where T: Serialize + Debug; - fn read_from_apub_id(conn: &PgConnection, object_id: &str) -> Result; + fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result; fn delete_olds(conn: &PgConnection) -> Result; /// Returns up to 20 activities of type `Announce/Create/Page` from the community fn read_community_outbox( conn: &PgConnection, - community_actor_id: &Url, + community_actor_id: &DbUrl, ) -> Result, Error>; } impl Activity_ for Activity { fn insert( conn: &PgConnection, - ap_id: String, + ap_id: DbUrl, data: &T, local: bool, sensitive: bool, @@ -88,7 +88,7 @@ impl Activity_ for Activity { } } - fn read_from_apub_id(conn: &PgConnection, object_id: &str) -> Result { + fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result { use lemmy_db_schema::schema::activity::dsl::*; activity.filter(ap_id.eq(object_id)).first::(conn) } @@ -100,7 +100,7 @@ impl Activity_ for Activity { fn read_community_outbox( conn: &PgConnection, - community_actor_id: &Url, + community_actor_id: &DbUrl, ) -> Result, Error> { use lemmy_db_schema::schema::activity::dsl::*; let res: Vec = activity @@ -121,6 +121,7 @@ impl Activity_ for Activity { #[cfg(test)] mod tests { + use super::*; use crate::{ establish_unpooled_connection, source::activity::Activity_, @@ -134,6 +135,7 @@ mod tests { }; use serde_json::Value; use serial_test::serial; + use url::Url; #[test] #[serial] @@ -171,8 +173,11 @@ mod tests { let inserted_creator = User_::create(&conn, &creator_form).unwrap(); - let ap_id = - "https://enterprise.lemmy.ml/activities/delete/f1b5d57c-80f8-4e03-a615-688d552e946c"; + let ap_id: DbUrl = Url::parse( + "https://enterprise.lemmy.ml/activities/delete/f1b5d57c-80f8-4e03-a615-688d552e946c", + ) + .unwrap() + .into(); let test_json: Value = serde_json::from_str( r#"{ "@context": "https://www.w3.org/ns/activitystreams", @@ -188,7 +193,7 @@ mod tests { ) .unwrap(); let activity_form = ActivityForm { - ap_id: ap_id.to_string(), + ap_id: ap_id.clone(), data: test_json.to_owned(), local: true, sensitive: false, @@ -198,7 +203,7 @@ mod tests { let inserted_activity = Activity::create(&conn, &activity_form).unwrap(); let expected_activity = Activity { - ap_id: Some(ap_id.to_string()), + ap_id: Some(ap_id.clone()), id: inserted_activity.id, data: test_json, local: true, @@ -208,7 +213,7 @@ mod tests { }; let read_activity = Activity::read(&conn, inserted_activity.id).unwrap(); - let read_activity_by_apub_id = Activity::read_from_apub_id(&conn, ap_id).unwrap(); + let read_activity_by_apub_id = Activity::read_from_apub_id(&conn, &ap_id).unwrap(); User_::delete(&conn, inserted_creator.id).unwrap(); Activity::delete(&conn, inserted_activity.id).unwrap(); diff --git a/crates/db_queries/src/source/comment.rs b/crates/db_queries/src/source/comment.rs index 918ae497..8dc9050c 100644 --- a/crates/db_queries/src/source/comment.rs +++ b/crates/db_queries/src/source/comment.rs @@ -10,11 +10,11 @@ use lemmy_db_schema::{ CommentSaved, CommentSavedForm, }, - Url, + DbUrl, }; pub trait Comment_ { - fn update_ap_id(conn: &PgConnection, comment_id: i32, apub_id: Url) -> Result; + fn update_ap_id(conn: &PgConnection, comment_id: i32, apub_id: DbUrl) -> Result; fn permadelete_for_creator( conn: &PgConnection, for_creator_id: i32, @@ -43,7 +43,7 @@ pub trait Comment_ { } impl Comment_ for Comment { - fn update_ap_id(conn: &PgConnection, comment_id: i32, apub_id: Url) -> Result { + fn update_ap_id(conn: &PgConnection, comment_id: i32, apub_id: DbUrl) -> Result { use lemmy_db_schema::schema::comment::dsl::*; diesel::update(comment.find(comment_id)) @@ -145,7 +145,7 @@ impl Crud for Comment { } impl ApubObject for Comment { - fn read_from_apub_id(conn: &PgConnection, object_id: &Url) -> Result { + fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result { use lemmy_db_schema::schema::comment::dsl::*; comment.filter(ap_id.eq(object_id)).first::(conn) } diff --git a/crates/db_queries/src/source/community.rs b/crates/db_queries/src/source/community.rs index 6a19d221..03484816 100644 --- a/crates/db_queries/src/source/community.rs +++ b/crates/db_queries/src/source/community.rs @@ -12,7 +12,7 @@ use lemmy_db_schema::{ CommunityUserBan, CommunityUserBanForm, }, - Url, + DbUrl, }; mod safe_type { @@ -90,7 +90,7 @@ impl Crud for Community { } impl ApubObject for Community { - fn read_from_apub_id(conn: &PgConnection, for_actor_id: &Url) -> Result { + fn read_from_apub_id(conn: &PgConnection, for_actor_id: &DbUrl) -> Result { use lemmy_db_schema::schema::community::dsl::*; community .filter(actor_id.eq(for_actor_id)) @@ -131,7 +131,10 @@ pub trait Community_ { new_creator_id: i32, ) -> Result; fn distinct_federated_communities(conn: &PgConnection) -> Result, Error>; - fn read_from_followers_url(conn: &PgConnection, followers_url: &Url) -> Result; + fn read_from_followers_url( + conn: &PgConnection, + followers_url: &DbUrl, + ) -> Result; } impl Community_ for Community { @@ -194,7 +197,7 @@ impl Community_ for Community { fn read_from_followers_url( conn: &PgConnection, - followers_url_: &Url, + followers_url_: &DbUrl, ) -> Result { use lemmy_db_schema::schema::community::dsl::*; community diff --git a/crates/db_queries/src/source/post.rs b/crates/db_queries/src/source/post.rs index f6b1342e..f105dc73 100644 --- a/crates/db_queries/src/source/post.rs +++ b/crates/db_queries/src/source/post.rs @@ -12,7 +12,7 @@ use lemmy_db_schema::{ PostSaved, PostSavedForm, }, - Url, + DbUrl, }; impl Crud for Post { @@ -42,7 +42,7 @@ impl Crud for Post { pub trait Post_ { //fn read(conn: &PgConnection, post_id: i32) -> Result; fn list_for_community(conn: &PgConnection, the_community_id: i32) -> Result, Error>; - fn update_ap_id(conn: &PgConnection, post_id: i32, apub_id: Url) -> Result; + fn update_ap_id(conn: &PgConnection, post_id: i32, apub_id: DbUrl) -> Result; fn permadelete_for_creator(conn: &PgConnection, for_creator_id: i32) -> Result, Error>; fn update_deleted(conn: &PgConnection, post_id: i32, new_deleted: bool) -> Result; fn update_removed(conn: &PgConnection, post_id: i32, new_removed: bool) -> Result; @@ -68,7 +68,7 @@ impl Post_ for Post { .load::(conn) } - fn update_ap_id(conn: &PgConnection, post_id: i32, apub_id: Url) -> Result { + fn update_ap_id(conn: &PgConnection, post_id: i32, apub_id: DbUrl) -> Result { use lemmy_db_schema::schema::post::dsl::*; diesel::update(post.find(post_id)) @@ -147,7 +147,7 @@ impl Post_ for Post { } impl ApubObject for Post { - fn read_from_apub_id(conn: &PgConnection, object_id: &Url) -> Result { + fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result { use lemmy_db_schema::schema::post::dsl::*; post.filter(ap_id.eq(object_id)).first::(conn) } diff --git a/crates/db_queries/src/source/private_message.rs b/crates/db_queries/src/source/private_message.rs index f1908bc1..c437252e 100644 --- a/crates/db_queries/src/source/private_message.rs +++ b/crates/db_queries/src/source/private_message.rs @@ -1,6 +1,6 @@ use crate::{ApubObject, Crud}; use diesel::{dsl::*, result::Error, *}; -use lemmy_db_schema::{naive_now, source::private_message::*, Url}; +use lemmy_db_schema::{naive_now, source::private_message::*, DbUrl}; impl Crud for PrivateMessage { fn read(conn: &PgConnection, private_message_id: i32) -> Result { @@ -28,7 +28,7 @@ impl Crud for PrivateMessage { } impl ApubObject for PrivateMessage { - fn read_from_apub_id(conn: &PgConnection, object_id: &Url) -> Result + fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result where Self: Sized, { @@ -53,7 +53,7 @@ pub trait PrivateMessage_ { fn update_ap_id( conn: &PgConnection, private_message_id: i32, - apub_id: Url, + apub_id: DbUrl, ) -> Result; fn update_content( conn: &PgConnection, @@ -80,7 +80,7 @@ impl PrivateMessage_ for PrivateMessage { fn update_ap_id( conn: &PgConnection, private_message_id: i32, - apub_id: Url, + apub_id: DbUrl, ) -> Result { use lemmy_db_schema::schema::private_message::dsl::*; diff --git a/crates/db_queries/src/source/user.rs b/crates/db_queries/src/source/user.rs index 56023f09..d0e7411a 100644 --- a/crates/db_queries/src/source/user.rs +++ b/crates/db_queries/src/source/user.rs @@ -5,7 +5,7 @@ use lemmy_db_schema::{ naive_now, schema::user_::dsl::*, source::user::{UserForm, UserSafeSettings, User_}, - Url, + DbUrl, }; use lemmy_utils::settings::structs::Settings; @@ -242,7 +242,7 @@ impl Crud for User_ { } impl ApubObject for User_ { - fn read_from_apub_id(conn: &PgConnection, object_id: &Url) -> Result { + fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result { use lemmy_db_schema::schema::user_::dsl::*; user_ .filter(deleted.eq(false)) diff --git a/crates/db_schema/Cargo.toml b/crates/db_schema/Cargo.toml index b637c022..1da8b68f 100644 --- a/crates/db_schema/Cargo.toml +++ b/crates/db_schema/Cargo.toml @@ -12,4 +12,4 @@ chrono = { version = "0.4.19", features = ["serde"] } serde = { version = "1.0.123", features = ["derive"] } serde_json = { version = "1.0.61", features = ["preserve_order"] } log = "0.4.14" -url = { version = "2.2.0", features = ["serde"] } +url = { version = "2.2.1", features = ["serde"] } diff --git a/crates/db_schema/src/lib.rs b/crates/db_schema/src/lib.rs index b0733884..f44567b9 100644 --- a/crates/db_schema/src/lib.rs +++ b/crates/db_schema/src/lib.rs @@ -8,21 +8,22 @@ use diesel::{ serialize::{Output, ToSql}, sql_types::Text, }; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use std::{ fmt::{Display, Formatter}, io::Write, }; +use url::Url; pub mod schema; pub mod source; #[repr(transparent)] -#[derive(Clone, PartialEq, Serialize, Debug, AsExpression, FromSqlRow)] +#[derive(Clone, PartialEq, Serialize, Deserialize, Debug, AsExpression, FromSqlRow)] #[sql_type = "Text"] -pub struct Url(url::Url); +pub struct DbUrl(Url); -impl ToSql for Url +impl ToSql for DbUrl where String: ToSql, { @@ -31,37 +32,37 @@ where } } -impl FromSql for Url +impl FromSql for DbUrl where String: FromSql, { fn from_sql(bytes: Option<&DB::RawValue>) -> diesel::deserialize::Result { let str = String::from_sql(bytes)?; - Ok(Url(url::Url::parse(&str)?)) + Ok(DbUrl(Url::parse(&str)?)) } } -impl Url { - pub fn into_inner(self) -> url::Url { +impl DbUrl { + pub fn into_inner(self) -> Url { self.0 } } -impl Display for Url { +impl Display for DbUrl { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { self.to_owned().into_inner().fmt(f) } } -impl From for url::Url { - fn from(url: Url) -> Self { +impl From for Url { + fn from(url: DbUrl) -> Self { url.0 } } -impl From for Url { - fn from(url: url::Url) -> Self { - Url(url) +impl From for DbUrl { + fn from(url: Url) -> Self { + DbUrl(url) } } diff --git a/crates/db_schema/src/source/activity.rs b/crates/db_schema/src/source/activity.rs index cf81ab8c..7b7f4aba 100644 --- a/crates/db_schema/src/source/activity.rs +++ b/crates/db_schema/src/source/activity.rs @@ -1,4 +1,4 @@ -use crate::schema::activity; +use crate::{schema::activity, DbUrl}; use serde_json::Value; use std::fmt::Debug; @@ -10,7 +10,7 @@ pub struct Activity { pub local: bool, pub published: chrono::NaiveDateTime, pub updated: Option, - pub ap_id: Option, + pub ap_id: Option, pub sensitive: Option, } @@ -20,6 +20,6 @@ pub struct ActivityForm { pub data: Value, pub local: bool, pub updated: Option, - pub ap_id: String, + pub ap_id: DbUrl, pub sensitive: bool, } diff --git a/crates/db_schema/src/source/comment.rs b/crates/db_schema/src/source/comment.rs index 72b9e740..a00738a0 100644 --- a/crates/db_schema/src/source/comment.rs +++ b/crates/db_schema/src/source/comment.rs @@ -1,7 +1,7 @@ use crate::{ schema::{comment, comment_alias_1, comment_like, comment_saved}, source::post::Post, - Url, + DbUrl, }; use serde::Serialize; @@ -26,7 +26,7 @@ pub struct Comment { pub published: chrono::NaiveDateTime, pub updated: Option, pub deleted: bool, - pub ap_id: Url, + pub ap_id: DbUrl, pub local: bool, } @@ -44,7 +44,7 @@ pub struct CommentAlias1 { pub published: chrono::NaiveDateTime, pub updated: Option, pub deleted: bool, - pub ap_id: Url, + pub ap_id: DbUrl, pub local: bool, } @@ -60,7 +60,7 @@ pub struct CommentForm { pub published: Option, pub updated: Option, pub deleted: Option, - pub ap_id: Option, + pub ap_id: Option, pub local: bool, } diff --git a/crates/db_schema/src/source/community.rs b/crates/db_schema/src/source/community.rs index b8702ca9..b9fe1249 100644 --- a/crates/db_schema/src/source/community.rs +++ b/crates/db_schema/src/source/community.rs @@ -1,6 +1,6 @@ use crate::{ schema::{community, community_follower, community_moderator, community_user_ban}, - Url, + DbUrl, }; use serde::Serialize; @@ -17,16 +17,16 @@ pub struct Community { pub updated: Option, pub deleted: bool, pub nsfw: bool, - pub actor_id: Url, + pub actor_id: DbUrl, pub local: bool, pub private_key: Option, pub public_key: Option, pub last_refreshed_at: chrono::NaiveDateTime, - pub icon: Option, - pub banner: Option, - pub followers_url: Url, - pub inbox_url: Url, - pub shared_inbox_url: Option, + pub icon: Option, + pub banner: Option, + pub followers_url: DbUrl, + pub inbox_url: DbUrl, + pub shared_inbox_url: Option, } /// A safe representation of community, without the sensitive info @@ -43,10 +43,10 @@ pub struct CommunitySafe { pub updated: Option, pub deleted: bool, pub nsfw: bool, - pub actor_id: Url, + pub actor_id: DbUrl, pub local: bool, - pub icon: Option, - pub banner: Option, + pub icon: Option, + pub banner: Option, } #[derive(Insertable, AsChangeset, Debug)] @@ -61,16 +61,16 @@ pub struct CommunityForm { pub updated: Option, pub deleted: Option, pub nsfw: bool, - pub actor_id: Option, + pub actor_id: Option, pub local: bool, pub private_key: Option, pub public_key: Option, pub last_refreshed_at: Option, - pub icon: Option>, - pub banner: Option>, - pub followers_url: Option, - pub inbox_url: Option, - pub shared_inbox_url: Option>, + pub icon: Option>, + pub banner: Option>, + pub followers_url: Option, + pub inbox_url: Option, + pub shared_inbox_url: Option>, } #[derive(Identifiable, Queryable, Associations, PartialEq, Debug)] diff --git a/crates/db_schema/src/source/post.rs b/crates/db_schema/src/source/post.rs index 4ec6b56d..c8cc51cd 100644 --- a/crates/db_schema/src/source/post.rs +++ b/crates/db_schema/src/source/post.rs @@ -1,6 +1,6 @@ use crate::{ schema::{post, post_like, post_read, post_saved}, - Url, + DbUrl, }; use serde::Serialize; @@ -9,7 +9,7 @@ use serde::Serialize; pub struct Post { pub id: i32, pub name: String, - pub url: Option, + pub url: Option, pub body: Option, pub creator_id: i32, pub community_id: i32, @@ -23,8 +23,8 @@ pub struct Post { pub embed_title: Option, pub embed_description: Option, pub embed_html: Option, - pub thumbnail_url: Option, - pub ap_id: Url, + pub thumbnail_url: Option, + pub ap_id: DbUrl, pub local: bool, } @@ -32,7 +32,7 @@ pub struct Post { #[table_name = "post"] pub struct PostForm { pub name: String, - pub url: Option, + pub url: Option, pub body: Option, pub creator_id: i32, pub community_id: i32, @@ -46,8 +46,8 @@ pub struct PostForm { pub embed_title: Option, pub embed_description: Option, pub embed_html: Option, - pub thumbnail_url: Option, - pub ap_id: Option, + pub thumbnail_url: Option, + pub ap_id: Option, pub local: bool, } diff --git a/crates/db_schema/src/source/post_report.rs b/crates/db_schema/src/source/post_report.rs index b75fb954..62ef31cd 100644 --- a/crates/db_schema/src/source/post_report.rs +++ b/crates/db_schema/src/source/post_report.rs @@ -1,4 +1,4 @@ -use crate::{schema::post_report, source::post::Post}; +use crate::{schema::post_report, source::post::Post, DbUrl}; use serde::{Deserialize, Serialize}; #[derive( @@ -11,7 +11,7 @@ pub struct PostReport { pub creator_id: i32, pub post_id: i32, pub original_post_name: String, - pub original_post_url: Option, + pub original_post_url: Option, pub original_post_body: Option, pub reason: String, pub resolved: bool, @@ -26,7 +26,7 @@ pub struct PostReportForm { pub creator_id: i32, pub post_id: i32, pub original_post_name: String, - pub original_post_url: Option, + pub original_post_url: Option, pub original_post_body: Option, pub reason: String, } diff --git a/crates/db_schema/src/source/private_message.rs b/crates/db_schema/src/source/private_message.rs index 376728a1..949c9770 100644 --- a/crates/db_schema/src/source/private_message.rs +++ b/crates/db_schema/src/source/private_message.rs @@ -1,4 +1,4 @@ -use crate::{schema::private_message, Url}; +use crate::{schema::private_message, DbUrl}; use serde::Serialize; #[derive(Clone, Queryable, Associations, Identifiable, PartialEq, Debug, Serialize)] @@ -12,7 +12,7 @@ pub struct PrivateMessage { pub read: bool, pub published: chrono::NaiveDateTime, pub updated: Option, - pub ap_id: Url, + pub ap_id: DbUrl, pub local: bool, } @@ -26,6 +26,6 @@ pub struct PrivateMessageForm { pub read: Option, pub published: Option, pub updated: Option, - pub ap_id: Option, + pub ap_id: Option, pub local: bool, } diff --git a/crates/db_schema/src/source/site.rs b/crates/db_schema/src/source/site.rs index 66319548..998e9f9d 100644 --- a/crates/db_schema/src/source/site.rs +++ b/crates/db_schema/src/source/site.rs @@ -1,4 +1,4 @@ -use crate::schema::site; +use crate::{schema::site, DbUrl}; use serde::Serialize; #[derive(Queryable, Identifiable, PartialEq, Debug, Clone, Serialize)] @@ -13,8 +13,8 @@ pub struct Site { pub enable_downvotes: bool, pub open_registration: bool, pub enable_nsfw: bool, - pub icon: Option, - pub banner: Option, + pub icon: Option, + pub banner: Option, } #[derive(Insertable, AsChangeset)] @@ -28,6 +28,6 @@ pub struct SiteForm { pub open_registration: bool, pub enable_nsfw: bool, // when you want to null out a column, you have to send Some(None)), since sending None means you just don't want to update that column. - pub icon: Option>, - pub banner: Option>, + pub icon: Option>, + pub banner: Option>, } diff --git a/crates/db_schema/src/source/user.rs b/crates/db_schema/src/source/user.rs index d72929fa..f04b9a60 100644 --- a/crates/db_schema/src/source/user.rs +++ b/crates/db_schema/src/source/user.rs @@ -1,6 +1,6 @@ use crate::{ schema::{user_, user_alias_1, user_alias_2}, - Url, + DbUrl, }; use serde::Serialize; @@ -12,7 +12,7 @@ pub struct User_ { pub preferred_username: Option, pub password_encrypted: String, pub email: Option, - pub avatar: Option, + pub avatar: Option, pub admin: bool, pub banned: bool, pub published: chrono::NaiveDateTime, @@ -25,16 +25,16 @@ pub struct User_ { pub show_avatars: bool, pub send_notifications_to_email: bool, pub matrix_user_id: Option, - pub actor_id: Url, + pub actor_id: DbUrl, pub bio: Option, pub local: bool, pub private_key: Option, pub public_key: Option, pub last_refreshed_at: chrono::NaiveDateTime, - pub banner: Option, + pub banner: Option, pub deleted: bool, - pub inbox_url: Url, - pub shared_inbox_url: Option, + pub inbox_url: DbUrl, + pub shared_inbox_url: Option, } /// A safe representation of user, without the sensitive info @@ -44,19 +44,19 @@ pub struct UserSafe { pub id: i32, pub name: String, pub preferred_username: Option, - pub avatar: Option, + pub avatar: Option, pub admin: bool, pub banned: bool, pub published: chrono::NaiveDateTime, pub updated: Option, pub matrix_user_id: Option, - pub actor_id: Url, + pub actor_id: DbUrl, pub bio: Option, pub local: bool, - pub banner: Option, + pub banner: Option, pub deleted: bool, - pub inbox_url: Url, - pub shared_inbox_url: Option, + pub inbox_url: DbUrl, + pub shared_inbox_url: Option, } /// A safe user view with only settings @@ -67,7 +67,7 @@ pub struct UserSafeSettings { pub name: String, pub preferred_username: Option, pub email: Option, - pub avatar: Option, + pub avatar: Option, pub admin: bool, pub banned: bool, pub published: chrono::NaiveDateTime, @@ -80,11 +80,11 @@ pub struct UserSafeSettings { pub show_avatars: bool, pub send_notifications_to_email: bool, pub matrix_user_id: Option, - pub actor_id: Url, + pub actor_id: DbUrl, pub bio: Option, pub local: bool, pub last_refreshed_at: chrono::NaiveDateTime, - pub banner: Option, + pub banner: Option, pub deleted: bool, } @@ -96,7 +96,7 @@ pub struct UserAlias1 { pub preferred_username: Option, pub password_encrypted: String, pub email: Option, - pub avatar: Option, + pub avatar: Option, pub admin: bool, pub banned: bool, pub published: chrono::NaiveDateTime, @@ -109,13 +109,13 @@ pub struct UserAlias1 { pub show_avatars: bool, pub send_notifications_to_email: bool, pub matrix_user_id: Option, - pub actor_id: Url, + pub actor_id: DbUrl, pub bio: Option, pub local: bool, pub private_key: Option, pub public_key: Option, pub last_refreshed_at: chrono::NaiveDateTime, - pub banner: Option, + pub banner: Option, pub deleted: bool, } @@ -125,16 +125,16 @@ pub struct UserSafeAlias1 { pub id: i32, pub name: String, pub preferred_username: Option, - pub avatar: Option, + pub avatar: Option, pub admin: bool, pub banned: bool, pub published: chrono::NaiveDateTime, pub updated: Option, pub matrix_user_id: Option, - pub actor_id: Url, + pub actor_id: DbUrl, pub bio: Option, pub local: bool, - pub banner: Option, + pub banner: Option, pub deleted: bool, } @@ -146,7 +146,7 @@ pub struct UserAlias2 { pub preferred_username: Option, pub password_encrypted: String, pub email: Option, - pub avatar: Option, + pub avatar: Option, pub admin: bool, pub banned: bool, pub published: chrono::NaiveDateTime, @@ -159,13 +159,13 @@ pub struct UserAlias2 { pub show_avatars: bool, pub send_notifications_to_email: bool, pub matrix_user_id: Option, - pub actor_id: Url, + pub actor_id: DbUrl, pub bio: Option, pub local: bool, pub private_key: Option, pub public_key: Option, pub last_refreshed_at: chrono::NaiveDateTime, - pub banner: Option, + pub banner: Option, pub deleted: bool, } @@ -175,16 +175,16 @@ pub struct UserSafeAlias2 { pub id: i32, pub name: String, pub preferred_username: Option, - pub avatar: Option, + pub avatar: Option, pub admin: bool, pub banned: bool, pub published: chrono::NaiveDateTime, pub updated: Option, pub matrix_user_id: Option, - pub actor_id: Url, + pub actor_id: DbUrl, pub bio: Option, pub local: bool, - pub banner: Option, + pub banner: Option, pub deleted: bool, } @@ -197,7 +197,7 @@ pub struct UserForm { pub admin: bool, pub banned: Option, pub email: Option>, - pub avatar: Option>, + pub avatar: Option>, pub published: Option, pub updated: Option, pub show_nsfw: bool, @@ -208,13 +208,13 @@ pub struct UserForm { pub show_avatars: bool, pub send_notifications_to_email: bool, pub matrix_user_id: Option>, - pub actor_id: Option, + pub actor_id: Option, pub bio: Option>, pub local: bool, pub private_key: Option, pub public_key: Option, pub last_refreshed_at: Option, - pub banner: Option>, - pub inbox_url: Option, - pub shared_inbox_url: Option>, + pub banner: Option>, + pub inbox_url: Option, + pub shared_inbox_url: Option>, } diff --git a/crates/db_views/Cargo.toml b/crates/db_views/Cargo.toml index e44f414a..42a942b4 100644 --- a/crates/db_views/Cargo.toml +++ b/crates/db_views/Cargo.toml @@ -12,7 +12,7 @@ lemmy_db_schema = { path = "../db_schema" } diesel = { version = "1.4.5", features = ["postgres","chrono","r2d2","serde_json"] } serde = { version = "1.0.123", features = ["derive"] } log = "0.4.14" -url = "2.2.0" +url = "2.2.1" [dev-dependencies] serial_test = "0.5.1" \ No newline at end of file diff --git a/crates/routes/Cargo.toml b/crates/routes/Cargo.toml index 80b94aa7..b6b464c3 100644 --- a/crates/routes/Cargo.toml +++ b/crates/routes/Cargo.toml @@ -25,6 +25,6 @@ chrono = { version = "0.4.19", features = ["serde"] } rss = "1.10.0" serde = { version = "1.0.123", features = ["derive"] } awc = { version = "2.0.3", default-features = false } -url = { version = "2.2.0", features = ["serde"] } +url = { version = "2.2.1", features = ["serde"] } strum = "0.20.0" lazy_static = "1.4.0" diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml index bb87cd30..f8c2012d 100644 --- a/crates/utils/Cargo.toml +++ b/crates/utils/Cargo.toml @@ -22,7 +22,7 @@ thiserror = "1.0.23" comrak = { version = "0.9.0", default-features = false } lazy_static = "1.4.0" openssl = "0.10.32" -url = { version = "2.2.0", features = ["serde"] } +url = { version = "2.2.1", features = ["serde"] } actix-web = { version = "3.3.2", default-features = false, features = ["rustls"] } actix-rt = { version = "1.1.1", default-features = false } anyhow = "1.0.38" diff --git a/crates/utils/src/request.rs b/crates/utils/src/request.rs index 428d7897..963bc9a2 100644 --- a/crates/utils/src/request.rs +++ b/crates/utils/src/request.rs @@ -6,6 +6,7 @@ use reqwest::Client; use serde::Deserialize; use std::future::Future; use thiserror::Error; +use url::Url; #[derive(Clone, Debug, Error)] #[error("Error sending request, {0}")] @@ -50,13 +51,13 @@ where pub(crate) struct IframelyResponse { title: Option, description: Option, - thumbnail_url: Option, + thumbnail_url: Option, html: Option, } pub(crate) async fn fetch_iframely( client: &Client, - url: &str, + url: &Url, ) -> Result { let fetch_url = format!("{}/oembed?url={}", Settings::get().iframely_url(), url); @@ -83,14 +84,14 @@ pub(crate) struct PictrsFile { pub(crate) async fn fetch_pictrs( client: &Client, - image_url: &str, + image_url: &Url, ) -> Result { is_image_content_type(client, image_url).await?; let fetch_url = format!( "{}/image/download?url={}", Settings::get().pictrs_url(), - utf8_percent_encode(image_url, NON_ALPHANUMERIC) // TODO this might not be needed + utf8_percent_encode(image_url.as_str(), NON_ALPHANUMERIC) // TODO this might not be needed ); let response = retry(|| client.get(&fetch_url).send()).await?; @@ -109,13 +110,8 @@ pub(crate) async fn fetch_pictrs( pub async fn fetch_iframely_and_pictrs_data( client: &Client, - url: Option, -) -> ( - Option, - Option, - Option, - Option, -) { + url: Option<&Url>, +) -> (Option, Option, Option, Option) { match &url { Some(url) => { // Fetch iframely data @@ -149,11 +145,19 @@ pub async fn fetch_iframely_and_pictrs_data( // The full urls are necessary for federation let pictrs_thumbnail = if let Some(pictrs_hash) = pictrs_hash { - Some(format!( + let url = Url::parse(&format!( "{}/pictrs/image/{}", Settings::get().get_protocol_and_hostname(), pictrs_hash - )) + )); + match url { + Ok(parsed_url) => Some(parsed_url), + Err(e) => { + // This really shouldn't happen unless the settings or hash are malformed + error!("Unexpected error constructing pictrs thumbnail URL: {}", e); + None + } + } } else { None }; @@ -169,9 +173,8 @@ pub async fn fetch_iframely_and_pictrs_data( } } -async fn is_image_content_type(client: &Client, test: &str) -> Result<(), LemmyError> { - let response = retry(|| client.get(test).send()).await?; - +async fn is_image_content_type(client: &Client, test: &Url) -> Result<(), LemmyError> { + let response = retry(|| client.get(test.to_owned()).send()).await?; if response .headers() .get("Content-Type") diff --git a/migrations/2021-02-28-162616_clean_empty_post_urls/down.sql b/migrations/2021-02-28-162616_clean_empty_post_urls/down.sql new file mode 100644 index 00000000..7195601c --- /dev/null +++ b/migrations/2021-02-28-162616_clean_empty_post_urls/down.sql @@ -0,0 +1,4 @@ +-- This is a clean-up migration that cannot be undone, +-- but Diesel requires a non-empty script so run a no-op. +SELECT 1; + diff --git a/migrations/2021-02-28-162616_clean_empty_post_urls/up.sql b/migrations/2021-02-28-162616_clean_empty_post_urls/up.sql new file mode 100644 index 00000000..24e2d16b --- /dev/null +++ b/migrations/2021-02-28-162616_clean_empty_post_urls/up.sql @@ -0,0 +1 @@ +UPDATE post SET url = NULL where url = ''; -- 2.44.1