diff --git a/pkg/routes/api/v2/avatar_upload.go b/pkg/routes/api/v2/avatar_upload.go index b56566ef8..2154d8b30 100644 --- a/pkg/routes/api/v2/avatar_upload.go +++ b/pkg/routes/api/v2/avatar_upload.go @@ -33,29 +33,17 @@ import ( "github.com/gabriel-vasile/mimetype" ) -// avatarUploadInput is the multipart/form-data request for the avatar upload. -// Huma's MultipartFormFiles renders the "avatar" field as a binary file in the -// generated OpenAPI spec; the file bytes are read from in.RawBody.Data().Avatar. type avatarUploadInput struct { - // contentType lists the part Content-Types Huma's MimeTypeValidator accepts - // before our handler runs. Browsers set a real image Content-Type on the - // part (image/png, image/jpeg, ...) while programmatic clients often send - // application/octet-stream, so both must be allowed or a legitimate upload - // would be rejected with a 422 before reaching the handler. This is NOT the - // security gate: the real, byte-level image check is done in the handler via - // mimetype.DetectReader (the same allow-list v1 uses); the part Content-Type - // is client-controlled and must never be trusted on its own. + // Broad allow-list because Huma's MimeTypeValidator rejects the part pre-handler; octet-stream covers programmatic clients. The real gate is mimetype.DetectReader in the handler. RawBody huma.MultipartFormFiles[struct { Avatar huma.FormFile `form:"avatar" contentType:"image/png,image/jpeg,image/gif,image/webp,image/svg+xml,application/octet-stream" required:"true" doc:"The avatar image to upload. Must be an image; it is resized server-side and re-encoded as PNG."` }] } -// avatarUploadBody wraps the success message returned after an upload. type avatarUploadBody struct { Body *models.Message } -// RegisterAvatarRoutes wires the authenticated user's avatar upload onto the Huma API. func RegisterAvatarRoutes(api huma.API) { tags := []string{"user"} @@ -66,11 +54,9 @@ func RegisterAvatarRoutes(api huma.API) { Method: http.MethodPut, Path: "/user/settings/avatar", Tags: tags, - // Avatars can be larger than Huma's 1 MB default body limit; allow up to - // the configured max file size so legitimate uploads aren't rejected before - // the handler runs. Echo's global BodyLimit middleware still caps the total. + // +2 MB mirrors Echo's global BodyLimit overhead so a max-sized file isn't rejected by multipart boundary/header bytes. // #nosec G115 - configured value won't exceed int64 max in practice. - MaxBodyBytes: int64(config.GetMaxFileSizeInMBytes()) * 1024 * 1024, + MaxBodyBytes: (int64(config.GetMaxFileSizeInMBytes()) + 2) * 1024 * 1024, DefaultStatus: http.StatusOK, }, avatarUpload) } @@ -90,8 +76,7 @@ func avatarUpload(ctx context.Context, in *avatarUploadInput) (*avatarUploadBody s := db.NewSession() defer s.Close() - // Re-fetch the full user so AvatarFileID/Provider are current (the auth - // user from the JWT claims is partial). + // Re-fetch the full user: the auth user from the JWT claims is partial. u, err := user.GetUserByID(s, authUser.ID) if err != nil { _ = s.Rollback() @@ -101,7 +86,7 @@ func avatarUpload(ctx context.Context, in *avatarUploadInput) (*avatarUploadBody src := in.RawBody.Data().Avatar defer func() { _ = src.Close() }() - // Validate we're dealing with an image (same allow-list as v1's UploadAvatar). + // Byte-level image check, same allow-list as v1's UploadAvatar. mime, err := mimetype.DetectReader(src) if err != nil { _ = s.Rollback() diff --git a/pkg/webtests/huma_avatar_upload_test.go b/pkg/webtests/huma_avatar_upload_test.go index f92a65418..326d5a4ba 100644 --- a/pkg/webtests/huma_avatar_upload_test.go +++ b/pkg/webtests/huma_avatar_upload_test.go @@ -37,10 +37,9 @@ import ( "github.com/stretchr/testify/require" ) -// avatarUploadPath is the v2 endpoint under test. const avatarUploadPath = "/api/v2/user/settings/avatar" -// pngBytes builds a small valid PNG so StoreAvatarFile can decode + resize it. +// pngBytes builds a small valid PNG so StoreAvatarFile can decode and resize it. func pngBytes(t *testing.T) []byte { t.Helper() img := image.NewRGBA(image.Rect(0, 0, 8, 8)) @@ -54,10 +53,7 @@ func pngBytes(t *testing.T) []byte { return buf.Bytes() } -// multipartAvatarBody returns a multipart/form-data body with a single -// "avatar" file field plus the matching Content-Type header (with boundary). -// CreateFormFile sets the part's Content-Type to application/octet-stream, which -// mirrors how many programmatic clients upload. +// multipartAvatarBody uses CreateFormFile, which sets the part Content-Type to application/octet-stream, mirroring how many programmatic clients upload. func multipartAvatarBody(t *testing.T, fieldName, filename string, content []byte) (*bytes.Buffer, string) { t.Helper() buf := &bytes.Buffer{} @@ -70,9 +66,7 @@ func multipartAvatarBody(t *testing.T, fieldName, filename string, content []byt return buf, w.FormDataContentType() } -// multipartAvatarBodyWithPartType is like multipartAvatarBody but lets the -// caller set the part's Content-Type header, mirroring how a browser declares a -// real image type (e.g. image/png) on the file part. +// multipartAvatarBodyWithPartType sets a caller-chosen part Content-Type, mirroring how a browser declares a real image type (e.g. image/png). func multipartAvatarBodyWithPartType(t *testing.T, fieldName, filename, partContentType string, content []byte) (*bytes.Buffer, string) { t.Helper() buf := &bytes.Buffer{} @@ -88,7 +82,6 @@ func multipartAvatarBodyWithPartType(t *testing.T, fieldName, filename, partCont return buf, w.FormDataContentType() } -// uploadAvatarRequest dispatches a multipart avatar upload against a prepared echo.Echo. func uploadAvatarRequest(t *testing.T, e *echo.Echo, body *bytes.Buffer, contentType, token string) *httptest.ResponseRecorder { t.Helper() req := httptest.NewRequest(http.MethodPut, avatarUploadPath, body) @@ -112,7 +105,6 @@ func TestAvatarUpload(t *testing.T) { require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) assert.Contains(t, rec.Body.String(), "uploaded successfully") - // The provider must be flipped to "upload" and an avatar file stored. s := db.NewSession() defer s.Close() u, err := user.GetUserByID(s, testuser1.ID) @@ -122,9 +114,7 @@ func TestAvatarUpload(t *testing.T) { }) t.Run("Real image content-type on the part", func(t *testing.T) { - // Browsers declare a real image Content-Type (e.g. image/png) on the - // file part. Huma's MimeTypeValidator must accept it so the request - // reaches the handler instead of being rejected with a 422. + // MimeTypeValidator must accept a real image part type or it 422s before the handler. e, err := setupTestEnv() require.NoError(t, err) token := humaTokenFor(t, &testuser1) @@ -151,7 +141,6 @@ func TestAvatarUpload(t *testing.T) { rec := uploadAvatarRequest(t, e, body, contentType, token) require.Equal(t, http.StatusBadRequest, rec.Code, "body: %s", rec.Body.String()) - // The provider must NOT have been changed. s := db.NewSession() defer s.Close() u, err := user.GetUserByID(s, testuser1.ID) @@ -177,8 +166,7 @@ func TestAvatarUpload(t *testing.T) { e.ServeHTTP(rec, req) require.Equal(t, http.StatusOK, rec.Code) - // Navigate the spec loosely: Huma can emit `type` as either a string - // or an array, so avoid binding it to a concrete Go type. + // Loose decode: Huma can emit `type` as a string or an array. var spec map[string]any require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &spec))