Improve github regexes #3643
Improve github regexes #3643
Conversation
The existing regexes were incorrect, because they would allow long owner names, which are not allowed, or in a format that is not appropriate (for example, containing consecutive `-`. According to the form validation messages on Join Github page, - Github username (owner) may only contain alphanumeric characters or hyphens. - Github username (owner) cannot have multiple consecutive hyphens. - Github username (owner) cannot begin or end with a hyphen. - Maximum is 39 characters. Similarly, the repository name can contain any alphanumeric, ., -, and _.
knl
force-pushed
the
fix-github-owner-and-repo-regexes
branch
from
88f9a3e
to
88c62ea
Compare
| // - Github username (owner) may only contain alphanumeric characters or hyphens. | ||
| // - Github username (owner) cannot have multiple consecutive hyphens. | ||
| // - Github username (owner) cannot begin or end with a hyphen. | ||
| // - Maximum is 39 characters. |
There was a problem hiding this comment.
Can you add some unit tests for these cases with google test? E.g.
https://github.com/NixOS/nix/pulls?q=is%3Apr+is%3Aclosed+unit+tests
There was a problem hiding this comment.
Sure. However, I noticed that these regexes are actually not used. I suspect the flakes branch uses them. Could you confirm that?
There was a problem hiding this comment.
The flake branch uses those regexes. Both for GitLab and Github. Do you know if they have the same limitations?
There was a problem hiding this comment.
Hah, they don't have the same limits!
GitLab is even quirkier:
- username can contain alphanumerics, -, _, and .
- cannot end with .git and .atom
- max length is 255 chars
Project name has:
- max length of 255 chars
- can contain only letters, digits, emojis, '_', '.', dash, space.
- It must start with letter, digit, emoji or '_'."
However, the max length of a path is 255 chars. Thus username/projectname in sum should be up to 255 chars... Oh boy.
Sources:
- https://gitlab.com/gitlab-org/gitlab/-/issues/197976
- https://gitlab.com/gitlab-org/gitlab-foss/-/blob/ae93b284016c07a8a4b47e2510789253d14870f3/lib/gitlab/path_regex.rb#L136
- https://gitlab.com/gitlab-org/gitlab-foss/-/blob/master/app/models/user.rb#L189
- https://gitlab.com/gitlab-org/gitlab-foss/-/blob/master/app/models/project.rb#L387
- https://gitlab.com/gitlab-org/gitlab-foss/-/blob/master/app/models/project.rb#L392
- https://gitlab.com/gitlab-org/gitlab-foss/-/blob/d8803c7e40bd35d883ef007ddc56907bd837a748/lib/gitlab/regex.rb#L11
There was a problem hiding this comment.
What's actually the advantage of Nix validating GitHub/GitLab org/repo names? We're not doing anything with them except pass them along.
There was a problem hiding this comment.
What's actually the advantage of Nix validating GitHub/GitLab org/repo names? We're not doing anything with them except pass them along.
That's what I've been asking myself. But since the code is there (I was on the verge on removing it before seeing the flakes branch), so since it is used better to make it used correctly.
The advantage of validating the org/repo names is that we could provide a better error message. Disadvantage is that we need to play catch-up (although, I don't think these limits will change).
There was a problem hiding this comment.
Note that the flake branch doesn't actually use these regexes anymore (not really a conscious decision, I think the code to check them got lost somewhere) so I deleted them in 5633c09.
It might be a good idea to at least check for names like .. since they might have security implications.
The existing regexes were incorrect, because they would allow long owner names,
which are not allowed, or in a format that is not appropriate (for example,
containing consecutive
-.According to the form validation messages on Join Github page,
Similarly, the repository name can contain any alphanumeric, ., -, and _.