Adding check to description and body length fields. #2805
Conversation
- Also making the checks return `LemmyError` - Fixes #1747
crates/utils/src/utils/validation.rs
Outdated
|
|
||
| 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) -> Result<(), LemmyError> { |
There was a problem hiding this comment.
These are all called from the API, and needed to return lemmyerror's on failure anyway.
crates/utils/src/utils/validation.rs
Outdated
| } | ||
| } | ||
|
|
||
| pub fn is_valid_bio_field(bio: &str) -> Result<(), LemmyError> { |
There was a problem hiding this comment.
You can also use LemmyResult which I added recently to simplify this a bit.
crates/utils/src/utils/validation.rs
Outdated
| } | ||
|
|
||
| pub fn is_valid_post_title(title: &str) -> bool { | ||
| VALID_POST_TITLE_REGEX.is_match(title) && !has_newline(title) | ||
| pub fn is_valid_body_field(body: &str) -> Result<(), LemmyError> { |
There was a problem hiding this comment.
Would call this is_valid_post_body() for clarity.
There was a problem hiding this comment.
Nevermind it seems to be used both for posts and comments. Would then add a comment mentioning that.
crates/api/src/community/ban.rs
Outdated
| @@ -46,6 +50,9 @@ 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?; | |||
| if let Some(reason) = &data.reason { | |||
| is_valid_body_field(reason)?; | |||
| } | |||
There was a problem hiding this comment.
As most of these are optional, I think it would be cleaner if the functions would also take Option<String> as param.
There was a problem hiding this comment.
I thought about that, but there's a lot of places where the bodies are possibly Option<Option<String>>, and some where its just String, so it seemed best to leave it outside, unless I want to write several overloaded wrapper functions rather than just one.
There was a problem hiding this comment.
Im not seeing Option<Option<String>> anywhere. And there are only a few cases without option, there it would work to pass the parameter as Some(body).
There was a problem hiding this comment.
Ok, it required a few clones in some places, but it works now.
|
CI is failing, feel free to merge when it passes. |
|
Tough one, cause I just tested this locally, and its working. |
* Adding check to description and body length fields. - Also making the checks return `LemmyError` - Fixes #1747 * Address PR comments. * PR comments 2
LemmyError