From 3d685b7fffd37620660f88b115f852ecb4c89194 Mon Sep 17 00:00:00 2001
From: Klaus Post <klauspost@gmail.com>
Date: Mon, 19 Apr 2021 17:44:18 +0200
Subject: [PATCH] fix: zip error races in WebDownload (#12086)

When an error is reported it is ignored and zipping continues with the next object.

However, if there is an error it will write a response to `writeWebErrorResponse(w, err)`, but responses are still being built.

Fixes #12082

Bonus: Exclude common compressed image types.
---
 cmd/globals.go      |  4 ++--
 cmd/web-handlers.go | 29 ++++++++++-------------------
 2 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/cmd/globals.go b/cmd/globals.go
index 6c151b419..f3d7367ed 100644
--- a/cmd/globals.go
+++ b/cmd/globals.go
@@ -29,7 +29,7 @@ import (
 	"github.com/minio/minio/pkg/handlers"
 	"github.com/minio/minio/pkg/kms"
 
-	humanize "github.com/dustin/go-humanize"
+	"github.com/dustin/go-humanize"
 	"github.com/minio/minio/cmd/config/cache"
 	"github.com/minio/minio/cmd/config/compress"
 	"github.com/minio/minio/cmd/config/dns"
@@ -258,7 +258,7 @@ var (
 	globalCompressConfig   compress.Config
 
 	// Some standard object extensions which we strictly dis-allow for compression.
-	standardExcludeCompressExtensions = []string{".gz", ".bz2", ".rar", ".zip", ".7z", ".xz", ".mp4", ".mkv", ".mov"}
+	standardExcludeCompressExtensions = []string{".gz", ".bz2", ".rar", ".zip", ".7z", ".xz", ".mp4", ".mkv", ".mov", ".jpg", ".png", ".gif"}
 
 	// Some standard content-types which we strictly dis-allow for compression.
 	standardExcludeCompressContentTypes = []string{"video/*", "audio/*", "application/zip", "application/x-gzip", "application/x-zip-compressed", " application/x-compress", "application/x-spoon"}
diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go
index 8a53b3dbf..ec890ef55 100644
--- a/cmd/web-handlers.go
+++ b/cmd/web-handlers.go
@@ -1688,6 +1688,9 @@ func (web *webAPIHandlers) DownloadZip(w http.ResponseWriter, r *http.Request) {
 	respElements := extractRespElements(w)
 
 	for i, object := range args.Objects {
+		if contextCanceled(ctx) {
+			return
+		}
 		// Writes compressed object file to the response.
 		zipit := func(objectName string) error {
 			var opts ObjectOptions
@@ -1706,38 +1709,26 @@ func (web *webAPIHandlers) DownloadZip(w http.ResponseWriter, r *http.Request) {
 				return err
 			}
 			header := &zip.FileHeader{
-				Name:     strings.TrimPrefix(objectName, args.Prefix),
-				Method:   zip.Deflate,
-				Flags:    1 << 11,
-				Modified: info.ModTime,
+				Name:               strings.TrimPrefix(objectName, args.Prefix),
+				Method:             zip.Deflate,
+				Flags:              1 << 11,
+				Modified:           info.ModTime,
+				UncompressedSize64: uint64(info.Size),
 			}
-			if hasStringSuffixInSlice(info.Name, standardExcludeCompressExtensions) || hasPattern(standardExcludeCompressContentTypes, info.ContentType) {
+			if info.Size < 20 || hasStringSuffixInSlice(info.Name, standardExcludeCompressExtensions) || hasPattern(standardExcludeCompressContentTypes, info.ContentType) {
 				// We strictly disable compression for standard extensions/content-types.
 				header.Method = zip.Store
 			}
 			writer, err := archive.CreateHeader(header)
 			if err != nil {
-				writeWebErrorResponse(w, errUnexpected)
 				return err
 			}
-			httpWriter := ioutil.WriteOnClose(writer)
 
 			// Write object content to response body
-			if _, err = io.Copy(httpWriter, gr); err != nil {
-				httpWriter.Close()
-				if !httpWriter.HasWritten() { // write error response only if no data or headers has been written to client yet
-					writeWebErrorResponse(w, err)
-				}
+			if _, err = io.Copy(writer, gr); err != nil {
 				return err
 			}
 
-			if err = httpWriter.Close(); err != nil {
-				if !httpWriter.HasWritten() { // write error response only if no data has been written to client yet
-					writeWebErrorResponse(w, err)
-					return err
-				}
-			}
-
 			// Notify object accessed via a GET request.
 			sendEvent(eventArgs{
 				EventName:    event.ObjectAccessedGet,