From dc673402d2f74e43562bb32ca9d5bed6205533be Mon Sep 17 00:00:00 2001 From: Marius Date: Sun, 28 Aug 2016 22:06:37 +0200 Subject: [PATCH] Add support for Creation With Upload extension See tus/tus-resumable-upload-protocol#88 for the current proposal --- concat_test.go | 2 +- options_test.go | 2 +- post_test.go | 114 +++++++++++++++++++++++++++++++++++++------- terminate_test.go | 2 +- unrouted_handler.go | 84 +++++++++++++++++++++++++------- 5 files changed, 166 insertions(+), 38 deletions(-) diff --git a/concat_test.go b/concat_test.go index 30b1a7b..0a1a108 100644 --- a/concat_test.go +++ b/concat_test.go @@ -46,7 +46,7 @@ func TestConcatPartial(t *testing.T) { Method: "OPTIONS", URL: "", ResHeader: map[string]string{ - "Tus-Extension": "creation,concatenation", + "Tus-Extension": "creation,creation-with-upload,concatenation", }, Code: http.StatusOK, }).Run(handler, t) diff --git a/options_test.go b/options_test.go index 206fdf2..2d89ddf 100644 --- a/options_test.go +++ b/options_test.go @@ -20,7 +20,7 @@ func TestOptions(t *testing.T) { Method: "OPTIONS", Code: http.StatusOK, ResHeader: map[string]string{ - "Tus-Extension": "creation", + "Tus-Extension": "creation,creation-with-upload", "Tus-Version": "1.0.0", "Tus-Resumable": "1.0.0", "Tus-Max-Size": "400", diff --git a/post_test.go b/post_test.go index 5f41ddd..ea91f21 100644 --- a/post_test.go +++ b/post_test.go @@ -1,44 +1,57 @@ package tusd_test import ( + "bytes" + "io" + "io/ioutil" "net/http" + "strings" "testing" + "github.com/stretchr/testify/assert" + . "github.com/tus/tusd" ) type postStore struct { - t *testing.T + t *assert.Assertions zeroStore } func (s postStore) NewUpload(info FileInfo) (string, error) { - if info.Size != 300 { - s.t.Errorf("Expected size to be 300 (got %v)", info.Size) - } + s.t.Equal(int64(300), info.Size) metaData := info.MetaData - if len(metaData) != 2 { - s.t.Errorf("Expected two elements in metadata") - } - - if v := metaData["foo"]; v != "hello" { - s.t.Errorf("Expected foo element to be 'hello' but got %s", v) - } - - if v := metaData["bar"]; v != "world" { - s.t.Errorf("Expected bar element to be 'world' but got %s", v) - } + s.t.Equal(2, len(metaData)) + s.t.Equal("hello", metaData["foo"]) + s.t.Equal("world", metaData["bar"]) return "foo", nil } +func (s postStore) WriteChunk(id string, offset int64, src io.Reader) (int64, error) { + s.t.Equal(int64(0), offset) + + data, err := ioutil.ReadAll(src) + s.t.Nil(err) + s.t.Equal("hello", string(data)) + + return 5, nil +} + +func (s postStore) ConcatUploads(id string, uploads []string) error { + s.t.True(false, "concatenation should not be attempted") + return nil +} + func TestPost(t *testing.T) { + a := assert.New(t) + handler, _ := NewHandler(Config{ MaxSize: 400, BasePath: "files", DataStore: postStore{ - t: t, + t: a, }, }) @@ -87,7 +100,7 @@ func TestPost(t *testing.T) { MaxSize: 400, BasePath: "files", DataStore: postStore{ - t: t, + t: a, }, RespectForwardedHeaders: true, }) @@ -141,3 +154,70 @@ func TestPost(t *testing.T) { }, }).Run(handler, t) } + +func TestPostWithUpload(t *testing.T) { + a := assert.New(t) + + handler, _ := NewHandler(Config{ + MaxSize: 400, + BasePath: "files", + DataStore: postStore{ + t: a, + }, + }) + + (&httpTest{ + Name: "Successful request", + Method: "POST", + ReqHeader: map[string]string{ + "Tus-Resumable": "1.0.0", + "Upload-Length": "300", + "Content-Type": "application/offset+octet-stream", + "Upload-Metadata": "foo aGVsbG8=, bar d29ybGQ=", + }, + ReqBody: strings.NewReader("hello"), + Code: http.StatusCreated, + ResHeader: map[string]string{ + "Location": "http://tus.io/files/foo", + "Upload-Offset": "5", + }, + }).Run(handler, t) + + (&httpTest{ + Name: "Exceeding upload size", + Method: "POST", + ReqHeader: map[string]string{ + "Tus-Resumable": "1.0.0", + "Upload-Length": "300", + "Content-Type": "application/offset+octet-stream", + "Upload-Metadata": "foo aGVsbG8=, bar d29ybGQ=", + }, + ReqBody: bytes.NewReader(make([]byte, 400)), + Code: http.StatusRequestEntityTooLarge, + }).Run(handler, t) + + (&httpTest{ + Name: "Incorrect content type", + Method: "POST", + ReqHeader: map[string]string{ + "Tus-Resumable": "1.0.0", + "Content-Type": "application/false", + }, + ReqBody: strings.NewReader("hello"), + Code: http.StatusBadRequest, + }).Run(handler, t) + + (&httpTest{ + Name: "Upload and final concatenation", + Method: "POST", + ReqHeader: map[string]string{ + "Tus-Resumable": "1.0.0", + "Upload-Length": "300", + "Content-Type": "application/offset+octet-stream", + "Upload-Metadata": "foo aGVsbG8=, bar d29ybGQ=", + "Upload-Concat": "final; http://tus.io/files/a http://tus.io/files/b", + }, + ReqBody: strings.NewReader("hello"), + Code: http.StatusForbidden, + }).Run(handler, t) +} diff --git a/terminate_test.go b/terminate_test.go index ba15164..55b6293 100644 --- a/terminate_test.go +++ b/terminate_test.go @@ -44,7 +44,7 @@ func TestTerminate(t *testing.T) { Method: "OPTIONS", URL: "", ResHeader: map[string]string{ - "Tus-Extension": "creation,termination", + "Tus-Extension": "creation,creation-with-upload,termination", }, Code: http.StatusOK, }).Run(handler, t) diff --git a/unrouted_handler.go b/unrouted_handler.go index ef7d8ea..0876e5f 100644 --- a/unrouted_handler.go +++ b/unrouted_handler.go @@ -89,7 +89,7 @@ func NewUnroutedHandler(config Config) (*UnroutedHandler, error) { } // Only promote extesions using the Tus-Extension header which are implemented - extensions := "creation" + extensions := "creation,creation-with-upload" if config.StoreComposer.UsesTerminater { extensions += ",termination" } @@ -191,6 +191,16 @@ func (handler *UnroutedHandler) Middleware(h http.Handler) http.Handler { // PostFile creates a new file upload using the datastore after validating the // length and parsing the metadata. func (handler *UnroutedHandler) PostFile(w http.ResponseWriter, r *http.Request) { + // Check for presence of application/offset+octet-stream + containsChunk := false + if contentType := r.Header.Get("Content-Type"); contentType != "" { + if contentType != "application/offset+octet-stream" { + handler.sendError(w, r, ErrInvalidContentType) + return + } + containsChunk = true + } + // Only use the proper Upload-Concat header if the concatenation extension // is even supported by the data store. var concatHeader string @@ -210,6 +220,12 @@ func (handler *UnroutedHandler) PostFile(w http.ResponseWriter, r *http.Request) // Upload-Length header) var size int64 if isFinal { + // A final upload must not contain a chunk within the creation request + if containsChunk { + handler.sendError(w, r, ErrModifyFinal) + return + } + size, err = handler.sizeOfUploads(partialUploads) if err != nil { handler.sendError(w, r, err) @@ -246,6 +262,13 @@ func (handler *UnroutedHandler) PostFile(w http.ResponseWriter, r *http.Request) return } + // Add the Location header directly after creating the new resource to even + // include it in cases of failure when an error is returned + url := handler.absFileURL(r, id) + w.Header().Set("Location", url) + + go handler.Metrics.incUploadsCreated() + if isFinal { if err := handler.composer.Concater.ConcatUploads(id, partialUploads); err != nil { handler.sendError(w, r, err) @@ -257,15 +280,26 @@ func (handler *UnroutedHandler) PostFile(w http.ResponseWriter, r *http.Request) info.ID = id handler.CompleteUploads <- info } - - go handler.Metrics.incUploadsFinished() } - url := handler.absFileURL(r, id) - w.Header().Set("Location", url) - w.WriteHeader(http.StatusCreated) + if containsChunk { + if handler.composer.UsesLocker { + locker := handler.composer.Locker + if err := locker.LockUpload(id); err != nil { + handler.sendError(w, r, err) + return + } - go handler.Metrics.incUploadsCreated() + defer locker.UnlockUpload(id) + } + + if err := handler.writeChunk(id, info, w, r); err != nil { + handler.sendError(w, r, err) + return + } + } + + w.WriteHeader(http.StatusCreated) } // HeadFile returns the length and offset for the HEAD request @@ -372,13 +406,23 @@ func (handler *UnroutedHandler) PatchFile(w http.ResponseWriter, r *http.Request return } + if err := handler.writeChunk(id, info, w, r); err != nil { + handler.sendError(w, r, err) + return + } + + w.WriteHeader(http.StatusNoContent) +} + +// PatchFile adds a chunk to an upload. Only allowed enough space is left. +func (handler *UnroutedHandler) writeChunk(id string, info FileInfo, w http.ResponseWriter, r *http.Request) error { // Get Content-Length if possible length := r.ContentLength + offset := info.Offset // Test if this upload fits into the file's size if offset+length > info.Size { - handler.sendError(w, r, ErrSizeExceeded) - return + return ErrSizeExceeded } maxSize := info.Size - offset @@ -386,13 +430,18 @@ func (handler *UnroutedHandler) PatchFile(w http.ResponseWriter, r *http.Request maxSize = length } - // Limit the data read from the request's body to the allowed maxiumum - reader := io.LimitReader(r.Body, maxSize) + var bytesWritten int64 + // Prevent a nil pointer derefernce when accessing the body which may not be + // available in the case of a malicious request. + if r.Body != nil { + // Limit the data read from the request's body to the allowed maxiumum + reader := io.LimitReader(r.Body, maxSize) - bytesWritten, err := handler.composer.Core.WriteChunk(id, offset, reader) - if err != nil { - handler.sendError(w, r, err) - return + var err error + bytesWritten, err = handler.composer.Core.WriteChunk(id, offset, reader) + if err != nil { + return err + } } // Send new offset to client @@ -405,8 +454,7 @@ func (handler *UnroutedHandler) PatchFile(w http.ResponseWriter, r *http.Request // ... allow custom mechanism to finish and cleanup the upload if handler.composer.UsesFinisher { if err := handler.composer.Finisher.FinishUpload(id); err != nil { - handler.sendError(w, r, err) - return + return err } } @@ -419,7 +467,7 @@ func (handler *UnroutedHandler) PatchFile(w http.ResponseWriter, r *http.Request go handler.Metrics.incUploadsFinished() } - w.WriteHeader(http.StatusNoContent) + return nil } // GetFile handles requests to download a file using a GET request. This is not