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

Update username regex to string literal with escaped - #1121

Merged
merged 2 commits into from
Mar 17, 2019

Conversation

LogicalPhallacy
Copy link
Contributor

Changes
The username regex was hard to parse because of the double \ and the unescaped -

Even though it worked this minor fix makes it clearer what it is doing

Emby.Server.Implementations/Library/UserManager.cs Outdated Show resolved Hide resolved
// Usernames can contain letters (a-z + whatever else unicode is cool with), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)
return Regex.IsMatch(username, "^[\\w-'._@]*$");
return Regex.IsMatch(username, @"^[\w\-'._@]*$");
Copy link
Contributor

Choose a reason for hiding this comment

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

A lot more readable, good job!

Copy link
Member

@cvium cvium left a comment

Choose a reason for hiding this comment

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

LGTM

@joshuaboniface joshuaboniface changed the title updated username regex to string literal with escaped - Updat username regex to string literal with escaped - Mar 16, 2019
Copy link
Member

@joshuaboniface joshuaboniface left a comment

Choose a reason for hiding this comment

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

LGTM pending my suggestion on @LeoVerto's review.

Re: the title: please capitalize and use imperative mood :)

@joshuaboniface joshuaboniface changed the title Updat username regex to string literal with escaped - Update username regex to string literal with escaped - Mar 16, 2019
Co-Authored-By: LogicalPhallacy <44458166+LogicalPhallacy@users.noreply.github.com>
Copy link
Contributor

@LeoVerto LeoVerto left a comment

Choose a reason for hiding this comment

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

LGTM!

@joshuaboniface joshuaboniface merged commit 67fbbcf into jellyfin:master Mar 17, 2019
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.

5 participants