[Bug] Write headers just before writing contents (#6726)

* Write headers just before writing contents
* Add test

esti/s3_gateway_test.go

@@ -4,9 +4,12 @@ import (
 	"bytes"
 	"io"
 	"math/rand"
+	"net/http"
+	"net/url"
 	"strings"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/minio/minio-go/v7"
 	"github.com/minio/minio-go/v7/pkg/credentials"
@@ -168,6 +171,25 @@ func TestS3ReadObject(t *testing.T) {
 				t.Errorf("Got contents \"%s\" but expected \"%s\"", string(got), contents)
 			}
 		})
+		t.Run("ExistsPreSigned", func(t *testing.T) {
+			// using presigned URL so we can check the headers
+			// We expect the Content-Length header to be set
+			preSignedURL, err := client.Presign(ctx, http.MethodGet, repo, goodPath, time.Second*60, url.Values{})
+			if err != nil {
+				t.Fatalf("client.Presign(%s, %s): %s", repo, goodPath, err)
+			}
+
+			resp, err := http.Get(preSignedURL.String())
+			if err != nil {
+				t.Fatalf("http.Get %s: %s", preSignedURL.String(), err)
+			}
+			defer func() { _ = resp.Body.Close() }()
+			contentLength := resp.Header.Get("Content-Length")
+			if contentLength == "" {
+				t.Errorf("Expected Content-Length header to be set")
+			}
+		})
+
 		t.Run("Doesn't exist", func(t *testing.T) {
 			res, err := client.GetObject(ctx, repo, badPath, minio.GetObjectOptions{})
 			if err != nil {

pkg/gateway/operations/getobject.go

@@ -29,6 +29,7 @@ func (controller *GetObject) RequiredPermissions(_ *http.Request, repoID, _, pat
 
 func (controller *GetObject) Handle(w http.ResponseWriter, req *http.Request, o *PathOperation) {
 	o.Incr("get_object", o.Principal, o.Repository.Name, o.Reference)
+	ctx := req.Context()
 	query := req.URL.Query()
 	if _, exists := query["versioning"]; exists {
 		o.EncodeResponse(w, req, serde.VersioningConfiguration{}, http.StatusOK)
@@ -41,7 +42,7 @@ func (controller *GetObject) Handle(w http.ResponseWriter, req *http.Request, o
 	}
 
 	beforeMeta := time.Now()
-	entry, err := o.Catalog.GetEntry(req.Context(), o.Repository.Name, o.Reference, o.Path, catalog.GetEntryParams{})
+	entry, err := o.Catalog.GetEntry(ctx, o.Repository.Name, o.Reference, o.Path, catalog.GetEntryParams{})
 	metaTook := time.Since(beforeMeta)
 	o.Log(req).
 		WithField("took", metaTook).
@@ -61,11 +62,7 @@ func (controller *GetObject) Handle(w http.ResponseWriter, req *http.Request, o
 		_ = o.EncodeError(w, req, err, gatewayerrors.Codes.ToAPIErr(gatewayerrors.ErrInternalError))
 		return
 	}
-	o.SetHeader(w, "Last-Modified", httputil.HeaderTimestamp(entry.CreationDate))
-	o.SetHeader(w, "ETag", httputil.ETag(entry.Checksum))
-	o.SetHeader(w, "Content-Type", entry.ContentType)
-	o.SetHeader(w, "Accept-Ranges", "bytes")
-	amzMetaWriteHeaders(w, entry.Metadata)
+
 	// TODO: the rest of https://docs.aws.amazon.com/en_pv/AmazonS3/latest/API/API_GetObject.html
 	// range query
 	var data io.ReadCloser
@@ -83,38 +80,43 @@ func (controller *GetObject) Handle(w http.ResponseWriter, req *http.Request, o
 		}
 		// by here, we have a range we can use.
 	}
+
+	statusCode := http.StatusOK
+	contentLength := entry.Size
+	contentRange := ""
+	objectPointer := block.ObjectPointer{
+		StorageNamespace: o.Repository.StorageNamespace,
+		IdentifierType:   entry.AddressType.ToIdentifierType(),
+		Identifier:       entry.PhysicalAddress,
+	}
+
 	if rangeSpec == "" || err != nil {
 		// assemble a response body (range-less query)
-		o.SetHeader(w, "Content-Type", entry.ContentType)
-		o.SetHeader(w, "Content-Length", fmt.Sprintf("%d", entry.Size))
-		data, err = o.BlockStore.Get(req.Context(), block.ObjectPointer{
-			StorageNamespace: o.Repository.StorageNamespace,
-			IdentifierType:   entry.AddressType.ToIdentifierType(),
-			Identifier:       entry.PhysicalAddress,
-		}, entry.Size)
-		if err != nil {
-			_ = o.EncodeError(w, req, err, gatewayerrors.Codes.ToAPIErr(gatewayerrors.ErrInternalError))
-			return
-		}
+		data, err = o.BlockStore.Get(ctx, objectPointer, entry.Size)
 	} else {
-		contentRange := fmt.Sprintf("bytes %d-%d/%d", rng.StartOffset, rng.EndOffset, entry.Size)
-		o.SetHeader(w, "Content-Range", contentRange)
-		o.SetHeader(w, "Content-Length", fmt.Sprintf("%d", rng.Size()))
-		data, err = o.BlockStore.GetRange(req.Context(), block.ObjectPointer{
-			StorageNamespace: o.Repository.StorageNamespace,
-			IdentifierType:   entry.AddressType.ToIdentifierType(),
-			Identifier:       entry.PhysicalAddress,
-		}, rng.StartOffset, rng.EndOffset)
-		if err != nil {
-			_ = o.EncodeError(w, req, err, gatewayerrors.Codes.ToAPIErr(gatewayerrors.ErrInternalError))
-			return
-		}
-		w.WriteHeader(http.StatusPartialContent)
+		contentLength = rng.Size()
+		contentRange = fmt.Sprintf("bytes %d-%d/%d", rng.StartOffset, rng.EndOffset, entry.Size)
+		statusCode = http.StatusPartialContent
+		data, err = o.BlockStore.GetRange(ctx, objectPointer, rng.StartOffset, rng.EndOffset)
+	}
+	if err != nil {
+		_ = o.EncodeError(w, req, err, gatewayerrors.Codes.ToAPIErr(gatewayerrors.ErrInternalError))
+		return
 	}
 
+	o.SetHeader(w, "Last-Modified", httputil.HeaderTimestamp(entry.CreationDate))
+	o.SetHeader(w, "ETag", httputil.ETag(entry.Checksum))
+	o.SetHeader(w, "Content-Type", entry.ContentType)
+	o.SetHeader(w, "Accept-Ranges", "bytes")
+	if contentRange != "" {
+		o.SetHeader(w, "Content-Range", contentRange)
+	}
+	o.SetHeader(w, "Content-Length", fmt.Sprintf("%d", contentLength))
 	o.SetHeader(w, "X-Content-Type-Options", "nosniff")
 	o.SetHeader(w, "X-Frame-Options", "SAMEORIGIN")
 	o.SetHeader(w, "Content-Security-Policy", "default-src 'none'")
+	amzMetaWriteHeaders(w, entry.Metadata)
+	w.WriteHeader(statusCode)
 
 	defer func() {
 		_ = data.Close()