From 0be3a44757352b6e617ef00eb47829bce29baab1 Mon Sep 17 00:00:00 2001 From: Anis Elleuch Date: Wed, 23 Jan 2019 03:32:59 +0100 Subject: [PATCH] Remove unnecessary header field in ErrorResponse (#1066) ErrorResponse.Headers is causing a panic inside golang http package. The following code can be easily crashed if GetObject API returns an error, such as ErrSlowDown: ``` reader, err := s3Client.GetObject("my-bucketname", "my-source-objectname", minio.GetObjectOptions{}) if err != nil { log.Fatalln(err) } defer reader.Close() stat, err := reader.Stat() if err != nil { log.Fatalln(err) } n, err := s3Client.PutObject("my-bucketname", "my-target-objectname", reader, stat.Size, minio.PutObjectOptions{}) if err != nil { log.Fatalln(err) } ``` The reason is that `reader` is passed to s3Client.PutObject therefore to golang http via request.Body. Since, reader.Read() can return an ErrorResponse, that error will be tested by golang http, but since ErrorResponse is uncomparable, then it will cause a crash with the following stackstrace: ``` panic: runtime error: comparing uncomparable type minio.ErrorResponse goroutine 20 [running]: net/http.(*Request).write(0xc0002c2300, 0x761e60, 0xc000069f80, 0x0, 0x0, 0x0, 0x7628a0, 0xc000518780) /home/vadmeste/work/go/src/net/http/request.go:647 +0x74c net/http.(*persistConn).writeLoop(0xc0000a17a0) /home/vadmeste/work/go/src/net/http/transport.go:1888 +0x1b8 created by net/http.(*Transport).dialConn /home/vadmeste/work/go/src/net/http/transport.go:1339 +0x966 exit status 2 ``` Hence, removing Headers since it is a map and it is uncomparable. --- api-error-response.go | 8 ++------ api-error-response_test.go | 11 ++++++++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/api-error-response.go b/api-error-response.go index 655991cff..0170b8de8 100644 --- a/api-error-response.go +++ b/api-error-response.go @@ -36,6 +36,8 @@ import ( */ // ErrorResponse - Is the typed error returned by all API operations. +// ErrorResponse struct should be comparable since it is compared inside +// golang http API (https://github.com/golang/go/issues/29768) type ErrorResponse struct { XMLName xml.Name `xml:"Error" json:"-"` Code string @@ -51,9 +53,6 @@ type ErrorResponse struct { // Underlying HTTP status code for the returned error StatusCode int `xml:"-" json:"-"` - - // Headers of the returned S3 XML error - Headers http.Header `xml:"-" json:"-"` } // ToErrorResponse - Returns parsed ErrorResponse struct from body and @@ -177,9 +176,6 @@ func httpRespToErrorResponse(resp *http.Response, bucketName, objectName string) errResp.Message = fmt.Sprintf("Region does not match, expecting region ā€˜%sā€™.", errResp.Region) } - // Save headers returned in the API XML error - errResp.Headers = resp.Header - return errResp } diff --git a/api-error-response_test.go b/api-error-response_test.go index bf10941b4..353103bdd 100644 --- a/api-error-response_test.go +++ b/api-error-response_test.go @@ -77,7 +77,6 @@ func TestHttpRespToErrorResponse(t *testing.T) { RequestID: resp.Header.Get("x-amz-request-id"), HostID: resp.Header.Get("x-amz-id-2"), Region: resp.Header.Get("x-amz-bucket-region"), - Headers: resp.Header, } return errResp } @@ -292,3 +291,13 @@ func TestErrWithoutMessage(t *testing.T) { t.Errorf("Expected \"Error response code InvalidArgument.\", got %s", errResp) } } + +// Tests if ErrorResponse is comparable since it is compared +// inside golang http code (https://github.com/golang/go/issues/29768) +func TestErrorResponseComparable(t *testing.T) { + var e1 interface{} = ErrorResponse{} + var e2 interface{} = ErrorResponse{} + if e1 != e2 { + t.Fatalf("ErrorResponse should be comparable") + } +}