Report non-XML formated error details (#1585)

This avoids hiding further error context in the case when it is coming
from a layer, which is not necessarily using the S3 XML error format.

Co-authored-by: Klaus Post <klauspost@gmail.com>

api-error-response.go

@@ -18,8 +18,11 @@
 package minio
 
 import (
+	"bytes"
 	"encoding/xml"
 	"fmt"
+	"io"
+	"io/ioutil"
 	"net/http"
 )
 
@@ -98,6 +101,19 @@ const (
 	reportIssue = "Please report this issue at https://github.com/minio/minio-go/issues."
 )
 
+// xmlDecodeAndBody reads the whole body up to 1MB and
+// tries to XML decode it into v.
+// The body that was read and any error from reading or decoding is returned.
+func xmlDecodeAndBody(bodyReader io.Reader, v interface{}) ([]byte, error) {
+	// read the whole body (up to 1MB)
+	const maxBodyLength = 1 << 20
+	body, err := ioutil.ReadAll(io.LimitReader(bodyReader, maxBodyLength))
+	if err != nil {
+		return nil, err
+	}
+	return bytes.TrimSpace(body), xmlDecoder(bytes.NewReader(body), v)
+}
+
 // httpRespToErrorResponse returns a new encoded ErrorResponse
 // structure as error.
 func httpRespToErrorResponse(resp *http.Response, bucketName, objectName string) error {
@@ -111,7 +127,7 @@ func httpRespToErrorResponse(resp *http.Response, bucketName, objectName string)
 		Server:     resp.Header.Get("Server"),
 	}
 
-	err := xmlDecoder(resp.Body, &errResp)
+	errBody, err := xmlDecodeAndBody(resp.Body, &errResp)
 	// Xml decoding failed with no body, fall back to HTTP headers.
 	if err != nil {
 		switch resp.StatusCode {
@@ -156,10 +172,17 @@ func httpRespToErrorResponse(resp *http.Response, bucketName, objectName string)
 				Key:        objectName,
 			}
 		default:
+			msg := resp.Status
+			if len(errBody) > 0 {
+				msg = string(errBody)
+				if len(msg) > 1024 {
+					msg = msg[:1024] + "..."
+				}
+			}
 			errResp = ErrorResponse{
 				StatusCode: resp.StatusCode,
 				Code:       resp.Status,
-				Message:    resp.Status,
+				Message:    msg,
 				BucketName: bucketName,
 			}
 		}

api-error-response_test.go

@@ -52,18 +52,22 @@ func TestHttpRespToErrorResponse(t *testing.T) {
 		return buf.Bytes()
 	}
 
+	// `createErrorResponse` Mocks a generic error response from the server.
+	createErrorResponse := func(statusCode int, body []byte) *http.Response {
+		resp := &http.Response{}
+		resp.StatusCode = statusCode
+		resp.Status = http.StatusText(statusCode)
+		resp.Body = ioutil.NopCloser(bytes.NewBuffer(body))
+		return resp
+	}
+
 	// `createAPIErrorResponse` Mocks XML error response from the server.
 	createAPIErrorResponse := func(APIErr APIError, bucketName string) *http.Response {
 		// generate error response.
 		// response body contains the XML error message.
-		resp := &http.Response{}
 		errorResponse := genAPIErrorResponse(APIErr, bucketName)
 		encodedErrorResponse := encodeErr(errorResponse)
-		// write Header.
-		resp.StatusCode = APIErr.HTTPStatusCode
-		resp.Body = ioutil.NopCloser(bytes.NewBuffer(encodedErrorResponse))
-
-		return resp
+		return createErrorResponse(APIErr.HTTPStatusCode, encodedErrorResponse)
 	}
 
 	// 'genErrResponse' contructs error response based http Status Code
@@ -108,6 +112,7 @@ func TestHttpRespToErrorResponse(t *testing.T) {
 	genEmptyBodyResponse := func(statusCode int) *http.Response {
 		resp := &http.Response{
 			StatusCode: statusCode,
+			Status:     http.StatusText(statusCode),
 			Body:       ioutil.NopCloser(bytes.NewReader(nil)),
 		}
 		setCommonHeaders(resp)
@@ -145,6 +150,8 @@ func TestHttpRespToErrorResponse(t *testing.T) {
 		genErrResponse(setCommonHeaders(&http.Response{StatusCode: http.StatusForbidden}), "AccessDenied", "Access Denied.", "minio-bucket", ""),
 		genErrResponse(setCommonHeaders(&http.Response{StatusCode: http.StatusConflict}), "Conflict", "Bucket not empty.", "minio-bucket", ""),
 		genErrResponse(setCommonHeaders(&http.Response{StatusCode: http.StatusBadRequest}), "Bad Request", "Bad Request", "minio-bucket", ""),
+		genErrResponse(setCommonHeaders(&http.Response{StatusCode: http.StatusInternalServerError}), "Internal Server Error", "my custom object store error", "minio-bucket", ""),
+		genErrResponse(setCommonHeaders(&http.Response{StatusCode: http.StatusInternalServerError}), "Internal Server Error", "my custom object store error, with way too long body", "minio-bucket", ""),
 	}
 
 	// List of http response to be used as input.
@@ -156,6 +163,8 @@ func TestHttpRespToErrorResponse(t *testing.T) {
 		genEmptyBodyResponse(http.StatusForbidden),
 		genEmptyBodyResponse(http.StatusConflict),
 		genEmptyBodyResponse(http.StatusBadRequest),
+		setCommonHeaders(createErrorResponse(http.StatusInternalServerError, []byte("my custom object store error\n"))),
+		setCommonHeaders(createErrorResponse(http.StatusInternalServerError, append([]byte("my custom object store error, with way too long body\n"), bytes.Repeat([]byte("\n"), 2*1024*1024)...))),
 	}
 
 	testCases := []struct {
@@ -173,6 +182,9 @@ func TestHttpRespToErrorResponse(t *testing.T) {
 		{"minio-bucket", "Asia/", inputResponses[3], expectedErrResponse[3]},
 		{"minio-bucket", "", inputResponses[4], expectedErrResponse[4]},
 		{"minio-bucket", "", inputResponses[5], expectedErrResponse[5]},
+		{"minio-bucket", "", inputResponses[6], expectedErrResponse[6]},
+		{"minio-bucket", "", inputResponses[7], expectedErrResponse[7]},
+		{"minio-bucket", "", inputResponses[8], expectedErrResponse[8]},
 	}
 
 	for i, testCase := range testCases {