Write xml.Header first instead of spaces to handle XML parsers (#7253)

Clients like AWS SDK Java and AWS cli XML parsers are
unable to handle on `\r\n` characters to avoid these
errors send XML header first and write white space characters
instead.

Also handle cases to avoid double WriteHeader calls

cmd/api-response-multipart.go

@@ -1,60 +0,0 @@
-/*
- * Minio Cloud Storage, (C) 2016 Minio, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package cmd
-
-import (
-	"context"
-	"net/http"
-)
-
-// Represents additional fields necessary for ErrPartTooSmall S3 error.
-type completeMultipartAPIError struct {
-	// Proposed size represents uploaded size of the part.
-	ProposedSize int64
-	// Minimum size allowed epresents the minimum size allowed per
-	// part. Defaults to 5MB.
-	MinSizeAllowed int64
-	// Part number of the part which is incorrect.
-	PartNumber int
-	// ETag of the part which is incorrect.
-	PartETag string
-	// Other default XML error responses.
-	APIErrorResponse
-}
-
-// writeErrorResponsePartTooSmall - function is used specifically to
-// construct a proper error response during CompleteMultipartUpload
-// when one of the parts is < 5MB.
-// The requirement comes due to the fact that generic ErrorResponse
-// XML doesn't carry the additional fields required to send this
-// error. So we construct a new type which lies well within the scope
-// of this function.
-func writePartSmallErrorResponse(ctx context.Context, w http.ResponseWriter, r *http.Request, err PartTooSmall) {
-	apiError := toAPIError(ctx, err)
-	// Generate complete multipart error response.
-	errorResponse := getAPIErrorResponse(ctx, apiError, r.URL.Path,
-		w.Header().Get(responseRequestIDKey),
-		w.Header().Get(responseDeploymentIDKey))
-	cmpErrResp := completeMultipartAPIError{err.PartSize, int64(5242880), err.PartNumber, err.PartETag, errorResponse}
-	encodedErrorResponse := encodeResponse(cmpErrResp)
-
-	// respond with 400 bad request.
-	w.WriteHeader(apiError.HTTPStatusCode)
-	// Write error body.
-	w.Write(encodedErrorResponse)
-	w.(http.Flusher).Flush()
-}

cmd/api-response.go

@@ -587,7 +587,8 @@ func writeErrorResponse(ctx context.Context, w http.ResponseWriter, err APIError
 	}
 
 	// Generate error response.
-	errorResponse := getAPIErrorResponse(ctx, err, reqURL.Path, w.Header().Get(responseRequestIDKey), w.Header().Get(responseDeploymentIDKey))
+	errorResponse := getAPIErrorResponse(ctx, err, reqURL.Path,
+		w.Header().Get(responseRequestIDKey), w.Header().Get(responseDeploymentIDKey))
 	encodedErrorResponse := encodeResponse(errorResponse)
 	writeResponse(w, err.HTTPStatusCode, encodedErrorResponse, mimeXML)
 }

cmd/object-handlers.go

@@ -2152,27 +2152,52 @@ func (api objectAPIHandlers) ListObjectPartsHandler(w http.ResponseWriter, r *ht
 	writeSuccessResponseXML(w, encodedSuccessResponse)
 }
 
-// Send white spaces every 10 seconds to the client till completeMultiPartUpload() is done so that the client does not time out.
-// Downside is we might send 200OK and then send error XML. But accoording to S3 spec
-// the client is supposed to check for error XML even if it received 200OK. But for erasure this is not a
-// problem as completeMultiPartUpload() is quick. Even For FS, it would not be an issue
-// as we do background append as and when the parts arrive and completeMultiPartUpload is quick.
-// Only in a rare case where parts would be out of order will FS' completeMultiPartUpload() take a longer time.
-func sendWhiteSpace(ctx context.Context, w http.ResponseWriter) <-chan struct{} {
-	doneCh := make(chan struct{})
+type whiteSpaceWriter struct {
+	http.ResponseWriter
+	http.Flusher
+	written bool
+}
+
+func (w *whiteSpaceWriter) Write(b []byte) (n int, err error) {
+	n, err = w.ResponseWriter.Write(b)
+	w.written = true
+	return
+}
+
+func (w *whiteSpaceWriter) WriteHeader(statusCode int) {
+	if !w.written {
+		w.ResponseWriter.WriteHeader(statusCode)
+	}
+}
+
+// Send empty whitespaces every 10 seconds to the client till completeMultiPartUpload() is
+// done so that the client does not time out. Downside is we might send 200 OK and
+// then send error XML. But accoording to S3 spec the client is supposed to check
+// for error XML even if it received 200 OK. But for erasure this is not a problem
+// as completeMultiPartUpload() is quick. Even For FS, it would not be an issue as
+// we do background append as and when the parts arrive and completeMultiPartUpload
+// is quick. Only in a rare case where parts would be out of order will
+// FS:completeMultiPartUpload() take a longer time.
+func sendWhiteSpace(ctx context.Context, w http.ResponseWriter) <-chan bool {
+	doneCh := make(chan bool)
 	go func() {
 		ticker := time.NewTicker(time.Second * 10)
+		headerWritten := false
 		for {
 			select {
 			case <-ticker.C:
-				// Write a white space char to the client to prevent client timeouts
-				// when the server takes a long time to completeMultiPartUpload()
-				if _, err := w.Write([]byte("\n\r")); err != nil {
-					logger.LogIf(ctx, err)
-					return
+				// Write header if not written yet.
+				if !headerWritten {
+					w.Write([]byte(xml.Header))
+					headerWritten = true
 				}
+
+				// Once header is written keep writing empty spaces
+				// which are ignored by client SDK XML parsers.
+				// This occurs when server takes long time to completeMultiPartUpload()
+				w.Write([]byte(" "))
 				w.(http.Flusher).Flush()
-			case doneCh <- struct{}{}:
+			case doneCh <- headerWritten:
 				ticker.Stop()
 				return
 			}
@@ -2309,18 +2334,36 @@ func (api objectAPIHandlers) CompleteMultipartUploadHandler(w http.ResponseWrite
 	if api.CacheAPI() != nil {
 		completeMultiPartUpload = api.CacheAPI().CompleteMultipartUpload
 	}
+
+	// This code is specifically to handle the requirements for slow
+	// complete multipart upload operations on FS mode.
+	writeErrorResponseWithoutXMLHeader := func(ctx context.Context, w http.ResponseWriter, err APIError, reqURL *url.URL) {
+		switch err.Code {
+		case "SlowDown", "XMinioServerNotInitialized", "XMinioReadQuorum", "XMinioWriteQuorum":
+			// Set retry-after header to indicate user-agents to retry request after 120secs.
+			// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After
+			w.Header().Set("Retry-After", "120")
+		}
+
+		// Generate error response.
+		errorResponse := getAPIErrorResponse(ctx, err, reqURL.Path,
+			w.Header().Get(responseRequestIDKey), w.Header().Get(responseDeploymentIDKey))
+		encodedErrorResponse, _ := xml.Marshal(errorResponse)
+		setCommonHeaders(w)
+		w.Header().Set("Content-Type", string(mimeXML))
+		w.Write(encodedErrorResponse)
+		w.(http.Flusher).Flush()
+	}
+	w = &whiteSpaceWriter{ResponseWriter: w, Flusher: w.(http.Flusher)}
 	completeDoneCh := sendWhiteSpace(ctx, w)
 	objInfo, err := completeMultiPartUpload(ctx, bucket, object, uploadID, completeParts, opts)
 	// Stop writing white spaces to the client. Note that close(doneCh) style is not used as it
 	// can cause white space to be written after we send XML response in a race condition.
-	<-completeDoneCh
+	headerWritten := <-completeDoneCh
 	if err != nil {
-		switch oErr := err.(type) {
-		case PartTooSmall:
-			// Write part too small error.
-			writePartSmallErrorResponse(ctx, w, r, oErr)
-		default:
-			// Handle all other generic issues.
+		if headerWritten {
+			writeErrorResponseWithoutXMLHeader(ctx, w, toAPIError(ctx, err), r.URL)
+		} else {
 			writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
 		}
 		return
@@ -2330,11 +2373,16 @@ func (api objectAPIHandlers) CompleteMultipartUploadHandler(w http.ResponseWrite
 	location := getObjectLocation(r, globalDomainName, bucket, object)
 	// Generate complete multipart response.
 	response := generateCompleteMultpartUploadResponse(bucket, object, location, objInfo.ETag)
-	encodedSuccessResponse := encodeResponse(response)
+	var encodedSuccessResponse []byte
+	if !headerWritten {
+		encodedSuccessResponse = encodeResponse(response)
+	} else {
+		encodedSuccessResponse, err = xml.Marshal(response)
 		if err != nil {
-		writeErrorResponse(ctx, w, toAdminAPIErr(ctx, err), r.URL, guessIsBrowserReq(r))
+			writeErrorResponseWithoutXMLHeader(ctx, w, toAPIError(ctx, err), r.URL)
 			return
 		}
+	}
 
 	// Set etag.
 	w.Header().Set("ETag", "\""+objInfo.ETag+"\"")

cmd/object-handlers_test.go

@@ -2740,9 +2740,9 @@ func testAPICompleteMultipartHandler(obj ObjectLayer, instanceType, bucketName s
 			accessKey: credentials.AccessKey,
 			secretKey: credentials.SecretKey,
 
-			expectedContent: encodeResponse(completeMultipartAPIError{int64(4), int64(5242880), 1, "e2fc714c4727ee9395f324cd2e7f331f",
-				getAPIErrorResponse(ctx, toAPIError(ctx, PartTooSmall{PartNumber: 1}),
-					getGetObjectURL("", bucketName, objectName), "", "")}),
+			expectedContent: encodeResponse(getAPIErrorResponse(ctx,
+				toAPIError(ctx, PartTooSmall{PartNumber: 1}),
+				getGetObjectURL("", bucketName, objectName), "", "")),
 			expectedRespStatus: http.StatusBadRequest,
 		},
 		// Test case - 5.