fix(file)!: stream uploads through staged storage

- fix: replace multipart form parsing with streaming multipart reads and
  apply request body limits when max_upload_size is configured.
- refactor: route uploads through staging paths before promotion to
  long-term data paths, keeping incomplete uploads out of durable
  storage records.
- test: cover oversized uploads, unlimited uploads, staged cleanup, and
  local storage promotion boundaries.
- docs: document the staged upload model and multipart parent_id query
  parameter.
This commit is contained in:
2026-07-05 17:18:19 +08:00
parent bfeb4b26e4
commit b988b4b15e
10 changed files with 457 additions and 50 deletions
+3 -3
View File
@@ -15,7 +15,7 @@ Repository (GORM data access) Storage (file I/O)
Rules: Rules:
- Handler has no business logic — parse request, call service, write response. - Handler has no business logic — parse request, call service, write response.
- Service has no HTTP awareness — operates on domain models and interfaces. - Service has no HTTP awareness — operates on domain models and interfaces.
- Repository abstracts the database; Storage abstracts where bytes live. - Repository abstracts the database; Storage abstracts where bytes live, including staged upload promotion.
- `internal/server` is the composition root — wires all dependencies together. - `internal/server` is the composition root — wires all dependencies together.
## Package Map ## Package Map
@@ -34,7 +34,7 @@ Rules:
| **Business** | `internal/service` | Business logic: `AuthService` (register, login, refresh, logout, passkey CRUD) | ✅ | | **Business** | `internal/service` | Business logic: `AuthService` (register, login, refresh, logout, passkey CRUD) | ✅ |
| | `internal/model` | Domain types (User, File, Credential, Session), error codes | ✅ | | | `internal/model` | Domain types (User, File, Credential, Session), error codes | ✅ |
| **Data** | `internal/repository` | Repository interfaces + GORM implementations (User, Session, File, Credential) | ✅ | | **Data** | `internal/repository` | Repository interfaces + GORM implementations (User, Session, File, Credential) | ✅ |
| | `internal/storage` | Storage backend interface + local disk impl | 🛠 WIP | | | `internal/storage` | Storage backend interface + local disk impl, with staging and promotion | 🛠 WIP |
| **Util** | `internal/auth` | JWT sign/verify (HS256), token type discrimination (access/refresh), password hashing (bcrypt), app passkey tokens | ✅ | | **Util** | `internal/auth` | JWT sign/verify (HS256), token type discrimination (access/refresh), password hashing (bcrypt), app passkey tokens | ✅ |
| | `internal/api` | Unified JSON error response helpers | ✅ | | | `internal/api` | Unified JSON error response helpers | ✅ |
@@ -54,7 +54,7 @@ POST /api/v1/account/passkeys
DELETE /api/v1/account/passkeys/:id DELETE /api/v1/account/passkeys/:id
GET /api/v1/files GET /api/v1/files
POST /api/v1/files POST /api/v1/files # JSON creates directories; multipart uploads files. File upload parent_id is a query parameter.
GET /api/v1/files/:id GET /api/v1/files/:id
GET /api/v1/files/:id/content GET /api/v1/files/:id/content
PUT /api/v1/files/:id PUT /api/v1/files/:id
+19
View File
@@ -67,3 +67,22 @@
- Refresh tokens cannot authenticate API requests; access tokens cannot be used to issue new token pairs. - Refresh tokens cannot authenticate API requests; access tokens cannot be used to issue new token pairs.
- The placeholder JWT secret is safe for local startup, but tokens signed with it are invalidated on restart. - The placeholder JWT secret is safe for local startup, but tokens signed with it are invalidated on restart.
- New duration config fields require zero boilerplate — declare as `time.Duration` in the struct. - New duration config fields require zero boilerplate — declare as `time.Duration` in the struct.
## 2026-07-05: Staged File Uploads
**Context**: Multipart uploads previously used `ParseMultipartForm`, which parses the request before service-level size checks and may spill oversized requests to temporary disk. The file service also wrote directly to the long-term storage path and then attempted compensating deletes on upload failure.
**Decisions**:
| Decision | Guidance |
|----------|----------|
| Stream multipart requests | Handlers use `Request.MultipartReader()` instead of `ParseMultipartForm`/`FormFile`, so uploads are streamed into the service. |
| Optional upload limits | `storage.max_upload_size = 0` means unlimited. Positive values enable both HTTP body limiting and service-level file content limiting. |
| Staging before promotion | Storage backends write upload bytes to a staging path first, then promote the object to the long-term data path only after validation succeeds. |
| Promote before DB create | The service promotes the object before creating the active file record, preventing visible DB rows from pointing at missing objects. If DB creation fails after promotion, the service best-effort deletes the promoted object. |
| Upload parent location | Multipart upload `parent_id` is passed as a query parameter, keeping the multipart body focused on the file stream. |
**Consequences**:
- Interrupted, malformed, and oversized uploads leave only staging objects, which are safe to clean by path prefix and age.
- Local storage can promote with `os.Rename`; future S3 storage can implement promotion with copy/delete while keeping business visibility controlled by the DB row.
- A DB failure after promotion can still leave a long-term orphan object, but it is not visible through the file API and can be cleaned independently.
+1 -1
View File
@@ -20,7 +20,7 @@ Package-level implementation order (each task includes unit tests):
3. `internal/model` — domain types, error codes ✅ 3. `internal/model` — domain types, error codes ✅
4. `internal/api` — error response helpers ✅ 4. `internal/api` — error response helpers ✅
5. `internal/auth` — JWT utils ✅ 5. `internal/auth` — JWT utils ✅
6. `internal/storage` — backend interface + local fs 6. `internal/storage` — backend interface + local fs with staged upload promotion
7. `internal/repository` — interfaces + GORM/SQLite impl ✅ 7. `internal/repository` — interfaces + GORM/SQLite impl ✅
8. `internal/service` — auth, file, admin services ✅ (auth done) 8. `internal/service` — auth, file, admin services ✅ (auth done)
9. `internal/middleware` — logger, cors, auth ✅ (auth done) 9. `internal/middleware` — logger, cors, auth ✅ (auth done)
+65 -12
View File
@@ -1,10 +1,12 @@
package handler package handler
import ( import (
"errors"
"fmt" "fmt"
"io" "io"
"log/slog" "log/slog"
"mime" "mime"
"mime/multipart"
"net/http" "net/http"
"net/url" "net/url"
"strconv" "strconv"
@@ -13,10 +15,15 @@ import (
"github.com/dhao2001/mygo/internal/api" "github.com/dhao2001/mygo/internal/api"
"github.com/dhao2001/mygo/internal/middleware" "github.com/dhao2001/mygo/internal/middleware"
"github.com/dhao2001/mygo/internal/model"
"github.com/dhao2001/mygo/internal/service" "github.com/dhao2001/mygo/internal/service"
) )
const (
jsonUploadBodyLimit = 64 << 10
multipartUploadOverhead = 1 << 20
multipartFileField = "file"
)
// FileHandler handles file management endpoints. // FileHandler handles file management endpoints.
type FileHandler struct { type FileHandler struct {
fileService *service.FileService fileService *service.FileService
@@ -47,9 +54,15 @@ func (h *FileHandler) Upload(c *gin.Context) {
// Directory creation via JSON. // Directory creation via JSON.
mediaType, _, _ := mime.ParseMediaType(contentType) mediaType, _, _ := mime.ParseMediaType(contentType)
if mediaType == "application/json" { if mediaType == "application/json" {
c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, jsonUploadBodyLimit)
var req createDirRequest var req createDirRequest
if err := c.ShouldBindJSON(&req); err != nil { if err := c.ShouldBindJSON(&req); err != nil {
slog.DebugContext(c.Request.Context(), "create dir bind failed", "error", err) slog.DebugContext(c.Request.Context(), "create dir bind failed", "error", err)
if isRequestBodyTooLarge(err) {
api.Error(c, http.StatusRequestEntityTooLarge, "request body is too large")
return
}
api.Error(c, http.StatusBadRequest, "invalid request body") api.Error(c, http.StatusBadRequest, "invalid request body")
return return
} }
@@ -65,33 +78,45 @@ func (h *FileHandler) Upload(c *gin.Context) {
} }
// File upload via multipart. // File upload via multipart.
if err := c.Request.ParseMultipartForm(32 << 20); err != nil { if mediaType != "multipart/form-data" {
slog.DebugContext(c.Request.Context(), "parse multipart form failed", "error", err) api.Error(c, http.StatusBadRequest, "unsupported content type")
api.Error(c, http.StatusBadRequest, "invalid multipart form")
return return
} }
parentIDStr := c.PostForm("parent_id") if maxUploadSize := h.fileService.MaxUploadSize(); maxUploadSize > 0 {
c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, maxUploadSize+multipartUploadOverhead)
}
var parentID *string var parentID *string
parentIDStr := c.Query("parent_id")
if parentIDStr != "" { if parentIDStr != "" {
parentID = &parentIDStr parentID = &parentIDStr
} }
header, err := c.FormFile("file") reader, err := c.Request.MultipartReader()
if err != nil { if err != nil {
slog.DebugContext(c.Request.Context(), "form file missing", "error", err) slog.DebugContext(c.Request.Context(), "create multipart reader failed", "error", err)
api.Error(c, http.StatusBadRequest, "missing file field") if isRequestBodyTooLarge(err) {
api.Error(c, http.StatusRequestEntityTooLarge, "request body is too large")
return
}
api.Error(c, http.StatusBadRequest, "invalid multipart form")
return return
} }
file, err := header.Open() part, fileName, err := nextUploadFilePart(reader)
if err != nil { if err != nil {
api.RespondError(c, model.NewInternalError("open uploaded file", err)) slog.DebugContext(c.Request.Context(), "read multipart file part failed", "error", err)
if isRequestBodyTooLarge(err) {
api.Error(c, http.StatusRequestEntityTooLarge, "request body is too large")
return return
} }
defer file.Close() api.Error(c, http.StatusBadRequest, err.Error())
return
}
defer part.Close()
info, err := h.fileService.Upload(c.Request.Context(), userID, parentID, header.Filename, file) info, err := h.fileService.Upload(c.Request.Context(), userID, parentID, fileName, part)
if err != nil { if err != nil {
api.RespondError(c, err) api.RespondError(c, err)
return return
@@ -100,6 +125,34 @@ func (h *FileHandler) Upload(c *gin.Context) {
c.JSON(http.StatusCreated, info) c.JSON(http.StatusCreated, info)
} }
func nextUploadFilePart(reader *multipart.Reader) (*multipart.Part, string, error) {
part, err := reader.NextPart()
if err != nil {
if err == io.EOF {
return nil, "", errors.New("missing file field")
}
return nil, "", err
}
if part.FormName() != multipartFileField {
part.Close()
return nil, "", fmt.Errorf("unexpected multipart field %q", part.FormName())
}
fileName := part.FileName()
if fileName == "" {
part.Close()
return nil, "", errors.New("missing file name")
}
return part, fileName, nil
}
func isRequestBodyTooLarge(err error) bool {
var maxBytesErr *http.MaxBytesError
return errors.As(err, &maxBytesErr)
}
// List handles GET /api/v1/files. // List handles GET /api/v1/files.
func (h *FileHandler) List(c *gin.Context) { func (h *FileHandler) List(c *gin.Context) {
userID := middleware.MustGetUserID(c) userID := middleware.MustGetUserID(c)
+86 -3
View File
@@ -8,6 +8,7 @@ import (
"mime/multipart" "mime/multipart"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"strings"
"testing" "testing"
"github.com/gin-gonic/gin" "github.com/gin-gonic/gin"
@@ -29,7 +30,7 @@ func newInMemStore() *inMemStore {
return &inMemStore{files: make(map[string][]byte)} return &inMemStore{files: make(map[string][]byte)}
} }
func (s *inMemStore) Save(_ context.Context, path string, reader io.Reader) (int64, error) { func (s *inMemStore) SaveStaged(_ context.Context, path string, reader io.Reader) (int64, error) {
data, err := io.ReadAll(reader) data, err := io.ReadAll(reader)
if err != nil { if err != nil {
return 0, err return 0, err
@@ -38,6 +39,16 @@ func (s *inMemStore) Save(_ context.Context, path string, reader io.Reader) (int
return int64(len(data)), nil return int64(len(data)), nil
} }
func (s *inMemStore) PromoteStaged(_ context.Context, stagedPath, finalPath string) error {
data, ok := s.files[stagedPath]
if !ok {
return io.ErrUnexpectedEOF
}
s.files[finalPath] = data
delete(s.files, stagedPath)
return nil
}
func (s *inMemStore) Open(_ context.Context, path string) (io.ReadCloser, error) { func (s *inMemStore) Open(_ context.Context, path string) (io.ReadCloser, error) {
data, ok := s.files[path] data, ok := s.files[path]
if !ok { if !ok {
@@ -46,6 +57,10 @@ func (s *inMemStore) Open(_ context.Context, path string) (io.ReadCloser, error)
return io.NopCloser(bytes.NewReader(data)), nil return io.NopCloser(bytes.NewReader(data)), nil
} }
func (s *inMemStore) DeleteStaged(ctx context.Context, path string) error {
return s.Delete(ctx, path)
}
func (s *inMemStore) Delete(_ context.Context, path string) error { func (s *inMemStore) Delete(_ context.Context, path string) error {
delete(s.files, path) delete(s.files, path)
return nil return nil
@@ -58,6 +73,10 @@ func (e *fsNotFoundError) Error() string { return "file not found on disk: " + e
var _ storage.StorageBackend = (*inMemStore)(nil) var _ storage.StorageBackend = (*inMemStore)(nil)
func setupFileHandler(t *testing.T) *FileHandler { func setupFileHandler(t *testing.T) *FileHandler {
return setupFileHandlerWithMaxUploadSize(t, 0)
}
func setupFileHandlerWithMaxUploadSize(t *testing.T, maxUploadSize int64) *FileHandler {
t.Helper() t.Helper()
db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{})
@@ -70,15 +89,19 @@ func setupFileHandler(t *testing.T) *FileHandler {
fileRepo := repository.NewFileRepository(db) fileRepo := repository.NewFileRepository(db)
store := newInMemStore() store := newInMemStore()
fileService := service.NewFileService(fileRepo, store, 0, nil) fileService := service.NewFileService(fileRepo, store, maxUploadSize, nil)
return NewFileHandler(fileService) return NewFileHandler(fileService)
} }
func setupFileRouter(t *testing.T) *gin.Engine { func setupFileRouter(t *testing.T) *gin.Engine {
return setupFileRouterWithMaxUploadSize(t, 0)
}
func setupFileRouterWithMaxUploadSize(t *testing.T, maxUploadSize int64) *gin.Engine {
t.Helper() t.Helper()
handler := setupFileHandler(t) handler := setupFileHandlerWithMaxUploadSize(t, maxUploadSize)
gin.SetMode(gin.TestMode) gin.SetMode(gin.TestMode)
r := gin.New() r := gin.New()
@@ -133,6 +156,66 @@ func TestFileHandler_Upload(t *testing.T) {
} }
} }
func TestFileHandler_UploadRejectsOversizedFile(t *testing.T) {
r := setupFileRouterWithMaxUploadSize(t, 5)
body := &bytes.Buffer{}
writer := multipart.NewWriter(body)
part, _ := writer.CreateFormFile("file", "too-large.txt")
part.Write([]byte("123456"))
writer.Close()
req := httptest.NewRequest(http.MethodPost, "/api/v1/files", body)
req.Header.Set("Content-Type", writer.FormDataContentType())
req.Header.Set("X-User-ID", "user1")
rec := httptest.NewRecorder()
r.ServeHTTP(rec, req)
if rec.Code != http.StatusRequestEntityTooLarge {
t.Errorf("status = %d, want %d; body = %s", rec.Code, http.StatusRequestEntityTooLarge, rec.Body.String())
}
}
func TestFileHandler_UploadAllowsExactMaxUploadSize(t *testing.T) {
r := setupFileRouterWithMaxUploadSize(t, 5)
body := &bytes.Buffer{}
writer := multipart.NewWriter(body)
part, _ := writer.CreateFormFile("file", "exact.txt")
part.Write([]byte("12345"))
writer.Close()
req := httptest.NewRequest(http.MethodPost, "/api/v1/files", body)
req.Header.Set("Content-Type", writer.FormDataContentType())
req.Header.Set("X-User-ID", "user1")
rec := httptest.NewRecorder()
r.ServeHTTP(rec, req)
if rec.Code != http.StatusCreated {
t.Errorf("status = %d, want %d; body = %s", rec.Code, http.StatusCreated, rec.Body.String())
}
}
func TestFileHandler_UploadUnlimitedWhenMaxUploadSizeIsZero(t *testing.T) {
r := setupFileRouter(t)
body := &bytes.Buffer{}
writer := multipart.NewWriter(body)
part, _ := writer.CreateFormFile("file", "unlimited.txt")
part.Write([]byte(strings.Repeat("a", 128)))
writer.Close()
req := httptest.NewRequest(http.MethodPost, "/api/v1/files", body)
req.Header.Set("Content-Type", writer.FormDataContentType())
req.Header.Set("X-User-ID", "user1")
rec := httptest.NewRecorder()
r.ServeHTTP(rec, req)
if rec.Code != http.StatusCreated {
t.Errorf("status = %d, want %d; body = %s", rec.Code, http.StatusCreated, rec.Body.String())
}
}
func TestFileHandler_UploadNoFile(t *testing.T) { func TestFileHandler_UploadNoFile(t *testing.T) {
r := setupFileRouter(t) r := setupFileRouter(t)
+76 -10
View File
@@ -41,6 +41,8 @@ type FileList struct {
Total int64 `json:"total"` Total int64 `json:"total"`
} }
var errUploadTooLarge = errors.New("upload exceeds maximum size")
// FileService handles file management business logic. // FileService handles file management business logic.
type FileService struct { type FileService struct {
fileRepo repository.FileRepository fileRepo repository.FileRepository
@@ -67,6 +69,12 @@ func NewFileService(
} }
} }
// MaxUploadSize returns the configured maximum upload size in bytes.
// A value of 0 means uploads are not size-limited by MyGO.
func (s *FileService) MaxUploadSize() int64 {
return s.maxUploadSize
}
// Upload stores a file's content and creates its metadata record. // Upload stores a file's content and creates its metadata record.
// The MIME type is always detected server-side from the file content. // The MIME type is always detected server-side from the file content.
func (s *FileService) Upload(ctx context.Context, userID string, parentID *string, fileName string, reader io.Reader) (*FileInfo, error) { func (s *FileService) Upload(ctx context.Context, userID string, parentID *string, fileName string, reader io.Reader) (*FileInfo, error) {
@@ -87,14 +95,16 @@ func (s *FileService) Upload(ctx context.Context, userID string, parentID *strin
return nil, model.NewInternalError("check name conflict", err) return nil, model.NewInternalError("check name conflict", err)
} }
// Limit the reader if a max upload size is configured.
if s.maxUploadSize > 0 { if s.maxUploadSize > 0 {
reader = io.LimitReader(reader, s.maxUploadSize+1) reader = &uploadLimitReader{reader: reader, remaining: s.maxUploadSize}
} }
// Detect MIME type from file content. // Detect MIME type from file content.
head := make([]byte, 512) head := make([]byte, 512)
n, readErr := io.ReadFull(reader, head) n, readErr := io.ReadFull(reader, head)
if isUploadTooLargeError(readErr) {
return nil, uploadTooLargeError(s.maxUploadSize)
}
if readErr != nil && readErr != io.ErrUnexpectedEOF && readErr != io.EOF { if readErr != nil && readErr != io.ErrUnexpectedEOF && readErr != io.EOF {
return nil, model.NewInternalError("read for mime detection", readErr) return nil, model.NewInternalError("read for mime detection", readErr)
} }
@@ -103,21 +113,30 @@ func (s *FileService) Upload(ctx context.Context, userID string, parentID *strin
reader = io.MultiReader(bytes.NewReader(head), reader) reader = io.MultiReader(bytes.NewReader(head), reader)
fileID := uuid.NewString() fileID := uuid.NewString()
storagePath := fmt.Sprintf("%s/%s", userID, fileID) stagedPath := fmt.Sprintf("staging/%s/%s", userID, uuid.NewString())
storagePath := fmt.Sprintf("data/%s/%s", userID, fileID)
// Compute SHA-256 hash while writing to storage. // Compute SHA-256 hash while writing to staging storage.
hasher := sha256.New() hasher := sha256.New()
teeReader := io.TeeReader(reader, hasher) teeReader := io.TeeReader(reader, hasher)
written, err := s.storage.Save(ctx, storagePath, teeReader) written, err := s.storage.SaveStaged(ctx, stagedPath, teeReader)
if err != nil { if err != nil {
return nil, model.NewInternalError("save file", err) _ = s.storage.DeleteStaged(ctx, stagedPath)
if isUploadTooLargeError(err) {
return nil, uploadTooLargeError(s.maxUploadSize)
}
return nil, model.NewInternalError("save staged file", err)
} }
if s.maxUploadSize > 0 && written > s.maxUploadSize { if s.maxUploadSize > 0 && written > s.maxUploadSize {
// Clean up the oversized file. _ = s.storage.DeleteStaged(ctx, stagedPath)
_ = s.storage.Delete(ctx, storagePath) return nil, uploadTooLargeError(s.maxUploadSize)
return nil, &model.AppError{Status: http.StatusBadRequest, Message: fmt.Sprintf("file size exceeds the maximum allowed size of %d bytes", s.maxUploadSize)} }
if err := s.storage.PromoteStaged(ctx, stagedPath, storagePath); err != nil {
_ = s.storage.DeleteStaged(ctx, stagedPath)
return nil, model.NewInternalError("promote staged file", err)
} }
file := &model.File{ file := &model.File{
@@ -134,7 +153,7 @@ func (s *FileService) Upload(ctx context.Context, userID string, parentID *strin
} }
if err := s.fileRepo.Create(ctx, file); err != nil { if err := s.fileRepo.Create(ctx, file); err != nil {
// Compensate: remove the stored file. // Compensate: remove the promoted file.
_ = s.storage.Delete(ctx, storagePath) _ = s.storage.Delete(ctx, storagePath)
if errors.Is(err, model.ErrDuplicate) { if errors.Is(err, model.ErrDuplicate) {
return nil, &model.AppError{Status: http.StatusConflict, Message: "a file with this name already exists in this directory"} return nil, &model.AppError{Status: http.StatusConflict, Message: "a file with this name already exists in this directory"}
@@ -145,6 +164,53 @@ func (s *FileService) Upload(ctx context.Context, userID string, parentID *strin
return modelToFileInfo(file), nil return modelToFileInfo(file), nil
} }
type uploadLimitReader struct {
reader io.Reader
remaining int64
}
func (r *uploadLimitReader) Read(p []byte) (int, error) {
if len(p) == 0 {
return 0, nil
}
if r.remaining == 0 {
var one [1]byte
n, err := r.reader.Read(one[:])
if n > 0 {
p[0] = one[0]
return 1, errUploadTooLarge
}
return 0, err
}
if int64(len(p)) > r.remaining {
p = p[:r.remaining]
}
n, err := r.reader.Read(p)
r.remaining -= int64(n)
return n, err
}
func isUploadTooLargeError(err error) bool {
if err == nil {
return false
}
if errors.Is(err, errUploadTooLarge) {
return true
}
var maxBytesErr *http.MaxBytesError
return errors.As(err, &maxBytesErr)
}
func uploadTooLargeError(maxUploadSize int64) *model.AppError {
if maxUploadSize > 0 {
return &model.AppError{Status: http.StatusRequestEntityTooLarge, Message: fmt.Sprintf("file size exceeds the maximum allowed size of %d bytes", maxUploadSize)}
}
return &model.AppError{Status: http.StatusRequestEntityTooLarge, Message: "request body is too large"}
}
// Download returns a reader for the file's content and its metadata. // Download returns a reader for the file's content and its metadata.
// The caller must close the returned ReadCloser. // The caller must close the returned ReadCloser.
func (s *FileService) Download(ctx context.Context, userID, fileID string) (io.ReadCloser, *FileInfo, error) { func (s *FileService) Download(ctx context.Context, userID, fileID string) (io.ReadCloser, *FileInfo, error) {
+52 -3
View File
@@ -43,7 +43,7 @@ func newMemStorage() *memStorage {
return &memStorage{files: make(map[string][]byte)} return &memStorage{files: make(map[string][]byte)}
} }
func (s *memStorage) Save(_ context.Context, path string, reader io.Reader) (int64, error) { func (s *memStorage) SaveStaged(_ context.Context, path string, reader io.Reader) (int64, error) {
data, err := io.ReadAll(reader) data, err := io.ReadAll(reader)
if err != nil { if err != nil {
return 0, err return 0, err
@@ -54,6 +54,19 @@ func (s *memStorage) Save(_ context.Context, path string, reader io.Reader) (int
return int64(len(data)), nil return int64(len(data)), nil
} }
func (s *memStorage) PromoteStaged(_ context.Context, stagedPath, finalPath string) error {
s.mu.Lock()
defer s.mu.Unlock()
data, ok := s.files[stagedPath]
if !ok {
return io.ErrUnexpectedEOF
}
s.files[finalPath] = data
delete(s.files, stagedPath)
return nil
}
func (s *memStorage) Open(_ context.Context, path string) (io.ReadCloser, error) { func (s *memStorage) Open(_ context.Context, path string) (io.ReadCloser, error) {
s.mu.RLock() s.mu.RLock()
data, ok := s.files[path] data, ok := s.files[path]
@@ -64,6 +77,10 @@ func (s *memStorage) Open(_ context.Context, path string) (io.ReadCloser, error)
return io.NopCloser(bytes.NewReader(data)), nil return io.NopCloser(bytes.NewReader(data)), nil
} }
func (s *memStorage) DeleteStaged(ctx context.Context, path string) error {
return s.Delete(ctx, path)
}
func (s *memStorage) Delete(_ context.Context, path string) error { func (s *memStorage) Delete(_ context.Context, path string) error {
s.mu.Lock() s.mu.Lock()
delete(s.files, path) delete(s.files, path)
@@ -74,6 +91,10 @@ func (s *memStorage) Delete(_ context.Context, path string) error {
var _ storage.StorageBackend = (*memStorage)(nil) var _ storage.StorageBackend = (*memStorage)(nil)
func setupFileServiceWithStorage(t *testing.T) (*FileService, *memStorage) { func setupFileServiceWithStorage(t *testing.T) (*FileService, *memStorage) {
return setupFileServiceWithStorageAndMaxUploadSize(t, 0)
}
func setupFileServiceWithStorageAndMaxUploadSize(t *testing.T, maxUploadSize int64) (*FileService, *memStorage) {
t.Helper() t.Helper()
db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{})
@@ -87,7 +108,7 @@ func setupFileServiceWithStorage(t *testing.T) (*FileService, *memStorage) {
fileRepo := repository.NewFileRepository(db) fileRepo := repository.NewFileRepository(db)
store := newMemStorage() store := newMemStorage()
return NewFileService(fileRepo, store, 0, nil), store // 0 = unlimited, nil logger defaults to slog.Default() return NewFileService(fileRepo, store, maxUploadSize, nil), store // nil logger defaults to slog.Default()
} }
func setupFileService(t *testing.T) *FileService { func setupFileService(t *testing.T) *FileService {
@@ -125,6 +146,34 @@ func TestFileService_Upload(t *testing.T) {
} }
} }
func TestFileService_UploadRejectsOversizedFileAndCleansStaging(t *testing.T) {
svc, store := setupFileServiceWithStorageAndMaxUploadSize(t, 5)
ctx := context.Background()
_, err := svc.Upload(ctx, "user1", nil, "too-large.txt", strings.NewReader("123456"))
assertAppError(t, err, http.StatusRequestEntityTooLarge)
store.mu.RLock()
defer store.mu.RUnlock()
if len(store.files) != 0 {
t.Fatalf("storage files = %#v, want empty after oversized upload", store.files)
}
}
func TestFileService_UploadUnlimitedWhenMaxUploadSizeIsZero(t *testing.T) {
svc := setupFileService(t)
ctx := context.Background()
content := strings.Repeat("a", 128)
info, err := svc.Upload(ctx, "user1", nil, "unlimited.txt", strings.NewReader(content))
if err != nil {
t.Fatalf("Upload = %v", err)
}
if info.Size != int64(len(content)) {
t.Errorf("Size = %d, want %d", info.Size, len(content))
}
}
func TestFileService_UploadIntoDirectory(t *testing.T) { func TestFileService_UploadIntoDirectory(t *testing.T) {
svc := setupFileService(t) svc := setupFileService(t)
ctx := context.Background() ctx := context.Background()
@@ -498,7 +547,7 @@ func TestFileService_SoftDeleteKeepsStorage(t *testing.T) {
t.Fatalf("Delete = %v", err) t.Fatalf("Delete = %v", err)
} }
storagePath := "user1/" + info.ID storagePath := "data/user1/" + info.ID
reader, err := store.Open(ctx, storagePath) reader, err := store.Open(ctx, storagePath)
if err != nil { if err != nil {
t.Fatalf("storage should retain content after soft-delete, got: %v", err) t.Fatalf("storage should retain content after soft-delete, got: %v", err)
+67 -2
View File
@@ -9,6 +9,11 @@ import (
"strings" "strings"
) )
const (
dataPathPrefix = "data"
stagingPathPrefix = "staging"
)
// LocalStorage implements StorageBackend using the local filesystem. // LocalStorage implements StorageBackend using the local filesystem.
type LocalStorage struct { type LocalStorage struct {
basePath string basePath string
@@ -24,12 +29,15 @@ func NewLocalStorage(basePath string) (*LocalStorage, error) {
return &LocalStorage{basePath: abs}, nil return &LocalStorage{basePath: abs}, nil
} }
// Save writes the contents of reader to path under the storage root. // SaveStaged writes the contents of reader to a staging path under the storage root.
func (s *LocalStorage) Save(_ context.Context, path string, reader io.Reader) (int64, error) { func (s *LocalStorage) SaveStaged(_ context.Context, path string, reader io.Reader) (int64, error) {
fullPath := filepath.Join(s.basePath, path) fullPath := filepath.Join(s.basePath, path)
if !isSubPath(s.basePath, fullPath) { if !isSubPath(s.basePath, fullPath) {
return 0, fmt.Errorf("storage: path traversal attempt: %s", path) return 0, fmt.Errorf("storage: path traversal attempt: %s", path)
} }
if !hasPathPrefix(path, stagingPathPrefix) {
return 0, fmt.Errorf("storage: staged path must be under %s/: %s", stagingPathPrefix, path)
}
if err := os.MkdirAll(filepath.Dir(fullPath), 0750); err != nil { if err := os.MkdirAll(filepath.Dir(fullPath), 0750); err != nil {
return 0, fmt.Errorf("create parent directory: %w", err) return 0, fmt.Errorf("create parent directory: %w", err)
@@ -51,6 +59,41 @@ func (s *LocalStorage) Save(_ context.Context, path string, reader io.Reader) (i
return written, nil return written, nil
} }
// PromoteStaged moves a staged file to its final path under the storage root.
func (s *LocalStorage) PromoteStaged(_ context.Context, stagedPath, finalPath string) error {
fullStagedPath := filepath.Join(s.basePath, stagedPath)
if !isSubPath(s.basePath, fullStagedPath) {
return fmt.Errorf("storage: path traversal attempt: %s", stagedPath)
}
if !hasPathPrefix(stagedPath, stagingPathPrefix) {
return fmt.Errorf("storage: staged path must be under %s/: %s", stagingPathPrefix, stagedPath)
}
fullFinalPath := filepath.Join(s.basePath, finalPath)
if !isSubPath(s.basePath, fullFinalPath) {
return fmt.Errorf("storage: path traversal attempt: %s", finalPath)
}
if !hasPathPrefix(finalPath, dataPathPrefix) {
return fmt.Errorf("storage: final path must be under %s/: %s", dataPathPrefix, finalPath)
}
if _, err := os.Stat(fullFinalPath); err == nil {
return fmt.Errorf("final file already exists: %s", finalPath)
} else if !os.IsNotExist(err) {
return fmt.Errorf("check final file: %w", err)
}
if err := os.MkdirAll(filepath.Dir(fullFinalPath), 0750); err != nil {
return fmt.Errorf("create parent directory: %w", err)
}
if err := os.Rename(fullStagedPath, fullFinalPath); err != nil {
return fmt.Errorf("promote staged file: %w", err)
}
return nil
}
// Open returns a reader for the file at path under the storage root. // Open returns a reader for the file at path under the storage root.
func (s *LocalStorage) Open(_ context.Context, path string) (io.ReadCloser, error) { func (s *LocalStorage) Open(_ context.Context, path string) (io.ReadCloser, error) {
fullPath := filepath.Join(s.basePath, path) fullPath := filepath.Join(s.basePath, path)
@@ -68,6 +111,23 @@ func (s *LocalStorage) Open(_ context.Context, path string) (io.ReadCloser, erro
return file, nil return file, nil
} }
// DeleteStaged removes a staged file under the storage root.
func (s *LocalStorage) DeleteStaged(_ context.Context, path string) error {
fullPath := filepath.Join(s.basePath, path)
if !isSubPath(s.basePath, fullPath) {
return fmt.Errorf("storage: path traversal attempt: %s", path)
}
if !hasPathPrefix(path, stagingPathPrefix) {
return fmt.Errorf("storage: staged path must be under %s/: %s", stagingPathPrefix, path)
}
err := os.Remove(fullPath)
if os.IsNotExist(err) {
return nil
}
return err
}
// Delete removes the file at path under the storage root. // Delete removes the file at path under the storage root.
func (s *LocalStorage) Delete(_ context.Context, path string) error { func (s *LocalStorage) Delete(_ context.Context, path string) error {
fullPath := filepath.Join(s.basePath, path) fullPath := filepath.Join(s.basePath, path)
@@ -95,3 +155,8 @@ func isSubPath(base, target string) bool {
// filepath.Rel returns ".." or starts with "../" for paths outside base. // filepath.Rel returns ".." or starts with "../" for paths outside base.
return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
} }
func hasPathPrefix(path, prefix string) bool {
clean := filepath.Clean(path)
return clean == prefix || strings.HasPrefix(clean, prefix+string(filepath.Separator))
}
+76 -10
View File
@@ -17,15 +17,19 @@ func TestSaveAndOpen(t *testing.T) {
ctx := context.Background() ctx := context.Background()
content := "hello, mygo storage" content := "hello, mygo storage"
written, err := store.Save(ctx, "test/hello.txt", strings.NewReader(content)) written, err := store.SaveStaged(ctx, "staging/test/hello.txt", strings.NewReader(content))
if err != nil { if err != nil {
t.Fatalf("Save: %v", err) t.Fatalf("SaveStaged: %v", err)
} }
if written != int64(len(content)) { if written != int64(len(content)) {
t.Errorf("written = %d, want %d", written, len(content)) t.Errorf("written = %d, want %d", written, len(content))
} }
reader, err := store.Open(ctx, "test/hello.txt") if err := store.PromoteStaged(ctx, "staging/test/hello.txt", "data/test/hello.txt"); err != nil {
t.Fatalf("PromoteStaged: %v", err)
}
reader, err := store.Open(ctx, "data/test/hello.txt")
if err != nil { if err != nil {
t.Fatalf("Open: %v", err) t.Fatalf("Open: %v", err)
} }
@@ -40,7 +44,7 @@ func TestSaveAndOpen(t *testing.T) {
} }
// Verify the file exists on disk under the base path. // Verify the file exists on disk under the base path.
expectedPath := dir + "/test/hello.txt" expectedPath := dir + "/data/test/hello.txt"
if _, err := os.Stat(expectedPath); err != nil { if _, err := os.Stat(expectedPath); err != nil {
t.Errorf("file not found on disk at %s: %v", expectedPath, err) t.Errorf("file not found on disk at %s: %v", expectedPath, err)
} }
@@ -54,16 +58,19 @@ func TestDelete(t *testing.T) {
} }
ctx := context.Background() ctx := context.Background()
_, err = store.Save(ctx, "to_delete.txt", strings.NewReader("delete me")) _, err = store.SaveStaged(ctx, "staging/to_delete.txt", strings.NewReader("delete me"))
if err != nil { if err != nil {
t.Fatalf("Save: %v", err) t.Fatalf("SaveStaged: %v", err)
}
if err := store.PromoteStaged(ctx, "staging/to_delete.txt", "data/to_delete.txt"); err != nil {
t.Fatalf("PromoteStaged: %v", err)
} }
if err := store.Delete(ctx, "to_delete.txt"); err != nil { if err := store.Delete(ctx, "data/to_delete.txt"); err != nil {
t.Fatalf("Delete: %v", err) t.Fatalf("Delete: %v", err)
} }
_, err = store.Open(ctx, "to_delete.txt") _, err = store.Open(ctx, "data/to_delete.txt")
if err == nil { if err == nil {
t.Error("Open should fail after delete") t.Error("Open should fail after delete")
} }
@@ -91,9 +98,23 @@ func TestPathTraversalRejected(t *testing.T) {
} }
ctx := context.Background() ctx := context.Background()
_, err = store.Save(ctx, "../escape.txt", strings.NewReader("malicious")) _, err = store.SaveStaged(ctx, "../escape.txt", strings.NewReader("malicious"))
if err == nil { if err == nil {
t.Error("Save should reject path traversal") t.Error("SaveStaged should reject path traversal")
} else if !strings.Contains(err.Error(), "path traversal") {
t.Errorf("expected path traversal error, got: %v", err)
}
err = store.PromoteStaged(ctx, "../escape.txt", "data/escape.txt")
if err == nil {
t.Error("PromoteStaged should reject staged path traversal")
} else if !strings.Contains(err.Error(), "path traversal") {
t.Errorf("expected path traversal error, got: %v", err)
}
err = store.PromoteStaged(ctx, "staging/escape.txt", "../escape.txt")
if err == nil {
t.Error("PromoteStaged should reject final path traversal")
} else if !strings.Contains(err.Error(), "path traversal") { } else if !strings.Contains(err.Error(), "path traversal") {
t.Errorf("expected path traversal error, got: %v", err) t.Errorf("expected path traversal error, got: %v", err)
} }
@@ -111,6 +132,51 @@ func TestPathTraversalRejected(t *testing.T) {
} else if !strings.Contains(err.Error(), "path traversal") { } else if !strings.Contains(err.Error(), "path traversal") {
t.Errorf("expected path traversal error, got: %v", err) t.Errorf("expected path traversal error, got: %v", err)
} }
err = store.DeleteStaged(ctx, "../escape.txt")
if err == nil {
t.Error("DeleteStaged should reject path traversal")
} else if !strings.Contains(err.Error(), "path traversal") {
t.Errorf("expected path traversal error, got: %v", err)
}
}
func TestStagedPathsRequireStagingPrefix(t *testing.T) {
dir := t.TempDir()
store, err := NewLocalStorage(dir)
if err != nil {
t.Fatalf("NewLocalStorage: %v", err)
}
ctx := context.Background()
_, err = store.SaveStaged(ctx, "data/not-staged.txt", strings.NewReader("bad"))
if err == nil {
t.Fatal("SaveStaged should reject non-staging path")
}
err = store.DeleteStaged(ctx, "data/not-staged.txt")
if err == nil {
t.Fatal("DeleteStaged should reject non-staging path")
}
}
func TestPromoteRequiresDataFinalPrefix(t *testing.T) {
dir := t.TempDir()
store, err := NewLocalStorage(dir)
if err != nil {
t.Fatalf("NewLocalStorage: %v", err)
}
ctx := context.Background()
_, err = store.SaveStaged(ctx, "staging/promote.txt", strings.NewReader("content"))
if err != nil {
t.Fatalf("SaveStaged: %v", err)
}
err = store.PromoteStaged(ctx, "staging/promote.txt", "other/promote.txt")
if err == nil {
t.Fatal("PromoteStaged should reject non-data final path")
}
} }
func TestOpenMissingFile(t *testing.T) { func TestOpenMissingFile(t *testing.T) {
+12 -6
View File
@@ -6,18 +6,24 @@ import (
"io" "io"
) )
// StorageBackend abstracts where file content is written, read, and deleted. // StorageBackend abstracts where file content is staged, promoted, read, and
// Paths passed to all methods are relative to the backend's root and must // deleted. Paths passed to all methods are relative to the backend's root and
// not contain path traversal sequences. // must not contain path traversal sequences.
type StorageBackend interface { type StorageBackend interface {
// Save writes the contents of reader to path, creating parent directories // SaveStaged writes the contents of reader to a staging path, creating
// as needed. It returns the number of bytes written. // parent directories as needed. It returns the number of bytes written.
Save(ctx context.Context, path string, reader io.Reader) (int64, error) SaveStaged(ctx context.Context, path string, reader io.Reader) (int64, error)
// PromoteStaged moves a staged object to its final path.
PromoteStaged(ctx context.Context, stagedPath, finalPath string) error
// Open returns a reader for the file at path. The caller must close the // Open returns a reader for the file at path. The caller must close the
// returned ReadCloser. // returned ReadCloser.
Open(ctx context.Context, path string) (io.ReadCloser, error) Open(ctx context.Context, path string) (io.ReadCloser, error)
// DeleteStaged removes a staged object. It is idempotent.
DeleteStaged(ctx context.Context, path string) error
// Delete removes the file at path. It is idempotent — deleting a // Delete removes the file at path. It is idempotent — deleting a
// non-existent file is not an error. // non-existent file is not an error.
Delete(ctx context.Context, path string) error Delete(ctx context.Context, path string) error