Allow upload ID to contain slashes (#1020)

* Allow upload ID to contain slashes

See https://community.transloadit.com/t/tusd-azure-changefileinfo-id-with-folder-support/16871/1

* Replace pat router with own logic

* Add test for Concatenation extension

* Remove unneeded regular expression

* filestore: Turn slashes into directories

* Document new feature

* Properly handle paths on Windows

docs/hooks.md

@@ -148,6 +148,8 @@ Below you can find an annotated, JSON-ish encoded example of a hook response:
     "ChangeFileInfo": {
         // Provides a custom upload ID, which influences the destination where the
         // upload is stored and the upload URL that is sent to the client.
+        // The ID can contain forward slashes (/) to store uploads in a hierarchical
+        // structure, such as nested directories.
         // Its exact effect depends on each data store.
         "ID": "my-custom-upload-id",
         // Set custom meta data that is saved with the upload and also accessible to

pkg/filestore/filestore.go

@@ -21,6 +21,7 @@ import (
 )
 
 var defaultFilePerm = os.FileMode(0664)
+var defaultDirectoryPerm = os.FileMode(0754)
 
 // See the handler.DataStore interface for documentation about the different
 // methods.
@@ -58,15 +59,7 @@ func (store FileStore) NewUpload(ctx context.Context, info handler.FileInfo) (ha
 	}
 
 	// Create binary file with no content
-	file, err := os.OpenFile(binPath, os.O_CREATE|os.O_WRONLY, defaultFilePerm)
-	if err != nil {
-		if os.IsNotExist(err) {
-			err = fmt.Errorf("upload directory does not exist: %s", store.Path)
-		}
-		return nil, err
-	}
-	err = file.Close()
-	if err != nil {
+	if err := createFile(binPath, nil); err != nil {
 		return nil, err
 	}
 
@@ -77,8 +70,7 @@ func (store FileStore) NewUpload(ctx context.Context, info handler.FileInfo) (ha
 	}
 
 	// writeInfo creates the file by itself if necessary
-	err = upload.writeInfo()
-	if err != nil {
+	if err := upload.writeInfo(); err != nil {
 		return nil, err
 	}
 
@@ -228,9 +220,42 @@ func (upload *fileUpload) writeInfo() error {
 	if err != nil {
 		return err
 	}
-	return os.WriteFile(upload.infoPath, data, defaultFilePerm)
+	return createFile(upload.infoPath, data)
 }
 
 func (upload *fileUpload) FinishUpload(ctx context.Context) error {
 	return nil
 }
+
+// createFile creates the file with the content. If the corresponding directory does not exist,
+// it is created. If the file already exists, its content is removed.
+func createFile(path string, content []byte) error {
+	file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, defaultFilePerm)
+	if err != nil {
+		if os.IsNotExist(err) {
+			// An upload ID containing slashes is mapped onto different directories on disk,
+			// for example, `myproject/uploadA` should be put into a folder called `myproject`.
+			// If we get an error indicating that a directory is missing, we try to create it.
+			if err := os.MkdirAll(filepath.Dir(path), defaultDirectoryPerm); err != nil {
+				return fmt.Errorf("failed to create directory for %s: %s", path, err)
+			}
+
+			// Try creating the file again.
+			file, err = os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, defaultFilePerm)
+			if err != nil {
+				// If that still doesn't work, error out.
+				return err
+			}
+		} else {
+			return err
+		}
+	}
+
+	if content != nil {
+		if _, err := file.Write(content); err != nil {
+			return err
+		}
+	}
+
+	return file.Close()
+}

pkg/filestore/filestore_test.go

@@ -76,16 +76,74 @@ func TestFilestore(t *testing.T) {
 	a.Equal(handler.ErrNotFound, err)
 }
 
-func TestMissingPath(t *testing.T) {
+// TestCreateDirectories tests whether an upload with a slash in its ID causes
+// the correct directories to be created.
+func TestCreateDirectories(t *testing.T) {
 	a := assert.New(t)
 
-	store := FileStore{"./path-that-does-not-exist"}
+	tmp, err := os.MkdirTemp("", "tusd-filestore-")
+	a.NoError(err)
+
+	store := FileStore{tmp}
 	ctx := context.Background()
 
-	upload, err := store.NewUpload(ctx, handler.FileInfo{})
-	a.Error(err)
-	a.Equal("upload directory does not exist: ./path-that-does-not-exist", err.Error())
+	// Create new upload
+	upload, err := store.NewUpload(ctx, handler.FileInfo{
+		ID:   "hello/world/123",
+		Size: 42,
+		MetaData: map[string]string{
+			"hello": "world",
+		},
+	})
+	a.NoError(err)
+	a.NotEqual(nil, upload)
+
+	// Check info without writing
+	info, err := upload.GetInfo(ctx)
+	a.NoError(err)
+	a.EqualValues(42, info.Size)
+	a.EqualValues(0, info.Offset)
+	a.Equal(handler.MetaData{"hello": "world"}, info.MetaData)
+	a.Equal(2, len(info.Storage))
+	a.Equal("filestore", info.Storage["Type"])
+	a.Equal(filepath.Join(tmp, info.ID), info.Storage["Path"])
+
+	// Write data to upload
+	bytesWritten, err := upload.WriteChunk(ctx, 0, strings.NewReader("hello world"))
+	a.NoError(err)
+	a.EqualValues(len("hello world"), bytesWritten)
+
+	// Check new offset
+	info, err = upload.GetInfo(ctx)
+	a.NoError(err)
+	a.EqualValues(42, info.Size)
+	a.EqualValues(11, info.Offset)
+
+	// Read content
+	reader, err := upload.GetReader(ctx)
+	a.NoError(err)
+
+	content, err := io.ReadAll(reader)
+	a.NoError(err)
+	a.Equal("hello world", string(content))
+	reader.(io.Closer).Close()
+
+	// Check that the file and directory exists on disk
+	statInfo, err := os.Stat(filepath.Join(tmp, "hello/world/123"))
+	a.NoError(err)
+	a.True(statInfo.Mode().IsRegular())
+	a.EqualValues(11, statInfo.Size())
+	statInfo, err = os.Stat(filepath.Join(tmp, "hello/world/"))
+	a.NoError(err)
+	a.True(statInfo.Mode().IsDir())
+
+	// Terminate upload
+	a.NoError(store.AsTerminatableUpload(upload).Terminate(ctx))
+
+	// Test if upload is deleted
+	upload, err = store.GetUpload(ctx, info.ID)
 	a.Equal(nil, upload)
+	a.Equal(handler.ErrNotFound, err)
 }
 
 func TestNotFound(t *testing.T) {

pkg/handler/concat_test.go

@@ -326,5 +326,60 @@ func TestConcat(t *testing.T) {
 				Code: http.StatusBadRequest,
 			}).Run(handler, t)
 		})
+
+		// Test that we can concatenate uploads, whose IDs contain slashes.
+		SubTest(t, "UploadIDsWithSlashes", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) {
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+			uploadA := NewMockFullUpload(ctrl)
+			uploadB := NewMockFullUpload(ctrl)
+			uploadC := NewMockFullUpload(ctrl)
+
+			gomock.InOrder(
+				store.EXPECT().GetUpload(gomock.Any(), "aaa/123").Return(uploadA, nil),
+				uploadA.EXPECT().GetInfo(gomock.Any()).Return(FileInfo{
+					IsPartial: true,
+					Size:      5,
+					Offset:    5,
+				}, nil),
+				store.EXPECT().GetUpload(gomock.Any(), "bbb/123").Return(uploadB, nil),
+				uploadB.EXPECT().GetInfo(gomock.Any()).Return(FileInfo{
+					IsPartial: true,
+					Size:      5,
+					Offset:    5,
+				}, nil),
+				store.EXPECT().NewUpload(gomock.Any(), FileInfo{
+					Size:           10,
+					IsPartial:      false,
+					IsFinal:        true,
+					PartialUploads: []string{"aaa/123", "bbb/123"},
+					MetaData:       make(map[string]string),
+				}).Return(uploadC, nil),
+				uploadC.EXPECT().GetInfo(gomock.Any()).Return(FileInfo{
+					ID:             "foo",
+					Size:           10,
+					IsPartial:      false,
+					IsFinal:        true,
+					PartialUploads: []string{"aaa/123", "bbb/123"},
+					MetaData:       make(map[string]string),
+				}, nil),
+				store.EXPECT().AsConcatableUpload(uploadC).Return(uploadC),
+				uploadC.EXPECT().ConcatUploads(gomock.Any(), []Upload{uploadA, uploadB}).Return(nil),
+			)
+
+			handler, _ := NewHandler(Config{
+				BasePath:      "files",
+				StoreComposer: composer,
+			})
+
+			(&httpTest{
+				Method: "POST",
+				ReqHeader: map[string]string{
+					"Tus-Resumable": "1.0.0",
+					"Upload-Concat": "final; http://tus.io/files/aaa/123 /files/bbb/123",
+				},
+				Code: http.StatusCreated,
+			}).Run(handler, t)
+		})
 	})
 }

pkg/handler/handler.go

@@ -2,11 +2,10 @@ package handler
 
 import (
 	"net/http"
-
-	"github.com/bmizerany/pat"
+	"strings"
 )
 
-// Handler is a ready to use handler with routing (using pat)
+// Handler is a ready to use handler with routing
 type Handler struct {
 	*UnroutedHandler
 	http.Handler
@@ -33,21 +32,46 @@ func NewHandler(config Config) (*Handler, error) {
 		UnroutedHandler: handler,
 	}
 
-	mux := pat.New()
-
-	routedHandler.Handler = handler.Middleware(mux)
+	mux := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		method := r.Method
+		path := strings.Trim(r.URL.Path, "/")
 
-	mux.Post("", http.HandlerFunc(handler.PostFile))
-	mux.Head(":id", http.HandlerFunc(handler.HeadFile))
-	mux.Add("PATCH", ":id", http.HandlerFunc(handler.PatchFile))
-	if !config.DisableDownload {
-		mux.Get(":id", http.HandlerFunc(handler.GetFile))
-	}
+		switch path {
+		case "":
+			// Root endpoint for upload creation
+			switch method {
+			case "POST":
+				handler.PostFile(w, r)
+			default:
+				w.Header().Add("Allow", "POST")
+				w.WriteHeader(http.StatusMethodNotAllowed)
+				w.Write([]byte(`method not allowed`))
+			}
+		default:
+			// URL points to an upload resource
+			switch {
+			case method == "HEAD" && r.URL.Path != "":
+				// Offset retrieval
+				handler.HeadFile(w, r)
+			case method == "PATCH" && r.URL.Path != "":
+				// Upload apppending
+				handler.PatchFile(w, r)
+			case method == "GET" && r.URL.Path != "" && !config.DisableDownload:
+				// Upload download
+				handler.GetFile(w, r)
+			case method == "DELETE" && r.URL.Path != "" && config.StoreComposer.UsesTerminater && !config.DisableTermination:
+				// Upload termination
+				handler.DelFile(w, r)
+			default:
+				// TODO: Only add GET and DELETE if they are supported
+				w.Header().Add("Allow", "GET, HEAD, PATCH, DELETE")
+				w.WriteHeader(http.StatusMethodNotAllowed)
+				w.Write([]byte(`method not allowed`))
+			}
+		}
+	})
 
-	// Only attach the DELETE handler if the Terminate() method is provided
-	if config.StoreComposer.UsesTerminater && !config.DisableTermination {
-		mux.Del(":id", http.HandlerFunc(handler.DelFile))
-	}
+	routedHandler.Handler = handler.Middleware(mux)
 
 	return routedHandler, nil
 }

pkg/handler/unrouted_handler.go

@@ -20,7 +20,6 @@ const UploadLengthDeferred = "1"
 const currentUploadDraftInteropVersion = "4"
 
 var (
-	reExtractFileID  = regexp.MustCompile(`([^/]+)\/?$`)
 	reForwardedHost  = regexp.MustCompile(`host="?([^;"]+)`)
 	reForwardedProto = regexp.MustCompile(`proto=(https?)`)
 	reMimeType       = regexp.MustCompile(`^[a-z]+\/[a-z0-9\-\+\.]+$`)
@@ -276,7 +275,7 @@ func (handler *UnroutedHandler) PostFile(w http.ResponseWriter, r *http.Request)
 	}
 
 	// Parse Upload-Concat header
-	isPartial, isFinal, partialUploadIDs, err := parseConcat(concatHeader)
+	isPartial, isFinal, partialUploadIDs, err := parseConcat(concatHeader, handler.basePath)
 	if err != nil {
 		handler.sendError(c, err)
 		return
@@ -1398,7 +1397,7 @@ func SerializeMetadataHeader(meta map[string]string) string {
 // Parse the Upload-Concat header, e.g.
 // Upload-Concat: partial
 // Upload-Concat: final;http://tus.io/files/a /files/b/
-func parseConcat(header string) (isPartial bool, isFinal bool, partialUploads []string, err error) {
+func parseConcat(header string, basePath string) (isPartial bool, isFinal bool, partialUploads []string, err error) {
 	if len(header) == 0 {
 		return
 	}
@@ -1419,7 +1418,7 @@ func parseConcat(header string) (isPartial bool, isFinal bool, partialUploads []
 				continue
 			}
 
-			id, extractErr := extractIDFromPath(value)
+			id, extractErr := extractIDFromURL(value, basePath)
 			if extractErr != nil {
 				err = extractErr
 				return
@@ -1438,13 +1437,25 @@ func parseConcat(header string) (isPartial bool, isFinal bool, partialUploads []
 	return
 }
 
-// extractIDFromPath pulls the last segment from the url provided
-func extractIDFromPath(url string) (string, error) {
-	result := reExtractFileID.FindStringSubmatch(url)
-	if len(result) != 2 {
+// extractIDFromPath extracts the upload ID from a path, which has already
+// been stripped of the base path (done by the user). Effectively, we only
+// remove leading and trailing slashes.
+func extractIDFromPath(path string) (string, error) {
+	return strings.Trim(path, "/"), nil
+}
+
+// extractIDFromURL extracts the upload ID from a full URL or a full path
+// (including the base path). For example:
+//
+//	https://example.com/files/1234/5678 -> 1234/5678
+//	/files/1234/5678 -> 1234/5678
+func extractIDFromURL(url string, basePath string) (string, error) {
+	_, id, ok := strings.Cut(url, basePath)
+	if !ok {
 		return "", ErrNotFound
 	}
-	return result[1], nil
+
+	return extractIDFromPath(id)
 }
 
 // getRequestId returns the value of the X-Request-ID header, if available,