Skip to content
Open
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
4 changes: 2 additions & 2 deletions caddytest/integration/listener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package integration
import (
"bytes"
"fmt"
"math/rand"
"math/rand/v2"
"net"
"net/http"
"strings"
Expand Down Expand Up @@ -54,7 +54,7 @@ func TestHTTPRedirectWrapperWithLargeUpload(t *testing.T) {
const uploadSize = (1024 * 1024) + 1 // 1 MB + 1 byte
// 1 more than an MB
body := make([]byte, uploadSize)
rand.New(rand.NewSource(0)).Read(body)
rand.NewChaCha8([32]byte{}).Read(body)

Choose a reason for hiding this comment

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

🟡 MEDIUM - Zero-initialized random seed produces deterministic test data
Agent: go

Category: quality

Description:
Using 'rand.NewChaCha8([32]byte{})' with a zero seed produces identical test data on every run. While this may be intentional for reproducibility, the test name suggests testing with varied data.

Suggestion:
If determinism is intended, add a comment explaining the choice. If varied data is desired, use a non-zero or time-based seed.

Confidence: 65%
Rule: qual_misleading_names_go
Review ID: 9bac5f9f-38e7-46a1-add2-7c064de630b1
Rate it 👍 or 👎 to improve future reviews | Powered by diffray


tester := setupListenerWrapperTest(t, func(writer http.ResponseWriter, request *http.Request) {
buf := new(bytes.Buffer)
Expand Down
4 changes: 2 additions & 2 deletions modules/caddyhttp/caddyauth/basicauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"encoding/hex"
"encoding/json"
"fmt"
weakrand "math/rand"
weakrand "math/rand/v2"
"net/http"
"strings"
"sync"
Expand Down Expand Up @@ -244,7 +244,7 @@ func (c *Cache) makeRoom() {
// strategy; generating random numbers is cheap and

Choose a reason for hiding this comment

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

🔴 CRITICAL - Off-by-one error in cache eviction loop condition
Agent: go

Category: bug

Description:
Loop condition 'deleted <= numToDelete' causes deletion of numToDelete+1 items instead of numToDelete. With numToDelete=10, the loop runs for deleted values 0 through 10 inclusive (11 iterations), deleting 11 items instead of 10.

Suggestion:
Change '<=' to '<' on line 240: 'for deleted := 0; deleted < numToDelete; deleted++'

Confidence: 95%
Rule: bug_slice_bounds_go
Review ID: 9bac5f9f-38e7-46a1-add2-7c064de630b1
Rate it 👍 or 👎 to improve future reviews | Powered by diffray

// ensures a much better distribution.
//nolint:gosec
rnd := weakrand.Intn(len(c.cache))
rnd := weakrand.IntN(len(c.cache))
i := 0
for key := range c.cache {
if i == rnd {
Comment on lines +247 to 250

Choose a reason for hiding this comment

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

🟠 HIGH - Inefficient O(n*m) cache eviction with repeated map traversals
Agent: go

Category: performance

Description:
The makeRoom() function performs numToDelete iterations, each generating a random index and iterating through the entire map to find and delete that element. This creates O(n*m) complexity where n is cache size and m is items to delete.

Suggestion:
Optimize by collecting all cache keys into a slice once, shuffling, and deleting the first numToDelete keys in a single pass for O(n) complexity.

Confidence: 85%
Rule: go_use_defer_to_release_resources
Review ID: 9bac5f9f-38e7-46a1-add2-7c064de630b1
Rate it 👍 or 👎 to improve future reviews | Powered by diffray

Expand Down
4 changes: 2 additions & 2 deletions modules/caddyhttp/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ package caddyhttp
import (
"errors"
"fmt"
weakrand "math/rand"
weakrand "math/rand/v2"
"path"
"runtime"
"strings"
Expand Down Expand Up @@ -98,7 +98,7 @@ func randString(n int, sameCase bool) string {
b := make([]byte, n)
for i := range b {
//nolint:gosec
b[i] = dict[weakrand.Int63()%int64(len(dict))]
b[i] = dict[weakrand.IntN(len(dict))]
}
return string(b)
}
Expand Down
4 changes: 2 additions & 2 deletions modules/caddyhttp/fileserver/staticfiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"fmt"
"io"
"io/fs"
weakrand "math/rand"
weakrand "math/rand/v2"
"mime"
"net/http"
"os"
Expand Down Expand Up @@ -601,7 +601,7 @@ func (fsrv *FileServer) openFile(fileSystem fs.FS, filename string, w http.Respo
// maybe the server is under load and ran out of file descriptors?
// have client wait arbitrary seconds to help prevent a stampede
//nolint:gosec
backoff := weakrand.Intn(maxBackoff-minBackoff) + minBackoff
backoff := weakrand.IntN(maxBackoff-minBackoff) + minBackoff
w.Header().Set("Retry-After", strconv.Itoa(backoff))
if c := fsrv.logger.Check(zapcore.DebugLevel, "retry after backoff"); c != nil {
c.Write(zap.String("filename", filename), zap.Int("backoff", backoff), zap.Error(err))
Expand Down
4 changes: 2 additions & 2 deletions modules/caddyhttp/reverseproxy/fastcgi/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"fmt"
"io"
"log"
"math/rand"
"math/rand/v2"
"net"
"net/http"
"net/http/fcgi"
Expand Down Expand Up @@ -197,7 +197,7 @@ func generateRandFile(size int) (p string, m string) {
h := md5.New()
for i := 0; i < size/16; i++ {
buf := make([]byte, 16)
binary.PutVarint(buf, rand.Int63())
binary.PutVarint(buf, rand.Int64())
if _, err := fo.Write(buf); err != nil {
log.Printf("[ERROR] failed to write buffer: %v\n", err)
}
Expand Down
4 changes: 2 additions & 2 deletions modules/caddyhttp/reverseproxy/httptransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
weakrand "math/rand"
weakrand "math/rand/v2"
"net"
"net/http"
"net/url"
Expand Down Expand Up @@ -265,7 +265,7 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e
PreferGo: true,
Dial: func(ctx context.Context, _, _ string) (net.Conn, error) {
//nolint:gosec
addr := h.Resolver.netAddrs[weakrand.Intn(len(h.Resolver.netAddrs))]
addr := h.Resolver.netAddrs[weakrand.IntN(len(h.Resolver.netAddrs))]
return d.DialContext(ctx, addr.Network, addr.JoinHostPort(0))
},
}
Expand Down
10 changes: 5 additions & 5 deletions modules/caddyhttp/reverseproxy/selectionpolicies.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"encoding/hex"
"encoding/json"
"fmt"
weakrand "math/rand"
weakrand "math/rand/v2"
"net"
"net/http"
"strconv"
Expand Down Expand Up @@ -225,7 +225,7 @@ func (r RandomChoiceSelection) Select(pool UpstreamPool, _ *http.Request, _ http
if !upstream.Available() {
continue
}
j := weakrand.Intn(i + 1) //nolint:gosec
j := weakrand.IntN(i + 1) //nolint:gosec
if j < k {
choices[j] = upstream
}
Expand Down Expand Up @@ -274,7 +274,7 @@ func (LeastConnSelection) Select(pool UpstreamPool, _ *http.Request, _ http.Resp
// sample: https://en.wikipedia.org/wiki/Reservoir_sampling
if numReqs == leastReqs {
count++
if count == 1 || (weakrand.Int()%count) == 0 { //nolint:gosec
if count == 1 || weakrand.IntN(count) == 0 { //nolint:gosec
bestHost = host
}
}
Expand Down Expand Up @@ -788,7 +788,7 @@ func selectRandomHost(pool []*Upstream) *Upstream {
// upstream will always be chosen if there is at
// least one available
count++
if (weakrand.Int() % count) == 0 { //nolint:gosec
if weakrand.IntN(count) == 0 { //nolint:gosec
randomHost = upstream
}
}
Expand Down Expand Up @@ -827,7 +827,7 @@ func leastRequests(upstreams []*Upstream) *Upstream {
if len(best) == 1 {
return best[0]
}
return best[weakrand.Intn(len(best))] //nolint:gosec
return best[weakrand.IntN(len(best))] //nolint:gosec
}

// hostByHashing returns an available host from pool based on a hashable string s.
Expand Down
2 changes: 1 addition & 1 deletion modules/caddyhttp/reverseproxy/streaming.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"errors"
"fmt"
"io"
weakrand "math/rand"
weakrand "math/rand/v2"
"mime"
"net/http"
"sync"
Expand Down
6 changes: 3 additions & 3 deletions modules/caddyhttp/reverseproxy/upstreams.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
weakrand "math/rand"
weakrand "math/rand/v2"
"net"
"net/http"
"strconv"
Expand Down Expand Up @@ -102,7 +102,7 @@ func (su *SRVUpstreams) Provision(ctx caddy.Context) error {
PreferGo: true,
Dial: func(ctx context.Context, _, _ string) (net.Conn, error) {
//nolint:gosec
addr := su.Resolver.netAddrs[weakrand.Intn(len(su.Resolver.netAddrs))]
addr := su.Resolver.netAddrs[weakrand.IntN(len(su.Resolver.netAddrs))]
return d.DialContext(ctx, addr.Network, addr.JoinHostPort(0))
},
}
Expand Down Expand Up @@ -322,7 +322,7 @@ func (au *AUpstreams) Provision(ctx caddy.Context) error {
PreferGo: true,
Dial: func(ctx context.Context, _, _ string) (net.Conn, error) {
//nolint:gosec
addr := au.Resolver.netAddrs[weakrand.Intn(len(au.Resolver.netAddrs))]
addr := au.Resolver.netAddrs[weakrand.IntN(len(au.Resolver.netAddrs))]
return d.DialContext(ctx, addr.Network, addr.JoinHostPort(0))
},
}
Expand Down
4 changes: 2 additions & 2 deletions modules/caddypki/acmeserver/acmeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ package acmeserver
import (
"context"
"fmt"
weakrand "math/rand"
weakrand "math/rand/v2"
"net"
"net/http"
"os"
Expand Down Expand Up @@ -307,7 +307,7 @@ func (ash Handler) makeClient() (acme.Client, error) {
PreferGo: true,
Dial: func(ctx context.Context, network, address string) (net.Conn, error) {
//nolint:gosec
addr := ash.resolvers[weakrand.Intn(len(ash.resolvers))]
addr := ash.resolvers[weakrand.IntN(len(ash.resolvers))]
return dialer.DialContext(ctx, addr.Network, addr.JoinHostPort(0))
},
}
Expand Down
Loading