From 79ba6b480a8c2003645a1b237bd4da7c3a915522 Mon Sep 17 00:00:00 2001 From: Bryan Frimin Date: Thu, 2 Jul 2026 18:29:52 +0200 Subject: [PATCH] Fix range issue Signed-off-by: Bryan Frimin --- pkg/filemanager/s3.go | 68 +++++++++++++++++-- pkg/filemanager/s3_test.go | 105 +++++++++++++++++++++++++++++ pkg/server/api/files/v1/handler.go | 1 + 3 files changed, 170 insertions(+), 4 deletions(-) diff --git a/pkg/filemanager/s3.go b/pkg/filemanager/s3.go index d12218308..b985bdc49 100644 --- a/pkg/filemanager/s3.go +++ b/pkg/filemanager/s3.go @@ -45,6 +45,7 @@ type FileObject struct { type FileConditions struct { IfNoneMatch string IfModifiedSince time.Time + IfRange string Range string } @@ -116,7 +117,36 @@ func (s *Service) OpenFile( } if conds.Range != "" { - input.Range = &conds.Range + honorRange := true + + if conds.IfRange != "" { + head, err := s.s3Client.HeadObject( + ctx, + &s3.HeadObjectInput{ + Bucket: new(file.BucketName), + Key: new(file.FileKey), + }, + ) + if err != nil { + return nil, fmt.Errorf("cannot head file in S3: %w", err) + } + + etag := "" + if head.ETag != nil { + etag = *head.ETag + } + + lastModified := file.UpdatedAt + if head.LastModified != nil { + lastModified = *head.LastModified + } + + honorRange = ifRangeMatches(conds.IfRange, etag, lastModified) + } + + if honorRange { + input.Range = &conds.Range + } } result, err := s.s3Client.GetObject(ctx, input) @@ -150,10 +180,11 @@ func (s *Service) OpenFile( obj.LastModified = *result.LastModified } - // A Range request that S3 honors comes back as 206 Partial Content with a + // A Range 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. + // unranged request (or one whose Range we dropped above after an If-Range + // mismatch) 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 @@ -234,6 +265,35 @@ func (s *Service) GeneratePresignedURL( return presignedReq.URL, nil } +// ifRangeMatches reports whether an If-Range validator still matches the +// current representation, following the strong-comparison rules of RFC 9110 +// section 13.1.3. S3 has no native If-Range support, so callers evaluate it +// before deciding whether to forward a Range header. An entity-tag validator +// must strong-match the current ETag (weak tags on either side never satisfy +// If-Range); otherwise the validator is an HTTP-date compared for equality +// against Last-Modified. +func ifRangeMatches(ifRange, etag string, lastModified time.Time) bool { + ifRange = strings.TrimSpace(ifRange) + if ifRange == "" { + return true + } + + if strings.HasPrefix(ifRange, `"`) { + return etag != "" && !strings.HasPrefix(etag, `W/`) && ifRange == etag + } + + if strings.HasPrefix(ifRange, `W/`) { + return false + } + + t, err := http.ParseTime(ifRange) + if err != nil { + return false + } + + return !lastModified.IsZero() && lastModified.Truncate(time.Second).Equal(t) +} + func asciiFilename(filename string) string { var b strings.Builder b.Grow(len(filename)) diff --git a/pkg/filemanager/s3_test.go b/pkg/filemanager/s3_test.go index 052c9bbc1..f3d08071a 100644 --- a/pkg/filemanager/s3_test.go +++ b/pkg/filemanager/s3_test.go @@ -139,6 +139,111 @@ func TestOpenFile_RangeRequestReturnsPartialContent(t *testing.T) { assert.Equal(t, content[:5], string(body)) } +func TestOpenFile_IfRangeMatchHonorsRange(t *testing.T) { + t.Parallel() + + const ( + etag = `"abc123"` + content = "hello world" + ) + + svc := newTestS3Service( + t, + func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodHead { + w.Header().Set("ETag", etag) + w.WriteHeader(http.StatusOK) + return + } + + 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", IfRange: etag}, + ) + require.NoError(t, err) + require.NotNil(t, obj) + + defer func() { _ = obj.Body.Close() }() + + assert.True(t, obj.PartialContent) + assert.Equal(t, "bytes 0-4/11", obj.ContentRange) + + body, err := io.ReadAll(obj.Body) + require.NoError(t, err) + assert.Equal(t, content[:5], string(body)) +} + +func TestOpenFile_IfRangeMismatchServesFullContent(t *testing.T) { + t.Parallel() + + const ( + staleETag = `"old"` + currentETag = `"new"` + content = "hello world" + ) + + svc := newTestS3Service( + t, + func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodHead { + w.Header().Set("ETag", currentETag) + w.WriteHeader(http.StatusOK) + + return + } + + // The stale If-Range guard must have dropped the Range so S3 + // returns the full object rather than a 206 of the fresh bytes. + assert.Empty(t, r.Header.Get("Range")) + + w.Header().Set("ETag", currentETag) + _, _ = io.WriteString(w, content) + }, + ) + + 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", IfRange: staleETag}, + ) + require.NoError(t, err) + require.NotNil(t, obj) + + defer func() { _ = obj.Body.Close() }() + + assert.False(t, obj.PartialContent) + assert.Empty(t, obj.ContentRange) + + body, err := io.ReadAll(obj.Body) + require.NoError(t, err) + assert.Equal(t, content, string(body)) +} + func TestOpenFile_RangeNotSatisfiable(t *testing.T) { t.Parallel() diff --git a/pkg/server/api/files/v1/handler.go b/pkg/server/api/files/v1/handler.go index 4d5f392f5..7fa6593db 100644 --- a/pkg/server/api/files/v1/handler.go +++ b/pkg/server/api/files/v1/handler.go @@ -125,6 +125,7 @@ func (h *Handler) handleGetPublicFile(w http.ResponseWriter, r *http.Request) { conds := filemanager.FileConditions{ IfNoneMatch: r.Header.Get("If-None-Match"), + IfRange: r.Header.Get("If-Range"), Range: r.Header.Get("Range"), } if ifModifiedSince := r.Header.Get("If-Modified-Since"); ifModifiedSince != "" {