From bee45bccfd54b55e3bfc17f652e1e87e7b38be5c Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 17 Aug 2026 12:35:14 +0100 Subject: [PATCH] azureblob: fix spurious vfs cache corruption errors during chunked reads - fixes #9782 On a ranged download the metadata decoder stored the response's Content-Length (the length of the range, not the blob) in the object's size and only corrected it from the Content-Range total afterwards. Object.Size() is read concurrently by the VFS cache and chunked reader while a download is in progress, so with --vfs-read-chunk-size a reader could observe the chunk length (e.g. 67108864 for 64M chunks) as the object size. The VFS cache then logged vfs cache: cached file (N) is unexpectedly larger than the remote object (67108864). The cached file is likely corrupted after an unclean shutdown; recovering ... and truncated the read request against the bogus size, breaking sequential reads of large blobs with --vfs-cache-mode full. This applies the Content-Range correction before the size is stored so the range length is never published as the object size. --- backend/azureblob/azureblob.go | 28 +++++------- backend/azureblob/azureblob_internal_test.go | 46 ++++++++++++++++++++ 2 files changed, 57 insertions(+), 17 deletions(-) diff --git a/backend/azureblob/azureblob.go b/backend/azureblob/azureblob.go index 32b834754..642d34d2e 100644 --- a/backend/azureblob/azureblob.go +++ b/backend/azureblob/azureblob.go @@ -19,7 +19,6 @@ import ( "path" "slices" "sort" - "strconv" "strings" "sync" "sync/atomic" @@ -51,6 +50,7 @@ import ( "github.com/rclone/rclone/lib/pacer" "github.com/rclone/rclone/lib/pool" "github.com/rclone/rclone/lib/readers" + "github.com/rclone/rclone/lib/rest" "github.com/rclone/rclone/lib/transferaccounter" "golang.org/x/sync/errgroup" ) @@ -2345,6 +2345,16 @@ func (o *Object) decodeMetaDataFromDownloadResponse(info *blob.DownloadStreamRes } else { size = *info.ContentLength } + // On a range request Content-Length is the length of the range, not the object, so take the + // object's size from the total in Content-Range instead. + if info.ContentRange != nil { + contentRange, err := rest.ParseContentRange(*info.ContentRange) + if err != nil { + fs.Debugf(o, "Failed to parse Content-Range %q: %v", *info.ContentRange, err) + } else if contentRange.Size >= 0 { + size = contentRange.Size + } + } if isDirectoryMarker(size, metadata, o.remote) { return fs.ErrorNotAFile } @@ -2378,22 +2388,6 @@ func (o *Object) decodeMetaDataFromDownloadResponse(info *blob.DownloadStreamRes // o.accessTier = blob.AccessTier(*info.AccessTier) // } o.setMetadata(metadata) - - // If it was a Range request, the size is wrong, so correct it - if info.ContentRange != nil { - contentRange := *info.ContentRange - slash := strings.IndexRune(contentRange, '/') - if slash >= 0 { - i, err := strconv.ParseInt(contentRange[slash+1:], 10, 64) - if err == nil { - o.size = i - } else { - fs.Debugf(o, "Failed to find parse integer from in %q: %v", contentRange, err) - } - } else { - fs.Debugf(o, "Failed to find length in %q", contentRange) - } - } o.contentEncoding = info.ContentEncoding // If decompressing then size and md5sum are unknown diff --git a/backend/azureblob/azureblob_internal_test.go b/backend/azureblob/azureblob_internal_test.go index c234463c2..f8deb0fdb 100644 --- a/backend/azureblob/azureblob_internal_test.go +++ b/backend/azureblob/azureblob_internal_test.go @@ -54,6 +54,52 @@ func TestBlockIDCreator(t *testing.T) { assert.ErrorContains(t, bic2.checkID(chunkNumber, got), "random bytes") } +func TestDecodeMetaDataFromDownloadResponse(t *testing.T) { + pInt64 := func(i int64) *int64 { return &i } + newTestObject := func() *Object { + return &Object{ + fs: &Fs{}, + remote: "test.bin", + size: -2, // sentinel to check it gets set + } + } + + t.Run("WholeBlob", func(t *testing.T) { + o := newTestObject() + info := blob.DownloadStreamResponse{} + info.ContentLength = pInt64(12345) + require.NoError(t, o.decodeMetaDataFromDownloadResponse(&info)) + assert.Equal(t, int64(12345), o.size) + }) + + t.Run("RangeRequest", func(t *testing.T) { + // On a ranged download Content-Length is the chunk length so the + // size must come from the total in Content-Range + o := newTestObject() + info := blob.DownloadStreamResponse{} + info.ContentLength = pInt64(67108864) + info.ContentRange = pString("bytes 67108864-134217727/169721004032") + require.NoError(t, o.decodeMetaDataFromDownloadResponse(&info)) + assert.Equal(t, int64(169721004032), o.size) + }) + + t.Run("BadContentRange", func(t *testing.T) { + o := newTestObject() + info := blob.DownloadStreamResponse{} + info.ContentLength = pInt64(67108864) + info.ContentRange = pString("potato") + require.NoError(t, o.decodeMetaDataFromDownloadResponse(&info)) + assert.Equal(t, int64(67108864), o.size) + }) + + t.Run("UnknownLength", func(t *testing.T) { + o := newTestObject() + info := blob.DownloadStreamResponse{} + require.NoError(t, o.decodeMetaDataFromDownloadResponse(&info)) + assert.Equal(t, int64(-1), o.size) + }) +} + func TestCopySASTimingConstants(t *testing.T) { require.Greater(t, sasCopyStartSkew, time.Duration(0)) require.Greater(t, sasCopyValidity, sasCopyStartSkew)