diff --git a/classes/media_store.go b/classes/media_store.go new file mode 100644 index 0000000..1fe37f0 --- /dev/null +++ b/classes/media_store.go @@ -0,0 +1,150 @@ +package classes + +import ( + "context" + "errors" + "fmt" + "io" + "path" + "regexp" + "strconv" + "strings" + + "git.golem15.com/golem15/summercms/modules/lagoon/attach" + "gocloud.dev/blob" + "gocloud.dev/gcerrors" +) + +// MediaMaxBytes is the largest journal media upload, 10240 KiB. +const MediaMaxBytes = 10240 << 10 + +// MediaFolderPattern is the characters a media upload folder may hold. A +// folder also holds at most 100 characters and no "..". +var MediaFolderPattern = regexp.MustCompile(`^[A-Za-z0-9_\-/]*$`) + +// mediaMaxSuffix is how many numbered names uniqueMediaKey tries. +const mediaMaxSuffix = 1000 + +// MediaError is an upload StoreMedia refuses. Field is the request field the +// message belongs to ("file" or "folder") and Message is the validation +// message of the public upload route. +type MediaError struct { + Field string + Message string +} + +// Error returns Message. +func (e *MediaError) Error() string { return e.Message } + +// StoredMedia is a stored media object: its public URL and its path +// (/journal//). +type StoredMedia struct { + URL string + Path string +} + +// validMediaFolder applies the upload folder rule. +func validMediaFolder(folder string) bool { + return len(folder) <= 100 && MediaFolderPattern.MatchString(folder) && !strings.Contains(folder, "..") +} + +// StoreMedia stores an uploaded image in bucket under +// /journal//, with a numbered suffix when the name is taken, +// and returns its public URL and path. It is the storage of POST +// /_journal/api/v1/media/upload and of the Blog Editor's image upload. The +// file name's base is kept; its extension must be jpg, jpeg, png, gif or +// webp, and the body may hold at most MediaMaxBytes. A refused upload is a +// *MediaError with the public route's message; any other error (no bucket, +// a read or write failure) is not. +func StoreMedia(ctx context.Context, bucket *blob.Bucket, folder, filename string, body io.Reader) (StoredMedia, error) { + if !validMediaFolder(folder) { + return StoredMedia{}, &MediaError{Field: "folder", Message: "The folder format is invalid."} + } + name := path.Base(strings.ReplaceAll(filename, `\`, "/")) + switch strings.ToLower(strings.TrimPrefix(path.Ext(name), ".")) { + case "jpg", "jpeg", "png", "gif", "webp": + default: + return StoredMedia{}, &MediaError{Field: "file", Message: "The file must be an image."} + } + if bucket == nil { + return StoredMedia{}, errors.New("journal: media bucket is not configured") + } + keyPath := mediaObjectPath(folder, name) + data, err := io.ReadAll(io.LimitReader(body, MediaMaxBytes+1)) + if err != nil { + return StoredMedia{}, fmt.Errorf("journal: read media upload: %w", err) + } + if len(data) > MediaMaxBytes { + return StoredMedia{}, &MediaError{Field: "file", Message: "The file may not be greater than 10240 kilobytes."} + } + keyPath, err = uniqueMediaKey(ctx, bucket, keyPath) + if err != nil { + return StoredMedia{}, err + } + key := strings.TrimPrefix(keyPath, "/") + w, err := bucket.NewWriter(ctx, key, nil) + if err != nil { + return StoredMedia{}, fmt.Errorf("journal: media writer: %w", err) + } + if _, err := w.Write(data); err != nil { + _ = w.Close() + return StoredMedia{}, fmt.Errorf("journal: write media: %w", err) + } + if err := w.Close(); err != nil { + return StoredMedia{}, fmt.Errorf("journal: close media: %w", err) + } + return StoredMedia{URL: attach.PublicURL(key), Path: keyPath}, nil +} + +// mediaObjectPath is /journal// with empty, "." and ".." +// folder segments dropped and a leading "journal" segment not repeated. +func mediaObjectPath(folder, fileName string) string { + segments := make([]string, 0, 4) + for _, part := range strings.Split(strings.Trim(folder, "/"), "/") { + if part == "" || part == "." || part == ".." { + continue + } + segments = append(segments, part) + } + if len(segments) > 0 && strings.EqualFold(segments[0], "journal") { + segments = segments[1:] + } + folder = strings.Join(segments, "/") + if folder == "" { + return "/journal/" + fileName + } + return "/journal/" + folder + "/" + fileName +} + +// uniqueMediaKey returns pathName, or pathName with "-1", "-2", ... before +// its extension when an object already exists there. It never returns the +// path of an existing object: when every numbered name is taken it fails. +func uniqueMediaKey(ctx context.Context, bucket *blob.Bucket, pathName string) (string, error) { + exists := func(p string) (bool, error) { + ok, err := bucket.Exists(ctx, strings.TrimPrefix(p, "/")) + if err != nil && gcerrors.Code(err) != gcerrors.NotFound { + return false, err + } + return ok, nil + } + taken, err := exists(pathName) + if err != nil { + return "", err + } + if !taken { + return pathName, nil + } + ext := path.Ext(pathName) + base := strings.TrimSuffix(pathName, ext) + for n := 1; n < mediaMaxSuffix; n++ { + candidate := base + "-" + strconv.Itoa(n) + ext + taken, err := exists(candidate) + if err != nil { + return "", err + } + if !taken { + return candidate, nil + } + } + return "", fmt.Errorf("journal: no free media name for %s", pathName) +} diff --git a/classes/media_store_test.go b/classes/media_store_test.go new file mode 100644 index 0000000..a5ef181 --- /dev/null +++ b/classes/media_store_test.go @@ -0,0 +1,39 @@ +package classes + +import ( + "strings" + "testing" +) + +func TestMediaObjectPath(t *testing.T) { + got := mediaObjectPath("../etc", "cover.png") + if !strings.HasPrefix(got, "/journal/") { + t.Fatalf("path = %q, want /journal/ prefix", got) + } + if strings.Contains(got, "..") { + t.Fatalf("path still contains ..: %q", got) + } + if got := mediaObjectPath("photos", "a.png"); got != "/journal/photos/a.png" { + t.Fatalf("nested = %q", got) + } + if got := mediaObjectPath("journal/photos", "a.png"); got != "/journal/photos/a.png" { + t.Fatalf("prefixed folder = %q", got) + } + if got := mediaObjectPath("", "a.png"); got != "/journal/a.png" { + t.Fatalf("no folder = %q", got) + } +} + +func TestMediaFolderPatternRejectsDotDot(t *testing.T) { + folder := "../etc" + if MediaFolderPattern.MatchString(folder) && !strings.Contains(folder, "..") { + t.Fatal("folder ../etc should be rejected") + } + if !strings.Contains(folder, "..") { + t.Fatal("test fixture lost ..") + } + if MediaFolderPattern.MatchString("ok_folder-1") { + return + } + t.Fatal("ok_folder-1 should match") +} diff --git a/controllers/api/media.go b/controllers/api/media.go index 7842a74..ff4502a 100644 --- a/controllers/api/media.go +++ b/controllers/api/media.go @@ -1,24 +1,20 @@ package api import ( - "io" + "errors" "net/http" - "path" - "regexp" "strings" + "git.golem15.com/golem15/sm-journal-plugin/classes" "git.golem15.com/golem15/summercms/modules/backpack" - "git.golem15.com/golem15/summercms/modules/lagoon/attach" "git.golem15.com/golem15/summercms/modules/wire" "gocloud.dev/blob" - "gocloud.dev/gcerrors" ) -const mediaMaxBytes = 10240 << 10 - -var mediaFolderPattern = regexp.MustCompile(`^[A-Za-z0-9_\-/]*$`) - -// UploadMedia is POST /_journal/api/v1/media/upload (JOURNAL-006). +// UploadMedia is POST /_journal/api/v1/media/upload (JOURNAL-006). The +// route authenticates, checks permissions, the folder and the form, then +// stores the file with classes.StoreMedia, which the Blog Editor's image +// upload shares. func UploadMedia(app *backpack.App) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { r, pr, ok := requireBackendPrincipal(app, w, r) @@ -29,7 +25,7 @@ func UploadMedia(app *backpack.App) http.HandlerFunc { return } folder := strings.TrimSpace(r.FormValue("folder")) - if len(folder) > 100 || !mediaFolderPattern.MatchString(folder) || strings.Contains(folder, "..") { + if len(folder) > 100 || !classes.MediaFolderPattern.MatchString(folder) || strings.Contains(folder, "..") { writeValidation(w, map[string][]string{"folder": {"The folder format is invalid."}}) return } @@ -39,109 +35,29 @@ func UploadMedia(app *backpack.App) http.HandlerFunc { return } defer file.Close() - name := path.Base(strings.ReplaceAll(header.Filename, `\`, "/")) - ext := strings.ToLower(strings.TrimPrefix(path.Ext(name), ".")) - switch ext { - case "jpg", "jpeg", "png", "gif", "webp": - default: - writeValidation(w, map[string][]string{"file": {"The file must be an image."}}) - return - } - bucket, ok := app.Lookup[*blob.Bucket]() - if !ok || bucket == nil { - writeAPIError(w, http.StatusInternalServerError, "Internal server error") - return - } - keyPath := mediaObjectPath(folder, name) - body, err := io.ReadAll(io.LimitReader(file, mediaMaxBytes+1)) + bucket, _ := app.Lookup[*blob.Bucket]() + stored, err := classes.StoreMedia(r.Context(), bucket, folder, header.Filename, file) if err != nil { - writeAPIError(w, http.StatusInternalServerError, "Internal server error") - return - } - if int64(len(body)) > mediaMaxBytes { - writeValidation(w, map[string][]string{"file": {"The file may not be greater than 10240 kilobytes."}}) - return - } - keyPath, err = uniqueMediaKey(r, bucket, keyPath) - if err != nil { - writeAPIError(w, http.StatusInternalServerError, "Internal server error") - return - } - wtr, err := bucket.NewWriter(r.Context(), strings.TrimPrefix(keyPath, "/"), nil) - if err != nil { - writeAPIError(w, http.StatusInternalServerError, "Internal server error") - return - } - if _, err := wtr.Write(body); err != nil { - _ = wtr.Close() - writeAPIError(w, http.StatusInternalServerError, "Internal server error") - return - } - if err := wtr.Close(); err != nil { - writeAPIError(w, http.StatusInternalServerError, "Internal server error") + writeMediaStoreError(w, err) return } wire.WriteJSON(w, http.StatusCreated, map[string]any{ "data": map[string]any{ - "url": attach.PublicURL(strings.TrimPrefix(keyPath, "/")), - "path": keyPath, + "url": stored.URL, + "path": stored.Path, }, }) } } -func mediaObjectPath(folder, fileName string) string { - segments := make([]string, 0, 4) - for _, part := range strings.Split(strings.Trim(folder, "/"), "/") { - if part == "" || part == "." || part == ".." { - continue - } - segments = append(segments, part) +// writeMediaStoreError answers a StoreMedia failure as the route always +// has: a classes.MediaError is a 422 on its field with its message, any +// other error the opaque 500. +func writeMediaStoreError(w http.ResponseWriter, err error) { + var refused *classes.MediaError + if errors.As(err, &refused) && refused != nil { + writeValidation(w, map[string][]string{refused.Field: {refused.Message}}) + return } - if len(segments) > 0 && strings.EqualFold(segments[0], "journal") { - segments = segments[1:] - } - folder = strings.Join(segments, "/") - if folder == "" { - return "/journal/" + fileName - } - return "/journal/" + folder + "/" + fileName -} - -func uniqueMediaKey(r *http.Request, bucket *blob.Bucket, pathName string) (string, error) { - key := strings.TrimPrefix(pathName, "/") - ok, err := bucket.Exists(r.Context(), key) - if err != nil && gcerrors.Code(err) != gcerrors.NotFound { - return "", err - } - if !ok { - return pathName, nil - } - ext := path.Ext(pathName) - base := strings.TrimSuffix(pathName, ext) - for n := 1; n < 1000; n++ { - candidate := base + "-" + itoa(n) + ext - exists, err := bucket.Exists(r.Context(), strings.TrimPrefix(candidate, "/")) - if err != nil && gcerrors.Code(err) != gcerrors.NotFound { - return "", err - } - if !exists { - return candidate, nil - } - } - return pathName, nil -} - -func itoa(n int) string { - if n == 0 { - return "0" - } - var b [16]byte - i := len(b) - for n > 0 { - i-- - b[i] = byte('0' + n%10) - n /= 10 - } - return string(b[i:]) + writeAPIError(w, http.StatusInternalServerError, "Internal server error") } diff --git a/controllers/api/media_test.go b/controllers/api/media_test.go index 634b966..39b9f3e 100644 --- a/controllers/api/media_test.go +++ b/controllers/api/media_test.go @@ -1,36 +1,47 @@ package api import ( - "strings" + "errors" + "net/http" + "net/http/httptest" "testing" + + "git.golem15.com/golem15/sm-journal-plugin/classes" ) -func TestMediaObjectPath(t *testing.T) { - got := mediaObjectPath("../etc", "cover.png") - if !strings.HasPrefix(got, "/journal/") { - t.Fatalf("path = %q, want /journal/ prefix", got) - } - if strings.Contains(got, "..") { - t.Fatalf("path still contains ..: %q", got) - } - if got := mediaObjectPath("photos", "a.png"); got != "/journal/photos/a.png" { - t.Fatalf("nested = %q", got) - } - if got := mediaObjectPath("journal/photos", "a.png"); got != "/journal/photos/a.png" { - t.Fatalf("prefixed folder = %q", got) +// TestWriteMediaStoreError pins the media upload route's error bodies (D-09) +// after the storage moved to classes.StoreMedia: a MediaError is the same +// 422 body writeValidation always wrote, anything else the opaque 500. +func TestWriteMediaStoreError(t *testing.T) { + for _, tc := range []struct { + name string + err error + status int + body string + }{ + {"size", &classes.MediaError{Field: "file", Message: "The file may not be greater than 10240 kilobytes."}, http.StatusUnprocessableEntity, + `{"error":"Validation failed","errors":{"file":["The file may not be greater than 10240 kilobytes."]}}`}, + {"extension", &classes.MediaError{Field: "file", Message: "The file must be an image."}, http.StatusUnprocessableEntity, + `{"error":"Validation failed","errors":{"file":["The file must be an image."]}}`}, + {"other", errors.New("disk full"), http.StatusInternalServerError, `{"error":"Internal server error"}`}, + } { + t.Run(tc.name, func(t *testing.T) { + rec := httptest.NewRecorder() + writeMediaStoreError(rec, tc.err) + want := httptest.NewRecorder() + if tc.status == http.StatusUnprocessableEntity { + var refused *classes.MediaError + errors.As(tc.err, &refused) + writeValidation(want, map[string][]string{refused.Field: {refused.Message}}) + } else { + writeAPIError(want, http.StatusInternalServerError, "Internal server error") + } + if rec.Code != tc.status || rec.Body.String() != want.Body.String() { + t.Fatalf("got %d %s, want %d %s", rec.Code, rec.Body.String(), tc.status, want.Body.String()) + } + if got := rec.Body.String(); got != tc.body && got != tc.body+"\n" { + t.Fatalf("body = %q, want %q", got, tc.body) + } + }) } } - -func TestMediaFolderPatternRejectsDotDot(t *testing.T) { - folder := "../etc" - if mediaFolderPattern.MatchString(folder) && !strings.Contains(folder, "..") { - t.Fatal("folder ../etc should be rejected") - } - if !strings.Contains(folder, "..") { - t.Fatal("test fixture lost ..") - } - if mediaFolderPattern.MatchString("ok_folder-1") { - return - } - t.Fatal("ok_folder-1 should match") -}