Skip to content

Commit

Permalink
[chore] Upgrade golangci-lint, ignore existing int overflow warnings (#…
Browse files Browse the repository at this point in the history
…3420)

* [chore] Bump tooling versions, bump go -> v1.23.0

* undo silly change

* sign

* bump go version in go.mod

* allow overflow in imaging

* goreleaser deprecation notices

* [chore] Upgrade golangci-lint, ignore existing int overflow warnings

There is a new lint for unchecked int casts. Integer overflows are bad,
but the old code that triggers this lint seems to be perfectly fine.
Instead of disabling the lint entirely for new code as well, grandfather
in existing code.

* fix golangci-lint documentation link

* revert unrelated changes

* revert another unrelated change

* get rid of remaining nolint:gosec

* swagger updates

* apply review feedback

* fix wrong formatting specifier thing

* fix the linter for real

---------

Co-authored-by: tobi <[email protected]>
  • Loading branch information
untitaker and tsmethurst authored Oct 16, 2024
1 parent 3e8c495 commit a48cce8
Show file tree
Hide file tree
Showing 22 changed files with 86 additions and 74 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ We use [golangci-lint](https://golangci-lint.run/) for linting, which allows us

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).
To do this, first install the linter following the instructions [here](https://golangci-lint.run/welcome/install/).

Then, you can run the linter with:

Expand Down
2 changes: 1 addition & 1 deletion docs/api/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2114,7 +2114,7 @@ definitions:
bitrate:
description: Bitrate of the media in bits per second.
example: 1000000
format: int64
format: uint64
type: integer
x-go-name: Bitrate
duration:
Expand Down
4 changes: 2 additions & 2 deletions internal/api/client/admin/emojicreate.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ func validateCreateEmoji(form *apimodel.EmojiCreateRequest) error {
return errors.New("no emoji given")
}

maxSize := config.GetMediaEmojiLocalMaxSize()
if form.Image.Size > int64(maxSize) {
maxSize := int64(config.GetMediaEmojiLocalMaxSize()) // #nosec G115 -- Already validated.
if form.Image.Size > maxSize {
return fmt.Errorf("emoji image too large: image is %dKB but size limit for custom emojis is %dKB", form.Image.Size/1024, maxSize/1024)
}

Expand Down
4 changes: 2 additions & 2 deletions internal/api/client/admin/emojiupdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ func validateUpdateEmoji(form *apimodel.EmojiUpdateRequest) error {
}

if hasImage {
maxSize := config.GetMediaEmojiLocalMaxSize()
if form.Image.Size > int64(maxSize) {
maxSize := int64(config.GetMediaEmojiLocalMaxSize()) // #nosec G115 -- Already validated.
if form.Image.Size > maxSize {
return fmt.Errorf("emoji image too large: image is %dKB but size limit for custom emojis is %dKB", form.Image.Size/1024, maxSize/1024)
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/api/model/attachment.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ type MediaDimensions struct {
Duration float32 `json:"duration,omitempty"`
// Bitrate of the media in bits per second.
// example: 1000000
Bitrate int `json:"bitrate,omitempty"`
Bitrate uint64 `json:"bitrate,omitempty"`
// Size of the media, in the format `[width]x[height]`.
// Not set for audio.
// example: 1920x1080
Expand Down
2 changes: 1 addition & 1 deletion internal/cache/domain/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (n *node) getChild(part string) *node {

for i < j {
// avoid overflow when computing h
h := int(uint(i+j) >> 1)
h := int(uint(i+j) >> 1) // #nosec G115
// i ≤ h < j

if n.child[h].part < part {
Expand Down
6 changes: 5 additions & 1 deletion internal/db/bundb/bundb.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"encoding/pem"
"errors"
"fmt"
"math"
"net/url"
"os"
"runtime"
Expand Down Expand Up @@ -489,7 +490,10 @@ func deriveBunDBPGOptions() (*pgx.ConnConfig, error) {
cfg.Host = address
}
if port := config.GetDbPort(); port > 0 {
cfg.Port = uint16(port)
if port > math.MaxUint16 {
return nil, errors.New("invalid port, must be in range 1-65535")
}
cfg.Port = uint16(port) // #nosec G115 -- Just validated above.
}
if u := config.GetDbUser(); u != "" {
cfg.User = u
Expand Down
12 changes: 6 additions & 6 deletions internal/federation/dereferencing/emoji.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ func (d *Dereferencer) GetEmoji(
}

// Get maximum supported remote emoji size.
maxsz := config.GetMediaEmojiRemoteMaxSize()
maxsz := int64(config.GetMediaEmojiRemoteMaxSize()) // #nosec G115 -- Already validated.

// Prepare data function to dereference remote emoji media.
data := func(context.Context) (io.ReadCloser, error) {
return tsport.DereferenceMedia(ctx, url, int64(maxsz))
return tsport.DereferenceMedia(ctx, url, maxsz)
}

// Create new emoji with prepared info.
Expand Down Expand Up @@ -189,11 +189,11 @@ func (d *Dereferencer) RefreshEmoji(
}

// Get maximum supported remote emoji size.
maxsz := config.GetMediaEmojiRemoteMaxSize()
maxsz := int64(config.GetMediaEmojiRemoteMaxSize()) // #nosec G115 -- Already validated.

// Prepare data function to dereference remote emoji media.
data := func(context.Context) (io.ReadCloser, error) {
return tsport.DereferenceMedia(ctx, url, int64(maxsz))
return tsport.DereferenceMedia(ctx, url, maxsz)
}

// Update emoji with prepared info.
Expand Down Expand Up @@ -255,11 +255,11 @@ func (d *Dereferencer) RecacheEmoji(
}

// Get maximum supported remote emoji size.
maxsz := config.GetMediaEmojiRemoteMaxSize()
maxsz := int64(config.GetMediaEmojiRemoteMaxSize()) // #nosec G115 -- Already validated.

// Prepare data function to dereference remote emoji media.
data := func(context.Context) (io.ReadCloser, error) {
return tsport.DereferenceMedia(ctx, url, int64(maxsz))
return tsport.DereferenceMedia(ctx, url, maxsz)
}

// Recache emoji with prepared info.
Expand Down
8 changes: 4 additions & 4 deletions internal/federation/dereferencing/media.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,14 @@ func (d *Dereferencer) GetMedia(
}

// Get maximum supported remote media size.
maxsz := config.GetMediaRemoteMaxSize()
maxsz := int64(config.GetMediaRemoteMaxSize()) // #nosec G115 -- Already validated.

// Create media with prepared info.
return d.mediaManager.CreateMedia(
ctx,
accountID,
func(ctx context.Context) (io.ReadCloser, error) {
return tsport.DereferenceMedia(ctx, url, int64(maxsz))
return tsport.DereferenceMedia(ctx, url, maxsz)
},
info,
)
Expand Down Expand Up @@ -168,14 +168,14 @@ func (d *Dereferencer) RefreshMedia(
}

// Get maximum supported remote media size.
maxsz := config.GetMediaRemoteMaxSize()
maxsz := int64(config.GetMediaRemoteMaxSize()) // #nosec G115 -- Already validated.

// Recache media with prepared info,
// this will also update media in db.
return d.mediaManager.CacheMedia(
attach,
func(ctx context.Context) (io.ReadCloser, error) {
return tsport.DereferenceMedia(ctx, url, int64(maxsz))
return tsport.DereferenceMedia(ctx, url, maxsz)
},
), nil
},
Expand Down
4 changes: 2 additions & 2 deletions internal/httpclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,14 +340,14 @@ func (c *Client) do(r *Request) (rsp *http.Response, retry bool, err error) {

if u, _ := strconv.ParseUint(after, 10, 32); u != 0 {
// An integer no. of backoff seconds was provided.
r.backoff = time.Duration(u) * time.Second
r.backoff = time.Duration(u) * time.Second // #nosec G115 -- We clamp backoff below.
} else if at, _ := http.ParseTime(after); !at.Before(now) {
// An HTTP formatted future date-time was provided.
r.backoff = at.Sub(now)
}

// Don't let their provided backoff exceed our max.
if max := baseBackoff * time.Duration(c.retries); //
if max := baseBackoff * time.Duration(c.retries); // #nosec G115 -- We control c.retries.
r.backoff > max {
r.backoff = max
}
Expand Down
4 changes: 2 additions & 2 deletions internal/media/ffmpeg.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,10 +556,10 @@ func (res *ffprobeResult) Process() (*result, error) {
if p := strings.SplitN(str, "/", 2); len(p) == 2 {
n, _ := strconv.ParseUint(p[0], 10, 32)
d, _ := strconv.ParseUint(p[1], 10, 32)
num, den = uint32(n), uint32(d)
num, den = uint32(n), uint32(d) // #nosec G115 -- ParseUint is configured to check
} else {
n, _ := strconv.ParseUint(p[0], 10, 32)
num = uint32(n)
num = uint32(n) // #nosec G115 -- ParseUint is configured to check
}

// Set final divised framerate.
Expand Down
40 changes: 20 additions & 20 deletions internal/media/imaging.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,9 +399,9 @@ func (s *scanner) scan(x1, y1, x2, y2 int, dst []uint8) {
g16 := uint16(s[1])
b16 := uint16(s[2])
a16 := uint16(a)
d[0] = uint8(r16 * 0xff / a16)
d[1] = uint8(g16 * 0xff / a16)
d[2] = uint8(b16 * 0xff / a16)
d[0] = uint8(r16 * 0xff / a16) // #nosec G115 -- Overflow desired.
d[1] = uint8(g16 * 0xff / a16) // #nosec G115 -- Overflow desired.
d[2] = uint8(b16 * 0xff / a16) // #nosec G115 -- Overflow desired.
d[3] = a
}
j += 4
Expand Down Expand Up @@ -431,9 +431,9 @@ func (s *scanner) scan(x1, y1, x2, y2 int, dst []uint8) {
g32 := uint32(s[2])<<8 | uint32(s[3])
b32 := uint32(s[4])<<8 | uint32(s[5])
a32 := uint32(s[6])<<8 | uint32(s[7])
d[0] = uint8((r32 * 0xffff / a32) >> 8)
d[1] = uint8((g32 * 0xffff / a32) >> 8)
d[2] = uint8((b32 * 0xffff / a32) >> 8)
d[0] = uint8((r32 * 0xffff / a32) >> 8) // #nosec G115 -- Overflow desired.
d[1] = uint8((g32 * 0xffff / a32) >> 8) // #nosec G115 -- Overflow desired.
d[2] = uint8((b32 * 0xffff / a32) >> 8) // #nosec G115 -- Overflow desired.
}
d[3] = a
j += 4
Expand Down Expand Up @@ -509,30 +509,30 @@ func (s *scanner) scan(x1, y1, x2, y2 int, dst []uint8) {
cr1 := int32(img.Cr[ic]) - 128

r := yy1 + 91881*cr1
if uint32(r)&0xff000000 == 0 {
if uint32(r)&0xff000000 == 0 { //nolint:gosec
r >>= 16
} else {
r = ^(r >> 31)
}

g := yy1 - 22554*cb1 - 46802*cr1
if uint32(g)&0xff000000 == 0 {
if uint32(g)&0xff000000 == 0 { //nolint:gosec
g >>= 16
} else {
g = ^(g >> 31)
}

b := yy1 + 116130*cb1
if uint32(b)&0xff000000 == 0 {
if uint32(b)&0xff000000 == 0 { //nolint:gosec
b >>= 16
} else {
b = ^(b >> 31)
}

d := dst[j : j+4 : j+4]
d[0] = uint8(r)
d[1] = uint8(g)
d[2] = uint8(b)
d[0] = uint8(r) // #nosec G115 -- Overflow desired.
d[1] = uint8(g) // #nosec G115 -- Overflow desired.
d[2] = uint8(b) // #nosec G115 -- Overflow desired.
d[3] = 0xff

iy++
Expand Down Expand Up @@ -569,20 +569,20 @@ func (s *scanner) scan(x1, y1, x2, y2 int, dst []uint8) {
d := dst[j : j+4 : j+4]
switch a16 {
case 0xffff:
d[0] = uint8(r16 >> 8)
d[1] = uint8(g16 >> 8)
d[2] = uint8(b16 >> 8)
d[0] = uint8(r16 >> 8) // #nosec G115 -- Overflow desired.
d[1] = uint8(g16 >> 8) // #nosec G115 -- Overflow desired.
d[2] = uint8(b16 >> 8) // #nosec G115 -- Overflow desired.
d[3] = 0xff
case 0:
d[0] = 0
d[1] = 0
d[2] = 0
d[3] = 0
default:
d[0] = uint8(((r16 * 0xffff) / a16) >> 8)
d[1] = uint8(((g16 * 0xffff) / a16) >> 8)
d[2] = uint8(((b16 * 0xffff) / a16) >> 8)
d[3] = uint8(a16 >> 8)
d[0] = uint8(((r16 * 0xffff) / a16) >> 8) // #nosec G115 -- Overflow desired.
d[1] = uint8(((g16 * 0xffff) / a16) >> 8) // #nosec G115 -- Overflow desired.
d[2] = uint8(((b16 * 0xffff) / a16) >> 8) // #nosec G115 -- Overflow desired.
d[3] = uint8(a16 >> 8) // #nosec G115 -- Overflow desired.
}
j += 4
}
Expand Down Expand Up @@ -617,7 +617,7 @@ func clampFloat(x float64) uint8 {
return 255
}
if v > 0 {
return uint8(v)
return uint8(v) // #nosec G115 -- Just checked.
}
return 0
}
7 changes: 3 additions & 4 deletions internal/media/refetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ func (m *Manager) RefetchEmojis(ctx context.Context, domain string, dereferenceM
refetchIDs []string
)

// Get max supported remote emoji media size.
maxsz := config.GetMediaEmojiRemoteMaxSize()

// page through emojis 20 at a time, looking for those with missing images
for {
// Fetch next block of emojis from database
Expand Down Expand Up @@ -111,8 +108,10 @@ func (m *Manager) RefetchEmojis(ctx context.Context, domain string, dereferenceM
continue
}

// Get max supported remote emoji media size.
maxsz := int64(config.GetMediaEmojiRemoteMaxSize()) // #nosec G115 -- Already validated.
dataFunc := func(ctx context.Context) (reader io.ReadCloser, err error) {
return dereferenceMedia(ctx, emojiImageIRI, int64(maxsz))
return dereferenceMedia(ctx, emojiImageIRI, maxsz)
}

processingEmoji, err := m.UpdateEmoji(ctx, emoji, dataFunc, AdditionalEmojiInfo{
Expand Down
2 changes: 1 addition & 1 deletion internal/media/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func drainToTmp(rc io.ReadCloser) (string, error) {
// Check to see if limit was reached,
// (produces more useful error messages).
if lr != nil && lr.N <= 0 {
err := fmt.Errorf("reached read limit %s", bytesize.Size(limit))
err := fmt.Errorf("reached read limit %s", bytesize.Size(limit)) // #nosec G115 -- Just logging
return path, gtserror.SetLimitReached(err)
}

Expand Down
2 changes: 1 addition & 1 deletion internal/middleware/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func Logger(logClientIP bool) gin.HandlerFunc {
}

// Generate a nicer looking bytecount
size := bytesize.Size(c.Writer.Size())
size := bytesize.Size(c.Writer.Size()) // #nosec G115 -- Just logging

// Finally, write log entry with status text + body size.
l.Logf(lvl, "%s: wrote %s", statusText, size)
Expand Down
2 changes: 1 addition & 1 deletion internal/middleware/requestid.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func NewRequestID() string {
b := make([]byte, 12)

// Get current time in milliseconds.
ms := uint64(time.Now().UnixMilli())
ms := uint64(time.Now().UnixMilli()) // #nosec G115 -- Pre-1970 clock?

// Store binary time data in byte buffer.
binary.LittleEndian.PutUint64(b[0:8], ms)
Expand Down
6 changes: 5 additions & 1 deletion internal/middleware/throttling.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,16 @@ func Throttle(cpuMultiplier int, retryAfter time.Duration) gin.HandlerFunc {
return func(c *gin.Context) {}
}

if retryAfter < 0 {
retryAfter = 0
}

var (
limit = runtime.GOMAXPROCS(0) * cpuMultiplier
queueLimit = limit * cpuMultiplier
tokens = make(chan token, limit)
requestCount = atomic.Int64{}
retryAfterStr = strconv.FormatUint(uint64(retryAfter/time.Second), 10)
retryAfterStr = strconv.FormatUint(uint64(retryAfter/time.Second), 10) // #nosec G115 -- Checked right above
)

// prefill token channel
Expand Down
10 changes: 6 additions & 4 deletions internal/processing/account/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,9 +463,10 @@ func (p *Processor) UpdateAvatar(
) {
// Get maximum supported local media size.
maxsz := config.GetMediaLocalMaxSize()
maxszInt64 := int64(maxsz) // #nosec G115 -- Already validated.

// Ensure media within size bounds.
if avatar.Size > int64(maxsz) {
if avatar.Size > maxszInt64 {
text := fmt.Sprintf("media exceeds configured max size: %s", maxsz)
return nil, gtserror.NewErrorBadRequest(errors.New(text), text)
}
Expand All @@ -478,7 +479,7 @@ func (p *Processor) UpdateAvatar(
}

// Wrap the multipart file reader to ensure is limited to max.
rc, _, _ := iotools.UpdateReadCloserLimit(mpfile, int64(maxsz))
rc, _, _ := iotools.UpdateReadCloserLimit(mpfile, maxszInt64)

// Write to instance storage.
return p.c.StoreLocalMedia(ctx,
Expand Down Expand Up @@ -508,9 +509,10 @@ func (p *Processor) UpdateHeader(
) {
// Get maximum supported local media size.
maxsz := config.GetMediaLocalMaxSize()
maxszInt64 := int64(maxsz) // #nosec G115 -- Already validated.

// Ensure media within size bounds.
if header.Size > int64(maxsz) {
if header.Size > maxszInt64 {
text := fmt.Sprintf("media exceeds configured max size: %s", maxsz)
return nil, gtserror.NewErrorBadRequest(errors.New(text), text)
}
Expand All @@ -523,7 +525,7 @@ func (p *Processor) UpdateHeader(
}

// Wrap the multipart file reader to ensure is limited to max.
rc, _, _ := iotools.UpdateReadCloserLimit(mpfile, int64(maxsz))
rc, _, _ := iotools.UpdateReadCloserLimit(mpfile, maxszInt64)

// Write to instance storage.
return p.c.StoreLocalMedia(ctx,
Expand Down
Loading

0 comments on commit a48cce8

Please sign in to comment.