fix

models/actions/run.go

@@ -88,7 +88,7 @@ func (run *ActionRun) RefLink() string {
 	if refName.IsPull() {
 		return run.Repo.Link() + "/pulls/" + refName.ShortName()
 	}
-	return git.RefURL(run.Repo.Link(), run.Ref)
+	return run.Repo.Link() + "/src/" + refName.RefWebLinkPath()
 }
 
 // PrettyRef return #id for pull ref or ShortName for others

models/activities/action.go

@@ -355,7 +355,7 @@ func (a *Action) GetBranch() string {
 
 // GetRefLink returns the action's ref link.
 func (a *Action) GetRefLink(ctx context.Context) string {
-	return git.RefURL(a.GetRepoLink(ctx), a.RefName)
+	return a.GetRepoLink(ctx) + "/src/" + git.RefName(a.RefName).RefWebLinkPath()
 }
 
 // GetTag returns the action's repository tag.

modules/git/commit.go

@@ -476,8 +476,12 @@ func (c *Commit) GetRepositoryDefaultPublicGPGKey(forceUpdate bool) (*GPGSetting
 }
 
 func IsStringLikelyCommitID(objFmt ObjectFormat, s string, minLength ...int) bool {
-	minLen := util.OptionalArg(minLength, objFmt.FullLength())
-	if len(s) < minLen || len(s) > objFmt.FullLength() {
+	maxLen := 64 // sha256
+	if objFmt != nil {
+		maxLen = objFmt.FullLength()
+	}
+	minLen := util.OptionalArg(minLength, maxLen)
+	if len(s) < minLen || len(s) > maxLen {
 		return false
 	}
 	for _, c := range s {

modules/git/ref.go

@@ -185,32 +185,38 @@ func (ref RefName) RefGroup() string {
 	return ""
 }
 
+// RefType is a simple ref type of the reference, it is used for UI and webhooks
+type RefType string
+
+const (
+	RefTypeBranch RefType = "branch"
+	RefTypeTag    RefType = "tag"
+	RefTypeCommit RefType = "commit"
+)
+
 // RefType returns the simple ref type of the reference, e.g. branch, tag
 // It's different from RefGroup, which is using the name of the directory under .git/refs
-// Here we using branch but not heads, using tag but not tags
-func (ref RefName) RefType() string {
-	var refType string
-	if ref.IsBranch() {
-		refType = "branch"
-	} else if ref.IsTag() {
-		refType = "tag"
+func (ref RefName) RefType() RefType {
+	switch {
+	case ref.IsBranch():
+		return RefTypeBranch
+	case ref.IsTag():
+		return RefTypeTag
+	case IsStringLikelyCommitID(nil, string(ref), 6):
+		return RefTypeCommit
 	}
-	return refType
+	return ""
 }
 
-// RefURL returns the absolute URL for a ref in a repository
-func RefURL(repoURL, ref string) string {
-	refFullName := RefName(ref)
-	refName := util.PathEscapeSegments(refFullName.ShortName())
-	switch {
-	case refFullName.IsBranch():
-		return repoURL + "/src/branch/" + refName
-	case refFullName.IsTag():
-		return repoURL + "/src/tag/" + refName
-	case !Sha1ObjectFormat.IsValid(ref):
-		// assume they mean a branch
-		return repoURL + "/src/branch/" + refName
-	default:
-		return repoURL + "/src/commit/" + refName
+// RefWebLinkPath returns a path for the reference that can be used in a web link:
+// * "branch/<branch_name>"
+// * "tag/<tag_name>"
+// * "commit/<commit_id>"
+// It returns an empty string if the reference is not a branch, tag or commit.
+func (ref RefName) RefWebLinkPath() string {
+	refType := ref.RefType()
+	if refType == "" {
+		return ""
 	}
+	return string(refType) + "/" + util.PathEscapeSegments(ref.ShortName())
 }

modules/git/ref_test.go

@@ -32,9 +32,8 @@ func TestRefName(t *testing.T) {
 	assert.Equal(t, "c0ffee", RefName("c0ffee").ShortName())
 }
 
-func TestRefURL(t *testing.T) {
-	repoURL := "/user/repo"
-	assert.Equal(t, repoURL+"/src/branch/foo", RefURL(repoURL, "refs/heads/foo"))
-	assert.Equal(t, repoURL+"/src/tag/foo", RefURL(repoURL, "refs/tags/foo"))
-	assert.Equal(t, repoURL+"/src/commit/c0ffee", RefURL(repoURL, "c0ffee"))
+func TestRefWebLinkPath(t *testing.T) {
+	assert.Equal(t, "branch/foo", RefName("refs/heads/foo").RefWebLinkPath())
+	assert.Equal(t, "tag/foo", RefName("refs/tags/foo").RefWebLinkPath())
+	assert.Equal(t, "commit/c0ffee", RefName("c0ffee").RefWebLinkPath())
 }

routers/web/web.go

@@ -1572,6 +1572,7 @@ func registerRoutes(m *web.Router) {
 		m.Get("/atom/branch/*", context.RepoRefByType(context.RepoRefBranch), feedEnabled, feed.RenderBranchFeed)
 
 		m.Group("/src", func() {
+			m.Get("", func(ctx *context.Context) { ctx.Redirect(ctx.Repo.RepoLink) }) // there is no "{owner}/{repo}/src" page, so redirect to "{owner}/{repo}" to avoid 404
 			m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.Home)
 			m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.Home)
 			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.Home)

services/actions/notifier.go

@@ -563,9 +563,9 @@ func (n *actionsNotifier) CreateRef(ctx context.Context, pusher *user_model.User
 	newNotifyInput(repo, pusher, webhook_module.HookEventCreate).
 		WithRef(refFullName.String()).
 		WithPayload(&api.CreatePayload{
-			Ref:     refFullName.String(),
+			Ref:     refFullName.String(), // HINT: here is inconsistent with the Webhook's payload: webhook uses ShortName
 			Sha:     refID,
-			RefType: refFullName.RefType(),
+			RefType: string(refFullName.RefType()),
 			Repo:    apiRepo,
 			Sender:  apiPusher,
 		}).
@@ -580,8 +580,8 @@ func (n *actionsNotifier) DeleteRef(ctx context.Context, pusher *user_model.User
 
 	newNotifyInput(repo, pusher, webhook_module.HookEventDelete).
 		WithPayload(&api.DeletePayload{
-			Ref:        refFullName.String(),
-			RefType:    refFullName.RefType(),
+			Ref:        refFullName.String(), // HINT: here is inconsistent with the Webhook's payload: webhook uses ShortName
+			RefType:    string(refFullName.RefType()),
 			PusherType: api.PusherTypeUser,
 			Repo:       apiRepo,
 			Sender:     apiPusher,

services/context/repo.go

@@ -210,16 +210,7 @@ func (r *Repository) GetCommitGraphsCount(ctx context.Context, hidePRRefs bool,
 // * "commit/123456"
 // It is usually used to construct a link like ".../src/{{RefTypeNameSubURL}}/{{PathEscapeSegments TreePath}}"
 func (r *Repository) RefTypeNameSubURL() string {
-	switch {
-	case r.IsViewBranch:
-		return "branch/" + util.PathEscapeSegments(r.BranchName)
-	case r.IsViewTag:
-		return "tag/" + util.PathEscapeSegments(r.TagName)
-	case r.IsViewCommit:
-		return "commit/" + util.PathEscapeSegments(r.CommitID)
-	}
-	log.Error("Unknown view type for repo: %v", r)
-	return ""
+	return r.RefFullName.RefWebLinkPath()
 }
 
 // GetEditorconfig returns the .editorconfig definition if found in the

services/feed/notifier.go

@@ -469,7 +469,7 @@ func (a *actionNotifier) NewRelease(ctx context.Context, rel *repo_model.Release
 		Repo:      rel.Repo,
 		IsPrivate: rel.Repo.IsPrivate,
 		Content:   rel.Title,
-		RefName:   rel.TagName, // FIXME: use a full ref name?
+		RefName:   git.RefNameFromTag(rel.TagName).String(), // Other functions in this file all use "refFullName.String()"
 	}); err != nil {
 		log.Error("NotifyWatchers: %v", err)
 	}

services/issue/issue.go

@@ -250,8 +250,9 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i
 	issueRefURLs := make(map[int64]string, len(issues))
 	for _, issue := range issues {
 		if issue.Ref != "" {
-			issueRefEndNames[issue.ID] = git.RefName(issue.Ref).ShortName()
-			issueRefURLs[issue.ID] = git.RefURL(repoLink, issue.Ref)
+			ref := git.RefName(issue.Ref)
+			issueRefEndNames[issue.ID] = ref.ShortName()
+			issueRefURLs[issue.ID] = repoLink + "/src/" + ref.RefWebLinkPath()
 		}
 	}
 	return issueRefEndNames, issueRefURLs

services/webhook/notifier.go

@@ -763,12 +763,10 @@ func (m *webhookNotifier) PullRequestReviewRequest(ctx context.Context, doer *us
 func (m *webhookNotifier) CreateRef(ctx context.Context, pusher *user_model.User, repo *repo_model.Repository, refFullName git.RefName, refID string) {
 	apiPusher := convert.ToUser(ctx, pusher, nil)
 	apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm.AccessModeNone})
-	refName := refFullName.ShortName()
-
 	if err := PrepareWebhooks(ctx, EventSource{Repository: repo}, webhook_module.HookEventCreate, &api.CreatePayload{
-		Ref:     refName, // FIXME: should it be a full ref name?
+		Ref:     refFullName.ShortName(), // FIXME: should it be a full ref name? But it will break the existing webhooks?
 		Sha:     refID,
-		RefType: refFullName.RefType(),
+		RefType: string(refFullName.RefType()),
 		Repo:    apiRepo,
 		Sender:  apiPusher,
 	}); err != nil {
@@ -800,11 +798,9 @@ func (m *webhookNotifier) PullRequestSynchronized(ctx context.Context, doer *use
 func (m *webhookNotifier) DeleteRef(ctx context.Context, pusher *user_model.User, repo *repo_model.Repository, refFullName git.RefName) {
 	apiPusher := convert.ToUser(ctx, pusher, nil)
 	apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm.AccessModeOwner})
-	refName := refFullName.ShortName()
-
 	if err := PrepareWebhooks(ctx, EventSource{Repository: repo}, webhook_module.HookEventDelete, &api.DeletePayload{
-		Ref:        refName, // FIXME: should it be a full ref name?
-		RefType:    refFullName.RefType(),
+		Ref:        refFullName.ShortName(), // FIXME: should it be a full ref name? But it will break the existing webhooks?
+		RefType:    string(refFullName.RefType()),
 		PusherType: api.PusherTypeUser,
 		Repo:       apiRepo,
 		Sender:     apiPusher,

services/webhook/slack.go

@@ -84,9 +84,9 @@ func SlackLinkFormatter(url, text string) string {
 // SlackLinkToRef slack-formatter link to a repo ref
 func SlackLinkToRef(repoURL, ref string) string {
 	// FIXME: SHA1 hardcoded here
-	url := git.RefURL(repoURL, ref)
-	refName := git.RefName(ref).ShortName()
-	return SlackLinkFormatter(url, refName)
+	refName := git.RefName(ref)
+	url := repoURL + "/src/" + refName.RefWebLinkPath()
+	return SlackLinkFormatter(url, refName.ShortName())
 }
 
 // Create implements payloadConvertor Create method