From e9ba938876e020ad5094cd51ae09ba80356a9f40 Mon Sep 17 00:00:00 2001
From: PJ <peterjan.brone@gmail.com>
Date: Wed, 6 Mar 2024 16:56:23 +0100
Subject: [PATCH] worker: implement CR remarks

---
 api/object.go           | 49 ++------------------------------------
 worker/client/client.go | 52 ++++++++++++++++++++++++++++++++++-------
 worker/worker.go        |  3 ++-
 3 files changed, 48 insertions(+), 56 deletions(-)

diff --git a/api/object.go b/api/object.go
index f53dbb5ac..4b1993341 100644
--- a/api/object.go
+++ b/api/object.go
@@ -85,13 +85,8 @@ type (
 
 	// GetObjectResponse is the response type for the GET /worker/object endpoint.
 	GetObjectResponse struct {
-		Content      io.ReadCloser      `json:"content"`
-		ContentType  string             `json:"contentType"`
-		LastModified string             `json:"lastModified"`
-		Range        *DownloadRange     `json:"range,omitempty"`
-		Size         int64              `json:"size"`
-		Metadata     ObjectUserMetadata `json:"metadata"`
-		// NOTE: keep HeadObjectResponse in sync with this type
+		Content io.ReadCloser `json:"content"`
+		HeadObjectResponse
 	}
 
 	// HeadObjectResponse is the response type for the HEAD /worker/object endpoint.
@@ -145,46 +140,6 @@ type (
 	}
 )
 
-func ParseObjectHeadResponseFrom(header http.Header) (HeadObjectResponse, error) {
-	// parse size
-	var size int64
-	_, err := fmt.Sscan(header.Get("Content-Length"), &size)
-	if err != nil {
-		return HeadObjectResponse{}, err
-	}
-
-	// parse range
-	var r *DownloadRange
-	if cr := header.Get("Content-Range"); cr != "" {
-		dr, err := ParseDownloadRange(cr)
-		if err != nil {
-			return HeadObjectResponse{}, err
-		}
-		r = &dr
-
-		// if a range is set, the size is the size extracted from the range
-		// since Content-Length will then only be the length of the returned
-		// range.
-		size = dr.Size
-	}
-
-	// parse headers
-	headers := make(map[string]string)
-	for k, v := range header {
-		if len(v) > 0 {
-			headers[k] = v[0]
-		}
-	}
-
-	return HeadObjectResponse{
-		ContentType:  header.Get("Content-Type"),
-		LastModified: header.Get("Last-Modified"),
-		Range:        r,
-		Size:         size,
-		Metadata:     ExtractObjectUserMetadataFrom(headers),
-	}, nil
-}
-
 func ExtractObjectUserMetadataFrom(metadata map[string]string) ObjectUserMetadata {
 	oum := make(map[string]string)
 	for k, v := range metadata {
diff --git a/worker/client/client.go b/worker/client/client.go
index ff8625541..410e4c66e 100644
--- a/worker/client/client.go
+++ b/worker/client/client.go
@@ -107,7 +107,7 @@ func (c *Client) HeadObject(ctx context.Context, bucket, path string, opts api.H
 		return nil, errors.New(string(err))
 	}
 
-	head, err := api.ParseObjectHeadResponseFrom(resp.Header)
+	head, err := parseObjectResponseHeaders(resp.Header)
 	if err != nil {
 		return nil, err
 	}
@@ -132,18 +132,14 @@ func (c *Client) GetObject(ctx context.Context, bucket, path string, opts api.Do
 		}
 	}()
 
-	head, err := api.ParseObjectHeadResponseFrom(header)
+	head, err := parseObjectResponseHeaders(header)
 	if err != nil {
 		return nil, err
 	}
 
 	return &api.GetObjectResponse{
-		Content:      body,
-		ContentType:  head.ContentType,
-		LastModified: head.LastModified,
-		Range:        head.Range,
-		Size:         head.Size,
-		Metadata:     head.Metadata,
+		Content:            body,
+		HeadObjectResponse: head,
 	}, nil
 }
 
@@ -296,6 +292,46 @@ func (c *Client) object(ctx context.Context, bucket, path string, opts api.Downl
 	return resp.Body, resp.Header, err
 }
 
+func parseObjectResponseHeaders(header http.Header) (api.HeadObjectResponse, error) {
+	// parse size
+	var size int64
+	_, err := fmt.Sscan(header.Get("Content-Length"), &size)
+	if err != nil {
+		return api.HeadObjectResponse{}, err
+	}
+
+	// parse range
+	var r *api.DownloadRange
+	if cr := header.Get("Content-Range"); cr != "" {
+		dr, err := api.ParseDownloadRange(cr)
+		if err != nil {
+			return api.HeadObjectResponse{}, err
+		}
+		r = &dr
+
+		// if a range is set, the size is the size extracted from the range
+		// since Content-Length will then only be the length of the returned
+		// range.
+		size = dr.Size
+	}
+
+	// parse headers
+	headers := make(map[string]string)
+	for k, v := range header {
+		if len(v) > 0 {
+			headers[k] = v[0]
+		}
+	}
+
+	return api.HeadObjectResponse{
+		ContentType:  header.Get("Content-Type"),
+		LastModified: header.Get("Last-Modified"),
+		Range:        r,
+		Size:         size,
+		Metadata:     api.ExtractObjectUserMetadataFrom(headers),
+	}, nil
+}
+
 func sizeFromSeeker(r io.Reader) (int64, error) {
 	s, ok := r.(io.Seeker)
 	if !ok {
diff --git a/worker/worker.go b/worker/worker.go
index 729c544c7..498269bf4 100644
--- a/worker/worker.go
+++ b/worker/worker.go
@@ -889,7 +889,8 @@ func (w *worker) objectsHandlerHEAD(jc jape.Context) {
 		return
 	}
 
-	// serve the content
+	// serve the content to ensure we're setting the exact same headers as we
+	// would for a GET request
 	status, err := serveContent(jc.ResponseWriter, jc.Request, *res.Object, func(io.Writer, int64, int64) error { return nil })
 	if errors.Is(err, http_range.ErrInvalid) || errors.Is(err, errMultiRangeNotSupported) {
 		jc.Error(err, http.StatusBadRequest)