Refactor legacy unknwon/com package, improve golangci lint (go-gite…

…a#19284) The main purpose is to refactor the legacy `unknwon/com` package. 1. Remove most imports of `unknwon/com`, only `util/legacy.go` imports the legacy `unknwon/com` 2. Use golangci's depguard to process denied packages 3. Fix some incorrect values in golangci.yml, eg, the version should be quoted string `"1.18"` 4. Use correctly escaped content for `go-import` and `go-source` meta tags 5. Refactor `com.Expand` to our stable (and the same fast) `vars.Expand`, our `vars.Expand` can still return partially rendered content even if the template is not good (eg: key mistach).
troytop · Apr 1, 2022 · 65f17bf · 65f17bf
1 parent 5b74660
commit 65f17bf
Show file tree

Hide file tree

Showing 22 changed files with 397 additions and 81 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -18,6 +18,7 @@ linters:
     - ineffassign
     - revive
     - gofumpt
+    - depguard
   enable-all: false
   disable-all: true
   fast: false
@@ -65,7 +66,15 @@ linters-settings:
       - name: modifies-value-receiver
   gofumpt:
     extra-rules: true
-    lang-version: 1.18
+    lang-version: "1.18"
+  depguard:
+    # TODO: use depguard to replace import checks in gitea-vet
+    list-type: denylist
+    # Check the list against standard lib.
+    include-go-root: true
+    packages-with-error-message:
+      - encoding/json: "use gitea's modules/json instead of encoding/json"
+      - github.com/unknwon/com: "use gitea's util and replacements"
 
 issues:
   exclude-rules:
@@ -153,5 +162,6 @@ issues:
     - path: models/user/openid.go
       linters:
         - golint
-    - linters: staticcheck
+    - linters:
+        - staticcheck
       text: "strings.Title is deprecated: The rule Title uses for word boundaries does not handle Unicode punctuation properly. Use golang.org/x/text/cases instead."
diff --git a/integrations/goget_test.go b/integrations/goget_test.go
@@ -29,8 +29,7 @@ func TestGoGet(t *testing.T) {
 	<body>
 		go get --insecure %[1]s:%[2]s/blah/glah
 	</body>
-</html>
-`, setting.Domain, setting.HTTPPort, setting.AppURL)
+</html>`, setting.Domain, setting.HTTPPort, setting.AppURL)
 
 	assert.Equal(t, expected, resp.Body.String())
 }
diff --git a/models/migrations/migrations_test.go b/models/migrations/migrations_test.go
@@ -24,7 +24,6 @@ import (
 	"code.gitea.io/gitea/modules/util"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/unknwon/com"
 	"xorm.io/xorm"
 	"xorm.io/xorm/names"
 )
@@ -204,7 +203,7 @@ func prepareTestEnv(t *testing.T, skip int, syncModels ...interface{}) (*xorm.En
 	deferFn := PrintCurrentTest(t, ourSkip)
 	assert.NoError(t, os.RemoveAll(setting.RepoRootPath))
 
-	assert.NoError(t, com.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"),
+	assert.NoError(t, util.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"),
 		setting.RepoRootPath))
 	ownerDirs, err := os.ReadDir(setting.RepoRootPath)
 	if err != nil {

diff --git a/modules/cache/cache_redis.go b/modules/cache/cache_redis.go
@@ -10,10 +10,10 @@ import (
 
 	"code.gitea.io/gitea/modules/graceful"
 	"code.gitea.io/gitea/modules/nosql"
+	"code.gitea.io/gitea/modules/util"
 
 	"gitea.com/go-chi/cache"
 	"github.com/go-redis/redis/v8"
-	"github.com/unknwon/com"
 )
 
 // RedisCacher represents a redis cache adapter implementation.
@@ -29,15 +29,15 @@ type RedisCacher struct {
 func (c *RedisCacher) Put(key string, val interface{}, expire int64) error {
 	key = c.prefix + key
 	if expire == 0 {
-		if err := c.c.Set(graceful.GetManager().HammerContext(), key, com.ToStr(val), 0).Err(); err != nil {
+		if err := c.c.Set(graceful.GetManager().HammerContext(), key, util.ToStr(val), 0).Err(); err != nil {
 			return err
 		}
 	} else {
-		dur, err := time.ParseDuration(com.ToStr(expire) + "s")
+		dur, err := time.ParseDuration(util.ToStr(expire) + "s")
 		if err != nil {
 			return err
 		}
-		if err = c.c.Set(graceful.GetManager().HammerContext(), key, com.ToStr(val), dur).Err(); err != nil {
+		if err = c.c.Set(graceful.GetManager().HammerContext(), key, util.ToStr(val), dur).Err(); err != nil {
 			return err
 		}
 	}

diff --git a/modules/context/context.go b/modules/context/context.go
@@ -31,13 +31,13 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/templates"
 	"code.gitea.io/gitea/modules/translation"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web/middleware"
 	"code.gitea.io/gitea/services/auth"
 
 	"gitea.com/go-chi/cache"
 	"gitea.com/go-chi/session"
 	chi "github.com/go-chi/chi/v5"
-	"github.com/unknwon/com"
 	"github.com/unrolled/render"
 	"golang.org/x/crypto/pbkdf2"
 )
@@ -475,7 +475,7 @@ func (ctx *Context) CookieDecrypt(secret, val string) (string, bool) {
 	}
 
 	key := pbkdf2.Key([]byte(secret), []byte(secret), 1000, 16, sha256.New)
-	text, err = com.AESGCMDecrypt(key, text)
+	text, err = util.AESGCMDecrypt(key, text)
 	return string(text), err == nil
 }
 
@@ -489,7 +489,7 @@ func (ctx *Context) SetSuperSecureCookie(secret, name, value string, expiry int)
 // CookieEncrypt encrypts a given value using the provided secret
 func (ctx *Context) CookieEncrypt(secret, value string) string {
 	key := pbkdf2.Key([]byte(secret), []byte(secret), 1000, 16, sha256.New)
-	text, err := com.AESGCMEncrypt(key, []byte(value))
+	text, err := util.AESGCMEncrypt(key, []byte(value))
 	if err != nil {
 		panic("error encrypting cookie: " + err.Error())
 	}

diff --git a/modules/context/csrf.go b/modules/context/csrf.go
@@ -19,13 +19,14 @@
 package context
 
 import (
+	"encoding/base32"
+	"fmt"
 	"net/http"
 	"time"
 
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web/middleware"
-
-	"github.com/unknwon/com"
 )
 
 // CSRF represents a CSRF service and is used to get the current token and validate a suspect token.
@@ -162,7 +163,12 @@ func prepareOptions(options []CsrfOptions) CsrfOptions {
 
 	// Defaults.
 	if len(opt.Secret) == 0 {
-		opt.Secret = string(com.RandomCreateBytes(10))
+		randBytes, err := util.CryptoRandomBytes(8)
+		if err != nil {
+			// this panic can be handled by the recover() in http handlers
+			panic(fmt.Errorf("failed to generate random bytes: %w", err))
+		}
+		opt.Secret = base32.StdEncoding.EncodeToString(randBytes)
 	}
 	if len(opt.Header) == 0 {
 		opt.Header = "X-CSRFToken"
@@ -211,7 +217,7 @@ func Csrfer(opt CsrfOptions, ctx *Context) CSRF {
 	x.ID = "0"
 	uid := ctx.Session.Get(opt.SessionKey)
 	if uid != nil {
-		x.ID = com.ToStr(uid)
+		x.ID = util.ToStr(uid)
 	}
 
 	needsNew := false

diff --git a/modules/context/repo.go b/modules/context/repo.go
@@ -8,6 +8,7 @@ package context
 import (
 	"context"
 	"fmt"
+	"html"
 	"io"
 	"net/http"
 	"net/url"
@@ -29,7 +30,6 @@ import (
 	asymkey_service "code.gitea.io/gitea/services/asymkey"
 
 	"github.com/editorconfig/editorconfig-core-go/v2"
-	"github.com/unknwon/com"
 )
 
 // IssueTemplateDirCandidates issue templates directory
@@ -308,11 +308,9 @@ func EarlyResponseForGoGetMeta(ctx *Context) {
 		ctx.PlainText(http.StatusBadRequest, "invalid repository path")
 		return
 	}
-	ctx.PlainText(http.StatusOK, com.Expand(`<meta name="go-import" content="{GoGetImport} git {CloneLink}">`,
-		map[string]string{
-			"GoGetImport": ComposeGoGetImport(username, reponame),
-			"CloneLink":   repo_model.ComposeHTTPSCloneURL(username, reponame),
-		}))
+	goImportContent := fmt.Sprintf("%s git %s", ComposeGoGetImport(username, reponame), repo_model.ComposeHTTPSCloneURL(username, reponame))
+	htmlMeta := fmt.Sprintf(`<meta name="go-import" content="%s">`, html.EscapeString(goImportContent))
+	ctx.PlainText(http.StatusOK, htmlMeta)
 }
 
 // RedirectToRepo redirect to a differently-named repository

diff --git a/modules/json/json.go b/modules/json/json.go
@@ -8,7 +8,7 @@ package json
 import (
 	"bytes"
 	"encoding/binary"
-	"encoding/json"
+	"encoding/json" //nolint:depguard
 	"io"
 
 	jsoniter "github.com/json-iterator/go"

diff --git a/modules/markup/html.go b/modules/markup/html.go
@@ -21,9 +21,9 @@ import (
 	"code.gitea.io/gitea/modules/markup/common"
 	"code.gitea.io/gitea/modules/references"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/templates/vars"
 	"code.gitea.io/gitea/modules/util"
 
-	"github.com/unknwon/com"
 	"golang.org/x/net/html"
 	"golang.org/x/net/html/atom"
 	"mvdan.cc/xurls/v2"
@@ -838,7 +838,14 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) {
 		reftext := node.Data[ref.RefLocation.Start:ref.RefLocation.End]
 		if exttrack && !ref.IsPull {
 			ctx.Metas["index"] = ref.Issue
-			link = createLink(com.Expand(ctx.Metas["format"], ctx.Metas), reftext, "ref-issue ref-external-issue")
+
+			res, err := vars.Expand(ctx.Metas["format"], ctx.Metas)
+			if err != nil {
+				// here we could just log the error and continue the rendering
+				log.Error("unable to expand template vars for ref %s, err: %v", ref.Issue, err)
+			}
+
+			link = createLink(res, reftext, "ref-issue ref-external-issue")
 		} else {
 			// Path determines the type of link that will be rendered. It's unknown at this point whether
 			// the linked item is actually a PR or an issue. Luckily it's of no real consequence because

diff --git a/modules/repository/init.go b/modules/repository/init.go
@@ -22,10 +22,9 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/options"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/templates/vars"
 	"code.gitea.io/gitea/modules/util"
 	asymkey_service "code.gitea.io/gitea/services/asymkey"
-
-	"github.com/unknwon/com"
 )
 
 var (
@@ -250,8 +249,13 @@ func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir,
 		"CloneURL.HTTPS": cloneLink.HTTPS,
 		"OwnerName":      repo.OwnerName,
 	}
+	res, err := vars.Expand(string(data), match)
+	if err != nil {
+		// here we could just log the error and continue the rendering
+		log.Error("unable to expand template vars for repo README: %s, err: %v", opts.Readme, err)
+	}
 	if err = os.WriteFile(filepath.Join(tmpDir, "README.md"),
-		[]byte(com.Expand(string(data), match)), 0o644); err != nil {
+		[]byte(res), 0o644); err != nil {
 		return fmt.Errorf("write README.md: %v", err)
 	}
 

diff --git a/modules/setting/setting.go b/modules/setting/setting.go
@@ -27,7 +27,6 @@ import (
 	"code.gitea.io/gitea/modules/user"
 	"code.gitea.io/gitea/modules/util"
 
-	"github.com/unknwon/com"
 	gossh "golang.org/x/crypto/ssh"
 	ini "gopkg.in/ini.v1"
 )
@@ -612,7 +611,7 @@ func loadFromConf(allowEmpty bool, extraConfig string) {
 
 	Cfg.NameMapper = ini.SnackCase
 
-	homeDir, err := com.HomeDir()
+	homeDir, err := util.HomeDir()
 	if err != nil {
 		log.Fatal("Failed to get home directory: %v", err)
 	}

diff --git a/modules/sync/unique_queue.go b/modules/sync/unique_queue.go
@@ -5,9 +5,7 @@
 
 package sync
 
-import (
-	"github.com/unknwon/com"
-)
+import "code.gitea.io/gitea/modules/util"
 
 // UniqueQueue is a queue which guarantees only one instance of same
 // identity is in the line. Instances with same identity will be
@@ -73,13 +71,13 @@ func (q *UniqueQueue) Queue() <-chan string {
 // Exist returns true if there is an instance with given identity
 // exists in the queue.
 func (q *UniqueQueue) Exist(id interface{}) bool {
-	return q.table.IsRunning(com.ToStr(id))
+	return q.table.IsRunning(util.ToStr(id))
 }
 
 // AddFunc adds new instance to the queue with a custom runnable function,
 // the queue is blocked until the function exits.
 func (q *UniqueQueue) AddFunc(id interface{}, fn func()) {
-	idStr := com.ToStr(id)
+	idStr := util.ToStr(id)
 	q.table.lock.Lock()
 	if _, ok := q.table.pool[idStr]; ok {
 		q.table.lock.Unlock()
@@ -105,5 +103,5 @@ func (q *UniqueQueue) Add(id interface{}) {
 
 // Remove removes instance from the queue.
 func (q *UniqueQueue) Remove(id interface{}) {
-	q.table.Stop(com.ToStr(id))
+	q.table.Stop(util.ToStr(id))
 }