Fix GetObject returning invalid object size when range is requested (#…

…660)

* worker: fix GetObject returning invalid object size when range is requested

* worker: return error in case of multirange

* worker: address review comment

api/worker.go

@@ -252,6 +252,7 @@ func UploadWithEncryptionOffset(offset int64) UploadOption {
 type DownloadRange struct {
 	Start  int64
 	Length int64
+	Size   int64
 }
 
 type UploadObjectResponse struct {
@@ -304,8 +305,13 @@ func ParseDownloadRange(contentRange string) (DownloadRange, error) {
 	if err != nil {
 		return DownloadRange{}, err
 	}
+	size, err := strconv.ParseInt(parts[1], 10, 64)
+	if err != nil {
+		return DownloadRange{}, err
+	}
 	return DownloadRange{
 		Start:  start,
 		Length: end - start + 1,
+		Size:   size,
 	}, nil
 }

internal/testing/cluster_test.go

@@ -1877,6 +1877,7 @@ func TestMultipartUploads(t *testing.T) {
 	etag2 := putPart(2, len(data1), data2)
 	etag1 := putPart(1, 0, data1)
 	etag3 := putPart(3, len(data1)+len(data2), data3)
+	size := int64(len(data1) + len(data2) + len(data3))
 
 	// List parts
 	mup, err := b.MultipartUploadParts(context.Background(), api.DefaultBucketName, objPath, mpr.UploadID, 0, 0)
@@ -1914,9 +1915,26 @@ func TestMultipartUploads(t *testing.T) {
 	// Download object
 	gor, err := w.GetObject(context.Background(), api.DefaultBucketName, objPath)
 	tt.OK(err)
-	if data, err := io.ReadAll(gor.Content); err != nil {
+	if gor.Range != nil {
+		t.Fatal("unexpected range:", gor.Range)
+	} else if gor.Size != size {
+		t.Fatal("unexpected size:", gor.Size)
+	} else if data, err := io.ReadAll(gor.Content); err != nil {
 		t.Fatal(err)
 	} else if expectedData := append(data1, append(data2, data3...)...); !bytes.Equal(data, expectedData) {
 		t.Fatal("unexpected data:", cmp.Diff(data, expectedData))
 	}
+
+	// Download a range of the object
+	gor, err = w.GetObject(context.Background(), api.DefaultBucketName, objPath, api.DownloadWithRange(0, 1))
+	tt.OK(err)
+	if gor.Range == nil || gor.Range.Start != 0 || gor.Range.Length != 1 {
+		t.Fatal("unexpected range:", gor.Range)
+	} else if gor.Size != size {
+		t.Fatal("unexpected size:", gor.Size)
+	} else if data, err := io.ReadAll(gor.Content); err != nil {
+		t.Fatal(err)
+	} else if expectedData := data1[:1]; !bytes.Equal(data, expectedData) {
+		t.Fatal("unexpected data:", cmp.Diff(data, expectedData))
+	}
 }

worker/client.go

@@ -336,6 +336,11 @@ func (c *Client) GetObject(ctx context.Context, bucket, path string, opts ...api
 			return nil, 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 Last-Modified
 	modTime, err := time.Parse(http.TimeFormat, header.Get("Last-Modified"))

worker/serve.go

@@ -26,6 +26,8 @@ type (
 	}
 )
 
+var errMultiRangeNotSupported = errors.New("multipart ranges are not supported")
+
 func newContentReader(r io.Reader, obj api.Object, offset int64) io.ReadSeeker {
 	return &contentReader{
 		r:          r,
@@ -92,9 +94,6 @@ func serveContent(rw http.ResponseWriter, req *http.Request, obj api.Object, dow
 	rw.Header().Set("ETag", api.FormatETag(buildETag(req, obj.ETag)))
 	rw.Header().Set("Content-Type", contentType)
 
-	// override the range request header to avoid seeks in http.ServeContent
-	req.Header.Set("Range", fmt.Sprintf("bytes=%d-%d", offset, offset+length-1))
-
 	http.ServeContent(rw, req, obj.Name, obj.ModTime, rs)
 	return http.StatusOK, nil
 }
@@ -109,11 +108,13 @@ func parseRangeHeader(req *http.Request, obj api.Object) (int64, int64, error) {
 	// extract requested offset and length
 	offset := int64(0)
 	length := obj.Size
-	if len(ranges) > 0 {
+	if len(ranges) == 1 {
 		offset, length = ranges[0].Start, ranges[0].Length
 		if offset < 0 || length < 0 || offset+length > obj.Size {
-			return 0, 0, fmt.Errorf("invalid range: %v %v", offset, length)
+			return 0, 0, fmt.Errorf("%w: %v %v", http_range.ErrInvalid, offset, length)
 		}
+	} else if len(ranges) > 1 {
+		return 0, 0, errMultiRangeNotSupported
 	}
 	return offset, length, nil
 }

worker/worker.go

@@ -15,6 +15,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/gotd/contrib/http_range"
 	"go.opentelemetry.io/otel/trace"
 	rhpv2 "go.sia.tech/core/rhp/v2"
 	rhpv3 "go.sia.tech/core/rhp/v3"
@@ -1062,7 +1063,11 @@ func (w *worker) objectsHandlerGET(jc jape.Context) {
 
 	// serve the content
 	status, err := serveContent(jc.ResponseWriter, jc.Request, *res.Object, downloadFn)
-	if err != nil {
+	if errors.Is(err, http_range.ErrInvalid) || errors.Is(err, errMultiRangeNotSupported) {
+		jc.Error(err, http.StatusBadRequest)
+	} else if errors.Is(err, http_range.ErrNoOverlap) {
+		jc.Error(err, http.StatusRequestedRangeNotSatisfiable)
+	} else if err != nil {
 		jc.Error(err, status)
 	}
 }