dropbox: fix chunked uploads of truncated files never finishing - fixes #9704

A source which returned EOF before supplying as many bytes as it
declared would either commit a truncated file (if the shortfall was
within the final chunk) or loop forever appending empty chunks to the
upload session. Return an error wrapping io.ErrUnexpectedEOF instead.

Note that all dropbox uploads use the chunked upload path with the
default batch_mode of sync, so this affected uploads of every size.

backend/dropbox/dropbox.go

@@ -2111,6 +2111,13 @@ func (o *Object) uploadChunked(ctx context.Context, in0 io.Reader, commitInfo *f
 		if err != nil {
 			return nil, err
 		}
+		if size >= 0 {
+			// Check for sources which truncate early
+			expected := min(uint64(currentChunk)*uint64(chunkSize), uint64(size))
+			if in.BytesRead() < expected {
+				return nil, fmt.Errorf("expected %d bytes in input, but only read %d: %w", size, in.BytesRead(), io.ErrUnexpectedEOF)
+			}
+		}
 		if appendArg.Close {
 			break
 		}

backend/dropbox/dropbox_internal_test.go

@@ -2,6 +2,7 @@ package dropbox
 
 import (
 	"context"
+	"errors"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -13,6 +14,7 @@ import (
 	"github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files"
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/fstest/fstests"
+	"github.com/rclone/rclone/lib/batcher"
 	"github.com/rclone/rclone/lib/pacer"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -141,6 +143,98 @@ func TestPaperExportRemote(t *testing.T) {
 	assert.Equal(t, "document.md", legacy.Remote())
 }
 
+// uploadSessionClient is a mock files.ContextClient which records the
+// chunked upload calls made to it
+type uploadSessionClient struct {
+	files.ContextClient
+	appends      int    // number of UploadSessionAppendV2Context calls
+	maxAppends   int    // fail the append after this many calls to stop runaway loops
+	bytesWritten int64  // bytes received by UploadSessionAppendV2Context
+	finishCalled bool   // set if UploadSessionFinishContext was called
+}
+
+var errTooManyAppends = errors.New("too many appends - upload looping?")
+
+func (c *uploadSessionClient) UploadSessionStartContext(ctx context.Context, arg *files.UploadSessionStartArg, content io.Reader) (*files.UploadSessionStartResult, error) {
+	return &files.UploadSessionStartResult{SessionId: "session"}, nil
+}
+
+func (c *uploadSessionClient) UploadSessionAppendV2Context(ctx context.Context, arg *files.UploadSessionAppendArg, content io.Reader) error {
+	// the real client fails the request if the context is cancelled
+	if err := ctx.Err(); err != nil {
+		return err
+	}
+	c.appends++
+	if c.appends > c.maxAppends {
+		return errTooManyAppends
+	}
+	n, err := io.Copy(io.Discard, content)
+	if err != nil {
+		return err
+	}
+	c.bytesWritten += n
+	return nil
+}
+
+func (c *uploadSessionClient) UploadSessionFinishContext(ctx context.Context, arg *files.UploadSessionFinishArg, content io.Reader) (*files.FileMetadata, error) {
+	c.finishCalled = true
+	return &files.FileMetadata{}, nil
+}
+
+// newUploadTestFs makes an Fs with a mock srv for testing uploadChunked
+func newUploadTestFs(t *testing.T, srv files.ContextClient, chunkSize fs.SizeSuffix) *Fs {
+	ctx := context.Background()
+	f := &Fs{
+		pacer: fs.NewPacer(ctx, pacer.NewDefault(pacer.MinSleep(time.Millisecond), pacer.MaxSleep(2*time.Millisecond))),
+		srv:   srv,
+	}
+	f.opt.ChunkSize = chunkSize
+	batcherOptions := defaultBatcherOptions
+	batcherOptions.Mode = "off"
+	var err error
+	f.batcher, err = batcher.New(ctx, f, f.commitBatch, batcherOptions)
+	require.NoError(t, err)
+	return f
+}
+
+func TestUploadChunkedEarlyEOF(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("MultiChunk", func(t *testing.T) {
+		// The declared size spans 4 chunks but the source ends after 1.5
+		client := &uploadSessionClient{maxAppends: 8}
+		f := newUploadTestFs(t, client, 100)
+		o := &Object{fs: f, remote: "test.bin"}
+		_, err := o.uploadChunked(ctx, strings.NewReader(strings.Repeat("a", 150)), files.NewCommitInfo("/test.bin"), 400)
+		require.Error(t, err)
+		assert.ErrorIs(t, err, io.ErrUnexpectedEOF)
+		assert.False(t, client.finishCalled, "must not commit a truncated upload")
+	})
+
+	t.Run("SingleChunk", func(t *testing.T) {
+		// The declared size fits in one chunk but the source ends early
+		client := &uploadSessionClient{maxAppends: 8}
+		f := newUploadTestFs(t, client, 500)
+		o := &Object{fs: f, remote: "test.bin"}
+		_, err := o.uploadChunked(ctx, strings.NewReader(strings.Repeat("a", 150)), files.NewCommitInfo("/test.bin"), 400)
+		require.Error(t, err)
+		assert.ErrorIs(t, err, io.ErrUnexpectedEOF)
+		assert.False(t, client.finishCalled, "must not commit a truncated upload")
+	})
+
+	t.Run("Complete", func(t *testing.T) {
+		// A source which supplies exactly the declared size uploads OK
+		client := &uploadSessionClient{maxAppends: 8}
+		f := newUploadTestFs(t, client, 100)
+		o := &Object{fs: f, remote: "test.bin"}
+		entry, err := o.uploadChunked(ctx, strings.NewReader(strings.Repeat("a", 250)), files.NewCommitInfo("/test.bin"), 250)
+		require.NoError(t, err)
+		require.NotNil(t, entry)
+		assert.True(t, client.finishCalled)
+		assert.Equal(t, int64(250), client.bytesWritten)
+	})
+}
+
 func (f *Fs) importPaperForTest(t *testing.T) {
 	content := `# test doc