Skip to content
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

Remove lint exceptions and fix remaining issues #1438

Merged
merged 5 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 0 additions & 2 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,3 @@ linters-settings:

issues:
exclude-use-default: false
exclude:
- "Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|os\\.Remove(All)?|.*printf?|os\\.(Un)?Setenv). is not checked"
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
Alex Cohn <alex@alexcohn.com>
Comodo CA Limited
Ed Maste <emaste@freebsd.org>
Elisha Silas <silaselisha66@gmail.com>
Fiaz Hossain <fiaz.hossain@salesforce.com>
Google LLC
Internet Security Research Group
Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Chris Kennelly <ckennelly@google.com> <ckennelly@ckennelly.com>
David Drysdale <drysdale@google.com>
Deyan Bektchiev <deyan.bektchiev@venafi.com> <deyan@bektchiev.net>
Ed Maste <emaste@freebsd.org>
Elisha Silas <silaselisha66@gmail.com>
Emilia Kasper <ekasper@google.com>
Eran Messeri <eranm@google.com> <eran.mes@gmail.com>
Fiaz Hossain <fiaz.hossain@salesforce.com>
Expand Down
12 changes: 10 additions & 2 deletions fixchain/chainfix/chainfix.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ func processChains(file string, fl *fixchain.FixAndLog) {
if err != nil {
log.Fatalf("Can't open %q: %s", file, err)
}
defer f.Close()
defer func() {
if err := f.Close(); err != nil {
log.Fatalf("Can't close file: %v", err)
}
}()

type Chain struct {
Chain [][]byte
Expand Down Expand Up @@ -84,7 +88,11 @@ func contentStore(baseDir string, subDir string, content []byte) {
if err != nil {
log.Fatalf("Can't create %q: %s", fn, err)
}
defer f.Close()
defer func() {
if err := f.Close(); err != nil {
log.Fatalf("Can't close file: %v", err)
}
}()
if _, err := f.Write(content); err != nil {
log.Fatalf("Can't write to %q: %v", fn, err)
}
Expand Down
14 changes: 12 additions & 2 deletions fixchain/roundtrip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ func (rt testRoundTripper) RoundTrip(request *http.Request) (*http.Response, err
}, nil
case "https://ct.googleapis.com/pilot/ct/v1/add-chain":
body, err := io.ReadAll(request.Body)
request.Body.Close()
if err := request.Body.Close(); err != nil {
errStr := fmt.Sprintf("#%d: Could not close request body: %s", rt.testIndex, err.Error())
rt.t.Error(errStr)
return nil, errors.New(errStr)
}

if err != nil {
errStr := fmt.Sprintf("#%d: Could not read request body: %s", rt.testIndex, err.Error())
rt.t.Error(errStr)
Expand Down Expand Up @@ -201,7 +206,12 @@ func (rt postTestRoundTripper) RoundTrip(request *http.Request) (*http.Response,

// Check Body
body, err := io.ReadAll(request.Body)
request.Body.Close()
if err := request.Body.Close(); err != nil {
errStr := fmt.Sprintf("#%d: Could not close request body: %s", rt.testIndex, err.Error())
rt.t.Error(errStr)
return nil, errors.New(errStr)
}

if err != nil {
errStr := fmt.Sprintf("#%d: Could not read request body: %s", rt.testIndex, err.Error())
rt.t.Error(errStr)
Expand Down
11 changes: 8 additions & 3 deletions fixchain/url_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ package fixchain
import (
"fmt"
"io"
"log"
"net/http"
"sync"
"sync/atomic"
"time"

"k8s.io/klog/v2"
)

type lockedCache struct {
Expand Down Expand Up @@ -68,7 +69,11 @@ func (u *urlCache) getURL(url string) ([]byte, error) {
atomic.AddUint32(&u.errors, 1)
return nil, err
}
defer c.Body.Close()
defer func() {
if err := c.Body.Close(); err != nil {
klog.Errorf("Operation to close response body failed: %v", err)
}
}()
// TODO(katjoyce): Add caching of permanent errors.
if c.StatusCode != 200 {
atomic.AddUint32(&u.badStatus, 1)
Expand All @@ -91,7 +96,7 @@ func newURLCache(c *http.Client, logStats bool) *urlCache {
t := time.NewTicker(time.Second)
go func() {
for range t.C {
log.Printf("url cache: %d hits, %d misses, %d errors, "+
klog.Infof("url cache: %d hits, %d misses, %d errors, "+
"%d bad status, %d read fail, %d cached", u.hit,
u.miss, u.errors, u.badStatus, u.readFail,
len(u.cache.m))
Expand Down
13 changes: 11 additions & 2 deletions internal/witness/client/http/witness_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"os"

wit_api "github.com/google/certificate-transparency-go/internal/witness/api"
"k8s.io/klog/v2"
)

// ErrSTHTooOld is returned if the STH passed to Update needs to be updated.
Expand All @@ -51,7 +52,11 @@ func (w Witness) GetLatestSTH(ctx context.Context, logID string) ([]byte, error)
if err != nil {
return nil, fmt.Errorf("failed to do http request: %v", err)
}
defer resp.Body.Close()
defer func() {
if err := resp.Body.Close(); err != nil {
klog.Errorf("Failed to close response body: %v", err)
}
}()
if resp.StatusCode == 404 {
return nil, os.ErrNotExist
} else if resp.StatusCode != 200 {
Expand Down Expand Up @@ -87,7 +92,11 @@ func (w Witness) Update(ctx context.Context, logID string, sth []byte, proof [][
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Redirections#permanent_redirections
return nil, fmt.Errorf("PUT request to %q was converted to %s request to %q", u.String(), resp.Request.Method, resp.Request.URL)
}
defer resp.Body.Close()
defer func() {
if err := resp.Body.Close(); err != nil {
klog.Errorf("Failed to close response body: %v", err)
}
}()
body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, fmt.Errorf("failed to read body: %v", err)
Expand Down
6 changes: 5 additions & 1 deletion internal/witness/cmd/client/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,11 @@ func readHTTP(u *url.URL) ([]byte, error) {
if err != nil {
return nil, err
}
defer resp.Body.Close()
defer func() {
if err := resp.Body.Close(); err != nil {
klog.Errorf("failed to close response body %v", err)
}
}()
return io.ReadAll(resp.Body)
}

Expand Down
6 changes: 5 additions & 1 deletion internal/witness/cmd/feeder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,11 @@ func readHTTP(u *url.URL) ([]byte, error) {
if err != nil {
return nil, err
}
defer resp.Body.Close()
defer func() {
if err := resp.Body.Close(); err != nil {
klog.Errorf("Failed to close response body: %v", err)
}
}()
return io.ReadAll(resp.Body)
}

Expand Down
6 changes: 5 additions & 1 deletion internal/witness/cmd/witness/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ func readHTTP(u *url.URL) ([]byte, error) {
if err != nil {
return nil, err
}
defer resp.Body.Close()
defer func() {
if err := resp.Body.Close(); err != nil {
klog.Errorf("Operation to close response body failed: %v", err)
}
}()
return io.ReadAll(resp.Body)
}

Expand Down
7 changes: 5 additions & 2 deletions internal/witness/cmd/witness/internal/witness/witness.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,11 @@ func (w *Witness) GetLogs() ([]string, error) {
if err != nil {
return nil, err
}
defer rows.Close()

defer func() {
if err := rows.Close(); err != nil {
klog.Errorf("Operation to close rows failed: %v", err)
}
}()
var logs []string
for rows.Next() {
var logID string
Expand Down
8 changes: 6 additions & 2 deletions jsonclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,9 @@ func (c *JSONClient) GetAndParse(ctx context.Context, path string, params map[st

// Read everything now so http.Client can reuse the connection.
body, err := io.ReadAll(httpRsp.Body)
httpRsp.Body.Close()
if err := httpRsp.Body.Close(); err != nil {
return nil, nil, err
}
if err != nil {
return nil, nil, RspError{Err: fmt.Errorf("failed to read response body: %v", err), StatusCode: httpRsp.StatusCode, Body: body}
}
Expand Down Expand Up @@ -240,7 +242,9 @@ func (c *JSONClient) PostAndParse(ctx context.Context, path string, req, rsp int
var body []byte
if httpRsp != nil {
body, err = io.ReadAll(httpRsp.Body)
httpRsp.Body.Close()
if err := httpRsp.Body.Close(); err != nil {
return nil, nil, err
}
}
if err != nil {
if httpRsp != nil {
Expand Down
16 changes: 12 additions & 4 deletions jsonclient/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,13 @@ func MockServer(t *testing.T, failCount int, retryAfter int) *httptest.Server {
decoder := json.NewDecoder(r.Body)
err := decoder.Decode(&s)
if err != nil {
panic("Failed to decode: " + err.Error())
t.Fatalf("Failed to decode: " + err.Error())
}
defer r.Body.Close()
defer func() {
if err := r.Body.Close(); err != nil {
t.Fatalf("Failed to close request body: " + err.Error())
}
}()
}
fmt.Fprintf(w, `{"tree_size": %d, "timestamp": %d, "data": "%s"}`, s.TreeSize, s.Timestamp, s.Data)
case "/error":
Expand All @@ -158,9 +162,13 @@ func MockServer(t *testing.T, failCount int, retryAfter int) *httptest.Server {
decoder := json.NewDecoder(r.Body)
err := decoder.Decode(&params)
if err != nil {
panic("Failed to decode: " + err.Error())
t.Fatalf("Failed to decode: " + err.Error())
}
defer r.Body.Close()
defer func() {
if err := r.Body.Close(); err != nil {
t.Fatalf("Failed to close request body: " + err.Error())
}
}()
}
http.Error(w, "error page", params.RespCode)
case "/malformed":
Expand Down
6 changes: 5 additions & 1 deletion preload/preloader/preloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,11 @@ func main() {
if err != nil {
klog.Exitf("Failed to create SCT file: %v", err)
}
defer sctFile.Close()
defer func() {
if err := sctFile.Close(); err != nil {
klog.Exitf("Failed to close SCT file: %v", err)
}
}()
sctFileWriter = sctFile
} else {
sctFileWriter = io.Discard
Expand Down
6 changes: 5 additions & 1 deletion submission/hammer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,11 @@ func main() {
if err != nil {
log.Fatalf("http.Post(%s)=(_,%q); want (_,nil)", url, err)
}
defer resp.Body.Close()
defer func() {
if err := resp.Body.Close(); err != nil {
log.Fatalf("Unable to close response body: %v", err)
}
}()
var scts submission.SCTBatch
err = json.NewDecoder(resp.Body).Decode(&scts)
if err != nil {
Expand Down
12 changes: 10 additions & 2 deletions submission/loglist_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ func TestFirstRefresh(t *testing.T) {
if err != nil {
t.Fatalf("createTempFile(%q) = (_, %q), want (_, nil)", testdata.SampleLogList3, err)
}
defer os.Remove(f)
defer func() {
if err := os.Remove(f); err != nil {
t.Fatalf("Operation to remove temp file failed: %v", err)
}
}()

llr := NewLogListRefresher(f)
llm := NewLogListManager(llr, nil)
Expand All @@ -75,7 +79,11 @@ func TestSecondRefresh(t *testing.T) {
if err != nil {
t.Fatalf("createTempFile(%q) = (_, %q), want (_, nil)", testdata.SampleLogList3, err)
}
defer os.Remove(f)
defer func() {
if err := os.Remove(f); err != nil {
t.Fatalf("Operation to remove temp file failed: %v", err)
}
}()

llr := NewLogListRefresher(f)
llm := NewLogListManager(llr, monitoring.InertMetricFactory{})
Expand Down
28 changes: 21 additions & 7 deletions submission/loglist_refresher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package submission
import (
"context"
"fmt"
"log"
"net/http"
"os"
"regexp"
Expand All @@ -36,13 +37,14 @@ func createTempFile(data string) (string, error) {
if err != nil {
return "", err
}
defer f.Close()
defer func() {
if err := f.Close(); err != nil {
log.Fatalf("Operation to close file failed: %v", err)
}
}()
if _, err := f.WriteString(data); err != nil {
return "", err
}
if err := f.Close(); err != nil {
return "", err
}
return f.Name(), nil
}

Expand All @@ -54,7 +56,11 @@ func ExampleLogListRefresher() {
if err != nil {
panic(err)
}
defer os.Remove(f)
defer func() {
if err := os.Remove(f); err != nil {
log.Fatalf("Operation to remove temp file failed: %v", err)
}
}()

llr := NewLogListRefresher(f)

Expand Down Expand Up @@ -137,7 +143,11 @@ func TestNewLogListRefresher(t *testing.T) {
if err != nil {
t.Fatalf("createTempFile(%q) = (_, %q), want (_, nil)", tc.ll, err)
}
defer os.Remove(f)
defer func() {
if err := os.Remove(f); err != nil {
t.Fatalf("Operation to remove temp file failed: %v", err)
}
}()

beforeRefresh := time.Now()
llr := NewLogListRefresher(f)
Expand Down Expand Up @@ -203,7 +213,11 @@ func TestNewLogListRefresherUpdate(t *testing.T) {
if err != nil {
t.Fatalf("createTempFile(%q) = (_, %q), want (_, nil)", tc.ll, err)
}
defer os.Remove(f)
defer func() {
if err := os.Remove(f); err != nil {
t.Fatalf("Operation to remove temp file failed: %v", err)
}
}()

llr := NewLogListRefresher(f)
if _, err := llr.Refresh(); err != nil {
Expand Down
6 changes: 5 additions & 1 deletion submission/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ func TestProxyInitState(t *testing.T) {
if err != nil {
t.Fatalf("createTempFile(%q) = (_, %q), want (_, nil)", testdata.SampleLogList3, err)
}
defer os.Remove(f)
defer func() {
if err := os.Remove(f); err != nil {
t.Fatalf("Operation to remove temp file failed: %v", err)
}
}()

llr := NewLogListRefresher(f)
p := NewProxy(NewLogListManager(llr, nil), GetDistributorBuilder(ChromeCTPolicy, buildStubNoRootsLogClient, imf), imf)
Expand Down
Loading