Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved validation of display names (Fixes #3436) #3437

Merged
merged 6 commits into from
Jul 4, 2023

Conversation

Josephos
Copy link
Contributor

@Josephos Josephos commented Jul 1, 2023

Fixed validation of display names: reject names beginning with invisible unicode characters.
Fixes #3436

Fixed validation of display names: reject names beginning with invisible unicode characters.
@Josephos Josephos changed the title Improved validation of display names Improved validation of display names (Fixes #3436) Jul 1, 2023
Formatting fix.
@@ -24,6 +24,9 @@ const BIO_MAX_LENGTH: usize = 300;
const SITE_NAME_MAX_LENGTH: usize = 20;
const SITE_NAME_MIN_LENGTH: usize = 1;
const SITE_DESCRIPTION_MAX_LENGTH: usize = 150;
const FORBIDDEN_DISPLAY_CHARS: [char; 7] = [
'@', '\u{180e}', '\u{200b}', '\u{2060}', '\u{2800}', '\u{3164}', '\u{ffef}',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a reference for this list? The site below lists even more.

https://invisible-characters.com/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used this: https://unicode-explorer.com/articles/space-characters and manually tested most of them, added those that worked. Your list is better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include a comment with the link as well, and mention that these are specifically invisible chars.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -42,8 +45,7 @@ pub fn is_valid_actor_name(name: &str, actor_name_max_length: usize) -> LemmyRes

// Can't do a regex here, reverse lookarounds not supported
pub fn is_valid_display_name(name: &str, actor_name_max_length: usize) -> LemmyResult<()> {
let check = !name.starts_with('@')
&& !name.starts_with('\u{200b}')
let check = !name.starts_with(FORBIDDEN_DISPLAY_CHARS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about disallowing these chars anywhere in the name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented. Check for '@' is still done only for the first letter.

@Josephos
Copy link
Contributor Author

Josephos commented Jul 3, 2023

Updated with expanded list of forbidden characters and check for occurrences anywhere in the name.

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll let nutomic also approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: It's possible to change display name to blank using invisible unicode characters.
3 participants