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

Invalidate username starting with ?. Fix #7205. #7242

Closed
wants to merge 1 commit into
base: master
from

Conversation

3 participants
@cirosantilli
Contributor

cirosantilli commented Jul 2, 2014

Fix #7205

This fixes the issue minimally, but what I would really like to do is:

/\A(?!(\.git|\.|\.\.)\z)[.]?[a-zA-Z0-9_][a-zA-Z0-9_.-]*\z/

Which would also allow:

  • usernames that end in .git, except .git exactly. Denying extension feels like an unnecessary restriction.
  • ., - and _ to be used anywhere, execpt for . and .. exactly. The validation would be simpler for humans to understand, and ugly usernames are already possible like ._ so it does not matter much.

I wish we had used:

/\A[a-zA-Z][a-zA-Z0-9-]*\z/

from day one like GitHub, but now would be data destructive =(

end
def path_regex_message
default_regex_message
"can contain only letters, digits, '_', '-' and '.'. " \
"It must start with letter, digit or '_', optionally preceeded by '.' or '?'. " \

This comment has been minimized.

@houndci-bot

houndci-bot Jul 2, 2014

Line is too long. [87/80]

@cirosantilli cirosantilli changed the title from Invalidate username starging with ?. Fix #7205. to Invalidate username starting with ?. Fix #7205. Jul 24, 2014

@@ -28,11 +30,14 @@ def name_regex_message
end
def path_regex
default_regex

This comment has been minimized.

@jvanbaarsen

jvanbaarsen Aug 16, 2014

Contributor

What do you think about moving the regex back to the default_regex method? that way you only have to change the regex once :)

@jvanbaarsen

This comment has been minimized.

Contributor

jvanbaarsen commented Aug 16, 2014

@cirosantilli I've left one comment

@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Aug 16, 2014

Looking at: https://github.com/gitlabhq/gitlabhq/blob/master/lib/gitlab/regex.rb it seems that this was fixed. I kept the regexes different to not invalidate potentially existing paths starting with ? (only usernames which could not have been created because of the 500), but I think randx invalidated them anyways since it does not matter much as they have already been created so validation won't be done anymore.

@jvanbaarsen

This comment has been minimized.

Contributor

jvanbaarsen commented Aug 16, 2014

Ok!

@cirosantilli cirosantilli deleted the cirosantilli:username-question branch Aug 16, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment