From bd17eb205d6ac464c64eb888a6f4b57445b6c59c Mon Sep 17 00:00:00 2001 From: Dave Henderson Date: Sun, 22 Nov 2020 16:50:29 -0500 Subject: ci: Use golangci's github action for linting (#3794) * ci: Use golangci's github action for linting Signed-off-by: Dave Henderson * Fix most of the staticcheck lint errors Signed-off-by: Dave Henderson * Fix the prealloc lint errors Signed-off-by: Dave Henderson * Fix the misspell lint errors Signed-off-by: Dave Henderson * Fix the varcheck lint errors Signed-off-by: Dave Henderson * Fix the errcheck lint errors Signed-off-by: Dave Henderson * Fix the bodyclose lint errors Signed-off-by: Dave Henderson * Fix the deadcode lint errors Signed-off-by: Dave Henderson * Fix the unused lint errors Signed-off-by: Dave Henderson * Fix the gosec lint errors Signed-off-by: Dave Henderson * Fix the gosimple lint errors Signed-off-by: Dave Henderson * Fix the ineffassign lint errors Signed-off-by: Dave Henderson * Fix the staticcheck lint errors Signed-off-by: Dave Henderson * Revert the misspell change, use a neutral English Signed-off-by: Dave Henderson * Remove broken golangci-lint CI job Signed-off-by: Dave Henderson * Re-add errantly-removed weakrand initialization Signed-off-by: Dave Henderson * don't break the loop and return * Removing extra handling for null rootKey * unignore RegisterModule/RegisterAdapter Co-authored-by: Mohammed Al Sahaf * single-line log message Co-authored-by: Matt Holt * Fix lint after a1808b0dbf209c615e438a496d257ce5e3acdce2 was merged Signed-off-by: Dave Henderson * Revert ticker change, ignore it instead Signed-off-by: Dave Henderson * Ignore some of the write errors Signed-off-by: Dave Henderson * Remove blank line Signed-off-by: Dave Henderson * Use lifetime Signed-off-by: Dave Henderson * close immediately Co-authored-by: Matt Holt * Preallocate configVals Signed-off-by: Dave Henderson * Update modules/caddytls/distributedstek/distributedstek.go Co-authored-by: Mohammed Al Sahaf Co-authored-by: Matt Holt --- modules/caddyhttp/app.go | 2 ++ modules/caddyhttp/autohttps.go | 5 ++- modules/caddyhttp/caddyauth/basicauth.go | 1 + modules/caddyhttp/caddyhttp.go | 4 --- modules/caddyhttp/encode/encode.go | 2 +- modules/caddyhttp/errors.go | 10 ++++-- modules/caddyhttp/fileserver/browse.go | 2 +- modules/caddyhttp/fileserver/browselisting.go | 20 +++++------ modules/caddyhttp/fileserver/browselisting_test.go | 40 ++++++++++++++++++++++ modules/caddyhttp/fileserver/staticfiles.go | 3 +- modules/caddyhttp/marshalers.go | 3 +- modules/caddyhttp/replacer.go | 3 +- modules/caddyhttp/reverseproxy/fastcgi/client.go | 7 ---- modules/caddyhttp/reverseproxy/healthchecks.go | 2 +- modules/caddyhttp/reverseproxy/httptransport.go | 1 + modules/caddyhttp/reverseproxy/reverseproxy.go | 29 +++------------- .../caddyhttp/reverseproxy/selectionpolicies.go | 2 +- modules/caddyhttp/reverseproxy/streaming.go | 11 ++++-- modules/caddyhttp/reverseproxy/streaming_test.go | 30 ++++++++++++++++ modules/caddyhttp/templates/tplcontext.go | 5 ++- 20 files changed, 120 insertions(+), 62 deletions(-) create mode 100644 modules/caddyhttp/fileserver/browselisting_test.go create mode 100644 modules/caddyhttp/reverseproxy/streaming_test.go (limited to 'modules/caddyhttp') diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index 43cc6f7..42e7725 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -363,6 +363,7 @@ func (app *App) Start() error { ErrorLog: serverLogger, }, } + //nolint:errcheck go h3srv.Serve(h3ln) app.h3servers = append(app.h3servers, h3srv) app.h3listeners = append(app.h3listeners, h3ln) @@ -391,6 +392,7 @@ func (app *App) Start() error { zap.Bool("tls", useTLS), ) + //nolint:errcheck go s.Serve(ln) app.servers = append(app.servers, s) } diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go index 805a37c..fff4c46 100644 --- a/modules/caddyhttp/autohttps.go +++ b/modules/caddyhttp/autohttps.go @@ -523,7 +523,10 @@ func (app *App) createAutomationPolicies(ctx caddy.Context, internalNames []stri // our base/catch-all policy - this will serve the // public-looking names as well as any other names // that don't match any other policy - app.tlsApp.AddAutomationPolicy(basePolicy) + err := app.tlsApp.AddAutomationPolicy(basePolicy) + if err != nil { + return err + } } else { // a base policy already existed; we might have // changed it, so re-provision it diff --git a/modules/caddyhttp/caddyauth/basicauth.go b/modules/caddyhttp/caddyauth/basicauth.go index f383199..33be70d 100644 --- a/modules/caddyhttp/caddyauth/basicauth.go +++ b/modules/caddyhttp/caddyauth/basicauth.go @@ -240,6 +240,7 @@ func (c *Cache) makeRoom() { // map with less code, this is a heavily skewed eviction // strategy; generating random numbers is cheap and // ensures a much better distribution. + //nolint:gosec rnd := weakrand.Intn(len(c.cache)) i := 0 for key := range c.cache { diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go index a7ac889..485afe0 100644 --- a/modules/caddyhttp/caddyhttp.go +++ b/modules/caddyhttp/caddyhttp.go @@ -18,18 +18,14 @@ import ( "bytes" "encoding/json" "io" - weakrand "math/rand" "net" "net/http" "strconv" - "time" "github.com/caddyserver/caddy/v2" ) func init() { - weakrand.Seed(time.Now().UnixNano()) - caddy.RegisterModule(tlsPlaceholderWrapper{}) } diff --git a/modules/caddyhttp/encode/encode.go b/modules/caddyhttp/encode/encode.go index 52205aa..e42eeed 100644 --- a/modules/caddyhttp/encode/encode.go +++ b/modules/caddyhttp/encode/encode.go @@ -262,7 +262,7 @@ func acceptedEncodings(r *http.Request) []string { return []string{} } - var prefs []encodingPreference + prefs := []encodingPreference{} for _, accepted := range strings.Split(acceptEncHeader, ",") { parts := strings.Split(accepted, ";") diff --git a/modules/caddyhttp/errors.go b/modules/caddyhttp/errors.go index 0593063..85dc3df 100644 --- a/modules/caddyhttp/errors.go +++ b/modules/caddyhttp/errors.go @@ -16,14 +16,19 @@ package caddyhttp import ( "fmt" - mathrand "math/rand" + weakrand "math/rand" "path" "runtime" "strings" + "time" "github.com/caddyserver/caddy/v2" ) +func init() { + weakrand.Seed(time.Now().UnixNano()) +} + // Error is a convenient way for a Handler to populate the // essential fields of a HandlerError. If err is itself a // HandlerError, then any essential fields that are not @@ -92,7 +97,8 @@ func randString(n int, sameCase bool) string { } b := make([]byte, n) for i := range b { - b[i] = dict[mathrand.Int63()%int64(len(dict))] + //nolint:gosec + b[i] = dict[weakrand.Int63()%int64(len(dict))] } return string(b) } diff --git a/modules/caddyhttp/fileserver/browse.go b/modules/caddyhttp/fileserver/browse.go index 3fecad9..cd41ea5 100644 --- a/modules/caddyhttp/fileserver/browse.go +++ b/modules/caddyhttp/fileserver/browse.go @@ -82,7 +82,7 @@ func (fsrv *FileServer) serveBrowse(dirPath string, w http.ResponseWriter, r *ht w.Header().Set("Content-Type", "text/html; charset=utf-8") } - buf.WriteTo(w) + _, _ = buf.WriteTo(w) return nil } diff --git a/modules/caddyhttp/fileserver/browselisting.go b/modules/caddyhttp/fileserver/browselisting.go index 79944f9..f3f85a3 100644 --- a/modules/caddyhttp/fileserver/browselisting.go +++ b/modules/caddyhttp/fileserver/browselisting.go @@ -30,10 +30,8 @@ import ( func (fsrv *FileServer) directoryListing(files []os.FileInfo, canGoUp bool, urlPath string, repl *caddy.Replacer) browseListing { filesToHide := fsrv.transformHidePaths(repl) - var ( - fileInfos []fileInfo - dirCount, fileCount int - ) + var dirCount, fileCount int + fileInfos := []fileInfo{} for _, f := range files { name := f.Name() @@ -109,10 +107,8 @@ type browseListing struct { // Breadcrumbs returns l.Path where every element maps // the link to the text to display. func (l browseListing) Breadcrumbs() []crumb { - var result []crumb - if len(l.Path) == 0 { - return result + return []crumb{} } // skip trailing slash @@ -122,13 +118,13 @@ func (l browseListing) Breadcrumbs() []crumb { } parts := strings.Split(lpath, "/") - for i := range parts { - txt := parts[i] - if i == 0 && parts[i] == "" { - txt = "/" + result := make([]crumb, len(parts)) + for i, p := range parts { + if i == 0 && p == "" { + p = "/" } lnk := strings.Repeat("../", len(parts)-i-1) - result = append(result, crumb{Link: lnk, Text: txt}) + result[i] = crumb{Link: lnk, Text: p} } return result diff --git a/modules/caddyhttp/fileserver/browselisting_test.go b/modules/caddyhttp/fileserver/browselisting_test.go new file mode 100644 index 0000000..6d58b7e --- /dev/null +++ b/modules/caddyhttp/fileserver/browselisting_test.go @@ -0,0 +1,40 @@ +package fileserver + +import ( + "testing" +) + +func TestBreadcrumbs(t *testing.T) { + testdata := []struct { + path string + expected []crumb + }{ + {"", []crumb{}}, + {"/", []crumb{{Text: "/"}}}, + {"foo/bar/baz", []crumb{ + {Link: "../../", Text: "foo"}, + {Link: "../", Text: "bar"}, + {Link: "", Text: "baz"}, + }}, + {"/qux/quux/corge/", []crumb{ + {Link: "../../../", Text: "/"}, + {Link: "../../", Text: "qux"}, + {Link: "../", Text: "quux"}, + {Link: "", Text: "corge"}, + }}, + } + + for _, d := range testdata { + l := browseListing{Path: d.path} + actual := l.Breadcrumbs() + if len(actual) != len(d.expected) { + t.Errorf("wrong size output, got %d elements but expected %d", len(actual), len(d.expected)) + continue + } + for i, c := range actual { + if c != d.expected[i] { + t.Errorf("got %#v but expected %#v at index %d", c, d.expected[i], i) + } + } + } +} diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index b9f2b23..caad0b4 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -249,7 +249,7 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c } w.WriteHeader(statusCode) if r.Method != http.MethodHead { - io.Copy(w, file) + _, _ = io.Copy(w, file) } return nil } @@ -278,6 +278,7 @@ func (fsrv *FileServer) openFile(filename string, w http.ResponseWriter) (*os.Fi } // 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 w.Header().Set("Retry-After", strconv.Itoa(backoff)) return nil, caddyhttp.Error(http.StatusServiceUnavailable, err) diff --git a/modules/caddyhttp/marshalers.go b/modules/caddyhttp/marshalers.go index 9e8bb9f..8001bd8 100644 --- a/modules/caddyhttp/marshalers.go +++ b/modules/caddyhttp/marshalers.go @@ -75,7 +75,8 @@ func (t LoggableTLSConnState) MarshalLogObject(enc zapcore.ObjectEncoder) error enc.AddUint16("version", t.Version) enc.AddUint16("cipher_suite", t.CipherSuite) enc.AddString("proto", t.NegotiatedProtocol) - enc.AddBool("proto_mutual", t.NegotiatedProtocolIsMutual) + // NegotiatedProtocolIsMutual is deprecated - it's always true + enc.AddBool("proto_mutual", true) enc.AddString("server_name", t.ServerName) if len(t.PeerCertificates) > 0 { enc.AddString("client_common_name", t.PeerCertificates[0].Subject.CommonName) diff --git a/modules/caddyhttp/replacer.go b/modules/caddyhttp/replacer.go index 3993433..5a0efce 100644 --- a/modules/caddyhttp/replacer.go +++ b/modules/caddyhttp/replacer.go @@ -362,7 +362,8 @@ func getReqTLSReplacement(req *http.Request, key string) (interface{}, bool) { case "proto": return req.TLS.NegotiatedProtocol, true case "proto_mutual": - return req.TLS.NegotiatedProtocolIsMutual, true + // req.TLS.NegotiatedProtocolIsMutual is deprecated - it's always true. + return true, true case "server_name": return req.TLS.ServerName, true } diff --git a/modules/caddyhttp/reverseproxy/fastcgi/client.go b/modules/caddyhttp/reverseproxy/fastcgi/client.go index ae0de00..94df0c7 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/client.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/client.go @@ -242,13 +242,6 @@ func (c *FCGIClient) writeBeginRequest(role uint16, flags uint8) error { return c.writeRecord(BeginRequest, b[:]) } -func (c *FCGIClient) writeEndRequest(appStatus int, protocolStatus uint8) error { - b := make([]byte, 8) - binary.BigEndian.PutUint32(b, uint32(appStatus)) - b[4] = protocolStatus - return c.writeRecord(EndRequest, b) -} - func (c *FCGIClient) writePairs(recType uint8, pairs map[string]string) error { w := newWriter(c, recType) b := make([]byte, 8) diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index 4e93320..285834b 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -263,7 +263,7 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, host H } defer func() { // drain any remaining body so connection could be re-used - io.Copy(ioutil.Discard, body) + _, _ = io.Copy(ioutil.Discard, body) resp.Body.Close() }() diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index d2a9951..61e9054 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -173,6 +173,7 @@ func (h *HTTPTransport) NewTransport(ctx caddy.Context) (*http.Transport, error) dialer.Resolver = &net.Resolver{ PreferGo: true, Dial: func(ctx context.Context, _, _ string) (net.Conn, error) { + //nolint:gosec addr := h.Resolver.netAddrs[weakrand.Intn(len(h.Resolver.netAddrs))] return d.DialContext(ctx, addr.Network, addr.JoinHostPort(0)) }, diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 7fc61ae..bed5289 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -314,7 +314,7 @@ func (h *Handler) Cleanup() error { // remove hosts from our config from the pool for _, upstream := range h.Upstreams { - hosts.Delete(upstream.String()) + _, _ = hosts.Delete(upstream.String()) } return nil @@ -339,7 +339,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht buf := bufPool.Get().(*bytes.Buffer) buf.Reset() defer bufPool.Put(buf) - io.Copy(buf, r.Body) + _, _ = io.Copy(buf, r.Body) r.Body.Close() r.Body = ioutil.NopCloser(buf) } @@ -518,7 +518,8 @@ func (h Handler) prepareRequest(req *http.Request) error { // (This method is mostly the beginning of what was borrowed from the net/http/httputil package in the // Go standard library which was used as the foundation.) func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, di DialInfo, next caddyhttp.Handler) error { - di.Upstream.Host.CountRequest(1) + _ = di.Upstream.Host.CountRequest(1) + //nolint:errcheck defer di.Upstream.Host.CountRequest(-1) // point the request to this upstream @@ -742,16 +743,6 @@ func copyHeader(dst, src http.Header) { } } -func cloneHeader(h http.Header) http.Header { - h2 := make(http.Header, len(h)) - for k, vv := range h { - vv2 := make([]string, len(vv)) - copy(vv2, vv) - h2[k] = vv2 - } - return h2 -} - func upgradeType(h http.Header) string { if !httpguts.HeaderValuesContainsToken(h["Connection"], "Upgrade") { return "" @@ -759,18 +750,6 @@ func upgradeType(h http.Header) string { return strings.ToLower(h.Get("Upgrade")) } -func singleJoiningSlash(a, b string) string { - aslash := strings.HasSuffix(a, "/") - bslash := strings.HasPrefix(b, "/") - switch { - case aslash && bslash: - return a + b[1:] - case !aslash && !bslash: - return a + "/" + b - } - return a + b -} - // removeConnectionHeaders removes hop-by-hop headers listed in the "Connection" header of h. // See RFC 7230, section 6.1 func removeConnectionHeaders(h http.Header) { diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index a1010f4..a391b2f 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -536,7 +536,7 @@ func hostByHashing(pool []*Upstream, s string) *Upstream { // hash calculates a fast hash based on s. func hash(s string) uint32 { h := fnv.New32a() - h.Write([]byte(s)) + _, _ = h.Write([]byte(s)) return h.Sum32() } diff --git a/modules/caddyhttp/reverseproxy/streaming.go b/modules/caddyhttp/reverseproxy/streaming.go index 4004b7a..f108a97 100644 --- a/modules/caddyhttp/reverseproxy/streaming.go +++ b/modules/caddyhttp/reverseproxy/streaming.go @@ -138,9 +138,9 @@ func (h Handler) copyResponse(dst io.Writer, src io.Reader, flushInterval time.D } } - buf := streamingBufPool.Get().([]byte) + buf := streamingBufPool.Get().(*[]byte) defer streamingBufPool.Put(buf) - _, err := h.copyBuffer(dst, src, buf) + _, err := h.copyBuffer(dst, src, *buf) return err } @@ -255,7 +255,12 @@ func (c switchProtocolCopier) copyToBackend(errc chan<- error) { var streamingBufPool = sync.Pool{ New: func() interface{} { - return make([]byte, defaultBufferSize) + // The Pool's New function should generally only return pointer + // types, since a pointer can be put into the return interface + // value without an allocation + // - (from the package docs) + b := make([]byte, defaultBufferSize) + return &b }, } diff --git a/modules/caddyhttp/reverseproxy/streaming_test.go b/modules/caddyhttp/reverseproxy/streaming_test.go new file mode 100644 index 0000000..4ed1f1e --- /dev/null +++ b/modules/caddyhttp/reverseproxy/streaming_test.go @@ -0,0 +1,30 @@ +package reverseproxy + +import ( + "bytes" + "strings" + "testing" +) + +func TestHandlerCopyResponse(t *testing.T) { + h := Handler{} + testdata := []string{ + "", + strings.Repeat("a", defaultBufferSize), + strings.Repeat("123456789 123456789 123456789 12", 3000), + } + dst := bytes.NewBuffer(nil) + + for _, d := range testdata { + src := bytes.NewBuffer([]byte(d)) + dst.Reset() + err := h.copyResponse(dst, src, 0) + if err != nil { + t.Errorf("failed with error: %v", err) + } + out := dst.String() + if out != d { + t.Errorf("bad read: got %q", out) + } + } +} diff --git a/modules/caddyhttp/templates/tplcontext.go b/modules/caddyhttp/templates/tplcontext.go index 7bc0ce7..8bdaeec 100644 --- a/modules/caddyhttp/templates/tplcontext.go +++ b/modules/caddyhttp/templates/tplcontext.go @@ -270,7 +270,10 @@ func (templateContext) funcMarkdown(input interface{}) (string, error) { buf.Reset() defer bufPool.Put(buf) - md.Convert([]byte(inputStr), buf) + err := md.Convert([]byte(inputStr), buf) + if err != nil { + return "", err + } return buf.String(), nil } -- cgit v1.2.3