Refactor commit signature parser (#30228)

To make it more flexible and support SSH signature.

The existing tests are not changed, there are also tests covering
`parseTagRef` which also calls `parsePayloadSignature` now. Add some new
tests to `Test_parseTagData`

models/user/email_address.go

@@ -256,14 +256,6 @@ func IsEmailUsed(ctx context.Context, email string) (bool, error) {
 	return db.GetEngine(ctx).Where("lower_email=?", strings.ToLower(email)).Get(&EmailAddress{})
 }
 
-// DeleteInactiveEmailAddresses deletes inactive email addresses
-func DeleteInactiveEmailAddresses(ctx context.Context) error {
-	_, err := db.GetEngine(ctx).
-		Where("is_activated = ?", false).
-		Delete(new(EmailAddress))
-	return err
-}
-
 // ActivateEmail activates the email address to given user.
 func ActivateEmail(ctx context.Context, email *EmailAddress) error {
 	ctx, committer, err := db.TxContext(ctx)

modules/git/commit.go

@@ -26,14 +26,14 @@ type Commit struct {
 	Author        *Signature
 	Committer     *Signature
 	CommitMessage string
-	Signature     *CommitGPGSignature
+	Signature     *CommitSignature
 
 	Parents        []ObjectID // ID strings
 	submoduleCache *ObjectCache
 }
 
-// CommitGPGSignature represents a git commit signature part.
-type CommitGPGSignature struct {
+// CommitSignature represents a git commit signature part.
+type CommitSignature struct {
 	Signature string
 	Payload   string // TODO check if can be reconstruct from the rest of commit information to not have duplicate data
 }

modules/git/commit_convert_gogit.go

@@ -13,7 +13,7 @@ import (
 	"github.com/go-git/go-git/v5/plumbing/object"
 )
 
-func convertPGPSignature(c *object.Commit) *CommitGPGSignature {
+func convertPGPSignature(c *object.Commit) *CommitSignature {
 	if c.PGPSignature == "" {
 		return nil
 	}
@@ -57,7 +57,7 @@ func convertPGPSignature(c *object.Commit) *CommitGPGSignature {
 		return nil
 	}
 
-	return &CommitGPGSignature{
+	return &CommitSignature{
 		Signature: c.PGPSignature,
 		Payload:   w.String(),
 	}

modules/git/commit_reader.go

@@ -99,7 +99,7 @@ readLoop:
 		}
 	}
 	commit.CommitMessage = messageSB.String()
-	commit.Signature = &CommitGPGSignature{
+	commit.Signature = &CommitSignature{
 		Signature: signatureSB.String(),
 		Payload:   payloadSB.String(),
 	}

modules/git/repo_tag.go

@@ -185,17 +185,15 @@ func parseTagRef(ref map[string]string) (tag *Tag, err error) {
 
 	tag.Tagger = parseSignatureFromCommitLine(ref["creator"])
 	tag.Message = ref["contents"]
-	// strip PGP signature if present in contents field
-	pgpStart := strings.Index(tag.Message, beginpgp)
-	if pgpStart >= 0 {
-		tag.Message = tag.Message[0:pgpStart]
-	}
+
+	// strip any signature if present in contents field
+	_, tag.Message, _ = parsePayloadSignature(util.UnsafeStringToBytes(tag.Message), 0)
 
 	// annotated tag with GPG signature
 	if tag.Type == "tag" && ref["contents:signature"] != "" {
 		payload := fmt.Sprintf("object %s\ntype commit\ntag %s\ntagger %s\n\n%s\n",
 			tag.Object, tag.Name, ref["creator"], strings.TrimSpace(tag.Message))
-		tag.Signature = &CommitGPGSignature{
+		tag.Signature = &CommitSignature{
 			Signature: ref["contents:signature"],
 			Payload:   payload,
 		}

modules/git/repo_tag_test.go

@@ -315,7 +315,7 @@ qbHDASXl
 				Type:    "tag",
 				Tagger:  parseSignatureFromCommitLine("Foo Bar <foo@bar.com> 1565789218 +0300"),
 				Message: "Add changelog of v1.9.1 (#7859)\n\n* add changelog of v1.9.1\n* Update CHANGELOG.md",
-				Signature: &CommitGPGSignature{
+				Signature: &CommitSignature{
 					Signature: `-----BEGIN PGP SIGNATURE-----
 
 aBCGzBAABCgAdFiEEyWRwv/q1Q6IjSv+D4IPOwzt33PoFAmI8jbIACgkQ4IPOwzt3

modules/git/tag.go

@@ -6,16 +6,10 @@ package git
 import (
 	"bytes"
 	"sort"
-	"strings"
 
 	"code.gitea.io/gitea/modules/util"
 )
 
-const (
-	beginpgp = "\n-----BEGIN PGP SIGNATURE-----\n"
-	endpgp   = "\n-----END PGP SIGNATURE-----"
-)
-
 // Tag represents a Git tag.
 type Tag struct {
 	Name      string
@@ -24,7 +18,7 @@ type Tag struct {
 	Type      string
 	Tagger    *Signature
 	Message   string
-	Signature *CommitGPGSignature
+	Signature *CommitSignature
 }
 
 // Commit return the commit of the tag reference
@@ -32,6 +26,36 @@ func (tag *Tag) Commit(gitRepo *Repository) (*Commit, error) {
 	return gitRepo.getCommit(tag.Object)
 }
 
+func parsePayloadSignature(data []byte, messageStart int) (payload, msg, sign string) {
+	pos := messageStart
+	signStart, signEnd := -1, -1
+	for {
+		eol := bytes.IndexByte(data[pos:], '\n')
+		if eol < 0 {
+			break
+		}
+		line := data[pos : pos+eol]
+		signType, hasPrefix := bytes.CutPrefix(line, []byte("-----BEGIN "))
+		signType, hasSuffix := bytes.CutSuffix(signType, []byte(" SIGNATURE-----"))
+		if hasPrefix && hasSuffix {
+			signEndBytes := append([]byte("\n-----END "), signType...)
+			signEndBytes = append(signEndBytes, []byte(" SIGNATURE-----")...)
+			signEnd = bytes.Index(data[pos:], signEndBytes)
+			if signEnd != -1 {
+				signStart = pos
+				signEnd = pos + signEnd + len(signEndBytes)
+			}
+		}
+		pos += eol + 1
+	}
+
+	if signStart != -1 && signEnd != -1 {
+		msgEnd := max(messageStart, signStart-1)
+		return string(data[:msgEnd]), string(data[messageStart:msgEnd]), string(data[signStart:signEnd])
+	}
+	return string(data), string(data[messageStart:]), ""
+}
+
 // Parse commit information from the (uncompressed) raw
 // data from the commit object.
 // \n\n separate headers from message
@@ -40,47 +64,37 @@ func parseTagData(objectFormat ObjectFormat, data []byte) (*Tag, error) {
 	tag.ID = objectFormat.EmptyObjectID()
 	tag.Object = objectFormat.EmptyObjectID()
 	tag.Tagger = &Signature{}
-	// we now have the contents of the commit object. Let's investigate...
-	nextline := 0
-l:
+
+	pos := 0
 	for {
-		eol := bytes.IndexByte(data[nextline:], '\n')
-		switch {
-		case eol > 0:
-			line := data[nextline : nextline+eol]
-			spacepos := bytes.IndexByte(line, ' ')
-			reftype := line[:spacepos]
-			switch string(reftype) {
-			case "object":
-				id, err := NewIDFromString(string(line[spacepos+1:]))
-				if err != nil {
-					return nil, err
-				}
-				tag.Object = id
-			case "type":
-				// A commit can have one or more parents
-				tag.Type = string(line[spacepos+1:])
-			case "tagger":
-				tag.Tagger = parseSignatureFromCommitLine(util.UnsafeBytesToString(line[spacepos+1:]))
-			}
-			nextline += eol + 1
-		case eol == 0:
-			tag.Message = string(data[nextline+1:])
-			break l
-		default:
-			break l
+		eol := bytes.IndexByte(data[pos:], '\n')
+		if eol == -1 {
+			break // shouldn't happen, but could just tolerate it
 		}
+		if eol == 0 {
+			pos++
+			break // end of headers
+		}
+		line := data[pos : pos+eol]
+		key, val, _ := bytes.Cut(line, []byte(" "))
+		switch string(key) {
+		case "object":
+			id, err := NewIDFromString(string(val))
+			if err != nil {
+				return nil, err
+			}
+			tag.Object = id
+		case "type":
+			tag.Type = string(val) // A commit can have one or more parents
+		case "tagger":
+			tag.Tagger = parseSignatureFromCommitLine(util.UnsafeBytesToString(val))
+		}
+		pos += eol + 1
 	}
-	idx := strings.LastIndex(tag.Message, beginpgp)
-	if idx > 0 {
-		endSigIdx := strings.Index(tag.Message[idx:], endpgp)
-		if endSigIdx > 0 {
-			tag.Signature = &CommitGPGSignature{
-				Signature: tag.Message[idx+1 : idx+endSigIdx+len(endpgp)],
-				Payload:   string(data[:bytes.LastIndex(data, []byte(beginpgp))+1]),
-			}
-			tag.Message = tag.Message[:idx+1]
-		}
+	payload, msg, sign := parsePayloadSignature(data, pos)
+	tag.Message = msg
+	if len(sign) > 0 {
+		tag.Signature = &CommitSignature{Signature: sign, Payload: payload}
 	}
 	return tag, nil
 }

modules/git/tag_test.go

@@ -12,24 +12,28 @@ import (
 
 func Test_parseTagData(t *testing.T) {
 	testData := []struct {
-		data []byte
-		tag  Tag
+		data     string
+		expected Tag
 	}{
-		{data: []byte(`object 3b114ab800c6432ad42387ccf6bc8d4388a2885a
+		{
+			data: `object 3b114ab800c6432ad42387ccf6bc8d4388a2885a
 type commit
 tag 1.22.0
 tagger Lucas Michot <lucas@semalead.com> 1484491741 +0100
 
-`), tag: Tag{
-			Name:      "",
-			ID:        Sha1ObjectFormat.EmptyObjectID(),
-			Object:    &Sha1Hash{0x3b, 0x11, 0x4a, 0xb8, 0x0, 0xc6, 0x43, 0x2a, 0xd4, 0x23, 0x87, 0xcc, 0xf6, 0xbc, 0x8d, 0x43, 0x88, 0xa2, 0x88, 0x5a},
-			Type:      "commit",
-			Tagger:    &Signature{Name: "Lucas Michot", Email: "lucas@semalead.com", When: time.Unix(1484491741, 0)},
-			Message:   "",
-			Signature: nil,
-		}},
-		{data: []byte(`object 7cdf42c0b1cc763ab7e4c33c47a24e27c66bfccc
+`,
+			expected: Tag{
+				Name:      "",
+				ID:        Sha1ObjectFormat.EmptyObjectID(),
+				Object:    MustIDFromString("3b114ab800c6432ad42387ccf6bc8d4388a2885a"),
+				Type:      "commit",
+				Tagger:    &Signature{Name: "Lucas Michot", Email: "lucas@semalead.com", When: time.Unix(1484491741, 0).In(time.FixedZone("", 3600))},
+				Message:   "",
+				Signature: nil,
+			},
+		},
+		{
+			data: `object 7cdf42c0b1cc763ab7e4c33c47a24e27c66bfccc
 type commit
 tag 1.22.1
 tagger Lucas Michot <lucas@semalead.com> 1484553735 +0100
@@ -37,37 +41,57 @@ tagger Lucas Michot <lucas@semalead.com> 1484553735 +0100
 test message
 o
 
-ono`), tag: Tag{
-			Name:      "",
-			ID:        Sha1ObjectFormat.EmptyObjectID(),
-			Object:    &Sha1Hash{0x7c, 0xdf, 0x42, 0xc0, 0xb1, 0xcc, 0x76, 0x3a, 0xb7, 0xe4, 0xc3, 0x3c, 0x47, 0xa2, 0x4e, 0x27, 0xc6, 0x6b, 0xfc, 0xcc},
-			Type:      "commit",
-			Tagger:    &Signature{Name: "Lucas Michot", Email: "lucas@semalead.com", When: time.Unix(1484553735, 0)},
-			Message:   "test message\no\n\nono",
-			Signature: nil,
-		}},
+ono`,
+			expected: Tag{
+				Name:      "",
+				ID:        Sha1ObjectFormat.EmptyObjectID(),
+				Object:    MustIDFromString("7cdf42c0b1cc763ab7e4c33c47a24e27c66bfccc"),
+				Type:      "commit",
+				Tagger:    &Signature{Name: "Lucas Michot", Email: "lucas@semalead.com", When: time.Unix(1484553735, 0).In(time.FixedZone("", 3600))},
+				Message:   "test message\no\n\nono",
+				Signature: nil,
+			},
+		},
+		{
+			data: `object 7cdf42c0b1cc763ab7e4c33c47a24e27c66bfaaa
+type commit
+tag v0
+tagger dummy user <dummy-email@example.com> 1484491741 +0100
+
+dummy message
+-----BEGIN SSH SIGNATURE-----
+dummy signature
+-----END SSH SIGNATURE-----
+`,
+			expected: Tag{
+				Name:    "",
+				ID:      Sha1ObjectFormat.EmptyObjectID(),
+				Object:  MustIDFromString("7cdf42c0b1cc763ab7e4c33c47a24e27c66bfaaa"),
+				Type:    "commit",
+				Tagger:  &Signature{Name: "dummy user", Email: "dummy-email@example.com", When: time.Unix(1484491741, 0).In(time.FixedZone("", 3600))},
+				Message: "dummy message",
+				Signature: &CommitSignature{
+					Signature: `-----BEGIN SSH SIGNATURE-----
+dummy signature
+-----END SSH SIGNATURE-----`,
+					Payload: `object 7cdf42c0b1cc763ab7e4c33c47a24e27c66bfaaa
+type commit
+tag v0
+tagger dummy user <dummy-email@example.com> 1484491741 +0100
+
+dummy message`,
+				},
+			},
+		},
 	}
 
 	for _, test := range testData {
-		tag, err := parseTagData(Sha1ObjectFormat, test.data)
+		tag, err := parseTagData(Sha1ObjectFormat, []byte(test.data))
 		assert.NoError(t, err)
-		assert.EqualValues(t, test.tag.ID, tag.ID)
-		assert.EqualValues(t, test.tag.Object, tag.Object)
-		assert.EqualValues(t, test.tag.Name, tag.Name)
-		assert.EqualValues(t, test.tag.Message, tag.Message)
-		assert.EqualValues(t, test.tag.Type, tag.Type)
-		if test.tag.Signature != nil && assert.NotNil(t, tag.Signature) {
-			assert.EqualValues(t, test.tag.Signature.Signature, tag.Signature.Signature)
-			assert.EqualValues(t, test.tag.Signature.Payload, tag.Signature.Payload)
-		} else {
-			assert.Nil(t, tag.Signature)
-		}
-		if test.tag.Tagger != nil && assert.NotNil(t, tag.Tagger) {
-			assert.EqualValues(t, test.tag.Tagger.Name, tag.Tagger.Name)
-			assert.EqualValues(t, test.tag.Tagger.Email, tag.Tagger.Email)
-			assert.EqualValues(t, test.tag.Tagger.When.Unix(), tag.Tagger.When.Unix())
-		} else {
-			assert.Nil(t, tag.Tagger)
-		}
+		assert.Equal(t, test.expected, *tag)
 	}
+
+	tag, err := parseTagData(Sha1ObjectFormat, []byte("type commit\n\nfoo\n-----BEGIN SSH SIGNATURE-----\ncorrupted..."))
+	assert.NoError(t, err)
+	assert.Equal(t, "foo\n-----BEGIN SSH SIGNATURE-----\ncorrupted...", tag.Message)
 }

options/license/AMD-newlib

@@ -0,0 +1,11 @@
+Copyright 1989, 1990 Advanced Micro Devices, Inc.
+
+This software is the property of Advanced Micro Devices, Inc  (AMD)  which
+specifically  grants the user the right to modify, use and distribute this
+software provided this notice is not removed or altered.  All other rights
+are reserved by AMD.
+
+AMD MAKES NO WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, WITH REGARD TO THIS
+SOFTWARE.  IN NO EVENT SHALL AMD BE LIABLE FOR INCIDENTAL OR CONSEQUENTIAL
+DAMAGES IN CONNECTION WITH OR ARISING FROM THE FURNISHING, PERFORMANCE, OR
+USE OF THIS SOFTWARE.

options/license/OAR

@@ -0,0 +1,12 @@
+COPYRIGHT (c) 1989-2013, 2015.
+On-Line Applications Research Corporation (OAR).
+
+Permission to use, copy, modify, and distribute this software for any
+purpose without fee is hereby granted, provided that this entire notice
+is included in all copies of any software which is or includes a copy
+or modification of this software.
+
+THIS SOFTWARE IS BEING PROVIDED "AS IS", WITHOUT ANY EXPRESS OR IMPLIED
+WARRANTY.  IN PARTICULAR,  THE AUTHOR MAKES NO REPRESENTATION
+OR WARRANTY OF ANY KIND CONCERNING THE MERCHANTABILITY OF THIS
+SOFTWARE OR ITS FITNESS FOR ANY PARTICULAR PURPOSE.

options/license/xzoom

@@ -0,0 +1,12 @@
+Copyright Itai Nahshon 1995, 1996.
+This program is distributed with no warranty.
+
+Source files for this program may be distributed freely.
+Modifications to this file are okay as long as:
+ a. This copyright notice and comment are preserved and
+    left at the top of the file.
+ b. The man page is fixed to reflect the change.
+ c. The author of this change adds his name and change
+    description to the list of changes below.
+Executable files may be distributed with sources, or with
+exact location where the source code can be obtained.

routers/api/v1/repo/repo.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	actions_model "code.gitea.io/gitea/models/actions"
 	activities_model "code.gitea.io/gitea/models/activities"
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/organization"
@@ -31,6 +32,7 @@ import (
 	"code.gitea.io/gitea/modules/validation"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/routers/api/v1/utils"
+	actions_service "code.gitea.io/gitea/services/actions"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/convert"
 	"code.gitea.io/gitea/services/issue"
@@ -1035,6 +1037,9 @@ func updateRepoArchivedState(ctx *context.APIContext, opts api.EditRepoOption) e
 				ctx.Error(http.StatusInternalServerError, "ArchiveRepoState", err)
 				return err
 			}
+			if err := actions_model.CleanRepoScheduleTasks(ctx, repo); err != nil {
+				log.Error("CleanRepoScheduleTasks for archived repo %s/%s: %v", ctx.Repo.Owner.Name, repo.Name, err)
+			}
 			log.Trace("Repository was archived: %s/%s", ctx.Repo.Owner.Name, repo.Name)
 		} else {
 			if err := repo_model.SetArchiveRepoState(ctx, repo, *opts.Archived); err != nil {
@@ -1042,6 +1047,11 @@ func updateRepoArchivedState(ctx *context.APIContext, opts api.EditRepoOption) e
 				ctx.Error(http.StatusInternalServerError, "ArchiveRepoState", err)
 				return err
 			}
+			if ctx.Repo.Repository.UnitEnabled(ctx, unit_model.TypeActions) {
+				if err := actions_service.DetectAndHandleSchedules(ctx, repo); err != nil {
+					log.Error("DetectAndHandleSchedules for un-archived repo %s/%s: %v", ctx.Repo.Owner.Name, repo.Name, err)
+				}
+			}
 			log.Trace("Repository was un-archived: %s/%s", ctx.Repo.Owner.Name, repo.Name)
 		}
 	}

routers/web/repo/blame.go

@@ -16,6 +16,7 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/highlight"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/templates"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/util"
@@ -87,9 +88,16 @@ func RefBlame(ctx *context.Context) {
 
 	ctx.Data["IsBlame"] = true
 
-	ctx.Data["FileSize"] = blob.Size()
+	fileSize := blob.Size()
+	ctx.Data["FileSize"] = fileSize
 	ctx.Data["FileName"] = blob.Name()
 
+	if fileSize >= setting.UI.MaxDisplayFileSize {
+		ctx.Data["IsFileTooLarge"] = true
+		ctx.HTML(http.StatusOK, tplRepoHome)
+		return
+	}
+
 	ctx.Data["NumLines"], err = blob.GetBlobLineCount()
 	ctx.Data["NumLinesSet"] = true
 

routers/web/repo/setting/lfs.go

@@ -287,22 +287,19 @@ func LFSFileGet(ctx *context.Context) {
 
 	st := typesniffer.DetectContentType(buf)
 	ctx.Data["IsTextFile"] = st.IsText()
-	isRepresentableAsText := st.IsRepresentableAsText()
-
-	fileSize := meta.Size
 	ctx.Data["FileSize"] = meta.Size
 	ctx.Data["RawFileLink"] = fmt.Sprintf("%s%s/%s.git/info/lfs/objects/%s/%s", setting.AppURL, url.PathEscape(ctx.Repo.Repository.OwnerName), url.PathEscape(ctx.Repo.Repository.Name), url.PathEscape(meta.Oid), "direct")
 	switch {
-	case isRepresentableAsText:
-		if st.IsSvgImage() {
-			ctx.Data["IsImageFile"] = true
-		}
-
-		if fileSize >= setting.UI.MaxDisplayFileSize {
+	case st.IsRepresentableAsText():
+		if meta.Size >= setting.UI.MaxDisplayFileSize {
 			ctx.Data["IsFileTooLarge"] = true
 			break
 		}
 
+		if st.IsSvgImage() {
+			ctx.Data["IsImageFile"] = true
+		}
+
 		rd := charset.ToUTF8WithFallbackReader(io.MultiReader(bytes.NewReader(buf), dataRc), charset.ConvertOpts{})
 
 		// Building code view blocks with line number on server side.
@@ -338,6 +335,8 @@ func LFSFileGet(ctx *context.Context) {
 		ctx.Data["IsAudioFile"] = true
 	case st.IsImage() && (setting.UI.SVG.Enabled || !st.IsSvgImage()):
 		ctx.Data["IsImageFile"] = true
+	default:
+		// TODO: the logic is not the same as "renderFile" in "view.go"
 	}
 	ctx.HTML(http.StatusOK, tplSettingsLFSFile)
 }

routers/web/repo/setting/setting.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/models"
+	actions_model "code.gitea.io/gitea/models/actions"
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/organization"
 	repo_model "code.gitea.io/gitea/models/repo"
@@ -29,6 +30,7 @@ import (
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/validation"
 	"code.gitea.io/gitea/modules/web"
+	actions_service "code.gitea.io/gitea/services/actions"
 	asymkey_service "code.gitea.io/gitea/services/asymkey"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/forms"
@@ -897,6 +899,10 @@ func SettingsPost(ctx *context.Context) {
 			return
 		}
 
+		if err := actions_model.CleanRepoScheduleTasks(ctx, repo); err != nil {
+			log.Error("CleanRepoScheduleTasks for archived repo %s/%s: %v", ctx.Repo.Owner.Name, repo.Name, err)
+		}
+
 		ctx.Flash.Success(ctx.Tr("repo.settings.archive.success"))
 
 		log.Trace("Repository was archived: %s/%s", ctx.Repo.Owner.Name, repo.Name)
@@ -915,6 +921,12 @@ func SettingsPost(ctx *context.Context) {
 			return
 		}
 
+		if ctx.Repo.Repository.UnitEnabled(ctx, unit_model.TypeActions) {
+			if err := actions_service.DetectAndHandleSchedules(ctx, repo); err != nil {
+				log.Error("DetectAndHandleSchedules for un-archived repo %s/%s: %v", ctx.Repo.Owner.Name, repo.Name, err)
+			}
+		}
+
 		ctx.Flash.Success(ctx.Tr("repo.settings.unarchive.success"))
 
 		log.Trace("Repository was un-archived: %s/%s", ctx.Repo.Owner.Name, repo.Name)

routers/web/repo/view.go

@@ -482,17 +482,17 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) {
 
 	switch {
 	case isRepresentableAsText:
+		if fInfo.fileSize >= setting.UI.MaxDisplayFileSize {
+			ctx.Data["IsFileTooLarge"] = true
+			break
+		}
+
 		if fInfo.st.IsSvgImage() {
 			ctx.Data["IsImageFile"] = true
 			ctx.Data["CanCopyContent"] = true
 			ctx.Data["HasSourceRenderedToggle"] = true
 		}
 
-		if fInfo.fileSize >= setting.UI.MaxDisplayFileSize {
-			ctx.Data["IsFileTooLarge"] = true
-			break
-		}
-
 		rd := charset.ToUTF8WithFallbackReader(io.MultiReader(bytes.NewReader(buf), dataRc), charset.ConvertOpts{})
 
 		shouldRenderSource := ctx.FormString("display") == "source"
@@ -606,6 +606,8 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) {
 			break
 		}
 
+		// TODO: this logic seems strange, it duplicates with "isRepresentableAsText=true", it is not the same as "LFSFileGet" in "lfs.go"
+		// maybe for this case, the file is a binary file, and shouldn't be rendered?
 		if markupType := markup.Type(blob.Name()); markupType != "" {
 			rd := io.MultiReader(bytes.NewReader(buf), dataRc)
 			ctx.Data["IsMarkup"] = true

services/actions/notifier_helper.go

@@ -117,7 +117,7 @@ func notify(ctx context.Context, input *notifyInput) error {
 		log.Debug("ignore executing %v for event %v whose doer is %v", getMethod(ctx), input.Event, input.Doer.Name)
 		return nil
 	}
-	if input.Repo.IsEmpty {
+	if input.Repo.IsEmpty || input.Repo.IsArchived {
 		return nil
 	}
 	if unit_model.TypeActions.UnitGlobalDisabled() {
@@ -501,7 +501,7 @@ func handleSchedules(
 
 // DetectAndHandleSchedules detects the schedule workflows on the default branch and create schedule tasks
 func DetectAndHandleSchedules(ctx context.Context, repo *repo_model.Repository) error {
-	if repo.IsEmpty {
+	if repo.IsEmpty || repo.IsArchived {
 		return nil
 	}
 

services/actions/schedule_tasks.go

@@ -66,6 +66,11 @@ func startTasks(ctx context.Context) error {
 				}
 			}
 
+			if row.Repo.IsArchived {
+				// Skip if the repo is archived
+				continue
+			}
+
 			cfg, err := row.Repo.GetUnit(ctx, unit.TypeActions)
 			if err != nil {
 				if repo_model.IsErrUnitTypeNotExist(err) {

services/user/user.go

@@ -126,7 +126,7 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
 		return fmt.Errorf("%s is an organization not a user", u.Name)
 	}
 
-	if user_model.IsLastAdminUser(ctx, u) {
+	if u.IsActive && user_model.IsLastAdminUser(ctx, u) {
 		return models.ErrDeleteLastAdminUser{UID: u.ID}
 	}
 
@@ -250,7 +250,7 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
 	if err := committer.Commit(); err != nil {
 		return err
 	}
-	committer.Close()
+	_ = committer.Close()
 
 	if err = asymkey_service.RewriteAllPublicKeys(ctx); err != nil {
 		return err
@@ -259,50 +259,45 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
 		return err
 	}
 
-	// Note: There are something just cannot be roll back,
-	//	so just keep error logs of those operations.
+	// Note: There are something just cannot be roll back, so just keep error logs of those operations.
 	path := user_model.UserPath(u.Name)
-	if err := util.RemoveAll(path); err != nil {
-		err = fmt.Errorf("Failed to RemoveAll %s: %w", path, err)
+	if err = util.RemoveAll(path); err != nil {
+		err = fmt.Errorf("failed to RemoveAll %s: %w", path, err)
 		_ = system_model.CreateNotice(ctx, system_model.NoticeTask, fmt.Sprintf("delete user '%s': %v", u.Name, err))
-		return err
 	}
 
 	if u.Avatar != "" {
 		avatarPath := u.CustomAvatarRelativePath()
-		if err := storage.Avatars.Delete(avatarPath); err != nil {
-			err = fmt.Errorf("Failed to remove %s: %w", avatarPath, err)
+		if err = storage.Avatars.Delete(avatarPath); err != nil {
+			err = fmt.Errorf("failed to remove %s: %w", avatarPath, err)
 			_ = system_model.CreateNotice(ctx, system_model.NoticeTask, fmt.Sprintf("delete user '%s': %v", u.Name, err))
-			return err
 		}
 	}
 
 	return nil
 }
 
-// DeleteInactiveUsers deletes all inactive users and email addresses.
+// DeleteInactiveUsers deletes all inactive users and their email addresses.
 func DeleteInactiveUsers(ctx context.Context, olderThan time.Duration) error {
-	users, err := user_model.GetInactiveUsers(ctx, olderThan)
+	inactiveUsers, err := user_model.GetInactiveUsers(ctx, olderThan)
 	if err != nil {
 		return err
 	}
 
 	// FIXME: should only update authorized_keys file once after all deletions.
-	for _, u := range users {
-		select {
-		case <-ctx.Done():
-			return db.ErrCancelledf("Before delete inactive user %s", u.Name)
-		default:
-		}
-		if err := DeleteUser(ctx, u, false); err != nil {
-			// Ignore users that were set inactive by admin.
-			if models.IsErrUserOwnRepos(err) || models.IsErrUserHasOrgs(err) ||
-				models.IsErrUserOwnPackages(err) || models.IsErrDeleteLastAdminUser(err) {
+	for _, u := range inactiveUsers {
+		if err = DeleteUser(ctx, u, false); err != nil {
+			// Ignore inactive users that were ever active but then were set inactive by admin
+			if models.IsErrUserOwnRepos(err) || models.IsErrUserHasOrgs(err) || models.IsErrUserOwnPackages(err) {
 				continue
 			}
-			return err
+			select {
+			case <-ctx.Done():
+				return db.ErrCancelledf("when deleting inactive user %q", u.Name)
+			default:
+				return err
+			}
 		}
 	}
-
-	return user_model.DeleteInactiveEmailAddresses(ctx)
+	return nil // TODO: there could be still inactive users left, and the number would increase gradually
 }

services/user/user_test.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+	"time"
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/models/auth"
@@ -16,6 +17,7 @@ import (
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/timeutil"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -185,3 +187,26 @@ func TestCreateUser_Issue5882(t *testing.T) {
 		assert.NoError(t, DeleteUser(db.DefaultContext, v.user, false))
 	}
 }
+
+func TestDeleteInactiveUsers(t *testing.T) {
+	addUser := func(name, email string, createdUnix timeutil.TimeStamp, active bool) {
+		inactiveUser := &user_model.User{Name: name, LowerName: strings.ToLower(name), Email: email, CreatedUnix: createdUnix, IsActive: active}
+		_, err := db.GetEngine(db.DefaultContext).NoAutoTime().Insert(inactiveUser)
+		assert.NoError(t, err)
+		inactiveUserEmail := &user_model.EmailAddress{UID: inactiveUser.ID, IsPrimary: true, Email: email, LowerEmail: strings.ToLower(email), IsActivated: active}
+		err = db.Insert(db.DefaultContext, inactiveUserEmail)
+		assert.NoError(t, err)
+	}
+	addUser("user-inactive-10", "user-inactive-10@test.com", timeutil.TimeStampNow().Add(-600), false)
+	addUser("user-inactive-5", "user-inactive-5@test.com", timeutil.TimeStampNow().Add(-300), false)
+	addUser("user-active-10", "user-active-10@test.com", timeutil.TimeStampNow().Add(-600), true)
+	addUser("user-active-5", "user-active-5@test.com", timeutil.TimeStampNow().Add(-300), true)
+	unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user-inactive-10"})
+	unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{Email: "user-inactive-10@test.com"})
+	assert.NoError(t, DeleteInactiveUsers(db.DefaultContext, 8*time.Minute))
+	unittest.AssertNotExistsBean(t, &user_model.User{Name: "user-inactive-10"})
+	unittest.AssertNotExistsBean(t, &user_model.EmailAddress{Email: "user-inactive-10@test.com"})
+	unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user-inactive-5"})
+	unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user-active-10"})
+	unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user-active-5"})
+}

templates/repo/blame.tmpl

@@ -30,6 +30,9 @@
 	</h4>
 	<div class="ui attached table unstackable segment">
 		<div class="file-view code-view unicode-escaped">
+			{{if .IsFileTooLarge}}
+				{{template "shared/filetoolarge" dict "RawFileLink" .RawFileLink}}
+			{{else}}
 			<table>
 				<tbody>
 					{{range $row := .BlameRows}}
@@ -75,6 +78,7 @@
 					{{end}}
 				</tbody>
 			</table>
+			{{end}}{{/* end if .IsFileTooLarge */}}
 			<div class="code-line-menu tippy-target">
 				{{if $.Permission.CanRead $.UnitTypeIssues}}
 					<a class="item ref-in-new-issue" role="menuitem" data-url-issue-new="{{.RepoLink}}/issues/new" data-url-param-body-link="{{.Repository.Link}}/src/commit/{{PathEscape .CommitID}}/{{PathEscapeSegments .TreePath}}{{if $.HasSourceRenderedToggle}}?display=source{{end}}" rel="nofollow noindex">{{ctx.Locale.Tr "repo.issues.context.reference_issue"}}</a>

templates/repo/settings/lfs_file.tmpl

@@ -14,7 +14,9 @@
 			<div class="ui attached table unstackable segment">
 				{{template "repo/unicode_escape_prompt" dict "EscapeStatus" .EscapeStatus "root" $}}
 				<div class="file-view{{if .IsMarkup}} markup {{.MarkupType}}{{else if .IsPlainText}} plain-text{{else if .IsTextFile}} code-view{{end}}">
-					{{if .IsMarkup}}
+					{{if .IsFileTooLarge}}
+						{{template "shared/filetoolarge" dict "RawFileLink" .RawFileLink}}
+					{{else if .IsMarkup}}
 						{{if .FileContent}}{{.FileContent | SafeHTML}}{{end}}
 					{{else if .IsPlainText}}
 						<pre>{{if .FileContent}}{{.FileContent | SafeHTML}}{{end}}</pre>
@@ -33,19 +35,15 @@
 							{{else if .IsPDFFile}}
 								<div class="pdf-content is-loading" data-src="{{$.RawFileLink}}" data-fallback-button-text="{{ctx.Locale.Tr "diff.view_file"}}"></div>
 							{{else}}
-								<a href="{{$.RawFileLink}}" rel="nofollow">{{ctx.Locale.Tr "repo.file_view_raw"}}</a>
+								<a href="{{$.RawFileLink}}" rel="nofollow" class="tw-p-4">{{ctx.Locale.Tr "repo.file_view_raw"}}</a>
 							{{end}}
 						</div>
 					{{else if .FileSize}}
 						<table>
 							<tbody>
 								<tr>
-								{{if .IsFileTooLarge}}
-									<td><strong>{{ctx.Locale.Tr "repo.file_too_large"}}</strong></td>
-								{{else}}
 									<td class="lines-num">{{.LineNums}}</td>
 									<td class="lines-code"><pre><code class="{{.HighlightClass}}"><ol>{{.FileContent}}</ol></code></pre></td>
-								{{end}}
 								</tr>
 							</tbody>
 						</table>

templates/repo/view_file.tmpl

@@ -89,7 +89,9 @@
 			{{template "repo/unicode_escape_prompt" dict "EscapeStatus" .EscapeStatus "root" $}}
 		{{end}}
 		<div class="file-view{{if .IsMarkup}} markup {{.MarkupType}}{{else if .IsPlainText}} plain-text{{else if .IsTextSource}} code-view{{end}}">
-			{{if .IsMarkup}}
+			{{if .IsFileTooLarge}}
+				{{template "shared/filetoolarge" dict "RawFileLink" .RawFileLink}}
+			{{else if .IsMarkup}}
 				{{if .FileContent}}{{.FileContent}}{{end}}
 			{{else if .IsPlainText}}
 				<pre>{{if .FileContent}}{{.FileContent}}{{end}}</pre>
@@ -108,19 +110,10 @@
 					{{else if .IsPDFFile}}
 						<div class="pdf-content is-loading" data-src="{{$.RawFileLink}}" data-fallback-button-text="{{ctx.Locale.Tr "repo.diff.view_file"}}"></div>
 					{{else}}
-						<a href="{{$.RawFileLink}}" rel="nofollow">{{ctx.Locale.Tr "repo.file_view_raw"}}</a>
+						<a href="{{$.RawFileLink}}" rel="nofollow" class="tw-p-4">{{ctx.Locale.Tr "repo.file_view_raw"}}</a>
 					{{end}}
 				</div>
 			{{else if .FileSize}}
-				{{if .IsFileTooLarge}}
-				<table>
-					<tbody>
-						<tr>
-							<td><strong>{{ctx.Locale.Tr "repo.file_too_large"}}</strong></td>
-						</tr>
-					</tbody>
-				</table>
-				{{else}}
 				<table>
 					<tbody>
 						{{range $idx, $code := .FileContent}}
@@ -142,7 +135,6 @@
 					<a class="item view_git_blame" role="menuitem" href="{{.Repository.Link}}/blame/commit/{{PathEscape .CommitID}}/{{PathEscapeSegments .TreePath}}">{{ctx.Locale.Tr "repo.view_git_blame"}}</a>
 					<a class="item copy-line-permalink" role="menuitem" data-url="{{.Repository.Link}}/src/commit/{{PathEscape .CommitID}}/{{PathEscapeSegments .TreePath}}{{if $.HasSourceRenderedToggle}}?display=source{{end}}">{{ctx.Locale.Tr "repo.file_copy_permalink"}}</a>
 				</div>
-				{{end}}
 			{{end}}
 		</div>
 	</div>

templates/shared/filetoolarge.tmpl

@@ -0,0 +1,4 @@
+<div class="tw-p-4">
+	{{ctx.Locale.Tr "repo.file_too_large"}}
+	{{if .RawFileLink}}<a href="{{.RawFileLink}}" rel="nofollow">{{ctx.Locale.Tr "repo.file_view_raw"}}</a>{{end}}
+</div>

web_src/js/features/repo-legacy.js

@@ -358,11 +358,11 @@ async function onEditContent(event) {
           input.name = 'files';
           input.type = 'hidden';
           input.value = data.uuid;
-          dropzone.querySelector('.files').insertAdjacentHTML('beforeend', input.outerHTML);
+          dropzone.querySelector('.files').append(input);
         });
         this.on('removedfile', async (file) => {
-          if (disableRemovedfileEvent) return;
           document.getElementById(file.uuid)?.remove();
+          if (disableRemovedfileEvent) return;
           if (dropzone.getAttribute('data-remove-url') && !fileUuidDict[file.uuid].submitted) {
             try {
               await POST(dropzone.getAttribute('data-remove-url'), {data: new URLSearchParams({file: file.uuid})});
@@ -384,6 +384,7 @@ async function onEditContent(event) {
             disableRemovedfileEvent = true;
             dz.removeAllFiles(true);
             dropzone.querySelector('.files').innerHTML = '';
+            for (const el of dropzone.querySelectorAll('.dz-preview')) el.remove();
             fileUuidDict = {};
             disableRemovedfileEvent = false;
 
@@ -392,7 +393,6 @@ async function onEditContent(event) {
               dz.emit('addedfile', attachment);
               dz.emit('thumbnail', attachment, imgSrc);
               dz.emit('complete', attachment);
-              dz.files.push(attachment);
               fileUuidDict[attachment.uuid] = {submitted: true};
               dropzone.querySelector(`img[src='${imgSrc}']`).style.maxWidth = '100%';
               const input = document.createElement('input');
@@ -400,7 +400,10 @@ async function onEditContent(event) {
               input.name = 'files';
               input.type = 'hidden';
               input.value = attachment.uuid;
-              dropzone.querySelector('.files').insertAdjacentHTML('beforeend', input.outerHTML);
+              dropzone.querySelector('.files').append(input);
+            }
+            if (!dropzone.querySelector('.dz-preview')) {
+              dropzone.classList.remove('dz-started');
             }
           } catch (error) {
             console.error(error);
@@ -412,24 +415,24 @@ async function onEditContent(event) {
     return dz;
   };
 
-  const cancelAndReset = (dz) => {
+  const cancelAndReset = (e) => {
+    e.preventDefault();
     showElem(renderContent);
     hideElem(editContentZone);
-    if (dz) {
-      dz.emit('reload');
-    }
+    comboMarkdownEditor.attachedDropzoneInst?.emit('reload');
   };
 
-  const saveAndRefresh = async (dz) => {
+  const saveAndRefresh = async (e) => {
+    e.preventDefault();
     showElem(renderContent);
     hideElem(editContentZone);
-
+    const dropzoneInst = comboMarkdownEditor.attachedDropzoneInst;
     try {
       const params = new URLSearchParams({
         content: comboMarkdownEditor.value(),
         context: editContentZone.getAttribute('data-context'),
       });
-      for (const file of dz.files) params.append('files[]', file.uuid);
+      for (const fileInput of dropzoneInst?.element.querySelectorAll('.files [name=files]')) params.append('files[]', fileInput.value);
 
       const response = await POST(editContentZone.getAttribute('data-update-url'), {data: params});
       const data = await response.json();
@@ -452,10 +455,8 @@ async function onEditContent(event) {
       } else {
         content.querySelector('.dropzone-attachments').outerHTML = data.attachments;
       }
-      if (dz) {
-        dz.emit('submit');
-        dz.emit('reload');
-      }
+      dropzoneInst?.emit('submit');
+      dropzoneInst?.emit('reload');
       initMarkupContent();
       initCommentContent();
     } catch (error) {
@@ -463,22 +464,13 @@ async function onEditContent(event) {
     }
   };
 
-  if (!editContentZone.innerHTML) {
+  comboMarkdownEditor = getComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor'));
+  if (!comboMarkdownEditor) {
     editContentZone.innerHTML = document.getElementById('issue-comment-editor-template').innerHTML;
     comboMarkdownEditor = await initComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor'));
-
-    const dropzone = editContentZone.querySelector('.dropzone');
-    const dz = await setupDropzone(dropzone);
-    editContentZone.querySelector('.cancel.button').addEventListener('click', (e) => {
-      e.preventDefault();
-      cancelAndReset(dz);
-    });
-    editContentZone.querySelector('.save.button').addEventListener('click', (e) => {
-      e.preventDefault();
-      saveAndRefresh(dz);
-    });
-  } else {
-    comboMarkdownEditor = getComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor'));
+    comboMarkdownEditor.attachedDropzoneInst = await setupDropzone(editContentZone.querySelector('.dropzone'));
+    editContentZone.querySelector('.cancel.button').addEventListener('click', cancelAndReset);
+    editContentZone.querySelector('.save.button').addEventListener('click', saveAndRefresh);
   }
 
   // Show write/preview tab and copy raw content as needed