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

@@ -19,7 +19,6 @@ import (
 	"path"
 	"slices"
 	"sort"
-	"strconv"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -52,6 +51,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"
 )
@@ -2347,6 +2347,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
 	}
@@ -2380,22 +2390,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

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)

lib/rest/headers.go

@@ -10,45 +10,49 @@ import (
 	"github.com/rclone/rclone/fs"
 )
 
-type contentRange struct {
-	start int64
-	end   int64
-	size  int64
+// ContentRange is a parsed Content-Range response header.
+type ContentRange struct {
+	Start int64 // first byte position of the range
+	End   int64 // last byte position of the range (inclusive)
+	Size  int64 // complete length of the representation, or -1 if unknown
 }
 
-func parseContentRange(value string) (contentRange, error) {
+// ParseContentRange parses a Content-Range header of the form
+// "bytes start-end/size" as returned with a ranged (206) response.
+// The Size is -1 if the complete length is unknown ("*").
+func ParseContentRange(value string) (ContentRange, error) {
 	const prefix = "bytes "
 	if !strings.HasPrefix(value, prefix) {
-		return contentRange{}, fmt.Errorf("doesn't start with %q", prefix)
+		return ContentRange{}, fmt.Errorf("doesn't start with %q", prefix)
 	}
 
 	rangeAndSize := strings.Split(value[len(prefix):], "/")
 	if len(rangeAndSize) != 2 {
-		return contentRange{}, errors.New("must contain one '/'")
+		return ContentRange{}, errors.New("must contain one '/'")
 	}
 	bounds := strings.Split(rangeAndSize[0], "-")
 	if len(bounds) != 2 {
-		return contentRange{}, errors.New("must contain one '-'")
+		return ContentRange{}, errors.New("must contain one '-'")
 	}
 
 	start, err := strconv.ParseInt(bounds[0], 10, 64)
 	if err != nil || start < 0 {
-		return contentRange{}, errors.New("invalid start")
+		return ContentRange{}, errors.New("invalid start")
 	}
 	end, err := strconv.ParseInt(bounds[1], 10, 64)
 	if err != nil || end < start {
-		return contentRange{}, errors.New("invalid end")
+		return ContentRange{}, errors.New("invalid end")
 	}
 
 	size := int64(-1)
 	if rangeAndSize[1] != "*" {
 		size, err = strconv.ParseInt(rangeAndSize[1], 10, 64)
 		if err != nil || size < 0 || end >= size {
-			return contentRange{}, errors.New("invalid complete length")
+			return ContentRange{}, errors.New("invalid complete length")
 		}
 	}
 
-	return contentRange{start: start, end: end, size: size}, nil
+	return ContentRange{Start: start, End: end, Size: size}, nil
 }
 
 // CheckContentRange checks that a response satisfies a Range open option.
@@ -106,18 +110,18 @@ func CheckContentRange(resp *http.Response, options []fs.OpenOption, size int64)
 	}
 
 	responseRange := resp.Header.Get("Content-Range")
-	got, err := parseContentRange(responseRange)
+	got, err := ParseContentRange(responseRange)
 	if err != nil {
 		return fmt.Errorf("invalid Content-Range %q: %w", responseRange, err)
 	}
 
-	if size >= 0 && got.size >= 0 && got.size != size {
+	if size >= 0 && got.Size >= 0 && got.Size != size {
 		return fmt.Errorf("Content-Range %q does not match expected size %d", responseRange, size)
 	}
 
 	rangeSize := size
 	if rangeSize < 0 {
-		rangeSize = got.size
+		rangeSize = got.Size
 	}
 	expectedStart := requested.Start
 	expectedEnd := requested.End
@@ -135,18 +139,18 @@ func CheckContentRange(resp *http.Response, options []fs.OpenOption, size int64)
 
 	var matches bool
 	if requested.Start < 0 && rangeSize < 0 {
-		matches = got.end-got.start+1 <= requested.End
+		matches = got.End-got.Start+1 <= requested.End
 	} else {
-		matches = got.start == expectedStart
+		matches = got.Start == expectedStart
 		if expectedEnd >= 0 {
-			matches = matches && got.end == expectedEnd
+			matches = matches && got.End == expectedEnd
 		}
 	}
 	if !matches {
 		return fmt.Errorf("Content-Range %q does not match requested range %q", responseRange, requestRange)
 	}
 
-	contentLength := got.end - got.start + 1
+	contentLength := got.End - got.Start + 1
 	if contentLength <= 0 {
 		return fmt.Errorf("invalid Content-Range %q: length overflows", responseRange)
 	}