Adding check to description and body length fields. (#2805)

* Adding check to description and body length fields.

- Also making the checks return `LemmyError`
- Fixes #1747

* Address PR comments.

* PR comments 2

crates/api/src/community/ban.rs

@@ -23,7 +23,11 @@ use lemmy_db_schema::{
   traits::{Bannable, Crud, Followable},
 };
 use lemmy_db_views_actor::structs::PersonView;
-use lemmy_utils::{error::LemmyError, utils::time::naive_from_unix, ConnectionId};
+use lemmy_utils::{
+  error::LemmyError,
+  utils::{time::naive_from_unix, validation::is_valid_body_field},
+  ConnectionId,
+};
 
 #[async_trait::async_trait(?Send)]
 impl Perform for BanFromCommunity {
@@ -46,6 +50,7 @@ impl Perform for BanFromCommunity {
 
     // Verify that only mods or admins can ban
     is_mod_or_admin(context.pool(), local_user_view.person.id, community_id).await?;
+    is_valid_body_field(&data.reason)?;
 
     let community_user_ban_form = CommunityPersonBanForm {
       community_id: data.community_id,

crates/api/src/local_user/ban_person.rs

@@ -14,7 +14,11 @@ use lemmy_db_schema::{
   traits::Crud,
 };
 use lemmy_db_views_actor::structs::PersonView;
-use lemmy_utils::{error::LemmyError, utils::time::naive_from_unix, ConnectionId};
+use lemmy_utils::{
+  error::LemmyError,
+  utils::{time::naive_from_unix, validation::is_valid_body_field},
+  ConnectionId,
+};
 
 #[async_trait::async_trait(?Send)]
 impl Perform for BanPerson {
@@ -33,6 +37,8 @@ impl Perform for BanPerson {
     // Make sure user is an admin
     is_admin(&local_user_view)?;
 
+    is_valid_body_field(&data.reason)?;
+
     let ban = data.ban;
     let banned_person_id = data.person_id;
     let expires = data.expires.map(naive_from_unix);

crates/api/src/local_user/save_settings.rs

@@ -21,6 +21,7 @@ use lemmy_utils::{
   utils::validation::{
     build_totp_2fa,
     generate_totp_2fa_secret,
+    is_valid_bio_field,
     is_valid_display_name,
     is_valid_matrix_id,
   },
@@ -67,24 +68,18 @@ impl Perform for SaveUserSettings {
     }
 
     if let Some(Some(bio)) = &bio {
-      if bio.chars().count() > 300 {
-        return Err(LemmyError::from_message("bio_length_overflow"));
-      }
+      is_valid_bio_field(bio)?;
     }
 
     if let Some(Some(display_name)) = &display_name {
-      if !is_valid_display_name(
+      is_valid_display_name(
         display_name.trim(),
         site_view.local_site.actor_name_max_length as usize,
-      ) {
-        return Err(LemmyError::from_message("invalid_username"));
-      }
+      )?;
     }
 
     if let Some(Some(matrix_user_id)) = &matrix_user_id {
-      if !is_valid_matrix_id(matrix_user_id) {
-        return Err(LemmyError::from_message("invalid_matrix_id"));
-      }
+      is_valid_matrix_id(matrix_user_id)?;
     }
 
     let local_user_id = local_user_view.local_user.id;

crates/api_crud/src/comment/create.rs

@@ -27,7 +27,11 @@ use lemmy_db_schema::{
 };
 use lemmy_utils::{
   error::LemmyError,
-  utils::{mention::scrape_text_for_mentions, slurs::remove_slurs},
+  utils::{
+    mention::scrape_text_for_mentions,
+    slurs::remove_slurs,
+    validation::is_valid_body_field,
+  },
   ConnectionId,
 };
 
@@ -50,6 +54,7 @@ impl PerformCrud for CreateComment {
       &data.content.clone(),
       &local_site_to_slur_regex(&local_site),
     );
+    is_valid_body_field(&Some(content_slurs_removed.clone()))?;
 
     // Check for a community ban
     let post_id = data.post_id;

crates/api_crud/src/comment/update.rs

@@ -18,7 +18,11 @@ use lemmy_db_schema::{
 use lemmy_db_views::structs::CommentView;
 use lemmy_utils::{
   error::LemmyError,
-  utils::{mention::scrape_text_for_mentions, slurs::remove_slurs},
+  utils::{
+    mention::scrape_text_for_mentions,
+    slurs::remove_slurs,
+    validation::is_valid_body_field,
+  },
   ConnectionId,
 };
 
@@ -65,6 +69,9 @@ impl PerformCrud for EditComment {
       .content
       .as_ref()
       .map(|c| remove_slurs(c, &local_site_to_slur_regex(&local_site)));
+
+    is_valid_body_field(&content_slurs_removed)?;
+
     let comment_id = data.comment_id;
     let form = CommentUpdateForm::builder()
       .content(content_slurs_removed)

crates/api_crud/src/community/create.rs

@@ -36,7 +36,7 @@ use lemmy_utils::{
   error::LemmyError,
   utils::{
     slurs::{check_slurs, check_slurs_opt},
-    validation::is_valid_actor_name,
+    validation::{is_valid_actor_name, is_valid_body_field},
   },
   ConnectionId,
 };
@@ -72,9 +72,8 @@ impl PerformCrud for CreateCommunity {
     check_slurs(&data.title, &slur_regex)?;
     check_slurs_opt(&data.description, &slur_regex)?;
 
-    if !is_valid_actor_name(&data.name, local_site.actor_name_max_length as usize) {
-      return Err(LemmyError::from_message("invalid_community_name"));
-    }
+    is_valid_actor_name(&data.name, local_site.actor_name_max_length as usize)?;
+    is_valid_body_field(&data.description)?;
 
     // Double check for duplicate community actor_ids
     let community_actor_id = generate_local_apub_endpoint(

crates/api_crud/src/community/update.rs

@@ -17,7 +17,11 @@ use lemmy_db_schema::{
   utils::{diesel_option_overwrite, diesel_option_overwrite_to_url, naive_now},
 };
 use lemmy_db_views_actor::structs::CommunityModeratorView;
-use lemmy_utils::{error::LemmyError, utils::slurs::check_slurs_opt, ConnectionId};
+use lemmy_utils::{
+  error::LemmyError,
+  utils::{slurs::check_slurs_opt, validation::is_valid_body_field},
+  ConnectionId,
+};
 
 #[async_trait::async_trait(?Send)]
 impl PerformCrud for EditCommunity {
@@ -41,6 +45,7 @@ impl PerformCrud for EditCommunity {
     let slur_regex = local_site_to_slur_regex(&local_site);
     check_slurs_opt(&data.title, &slur_regex)?;
     check_slurs_opt(&data.description, &slur_regex)?;
+    is_valid_body_field(&data.description)?;
 
     // Verify its a mod (only mods can edit it)
     let community_id = data.community_id;

crates/api_crud/src/post/create.rs

@@ -31,7 +31,7 @@ use lemmy_utils::{
   error::LemmyError,
   utils::{
     slurs::{check_slurs, check_slurs_opt},
-    validation::{clean_url_params, is_valid_post_title},
+    validation::{clean_url_params, is_valid_body_field, is_valid_post_title},
   },
   ConnectionId,
 };
@@ -62,9 +62,8 @@ impl PerformCrud for CreatePost {
     let data_url = data.url.as_ref();
     let url = data_url.map(clean_url_params).map(Into::into); // TODO no good way to handle a "clear"
 
-    if !is_valid_post_title(&data.name) {
-      return Err(LemmyError::from_message("invalid_post_title"));
-    }
+    is_valid_post_title(&data.name)?;
+    is_valid_body_field(&data.body)?;
 
     check_community_ban(local_user_view.person.id, data.community_id, context.pool()).await?;
     check_community_deleted_or_removed(data.community_id, context.pool()).await?;

crates/api_crud/src/post/update.rs

@@ -20,7 +20,7 @@ use lemmy_utils::{
   error::LemmyError,
   utils::{
     slurs::check_slurs_opt,
-    validation::{clean_url_params, is_valid_post_title},
+    validation::{clean_url_params, is_valid_body_field, is_valid_post_title},
   },
   ConnectionId,
 };
@@ -52,11 +52,11 @@ impl PerformCrud for EditPost {
     check_slurs_opt(&data.body, &slur_regex)?;
 
     if let Some(name) = &data.name {
-      if !is_valid_post_title(name) {
-        return Err(LemmyError::from_message("invalid_post_title"));
-      }
+      is_valid_post_title(name)?;
     }
 
+    is_valid_body_field(&data.body)?;
+
     let post_id = data.post_id;
     let orig_post = Post::read(context.pool(), post_id).await?;
 

crates/api_crud/src/private_message/create.rs

@@ -22,7 +22,11 @@ use lemmy_db_schema::{
   traits::Crud,
 };
 use lemmy_db_views::structs::LocalUserView;
-use lemmy_utils::{error::LemmyError, utils::slurs::remove_slurs, ConnectionId};
+use lemmy_utils::{
+  error::LemmyError,
+  utils::{slurs::remove_slurs, validation::is_valid_body_field},
+  ConnectionId,
+};
 
 #[async_trait::async_trait(?Send)]
 impl PerformCrud for CreatePrivateMessage {
@@ -43,6 +47,7 @@ impl PerformCrud for CreatePrivateMessage {
       &data.content.clone(),
       &local_site_to_slur_regex(&local_site),
     );
+    is_valid_body_field(&Some(content_slurs_removed.clone()))?;
 
     check_person_block(local_user_view.person.id, data.recipient_id, context.pool()).await?;
 

crates/api_crud/src/private_message/update.rs

@@ -14,7 +14,11 @@ use lemmy_db_schema::{
   traits::Crud,
   utils::naive_now,
 };
-use lemmy_utils::{error::LemmyError, utils::slurs::remove_slurs, ConnectionId};
+use lemmy_utils::{
+  error::LemmyError,
+  utils::{slurs::remove_slurs, validation::is_valid_body_field},
+  ConnectionId,
+};
 
 #[async_trait::async_trait(?Send)]
 impl PerformCrud for EditPrivateMessage {
@@ -40,6 +44,8 @@ impl PerformCrud for EditPrivateMessage {
 
     // Doing the update
     let content_slurs_removed = remove_slurs(&data.content, &local_site_to_slur_regex(&local_site));
+    is_valid_body_field(&Some(content_slurs_removed.clone()))?;
+
     let private_message_id = data.private_message_id;
     PrivateMessage::update(
       context.pool(),

crates/api_crud/src/site/create.rs

@@ -26,7 +26,10 @@ use lemmy_db_schema::{
 use lemmy_db_views::structs::SiteView;
 use lemmy_utils::{
   error::LemmyError,
-  utils::slurs::{check_slurs, check_slurs_opt},
+  utils::{
+    slurs::{check_slurs, check_slurs_opt},
+    validation::is_valid_body_field,
+  },
   ConnectionId,
 };
 use url::Url;
@@ -68,6 +71,8 @@ impl PerformCrud for CreateSite {
       site_description_length_check(desc)?;
     }
 
+    is_valid_body_field(&data.sidebar)?;
+
     let application_question = diesel_option_overwrite(&data.application_question);
     check_application_question(
       &application_question,

crates/api_crud/src/site/update.rs

@@ -28,7 +28,11 @@ use lemmy_db_schema::{
   ListingType,
 };
 use lemmy_db_views::structs::SiteView;
-use lemmy_utils::{error::LemmyError, utils::slurs::check_slurs_opt, ConnectionId};
+use lemmy_utils::{
+  error::LemmyError,
+  utils::{slurs::check_slurs_opt, validation::is_valid_body_field},
+  ConnectionId,
+};
 use std::str::FromStr;
 
 #[async_trait::async_trait(?Send)]
@@ -60,6 +64,8 @@ impl PerformCrud for EditSite {
       site_description_length_check(desc)?;
     }
 
+    is_valid_body_field(&data.sidebar)?;
+
     let application_question = diesel_option_overwrite(&data.application_question);
     check_application_question(
       &application_question,

crates/api_crud/src/user/create.rs

@@ -96,9 +96,7 @@ impl PerformCrud for Register {
     check_slurs_opt(&data.answer, &slur_regex)?;
 
     let actor_keypair = generate_actor_keypair()?;
-    if !is_valid_actor_name(&data.username, local_site.actor_name_max_length as usize) {
-      return Err(LemmyError::from_message("invalid_username"));
-    }
+    is_valid_actor_name(&data.username, local_site.actor_name_max_length as usize)?;
     let actor_id = generate_local_apub_endpoint(
       EndpointType::Person,
       &data.username,

crates/utils/src/utils/validation.rs

@@ -1,4 +1,4 @@
-use crate::error::LemmyError;
+use crate::error::{LemmyError, LemmyResult};
 use itertools::Itertools;
 use once_cell::sync::Lazy;
 use regex::Regex;
@@ -17,32 +17,77 @@ static CLEAN_URL_PARAMS_REGEX: Lazy<Regex> = Lazy::new(|| {
   Regex::new(r"^utm_source|utm_medium|utm_campaign|utm_term|utm_content|gclid|gclsrc|dclid|fbclid$")
     .expect("compile regex")
 });
+const BODY_MAX_LENGTH: usize = 10000;
+const BIO_MAX_LENGTH: usize = 300;
 
 fn has_newline(name: &str) -> bool {
   name.contains('\n')
 }
 
-pub fn is_valid_actor_name(name: &str, actor_name_max_length: usize) -> bool {
-  name.chars().count() <= actor_name_max_length
+pub fn is_valid_actor_name(name: &str, actor_name_max_length: usize) -> LemmyResult<()> {
+  let check = name.chars().count() <= actor_name_max_length
     && VALID_ACTOR_NAME_REGEX.is_match(name)
-    && !has_newline(name)
+    && !has_newline(name);
+  if !check {
+    Err(LemmyError::from_message("invalid_name"))
+  } else {
+    Ok(())
+  }
 }
 
 // Can't do a regex here, reverse lookarounds not supported
-pub fn is_valid_display_name(name: &str, actor_name_max_length: usize) -> bool {
-  !name.starts_with('@')
+pub fn is_valid_display_name(name: &str, actor_name_max_length: usize) -> LemmyResult<()> {
+  let check = !name.starts_with('@')
     && !name.starts_with('\u{200b}')
     && name.chars().count() >= 3
     && name.chars().count() <= actor_name_max_length
-    && !has_newline(name)
+    && !has_newline(name);
+  if !check {
+    Err(LemmyError::from_message("invalid_username"))
+  } else {
+    Ok(())
+  }
+}
+
+pub fn is_valid_matrix_id(matrix_id: &str) -> LemmyResult<()> {
+  let check = VALID_MATRIX_ID_REGEX.is_match(matrix_id) && !has_newline(matrix_id);
+  if !check {
+    Err(LemmyError::from_message("invalid_matrix_id"))
+  } else {
+    Ok(())
+  }
 }
 
-pub fn is_valid_matrix_id(matrix_id: &str) -> bool {
-  VALID_MATRIX_ID_REGEX.is_match(matrix_id) && !has_newline(matrix_id)
+pub fn is_valid_post_title(title: &str) -> LemmyResult<()> {
+  let check = VALID_POST_TITLE_REGEX.is_match(title) && !has_newline(title);
+  if !check {
+    Err(LemmyError::from_message("invalid_post_title"))
+  } else {
+    Ok(())
+  }
 }
 
-pub fn is_valid_post_title(title: &str) -> bool {
-  VALID_POST_TITLE_REGEX.is_match(title) && !has_newline(title)
+/// This could be post bodies, comments, or any description field
+pub fn is_valid_body_field(body: &Option<String>) -> LemmyResult<()> {
+  if let Some(body) = body {
+    let check = body.chars().count() <= BODY_MAX_LENGTH;
+    if !check {
+      Err(LemmyError::from_message("invalid_body_field"))
+    } else {
+      Ok(())
+    }
+  } else {
+    Ok(())
+  }
+}
+
+pub fn is_valid_bio_field(bio: &str) -> LemmyResult<()> {
+  let check = bio.chars().count() <= BIO_MAX_LENGTH;
+  if !check {
+    Err(LemmyError::from_message("bio_length_overflow"))
+  } else {
+    Ok(())
+  }
 }
 
 pub fn clean_url_params(url: &Url) -> Url {
@@ -63,7 +108,7 @@ pub fn check_totp_2fa_valid(
   totp_token: &Option<String>,
   site_name: &str,
   username: &str,
-) -> Result<(), LemmyError> {
+) -> LemmyResult<()> {
   // Check only if they have a totp_secret in the DB
   if let Some(totp_secret) = totp_secret {
     // Throw an error if their token is missing
@@ -131,52 +176,48 @@ mod tests {
 
   #[test]
   fn regex_checks() {
-    assert!(!is_valid_post_title("hi"));
-    assert!(is_valid_post_title("him"));
-    assert!(!is_valid_post_title("n\n\n\n\nanother"));
-    assert!(!is_valid_post_title("hello there!\n this is a test."));
-    assert!(is_valid_post_title("hello there! this is a test."));
+    assert!(is_valid_post_title("hi").is_err());
+    assert!(is_valid_post_title("him").is_ok());
+    assert!(is_valid_post_title("n\n\n\n\nanother").is_err());
+    assert!(is_valid_post_title("hello there!\n this is a test.").is_err());
+    assert!(is_valid_post_title("hello there! this is a test.").is_ok());
   }
 
   #[test]
   fn test_valid_actor_name() {
     let actor_name_max_length = 20;
-    assert!(is_valid_actor_name("Hello_98", actor_name_max_length));
-    assert!(is_valid_actor_name("ten", actor_name_max_length));
-    assert!(!is_valid_actor_name("Hello-98", actor_name_max_length));
-    assert!(!is_valid_actor_name("a", actor_name_max_length));
-    assert!(!is_valid_actor_name("", actor_name_max_length));
+    assert!(is_valid_actor_name("Hello_98", actor_name_max_length).is_ok());
+    assert!(is_valid_actor_name("ten", actor_name_max_length).is_ok());
+    assert!(is_valid_actor_name("Hello-98", actor_name_max_length).is_err());
+    assert!(is_valid_actor_name("a", actor_name_max_length).is_err());
+    assert!(is_valid_actor_name("", actor_name_max_length).is_err());
   }
 
   #[test]
   fn test_valid_display_name() {
     let actor_name_max_length = 20;
-    assert!(is_valid_display_name("hello @there", actor_name_max_length));
-    assert!(!is_valid_display_name(
-      "@hello there",
-      actor_name_max_length
-    ));
+    assert!(is_valid_display_name("hello @there", actor_name_max_length).is_ok());
+    assert!(is_valid_display_name("@hello there", actor_name_max_length).is_err());
 
     // Make sure zero-space with an @ doesn't work
-    assert!(!is_valid_display_name(
-      &format!("{}@my name is", '\u{200b}'),
-      actor_name_max_length
-    ));
+    assert!(
+      is_valid_display_name(&format!("{}@my name is", '\u{200b}'), actor_name_max_length).is_err()
+    );
   }
 
   #[test]
   fn test_valid_post_title() {
-    assert!(is_valid_post_title("Post Title"));
-    assert!(is_valid_post_title("   POST TITLE 😃😃😃😃😃"));
-    assert!(!is_valid_post_title("\n \n \n \n    		")); // tabs/spaces/newlines
+    assert!(is_valid_post_title("Post Title").is_ok());
+    assert!(is_valid_post_title("   POST TITLE 😃😃😃😃😃").is_ok());
+    assert!(is_valid_post_title("\n \n \n \n    		").is_err()); // tabs/spaces/newlines
   }
 
   #[test]
   fn test_valid_matrix_id() {
-    assert!(is_valid_matrix_id("@dess:matrix.org"));
-    assert!(!is_valid_matrix_id("dess:matrix.org"));
-    assert!(!is_valid_matrix_id(" @dess:matrix.org"));
-    assert!(!is_valid_matrix_id("@dess:matrix.org t"));
+    assert!(is_valid_matrix_id("@dess:matrix.org").is_ok());
+    assert!(is_valid_matrix_id("dess:matrix.org").is_err());
+    assert!(is_valid_matrix_id(" @dess:matrix.org").is_err());
+    assert!(is_valid_matrix_id("@dess:matrix.org t").is_err());
   }
 
   #[test]