Is there really an invalid username? #259

New Issue

OVERLORD · 2026-02-06T19:30:39+03:00

OVERLORD commented

2026-02-06 19:30:39 +03:00

Originally created by @ploughpuff on GitHub (Jan 10, 2019).

UserManager has function IsValidUsername() and a comment within suggests usernames are limits to characters [a-z0-9-_'.]

Looking at the code, I cannot see how that's achieved (stale comment perhaps?). And in testing it sure does only guard against the use of '<' (U+003C) and '>' (U+003E) characters.

Question is, do we care what a valid username looks like?

Can this bunch of code be pruned away?

        if (!IsValidUsername(name))
        {
                throw new ArgumentException("Usernames can contain letters (a-z), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)");
        }

        public bool IsValidUsername(string username)
        {
            // Usernames can contain letters (a-z), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)
            foreach (var currentChar in username)
            {
                if (!IsValidUsernameCharacter(currentChar))
                {
                    return false;
                }
            }
            return true;
        }

        private bool IsValidUsernameCharacter(char i)
        {
            return !char.Equals(i, '<') && !char.Equals(i, '>');
        }

Edit by @JustAMan: I've added syntax highlighting

Originally created by @ploughpuff on GitHub (Jan 10, 2019). UserManager has function IsValidUsername() and a comment within suggests usernames are limits to characters [a-z0-9-_'.] Looking at the code, I cannot see how that's achieved (stale comment perhaps?). And in testing it sure does only guard against the use of '<' (U+003C) and '>' (U+003E) characters. Question is, do we care what a valid username looks like? Can this bunch of code be pruned away? ```cs if (!IsValidUsername(name)) { throw new ArgumentException("Usernames can contain letters (a-z), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)"); } public bool IsValidUsername(string username) { // Usernames can contain letters (a-z), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.) foreach (var currentChar in username) { if (!IsValidUsernameCharacter(currentChar)) { return false; } } return true; } private bool IsValidUsernameCharacter(char i) { return !char.Equals(i, '<') && !char.Equals(i, '>'); } ``` **Edit by @JustAMan**: I've added syntax highlighting

OVERLORD added the enhancement backend labels 2026-02-06 19:30:39 +03:00

OVERLORD closed this issue

2026-02-06 19:30:39 +03:00

OVERLORD commented

2026-02-06 19:30:42 +03:00

@LogicalPhallacy commented on GitHub (Jan 11, 2019):

This should probably not be pruned but should be updated to a piece of
regex that would restrict usernames to a reasonable policy for their
inevitable sql storage destination.

On Thu, Jan 10, 2019 at 12:52 PM ploughpuff notifications@github.com
wrote:

UserManager has function IsValidUsername() and a comment within suggests
usernames are limits to characters [a-z0-9-_'.]

Looking at the code, I cannot see how that's achieved (stale comment
perhaps?). And in testing it sure does only guard against the use of '<'
(U+003C) and '>' (U+003E) characters.

Question is, do we care what a valid username looks like?

Can this bunch of code be pruned away?
    if (!IsValidUsername(name))
    {
            throw new ArgumentException("Usernames can contain letters (a-z), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)");
    }

    public bool IsValidUsername(string username)
    {
        // Usernames can contain letters (a-z), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)
        foreach (var currentChar in username)
        {
            if (!IsValidUsernameCharacter(currentChar))
            {
                return false;
            }
        }
        return true;
    }

    private bool IsValidUsernameCharacter(char i)
    {
        return !char.Equals(i, '<') && !char.Equals(i, '>');
    }
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/jellyfin/jellyfin/issues/545, or mute the thread
https://github.com/notifications/unsubscribe-auth/AqZgtn-vHY9Nfhs7driYreGAcFEiHYlJks5vB6gngaJpZM4Z6S3p
.

--
It has always been the prerogative of children and half-wits to point out
that the emperor has no clothes. But the half-wit remains a half- wit, and
the emperor remains an emperor.

* Dream, in SANDMAN #60: "The Kindly Ones:4"

@LogicalPhallacy commented on GitHub (Jan 11, 2019): This should probably not be pruned but should be updated to a piece of regex that would restrict usernames to a reasonable policy for their inevitable sql storage destination. On Thu, Jan 10, 2019 at 12:52 PM ploughpuff <notifications@github.com> wrote: > UserManager has function IsValidUsername() and a comment within suggests > usernames are limits to characters [a-z0-9-_'.] > > Looking at the code, I cannot see how that's achieved (stale comment > perhaps?). And in testing it sure does only guard against the use of '<' > (U+003C) and '>' (U+003E) characters. > > Question is, do we care what a valid username looks like? > > Can this bunch of code be pruned away? > > if (!IsValidUsername(name)) > { > throw new ArgumentException("Usernames can contain letters (a-z), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)"); > } > > public bool IsValidUsername(string username) > { > // Usernames can contain letters (a-z), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.) > foreach (var currentChar in username) > { > if (!IsValidUsernameCharacter(currentChar)) > { > return false; > } > } > return true; > } > > private bool IsValidUsernameCharacter(char i) > { > return !char.Equals(i, '<') && !char.Equals(i, '>'); > } > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <https://github.com/jellyfin/jellyfin/issues/545>, or mute the thread > <https://github.com/notifications/unsubscribe-auth/AqZgtn-vHY9Nfhs7driYreGAcFEiHYlJks5vB6gngaJpZM4Z6S3p> > . > -- It has always been the prerogative of children and half-wits to point out that the emperor has no clothes. But the half-wit remains a half- wit, and the emperor remains an emperor. * Dream, in SANDMAN #60: "The Kindly Ones:4"

OVERLORD commented

2026-02-06 19:30:43 +03:00

@ploughpuff commented on GitHub (Jan 11, 2019):

At the moment the username is stored in the users db as part of a BLOB, so I guess there are no storage restrictions. If that approach changes I guess the datatype TEXT shall be used which supports UTF-8.

I also performed some testing with 2-byte wide chars (Greek letters) which are handled and displayed correctly. I'm not sure how you would craft a regex to handle Roman+Greek+Cyrillic+Chinese+BIG5 etc. etc. ?? Is that possible?

Alternatively, we scan the string for control characters (bytes <0x20)?

@ploughpuff commented on GitHub (Jan 11, 2019): At the moment the username is stored in the users db as part of a BLOB, so I guess there are no storage restrictions. If that approach changes I guess the datatype TEXT shall be used which supports UTF-8. I also performed some testing with 2-byte wide chars (Greek letters) which are handled and displayed correctly. I'm not sure how you would craft a regex to handle Roman+Greek+Cyrillic+Chinese+BIG5 etc. etc. ?? Is that possible? Alternatively, we scan the string for control characters (bytes <0x20)?

OVERLORD commented

2026-02-06 19:30:44 +03:00

@JustAMan commented on GitHub (Jan 12, 2019):

I think there should be no restrictions. The only ones that remotely make sense are applied if username is passed in a web request or a DB query, but sane implementations of these escape their arguments properly anyway...

@JustAMan commented on GitHub (Jan 12, 2019): I think there should be no restrictions. The only ones that remotely make sense are applied if username is passed in a web request or a DB query, but _sane_ implementations of these escape their arguments properly anyway...

Sign in to join this conversation.

Branches Tags

master

release-10.11.z

renovate/skiasharp-monorepo

JPVenson-patch-1

renovate/morestachio-5.x

release-10.9.z

openapi-cache

release-10.10.z

explicit-load

explicit-load-v2

feature/entity-framework

only-count-folder-item

fix/ci-workflow-refactor

release-10.8.z

release-10.7.z

EraYaN-add-management-interface

release-10.6.z

release-10.5.z

release-10.4.z

release-10.3.z

release-10.2.z

release-10.1.0

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/jellyfin#259