Skip to content

Conversation

higgs01
Copy link
Contributor

@higgs01 higgs01 commented Jul 16, 2025

Fixes #257

This PR contains the following:

  • Usernames are now sanitized on create/username update and prohibited words/chars are removed from the username
  • If prohibited words/chars were sanitized the display_name is set to the username originally provided by the user
  • If the sanitised username is shorter than the min required length the username will be padded with _
  • Tests for validation and sanitisation of usernames
  • The decancer crate is updated from 1.6.2 to 3.3.3 to facilitate preserving capitalisation on sanitisation

In the Issue it was mentioned that it might be a good idea to make the min-length of the username configurable. I left this out of this PR for now, since I don't see a nice was to do this. If you've got an Idea I'm open to suggestions.

While testing this I noticed that while creating/renaming a bot the username is updated in the UI only after reloading. I'm not quite shure as to where I'd have to fix this and would like some guidance so that I can fix that there so that the UX is consistent for normal and bot users.

Please make sure to check the following tasks before opening and submitting a PR

  • I understand and have followed the contribution guide
  • I have tested my changes locally and they are working as intended

// sanitise homoglyphs
let options = decancer::Options::default()
.retain_capitalization();
let mut username = decancer::cure(&username, options).unwrap().to_string();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let mut username = decancer::cure(&username, options).unwrap().to_string();
let mut username = decancer::cure(username, options).unwrap().to_string();

Comment on lines +316 to 318
if BLOCKED_USERNAME_PATTERNS.is_match(&username) {
username = BLOCKED_USERNAME_PATTERNS.replace_all(&mut username, "").into_owned();
}
Copy link
Member

Choose a reason for hiding this comment

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

Running the regex twice seems redundant. replace_all has no performance hit if a match isn't found.

username.push_str(&"_".repeat(username_length_diff))
}

(original_username != username, username)
Copy link
Member

Choose a reason for hiding this comment

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

returning a boolean here is strange considering its usages already hold the original name, and it can simply be checked out there.

@github-project-automation github-project-automation bot moved this from 🆕 Untriaged to 🛑 Changes requested in Pull Request Overview Aug 3, 2025
@IAmTomahawkx
Copy link
Member

Making the min length configurable would just be a matter of adding an entry into revolt_config's models and the built-in Revolt.toml, then calling revolt_config::config().await. You'd need to make sanitise_username async too.

@higgs01
Copy link
Contributor Author

higgs01 commented Aug 4, 2025

Thanks for the review - I'll probably get to looking at it next week.

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

Successfully merging this pull request may close these issues.

feat: auto-sanitise name when creating bot / user
2 participants