Skip to content

Refactor legacy unknwon/com package, improve golangci lint #19284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Apr 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ linters:
- ineffassign
- revive
- gofumpt
- depguard
enable-all: false
disable-all: true
fast: false
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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."
3 changes: 1 addition & 2 deletions integrations/goget_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
3 changes: 1 addition & 2 deletions models/migrations/migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions modules/cache/cache_redis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
}
Expand Down
6 changes: 3 additions & 3 deletions modules/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}

Expand All @@ -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())
}
Expand Down
14 changes: 10 additions & 4 deletions modules/context/csrf.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note for other reviewers/maintainers that the reduction of this isn't a concern.

Old situation:
The com.RandomCreateBytes actually doesn't give you the full 8 bits of randomness per byte(so 80 bits in the old case), because it reduces itself to only use the alphanum: https://github.com/unknwon/com/blob/b41c64acd94be7e673c9c8301344d31cce99e06c/string.go#L130 which only gives 6 bits per byte of randomness so it would only receive 48 bits of randomness.

New situation:
With CryptoRandomBytes the full 64 bits would be random.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And before, IIRC the com.RandomCreateBytes uses a math PRNG ..... not crypto-safe.

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"
Expand Down Expand Up @@ -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
Expand Down
10 changes: 4 additions & 6 deletions modules/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package context
import (
"context"
"fmt"
"html"
"io"
"net/http"
"net/url"
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion modules/json/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ package json
import (
"bytes"
"encoding/binary"
"encoding/json"
"encoding/json" //nolint:depguard
"io"

jsoniter "github.com/json-iterator/go"
Expand Down
11 changes: 9 additions & 2 deletions modules/markup/html.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
10 changes: 7 additions & 3 deletions modules/repository/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
}

Expand Down
3 changes: 1 addition & 2 deletions modules/setting/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}
Expand Down
10 changes: 4 additions & 6 deletions modules/sync/unique_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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))
}
Loading