From afba1b0c9713fbdf8be12fecd373242f09089941 Mon Sep 17 00:00:00 2001 From: Marius Kleidl Date: Sat, 2 Nov 2024 15:38:41 +0000 Subject: [PATCH] `Upload-Length` in POST requests --- pkg/handler/post_test.go | 128 ++++++++++++++++++++++++++++++++ pkg/handler/unrouted_handler.go | 55 +++++++++++++- 2 files changed, 180 insertions(+), 3 deletions(-) diff --git a/pkg/handler/post_test.go b/pkg/handler/post_test.go index 8ac9567c..8d5c2025 100644 --- a/pkg/handler/post_test.go +++ b/pkg/handler/post_test.go @@ -678,6 +678,134 @@ func TestPost(t *testing.T) { }, }, res.InformationalResponses) }) + + if interopVersion != "3" && interopVersion != "4" && interopVersion != "5" { + SubTest(t, "UploadLengthAndContentLengthMatch", func(t *testing.T, store *MockFullDataStore, _ *StoreComposer) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + locker := NewMockFullLocker(ctrl) + lock := NewMockFullLock(ctrl) + upload := NewMockFullUpload(ctrl) + + gomock.InOrder( + store.EXPECT().NewUpload(gomock.Any(), FileInfo{ + SizeIsDeferred: false, + Size: 11, + MetaData: map[string]string{}, + }).Return(upload, nil), + upload.EXPECT().GetInfo(gomock.Any()).Return(FileInfo{ + ID: "foo", + SizeIsDeferred: false, + Size: 11, + }, nil), + locker.EXPECT().NewLock("foo").Return(lock, nil), + lock.EXPECT().Lock(gomock.Any(), gomock.Any()).Return(nil), + upload.EXPECT().WriteChunk(gomock.Any(), int64(0), NewReaderMatcher("hello world")).Return(int64(11), nil), + upload.EXPECT().FinishUpload(gomock.Any()).Return(nil), + lock.EXPECT().Unlock().Return(nil), + ) + + composer := NewStoreComposer() + composer.UseCore(store) + composer.UseLocker(locker) + + handler, _ := NewHandler(Config{ + StoreComposer: composer, + BasePath: "/files/", + EnableExperimentalProtocol: true, + }) + + (&httpTest{ + Method: "POST", + ReqHeader: map[string]string{ + "Upload-Draft-Interop-Version": interopVersion, + "Upload-Length": "11", + "Upload-Complete": "?1", + }, + ReqBody: strings.NewReader("hello world"), + Code: http.StatusCreated, + ResHeader: map[string]string{ + "Upload-Draft-Interop-Version": interopVersion, + "Location": "http://tus.io/files/foo", + "Upload-Offset": "11", + }, + }).Run(handler, t) + }) + + SubTest(t, "UploadLengthAndContentLengthMismatch", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + handler, _ := NewHandler(Config{ + StoreComposer: composer, + BasePath: "/files/", + EnableExperimentalProtocol: true, + }) + + (&httpTest{ + Method: "POST", + ReqHeader: map[string]string{ + "Upload-Draft-Interop-Version": interopVersion, + "Upload-Length": "999999", + "Upload-Complete": "?1", + }, + ReqBody: strings.NewReader("hello world"), + Code: http.StatusBadRequest, + ResBody: "ERR_INVALID_UPLOAD_LENGTH: missing or invalid Upload-Length header\n", + }).Run(handler, t) + }) + + SubTest(t, "OnlyUploadLength", func(t *testing.T, store *MockFullDataStore, _ *StoreComposer) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + locker := NewMockFullLocker(ctrl) + lock := NewMockFullLock(ctrl) + upload := NewMockFullUpload(ctrl) + + gomock.InOrder( + store.EXPECT().NewUpload(gomock.Any(), FileInfo{ + SizeIsDeferred: false, + Size: 11, + MetaData: map[string]string{}, + }).Return(upload, nil), + upload.EXPECT().GetInfo(gomock.Any()).Return(FileInfo{ + ID: "foo", + SizeIsDeferred: false, + Size: 11, + }, nil), + locker.EXPECT().NewLock("foo").Return(lock, nil), + lock.EXPECT().Lock(gomock.Any(), gomock.Any()).Return(nil), + upload.EXPECT().WriteChunk(gomock.Any(), int64(0), NewReaderMatcher("hello ")).Return(int64(6), nil), + lock.EXPECT().Unlock().Return(nil), + ) + + composer := NewStoreComposer() + composer.UseCore(store) + composer.UseLocker(locker) + + handler, _ := NewHandler(Config{ + StoreComposer: composer, + BasePath: "/files/", + EnableExperimentalProtocol: true, + }) + + (&httpTest{ + Method: "POST", + ReqHeader: map[string]string{ + "Upload-Draft-Interop-Version": interopVersion, + "Upload-Length": "11", + "Upload-Complete": "?0", + }, + ReqBody: strings.NewReader("hello "), + Code: http.StatusCreated, + ResHeader: map[string]string{ + "Upload-Draft-Interop-Version": interopVersion, + "Location": "http://tus.io/files/foo", + "Upload-Offset": "6", + }, + }).Run(handler, t) + }) + } }) } }) diff --git a/pkg/handler/unrouted_handler.go b/pkg/handler/unrouted_handler.go index 30c5fba5..ad209c7d 100644 --- a/pkg/handler/unrouted_handler.go +++ b/pkg/handler/unrouted_handler.go @@ -463,9 +463,15 @@ func (handler *UnroutedHandler) PostFileV2(w http.ResponseWriter, r *http.Reques info := FileInfo{ MetaData: make(MetaData), } - if willCompleteUpload && r.ContentLength != -1 { - // If the client wants to perform the upload in one request with Content-Length, we know the final upload size. - info.Size = r.ContentLength + + size, sizeIsDeferred, err := getIETFDraftUploadLength(r) + if err != nil { + handler.sendError(c, err) + return + } + + if !sizeIsDeferred { + info.Size = size } else { // Error out if the storage does not support upload length deferring, but we need it. if !handler.composer.UsesLengthDeferrer { @@ -1432,6 +1438,49 @@ func setIETFDraftUploadComplete(r *http.Request, resp HTTPResponse, isComplete b } } +// getIETFDraftUploadLength returns the length of an upload as defined in the +// resumable upload draft from IETF. This can either be in the Upload-Length +// header or in the Content-Length header. +func getIETFDraftUploadLength(r *http.Request) (length int64, lengthIsDeferred bool, err error) { + var lengthFromUploadLength int64 + hasLengthFromUploadLength := false + var lengthFromContentLength int64 + hasLengthFromContentLength := false + + willCompleteUpload := isIETFDraftUploadComplete(r) + if willCompleteUpload && r.ContentLength != -1 { + lengthFromContentLength = r.ContentLength + hasLengthFromContentLength = true + } + + uploadLengthStr := r.Header.Get("Upload-Length") + if uploadLengthStr != "" { + var err error + lengthFromUploadLength, err = strconv.ParseInt(uploadLengthStr, 10, 64) + if err != nil { + return 0, false, ErrInvalidUploadLength + } + + hasLengthFromUploadLength = true + } + + // If both lengths are set, they must match + if hasLengthFromContentLength && hasLengthFromUploadLength && lengthFromUploadLength != lengthFromContentLength { + return 0, false, ErrInvalidUploadLength + } + + // Return whichever length is set + if hasLengthFromUploadLength { + return lengthFromUploadLength, false, nil + } + if hasLengthFromContentLength { + return lengthFromContentLength, false, nil + } + + // No length set, so it's deferred + return 0, true, nil +} + // ParseMetadataHeader parses the Upload-Metadata header as defined in the // File Creation extension. // e.g. Upload-Metadata: name bHVucmpzLnBuZw==,type aW1hZ2UvcG5n