s3store: fix silent truncation of deferred-length uploads

[abh]

Summary

Deferred-length uploads to S3 can lose their final chunk with no error. The .info object says the upload is done and the offset matches the declared size, but the S3 object is short (or 0 bytes for uploads under MinPartSize). The client gets a 2xx and a truncated file.

Root cause

A chunk smaller than MinPartSize, written while the length is still deferred, can't become a real multipart part yet, so WriteChunk stashes it in a side object (<id>.part). FinishUpload assembled the object only from real multipart parts (ListParts) and never looked at .part. So when a client declares the length and finishes without one more WriteChunk, the stashed bytes are dropped.

This is the promotion gap from #396 (2021) and #798 (2022). The value-receiver half of #396 got fixed; promoting the incomplete part in FinishUpload was proposed but never implemented. The classic tus v1 flow stayed safe by accident, because tusd always sends one more WriteChunk after DeclareLength. The IETF Upload-Complete flow doesn't, which reopens it.

Fix

FinishUpload promotes the .part object into a real final part before completing, reusing the same download/upload/delete helpers as WriteChunk. It's guarded by the declared size so retries stay correct: parts already summing to the declared size mean a stale .part (delete it, don't re-promote); parts plus .part summing to it mean promote; anything else returns an error instead of completing short. Promotion runs only when an incomplete part actually lingers, so normal completions and the concat path are untouched.

This also resets the cached incompletePartSize in WriteChunk after the .part is deleted, so a FinishUpload later in the same request doesn't fire a redundant DeleteObject.

Tests

Three gomock regression tests drive the real NewUpload/WriteChunk/DeclareLength/FinishUpload flow: promote a tail after a real part, promote when the whole upload is under MinPartSize (this used to complete as 0 bytes), and don't re-upload an already-promoted part on retry. (Thanks to Claude for writing these tedious, detailed, inscrutable mock tests.)

Storage-layer only, no dependency on the in-flight handler idempotency work (#1374/#1366).

Refs #396, #798.

[Copilot]

Pull request overview

This PR fixes a long-standing S3Store correctness bug where deferred-length uploads could be completed successfully while silently dropping a sub-MinPartSize tail that had been parked in the <id>.part object (not yet promotable to a multipart part). The change ensures FinishUpload accounts for and, when appropriate, promotes that lingering incomplete part before completing the multipart upload, preventing truncated (or empty) final objects.

Changes:

• Update S3Store’s FinishUpload to detect a lingering .part object and promote it into a real final multipart part (or delete it if stale), validating against the declared size to keep retries safe.
• Reset incompletePartSize cache in WriteChunk after deleting the .part object to avoid redundant cleanup within the same request.
• Add gomock regression tests covering promotion scenarios (tail after real part, whole upload under MinPartSize, and retry behavior), plus documentation about consistency/locking expectations.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
pkg/s3store/s3store.go	Promotes lingering incomplete .part during FinishUpload to prevent silent truncation; keeps cached incomplete-part size consistent after deletion.
pkg/s3store/s3store_deferred_length_repro_test.go	Adds regression tests reproducing and guarding the deferred-length truncation scenarios.
docs/_storage-backends/aws-s3.md	Documents read-after-write consistency and multi-instance locking considerations for the S3 backend.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.