diff --git a/swift.go b/swift.go index 11de54407..790411791 100644 --- a/swift.go +++ b/swift.go @@ -9,6 +9,7 @@ import ( "crypto/sha1" "encoding/hex" "encoding/json" + "errors" "fmt" "hash" "io" @@ -24,14 +25,15 @@ import ( ) const ( - DefaultUserAgent = "goswift/1.0" // Default user agent - DefaultRetries = 3 // Default number of retries on token expiry - TimeFormat = "2006-01-02T15:04:05" // Python date format for json replies parsed as UTC - UploadTar = "tar" // Data format specifier for Connection.BulkUpload(). - UploadTarGzip = "tar.gz" // Data format specifier for Connection.BulkUpload(). - UploadTarBzip2 = "tar.bz2" // Data format specifier for Connection.BulkUpload(). - allContainersLimit = 10000 // Number of containers to fetch at once - allObjectsChanLimit = 1000 // Number objects to fetch when fetching to a channel + DefaultUserAgent = "goswift/1.0" // Default user agent + DefaultRetries = 3 // Default number of retries on token expiry + TimeFormat = "2006-01-02T15:04:05" // Python date format for json replies parsed as UTC + UploadTar = "tar" // Data format specifier for Connection.BulkUpload(). + UploadTarGzip = "tar.gz" // Data format specifier for Connection.BulkUpload(). + UploadTarBzip2 = "tar.bz2" // Data format specifier for Connection.BulkUpload(). + allContainersLimit = 10000 // Number of containers to fetch at once + allObjectsChanLimit = 1000 // Number objects to fetch when fetching to a channel + respBodyErrSizeLimit = 1024 // Maximum size of response body to read when appending to error messages ) // ObjectType is the type of the swift object, regular, static large, @@ -372,22 +374,65 @@ func drainAndClose(rd io.ReadCloser, err *error) { } // parseHeaders checks a response for errors and translates into -// standard errors if necessary. If an error is returned, resp.Body +// standard errors if necessary. If an error message is present in the response body, +// it will be included in the error. If an error is returned, resp.Body // has been drained and closed. func (c *Connection) parseHeaders(resp *http.Response, errorMap errorMap) error { if errorMap != nil { if err, ok := errorMap[resp.StatusCode]; ok { + err = appendResponseBodyToError(resp, err) drainAndClose(resp.Body, nil) return err } } if resp.StatusCode < 200 || resp.StatusCode > 299 { + var err error = newErrorf(resp.StatusCode, "HTTP Error: %d: %s", resp.StatusCode, resp.Status) + err = appendResponseBodyToError(resp, err) drainAndClose(resp.Body, nil) - return newErrorf(resp.StatusCode, "HTTP Error: %d: %s", resp.StatusCode, resp.Status) + return err } return nil } +// appendResponseBodyToError tries to append the response body to the error message. +func appendResponseBodyToError(resp *http.Response, err error) error { + if resp == nil || resp.Body == nil || err == nil { + return err + } + + if resp.Header.Get("Content-Length") == "0" { + return err + } + + ct := resp.Header.Get("Content-Type") + if ct == "" { + return err + } + + lowerCT := strings.ToLower(ct) + if !(strings.Contains(lowerCT, "text") || + strings.Contains(lowerCT, "json") || + strings.Contains(lowerCT, "xml") || + strings.Contains(lowerCT, "html") || + strings.Contains(lowerCT, "plain")) { + return err + } + + buf := make([]byte, respBodyErrSizeLimit) + limitedReader := io.LimitReader(resp.Body, respBodyErrSizeLimit) + n, readErr := limitedReader.Read(buf) + if readErr != nil || n == 0 { + return err + } + + trimmed := strings.TrimSpace(string(buf[:n])) + if trimmed == "" { + return err + } + + return fmt.Errorf("%w: %s", err, trimmed) +} + // readHeaders returns a Headers object from the http.Response. // // If it receives multiple values for a key (which should never @@ -519,7 +564,7 @@ again: // Try again for a limited number of times on // AuthorizationFailed or BadRequest. This allows us // to try some alternate forms of the request - if (err == AuthorizationFailed || err == BadRequest) && retries > 0 { + if (errors.Is(err, AuthorizationFailed) || errors.Is(err, BadRequest)) && retries > 0 { retries-- goto again } diff --git a/swift_internal_test.go b/swift_internal_test.go index e84080084..a3f671354 100644 --- a/swift_internal_test.go +++ b/swift_internal_test.go @@ -7,12 +7,14 @@ package swift import ( "context" + "errors" "fmt" "io" "net" "net/http" "os" "reflect" + "strings" "testing" "time" ) @@ -284,6 +286,84 @@ func TestInternalParseHeaders(t *testing.T) { } } +func TestInternalParseHeadersWithErrorMessageInBody(t *testing.T) { + testCases := []struct { + name string + resp *http.Response + errMap map[int]error + expected string + }{ + { + name: "Container error with body", + resp: &http.Response{ + StatusCode: 400, + Header: http.Header{ + "Content-Type": []string{"text/plain"}, + }, + ContentLength: 15, + Body: io.NopCloser(strings.NewReader("Body message")), + Status: "Status message", + }, + errMap: ContainerErrorMap, + expected: "Bad Request: Body message", + }, + { + name: "Error with body", + resp: &http.Response{ + StatusCode: 400, + Header: http.Header{ + "Content-Type": []string{"text/plain"}, + }, + ContentLength: 15, + Body: io.NopCloser(strings.NewReader("Body message")), + Status: "Status message", + }, + errMap: nil, + expected: "HTTP Error: 400: Status message: Body message", + }, + { + name: "Object error with body", + resp: &http.Response{ + StatusCode: 400, + Header: http.Header{ + "Content-Type": []string{"text/plain"}, + }, + ContentLength: 15, + Body: io.NopCloser(strings.NewReader("Body message")), + Status: "Status message", + }, + errMap: objectErrorMap, + expected: "Bad Request: Body message", + }, + { + name: "Object error with body over 1024 bytes", + resp: &http.Response{ + StatusCode: 400, + Header: http.Header{ + "Content-Type": []string{"text/plain"}, + }, + ContentLength: 1025, + Body: io.NopCloser(strings.NewReader(strings.Repeat("a", 1025))), + Status: "Status message", + }, + errMap: objectErrorMap, + expected: fmt.Sprintf("%s: %s", "Bad Request", strings.Repeat("a", 1024)), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := c.parseHeaders(tc.resp, tc.errMap) + if err == nil { + t.Fatalf("Expected error, got nil") + } + if err.Error() != tc.expected { + t.Errorf("Expected error %q, got %q", tc.expected, err.Error()) + } + }) + } +} + func TestInternalReadHeaders(t *testing.T) { resp := &http.Response{Header: http.Header{}} compareMaps(t, readHeaders(resp), Headers{}) @@ -341,7 +421,7 @@ func TestInternalAuthenticateDenied(t *testing.T) { defer server.Finished() c.UnAuthenticate() err := c.Authenticate(context.Background()) - if err != AuthorizationFailed { + if !errors.Is(err, AuthorizationFailed) { t.Fatal("Expecting AuthorizationFailed", err) } // FIXME diff --git a/swift_test.go b/swift_test.go index 12f03dddb..1c71fd40d 100644 --- a/swift_test.go +++ b/swift_test.go @@ -21,6 +21,7 @@ import ( "crypto/tls" "encoding/json" "encoding/xml" + "errors" "fmt" "io" "net/http" @@ -1207,7 +1208,7 @@ func TestObjectCreate(t *testing.T) { // FIXME: work around bug which produces 503 not 422 for empty corrupted files _, _ = fmt.Fprintf(out, "Sausage") err = out.Close() - if err != swift.ObjectCorrupted { + if !errors.Is(err, swift.ObjectCorrupted) { t.Error("Expecting object corrupted not", err) } } @@ -1238,7 +1239,7 @@ func TestObjectCreateAbort(t *testing.T) { } _, err = c.ObjectGetString(ctx, CONTAINER, OBJECT2) - if err != swift.ObjectNotFound { + if !errors.Is(err, swift.ObjectNotFound) { t.Errorf("Unexpected error: %#v", err) } } @@ -1989,7 +1990,7 @@ func TestVersionDeleteContent(t *testing.T) { if err := c.ObjectDelete(ctx, CURRENT_CONTAINER, OBJECT); err != nil { t.Fatal(err) } - if err := c.ObjectDelete(ctx, CURRENT_CONTAINER, OBJECT); err != swift.ObjectNotFound { + if err := c.ObjectDelete(ctx, CURRENT_CONTAINER, OBJECT); !errors.Is(err, swift.ObjectNotFound) { t.Fatalf("Expecting Object not found error, got: %v", err) } } @@ -2000,7 +2001,7 @@ func testExistenceAfterDelete(t *testing.T, c *swift.Connection, container, obje ctx := context.Background() for i := 10; i <= 0; i-- { _, _, err := c.Object(ctx, container, object) - if err == swift.ObjectNotFound { + if errors.Is(err, swift.ObjectNotFound) { break } if i == 0 { @@ -2020,7 +2021,7 @@ func TestObjectDelete(t *testing.T) { } testExistenceAfterDelete(t, c, CONTAINER, OBJECT) err = c.ObjectDelete(ctx, CONTAINER, OBJECT) - if err != swift.ObjectNotFound { + if !errors.Is(err, swift.ObjectNotFound) { t.Fatal("Expecting Object not found", err) } } @@ -2030,7 +2031,7 @@ func TestBulkDelete(t *testing.T) { c, rollback := makeConnectionWithContainer(t) defer rollback() result, err := c.BulkDelete(ctx, CONTAINER, []string{OBJECT}) - if err == swift.Forbidden { + if errors.Is(err, swift.Forbidden) { t.Log("Server doesn't support BulkDelete - skipping test") return } @@ -3237,11 +3238,11 @@ func TestContainerDelete(t *testing.T) { t.Fatal(err) } err = c.ContainerDelete(ctx, CONTAINER) - if err != swift.ContainerNotFound { + if !errors.Is(err, swift.ContainerNotFound) { t.Fatal("Expecting container not found", err) } _, _, err = c.Container(ctx, CONTAINER) - if err != swift.ContainerNotFound { + if !errors.Is(err, swift.ContainerNotFound) { t.Fatal("Expecting container not found", err) } }