From c28e1fa219f40f5583401aa86cbf3c2e2a4870e7 Mon Sep 17 00:00:00 2001 From: Marius <1375043+Acconut@users.noreply.github.com> Date: Sat, 2 Nov 2024 18:00:41 +0000 Subject: [PATCH] handler: Support IETF draft -04 and -05 (#1213) * `Upload-Draft-Interop-Version: 6` * `Content-Type: application/partial-upload` * `Upload-Length` in POST requests * `Upload-Length` in HEAD responses * `Upload-Limit` in OPTIONS responses * `Upload-Limit` in POST responses * `Upload-Limit` in HEAD responses --- pkg/handler/head_test.go | 9 ++- pkg/handler/options_test.go | 22 +++++ pkg/handler/patch_test.go | 40 +++++++--- pkg/handler/post_test.go | 137 +++++++++++++++++++++++++++++++- pkg/handler/unrouted_handler.go | 106 ++++++++++++++++++++++-- pkg/handler/utils_test.go | 11 ++- 6 files changed, 305 insertions(+), 20 deletions(-) diff --git a/pkg/handler/head_test.go b/pkg/handler/head_test.go index 7b645e16d..48664e11e 100644 --- a/pkg/handler/head_test.go +++ b/pkg/handler/head_test.go @@ -145,7 +145,7 @@ func TestHead(t *testing.T) { }) SubTest(t, "ExperimentalProtocol", func(t *testing.T, _ *MockFullDataStore, _ *StoreComposer) { - for _, interopVersion := range []string{"3", "4", "5"} { + for _, interopVersion := range []string{"3", "4", "5", "6"} { SubTest(t, "InteropVersion"+interopVersion, func(t *testing.T, _ *MockFullDataStore, _ *StoreComposer) { SubTest(t, "IncompleteUpload", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { ctrl := gomock.NewController(t) @@ -176,6 +176,8 @@ func TestHead(t *testing.T) { ResHeader: addIETFUploadCompleteHeader(map[string]string{ "Upload-Draft-Interop-Version": interopVersion, "Upload-Offset": "5", + "Upload-Length": "10", + "Upload-Limit": "min-size=0,max-size=10", }, false, interopVersion), }).Run(handler, t) }) @@ -209,6 +211,8 @@ func TestHead(t *testing.T) { ResHeader: addIETFUploadCompleteHeader(map[string]string{ "Upload-Draft-Interop-Version": interopVersion, "Upload-Offset": "10", + "Upload-Length": "10", + "Upload-Limit": "min-size=0,max-size=10", }, true, interopVersion), }).Run(handler, t) }) @@ -229,6 +233,7 @@ func TestHead(t *testing.T) { handler, _ := NewHandler(Config{ StoreComposer: composer, EnableExperimentalProtocol: true, + MaxSize: 400, }) (&httpTest{ @@ -241,6 +246,8 @@ func TestHead(t *testing.T) { ResHeader: addIETFUploadCompleteHeader(map[string]string{ "Upload-Draft-Interop-Version": interopVersion, "Upload-Offset": "5", + "Upload-Length": "", + "Upload-Limit": "min-size=0,max-size=400", }, false, interopVersion), }).Run(handler, t) }) diff --git a/pkg/handler/options_test.go b/pkg/handler/options_test.go index fdcb21fa4..c29140211 100644 --- a/pkg/handler/options_test.go +++ b/pkg/handler/options_test.go @@ -42,4 +42,26 @@ func TestOptions(t *testing.T) { Code: http.StatusPreconditionFailed, }).Run(handler, t) }) + + SubTest(t, "ExperimentalProtocol", func(t *testing.T, store *MockFullDataStore, _ *StoreComposer) { + composer := NewStoreComposer() + composer.UseCore(store) + + handler, _ := NewHandler(Config{ + StoreComposer: composer, + EnableExperimentalProtocol: true, + MaxSize: 400, + }) + + (&httpTest{ + Method: "OPTIONS", + ReqHeader: map[string]string{ + "Upload-Draft-Interop-Version": "6", + }, + ResHeader: map[string]string{ + "Upload-Limit": "min-size=0,max-size=400", + }, + Code: http.StatusOK, + }).Run(handler, t) + }) } diff --git a/pkg/handler/patch_test.go b/pkg/handler/patch_test.go index a78c5e429..15d413f53 100644 --- a/pkg/handler/patch_test.go +++ b/pkg/handler/patch_test.go @@ -816,7 +816,7 @@ func TestPatch(t *testing.T) { }) SubTest(t, "ExperimentalProtocol", func(t *testing.T, _ *MockFullDataStore, _ *StoreComposer) { - for _, interopVersion := range []string{"3", "4", "5"} { + for _, interopVersion := range []string{"3", "4", "5", "6"} { SubTest(t, "InteropVersion"+interopVersion, func(t *testing.T, _ *MockFullDataStore, _ *StoreComposer) { SubTest(t, "CompleteUploadWithKnownSize", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { ctrl := gomock.NewController(t) @@ -843,10 +843,10 @@ func TestPatch(t *testing.T) { (&httpTest{ Method: "PATCH", URL: "yes", - ReqHeader: addIETFUploadCompleteHeader(map[string]string{ + ReqHeader: addIETFContentTypeHeader(addIETFUploadCompleteHeader(map[string]string{ "Upload-Draft-Interop-Version": interopVersion, "Upload-Offset": "5", - }, true, interopVersion), + }, true, interopVersion), interopVersion), ReqBody: strings.NewReader("hello"), Code: http.StatusNoContent, ResHeader: map[string]string{ @@ -887,10 +887,10 @@ func TestPatch(t *testing.T) { (&httpTest{ Method: "PATCH", URL: "yes", - ReqHeader: addIETFUploadCompleteHeader(map[string]string{ + ReqHeader: addIETFContentTypeHeader(addIETFUploadCompleteHeader(map[string]string{ "Upload-Draft-Interop-Version": interopVersion, "Upload-Offset": "5", - }, true, interopVersion), + }, true, interopVersion), interopVersion), ReqBody: strings.NewReader("hello"), Code: http.StatusNoContent, ResHeader: map[string]string{ @@ -922,10 +922,10 @@ func TestPatch(t *testing.T) { (&httpTest{ Method: "PATCH", URL: "yes", - ReqHeader: addIETFUploadCompleteHeader(map[string]string{ + ReqHeader: addIETFContentTypeHeader(addIETFUploadCompleteHeader(map[string]string{ "Upload-Draft-Interop-Version": interopVersion, "Upload-Offset": "5", - }, false, interopVersion), + }, false, interopVersion), interopVersion), ReqBody: strings.NewReader("hel"), Code: http.StatusNoContent, ResHeader: map[string]string{ @@ -957,10 +957,10 @@ func TestPatch(t *testing.T) { (&httpTest{ Method: "PATCH", URL: "yes", - ReqHeader: addIETFUploadCompleteHeader(map[string]string{ + ReqHeader: addIETFContentTypeHeader(addIETFUploadCompleteHeader(map[string]string{ "Upload-Draft-Interop-Version": interopVersion, "Upload-Offset": "5", - }, false, interopVersion), + }, false, interopVersion), interopVersion), ReqBody: strings.NewReader("hel"), Code: http.StatusNoContent, ResHeader: map[string]string{ @@ -968,6 +968,28 @@ func TestPatch(t *testing.T) { }, }).Run(handler, t) }) + + if interopVersion != "3" && interopVersion != "4" && interopVersion != "5" { + SubTest(t, "InvalidContentType", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { + handler, _ := NewHandler(Config{ + StoreComposer: composer, + EnableExperimentalProtocol: true, + }) + + (&httpTest{ + Method: "PATCH", + URL: "yes", + ReqHeader: map[string]string{ + "Upload-Draft-Interop-Version": interopVersion, + "Content-Type": "application/not-partial-upload", + "Upload-Offset": "0", + }, + ReqBody: strings.NewReader("test"), + Code: http.StatusBadRequest, + ResBody: "ERR_INVALID_CONTENT_TYPE: missing or invalid Content-Type header\n", + }).Run(handler, t) + }) + } }) } }) diff --git a/pkg/handler/post_test.go b/pkg/handler/post_test.go index c08b10276..64746e04e 100644 --- a/pkg/handler/post_test.go +++ b/pkg/handler/post_test.go @@ -546,7 +546,7 @@ func TestPost(t *testing.T) { }) SubTest(t, "ExperimentalProtocol", func(t *testing.T, _ *MockFullDataStore, _ *StoreComposer) { - for _, interopVersion := range []string{"3", "4", "5"} { + for _, interopVersion := range []string{"3", "4", "5", "6"} { SubTest(t, "InteropVersion"+interopVersion, func(t *testing.T, _ *MockFullDataStore, _ *StoreComposer) { SubTest(t, "CompleteUpload", func(t *testing.T, store *MockFullDataStore, _ *StoreComposer) { ctrl := gomock.NewController(t) @@ -603,6 +603,7 @@ func TestPost(t *testing.T) { "Upload-Draft-Interop-Version": interopVersion, "Location": "http://tus.io/files/foo", "Upload-Offset": "11", + "Upload-Limit": "min-size=0,max-size=11", }, }).Run(handler, t) @@ -614,6 +615,7 @@ func TestPost(t *testing.T) { "Upload-Draft-Interop-Version": []string{interopVersion}, "Location": []string{"http://tus.io/files/foo"}, "X-Content-Type-Options": []string{"nosniff"}, + "Upload-Limit": []string{"min-size=0,max-size=11"}, }, }, }, res.InformationalResponses) @@ -650,6 +652,7 @@ func TestPost(t *testing.T) { StoreComposer: composer, BasePath: "/files/", EnableExperimentalProtocol: true, + MaxSize: 400, }) res := (&httpTest{ @@ -663,6 +666,7 @@ func TestPost(t *testing.T) { "Upload-Draft-Interop-Version": interopVersion, "Location": "http://tus.io/files/foo", "Upload-Offset": "11", + "Upload-Limit": "min-size=0,max-size=400", }, }).Run(handler, t) @@ -674,10 +678,141 @@ func TestPost(t *testing.T) { "Upload-Draft-Interop-Version": []string{interopVersion}, "Location": []string{"http://tus.io/files/foo"}, "X-Content-Type-Options": []string{"nosniff"}, + "Upload-Limit": []string{"min-size=0,max-size=400"}, }, }, }, 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", + "Upload-Limit": "min-size=0,max-size=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", + "Upload-Limit": "min-size=0,max-size=11", + }, + }).Run(handler, t) + }) + } }) } }) diff --git a/pkg/handler/unrouted_handler.go b/pkg/handler/unrouted_handler.go index 02f59c8d5..be4673547 100644 --- a/pkg/handler/unrouted_handler.go +++ b/pkg/handler/unrouted_handler.go @@ -28,6 +28,7 @@ const ( interopVersion3 draftVersion = "3" // From draft version -01 interopVersion4 draftVersion = "4" // From draft version -02 interopVersion5 draftVersion = "5" // From draft version -03 + interopVersion6 draftVersion = "6" // From draft version -04 and -05 ) var ( @@ -235,8 +236,16 @@ func (handler *UnroutedHandler) Middleware(h http.Handler) http.Handler { // Set appropriated headers in case of OPTIONS method allowing protocol // discovery and end with an 204 No Content if r.Method == "OPTIONS" { + ietfDraftLimits := "min-size=0" + if handler.config.MaxSize > 0 { - header.Set("Tus-Max-Size", strconv.FormatInt(handler.config.MaxSize, 10)) + maxSizeStr := strconv.FormatInt(handler.config.MaxSize, 10) + header.Set("Tus-Max-Size", maxSizeStr) + ietfDraftLimits += ",max-size=" + maxSizeStr + } + + if handler.usesIETFDraft(r) { + header.Set("Upload-Limit", ietfDraftLimits) } header.Set("Tus-Version", "1.0.0") @@ -462,9 +471,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 { @@ -545,11 +560,14 @@ func (handler *UnroutedHandler) PostFileV2(w http.ResponseWriter, r *http.Reques id := info.ID url := handler.absFileURL(r, id) + limits := handler.getIETFDraftUploadLimits(info) resp.Header["Location"] = url + resp.Header["Upload-Limit"] = limits // Send 104 response w.Header().Set("Location", url) w.Header().Set("Upload-Draft-Interop-Version", string(currentUploadDraftInteropVersion)) + w.Header().Set("Upload-Limit", limits) w.WriteHeader(104) handler.Metrics.incUploadsCreated() @@ -673,6 +691,8 @@ func (handler *UnroutedHandler) HeadFile(w http.ResponseWriter, r *http.Request) resp.Header["Upload-Defer-Length"] = UploadLengthDeferred } else { resp.Header["Upload-Length"] = strconv.FormatInt(info.Size, 10) + // TODO: Shouldn't this rather be offset? Basically, whatever GET would return. + // But this then also depends on the storage backend if that's even supported. resp.Header["Content-Length"] = strconv.FormatInt(info.Size, 10) } @@ -682,6 +702,12 @@ func (handler *UnroutedHandler) HeadFile(w http.ResponseWriter, r *http.Request) setIETFDraftUploadComplete(r, resp, isUploadCompleteNow) resp.Header["Upload-Draft-Interop-Version"] = string(getIETFDraftInteropVersion(r)) + if !info.SizeIsDeferred { + resp.Header["Upload-Length"] = strconv.FormatInt(info.Size, 10) + } + + resp.Header["Upload-Limit"] = handler.getIETFDraftUploadLimits(info) + // Draft -01 and -02 require a 204 No Content response. Version -03 allows 200 OK as well, // but we stick to 204 to not make the logic less complex. resp.StatusCode = http.StatusNoContent @@ -697,12 +723,20 @@ func (handler *UnroutedHandler) PatchFile(w http.ResponseWriter, r *http.Request isTusV1 := !handler.usesIETFDraft(r) - // Check for presence of application/offset+octet-stream + // Check for presence of application/offset+octet-stream (tus v1) or application/partial-upload (IETF draft since -04) if isTusV1 && r.Header.Get("Content-Type") != "application/offset+octet-stream" { handler.sendError(c, ErrInvalidContentType) return } + if !isTusV1 { + currentInteropVersion := getIETFDraftInteropVersion(r) + if currentInteropVersion != interopVersion3 && currentInteropVersion != interopVersion4 && currentInteropVersion != interopVersion5 && r.Header.Get("Content-Type") != "application/partial-upload" { + handler.sendError(c, ErrInvalidContentType) + return + } + } + // Check for presence of a valid Upload-Offset Header offset, err := strconv.ParseInt(r.Header.Get("Upload-Offset"), 10, 64) if err != nil || offset < 0 { @@ -1377,11 +1411,24 @@ func (handler UnroutedHandler) usesIETFDraft(r *http.Request) bool { return handler.config.EnableExperimentalProtocol && interopVersionHeader != "" } +// getIETFDraftUploadLimits returns the Upload-Limit header for a given upload +// according to the set resumable upload draft version from IETF. +func (handler UnroutedHandler) getIETFDraftUploadLimits(info FileInfo) string { + limits := "min-size=0" + if handler.config.MaxSize > 0 { + limits += ",max-size=" + strconv.FormatInt(handler.config.MaxSize, 10) + } else if !info.SizeIsDeferred { + limits += ",max-size=" + strconv.FormatInt(info.Size, 10) + } + + return limits +} + // getIETFDraftInteropVersion returns the resumable upload draft interop version from the headers. func getIETFDraftInteropVersion(r *http.Request) draftVersion { version := draftVersion(r.Header.Get("Upload-Draft-Interop-Version")) switch version { - case interopVersion3, interopVersion4, interopVersion5: + case interopVersion3, interopVersion4, interopVersion5, interopVersion6: return version default: return "" @@ -1393,7 +1440,7 @@ func getIETFDraftInteropVersion(r *http.Request) draftVersion { func isIETFDraftUploadComplete(r *http.Request) bool { currentUploadDraftInteropVersion := getIETFDraftInteropVersion(r) switch currentUploadDraftInteropVersion { - case interopVersion4, interopVersion5: + case interopVersion4, interopVersion5, interopVersion6: return r.Header.Get("Upload-Complete") == "?1" case interopVersion3: return r.Header.Get("Upload-Incomplete") == "?0" @@ -1414,7 +1461,7 @@ func setIETFDraftUploadComplete(r *http.Request, resp HTTPResponse, isComplete b } else { resp.Header["Upload-Incomplete"] = "?1" } - case interopVersion4, interopVersion5: + case interopVersion4, interopVersion5, interopVersion6: if isComplete { resp.Header["Upload-Complete"] = "?1" } else { @@ -1423,6 +1470,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 diff --git a/pkg/handler/utils_test.go b/pkg/handler/utils_test.go index a29d47732..35ec8a744 100644 --- a/pkg/handler/utils_test.go +++ b/pkg/handler/utils_test.go @@ -142,7 +142,7 @@ func addIETFUploadCompleteHeader(header map[string]string, isComplete bool, inte } else { header["Upload-Incomplete"] = "?1" } - case "4", "5": + case "4", "5", "6": if isComplete { header["Upload-Complete"] = "?1" } else { @@ -152,3 +152,12 @@ func addIETFUploadCompleteHeader(header map[string]string, isComplete bool, inte return header } + +// addIETFContentTypeHeader writes the Content-Type header depending on the interop version. +func addIETFContentTypeHeader(header map[string]string, interopVersion string) map[string]string { + switch interopVersion { + case "6": + header["Content-Type"] = "application/partial-upload" + } + return header +}