Remove most calls to com, improve vars.Expand

Author: wxiaoguang

.golangci.yml

@@ -18,6 +18,7 @@ linters:
     - ineffassign
     - revive
     - gofumpt
+    - depguard
   enable-all: false
   disable-all: true
   fast: false
@@ -64,7 +65,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:
@@ -152,5 +161,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."

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"
 )
@@ -452,7 +452,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
 }
 
@@ -466,7 +466,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())
 	}

modules/context/csrf.go

@@ -19,14 +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.
@@ -163,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"

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"

modules/markup/html.go

@@ -840,8 +840,8 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) {
 
 			res, err := vars.Expand(ctx.Metas["format"], ctx.Metas)
 			if err != nil {
-				log.Error(err.Error())
-				return
+				// here we could just log the error and continue the rendering
+				log.Error("unable to expand template vars for ref %s, err:%s", ref.Issue, err.Error())
 			}
 
 			link = createLink(res, reftext, "ref-issue ref-external-issue")

modules/repository/init.go

@@ -62,7 +62,8 @@ func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir,
 	}
 	res, err := vars.Expand(string(data), match)
 	if err != nil {
-		return fmt.Errorf("expand README.md: %v", err)
+		// here we could just log the error and continue the rendering
+		log.Error("unable to expand template vars for repo README: %s, err: %s", opts.Readme, err.Error())
 	}
 	if err = os.WriteFile(filepath.Join(tmpDir, "README.md"),
 		[]byte(res), 0o644); err != nil {

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"
 )
@@ -601,7 +600,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)
 	}

modules/templates/vars/vars.go

@@ -40,67 +40,65 @@ func IsErrNoMatchedVar(err error) bool {
 	return ok
 }
 
-// Expand replaces all variables like {var} to match
-func Expand(template string, match map[string]string, subs ...string) (string, error) {
-	var (
-		buf   strings.Builder
-		key   strings.Builder
-		enter bool
-	)
-	for _, c := range template {
-		switch {
-		case c == '{':
-			if enter {
-				return "", ErrWrongSyntax{
-					Template: template,
-				}
-			}
-			enter = true
-		case c == '}':
-			if !enter {
-				return "", ErrWrongSyntax{
-					Template: template,
-				}
-			}
-			if key.Len() == 0 {
-				return "", ErrWrongSyntax{
-					Template: template,
-				}
-			}
+// Expand replaces all variables like {var} to match, if error occurs, the error part doesn't change and is returned as it is.
+// `#' is a reversed char, templates can use `{#{}` to do escape and output char '{'.
+func Expand(template string, match map[string]string) (string, error) {
+	var buf strings.Builder
+	var err error
 
-			if len(match) == 0 {
-				return "", ErrNoMatchedVar{
-					Template: template,
-					Var:      key.String(),
-				}
-			}
+	posBegin := 0
+	strLen := len(template)
+	for posBegin < strLen {
+		// find the next `{`
+		pos := strings.IndexByte(template[posBegin:], '{')
+		if pos == -1 {
+			buf.WriteString(template[posBegin:])
+			break
+		}
 
-			v, ok := match[key.String()]
-			if !ok {
-				if len(subs) == 0 {
-					return "", ErrNoMatchedVar{
-						Template: template,
-						Var:      key.String(),
-					}
-				}
-				v = subs[0]
-			}
+		// copy texts between vars
+		buf.WriteString(template[posBegin : posBegin+pos])
 
-			if _, err := buf.WriteString(v); err != nil {
-				return "", err
+		// find the var between `{` and `}`/end
+		posBegin += pos
+		posEnd := posBegin + 1
+		for posEnd < strLen {
+			if template[posEnd] == '#' {
+				// escape char, skip next
+				posEnd += 2
+				continue
+			} else if template[posEnd] == '}' {
+				posEnd++
+				break
 			}
-			key.Reset()
+			posEnd++
+		}
 
-			enter = false
-		case enter:
-			if _, err := key.WriteRune(c); err != nil {
-				return "", err
-			}
-		default:
-			if _, err := buf.WriteRune(c); err != nil {
-				return "", err
+		// the var part, it can be "{", "{}", "{..." or or "{...}"
+		part := template[posBegin:posEnd]
+		posBegin = posEnd
+		if part == "{}" || part[len(part)-1] != '}' {
+			// treat "{}" or "{..." as error
+			err = ErrWrongSyntax{Template: template}
+			buf.WriteString(part)
+		} else {
+			// now we get a valid key "{...}"
+			key := part[1 : len(part)-1]
+			if key[0] == '#' {
+				// escaped char
+				buf.WriteString(key[1:])
+			} else {
+				// look up in the map
+				if val, ok := match[key]; ok {
+					buf.WriteString(val)
+				} else {
+					// write the non-existing var as it is
+					buf.WriteString(part)
+					err = ErrNoMatchedVar{Template: template, Var: key}
+				}
 			}
 		}
 	}
-	return buf.String(), nil
+
+	return buf.String(), err
 }

modules/templates/vars/vars_test.go

@@ -25,13 +25,13 @@ func TestExpandVars(t *testing.T) {
 			expected: "1",
 		},
 		{
-			template: "expand {a}, {b} and {c}",
+			template: "expand {a}, {b} and {c}, with escaped {#{}",
 			maps: map[string]string{
 				"a": "1",
 				"b": "2",
 				"c": "3",
 			},
-			expected: "expand 1, 2 and 3",
+			expected: "expand 1, 2 and 3, with escaped {",
 		},
 		{
 			template: "中文内容 {一}, {二} 和 {三} 中文结尾",
@@ -44,14 +44,16 @@ func TestExpandVars(t *testing.T) {
 		},
 		{
 			template: "expand {{a}, {b} and {c}",
+			maps: map[string]string{
+				"a": "foo",
+				"b": "bar",
+			},
+			expected: "expand {{a}, bar and {c}",
 			fail:     true,
 		},
 		{
-			template: "expand {}, {b} and {c}",
-			fail:     true,
-		},
-		{
-			template: "expand }, {b} and {c}",
+			template: "expand } {} and {",
+			expected: "expand } {} and {",
 			fail:     true,
 		},
 	}

modules/util/copy.go

@@ -5,7 +5,7 @@
 package util
 
 import (
-	"github.com/unknwon/com"
+	"github.com/unknwon/com" //nolint:depguard
 )
 
 // CopyFile copies file from source to target path.

modules/util/path.go

@@ -154,6 +154,10 @@ func StatDir(rootPath string, includeDir ...bool) ([]string, error) {
 	return statDir(rootPath, "", isIncludeDir, false, false)
 }
 
+func isOSWindows() bool {
+	return runtime.GOOS == "windows"
+}
+
 // FileURLToPath extracts the path information from a file://... url.
 func FileURLToPath(u *url.URL) (string, error) {
 	if u.Scheme != "file" {
@@ -162,7 +166,7 @@ func FileURLToPath(u *url.URL) (string, error) {
 
 	path := u.Path
 
-	if runtime.GOOS != "windows" {
+	if !isOSWindows() {
 		return path, nil
 	}
 
@@ -173,3 +177,22 @@ func FileURLToPath(u *url.URL) (string, error) {
 	}
 	return path, nil
 }
+
+// HomeDir returns path of '~'(in Linux) on Windows,
+// it returns error when the variable does not exist.
+func HomeDir() (home string, err error) {
+	if isOSWindows() {
+		home = os.Getenv("USERPROFILE")
+		if home == "" {
+			home = os.Getenv("HOMEDRIVE") + os.Getenv("HOMEPATH")
+		}
+	} else {
+		home = os.Getenv("HOME")
+	}
+
+	if home == "" {
+		return "", errors.New("cannot get home directory")
+	}
+
+	return home, nil
+}

modules/util/util.go

@@ -12,7 +12,7 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/unknwon/com"
+	"github.com/unknwon/com" //nolint:depguard
 )
 
 // OptionalBool a boolean that can be "null"
@@ -184,7 +184,22 @@ func ToUpperASCII(s string) string {
 	return string(b)
 }
 
-// ToStr should be replaced
+// ToStr (from unknwon/com): should be replaced.
 func ToStr(value interface{}, args ...int) string {
 	return com.ToStr(value, args...)
 }
+
+// ToSnakeCase (from unknwon/com): should be replaced.
+func ToSnakeCase(str string) string {
+	return com.ToSnakeCase(str)
+}
+
+// AESGCMEncrypt (from unknwon/com): encrypts plaintext with the given key using AES in GCM mode. should be replaced.
+func AESGCMEncrypt(key, plaintext []byte) ([]byte, error) {
+	return com.AESGCMEncrypt(key, plaintext)
+}
+
+// AESGCMDecrypt (from unknwon/com): decrypts ciphertext with the given key using AES in GCM mode. should be replaced.
+func AESGCMDecrypt(key, ciphertext []byte) ([]byte, error) {
+	return com.AESGCMDecrypt(key, ciphertext)
+}