Skip to content

Commit

Permalink
Prepend refs/heads/ to issue template refs (go-gitea#20461)
Browse files Browse the repository at this point in the history
Fix go-gitea#20456

At some point during the 1.17 cycle abbreviated refishs to issue
branches started breaking. This is likely due serious inconsistencies in
our management of refs throughout Gitea - which is a bug needing to be
addressed in a different PR. (Likely more than one)

We should try to use non-abbreviated `fullref`s as much as possible.
That is where a user has inputted a abbreviated `refish` we should add
`refs/heads/` if it is `branch` etc. I know people keep writing and
merging PRs that remove prefixes from stored content but it is just
wrong and it keeps causing problems like this. We should only remove the
prefix at the time of
presentation as the prefix is the only way of knowing umambiguously and
permanently if the `ref` is referring to a `branch`, `tag` or `commit` /
`SHA`. We need to make it so that every ref has the appropriate prefix,
and probably also need to come up with some definitely unambiguous way
of storing `SHA`s if they're used in a `ref` or `refish` field. We must
not store a potentially
ambiguous `refish` as a `ref`. (Especially when referring a `tag` -
there is no reason why users cannot create a `branch` with the same
short name as a `tag` and vice versa and any attempt to prevent this
will fail. You can even create a `branch` and a
`tag` that matches the `SHA` pattern.)

To that end in order to fix this bug, when parsing issue templates check
the provided `Ref` (here a `refish` because almost all users do not know
or understand the subtly), if it does not start with `refs/` add the
`BranchPrefix` to it. This allows people to make their templates refer
to a `tag` but not to a `SHA` directly. (I don't think that is
particularly unreasonable but if people disagree I can make the `refish`
be checked to see if it matches the `SHA` pattern.)

Next we need to handle the issue links that are already written. The
links here are created with `git.RefURL`

Here we see there is a bug introduced in go-gitea#17551 whereby the provided
`ref` argument can be double-escaped so we remove the incorrect external
escape. (The escape added in go-gitea#17551 is in the right place -
unfortunately I missed that the calling function was doing the wrong
thing.)

Then within `RefURL()` we check if an unprefixed `ref` (therefore
potentially a `refish`) matches the `SHA` pattern before assuming that
is actually a `commit` - otherwise is assumed to be a `branch`. This
will handle most of the problem cases excepting the very unusual cases
where someone has deliberately written a `branch` to look like a `SHA1`.

But please if something is called a `ref` or interpreted as a `ref` make
it a full-ref before storing or using it. By all means if something is a
`branch` assume the prefix is removed but always add it back in if you
are using it as a `ref`. Stop storing abbreviated `branch` names and
`tag` names - which are `refish` as a `ref`. It will keep on causing
problems like this.

Fix go-gitea#20456

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
  • Loading branch information
4 people authored and fsologureng committed Nov 22, 2022
1 parent 4a5afa1 commit e0467a7
Showing 4 changed files with 11 additions and 2 deletions.
3 changes: 3 additions & 0 deletions modules/context/repo.go
Original file line number Diff line number Diff line change
@@ -1089,6 +1089,9 @@ func (ctx *Context) IssueTemplatesErrorsFromDefaultBranch() ([]*api.IssueTemplat
if it, err := template.UnmarshalFromEntry(entry, dirName); err != nil {
invalidFiles[fullName] = err
} else {
if !strings.HasPrefix(it.Ref, "refs/") { // Assume that the ref intended is always a branch - for tags users should use refs/tags/<ref>
it.Ref = git.BranchPrefix + it.Ref
}
issueTemplates = append(issueTemplates, it)
}
}
3 changes: 3 additions & 0 deletions modules/git/utils.go
Original file line number Diff line number Diff line change
@@ -100,6 +100,9 @@ func RefURL(repoURL, ref string) string {
return repoURL + "/src/branch/" + refName
case strings.HasPrefix(ref, TagPrefix):
return repoURL + "/src/tag/" + refName
case !IsValidSHAPattern(ref):
// assume they mean a branch
return repoURL + "/src/branch/" + refName
default:
return repoURL + "/src/commit/" + refName
}
4 changes: 4 additions & 0 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
@@ -784,6 +784,10 @@ func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles
}
}
}

}
if !strings.HasPrefix(template.Ref, "refs/") { // Assume that the ref intended is always a branch - for tags users should use refs/tags/<ref>
template.Ref = git.BranchPrefix + template.Ref
}
ctx.Data["HasSelectedLabel"] = len(labelIDs) > 0
ctx.Data["label_ids"] = strings.Join(labelIDs, ",")
3 changes: 1 addition & 2 deletions services/issue/issue.go
Original file line number Diff line number Diff line change
@@ -18,7 +18,6 @@ import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/notification"
"code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/util"
)

// NewIssue creates new issue with labels for repository.
@@ -201,7 +200,7 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i
for _, issue := range issues {
if issue.Ref != "" {
issueRefEndNames[issue.ID] = git.RefEndName(issue.Ref)
issueRefURLs[issue.ID] = git.RefURL(repoLink, util.PathEscapeSegments(issue.Ref))
issueRefURLs[issue.ID] = git.RefURL(repoLink, issue.Ref)
}
}
return issueRefEndNames, issueRefURLs

0 comments on commit e0467a7

Please sign in to comment.