upstream/mattermost
1
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2026-05-28 04:35:04 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[MM-29240] Improve buffer handling of UploadEmojiImage (#25536)

Browse source
This commit is contained in:
Ben Schumacher 2023-12-19 10:00:01 +01:00 committed by GitHub
parent 8a0cef910a
commit 799ec680bd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 196 additions and 79 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
server/channels/app/app_iface.go
View file

@ -1163,7 +1163,6 @@ type AppIface interface {
UpdateUserRoles(c request.CTX, userID string, newRoles string, sendWebSocketEvent bool) (*model.User, *model.AppError)
UpdateUserRolesWithUser(c request.CTX, user *model.User, newRoles string, sendWebSocketEvent bool) (*model.User, *model.AppError)
UploadData(c request.CTX, us *model.UploadSession, rd io.Reader) (*model.FileInfo, *model.AppError)
UploadEmojiImage(c request.CTX, id string, imageData *multipart.FileHeader) *model.AppError
UploadFileForUserAndTeam(c request.CTX, data []byte, channelID string, filename string, rawUserId string, rawTeamId string) (*model.FileInfo, *model.AppError)
UpsertDraft(c request.CTX, draft *model.Draft, connectionID string) (*model.Draft, *model.AppError)
UpsertGroupMember(groupID string, userID string) (*model.GroupMember, *model.AppError)

100
server/channels/app/emoji.go
View file

@ -38,11 +38,11 @@ const (
func (a *App) CreateEmoji(c request.CTX, sessionUserId string, emoji *model.Emoji, multiPartImageData *multipart.Form) (*model.Emoji, *model.AppError) {
if !*a.Config().ServiceSettings.EnableCustomEmoji {
return nil, model.NewAppError("UploadEmojiImage", "api.emoji.disabled.app_error", nil, "", http.StatusForbidden)
return nil, model.NewAppError("CreateEmoji", "api.emoji.disabled.app_error", nil, "", http.StatusForbidden)
}
if *a.Config().FileSettings.DriverName == "" {
return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusForbidden)
return nil, model.NewAppError("CreateEmoji", "api.emoji.storage.app_error", nil, "", http.StatusForbidden)
}
// wipe the emoji id so that existing emojis can't get overwritten
@ -56,11 +56,11 @@ func (a *App) CreateEmoji(c request.CTX, sessionUserId string, emoji *model.Emoj
}
if emoji.CreatorId != sessionUserId {
return nil, model.NewAppError("createEmoji", "api.emoji.create.other_user.app_error", nil, "", http.StatusForbidden)
return nil, model.NewAppError("CreateEmoji", "api.emoji.create.other_user.app_error", nil, "", http.StatusForbidden)
}
if existingEmoji, err := a.Srv().Store().Emoji().GetByName(c, emoji.Name, true); err == nil && existingEmoji != nil {
return nil, model.NewAppError("createEmoji", "api.emoji.create.duplicate.app_error", nil, "", http.StatusBadRequest).Wrap(err)
return nil, model.NewAppError("CreateEmoji", "api.emoji.create.duplicate.app_error", nil, "", http.StatusBadRequest).Wrap(err)
}
imageData := multiPartImageData.File["image"]
@ -68,11 +68,18 @@ func (a *App) CreateEmoji(c request.CTX, sessionUserId string, emoji *model.Emoj
return nil, model.NewAppError("Context", "api.context.invalid_body_param.app_error", map[string]any{"Name": "createEmoji"}, "", http.StatusBadRequest)
}
if appErr := a.UploadEmojiImage(c, emoji.Id, imageData[0]); appErr != nil {
filename := imageData[0].Filename
file, err := imageData[0].Open()
if err != nil {
return nil, model.NewAppError("CreateEmoji", "api.emoji.upload.open.app_error", nil, "", http.StatusBadRequest).Wrap(err)
}
defer file.Close()
if appErr := a.uploadEmojiImage(c, emoji.Id, filename, file); appErr != nil {
return nil, appErr
}
emoji, err := a.Srv().Store().Emoji().Save(emoji)
emoji, err = a.Srv().Store().Emoji().Save(emoji)
if err != nil {
return nil, model.NewAppError("CreateEmoji", "app.emoji.create.internal_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
@ -96,26 +103,9 @@ func (a *App) GetEmojiList(c request.CTX, page, perPage int, sort string) ([]*mo
return list, nil
}
func (a *App) UploadEmojiImage(c request.CTX, id string, imageData *multipart.FileHeader) *model.AppError {
if !*a.Config().ServiceSettings.EnableCustomEmoji {
return model.NewAppError("UploadEmojiImage", "api.emoji.disabled.app_error", nil, "", http.StatusForbidden)
}
if *a.Config().FileSettings.DriverName == "" {
return model.NewAppError("UploadEmojiImage", "api.emoji.storage.app_error", nil, "", http.StatusForbidden)
}
file, err := imageData.Open()
if err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.open.app_error", nil, "", http.StatusBadRequest).Wrap(err)
}
defer file.Close()
buf := bytes.NewBuffer(nil)
io.Copy(buf, file)
func (a *App) uploadEmojiImage(c request.CTX, id string, filename string, file io.ReadSeeker) *model.AppError {
// make sure the file is an image and is within the required dimensions
config, _, err := image.DecodeConfig(bytes.NewReader(buf.Bytes()))
config, _, err := image.DecodeConfig(file)
if err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.image.app_error", nil, "", http.StatusBadRequest).Wrap(err)
}
@ -127,37 +117,40 @@ func (a *App) UploadEmojiImage(c request.CTX, id string, imageData *multipart.Fi
}, "", http.StatusBadRequest)
}
if config.Width > MaxEmojiWidth || config.Height > MaxEmojiHeight {
data := buf.Bytes()
newbuf := bytes.NewBuffer(nil)
info, err := getInfoForBytes(imageData.Filename, bytes.NewReader(data), len(data))
_, err = file.Seek(0, io.SeekStart)
if err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.seek.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
if config.Width <= MaxEmojiWidth && config.Height <= MaxEmojiHeight {
// No need to resize the image
_, appErr := a.WriteFile(file, getEmojiImagePath(id))
return appErr
}
// Create a buffer for the resized image
buf := &bytes.Buffer{}
info := model.NewInfo(filename)
if info.MimeType == "image/gif" {
g, err := gif.DecodeAll(file)
if err != nil {
return err
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.gif_decode_error", nil, "", http.StatusBadRequest).Wrap(err)
}
if info.MimeType == "image/gif" {
gif_data, err := gif.DecodeAll(bytes.NewReader(data))
if err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.gif_decode_error", nil, "", http.StatusBadRequest).Wrap(err)
}
resizeEmojiGif(g)
if err := gif.EncodeAll(buf, g); err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.gif_encode_error", nil, "", http.StatusBadRequest).Wrap(err)
}
} else {
img, _, err := image.Decode(file)
if err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.decode_error", nil, "", http.StatusBadRequest).Wrap(err)
}
resized_gif := resizeEmojiGif(gif_data)
if err := gif.EncodeAll(newbuf, resized_gif); err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.gif_encode_error", nil, "", http.StatusBadRequest).Wrap(err)
}
buf = newbuf
} else {
img, _, err := image.Decode(bytes.NewReader(data))
if err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.decode_error", nil, "", http.StatusBadRequest).Wrap(err)
}
resizedImg := resizeEmoji(img, config.Width, config.Height)
if err := a.ch.imgEncoder.EncodePNG(newbuf, resizedImg); err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.encode_error", nil, "", http.StatusBadRequest).Wrap(err)
}
buf = newbuf
resizedImg := resizeEmoji(img, config.Width, config.Height)
if err := a.ch.imgEncoder.EncodePNG(buf, resizedImg); err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.encode_error", nil, "", http.StatusBadRequest).Wrap(err)
}
}
@ -314,7 +307,7 @@ func (a *App) GetEmojiStaticURL(c request.CTX, emojiName string) (string, *model
}
}
func resizeEmojiGif(gifImg *gif.GIF) *gif.GIF {
func resizeEmojiGif(gifImg *gif.GIF) {
// Create a new RGBA image to hold the incremental frames.
firstFrame := gifImg.Image[0].Bounds()
b := image.Rect(0, 0, firstFrame.Dx(), firstFrame.Dy())
@ -331,7 +324,6 @@ func resizeEmojiGif(gifImg *gif.GIF) *gif.GIF {
// Set new gif width and height
gifImg.Config.Width = resizedImage.Bounds().Dx()
gifImg.Config.Height = resizedImage.Bounds().Dy()
return gifImg
}
func getEmojiImagePath(id string) string {

144
server/channels/app/emoji_bench_test.go Normal file
View file

@ -0,0 +1,144 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package app
import (
"strings"
"testing"
"github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/public/shared/request"
"github.com/mattermost/mattermost/server/v8/channels/utils"
"github.com/stretchr/testify/require"
)
func BenchmarkUploadEmojiImage(b *testing.B) {
th := Setup(b)
b.Cleanup(func() {
b.StopTimer()
th.TearDown()
})
rctx := request.TestContext(b)
b.Run("gif", func(b *testing.B) {
filename := "image.gif"
b.Run("small", func(b *testing.B) {
file := strings.NewReader(string(utils.CreateTestGif(b, 10, 10)))
b.ResetTimer()
for i := 0; i < b.N; i++ {
id := model.NewId()
appErr := th.App.uploadEmojiImage(rctx, id, filename, file)
require.Nil(b, appErr)
_, err := file.Seek(0, 0)
require.NoError(b, err)
}
})
b.Run("max size", func(b *testing.B) {
file := strings.NewReader(string(utils.CreateTestGif(b, MaxEmojiWidth, MaxEmojiHeight)))
b.ResetTimer()
for i := 0; i < b.N; i++ {
id := model.NewId()
appErr := th.App.uploadEmojiImage(rctx, id, filename, file)
require.Nil(b, appErr)
_, err := file.Seek(0, 0)
require.NoError(b, err)
}
})
b.Run("too wide", func(b *testing.B) {
file := strings.NewReader(string(utils.CreateTestGif(b, MaxEmojiOriginalWidth, MaxEmojiHeight)))
b.ResetTimer()
for i := 0; i < b.N; i++ {
id := model.NewId()
appErr := th.App.uploadEmojiImage(rctx, id, filename, file)
require.Nil(b, appErr)
_, err := file.Seek(0, 0)
require.NoError(b, err)
}
})
b.Run("too tall", func(b *testing.B) {
file := strings.NewReader(string(utils.CreateTestGif(b, MaxEmojiWidth, MaxEmojiOriginalWidth)))
b.ResetTimer()
for i := 0; i < b.N; i++ {
id := model.NewId()
appErr := th.App.uploadEmojiImage(rctx, id, filename, file)
require.Nil(b, appErr)
_, err := file.Seek(0, 0)
require.NoError(b, err)
}
})
b.Run("too tall and too wide", func(b *testing.B) {
file := strings.NewReader(string(utils.CreateTestGif(b, MaxEmojiOriginalWidth, MaxEmojiOriginalWidth)))
b.ResetTimer()
for i := 0; i < b.N; i++ {
id := model.NewId()
appErr := th.App.uploadEmojiImage(rctx, id, filename, file)
require.Nil(b, appErr)
_, err := file.Seek(0, 0)
require.NoError(b, err)
}
})
})
b.Run("png", func(b *testing.B) {
filename := "image.png"
b.Run("small", func(b *testing.B) {
file := strings.NewReader(string(utils.CreateTestPng(b, 10, 10)))
b.ResetTimer()
for i := 0; i < b.N; i++ {
id := model.NewId()
appErr := th.App.uploadEmojiImage(rctx, id, filename, file)
require.Nil(b, appErr)
_, err := file.Seek(0, 0)
require.NoError(b, err)
}
})
b.Run("max size", func(b *testing.B) {
file := strings.NewReader(string(utils.CreateTestPng(b, MaxEmojiWidth, MaxEmojiHeight)))
b.ResetTimer()
for i := 0; i < b.N; i++ {
id := model.NewId()
appErr := th.App.uploadEmojiImage(rctx, id, filename, file)
require.Nil(b, appErr)
_, err := file.Seek(0, 0)
require.NoError(b, err)
}
})
b.Run("too wide", func(b *testing.B) {
file := strings.NewReader(string(utils.CreateTestPng(b, MaxEmojiOriginalWidth, MaxEmojiHeight)))
b.ResetTimer()
for i := 0; i < b.N; i++ {
id := model.NewId()
appErr := th.App.uploadEmojiImage(rctx, id, filename, file)
require.Nil(b, appErr)
_, err := file.Seek(0, 0)
require.NoError(b, err)
}
})
b.Run("too tall", func(b *testing.B) {
file := strings.NewReader(string(utils.CreateTestPng(b, MaxEmojiWidth, MaxEmojiOriginalWidth)))
b.ResetTimer()
for i := 0; i < b.N; i++ {
id := model.NewId()
appErr := th.App.uploadEmojiImage(rctx, id, filename, file)
require.Nil(b, appErr)
_, err := file.Seek(0, 0)
require.NoError(b, err)
}
})
b.Run("too tall and too wide", func(b *testing.B) {
file := strings.NewReader(string(utils.CreateTestPng(b, MaxEmojiOriginalWidth, MaxEmojiOriginalWidth)))
b.ResetTimer()
for i := 0; i < b.N; i++ {
id := model.NewId()
appErr := th.App.uploadEmojiImage(rctx, id, filename, file)
require.Nil(b, appErr)
_, err := file.Seek(0, 0)
require.NoError(b, err)
}
})
})
}

22
server/channels/app/opentracing/opentracing_layer.go
View file

@ -18336,28 +18336,6 @@ func (a *OpenTracingAppLayer) UploadData(c request.CTX, us *model.UploadSession,
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) UploadEmojiImage(c request.CTX, id string, imageData *multipart.FileHeader) *model.AppError {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UploadEmojiImage")
a.ctx = newCtx
a.app.Srv().Store().SetContext(newCtx)
defer func() {
a.app.Srv().Store().SetContext(origCtx)
a.ctx = origCtx
}()
defer span.Finish()
resultVar0 := a.app.UploadEmojiImage(c, id, imageData)
if resultVar0 != nil {
span.LogFields(spanlog.Error(resultVar0))
ext.Error.Set(span, true)
}
return resultVar0
}
func (a *OpenTracingAppLayer) UploadFile(c request.CTX, data []byte, channelID string, filename string) (*model.FileInfo, *model.AppError) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UploadFile")

4
server/channels/utils/emoji.go
View file

@ -15,7 +15,7 @@ import (
"github.com/stretchr/testify/require"
)
func CreateTestGif(t *testing.T, width int, height int) []byte {
func CreateTestGif(t testing.TB, width int, height int) []byte {
var buffer bytes.Buffer
err := gif.Encode(&buffer, image.NewRGBA(image.Rect(0, 0, width, height)), nil)
@ -50,7 +50,7 @@ func CreateTestJpeg(t *testing.T, width int, height int) []byte {
return buffer.Bytes()
}
func CreateTestPng(t *testing.T, width int, height int) []byte {
func CreateTestPng(t testing.TB, width int, height int) []byte {
var buffer bytes.Buffer
err := png.Encode(&buffer, image.NewRGBA(image.Rect(0, 0, width, height)))

4
server/i18n/en.json
View file

@ -1852,6 +1852,10 @@
"id": "api.emoji.upload.open.app_error",
"translation": "Unable to create the emoji. An error occurred when trying to open the attached image."
},
{
"id": "api.emoji.upload.seek.app_error",
"translation": "Unable to seek to file start."
},
{
"id": "api.error_get_first_admin_complete_setup",
"translation": "Error trying to retrieve first admin complete setup from the store."