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

@@ -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",

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"

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"] }

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 icon = diesel_option_overwrite_to_url(&data.icon)?;
-    let banner = diesel_option_overwrite(&data.banner);
+    let banner = diesel_option_overwrite_to_url(&data.banner)?;
-
-    check_optional_url(&icon)?;
-    check_optional_url(&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 icon = diesel_option_overwrite_to_url(&data.icon)?;
-    let banner = diesel_option_overwrite(&data.banner);
+    let banner = diesel_option_overwrite_to_url(&data.banner)?;
-
-    check_optional_url(&icon)?;
-    check_optional_url(&banner)?;
 
     let community_form = CommunityForm {
       name: read_community.name,

crates/api/src/lib.rs

@@ -186,15 +186,6 @@ pub(crate) async fn collect_moderated_communities(
   }
 }
 
-pub(crate) fn check_optional_url(item: &Option<Option<String>>) -> 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<Option<FederatedInstances>, LemmyError> {

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,

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()),
+      icon: Some(data.icon.to_owned().map(|url| url.into())),
-      banner: Some(data.banner.to_owned()),
+      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 icon = diesel_option_overwrite_to_url(&data.icon)?;
-    let banner = diesel_option_overwrite(&data.banner);
+    let banner = diesel_option_overwrite_to_url(&data.banner)?;
 
     let site_form = SiteForm {
       name: data.name.to_owned(),

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 avatar = diesel_option_overwrite_to_url(&data.avatar)?;
-    let banner = diesel_option_overwrite(&data.banner);
+    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());

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"

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<String>,
+  pub url: Option<Url>,
   pub body: Option<String>,
   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<String>,
+  pub url: Option<Url>,
   pub body: Option<String>,
   pub nsfw: bool,
   pub auth: String,

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<String>,
-  pub icon: Option<String>,
+  pub icon: Option<Url>,
-  pub banner: Option<String>,
+  pub banner: Option<Url>,
   pub enable_downvotes: bool,
   pub open_registration: bool,
   pub enable_nsfw: bool,

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"

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<LemmyContext>,
 ) -> Result<HttpResponse<Body>, 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)
   })

crates/apub/src/inbox/mod.rs

@@ -45,7 +45,7 @@ pub(crate) async fn is_activity_already_known(
   pool: &DbPool,
   activity_id: &Url,
 ) -> Result<bool, LemmyError> {
-  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)
   })

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::Post(p)) => receive_delete_post(context, *p).await,
-    Ok(PostOrComment::Comment(c)) => receive_delete_comment(context, c).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::Post(p)) => receive_remove_post(context, remove, *p).await,
-    Ok(PostOrComment::Comment(c)) => receive_remove_comment(context, remove, c).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::Post(p)) => receive_undo_delete_post(context, *p).await,
-    Ok(PostOrComment::Comment(c)) => receive_undo_delete_comment(context, c).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::Post(p)) => receive_undo_remove_post(context, *p).await,
-    Ok(PostOrComment::Comment(c)) => receive_undo_remove_comment(context, c).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<PostOrComment, LemmyError> {
   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())

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::{
+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<lemmy_db_schema::Url, ParseError> {
+) -> Result<DbUrl, ParseError> {
   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(
+pub fn generate_followers_url(actor_id: &DbUrl) -> Result<DbUrl, ParseError> {
-  actor_id: &lemmy_db_schema::Url,
-) -> Result<lemmy_db_schema::Url, ParseError> {
   Ok(Url::parse(&format!("{}/followers", actor_id))?.into())
 }
 
-pub fn generate_inbox_url(
+pub fn generate_inbox_url(actor_id: &DbUrl) -> Result<DbUrl, ParseError> {
-  actor_id: &lemmy_db_schema::Url,
-) -> Result<lemmy_db_schema::Url, ParseError> {
   Ok(Url::parse(&format!("{}/inbox", actor_id))?.into())
 }
 
-pub fn generate_shared_inbox_url(
+pub fn generate_shared_inbox_url(actor_id: &DbUrl) -> Result<DbUrl, LemmyError> {
-  actor_id: &lemmy_db_schema::Url,
-) -> Result<lemmy_db_schema::Url, LemmyError> {
   let actor_id = actor_id.clone().into_inner();
   let url = format!(
     "{}://{}{}/inbox",
@@ -277,7 +274,7 @@ pub(crate) async fn insert_activity<T>(
 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),
+  Comment(Box<Comment>),
-  Post(Post),
+  Post(Box<Post>),
 }
 
 /// 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::Post(p) => Object::Post(*p),
-      PostOrComment::Comment(c) => Object::Comment(c),
+      PostOrComment::Comment(c) => Object::Comment(*c),
     });
   }
 

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::<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::<Url>(banner_url.to_owned().into());
       group.set_image(image.into_any_base()?);
     }
 
@@ -173,7 +173,7 @@ impl FromApubToForm<GroupExt> 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<GroupExt> for CommunityForm {
           .url()
           .context(location_info!())?
           .as_single_xsd_any_uri()
-          .map(|u| u.to_string()),
+          .map(|u| u.to_owned().into()),
       ),
       None => None,
     };

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<T, Kind>(
   apub: &T,
   expected_domain: Url,
-) -> Result<lemmy_db_schema::Url, LemmyError>
+) -> Result<DbUrl, LemmyError>
 where
   T: Base + AsBase<Kind>,
 {

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::{
+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("")
+    if let Some(url) = &self.url {
-    // https://github.com/LemmyNet/lemmy/issues/602
+      page.set_url::<Url>(url.to_owned().into());
-    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(thumbnail_url) = &self.thumbnail_url {
       let mut image = Image::new();
-      image.set_url(Url::parse(thumbnail_url)?);
+      image.set_url::<Url>(thumbnail_url.to_owned().into());
       page.set_image(image.into_any_base()?);
     }
 
@@ -146,7 +146,7 @@ impl FromApubToForm<PageExt> for PostForm {
 
     let community = get_to_community(page, context, request_counter).await?;
 
-    let thumbnail_url = match &page.inner.image() {
+    let thumbnail_url: Option<Url> = match &page.inner.image() {
       Some(any_image) => Image::from_any_base(
         any_image
           .to_owned()
@@ -158,7 +158,7 @@ impl FromApubToForm<PageExt> 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<PageExt> 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<PageExt> 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<PageExt> 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,
     })

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::<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::<Url>(banner_url.to_owned().into());
       person.set_image(image.into_any_base()?);
     }
 
@@ -126,7 +126,7 @@ impl FromApubToForm<PersonExt> 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<PersonExt> 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<PersonExt> for UserForm {
       admin: false,
       banned: None,
       email: None,
-      avatar,
+      avatar: avatar.map(|o| o.map(|i| i.into())),
-      banner,
+      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,

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"

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<T> {
 }
 
 pub trait ApubObject<T> {
-  fn read_from_apub_id(conn: &PgConnection, object_id: &Url) -> Result<Self, Error>
+  fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result<Self, Error>
   where
     Self: Sized;
   fn upsert(conn: &PgConnection, user_form: &T) -> Result<Self, Error>
@@ -219,6 +221,20 @@ pub fn diesel_option_overwrite(opt: &Option<String>) -> Option<Option<String>> {
   }
 }
 
+pub fn diesel_option_overwrite_to_url(
+  opt: &Option<String>,
+) -> Result<Option<Option<DbUrl>>, 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()
+    ));
+  }
 }

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<ActivityForm> for Activity {
 pub trait Activity_ {
   fn insert<T>(
     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<Activity, Error>;
+  fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result<Activity, Error>;
   fn delete_olds(conn: &PgConnection) -> Result<usize, Error>;
 
   /// 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<Vec<Value>, Error>;
 }
 
 impl Activity_ for Activity {
   fn insert<T>(
     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<Activity, Error> {
+  fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result<Activity, Error> {
     use lemmy_db_schema::schema::activity::dsl::*;
     activity.filter(ap_id.eq(object_id)).first::<Self>(conn)
   }
@@ -100,7 +100,7 @@ impl Activity_ for Activity {
 
   fn read_community_outbox(
     conn: &PgConnection,
-    community_actor_id: &Url,
+    community_actor_id: &DbUrl,
   ) -> Result<Vec<Value>, Error> {
     use lemmy_db_schema::schema::activity::dsl::*;
     let res: Vec<Value> = 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 =
+    let ap_id: DbUrl = Url::parse(
-      "https://enterprise.lemmy.ml/activities/delete/f1b5d57c-80f8-4e03-a615-688d552e946c";
+      "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();
 

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<Comment, Error>;
+  fn update_ap_id(conn: &PgConnection, comment_id: i32, apub_id: DbUrl) -> Result<Comment, Error>;
   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<Self, Error> {
+  fn update_ap_id(conn: &PgConnection, comment_id: i32, apub_id: DbUrl) -> Result<Self, Error> {
     use lemmy_db_schema::schema::comment::dsl::*;
 
     diesel::update(comment.find(comment_id))
@@ -145,7 +145,7 @@ impl Crud<CommentForm> for Comment {
 }
 
 impl ApubObject<CommentForm> for Comment {
-  fn read_from_apub_id(conn: &PgConnection, object_id: &Url) -> Result<Self, Error> {
+  fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result<Self, Error> {
     use lemmy_db_schema::schema::comment::dsl::*;
     comment.filter(ap_id.eq(object_id)).first::<Self>(conn)
   }

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<CommunityForm> for Community {
 }
 
 impl ApubObject<CommunityForm> for Community {
-  fn read_from_apub_id(conn: &PgConnection, for_actor_id: &Url) -> Result<Self, Error> {
+  fn read_from_apub_id(conn: &PgConnection, for_actor_id: &DbUrl) -> Result<Self, Error> {
     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<Community, Error>;
   fn distinct_federated_communities(conn: &PgConnection) -> Result<Vec<String>, Error>;
-  fn read_from_followers_url(conn: &PgConnection, followers_url: &Url) -> Result<Community, Error>;
+  fn read_from_followers_url(
+    conn: &PgConnection,
+    followers_url: &DbUrl,
+  ) -> Result<Community, Error>;
 }
 
 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<Community, Error> {
     use lemmy_db_schema::schema::community::dsl::*;
     community

crates/db_queries/src/source/post.rs

@@ -12,7 +12,7 @@ use lemmy_db_schema::{
     PostSaved,
     PostSavedForm,
   },
-  Url,
+  DbUrl,
 };
 
 impl Crud<PostForm> for Post {
@@ -42,7 +42,7 @@ impl Crud<PostForm> for Post {
 pub trait Post_ {
   //fn read(conn: &PgConnection, post_id: i32) -> Result<Post, Error>;
   fn list_for_community(conn: &PgConnection, the_community_id: i32) -> Result<Vec<Post>, Error>;
-  fn update_ap_id(conn: &PgConnection, post_id: i32, apub_id: Url) -> Result<Post, Error>;
+  fn update_ap_id(conn: &PgConnection, post_id: i32, apub_id: DbUrl) -> Result<Post, Error>;
   fn permadelete_for_creator(conn: &PgConnection, for_creator_id: i32) -> Result<Vec<Post>, Error>;
   fn update_deleted(conn: &PgConnection, post_id: i32, new_deleted: bool) -> Result<Post, Error>;
   fn update_removed(conn: &PgConnection, post_id: i32, new_removed: bool) -> Result<Post, Error>;
@@ -68,7 +68,7 @@ impl Post_ for Post {
       .load::<Self>(conn)
   }
 
-  fn update_ap_id(conn: &PgConnection, post_id: i32, apub_id: Url) -> Result<Self, Error> {
+  fn update_ap_id(conn: &PgConnection, post_id: i32, apub_id: DbUrl) -> Result<Self, Error> {
     use lemmy_db_schema::schema::post::dsl::*;
 
     diesel::update(post.find(post_id))
@@ -147,7 +147,7 @@ impl Post_ for Post {
 }
 
 impl ApubObject<PostForm> for Post {
-  fn read_from_apub_id(conn: &PgConnection, object_id: &Url) -> Result<Self, Error> {
+  fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result<Self, Error> {
     use lemmy_db_schema::schema::post::dsl::*;
     post.filter(ap_id.eq(object_id)).first::<Self>(conn)
   }

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<PrivateMessageForm> for PrivateMessage {
   fn read(conn: &PgConnection, private_message_id: i32) -> Result<Self, Error> {
@@ -28,7 +28,7 @@ impl Crud<PrivateMessageForm> for PrivateMessage {
 }
 
 impl ApubObject<PrivateMessageForm> for PrivateMessage {
-  fn read_from_apub_id(conn: &PgConnection, object_id: &Url) -> Result<Self, Error>
+  fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result<Self, Error>
   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<PrivateMessage, Error>;
   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<PrivateMessage, Error> {
     use lemmy_db_schema::schema::private_message::dsl::*;
 

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<UserForm> for User_ {
 }
 
 impl ApubObject<UserForm> for User_ {
-  fn read_from_apub_id(conn: &PgConnection, object_id: &Url) -> Result<Self, Error> {
+  fn read_from_apub_id(conn: &PgConnection, object_id: &DbUrl) -> Result<Self, Error> {
     use lemmy_db_schema::schema::user_::dsl::*;
     user_
       .filter(deleted.eq(false))

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"] }

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<DB: Backend> ToSql<Text, DB> for Url
+impl<DB: Backend> ToSql<Text, DB> for DbUrl
 where
   String: ToSql<Text, DB>,
 {
@@ -31,37 +32,37 @@ where
   }
 }
 
-impl<DB: Backend> FromSql<Text, DB> for Url
+impl<DB: Backend> FromSql<Text, DB> for DbUrl
 where
   String: FromSql<Text, DB>,
 {
   fn from_sql(bytes: Option<&DB::RawValue>) -> diesel::deserialize::Result<Self> {
     let str = String::from_sql(bytes)?;
-    Ok(Url(url::Url::parse(&str)?))
+    Ok(DbUrl(Url::parse(&str)?))
   }
 }
 
-impl Url {
+impl DbUrl {
-  pub fn into_inner(self) -> url::Url {
+  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<Url> for url::Url {
+impl From<DbUrl> for Url {
-  fn from(url: Url) -> Self {
+  fn from(url: DbUrl) -> Self {
     url.0
   }
 }
 
-impl From<url::Url> for Url {
+impl From<Url> for DbUrl {
-  fn from(url: url::Url) -> Self {
+  fn from(url: Url) -> Self {
-    Url(url)
+    DbUrl(url)
   }
 }
 

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<chrono::NaiveDateTime>,
-  pub ap_id: Option<String>,
+  pub ap_id: Option<DbUrl>,
   pub sensitive: Option<bool>,
 }
 
@@ -20,6 +20,6 @@ pub struct ActivityForm {
   pub data: Value,
   pub local: bool,
   pub updated: Option<chrono::NaiveDateTime>,
-  pub ap_id: String,
+  pub ap_id: DbUrl,
   pub sensitive: bool,
 }

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<chrono::NaiveDateTime>,
   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<chrono::NaiveDateTime>,
   pub deleted: bool,
-  pub ap_id: Url,
+  pub ap_id: DbUrl,
   pub local: bool,
 }
 
@@ -60,7 +60,7 @@ pub struct CommentForm {
   pub published: Option<chrono::NaiveDateTime>,
   pub updated: Option<chrono::NaiveDateTime>,
   pub deleted: Option<bool>,
-  pub ap_id: Option<Url>,
+  pub ap_id: Option<DbUrl>,
   pub local: bool,
 }
 

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<chrono::NaiveDateTime>,
   pub deleted: bool,
   pub nsfw: bool,
-  pub actor_id: Url,
+  pub actor_id: DbUrl,
   pub local: bool,
   pub private_key: Option<String>,
   pub public_key: Option<String>,
   pub last_refreshed_at: chrono::NaiveDateTime,
-  pub icon: Option<String>,
+  pub icon: Option<DbUrl>,
-  pub banner: Option<String>,
+  pub banner: Option<DbUrl>,
-  pub followers_url: Url,
+  pub followers_url: DbUrl,
-  pub inbox_url: Url,
+  pub inbox_url: DbUrl,
-  pub shared_inbox_url: Option<Url>,
+  pub shared_inbox_url: Option<DbUrl>,
 }
 
 /// A safe representation of community, without the sensitive info
@@ -43,10 +43,10 @@ pub struct CommunitySafe {
   pub updated: Option<chrono::NaiveDateTime>,
   pub deleted: bool,
   pub nsfw: bool,
-  pub actor_id: Url,
+  pub actor_id: DbUrl,
   pub local: bool,
-  pub icon: Option<String>,
+  pub icon: Option<DbUrl>,
-  pub banner: Option<String>,
+  pub banner: Option<DbUrl>,
 }
 
 #[derive(Insertable, AsChangeset, Debug)]
@@ -61,16 +61,16 @@ pub struct CommunityForm {
   pub updated: Option<chrono::NaiveDateTime>,
   pub deleted: Option<bool>,
   pub nsfw: bool,
-  pub actor_id: Option<Url>,
+  pub actor_id: Option<DbUrl>,
   pub local: bool,
   pub private_key: Option<String>,
   pub public_key: Option<String>,
   pub last_refreshed_at: Option<chrono::NaiveDateTime>,
-  pub icon: Option<Option<String>>,
+  pub icon: Option<Option<DbUrl>>,
-  pub banner: Option<Option<String>>,
+  pub banner: Option<Option<DbUrl>>,
-  pub followers_url: Option<Url>,
+  pub followers_url: Option<DbUrl>,
-  pub inbox_url: Option<Url>,
+  pub inbox_url: Option<DbUrl>,
-  pub shared_inbox_url: Option<Option<Url>>,
+  pub shared_inbox_url: Option<Option<DbUrl>>,
 }
 
 #[derive(Identifiable, Queryable, Associations, PartialEq, Debug)]

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<String>,
+  pub url: Option<DbUrl>,
   pub body: Option<String>,
   pub creator_id: i32,
   pub community_id: i32,
@@ -23,8 +23,8 @@ pub struct Post {
   pub embed_title: Option<String>,
   pub embed_description: Option<String>,
   pub embed_html: Option<String>,
-  pub thumbnail_url: Option<String>,
+  pub thumbnail_url: Option<DbUrl>,
-  pub ap_id: Url,
+  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<String>,
+  pub url: Option<DbUrl>,
   pub body: Option<String>,
   pub creator_id: i32,
   pub community_id: i32,
@@ -46,8 +46,8 @@ pub struct PostForm {
   pub embed_title: Option<String>,
   pub embed_description: Option<String>,
   pub embed_html: Option<String>,
-  pub thumbnail_url: Option<String>,
+  pub thumbnail_url: Option<DbUrl>,
-  pub ap_id: Option<Url>,
+  pub ap_id: Option<DbUrl>,
   pub local: bool,
 }
 

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<String>,
+  pub original_post_url: Option<DbUrl>,
   pub original_post_body: Option<String>,
   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<String>,
+  pub original_post_url: Option<DbUrl>,
   pub original_post_body: Option<String>,
   pub reason: String,
 }

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<chrono::NaiveDateTime>,
-  pub ap_id: Url,
+  pub ap_id: DbUrl,
   pub local: bool,
 }
 
@@ -26,6 +26,6 @@ pub struct PrivateMessageForm {
   pub read: Option<bool>,
   pub published: Option<chrono::NaiveDateTime>,
   pub updated: Option<chrono::NaiveDateTime>,
-  pub ap_id: Option<Url>,
+  pub ap_id: Option<DbUrl>,
   pub local: bool,
 }

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<String>,
+  pub icon: Option<DbUrl>,
-  pub banner: Option<String>,
+  pub banner: Option<DbUrl>,
 }
 
 #[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<Option<String>>,
+  pub icon: Option<Option<DbUrl>>,
-  pub banner: Option<Option<String>>,
+  pub banner: Option<Option<DbUrl>>,
 }

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<String>,
   pub password_encrypted: String,
   pub email: Option<String>,
-  pub avatar: Option<String>,
+  pub avatar: Option<DbUrl>,
   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<String>,
-  pub actor_id: Url,
+  pub actor_id: DbUrl,
   pub bio: Option<String>,
   pub local: bool,
   pub private_key: Option<String>,
   pub public_key: Option<String>,
   pub last_refreshed_at: chrono::NaiveDateTime,
-  pub banner: Option<String>,
+  pub banner: Option<DbUrl>,
   pub deleted: bool,
-  pub inbox_url: Url,
+  pub inbox_url: DbUrl,
-  pub shared_inbox_url: Option<Url>,
+  pub shared_inbox_url: Option<DbUrl>,
 }
 
 /// 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<String>,
-  pub avatar: Option<String>,
+  pub avatar: Option<DbUrl>,
   pub admin: bool,
   pub banned: bool,
   pub published: chrono::NaiveDateTime,
   pub updated: Option<chrono::NaiveDateTime>,
   pub matrix_user_id: Option<String>,
-  pub actor_id: Url,
+  pub actor_id: DbUrl,
   pub bio: Option<String>,
   pub local: bool,
-  pub banner: Option<String>,
+  pub banner: Option<DbUrl>,
   pub deleted: bool,
-  pub inbox_url: Url,
+  pub inbox_url: DbUrl,
-  pub shared_inbox_url: Option<Url>,
+  pub shared_inbox_url: Option<DbUrl>,
 }
 
 /// A safe user view with only settings
@@ -67,7 +67,7 @@ pub struct UserSafeSettings {
   pub name: String,
   pub preferred_username: Option<String>,
   pub email: Option<String>,
-  pub avatar: Option<String>,
+  pub avatar: Option<DbUrl>,
   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<String>,
-  pub actor_id: Url,
+  pub actor_id: DbUrl,
   pub bio: Option<String>,
   pub local: bool,
   pub last_refreshed_at: chrono::NaiveDateTime,
-  pub banner: Option<String>,
+  pub banner: Option<DbUrl>,
   pub deleted: bool,
 }
 
@@ -96,7 +96,7 @@ pub struct UserAlias1 {
   pub preferred_username: Option<String>,
   pub password_encrypted: String,
   pub email: Option<String>,
-  pub avatar: Option<String>,
+  pub avatar: Option<DbUrl>,
   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<String>,
-  pub actor_id: Url,
+  pub actor_id: DbUrl,
   pub bio: Option<String>,
   pub local: bool,
   pub private_key: Option<String>,
   pub public_key: Option<String>,
   pub last_refreshed_at: chrono::NaiveDateTime,
-  pub banner: Option<String>,
+  pub banner: Option<DbUrl>,
   pub deleted: bool,
 }
 
@@ -125,16 +125,16 @@ pub struct UserSafeAlias1 {
   pub id: i32,
   pub name: String,
   pub preferred_username: Option<String>,
-  pub avatar: Option<String>,
+  pub avatar: Option<DbUrl>,
   pub admin: bool,
   pub banned: bool,
   pub published: chrono::NaiveDateTime,
   pub updated: Option<chrono::NaiveDateTime>,
   pub matrix_user_id: Option<String>,
-  pub actor_id: Url,
+  pub actor_id: DbUrl,
   pub bio: Option<String>,
   pub local: bool,
-  pub banner: Option<String>,
+  pub banner: Option<DbUrl>,
   pub deleted: bool,
 }
 
@@ -146,7 +146,7 @@ pub struct UserAlias2 {
   pub preferred_username: Option<String>,
   pub password_encrypted: String,
   pub email: Option<String>,
-  pub avatar: Option<String>,
+  pub avatar: Option<DbUrl>,
   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<String>,
-  pub actor_id: Url,
+  pub actor_id: DbUrl,
   pub bio: Option<String>,
   pub local: bool,
   pub private_key: Option<String>,
   pub public_key: Option<String>,
   pub last_refreshed_at: chrono::NaiveDateTime,
-  pub banner: Option<String>,
+  pub banner: Option<DbUrl>,
   pub deleted: bool,
 }
 
@@ -175,16 +175,16 @@ pub struct UserSafeAlias2 {
   pub id: i32,
   pub name: String,
   pub preferred_username: Option<String>,
-  pub avatar: Option<String>,
+  pub avatar: Option<DbUrl>,
   pub admin: bool,
   pub banned: bool,
   pub published: chrono::NaiveDateTime,
   pub updated: Option<chrono::NaiveDateTime>,
   pub matrix_user_id: Option<String>,
-  pub actor_id: Url,
+  pub actor_id: DbUrl,
   pub bio: Option<String>,
   pub local: bool,
-  pub banner: Option<String>,
+  pub banner: Option<DbUrl>,
   pub deleted: bool,
 }
 
@@ -197,7 +197,7 @@ pub struct UserForm {
   pub admin: bool,
   pub banned: Option<bool>,
   pub email: Option<Option<String>>,
-  pub avatar: Option<Option<String>>,
+  pub avatar: Option<Option<DbUrl>>,
   pub published: Option<chrono::NaiveDateTime>,
   pub updated: Option<chrono::NaiveDateTime>,
   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<Option<String>>,
-  pub actor_id: Option<Url>,
+  pub actor_id: Option<DbUrl>,
   pub bio: Option<Option<String>>,
   pub local: bool,
   pub private_key: Option<String>,
   pub public_key: Option<String>,
   pub last_refreshed_at: Option<chrono::NaiveDateTime>,
-  pub banner: Option<Option<String>>,
+  pub banner: Option<Option<DbUrl>>,
-  pub inbox_url: Option<Url>,
+  pub inbox_url: Option<DbUrl>,
-  pub shared_inbox_url: Option<Option<Url>>,
+  pub shared_inbox_url: Option<Option<DbUrl>>,
 }

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"

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"

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"

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<String>,
   description: Option<String>,
-  thumbnail_url: Option<String>,
+  thumbnail_url: Option<Url>,
   html: Option<String>,
 }
 
 pub(crate) async fn fetch_iframely(
   client: &Client,
-  url: &str,
+  url: &Url,
 ) -> Result<IframelyResponse, LemmyError> {
   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<PictrsResponse, LemmyError> {
   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<String>,
+  url: Option<&Url>,
-) -> (
+) -> (Option<String>, Option<String>, Option<String>, Option<Url>) {
-  Option<String>,
-  Option<String>,
-  Option<String>,
-  Option<String>,
-) {
   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> {
+async fn is_image_content_type(client: &Client, test: &Url) -> Result<(), LemmyError> {
-  let response = retry(|| client.get(test).send()).await?;
+  let response = retry(|| client.get(test.to_owned()).send()).await?;
-
   if response
     .headers()
     .get("Content-Type")

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;
+

migrations/2021-02-28-162616_clean_empty_post_urls/up.sql

@@ -0,0 +1 @@
+UPDATE post SET url = NULL where url = '';