Remove lint exceptions and fix remaining issues (#1438)

Remove lint exceptions and fix remaining issues

---------

Co-authored-by: Elisha Silas <silaselisha66@gmail.com>
Co-authored-by: Al Cutter <al@google.com>

.golangci.yaml

@@ -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"

AUTHORS

@@ -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

CONTRIBUTORS

@@ -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>

fixchain/chainfix/chainfix.go

@@ -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
@@ -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)
 	}

fixchain/roundtrip_test.go

@@ -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)
@@ -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)

fixchain/url_cache.go

@@ -17,11 +17,12 @@ package fixchain
 import (
 	"fmt"
 	"io"
-	"log"
 	"net/http"
 	"sync"
 	"sync/atomic"
 	"time"
+
+	"k8s.io/klog/v2"
 )
 
 type lockedCache struct {
@@ -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)
@@ -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))

internal/witness/client/http/witness_client.go

@@ -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.
@@ -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 {
@@ -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)

internal/witness/cmd/client/main.go

@@ -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)
 }
 

internal/witness/cmd/feeder/main.go

@@ -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)
 }
 

internal/witness/cmd/witness/config/config.go

@@ -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)
 }
 

internal/witness/cmd/witness/internal/witness/witness.go

@@ -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

jsonclient/client.go

@@ -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}
 	}
@@ -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 {

jsonclient/client_test.go

@@ -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":
@@ -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":

preload/preloader/preloader.go

@@ -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

submission/hammer/main.go

@@ -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 {

submission/loglist_manager_test.go

@@ -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)
@@ -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{})

submission/loglist_refresher_test.go

@@ -17,6 +17,7 @@ package submission
 import (
 	"context"
 	"fmt"
+	"log"
 	"net/http"
 	"os"
 	"regexp"
@@ -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
 }
 
@@ -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)
 
@@ -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)
@@ -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 {

submission/proxy_test.go

@@ -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)