commit f8630348b4c14215d87921962a2acbe8d3b6c981 parent 38d73f031640302011f4be1933998681fc70b193 Author: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Mon, 22 Nov 2021 08:46:19 +0100 Enable stricter linting with golangci-lint (#316) * update golangci-lint * add golangci config file w/ more linters * correct issues flagged by stricter linters * add more generous timeout for golangci-lint * add some style + formatting guidelines * move timeout to config file * go fmt Diffstat:
47 files changed, 222 insertions(+), 158 deletions(-)
diff --git a/.drone.yml b/.drone.yml @@ -13,7 +13,7 @@ steps: # We use golangci-lint for linting. # See: https://golangci-lint.run/ - name: lint - image: golangci/golangci-lint:v1.42.1 + image: golangci/golangci-lint:v1.43.0 volumes: - name: go-build-cache path: /root/.cache/go-build @@ -22,7 +22,7 @@ steps: - name: go-src path: /go commands: - - golangci-lint run --timeout 5m0s --tests=false --verbose + - golangci-lint run when: event: include: @@ -115,6 +115,6 @@ trigger: --- kind: signature -hmac: aa44c4655421fb0ed3141b8d7255a9a240dd4312244081d9e95929c4a196fd92 +hmac: 07d6ed18510f9591c6b347d6768cbe8e613561b3759f1a8dda8721d1d231a522 ... diff --git a/.golangci.yml b/.golangci.yml @@ -0,0 +1,26 @@ +# Configuration file for golangci-lint linter. +# This will be automatically picked up when golangci-lint is invoked. +# For all config options, see https://golangci-lint.run/usage/configuration/#config-file +# +# For GoToSocial we mostly take the default linters, but we add a few to catch style issues as well. + +# options for analysis running +run: + # include test files or not, default is true + tests: false + # timeout for analysis, e.g. 30s, 5m, default is 1m + timeout: 5m + +linters: + # enable some extra linters, see here for the list: https://golangci-lint.run/usage/linters/ + enable: + - contextcheck + - forcetypeassert + - goconst + - gocritic + - gofmt + - gosec + - ifshort + - nilerr + - revive + - wastedassign diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md @@ -21,7 +21,9 @@ Check the [issues](https://github.com/superseriousbusiness/gotosocial/issues) to - [SQLite](#sqlite) - [Postgres](#postgres) - [Both](#both) -- [Linting](#linting) +- [Project Structure](#project-structure) +- [Style](#style) +- [Linting and Formatting](#linting-and-formatting) - [Updating Swagger docs](#updating-swagger-docs) - [CI/CD configuration](#cicd-configuration) - [Building releases and Docker containers](#building-releases-and-docker-containers) @@ -179,33 +181,48 @@ Finally, to run tests against both database types one after the other, use: ./scripts/test.sh ``` -## Linting +## Project Structure -We use [golangci-lint](https://golangci-lint.run/) for linting. To run this locally, first install the linter following the instructions [here](https://golangci-lint.run/usage/install/#local-installation). +For project structure, GoToSocial follows a standard and widely-accepted project layout [defined here](https://github.com/golang-standards/project-layout). As the author writes: -Then, you can run the linter with: +> This is a basic layout for Go application projects. It's not an official standard defined by the core Go dev team; however, it is a set of common historical and emerging project layout patterns in the Go ecosystem. -```bash -golangci-lint run --tests=false -``` +Most of the crucial business logic of the application is found inside the various packages and subpackages of the `internal` directory. -Note that this linter also runs as a job on the Github repo, so if you make a PR that doesn't pass the linter, it will be rejected. As such, it's good practice to run the linter locally before pushing or opening a PR. +Where possible, we prefer more files and packages of shorter length that very clearly pertain to definable chunks of application logic, rather than fewer but longer files: if one `.go` file is pushing 1,000 lines of code, it's probably too long. -Another useful linter is [golint](https://pkg.go.dev/github.com/360EntSecGroup-Skylar/goreporter/linters/golint), which catches some style issues that golangci-lint does not. +## Style -To install golint, run: +It is a good idea to read the short official [Effective Go](https://golang.org/doc/effective_go) page before submitting code: this document is the foundation of many a style guide, for good reason, and GoToSocial more or less follows its advice. -```bash -go get -u github.com/golang/lint/golint -``` +Another useful style guide that we try to follow: [this one](https://github.com/bahlo/go-styleguide). + +In addition, here are some specific highlights from Uber's Go style guide which agree with what we try to do in GtS: + +- [Group Similar Declarations](https://github.com/uber-go/guide/blob/master/style.md#group-similar-declarations). +- [Reduce Nesting](https://github.com/uber-go/guide/blob/master/style.md#reduce-nesting). +- [Unnecessary Else](https://github.com/uber-go/guide/blob/master/style.md#unnecessary-else). +- [Local Variable Declarations](https://github.com/uber-go/guide/blob/master/style.md#local-variable-declarations). +- [Reduce Scope of Variables](https://github.com/uber-go/guide/blob/master/style.md#reduce-scope-of-variables). +- [Initializing Structs](https://github.com/uber-go/guide/blob/master/style.md#initializing-structs). -To run the linter, use: +## Linting and Formatting + +Before you submit any code, make sure to run `go fmt ./...` to update whitespace and other opinionated formatting. + +We use [golangci-lint](https://golangci-lint.run/) for linting, which allows us to catch style inconsistencies and potential bugs or security issues, using static code analysis. + +If you make a PR that doesn't pass the linter, it will be rejected. As such, it's good practice to run the linter locally before pushing or opening a PR. + +To do this, first install the linter following the instructions [here](https://golangci-lint.run/usage/install/#local-installation). + +Then, you can run the linter with: ```bash -golint ./internal/... +golangci-lint run ``` -Then make sure to run `go fmt ./...` to update whitespace and other opinionated formatting. +If there's no output, great! It passed :) ## Updating Swagger docs diff --git a/cmd/gotosocial/main.go b/cmd/gotosocial/main.go @@ -49,8 +49,7 @@ func main() { Commands: getCommands(), } - err := app.Run(os.Args) - if err != nil { + if err := app.Run(os.Args); err != nil { logrus.Fatal(err) } } diff --git a/internal/ap/activitystreams.go b/internal/ap/activitystreams.go @@ -67,6 +67,6 @@ const ( ObjectRelationship = "Relationship" // ActivityStreamsRelationship https://www.w3.org/TR/activitystreams-vocabulary/#dfn-relationship ObjectTombstone = "Tombstone" // ActivityStreamsTombstone https://www.w3.org/TR/activitystreams-vocabulary/#dfn-tombstone ObjectVideo = "Video" // ActivityStreamsVideo https://www.w3.org/TR/activitystreams-vocabulary/#dfn-video - ObjectCollection = "Collection" //ActivityStreamsCollection https://www.w3.org/TR/activitystreams-vocabulary/#dfn-collection + ObjectCollection = "Collection" // ActivityStreamsCollection https://www.w3.org/TR/activitystreams-vocabulary/#dfn-collection ObjectCollectionPage = "CollectionPage" // ActivityStreamsCollectionPage https://www.w3.org/TR/activitystreams-vocabulary/#dfn-collectionpage ) diff --git a/internal/api/client/app/appcreate.go b/internal/api/client/app/appcreate.go @@ -20,14 +20,22 @@ package app import ( "fmt" - "github.com/sirupsen/logrus" "net/http" + "github.com/sirupsen/logrus" + "github.com/gin-gonic/gin" "github.com/superseriousbusiness/gotosocial/internal/api/model" "github.com/superseriousbusiness/gotosocial/internal/oauth" ) +const ( + // permitted length for most fields + formFieldLen = 64 + // redirect can be a bit bigger because we probably need to encode data in the redirect uri + formRedirectLen = 512 +) + // AppsPOSTHandler swagger:operation POST /api/v1/apps appCreate // // Register a new application on this instance. @@ -79,11 +87,6 @@ func (m *Module) AppsPOSTHandler(c *gin.Context) { return } - // permitted length for most fields - formFieldLen := 64 - // redirect can be a bit bigger because we probably need to encode data in the redirect uri - formRedirectLen := 512 - // check lengths of fields before proceeding so the user can't spam huge entries into the database if len(form.ClientName) > formFieldLen { c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("client_name must be less than %d bytes", formFieldLen)}) diff --git a/internal/api/client/auth/auth.go b/internal/api/client/auth/auth.go @@ -29,6 +29,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/router" ) +/* #nosec G101 */ const ( // AuthSignInPath is the API path for users to sign in through AuthSignInPath = "/auth/sign_in" diff --git a/internal/api/client/auth/callback.go b/internal/api/client/auth/callback.go @@ -182,7 +182,7 @@ func (m *Module) parseUserFromClaims(ctx context.Context, claims *oidc.Claims, i // // note that for the first iteration, iString is still "" when the check is made, so our first choice // is still the raw username with no integer stuck on the end - for i := 1; !found; i = i + 1 { + for i := 1; !found; i++ { usernameAvailable, err := m.db.IsUsernameAvailable(ctx, username+iString) if err != nil { return nil, err @@ -190,7 +190,7 @@ func (m *Module) parseUserFromClaims(ctx context.Context, claims *oidc.Claims, i if usernameAvailable { // no error so we've found a username that works found = true - username = username + iString + username += iString continue } iString = strconv.Itoa(i) diff --git a/internal/api/client/status/status.go b/internal/api/client/status/status.go @@ -19,10 +19,11 @@ package status import ( - "github.com/sirupsen/logrus" "net/http" "strings" + "github.com/sirupsen/logrus" + "github.com/gin-gonic/gin" "github.com/superseriousbusiness/gotosocial/internal/api" "github.com/superseriousbusiness/gotosocial/internal/config" @@ -110,13 +111,13 @@ func (m *Module) muxHandler(c *gin.Context) { logrus.Debug("entering mux handler") ru := c.Request.RequestURI - switch c.Request.Method { - case http.MethodGet: - if strings.HasPrefix(ru, ContextPath) { + if c.Request.Method == http.MethodGet { + switch { + case strings.HasPrefix(ru, ContextPath): // TODO - } else if strings.HasPrefix(ru, FavouritedPath) { + case strings.HasPrefix(ru, FavouritedPath): m.StatusFavedByGETHandler(c) - } else { + default: m.StatusGETHandler(c) } } diff --git a/internal/api/s2s/user/outboxget.go b/internal/api/s2s/user/outboxget.go @@ -90,9 +90,8 @@ func (m *Module) OutboxGETHandler(c *gin.Context) { return } - page := false - pageString := c.Query(PageKey) - if pageString != "" { + var page bool + if pageString := c.Query(PageKey); pageString != "" { i, err := strconv.ParseBool(pageString) if err != nil { l.Debugf("error parsing page string: %s", err) diff --git a/internal/api/s2s/user/repliesget.go b/internal/api/s2s/user/repliesget.go @@ -102,9 +102,8 @@ func (m *Module) StatusRepliesGETHandler(c *gin.Context) { return } - page := false - pageString := c.Query(PageKey) - if pageString != "" { + var page bool + if pageString := c.Query(PageKey); pageString != "" { i, err := strconv.ParseBool(pageString) if err != nil { l.Debugf("error parsing page string: %s", err) diff --git a/internal/api/security/useragentblock.go b/internal/api/security/useragentblock.go @@ -31,8 +31,7 @@ func (m *Module) UserAgentBlock(c *gin.Context) { "func": "UserAgentBlock", }) - ua := c.Request.UserAgent() - if ua == "" { + if ua := c.Request.UserAgent(); ua == "" { l.Debug("aborting request because there's no user-agent set") c.AbortWithStatus(http.StatusTeapot) return diff --git a/internal/cache/account.go b/internal/cache/account.go @@ -4,6 +4,7 @@ import ( "sync" "github.com/ReneKroon/ttlcache" + "github.com/sirupsen/logrus" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" ) @@ -26,7 +27,10 @@ func NewAccountCache() *AccountCache { // Set callback to purge lookup maps on expiration c.cache.SetExpirationCallback(func(key string, value interface{}) { - account := value.(*gtsmodel.Account) + account, ok := value.(*gtsmodel.Account) + if !ok { + logrus.Panicf("AccountCache could not assert entry with key %s to *gtsmodel.Account", key) + } c.mutex.Lock() delete(c.urls, account.URL) diff --git a/internal/cache/status.go b/internal/cache/status.go @@ -4,6 +4,7 @@ import ( "sync" "github.com/ReneKroon/ttlcache" + "github.com/sirupsen/logrus" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" ) @@ -26,7 +27,10 @@ func NewStatusCache() *StatusCache { // Set callback to purge lookup maps on expiration c.cache.SetExpirationCallback(func(key string, value interface{}) { - status := value.(*gtsmodel.Status) + status, ok := value.(*gtsmodel.Status) + if !ok { + logrus.Panicf("StatusCache could not assert entry with key %s to *gtsmodel.Status", key) + } c.mutex.Lock() delete(c.urls, status.URL) diff --git a/internal/config/db.go b/internal/config/db.go @@ -46,4 +46,4 @@ var DBTLSModeEnable DBTLSMode = "enable" var DBTLSModeRequire DBTLSMode = "require" // DBTLSModeUnset means that the TLS mode has not been set. -var DBTLSModeUnset DBTLSMode = "" +var DBTLSModeUnset DBTLSMode diff --git a/internal/config/default.go b/internal/config/default.go @@ -180,8 +180,8 @@ func GetDefaults() Defaults { AccountsRequireApproval: true, AccountsReasonRequired: true, - MediaMaxImageSize: 2097152, //2mb - MediaMaxVideoSize: 10485760, //10mb + MediaMaxImageSize: 2097152, // 2mb + MediaMaxVideoSize: 10485760, // 10mb MediaMinDescriptionChars: 0, MediaMaxDescriptionChars: 500, @@ -244,8 +244,8 @@ func GetTestDefaults() Defaults { AccountsRequireApproval: true, AccountsReasonRequired: true, - MediaMaxImageSize: 1048576, //1mb - MediaMaxVideoSize: 5242880, //5mb + MediaMaxImageSize: 1048576, // 1mb + MediaMaxVideoSize: 5242880, // 5mb MediaMinDescriptionChars: 0, MediaMaxDescriptionChars: 500, diff --git a/internal/db/bundb/account.go b/internal/db/bundb/account.go @@ -137,8 +137,7 @@ func (a *accountDB) GetInstanceAccount(ctx context.Context, domain string) (*gts WhereGroup(" AND ", whereEmptyOrNull("domain")) } - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, a.conn.ProcessError(err) } return account, nil @@ -155,8 +154,7 @@ func (a *accountDB) GetAccountLastPosted(ctx context.Context, accountID string) Where("account_id = ?", accountID). Column("created_at") - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return time.Time{}, a.conn.ProcessError(err) } return status.CreatedAt, nil @@ -168,11 +166,12 @@ func (a *accountDB) SetAccountHeaderOrAvatar(ctx context.Context, mediaAttachmen } var headerOrAVI string - if mediaAttachment.Avatar { + switch { + case mediaAttachment.Avatar: headerOrAVI = "avatar" - } else if mediaAttachment.Header { + case mediaAttachment.Header: headerOrAVI = "header" - } else { + default: return errors.New("given media attachment was neither a header nor an avatar") } @@ -202,8 +201,7 @@ func (a *accountDB) GetLocalAccountByUsername(ctx context.Context, username stri Where("username = ?", username). WhereGroup(" AND ", whereEmptyOrNull("domain")) - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, a.conn.ProcessError(err) } return account, nil @@ -308,8 +306,7 @@ func (a *accountDB) GetAccountBlocks(ctx context.Context, accountID string, maxI fq = fq.Limit(limit) } - err := fq.Scan(ctx) - if err != nil { + if err := fq.Scan(ctx); err != nil { return nil, "", "", a.conn.ProcessError(err) } diff --git a/internal/db/bundb/bundb.go b/internal/db/bundb/bundb.go @@ -54,7 +54,7 @@ const ( dbTypeSqlite = "sqlite" ) -var registerTables []interface{} = []interface{}{ +var registerTables = []interface{}{ >smodel.StatusToEmoji{}, >smodel.StatusToTag{}, } @@ -220,7 +220,7 @@ func sqliteConn(ctx context.Context, c *config.Config) (*DBConn, error) { } tweakConnectionValues(sqldb) - + if c.DBConfig.Address == "file::memory:?cache=shared" { logrus.Warn("sqlite in-memory database should only be used for debugging") // don't close connections on disconnect -- otherwise @@ -248,11 +248,11 @@ func pgConn(ctx context.Context, c *config.Config) (*DBConn, error) { if err != nil { return nil, fmt.Errorf("could not create bundb postgres options: %s", err) } - + sqldb := stdlib.OpenDB(*opts) - + tweakConnectionValues(sqldb) - + conn := WrapDBConn(bun.NewDB(sqldb, pgdialect.New())) // ping to check the db is there and listening @@ -305,6 +305,7 @@ func deriveBunDBPGOptions(c *config.Config) (*pgx.ConnConfig, error) { case config.DBTLSModeDisable, config.DBTLSModeUnset: break // nothing to do case config.DBTLSModeEnable: + /* #nosec G402 */ tlsConfig = &tls.Config{ InsecureSkipVerify: true, } @@ -312,6 +313,7 @@ func deriveBunDBPGOptions(c *config.Config) (*pgx.ConnConfig, error) { tlsConfig = &tls.Config{ InsecureSkipVerify: false, ServerName: c.DBConfig.Address, + MinVersion: tls.VersionTLS12, } } diff --git a/internal/db/bundb/instance.go b/internal/db/bundb/instance.go @@ -116,8 +116,7 @@ func (i *instanceDB) GetInstanceAccounts(ctx context.Context, domain string, max q = q.Limit(limit) } - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, i.conn.ProcessError(err) } return accounts, nil diff --git a/internal/db/bundb/media.go b/internal/db/bundb/media.go @@ -45,8 +45,7 @@ func (m *mediaDB) GetAttachmentByID(ctx context.Context, id string) (*gtsmodel.M q := m.newMediaQ(attachment). Where("media_attachment.id = ?", id) - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, m.conn.ProcessError(err) } return attachment, nil diff --git a/internal/db/bundb/mention.go b/internal/db/bundb/mention.go @@ -61,8 +61,7 @@ func (m *mentionDB) getMentionDB(ctx context.Context, id string) (*gtsmodel.Ment q := m.newMentionQ(mention). Where("mention.id = ?", id) - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, m.conn.ProcessError(err) } diff --git a/internal/db/bundb/notification.go b/internal/db/bundb/notification.go @@ -125,8 +125,7 @@ func (n *notificationDB) getNotificationDB(ctx context.Context, id string, dst * q := n.newNotificationQ(dst). Where("notification.id = ?", id) - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return n.conn.ProcessError(err) } diff --git a/internal/db/bundb/relationship.go b/internal/db/bundb/relationship.go @@ -74,8 +74,7 @@ func (r *relationshipDB) GetBlock(ctx context.Context, account1 string, account2 Where("block.account_id = ?", account1). Where("block.target_account_id = ?", account2) - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, r.conn.ProcessError(err) } return block, nil @@ -286,8 +285,7 @@ func (r *relationshipDB) GetAccountFollowRequests(ctx context.Context, accountID q := r.newFollowQ(&followRequests). Where("target_account_id = ?", accountID) - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, r.conn.ProcessError(err) } return followRequests, nil @@ -299,8 +297,7 @@ func (r *relationshipDB) GetAccountFollows(ctx context.Context, accountID string q := r.newFollowQ(&follows). Where("account_id = ?", accountID) - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, r.conn.ProcessError(err) } return follows, nil diff --git a/internal/db/bundb/session.go b/internal/db/bundb/session.go @@ -47,7 +47,7 @@ func (s *sessionDB) GetSession(ctx context.Context) (*gtsmodel.RouterSession, db return nil, s.conn.ProcessError(err) } - if len(rss) <= 0 { + if len(rss) == 0 { // no session created yet, so make one return s.createSession(ctx) } diff --git a/internal/db/bundb/status.go b/internal/db/bundb/status.go @@ -23,6 +23,7 @@ import ( "context" "time" + "github.com/sirupsen/logrus" "github.com/superseriousbusiness/gotosocial/internal/cache" "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/db" @@ -206,7 +207,11 @@ func (s *statusDB) GetStatusChildren(ctx context.Context, status *gtsmodel.Statu children := []*gtsmodel.Status{} for e := foundStatuses.Front(); e != nil; e = e.Next() { // only append children, not the overall parent status - entry := e.Value.(*gtsmodel.Status) + entry, ok := e.Value.(*gtsmodel.Status) + if !ok { + logrus.Panic("GetStatusChildren: found status could not be asserted to *gtsmodel.Status") + } + if entry.ID != status.ID { children = append(children, entry) } @@ -233,7 +238,11 @@ func (s *statusDB) statusChildren(ctx context.Context, status *gtsmodel.Status, for _, child := range immediateChildren { insertLoop: for e := foundStatuses.Front(); e != nil; e = e.Next() { - entry := e.Value.(*gtsmodel.Status) + entry, ok := e.Value.(*gtsmodel.Status) + if !ok { + logrus.Panic("statusChildren: found status could not be asserted to *gtsmodel.Status") + } + if child.InReplyToAccountID != "" && entry.ID == child.InReplyToID { foundStatuses.InsertAfter(child, e) break insertLoop @@ -306,8 +315,7 @@ func (s *statusDB) GetStatusFaves(ctx context.Context, status *gtsmodel.Status) q := s.newFaveQ(&faves). Where("status_id = ?", status.ID) - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, s.conn.ProcessError(err) } return faves, nil @@ -319,8 +327,7 @@ func (s *statusDB) GetStatusReblogs(ctx context.Context, status *gtsmodel.Status q := s.newStatusQ(&reblogs). Where("boost_of_id = ?", status.ID) - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, s.conn.ProcessError(err) } return reblogs, nil diff --git a/internal/db/bundb/timeline.go b/internal/db/bundb/timeline.go @@ -91,8 +91,7 @@ func (t *timelineDB) GetHomeTimeline(ctx context.Context, accountID string, maxI q = q.WhereGroup(" AND ", whereGroup) - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, t.conn.ProcessError(err) } return statuses, nil @@ -136,8 +135,7 @@ func (t *timelineDB) GetPublicTimeline(ctx context.Context, accountID string, ma q = q.Limit(limit) } - err := q.Scan(ctx) - if err != nil { + if err := q.Scan(ctx); err != nil { return nil, t.conn.ProcessError(err) } return statuses, nil diff --git a/internal/federation/authenticate.go b/internal/federation/authenticate.go @@ -149,7 +149,7 @@ func (f *federator) AuthenticateFederatedRequest(ctx context.Context, requestedU requestingPublicKeyID, err := url.Parse(verifier.KeyId()) if err != nil { l.Debug("couldn't parse public key URL") - return nil, false, nil // couldn't parse the public key ID url + return nil, false, err // couldn't parse the public key ID url } requestingRemoteAccount := >smodel.Account{} diff --git a/internal/federation/dereferencing/thread.go b/internal/federation/dereferencing/thread.go @@ -172,7 +172,7 @@ pageLoop: l.Debugf("dereferencing page %s", currentPageIRI) nextPage, err := d.DereferenceCollectionPage(ctx, username, currentPageIRI) if err != nil { - return nil + return err } // next items could be either a list of URLs or a list of statuses @@ -188,19 +188,19 @@ pageLoop: // We're looking for a url to feed to GetRemoteStatus. // Items can be either an IRI, or a Note. // If a note, we grab the ID from it and call it, rather than parsing the note. - var itemURI *url.URL - if iter.IsIRI() { + switch { + case iter.IsIRI(): // iri, easy itemURI = iter.GetIRI() - } else if iter.IsActivityStreamsNote() { + case iter.IsActivityStreamsNote(): // note, get the id from it to use as iri n := iter.GetActivityStreamsNote() id := n.GetJSONLDId() if id != nil && id.IsIRI() { itemURI = id.GetIRI() } - } else { + default: // if it's not an iri or a note, we don't know how to process it continue } @@ -211,7 +211,7 @@ pageLoop: } // we can confidently say now that we found something - foundReplies = foundReplies + 1 + foundReplies++ // get the remote statusable and put it in the db _, statusable, new, err := d.GetRemoteStatus(ctx, username, itemURI, false, false) diff --git a/internal/federation/federatingdb/accept.go b/internal/federation/federatingdb/accept.go @@ -97,9 +97,7 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA if iter.GetType() == nil { continue } - switch iter.GetType().GetTypeName() { - // we have the whole object so we can figure out what we're accepting - case ap.ActivityFollow: + if iter.GetType().GetTypeName() == ap.ActivityFollow { // ACCEPT FOLLOW asFollow, ok := iter.GetType().(vocab.ActivityStreamsFollow) if !ok { diff --git a/internal/federation/federatingdb/lock.go b/internal/federation/federatingdb/lock.go @@ -24,6 +24,8 @@ import ( "net/url" "sync" "sync/atomic" + + "github.com/sirupsen/logrus" ) // Lock takes a lock for the object at the specified id. If an error @@ -54,7 +56,10 @@ func (f *federatingDB) Lock(c context.Context, id *url.URL) error { // Get mutex, or create new mu, ok := f.locks[idStr] if !ok { - mu = f.pool.Get().(*mutex) + mu, ok = f.pool.Get().(*mutex) + if !ok { + logrus.Panic("Lock: pool entry was not a *mutex") + } f.locks[idStr] = mu } diff --git a/internal/federation/federatingdb/reject.go b/internal/federation/federatingdb/reject.go @@ -90,9 +90,8 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR continue } - switch iter.GetType().GetTypeName() { - // we have the whole object so we can figure out what we're rejecting - case ap.ActivityFollow: + if iter.GetType().GetTypeName() == ap.ActivityFollow { + // we have the whole object so we can figure out what we're rejecting // REJECT FOLLOW asFollow, ok := iter.GetType().(vocab.ActivityStreamsFollow) if !ok { diff --git a/internal/federation/federatingdb/util.go b/internal/federation/federatingdb/util.go @@ -308,17 +308,29 @@ func (f *federatingDB) collectIRIs(ctx context.Context, iris []*url.URL) (vocab. func extractFromCtx(ctx context.Context) (receivingAccount, requestingAccount *gtsmodel.Account, fromFederatorChan chan messages.FromFederator) { receivingAccountI := ctx.Value(util.APReceivingAccount) if receivingAccountI != nil { - receivingAccount = receivingAccountI.(*gtsmodel.Account) + var ok bool + receivingAccount, ok = receivingAccountI.(*gtsmodel.Account) + if !ok { + logrus.Panicf("extractFromCtx: context entry with key %s could not be asserted to *gtsmodel.Account", util.APReceivingAccount) + } } requestingAcctI := ctx.Value(util.APRequestingAccount) if requestingAcctI != nil { - requestingAccount = requestingAcctI.(*gtsmodel.Account) + var ok bool + requestingAccount, ok = requestingAcctI.(*gtsmodel.Account) + if !ok { + logrus.Panicf("extractFromCtx: context entry with key %s could not be asserted to *gtsmodel.Account", util.APRequestingAccount) + } } fromFederatorChanI := ctx.Value(util.APFromFederatorChanKey) if fromFederatorChanI != nil { - fromFederatorChan = fromFederatorChanI.(chan messages.FromFederator) + var ok bool + fromFederatorChan, ok = fromFederatorChanI.(chan messages.FromFederator) + if !ok { + logrus.Panicf("extractFromCtx: context entry with key %s could not be asserted to chan messages.FromFederator", util.APFromFederatorChanKey) + } } return diff --git a/internal/federation/transport.go b/internal/federation/transport.go @@ -54,17 +54,18 @@ func (f *federator) NewTransport(ctx context.Context, actorBoxIRI *url.URL, gofe var username string var err error - if util.IsInboxPath(actorBoxIRI) { + switch { + case util.IsInboxPath(actorBoxIRI): username, err = util.ParseInboxPath(actorBoxIRI) if err != nil { return nil, fmt.Errorf("couldn't parse path %s as an inbox: %s", actorBoxIRI.String(), err) } - } else if util.IsOutboxPath(actorBoxIRI) { + case util.IsOutboxPath(actorBoxIRI): username, err = util.ParseOutboxPath(actorBoxIRI) if err != nil { return nil, fmt.Errorf("couldn't parse path %s as an outbox: %s", actorBoxIRI.String(), err) } - } else { + default: return nil, fmt.Errorf("id %s was neither an inbox path nor an outbox path", actorBoxIRI.String()) } diff --git a/internal/processing/federation/getstatusreplies.go b/internal/processing/federation/getstatusreplies.go @@ -82,10 +82,9 @@ func (p *processor) GetStatusReplies(ctx context.Context, requestedUsername stri // 1. we're asked for the whole collection and not a page -- we can just return the collection, with no items, but a link to 'first' page. // 2. we're asked for a page but only_other_accounts has not been set in the query -- so we should just return the first page of the collection, with no items. // 3. we're asked for a page, and only_other_accounts has been set, and min_id has optionally been set -- so we need to return some actual items! - - if !page { + switch { + case !page: // scenario 1 - // get the collection collection, err := p.tc.StatusToASRepliesCollection(ctx, s, onlyOtherAccounts) if err != nil { @@ -96,9 +95,8 @@ func (p *processor) GetStatusReplies(ctx context.Context, requestedUsername stri if err != nil { return nil, gtserror.NewErrorInternalError(err) } - } else if page && requestURL.Query().Get("only_other_accounts") == "" { + case page && requestURL.Query().Get("only_other_accounts") == "": // scenario 2 - // get the collection collection, err := p.tc.StatusToASRepliesCollection(ctx, s, onlyOtherAccounts) if err != nil { @@ -109,7 +107,7 @@ func (p *processor) GetStatusReplies(ctx context.Context, requestedUsername stri if err != nil { return nil, gtserror.NewErrorInternalError(err) } - } else { + default: // scenario 3 // get immediate children replies, err := p.db.GetStatusChildren(ctx, s, true, minID) diff --git a/internal/processing/federation/getuser.go b/internal/processing/federation/getuser.go @@ -38,13 +38,14 @@ func (p *processor) GetUser(ctx context.Context, requestedUsername string, reque } var requestedPerson vocab.ActivityStreamsPerson - if util.IsPublicKeyPath(requestURL) { + switch { + case util.IsPublicKeyPath(requestURL): // if it's a public key path, we don't need to authenticate but we'll only serve the bare minimum user profile needed for the public key requestedPerson, err = p.tc.AccountToASMinimal(ctx, requestedAccount) if err != nil { return nil, gtserror.NewErrorInternalError(err) } - } else if util.IsUserPath(requestURL) { + case util.IsUserPath(requestURL): // if it's a user path, we want to fully authenticate the request before we serve any data, and then we can serve a more complete profile requestingAccountURI, authenticated, err := p.federator.AuthenticateFederatedRequest(ctx, requestedUsername) if err != nil || !authenticated { @@ -72,7 +73,7 @@ func (p *processor) GetUser(ctx context.Context, requestedUsername string, reque if err != nil { return nil, gtserror.NewErrorInternalError(err) } - } else { + default: return nil, gtserror.NewErrorBadRequest(fmt.Errorf("path was not public key path or user path")) } diff --git a/internal/processing/fromclientapi.go b/internal/processing/fromclientapi.go @@ -64,15 +64,13 @@ func (p *processor) ProcessFromClientAPI(ctx context.Context, clientMsg messages } case ap.ActivityAccept: // ACCEPT - switch clientMsg.APObjectType { - case ap.ActivityFollow: + if clientMsg.APObjectType == ap.ActivityFollow { // ACCEPT FOLLOW return p.processAcceptFollowFromClientAPI(ctx, clientMsg) } case ap.ActivityReject: // REJECT - switch clientMsg.APObjectType { - case ap.ActivityFollow: + if clientMsg.APObjectType == ap.ActivityFollow { // REJECT FOLLOW (request) return p.processRejectFollowFromClientAPI(ctx, clientMsg) } diff --git a/internal/processing/fromfederator.go b/internal/processing/fromfederator.go @@ -64,8 +64,7 @@ func (p *processor) ProcessFromFederator(ctx context.Context, federatorMsg messa } case ap.ActivityUpdate: // UPDATE SOMETHING - switch federatorMsg.APObjectType { - case ap.ObjectProfile: + if federatorMsg.APObjectType == ap.ObjectProfile { // UPDATE AN ACCOUNT return p.processUpdateAccountFromFederator(ctx, federatorMsg) } diff --git a/internal/processing/status/util.go b/internal/processing/status/util.go @@ -38,14 +38,15 @@ func (p *processor) ProcessVisibility(ctx context.Context, form *apimodel.Advanc replyable := true likeable := true - var vis gtsmodel.Visibility // If visibility isn't set on the form, then just take the account default. // If that's also not set, take the default for the whole instance. - if form.Visibility != "" { + var vis gtsmodel.Visibility + switch { + case form.Visibility != "": vis = p.tc.APIVisToVis(form.Visibility) - } else if accountDefaultVis != "" { + case accountDefaultVis != "": vis = accountDefaultVis - } else { + default: vis = gtsmodel.VisibilityDefault } diff --git a/internal/router/template.go b/internal/router/template.go @@ -51,6 +51,7 @@ func oddOrEven(n int) string { } func noescape(str string) template.HTML { + /* #nosec G203 */ return template.HTML(str) } @@ -67,19 +68,21 @@ type iconWithLabel struct { func visibilityIcon(visibility model.Visibility) template.HTML { var icon iconWithLabel - if visibility == model.VisibilityPublic { + switch visibility { + case model.VisibilityPublic: icon = iconWithLabel{"globe", "public"} - } else if visibility == model.VisibilityUnlisted { + case model.VisibilityUnlisted: icon = iconWithLabel{"unlock", "unlisted"} - } else if visibility == model.VisibilityPrivate { + case model.VisibilityPrivate: icon = iconWithLabel{"lock", "private"} - } else if visibility == model.VisibilityMutualsOnly { + case model.VisibilityMutualsOnly: icon = iconWithLabel{"handshake-o", "mutuals only"} - } else if visibility == model.VisibilityDirect { + case model.VisibilityDirect: icon = iconWithLabel{"envelope", "direct"} } - return template.HTML(fmt.Sprintf(`<i aria-label="Visiblity: %v" class="fa fa-%v"></i>`, icon.label, icon.faIcon)) + /* #nosec G203 */ + return template.HTML(fmt.Sprintf(`<i aria-label="Visibility: %v" class="fa fa-%v"></i>`, icon.label, icon.faIcon)) } func loadTemplateFunctions(engine *gin.Engine) { diff --git a/internal/text/link.go b/internal/text/link.go @@ -98,7 +98,7 @@ func (f *formatter) ReplaceLinks(ctx context.Context, in string) string { shortString := thisURL.Hostname() if thisURL.Path != "" { - shortString = shortString + thisURL.Path + shortString += thisURL.Path } if thisURL.Fragment != "" { diff --git a/internal/timeline/get.go b/internal/timeline/get.go @@ -127,7 +127,7 @@ func (t *timeline) GetXFromTop(ctx context.Context, amount int) ([]*apimodel.Sta return nil, errors.New("GetXFromTop: could not parse e as a preparedPostsEntry") } statuses = append(statuses, entry.prepared) - served = served + 1 + served++ if served >= amount { break } @@ -145,7 +145,7 @@ func (t *timeline) GetXBehindID(ctx context.Context, amount int, behindID string }) newAttempts := *attempts - newAttempts = newAttempts + 1 + newAttempts++ attempts = &newAttempts // make a slice of statuses with the length we need to return @@ -161,7 +161,7 @@ func (t *timeline) GetXBehindID(ctx context.Context, amount int, behindID string findMarkLoop: for e := t.preparedPosts.data.Front(); e != nil; e = e.Next() { - position = position + 1 + position++ entry, ok := e.Value.(*preparedPostsEntry) if !ok { return nil, errors.New("GetXBehindID: could not parse e as a preparedPostsEntry") @@ -218,7 +218,7 @@ serveloop: // serve up to the amount requested statuses = append(statuses, entry.prepared) - served = served + 1 + served++ if served >= amount { break serveloop } @@ -272,7 +272,7 @@ findMarkLoop: // serve up to the amount requested statuses = append(statuses, entry.prepared) - served = served + 1 + served++ if served >= amount { break serveloopFromTop } @@ -288,7 +288,7 @@ findMarkLoop: // serve up to the amount requested statuses = append(statuses, entry.prepared) - served = served + 1 + served++ if served >= amount { break serveloopFromBottom } @@ -311,7 +311,7 @@ func (t *timeline) GetXBetweenID(ctx context.Context, amount int, behindID strin var behindIDMark *list.Element findMarkLoop: for e := t.preparedPosts.data.Front(); e != nil; e = e.Next() { - position = position + 1 + position++ entry, ok := e.Value.(*preparedPostsEntry) if !ok { return nil, errors.New("GetXBetweenID: could not parse e as a preparedPostsEntry") @@ -350,7 +350,7 @@ serveloop: // serve up to the amount requested statuses = append(statuses, entry.prepared) - served = served + 1 + served++ if served >= amount { break serveloop } diff --git a/internal/timeline/index.go b/internal/timeline/index.go @@ -50,7 +50,7 @@ func (t *timeline) IndexBefore(ctx context.Context, statusID string, include boo i := 0 grabloop: - for ; len(filtered) < amount && i < 5; i = i + 1 { // try the grabloop 5 times only + for ; len(filtered) < amount && i < 5; i++ { // try the grabloop 5 times only statuses, err := t.db.GetHomeTimeline(ctx, t.accountID, "", "", offsetStatus, amount, false) if err != nil { if err == db.ErrNoEntries { @@ -129,7 +129,7 @@ positionLoop: i := 0 grabloop: - for ; len(filtered) < amount && i < 5; i = i + 1 { // try the grabloop 5 times only + for ; len(filtered) < amount && i < 5; i++ { // try the grabloop 5 times only l.Tracef("entering grabloop; i is %d; len(filtered) is %d", i, len(filtered)) statuses, err := t.db.GetHomeTimeline(ctx, t.accountID, offsetStatus, "", "", amount, false) if err != nil { diff --git a/internal/timeline/postindex.go b/internal/timeline/postindex.go @@ -50,7 +50,7 @@ func (p *postIndex) insertIndexed(i *postIndexEntry) (bool, error) { // We need to iterate through the index to make sure we put this post in the appropriate place according to when it was created. // We also need to make sure we're not inserting a duplicate post -- this can happen sometimes and it's not nice UX (*shudder*). for e := p.data.Front(); e != nil; e = e.Next() { - position = position + 1 + position++ entry, ok := e.Value.(*postIndexEntry) if !ok { diff --git a/internal/timeline/prepare.go b/internal/timeline/prepare.go @@ -107,7 +107,7 @@ prepareloop: // we're done break prepareloop } - prepared = prepared + 1 + prepared++ } } @@ -162,7 +162,7 @@ prepareloop: // we're done break prepareloop } - prepared = prepared + 1 + prepared++ } } @@ -214,7 +214,7 @@ prepareloop: continue } - prepared = prepared + 1 + prepared++ if prepared == amount { // we're done l.Trace("leaving prepareloop") diff --git a/internal/timeline/preparedposts.go b/internal/timeline/preparedposts.go @@ -53,7 +53,7 @@ func (p *preparedPosts) insertPrepared(i *preparedPostsEntry) error { // We need to iterate through the index to make sure we put this post in the appropriate place according to when it was created. // We also need to make sure we're not inserting a duplicate post -- this can happen sometimes and it's not nice UX (*shudder*). for e := p.data.Front(); e != nil; e = e.Next() { - position = position + 1 + position++ entry, ok := e.Value.(*preparedPostsEntry) if !ok { diff --git a/internal/timeline/remove.go b/internal/timeline/remove.go @@ -52,7 +52,7 @@ func (t *timeline) Remove(ctx context.Context, statusID string) (int, error) { } for _, e := range removeIndexes { t.postIndex.data.Remove(e) - removed = removed + 1 + removed++ } // remove entr(ies) from prepared posts @@ -71,7 +71,7 @@ func (t *timeline) Remove(ctx context.Context, statusID string) (int, error) { } for _, e := range removePrepared { t.preparedPosts.data.Remove(e) - removed = removed + 1 + removed++ } l.Debugf("removed %d entries", removed) @@ -104,7 +104,7 @@ func (t *timeline) RemoveAllBy(ctx context.Context, accountID string) (int, erro } for _, e := range removeIndexes { t.postIndex.data.Remove(e) - removed = removed + 1 + removed++ } // remove entr(ies) from prepared posts @@ -123,7 +123,7 @@ func (t *timeline) RemoveAllBy(ctx context.Context, accountID string) (int, erro } for _, e := range removePrepared { t.preparedPosts.data.Remove(e) - removed = removed + 1 + removed++ } l.Debugf("removed %d entries", removed) diff --git a/testrig/db.go b/testrig/db.go @@ -29,7 +29,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" ) -var testModels []interface{} = []interface{}{ +var testModels = []interface{}{ >smodel.Account{}, >smodel.Application{}, >smodel.Block{},