Skip to content

Commit

Permalink
lint: enable errcheck and fix problems
Browse files Browse the repository at this point in the history
  • Loading branch information
ncw committed Apr 13, 2024
1 parent baba384 commit 0d0c480
Show file tree
Hide file tree
Showing 12 changed files with 288 additions and 92 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

linters:
enable:
#- errcheck
- errcheck
- goimports
#- revive
- ineffassign
Expand Down
47 changes: 31 additions & 16 deletions gofakes3.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,10 @@ func (g *GoFakeS3) createBucket(bucket string, w http.ResponseWriter, r *http.Re
}

w.Header().Set("Location", "/"+bucket)
w.Write([]byte{})
_, err := w.Write([]byte{})
if err != nil {
return err
}
return nil
}

Expand Down Expand Up @@ -417,17 +420,29 @@ func (g *GoFakeS3) headBucket(bucket string, w http.ResponseWriter, r *http.Requ
return err
}

w.Write([]byte{})
_, err := w.Write([]byte{})
if err != nil {
return err
}
return nil
}

// CheckClose is a utility function used to check the return from
// Close in a defer statement.
func CheckClose(c io.Closer, err *error) {
cerr := c.Close()
if *err == nil {
*err = cerr
}
}

// GetObject retrievs a bucket object.
func (g *GoFakeS3) getObject(
bucket, object string,
versionID VersionID,
w http.ResponseWriter,
r *http.Request,
) error {
) (err error) {

g.log.Print(LogInfo, "GET OBJECT", "Bucket:", bucket, "Object:", object)

Expand Down Expand Up @@ -463,7 +478,7 @@ func (g *GoFakeS3) getObject(
g.log.Print(LogErr, "unexpected nil object for key", bucket, object)
return ErrInternal
}
defer obj.Contents.Close()
defer CheckClose(obj.Contents, &err)

if err := g.writeGetOrHeadObjectResponse(obj, w, r); err != nil {
return err
Expand Down Expand Up @@ -517,7 +532,7 @@ func (g *GoFakeS3) headObject(
versionID VersionID,
w http.ResponseWriter,
r *http.Request,
) error {
) (err error) {

g.log.Print(LogInfo, "HEAD OBJECT", bucket, object)

Expand All @@ -533,7 +548,7 @@ func (g *GoFakeS3) headObject(
g.log.Print(LogErr, "unexpected nil object for key", bucket, object)
return ErrInternal
}
defer obj.Contents.Close()
defer CheckClose(obj.Contents, &err)

if err := g.writeGetOrHeadObjectResponse(obj, w, r); err != nil {
return err
Expand All @@ -546,7 +561,7 @@ func (g *GoFakeS3) headObject(

// createObjectBrowserUpload allows objects to be created from a multipart upload initiated
// by a browser form.
func (g *GoFakeS3) createObjectBrowserUpload(bucket string, w http.ResponseWriter, r *http.Request) error {
func (g *GoFakeS3) createObjectBrowserUpload(bucket string, w http.ResponseWriter, r *http.Request) (err error) {
g.log.Print(LogInfo, "CREATE OBJECT THROUGH BROWSER UPLOAD")

if err := g.ensureBucketExists(r, bucket); err != nil {
Expand Down Expand Up @@ -577,7 +592,7 @@ func (g *GoFakeS3) createObjectBrowserUpload(bucket string, w http.ResponseWrite
if err != nil {
return err
}
defer infile.Close()
defer CheckClose(infile, &err)

meta, err := metadataHeaders(r.MultipartForm.Value, g.timeSource.Now(), g.metadataSizeLimit)
if err != nil {
Expand Down Expand Up @@ -663,7 +678,7 @@ func (g *GoFakeS3) createObject(bucket, object string, w http.ResponseWriter, r
// hashingReader is still needed to get the ETag even if integrityCheck
// is set to false:
rdr, err := newHashingReader(reader, md5Base64)
defer r.Body.Close()
defer CheckClose(r.Body, &err)
if err != nil {
return err
}
Expand Down Expand Up @@ -802,7 +817,7 @@ func (g *GoFakeS3) deleteObjectVersion(bucket, object string, version VersionID,

// deleteMulti deletes multiple S3 objects from the bucket.
// https://docs.aws.amazon.com/AmazonS3/latest/API/multiobjectdeleteapi.html
func (g *GoFakeS3) deleteMulti(bucket string, w http.ResponseWriter, r *http.Request) error {
func (g *GoFakeS3) deleteMulti(bucket string, w http.ResponseWriter, r *http.Request) (err error) {
g.log.Print(LogInfo, "delete multi", bucket)

if err := g.ensureBucketExists(r, bucket); err != nil {
Expand All @@ -811,7 +826,7 @@ func (g *GoFakeS3) deleteMulti(bucket string, w http.ResponseWriter, r *http.Req

var in DeleteRequest

defer r.Body.Close()
defer CheckClose(r.Body, &err)
dc := xml.NewDecoder(r.Body)
if err := dc.Decode(&in); err != nil {
return ErrorMessage(ErrMalformedXML, err.Error())
Expand Down Expand Up @@ -861,7 +876,7 @@ func (g *GoFakeS3) initiateMultipartUpload(bucket, object string, w http.Respons
// part number that was used with a previous part, the previously uploaded part
// is overwritten. Each part must be at least 5 MB in size, except the last
// part. There is no size limit on the last part of your multipart upload.
func (g *GoFakeS3) putMultipartUploadPart(bucket, object string, uploadID UploadID, w http.ResponseWriter, r *http.Request) error {
func (g *GoFakeS3) putMultipartUploadPart(bucket, object string, uploadID UploadID, w http.ResponseWriter, r *http.Request) (err error) {
g.log.Print(LogInfo, "put multipart upload", bucket, object, uploadID)

partNumber, err := strconv.ParseInt(r.URL.Query().Get("partNumber"), 10, 0)
Expand All @@ -883,7 +898,7 @@ func (g *GoFakeS3) putMultipartUploadPart(bucket, object string, uploadID Upload
return err
}

defer r.Body.Close()
defer CheckClose(r.Body, &err)

meta, err := metadataHeaders(r.Header, g.timeSource.Now(), g.metadataSizeLimit)
if err != nil {
Expand Down Expand Up @@ -1089,17 +1104,17 @@ func (g *GoFakeS3) ensureBucketExists(r *http.Request, bucket string) error {
}

func (g *GoFakeS3) xmlEncoder(w http.ResponseWriter) *xml.Encoder {
w.Write([]byte(xml.Header))
_, _ = w.Write([]byte(xml.Header))
w.Header().Set("Content-Type", "application/xml")

xe := xml.NewEncoder(w)
xe.Indent("", " ")
return xe
}

func (g *GoFakeS3) xmlDecodeBody(rdr io.ReadCloser, into interface{}) error {
func (g *GoFakeS3) xmlDecodeBody(rdr io.ReadCloser, into interface{}) (err error) {
body, err := ioutil.ReadAll(rdr)
defer rdr.Close()
defer CheckClose(rdr, &err)
if err != nil {
return err
}
Expand Down
40 changes: 34 additions & 6 deletions gofakes3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,12 @@ func TestCreateObjectWithInvalidContentLength(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer rs.Body.Close()
defer func() {
err := rs.Body.Close()
if err != nil {
t.Error(err)
}
}()

if rs.StatusCode != http.StatusBadRequest {
t.Fatal(rs.StatusCode, "!=", http.StatusBadRequest)
Expand Down Expand Up @@ -380,7 +385,12 @@ func TestCopyObject(t *testing.T) {
obj, err := ts.backend.GetObject(mockR.Context(), defaultBucket, "dst-key", nil)
ts.OK(err)

defer obj.Contents.Close()
defer func() {
err := obj.Contents.Close()
if err != nil {
t.Error(err)
}
}()
data, err := ioutil.ReadAll(obj.Contents)
ts.OK(err)

Expand Down Expand Up @@ -581,7 +591,12 @@ func TestGetObjectRange(t *testing.T) {
}
if !fail {
ts.OK(err)
defer obj.Body.Close()
defer func() {
err := obj.Body.Close()
if err != nil {
t.Error(err)
}
}()

out, err := ioutil.ReadAll(obj.Body)
ts.OK(err)
Expand Down Expand Up @@ -718,7 +733,10 @@ func TestCreateObjectBrowserUpload(t *testing.T) {
}

upload := func(ts *testServer, bucket string, w *multipart.Writer, body io.Reader) (*http.Response, error) {
w.Close()
err := w.Close()
if err != nil {
ts.Error(err)
}
req, err := http.NewRequest("POST", ts.url("/"+bucket), body)
ts.OK(err)
req.Header.Set("Content-Type", w.FormDataContentType())
Expand All @@ -742,7 +760,12 @@ func TestCreateObjectBrowserUpload(t *testing.T) {
if res.StatusCode != expectedCode.Status() {
ts.Fatal("bad status", res.StatusCode, "!=", expectedCode.Status())
}
defer res.Body.Close()
defer func() {
err := res.Body.Close()
if err != nil {
ts.Error(err)
}
}()
var errResp gofakes3.ErrorResponse
dec := xml.NewDecoder(res.Body)
ts.OK(dec.Decode(&errResp))
Expand Down Expand Up @@ -891,7 +914,12 @@ func TestObjectVersions(t *testing.T) {
}
out, err := svc.GetObject(input)
ts.OK(err)
defer out.Body.Close()
defer func() {
err := out.Body.Close()
if err != nil {
ts.Error(err)
}
}()
bts, err := ioutil.ReadAll(out.Body)
ts.OK(err)
if !bytes.Equal(bts, contents) {
Expand Down
27 changes: 19 additions & 8 deletions init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,11 @@ func TestMain(m *testing.M) {
os.Exit(0)
}

func runTestMain(m *testing.M) error {
func runTestMain(m *testing.M) (err error) {
flag.StringVar(&logFile, "fakes3.log", "", "Log file (temp file by default)")
flag.Parse()

var logOutput *os.File
var err error

if logFile == "" {
logOutput, err = ioutil.TempFile("", "gofakes3-*.log")
Expand All @@ -82,7 +81,7 @@ func runTestMain(m *testing.M) error {
if err != nil {
return err
}
defer logOutput.Close()
defer gofakes3.CheckClose(logOutput, &err)

fmt.Fprintf(os.Stderr, "log output redirected to %q\n", logOutput.Name())
log.SetOutput(logOutput)
Expand Down Expand Up @@ -261,7 +260,12 @@ func (ts *testServer) backendGetString(bucket, key string, rnge *gofakes3.Object
obj, err := ts.backend.GetObject(mockR.Context(), bucket, key, rnge)
ts.OK(err)

defer obj.Contents.Close()
defer func() {
err := obj.Contents.Close()
if err != nil {
ts.Error(err)
}
}()
data, err := ioutil.ReadAll(obj.Contents)
ts.OK(err)

Expand Down Expand Up @@ -608,7 +612,12 @@ func (ts *testServer) assertObject(bucket string, object string, meta map[string

obj, err := ts.backend.GetObject(mockR.Context(), bucket, object, nil)
ts.OK(err)
defer obj.Contents.Close()
defer func() {
err := obj.Contents.Close()
if err != nil {
ts.Error(err)
}
}()

data, err := gofakes3.ReadAll(obj.Contents, obj.Size)
ts.OK(err)
Expand Down Expand Up @@ -874,7 +883,7 @@ func (c *rawClient) SetHeaders(rq *http.Request, body []byte) {
// SendRaw can be used to bypass Go's http client, which helps us out a lot by taking
// care of some things for us, but which we actually want to test messing up from
// time to time.
func (c *rawClient) SendRaw(rq *http.Request) ([]byte, error) {
func (c *rawClient) SendRaw(rq *http.Request) (out []byte, err error) {
b, err := httputil.DumpRequest(rq, true)
if err != nil {
return nil, err
Expand All @@ -883,10 +892,12 @@ func (c *rawClient) SendRaw(rq *http.Request) ([]byte, error) {
if err != nil {
return nil, err
}
defer conn.Close()
defer gofakes3.CheckClose(conn, &err)

deadline := time.Now().Add(2 * time.Second)
conn.SetDeadline(deadline)
if err := conn.SetDeadline(deadline); err != nil {
return nil, err
}
if _, err := conn.Write(b); err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,13 @@ func (t *S300001GetVersionAfterVersioningSuspended) Run(ctx *Context) error {
}
}

readCloseBody := func(rdr io.ReadCloser) ([]byte, error) {
defer rdr.Close()
readCloseBody := func(rdr io.ReadCloser) (out []byte, err error) {
defer func() {
closeErr := rdr.Close()
if closeErr != nil && err == nil {
err = closeErr
}
}()
return ioutil.ReadAll(rdr)
}

Expand Down
7 changes: 6 additions & 1 deletion s3mem/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,12 @@ func (db *Backend) CopyObject(ctx context.Context, srcBucket, srcKey, dstBucket,
if err != nil {
return
}
defer c.Contents.Close()
defer func() {
closeErr := c.Contents.Close()
if closeErr != nil && err == nil {
err = closeErr
}
}()

for k, v := range c.Metadata {
if _, found := meta[k]; !found && k != "X-Amz-Acl" {
Expand Down
2 changes: 1 addition & 1 deletion xml/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func ExampleMarshalIndent() {
fmt.Printf("error: %v\n", err)
}

os.Stdout.Write(output)
_, _ = os.Stdout.Write(output)
// Output:
// <person id="13">
// <name>
Expand Down
Loading

0 comments on commit 0d0c480

Please sign in to comment.