From 7495d1d5a01ef3585de2e0ad411446f8cc2e7f5f Mon Sep 17 00:00:00 2001 From: Bryan Frimin Date: Thu, 2 Jul 2026 17:56:03 +0200 Subject: [PATCH] Add range support Signed-off-by: Bryan Frimin --- e2e/trust/trust_center_logo_test.go | 11 ++-- pkg/filemanager/s3.go | 41 ++++++++++++--- pkg/filemanager/s3_test.go | 81 +++++++++++++++++++++++++++++ pkg/server/api/files/v1/handler.go | 20 ++++++- 4 files changed, 141 insertions(+), 12 deletions(-) diff --git a/e2e/trust/trust_center_logo_test.go b/e2e/trust/trust_center_logo_test.go index 39c3c4099..045f8d991 100644 --- a/e2e/trust/trust_center_logo_test.go +++ b/e2e/trust/trust_center_logo_test.go @@ -19,6 +19,7 @@ import ( "net/http" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -166,9 +167,13 @@ func TestTrustCenter_LogoFileDownloadURL(t *testing.T) { downloadURL, ) + // Match the e2e HTTP client convention (see internal/testutil) so a hung + // server fails the request instead of blocking the parallel suite forever. + httpClient := &http.Client{Timeout: 30 * time.Second} + // The public endpoint streams the file bytes directly (no presigned // redirect) with cache headers, so the stable URL is CDN/browser cacheable. - resp, err := http.Get(downloadURL) + resp, err := httpClient.Get(downloadURL) require.NoError(t, err) defer func() { _ = resp.Body.Close() }() @@ -198,7 +203,7 @@ func TestTrustCenter_LogoFileDownloadURL(t *testing.T) { require.NoError(t, err) revalidateReq.Header.Set("If-None-Match", etag) - revalidateResp, err := http.DefaultClient.Do(revalidateReq) + revalidateResp, err := httpClient.Do(revalidateReq) require.NoError(t, err) defer func() { _ = revalidateResp.Body.Close() }() @@ -214,7 +219,7 @@ func TestTrustCenter_LogoFileDownloadURL(t *testing.T) { require.NoError(t, err) sinceReq.Header.Set("If-Modified-Since", lastModified) - sinceResp, err := http.DefaultClient.Do(sinceReq) + sinceResp, err := httpClient.Do(sinceReq) require.NoError(t, err) defer func() { _ = sinceResp.Body.Close() }() diff --git a/pkg/filemanager/s3.go b/pkg/filemanager/s3.go index d8ccca140..d12218308 100644 --- a/pkg/filemanager/s3.go +++ b/pkg/filemanager/s3.go @@ -31,17 +31,21 @@ import ( ) type FileObject struct { - Body io.ReadCloser - ContentType string - ContentLength int64 - ETag string - LastModified time.Time - NotModified bool + Body io.ReadCloser + ContentType string + ContentLength int64 + ContentRange string + ETag string + LastModified time.Time + NotModified bool + PartialContent bool + RangeNotSatisfiable bool } type FileConditions struct { IfNoneMatch string IfModifiedSince time.Time + Range string } func (s *Service) GetFileBase64( @@ -111,11 +115,21 @@ func (s *Service) OpenFile( input.IfModifiedSince = &conds.IfModifiedSince } + if conds.Range != "" { + input.Range = &conds.Range + } + result, err := s.s3Client.GetObject(ctx, input) if err != nil { if respErr, ok := errors.AsType[*smithyhttp.ResponseError](err); ok { - if respErr.HTTPStatusCode() == http.StatusNotModified { + switch respErr.HTTPStatusCode() { + case http.StatusNotModified: return &FileObject{NotModified: true}, nil + case http.StatusRequestedRangeNotSatisfiable: + return &FileObject{ + RangeNotSatisfiable: true, + ContentLength: file.FileSize, + }, nil } } @@ -136,6 +150,19 @@ func (s *Service) OpenFile( obj.LastModified = *result.LastModified } + // A Range request that S3 honors comes back as 206 Partial Content with a + // Content-Range header and a ContentLength scoped to the returned slice. An + // If-Range mismatch (or no Range) yields a normal 200 with the full object, + // so we only override the length/status when Content-Range is present. + if result.ContentRange != nil { + obj.ContentRange = *result.ContentRange + obj.PartialContent = true + } + + if result.ContentLength != nil { + obj.ContentLength = *result.ContentLength + } + return obj, nil } diff --git a/pkg/filemanager/s3_test.go b/pkg/filemanager/s3_test.go index 71005f089..052c9bbc1 100644 --- a/pkg/filemanager/s3_test.go +++ b/pkg/filemanager/s3_test.go @@ -91,6 +91,87 @@ func TestOpenFile_StreamsBody(t *testing.T) { assert.Equal(t, content, string(body)) } +func TestOpenFile_RangeRequestReturnsPartialContent(t *testing.T) { + t.Parallel() + + const ( + etag = `"abc123"` + content = "hello world" + ) + + svc := newTestS3Service( + t, + func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "bytes=0-4", r.Header.Get("Range")) + + w.Header().Set("ETag", etag) + w.Header().Set("Content-Range", "bytes 0-4/11") + w.Header().Set("Content-Length", "5") + w.WriteHeader(http.StatusPartialContent) + _, _ = io.WriteString(w, content[:5]) + }, + ) + + file := &coredata.File{ + BucketName: "uploads", + FileKey: "tenant/file", + MimeType: "text/plain", + FileSize: int64(len(content)), + } + + obj, err := svc.OpenFile( + context.Background(), + file, + filemanager.FileConditions{Range: "bytes=0-4"}, + ) + require.NoError(t, err) + require.NotNil(t, obj) + + defer func() { _ = obj.Body.Close() }() + + assert.True(t, obj.PartialContent) + assert.False(t, obj.NotModified) + assert.Equal(t, "bytes 0-4/11", obj.ContentRange) + assert.Equal(t, int64(5), obj.ContentLength) + + body, err := io.ReadAll(obj.Body) + require.NoError(t, err) + assert.Equal(t, content[:5], string(body)) +} + +func TestOpenFile_RangeNotSatisfiable(t *testing.T) { + t.Parallel() + + const content = "hello world" + + svc := newTestS3Service( + t, + func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Range", "bytes */11") + w.WriteHeader(http.StatusRequestedRangeNotSatisfiable) + }, + ) + + file := &coredata.File{ + BucketName: "uploads", + FileKey: "tenant/file", + MimeType: "text/plain", + FileSize: int64(len(content)), + } + + obj, err := svc.OpenFile( + context.Background(), + file, + filemanager.FileConditions{Range: "bytes=999-1000"}, + ) + require.NoError(t, err) + require.NotNil(t, obj) + assert.True(t, obj.RangeNotSatisfiable) + assert.False(t, obj.PartialContent) + assert.Nil(t, obj.Body) + assert.Equal(t, int64(len(content)), obj.ContentLength) +} + func TestOpenFile_NotModifiedByETag(t *testing.T) { t.Parallel() diff --git a/pkg/server/api/files/v1/handler.go b/pkg/server/api/files/v1/handler.go index 435559928..4d5f392f5 100644 --- a/pkg/server/api/files/v1/handler.go +++ b/pkg/server/api/files/v1/handler.go @@ -123,7 +123,10 @@ func (h *Handler) handleGetPublicFile(w http.ResponseWriter, r *http.Request) { return } - conds := filemanager.FileConditions{IfNoneMatch: r.Header.Get("If-None-Match")} + conds := filemanager.FileConditions{ + IfNoneMatch: r.Header.Get("If-None-Match"), + Range: r.Header.Get("Range"), + } if ifModifiedSince := r.Header.Get("If-Modified-Since"); ifModifiedSince != "" { if t, parseErr := http.ParseTime(ifModifiedSince); parseErr == nil { conds.IfModifiedSince = t @@ -144,6 +147,7 @@ func (h *Handler) handleGetPublicFile(w http.ResponseWriter, r *http.Request) { } w.Header().Set("Cache-Control", "public, max-age=31536000, immutable") + w.Header().Set("Accept-Ranges", "bytes") if obj.ETag != "" { w.Header().Set("ETag", obj.ETag) @@ -154,15 +158,27 @@ func (h *Handler) handleGetPublicFile(w http.ResponseWriter, r *http.Request) { return } + if obj.RangeNotSatisfiable { + w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", file.FileSize)) + w.WriteHeader(http.StatusRequestedRangeNotSatisfiable) + + return + } + defer func() { _ = obj.Body.Close() }() w.Header().Set("Content-Type", file.MimeType) - w.Header().Set("Content-Length", strconv.FormatInt(file.FileSize, 10)) + w.Header().Set("Content-Length", strconv.FormatInt(obj.ContentLength, 10)) if !obj.LastModified.IsZero() { w.Header().Set("Last-Modified", obj.LastModified.UTC().Format(http.TimeFormat)) } + if obj.PartialContent { + w.Header().Set("Content-Range", obj.ContentRange) + w.WriteHeader(http.StatusPartialContent) + } + if _, err := io.Copy(w, obj.Body); err != nil { h.logger.ErrorCtx( r.Context(),