fix(api/v2): align avatar upload body limit with global overhead
MaxBodyBytes was set to exactly the configured max file size, but a multipart request carries extra bytes (boundary, part headers) on top of the file, so a file at the limit could be rejected by Huma before the handler runs. Mirror the +2 MB overhead that Echo's global BodyLimit middleware already allows so a max-sized avatar isn't rejected.
This commit is contained in:
parent
cfac0773d7
commit
2f4e3ecb91
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue