Fixes #2900 - Checks slur regex to see if it is too permissive

crates/api_common/src/utils.rs

@@ -312,15 +312,6 @@ pub fn password_length_check(pass: &str) -> Result<(), LemmyError> {
   }
 }
 
-/// Checks the site description length
-pub fn site_description_length_check(description: &str) -> Result<(), LemmyError> {
-  if description.len() > 150 {
-    Err(LemmyError::from_message("site_description_length_overflow"))
-  } else {
-    Ok(())
-  }
-}
-
 /// Checks for a honeypot. If this field is filled, fail the rest of the function
 pub fn honeypot_check(honeypot: &Option<String>) -> Result<(), LemmyError> {
   if honeypot.is_some() && honeypot != &Some(String::new()) {

crates/api_crud/src/site/create.rs

@@ -8,9 +8,7 @@ use lemmy_api_common::{
     generate_site_inbox_url,
     is_admin,
     local_site_rate_limit_to_rate_limit_config,
-    local_site_to_slur_regex,
     local_user_view_from_jwt,
-    site_description_length_check,
   },
 };
 use lemmy_db_schema::{
@@ -26,10 +24,15 @@ use lemmy_db_schema::{
 };
 use lemmy_db_views::structs::SiteView;
 use lemmy_utils::{
-  error::LemmyError,
+  error::{LemmyError, LemmyResult},
   utils::{
     slurs::{check_slurs, check_slurs_opt},
-    validation::is_valid_body_field,
+    validation::{
+      build_and_check_regex,
+      is_valid_body_field,
+      site_description_length_check,
+      site_name_length_check,
+    },
   },
 };
 use url::Url;
@@ -41,32 +44,13 @@ impl PerformCrud for CreateSite {
   #[tracing::instrument(skip(context))]
   async fn perform(&self, context: &Data<LemmyContext>) -> Result<SiteResponse, LemmyError> {
     let data: &CreateSite = self;
-
-    let local_site = LocalSite::read(context.pool()).await?;
-
-    if local_site.site_setup {
-      return Err(LemmyError::from_message("site_already_exists"));
-    };
-
     let local_user_view = local_user_view_from_jwt(&data.auth, context).await?;
+    let local_site = LocalSite::read(context.pool()).await?;
 
-    let sidebar = diesel_option_overwrite(&data.sidebar);
-    let description = diesel_option_overwrite(&data.description);
-    let icon = diesel_option_overwrite_to_url(&data.icon)?;
-    let banner = diesel_option_overwrite_to_url(&data.banner)?;
-
-    let slur_regex = local_site_to_slur_regex(&local_site);
-    check_slurs(&data.name, &slur_regex)?;
-    check_slurs_opt(&data.description, &slur_regex)?;
-
-    // Make sure user is an admin
+    // Make sure user is an admin; other types of users should not create site data...
     is_admin(&local_user_view)?;
 
-    if let Some(Some(desc)) = &description {
-      site_description_length_check(desc)?;
-    }
-
-    is_valid_body_field(&data.sidebar)?;
+    validate_create_payload(local_site.site_setup, local_site.slur_filter_regex, data)?;
 
     let application_question = diesel_option_overwrite(&data.application_question);
     check_application_question(
@@ -81,10 +65,10 @@ impl PerformCrud for CreateSite {
     let keypair = generate_actor_keypair()?;
     let site_form = SiteUpdateForm::builder()
       .name(Some(data.name.clone()))
-      .sidebar(sidebar)
-      .description(description)
-      .icon(icon)
-      .banner(banner)
+      .sidebar(diesel_option_overwrite(&data.sidebar))
+      .description(diesel_option_overwrite(&data.description))
+      .icon(diesel_option_overwrite_to_url(&data.icon)?)
+      .banner(diesel_option_overwrite_to_url(&data.banner)?)
       .actor_id(Some(actor_id))
       .last_refreshed_at(Some(naive_now()))
       .inbox_url(inbox_url)
@@ -157,3 +141,197 @@ impl PerformCrud for CreateSite {
     })
   }
 }
+
+fn validate_create_payload(
+  is_site_setup: bool,
+  local_site_slur_filter_regex: Option<String>,
+  create_site: &CreateSite,
+) -> LemmyResult<()> {
+  // Make sure the site hasn't already been set up...
+  if is_site_setup {
+    return Err(LemmyError::from_message("site_already_exists"));
+  };
+
+  // Check that the slur regex compiles, and returns the regex if valid...
+  // Prioritize using new slur regex from the request; if not provided, use the existing regex.
+  let slur_regex = build_and_check_regex(
+    &create_site
+      .slur_filter_regex
+      .as_deref()
+      .or(local_site_slur_filter_regex.as_deref()),
+  )?;
+
+  // Validate the site name...
+  site_name_length_check(&create_site.name)?;
+  check_slurs(&create_site.name, &slur_regex)?;
+
+  // If provided, validate the site description...
+  if let Some(desc) = &create_site.description {
+    site_description_length_check(desc)?;
+    check_slurs_opt(&create_site.description, &slur_regex)?;
+  }
+
+  // Ensure that the sidebar has fewer than the max num characters...
+  is_valid_body_field(&create_site.sidebar)
+}
+
+#[cfg(test)]
+mod tests {
+  use crate::site::create::validate_create_payload;
+  use lemmy_api_common::site::CreateSite;
+
+  #[test]
+  fn test_validate_create() {
+    fn create_payload(
+      site_name: String,
+      site_description: Option<String>,
+      site_sidebar: Option<String>,
+      site_slur_filter_regex: Option<String>,
+    ) -> CreateSite {
+      CreateSite {
+        name: site_name,
+        sidebar: site_sidebar,
+        description: site_description,
+        icon: None,
+        banner: None,
+        enable_downvotes: None,
+        enable_nsfw: None,
+        community_creation_admin_only: None,
+        require_email_verification: None,
+        application_question: None,
+        private_instance: None,
+        default_theme: None,
+        default_post_listing_type: None,
+        legal_information: None,
+        application_email_admins: None,
+        hide_modlog_mod_names: None,
+        discussion_languages: None,
+        slur_filter_regex: site_slur_filter_regex,
+        actor_name_max_length: None,
+        rate_limit_message: None,
+        rate_limit_message_per_second: None,
+        rate_limit_post: None,
+        rate_limit_post_per_second: None,
+        rate_limit_register: None,
+        rate_limit_register_per_second: None,
+        rate_limit_image: None,
+        rate_limit_image_per_second: None,
+        rate_limit_comment: None,
+        rate_limit_comment_per_second: None,
+        rate_limit_search: None,
+        rate_limit_search_per_second: None,
+        federation_enabled: None,
+        federation_debug: None,
+        federation_worker_count: None,
+        captcha_enabled: None,
+        captcha_difficulty: None,
+        allowed_instances: None,
+        blocked_instances: None,
+        taglines: None,
+        registration_mode: None,
+        auth: Default::default(),
+      }
+    }
+
+    let invalid_payloads = [
+      (
+        false,
+        &Some(String::from("(foo|bar)")),
+        &create_payload(
+          String::from("foo site_name"),
+          None::<String>,
+          None::<String>,
+          None::<String>,
+        ),
+        "slurs",
+      ),
+      (
+        false,
+        &Some(String::from("(foo|bar)")),
+        &create_payload(
+          String::from("zeta site_name"),
+          None::<String>,
+          None::<String>,
+          Some(String::from("(zeta|alpha)")),
+        ),
+        "slurs",
+      ),
+      (
+        true,
+        &None::<String>,
+        &create_payload(
+          String::from("site_name"),
+          None::<String>,
+          None::<String>,
+          None::<String>,
+        ),
+        "site_already_exists",
+      ),
+    ];
+    let valid_payloads = [
+      (
+        false,
+        &None::<String>,
+        &create_payload(
+          String::from("site_name"),
+          None::<String>,
+          None::<String>,
+          None::<String>,
+        ),
+      ),
+      (
+        false,
+        &None::<String>,
+        &create_payload(
+          String::from("site_name"),
+          Some(String::new()),
+          Some(String::new()),
+          None::<String>,
+        ),
+      ),
+      (
+        false,
+        &Some(String::from("(foo|bar)")),
+        &create_payload(
+          String::from("foo site_name"),
+          Some(String::new()),
+          Some(String::new()),
+          Some(String::new()),
+        ),
+      ),
+    ];
+
+    invalid_payloads.iter().enumerate().for_each(
+      |(idx, &(is_site_setup, local_slur_filter_regex, create_site, expected_err))| {
+        match validate_create_payload(is_site_setup, local_slur_filter_regex.clone(), create_site) {
+          Ok(_) => {
+            panic!(
+            "Got Ok, but validation should have failed with error: {} for invalid_payloads.nth({})",
+            expected_err, idx
+          )
+          }
+          Err(error) => {
+            assert!(
+              error.message.eq(&Some(String::from(expected_err))),
+              "Got Err {:?}, but should have failed with message: {} for invalid_payloads.nth({})",
+              error.message,
+              expected_err,
+              idx
+            )
+          }
+        }
+      },
+    );
+
+    valid_payloads.iter().enumerate().for_each(
+      |(idx, &(is_site_setup, local_slur_filter_regex, create_site))| {
+        assert!(
+          validate_create_payload(is_site_setup, local_slur_filter_regex.clone(), create_site)
+            .is_ok(),
+          "Got Err, but should have got Ok for valid_payloads.nth({})",
+          idx
+        );
+      },
+    )
+  }
+}