s3api: return NoSuchUpload when aborting an upload that was already completed (#12227)

s3: AbortMultipartUpload answers NoSuchUpload for an upload that no longer exists

Complete and abort both remove .uploads/<id>. abortMultipartUpload answered
204 whenever that record was missing, so aborting an upload that had already
completed reported success while the object stayed in place, and a client
could read it as the object being discarded. AWS returns 404 NoSuchUpload
("The specified multipart upload does not exist") for that case, and
ListParts here already does.

Answer ErrNoSuchUpload when the record is not found. checkUploadStillOpen
reuses abort for cleanup after the upload is gone; NoSuchUpload is not a
failed cleanup there.

The leftover-directory path from #11385 is unchanged: a .uploads/<id> that
outlived its completed object is still removed metadata-only and answered.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

weed/s3api/filer_multipart.go

@@ -1156,15 +1156,17 @@ func (s3a *S3ApiServer) abortMultipartUpload(input *s3.AbortMultipartUploadInput
 
 	uploadsFolder := s3a.genUploadsFolder(*input.Bucket)
 	uploadEntry, err := s3a.getEntry(uploadsFolder, *input.UploadId)
+	// complete and abort both remove .uploads/<id>; a missing record is an upload
+	// that does not exist (NoSuchUpload on AWS), not one this call discarded
 	if err != nil {
 		if isFilerNotFound(err) {
-			return &s3.AbortMultipartUploadOutput{}, s3err.ErrNone
+			return nil, s3err.ErrNoSuchUpload
 		}
 		glog.Errorf("bucket %s abort upload %s: %v", *input.Bucket, *input.UploadId, err)
 		return nil, s3err.ErrInternalError
 	}
 	if uploadEntry == nil {
-		return &s3.AbortMultipartUploadOutput{}, s3err.ErrNone
+		return nil, s3err.ErrNoSuchUpload
 	}
 
 	object := string(uploadEntry.Extended[s3_constants.ExtMultipartObjectKey])

weed/s3api/filer_multipart_abort_test.go

@@ -189,14 +189,36 @@ func TestAbortUndecidableCheckRefuses(t *testing.T) {
 	}
 }
 
-// No upload directory at all answers success, the same as before.
-func TestAbortGoneUploadAnswersSuccess(t *testing.T) {
-	s3a := newAbortTestServer(t, &fakeAbortFiler{})
+// No upload directory at all is an upload that does not exist: NoSuchUpload
+// (404), as on AWS, not a successful abort.
+func TestAbortGoneUploadAnswersNoSuchUpload(t *testing.T) {
+	f := &fakeAbortFiler{}
+	s3a := newAbortTestServer(t, f)
 
 	_, code := s3a.abortMultipartUpload(abortInput("gone"))
 
-	if code != s3err.ErrNone {
-		t.Fatalf("code = %v, want ErrNone", code)
+	if code != s3err.ErrNoSuchUpload {
+		t.Fatalf("code = %v, want ErrNoSuchUpload", code)
+	}
+	if f.deleteReq != nil {
+		t.Fatalf("delete issued for a missing upload: %+v", f.deleteReq)
+	}
+}
+
+// Completion removes .uploads/<id>; aborting the completed upload afterwards
+// must say NoSuchUpload, not 204, so the client cannot read it as the object
+// being discarded, and must not touch the object.
+func TestAbortAfterCompleteAnswersNoSuchUpload(t *testing.T) {
+	f := &fakeAbortFiler{objectEntry: versionEntry("a.bin", "up1")}
+	s3a := newAbortTestServer(t, f)
+
+	_, code := s3a.abortMultipartUpload(abortInput("up1"))
+
+	if code != s3err.ErrNoSuchUpload {
+		t.Fatalf("code = %v, want ErrNoSuchUpload", code)
+	}
+	if f.deleteReq != nil {
+		t.Fatalf("delete issued after completion: %+v", f.deleteReq)
 	}
 }
 

weed/s3api/s3api_object_handlers_multipart.go

@@ -546,7 +546,7 @@ func (s3a *S3ApiServer) checkUploadStillOpen(w http.ResponseWriter, r *http.Requ
 		Bucket:   aws.String(bucket),
 		Key:      objectKey(aws.String(object)),
 		UploadId: aws.String(uploadID),
-	}); code != s3err.ErrNone {
+	}); code != s3err.ErrNone && code != s3err.ErrNoSuchUpload {
 		glog.Warningf("checkUploadStillOpen %s/%s: part left behind, cleanup failed", bucket, uploadID)
 	}
 	s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchUpload)