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

Improve ref validity checking in fetchgit #3642

Merged

Conversation

knl
Copy link
Contributor

@knl knl commented May 30, 2020

The previous regex was too strict and did not match what git was allowing. It
could lead to fetchGit not accepting valid branch names, even though they
exist in a repository (for example, branch names containing /, which are
pretty standard, like release/1.0 branches).

The new regex defines what a branch name should NOT contain. It takes the
definitions from refs.c in https://github.com/git/git and git help check-ref-format pages.

This change also introduces a test for ref name validity checking, which
compares the result from Nix with the result of git check-ref-format --branch.

Should fix #3612

knl added 2 commits May 30, 2020 12:29
The previous regex was too strict and did not match what git was allowing. It
could lead to `fetchGit` not accepting valid branch names, even though they
exist in a repository (for example, branch names containing `/`, which are
pretty standard, like `release/1.0` branches).

The new regex defines what a branch name should **NOT** contain. It takes the
definitions from `refs.c` in https://github.com/git/git and `git help
check-ref-format` pages.

This change also introduces a test for ref name validity checking, which
compares the result from Nix with the result of `git check-ref-format --branch`.
As `git fetch` may chose to interpret refspec to it's liking, ensure that we
only pass refs that begin with `refs/` as is, otherwise, prepend them with
`refs/heads`. Otherwise, branches named `heads/foo` (I know it's bad, but it's
allowed), would be fetched as `foo`, instead of `heads/foo`.
@edolstra edolstra merged commit 0748a72 into NixOS:master Jun 2, 2020
@edolstra
Copy link
Member

edolstra commented Jun 2, 2020

Thanks!

@knl knl deleted the improve-ref-validity-checking-in-fetchgit branch June 2, 2020 10:20
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.

flakes: incorrectly rejects valid Git branch/tag names
2 participants