From eef51e35d2dfd51a968b1593b11a1ec126590416 Mon Sep 17 00:00:00 2001 From: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> Date: Sun, 3 Nov 2024 21:45:26 -0500 Subject: [PATCH 1/8] Add lowercase only check for community names --- crates/api_crud/src/community/create.rs | 23 ++--- crates/api_crud/src/user/create.rs | 18 ++-- crates/utils/src/utils/validation.rs | 106 ++++++++++++++---------- 3 files changed, 73 insertions(+), 74 deletions(-) diff --git a/crates/api_crud/src/community/create.rs b/crates/api_crud/src/community/create.rs index cd0fc985ef..cc649bc89a 100644 --- a/crates/api_crud/src/community/create.rs +++ b/crates/api_crud/src/community/create.rs @@ -5,26 +5,15 @@ use lemmy_api_common::{ community::{CommunityResponse, CreateCommunity}, context::LemmyContext, utils::{ - generate_followers_url, - generate_inbox_url, - generate_local_apub_endpoint, - get_url_blocklist, - is_admin, - local_site_to_slur_regex, - process_markdown_opt, - proxy_image_link_api, - EndpointType, + generate_followers_url, generate_inbox_url, generate_local_apub_endpoint, get_url_blocklist, + is_admin, local_site_to_slur_regex, process_markdown_opt, proxy_image_link_api, EndpointType, }, }; use lemmy_db_schema::{ source::{ actor_language::{CommunityLanguage, SiteLanguage}, community::{ - Community, - CommunityFollower, - CommunityFollowerForm, - CommunityInsertForm, - CommunityModerator, + Community, CommunityFollower, CommunityFollowerForm, CommunityInsertForm, CommunityModerator, CommunityModeratorForm, }, }, @@ -37,9 +26,7 @@ use lemmy_utils::{ utils::{ slurs::check_slurs, validation::{ - is_valid_actor_name, - is_valid_body_field, - site_or_community_description_length_check, + is_valid_body_field, is_valid_community_name, site_or_community_description_length_check, }, }, }; @@ -80,7 +67,7 @@ pub async fn create_community( let banner = diesel_url_create(data.banner.as_deref())?; let banner = proxy_image_link_api(banner, &context).await?; - is_valid_actor_name(&data.name, local_site.actor_name_max_length as usize)?; + is_valid_community_name(&data.name, local_site.actor_name_max_length as usize)?; // Double check for duplicate community actor_ids let community_actor_id = generate_local_apub_endpoint( diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index ed560e3d6a..adcb0082b8 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -6,17 +6,9 @@ use lemmy_api_common::{ oauth_provider::AuthenticateWithOauth, person::{LoginResponse, Register}, utils::{ - check_email_verified, - check_registration_application, - check_user_valid, - generate_inbox_url, - generate_local_apub_endpoint, - honeypot_check, - local_site_to_slur_regex, - password_length_check, - send_new_applicant_email_to_admins, - send_verification_email, - EndpointType, + check_email_verified, check_registration_application, check_user_valid, generate_inbox_url, + generate_local_apub_endpoint, honeypot_check, local_site_to_slur_regex, password_length_check, + send_new_applicant_email_to_admins, send_verification_email, EndpointType, }, }; use lemmy_db_schema::{ @@ -41,7 +33,7 @@ use lemmy_utils::{ error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult}, utils::{ slurs::{check_slurs, check_slurs_opt}, - validation::is_valid_actor_name, + validation::is_valid_username, }, }; use serde::{Deserialize, Serialize}; @@ -407,7 +399,7 @@ async fn create_person( context: &Data, ) -> Result { let actor_keypair = generate_actor_keypair()?; - is_valid_actor_name(&username, local_site.actor_name_max_length as usize)?; + is_valid_username(&username, local_site.actor_name_max_length as usize)?; let actor_id = generate_local_apub_endpoint( EndpointType::Person, &username, diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index f8da6f609c..ab7b243a14 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -84,27 +84,37 @@ fn has_newline(name: &str) -> bool { name.contains('\n') } -pub fn is_valid_actor_name(name: &str, actor_name_max_length: usize) -> LemmyResult<()> { - static VALID_ACTOR_NAME_REGEX_EN: LazyLock = - LazyLock::new(|| Regex::new(r"^[a-zA-Z0-9_]{3,}$").expect("compile regex")); - static VALID_ACTOR_NAME_REGEX_AR: LazyLock = - LazyLock::new(|| Regex::new(r"^[\p{Arabic}0-9_]{3,}$").expect("compile regex")); - static VALID_ACTOR_NAME_REGEX_RU: LazyLock = - LazyLock::new(|| Regex::new(r"^[\p{Cyrillic}0-9_]{3,}$").expect("compile regex")); +pub fn is_valid_username(name: &str, actor_name_max_length: usize) -> LemmyResult<()> { + // Only allow characters from a single alphabet per username. This avoids problems with lookalike + // characters like `o` which looks identical in Latin and Cyrillic, and can be used to imitate + // other users. Checks for additional alphabets can be added in the same way. + static VALID_USERNAME_REGEX: LazyLock = LazyLock::new(|| { + Regex::new(r"^(?:[a-zA-Z0-9_]{3,}|[0-9_\p{Arabic}]{3,}|[0-9_\p{Cyrillic}]{3,})$") + .expect("compile regex") + }); - let check = name.chars().count() <= actor_name_max_length && !has_newline(name); + is_valid_actor_name(name, actor_name_max_length, &VALID_USERNAME_REGEX) +} +pub fn is_valid_community_name(name: &str, actor_name_max_length: usize) -> LemmyResult<()> { // Only allow characters from a single alphabet per username. This avoids problems with lookalike // characters like `o` which looks identical in Latin and Cyrillic, and can be used to imitate // other users. Checks for additional alphabets can be added in the same way. - let lang_check = VALID_ACTOR_NAME_REGEX_EN.is_match(name) - || VALID_ACTOR_NAME_REGEX_AR.is_match(name) - || VALID_ACTOR_NAME_REGEX_RU.is_match(name); + static VALID_COMMUNITY_NAME_REGEX: LazyLock = LazyLock::new(|| { + Regex::new( + r"^(?:[0-9_a-z]{3,}|[0-9_[\p{Arabic}]&&\P{Lu}&&\P{Lt}]{3,}|[0-9_[\p{Cyrillic}&&\P{Lu}&&\P{Lt}]]{3,})$", + ) + .expect("compile regex") + }); - if !check || !lang_check { - Err(LemmyErrorType::InvalidName.into()) - } else { + is_valid_actor_name(name, actor_name_max_length, &VALID_COMMUNITY_NAME_REGEX) +} + +fn is_valid_actor_name(name: &str, actor_name_max_length: usize, r: &Regex) -> LemmyResult<()> { + if name.len() <= actor_name_max_length && !has_newline(name) && r.is_match(name) { Ok(()) + } else { + Err(LemmyErrorType::InvalidName.into()) } } @@ -356,24 +366,11 @@ mod tests { use crate::{ error::{LemmyErrorType, LemmyResult}, utils::validation::{ - build_and_check_regex, - check_site_visibility_valid, - check_urls_are_valid, - clean_url, - clean_urls_in_text, - is_url_blocked, - is_valid_actor_name, - is_valid_bio_field, - is_valid_display_name, - is_valid_matrix_id, - is_valid_post_title, - is_valid_url, - site_name_length_check, - site_or_community_description_length_check, - BIO_MAX_LENGTH, - SITE_DESCRIPTION_MAX_LENGTH, - SITE_NAME_MAX_LENGTH, - URL_MAX_LENGTH, + build_and_check_regex, check_site_visibility_valid, check_urls_are_valid, clean_url, + clean_urls_in_text, is_url_blocked, is_valid_bio_field, is_valid_community_name, + is_valid_display_name, is_valid_matrix_id, is_valid_post_title, is_valid_url, + is_valid_username, site_name_length_check, site_or_community_description_length_check, + BIO_MAX_LENGTH, SITE_DESCRIPTION_MAX_LENGTH, SITE_NAME_MAX_LENGTH, URL_MAX_LENGTH, }, }; use pretty_assertions::assert_eq; @@ -421,23 +418,46 @@ mod tests { } #[test] - fn test_valid_actor_name() { + fn test_valid_username() { let actor_name_max_length = 20; - 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("تجريب", actor_name_max_length).is_ok()); - assert!(is_valid_actor_name("تجريب_123", actor_name_max_length).is_ok()); - assert!(is_valid_actor_name("Владимир", actor_name_max_length).is_ok()); + assert!(is_valid_username("Hello_98", actor_name_max_length).is_ok()); + assert!(is_valid_username("ten", actor_name_max_length).is_ok()); + assert!(is_valid_username("تجريب", actor_name_max_length).is_ok()); + assert!(is_valid_username("تجريب_123", actor_name_max_length).is_ok()); + assert!(is_valid_username("Владимир", actor_name_max_length).is_ok()); + + // mixed scripts + assert!(is_valid_username("تجريب_abc", actor_name_max_length).is_err()); + assert!(is_valid_username("Влад_abc", actor_name_max_length).is_err()); + // dash + assert!(is_valid_username("Hello-98", actor_name_max_length).is_err()); + // too short + assert!(is_valid_username("a", actor_name_max_length).is_err()); + // empty + assert!(is_valid_username("", actor_name_max_length).is_err()); + } + #[test] + fn test_valid_community_name() { + let actor_name_max_length = 20; + assert!(is_valid_community_name("hello_98", actor_name_max_length).is_ok()); + assert!(is_valid_community_name("ten", actor_name_max_length).is_ok()); + assert!(is_valid_community_name("تجريب", actor_name_max_length).is_ok()); + assert!(is_valid_community_name("تجريب_123", actor_name_max_length).is_ok()); + assert!(is_valid_community_name("владимир", actor_name_max_length).is_ok()); + + // uppercase + assert!(is_valid_community_name("Ten", actor_name_max_length).is_err()); + assert!(is_valid_community_name("Владимир", actor_name_max_length).is_err()); // mixed scripts - assert!(is_valid_actor_name("تجريب_abc", actor_name_max_length).is_err()); - assert!(is_valid_actor_name("Влад_abc", actor_name_max_length).is_err()); + assert!(is_valid_community_name("تجريب_abc", actor_name_max_length).is_err()); + assert!(is_valid_community_name("Влад_abc", actor_name_max_length).is_err()); // dash - assert!(is_valid_actor_name("Hello-98", actor_name_max_length).is_err()); + assert!(is_valid_community_name("hello-98", actor_name_max_length).is_err()); // too short - assert!(is_valid_actor_name("a", actor_name_max_length).is_err()); + assert!(is_valid_community_name("a", actor_name_max_length).is_err()); // empty - assert!(is_valid_actor_name("", actor_name_max_length).is_err()); + assert!(is_valid_community_name("", actor_name_max_length).is_err()); } #[test] From 1d086518d068188bf9db6c71e9810f37bb0cef0d Mon Sep 17 00:00:00 2001 From: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> Date: Sun, 3 Nov 2024 21:50:22 -0500 Subject: [PATCH 2/8] Lint --- crates/api_crud/src/community/create.rs | 21 +++++++++++++++++---- crates/api_crud/src/user/create.rs | 14 +++++++++++--- crates/utils/src/utils/validation.rs | 24 +++++++++++++++++++----- 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/crates/api_crud/src/community/create.rs b/crates/api_crud/src/community/create.rs index cc649bc89a..62929bb1e9 100644 --- a/crates/api_crud/src/community/create.rs +++ b/crates/api_crud/src/community/create.rs @@ -5,15 +5,26 @@ use lemmy_api_common::{ community::{CommunityResponse, CreateCommunity}, context::LemmyContext, utils::{ - generate_followers_url, generate_inbox_url, generate_local_apub_endpoint, get_url_blocklist, - is_admin, local_site_to_slur_regex, process_markdown_opt, proxy_image_link_api, EndpointType, + generate_followers_url, + generate_inbox_url, + generate_local_apub_endpoint, + get_url_blocklist, + is_admin, + local_site_to_slur_regex, + process_markdown_opt, + proxy_image_link_api, + EndpointType, }, }; use lemmy_db_schema::{ source::{ actor_language::{CommunityLanguage, SiteLanguage}, community::{ - Community, CommunityFollower, CommunityFollowerForm, CommunityInsertForm, CommunityModerator, + Community, + CommunityFollower, + CommunityFollowerForm, + CommunityInsertForm, + CommunityModerator, CommunityModeratorForm, }, }, @@ -26,7 +37,9 @@ use lemmy_utils::{ utils::{ slurs::check_slurs, validation::{ - is_valid_body_field, is_valid_community_name, site_or_community_description_length_check, + is_valid_body_field, + is_valid_community_name, + site_or_community_description_length_check, }, }, }; diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index adcb0082b8..52cda77b3c 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -6,9 +6,17 @@ use lemmy_api_common::{ oauth_provider::AuthenticateWithOauth, person::{LoginResponse, Register}, utils::{ - check_email_verified, check_registration_application, check_user_valid, generate_inbox_url, - generate_local_apub_endpoint, honeypot_check, local_site_to_slur_regex, password_length_check, - send_new_applicant_email_to_admins, send_verification_email, EndpointType, + check_email_verified, + check_registration_application, + check_user_valid, + generate_inbox_url, + generate_local_apub_endpoint, + honeypot_check, + local_site_to_slur_regex, + password_length_check, + send_new_applicant_email_to_admins, + send_verification_email, + EndpointType, }, }; use lemmy_db_schema::{ diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index ab7b243a14..04014981ea 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -366,11 +366,25 @@ mod tests { use crate::{ error::{LemmyErrorType, LemmyResult}, utils::validation::{ - build_and_check_regex, check_site_visibility_valid, check_urls_are_valid, clean_url, - clean_urls_in_text, is_url_blocked, is_valid_bio_field, is_valid_community_name, - is_valid_display_name, is_valid_matrix_id, is_valid_post_title, is_valid_url, - is_valid_username, site_name_length_check, site_or_community_description_length_check, - BIO_MAX_LENGTH, SITE_DESCRIPTION_MAX_LENGTH, SITE_NAME_MAX_LENGTH, URL_MAX_LENGTH, + build_and_check_regex, + check_site_visibility_valid, + check_urls_are_valid, + clean_url, + clean_urls_in_text, + is_url_blocked, + is_valid_bio_field, + is_valid_community_name, + is_valid_display_name, + is_valid_matrix_id, + is_valid_post_title, + is_valid_url, + is_valid_username, + site_name_length_check, + site_or_community_description_length_check, + BIO_MAX_LENGTH, + SITE_DESCRIPTION_MAX_LENGTH, + SITE_NAME_MAX_LENGTH, + URL_MAX_LENGTH, }, }; use pretty_assertions::assert_eq; From 982b3312f4fbc7bcb8575884d1e8c0c601fa5585 Mon Sep 17 00:00:00 2001 From: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> Date: Sun, 3 Nov 2024 21:56:08 -0500 Subject: [PATCH 3/8] Remove redundant check --- crates/utils/src/utils/validation.rs | 42 +++++++++++++++------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index 04014981ea..7dafa39da0 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -111,7 +111,7 @@ pub fn is_valid_community_name(name: &str, actor_name_max_length: usize) -> Lemm } fn is_valid_actor_name(name: &str, actor_name_max_length: usize, r: &Regex) -> LemmyResult<()> { - if name.len() <= actor_name_max_length && !has_newline(name) && r.is_match(name) { + if name.len() <= actor_name_max_length && r.is_match(name) { Ok(()) } else { Err(LemmyErrorType::InvalidName.into()) @@ -366,25 +366,11 @@ mod tests { use crate::{ error::{LemmyErrorType, LemmyResult}, utils::validation::{ - build_and_check_regex, - check_site_visibility_valid, - check_urls_are_valid, - clean_url, - clean_urls_in_text, - is_url_blocked, - is_valid_bio_field, - is_valid_community_name, - is_valid_display_name, - is_valid_matrix_id, - is_valid_post_title, - is_valid_url, - is_valid_username, - site_name_length_check, - site_or_community_description_length_check, - BIO_MAX_LENGTH, - SITE_DESCRIPTION_MAX_LENGTH, - SITE_NAME_MAX_LENGTH, - URL_MAX_LENGTH, + build_and_check_regex, check_site_visibility_valid, check_urls_are_valid, clean_url, + clean_urls_in_text, is_url_blocked, is_valid_bio_field, is_valid_community_name, + is_valid_display_name, is_valid_matrix_id, is_valid_post_title, is_valid_url, + is_valid_username, site_name_length_check, site_or_community_description_length_check, + BIO_MAX_LENGTH, SITE_DESCRIPTION_MAX_LENGTH, SITE_NAME_MAX_LENGTH, URL_MAX_LENGTH, }, }; use pretty_assertions::assert_eq; @@ -449,6 +435,14 @@ mod tests { assert!(is_valid_username("a", actor_name_max_length).is_err()); // empty assert!(is_valid_username("", actor_name_max_length).is_err()); + // newline + assert!(is_valid_username( + r"Line1 + +Line3", + actor_name_max_length + ) + .is_err()); } #[test] @@ -472,6 +466,14 @@ mod tests { assert!(is_valid_community_name("a", actor_name_max_length).is_err()); // empty assert!(is_valid_community_name("", actor_name_max_length).is_err()); + // newline + assert!(is_valid_community_name( + r"Line1 + +Line3", + actor_name_max_length + ) + .is_err()); } #[test] From 34ee03f906e7fbcb89c4a56b39fb1ae3fef6b431 Mon Sep 17 00:00:00 2001 From: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> Date: Sun, 3 Nov 2024 21:56:39 -0500 Subject: [PATCH 4/8] Lint --- crates/utils/src/utils/validation.rs | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index 7dafa39da0..bd4615d76b 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -366,11 +366,25 @@ mod tests { use crate::{ error::{LemmyErrorType, LemmyResult}, utils::validation::{ - build_and_check_regex, check_site_visibility_valid, check_urls_are_valid, clean_url, - clean_urls_in_text, is_url_blocked, is_valid_bio_field, is_valid_community_name, - is_valid_display_name, is_valid_matrix_id, is_valid_post_title, is_valid_url, - is_valid_username, site_name_length_check, site_or_community_description_length_check, - BIO_MAX_LENGTH, SITE_DESCRIPTION_MAX_LENGTH, SITE_NAME_MAX_LENGTH, URL_MAX_LENGTH, + build_and_check_regex, + check_site_visibility_valid, + check_urls_are_valid, + clean_url, + clean_urls_in_text, + is_url_blocked, + is_valid_bio_field, + is_valid_community_name, + is_valid_display_name, + is_valid_matrix_id, + is_valid_post_title, + is_valid_url, + is_valid_username, + site_name_length_check, + site_or_community_description_length_check, + BIO_MAX_LENGTH, + SITE_DESCRIPTION_MAX_LENGTH, + SITE_NAME_MAX_LENGTH, + URL_MAX_LENGTH, }, }; use pretty_assertions::assert_eq; From dd92f37eeb9c8a1e3542b1d4e87f21b4ac609230 Mon Sep 17 00:00:00 2001 From: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> Date: Mon, 4 Nov 2024 17:28:27 -0500 Subject: [PATCH 5/8] Add newline escape tests --- crates/utils/src/utils/validation.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index bd4615d76b..f55b488fd7 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -457,6 +457,7 @@ Line3", actor_name_max_length ) .is_err()); + assert!(is_valid_username("Line1\nLine3", actor_name_max_length).is_err()); } #[test] @@ -488,6 +489,7 @@ Line3", actor_name_max_length ) .is_err()); + assert!(is_valid_community_name("Line1\nLine3", actor_name_max_length).is_err()); } #[test] From 6c70cecc5f8a9657d747ef5c39fd6457786e2876 Mon Sep 17 00:00:00 2001 From: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> Date: Mon, 4 Nov 2024 17:31:17 -0500 Subject: [PATCH 6/8] Eliminate confounding factor in community name test --- crates/utils/src/utils/validation.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index f55b488fd7..bcb5787905 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -483,13 +483,13 @@ Line3", assert!(is_valid_community_name("", actor_name_max_length).is_err()); // newline assert!(is_valid_community_name( - r"Line1 + r"line1 -Line3", +line3", actor_name_max_length ) .is_err()); - assert!(is_valid_community_name("Line1\nLine3", actor_name_max_length).is_err()); + assert!(is_valid_community_name("line1\nline3", actor_name_max_length).is_err()); } #[test] From 28b7f08570cf9c1af1d35b1a2abf1f5a5df0426a Mon Sep 17 00:00:00 2001 From: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> Date: Wed, 6 Nov 2024 09:20:08 -0500 Subject: [PATCH 7/8] Use same check for user names and community names --- crates/api_crud/src/community/create.rs | 4 +- crates/api_crud/src/user/create.rs | 4 +- crates/utils/src/utils/validation.rs | 89 ++++++------------------- 3 files changed, 23 insertions(+), 74 deletions(-) diff --git a/crates/api_crud/src/community/create.rs b/crates/api_crud/src/community/create.rs index 62929bb1e9..cd0fc985ef 100644 --- a/crates/api_crud/src/community/create.rs +++ b/crates/api_crud/src/community/create.rs @@ -37,8 +37,8 @@ use lemmy_utils::{ utils::{ slurs::check_slurs, validation::{ + is_valid_actor_name, is_valid_body_field, - is_valid_community_name, site_or_community_description_length_check, }, }, @@ -80,7 +80,7 @@ pub async fn create_community( let banner = diesel_url_create(data.banner.as_deref())?; let banner = proxy_image_link_api(banner, &context).await?; - is_valid_community_name(&data.name, local_site.actor_name_max_length as usize)?; + is_valid_actor_name(&data.name, local_site.actor_name_max_length as usize)?; // Double check for duplicate community actor_ids let community_actor_id = generate_local_apub_endpoint( diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index 52cda77b3c..ed560e3d6a 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -41,7 +41,7 @@ use lemmy_utils::{ error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult}, utils::{ slurs::{check_slurs, check_slurs_opt}, - validation::is_valid_username, + validation::is_valid_actor_name, }, }; use serde::{Deserialize, Serialize}; @@ -407,7 +407,7 @@ async fn create_person( context: &Data, ) -> Result { let actor_keypair = generate_actor_keypair()?; - is_valid_username(&username, local_site.actor_name_max_length as usize)?; + is_valid_actor_name(&username, local_site.actor_name_max_length as usize)?; let actor_id = generate_local_apub_endpoint( EndpointType::Person, &username, diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index bcb5787905..f4f2497688 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -84,34 +84,16 @@ fn has_newline(name: &str) -> bool { name.contains('\n') } -pub fn is_valid_username(name: &str, actor_name_max_length: usize) -> LemmyResult<()> { +pub fn is_valid_actor_name(name: &str, actor_name_max_length: usize) -> LemmyResult<()> { // Only allow characters from a single alphabet per username. This avoids problems with lookalike // characters like `o` which looks identical in Latin and Cyrillic, and can be used to imitate // other users. Checks for additional alphabets can be added in the same way. - static VALID_USERNAME_REGEX: LazyLock = LazyLock::new(|| { - Regex::new(r"^(?:[a-zA-Z0-9_]{3,}|[0-9_\p{Arabic}]{3,}|[0-9_\p{Cyrillic}]{3,})$") - .expect("compile regex") + static VALID_ACTOR_NAME_REGEX: LazyLock = LazyLock::new(|| { + Regex::new(r"^(?:[a-zA-Z0-9_]+|[0-9_\p{Arabic}]+|[0-9_\p{Cyrillic}]+)$").expect("compile regex") }); - is_valid_actor_name(name, actor_name_max_length, &VALID_USERNAME_REGEX) -} - -pub fn is_valid_community_name(name: &str, actor_name_max_length: usize) -> LemmyResult<()> { - // Only allow characters from a single alphabet per username. This avoids problems with lookalike - // characters like `o` which looks identical in Latin and Cyrillic, and can be used to imitate - // other users. Checks for additional alphabets can be added in the same way. - static VALID_COMMUNITY_NAME_REGEX: LazyLock = LazyLock::new(|| { - Regex::new( - r"^(?:[0-9_a-z]{3,}|[0-9_[\p{Arabic}]&&\P{Lu}&&\P{Lt}]{3,}|[0-9_[\p{Cyrillic}&&\P{Lu}&&\P{Lt}]]{3,})$", - ) - .expect("compile regex") - }); - - is_valid_actor_name(name, actor_name_max_length, &VALID_COMMUNITY_NAME_REGEX) -} - -fn is_valid_actor_name(name: &str, actor_name_max_length: usize, r: &Regex) -> LemmyResult<()> { - if name.len() <= actor_name_max_length && r.is_match(name) { + if name.len() <= actor_name_max_length && name.len() >= 3 && VALID_ACTOR_NAME_REGEX.is_match(name) + { Ok(()) } else { Err(LemmyErrorType::InvalidName.into()) @@ -372,13 +354,12 @@ mod tests { clean_url, clean_urls_in_text, is_url_blocked, + is_valid_actor_name, is_valid_bio_field, - is_valid_community_name, is_valid_display_name, is_valid_matrix_id, is_valid_post_title, is_valid_url, - is_valid_username, site_name_length_check, site_or_community_description_length_check, BIO_MAX_LENGTH, @@ -432,64 +413,32 @@ mod tests { } #[test] - fn test_valid_username() { + fn test_valid_actor_name() { let actor_name_max_length = 20; - assert!(is_valid_username("Hello_98", actor_name_max_length).is_ok()); - assert!(is_valid_username("ten", actor_name_max_length).is_ok()); - assert!(is_valid_username("تجريب", actor_name_max_length).is_ok()); - assert!(is_valid_username("تجريب_123", actor_name_max_length).is_ok()); - assert!(is_valid_username("Владимир", actor_name_max_length).is_ok()); + 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("تجريب", actor_name_max_length).is_ok()); + assert!(is_valid_actor_name("تجريب_123", actor_name_max_length).is_ok()); + assert!(is_valid_actor_name("Владимир", actor_name_max_length).is_ok()); // mixed scripts - assert!(is_valid_username("تجريب_abc", actor_name_max_length).is_err()); - assert!(is_valid_username("Влад_abc", actor_name_max_length).is_err()); + assert!(is_valid_actor_name("تجريب_abc", actor_name_max_length).is_err()); + assert!(is_valid_actor_name("Влад_abc", actor_name_max_length).is_err()); // dash - assert!(is_valid_username("Hello-98", actor_name_max_length).is_err()); + assert!(is_valid_actor_name("Hello-98", actor_name_max_length).is_err()); // too short - assert!(is_valid_username("a", actor_name_max_length).is_err()); + assert!(is_valid_actor_name("a", actor_name_max_length).is_err()); // empty - assert!(is_valid_username("", actor_name_max_length).is_err()); + assert!(is_valid_actor_name("", actor_name_max_length).is_err()); // newline - assert!(is_valid_username( + assert!(is_valid_actor_name( r"Line1 Line3", actor_name_max_length ) .is_err()); - assert!(is_valid_username("Line1\nLine3", actor_name_max_length).is_err()); - } - - #[test] - fn test_valid_community_name() { - let actor_name_max_length = 20; - assert!(is_valid_community_name("hello_98", actor_name_max_length).is_ok()); - assert!(is_valid_community_name("ten", actor_name_max_length).is_ok()); - assert!(is_valid_community_name("تجريب", actor_name_max_length).is_ok()); - assert!(is_valid_community_name("تجريب_123", actor_name_max_length).is_ok()); - assert!(is_valid_community_name("владимир", actor_name_max_length).is_ok()); - - // uppercase - assert!(is_valid_community_name("Ten", actor_name_max_length).is_err()); - assert!(is_valid_community_name("Владимир", actor_name_max_length).is_err()); - // mixed scripts - assert!(is_valid_community_name("تجريب_abc", actor_name_max_length).is_err()); - assert!(is_valid_community_name("Влад_abc", actor_name_max_length).is_err()); - // dash - assert!(is_valid_community_name("hello-98", actor_name_max_length).is_err()); - // too short - assert!(is_valid_community_name("a", actor_name_max_length).is_err()); - // empty - assert!(is_valid_community_name("", actor_name_max_length).is_err()); - // newline - assert!(is_valid_community_name( - r"line1 - -line3", - actor_name_max_length - ) - .is_err()); - assert!(is_valid_community_name("line1\nline3", actor_name_max_length).is_err()); + assert!(is_valid_actor_name("Line1\nLine3", actor_name_max_length).is_err()); } #[test] From 686ae382bfd668065953dd980c5239f22fa8e4dc Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 12 Nov 2024 11:08:33 +0100 Subject: [PATCH 8/8] Use min/max length check --- crates/utils/src/utils/validation.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index f4f2497688..a858a6b5c8 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -92,8 +92,9 @@ pub fn is_valid_actor_name(name: &str, actor_name_max_length: usize) -> LemmyRes Regex::new(r"^(?:[a-zA-Z0-9_]+|[0-9_\p{Arabic}]+|[0-9_\p{Cyrillic}]+)$").expect("compile regex") }); - if name.len() <= actor_name_max_length && name.len() >= 3 && VALID_ACTOR_NAME_REGEX.is_match(name) - { + min_length_check(name, 3, LemmyErrorType::InvalidName)?; + max_length_check(name, actor_name_max_length, LemmyErrorType::InvalidName)?; + if VALID_ACTOR_NAME_REGEX.is_match(name) { Ok(()) } else { Err(LemmyErrorType::InvalidName.into())