diff --git a/weed/s3api/filer_multipart.go b/weed/s3api/filer_multipart.go index 6be70b8adf..9a66b6f879 100644 --- a/weed/s3api/filer_multipart.go +++ b/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/; 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]) diff --git a/weed/s3api/filer_multipart_abort_test.go b/weed/s3api/filer_multipart_abort_test.go index 37f3945afd..fc1af1d300 100644 --- a/weed/s3api/filer_multipart_abort_test.go +++ b/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/; 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) } } diff --git a/weed/s3api/s3api_object_handlers_multipart.go b/weed/s3api/s3api_object_handlers_multipart.go index c28778a594..c817027f60 100644 --- a/weed/s3api/s3api_object_handlers_multipart.go +++ b/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)