From 9a67ae9ac62dd3722d3e6d050ef8383f8a581a00 Mon Sep 17 00:00:00 2001
From: Kyriakos Z <3829551+koox00@users.noreply.github.com>
Date: Fri, 20 May 2022 17:26:21 +0300
Subject: [PATCH] MM-38982: fixes SVG uploading (#20141)

* MM-38982: fixes SVG uploading

Uploading SVG files was "almost" treated like an image in some places,
resulting in not showing the SVG on the client.
This is happening because when we fail to generate a mini preview for an
image, we prepend "invalid-" to the MimeType.

This commit adds a check to guard against SVG files in methods that make
no sense for SVGs.

* Reverts invalid-{mimetype} fix

* Uses fileInfo.IsSvg() where it can
---
 app/file.go                         | 19 +++++--------------
 app/import_functions.go             |  2 +-
 app/upload.go                       |  2 +-
 model/file_info.go                  |  4 ++++
 services/slackimport/slackimport.go |  2 +-
 5 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/app/file.go b/app/file.go
index edabfaed9a..635ecd391d 100644
--- a/app/file.go
+++ b/app/file.go
@@ -249,7 +249,7 @@ func (a *App) getInfoForFilename(post *model.Post, teamID, channelID, userID, ol
 	info.UpdateAt = post.UpdateAt
 	info.Path = path
 
-	if info.IsImage() {
+	if info.IsImage() && !info.IsSvg() {
 		nameWithoutExtension := name[:strings.LastIndex(name, ".")]
 		info.PreviewPath = pathPrefix + nameWithoutExtension + "_preview.jpg"
 		info.ThumbnailPath = pathPrefix + nameWithoutExtension + "_thumb.jpg"
@@ -752,7 +752,7 @@ func (a *App) UploadFileX(c *request.Context, channelID, name string, input io.R
 
 func (t *UploadFileTask) preprocessImage() *model.AppError {
 	// If SVG, attempt to extract dimensions and then return
-	if t.fileinfo.MimeType == "image/svg+xml" {
+	if t.fileinfo.IsSvg() {
 		svgInfo, err := imaging.ParseSVG(t.teeInput)
 		if err != nil {
 			mlog.Warn("Failed to parse SVG", mlog.Err(err))
@@ -809,7 +809,7 @@ func (t *UploadFileTask) preprocessImage() *model.AppError {
 
 func (t *UploadFileTask) postprocessImage(file io.Reader) {
 	// don't try to process SVG files
-	if t.fileinfo.MimeType == "image/svg+xml" {
+	if t.fileinfo.IsSvg() {
 		return
 	}
 
@@ -937,7 +937,7 @@ func (a *App) DoUploadFileExpectModification(c *request.Context, now time.Time,
 	pathPrefix := now.Format("20060102") + "/teams/" + teamID + "/channels/" + channelID + "/users/" + userID + "/" + info.Id + "/"
 	info.Path = pathPrefix + filename
 
-	if info.IsImage() {
+	if info.IsImage() && !info.IsSvg() {
 		if limitErr := checkImageResolutionLimit(info.Width, info.Height, *a.Config().FileSettings.MaxImageResolution); limitErr != nil {
 			err := model.NewAppError("uploadFile", "api.file.upload_file.large_image.app_error", map[string]interface{}{"Filename": filename}, limitErr.Error(), http.StatusBadRequest)
 			return nil, data, err
@@ -1081,7 +1081,7 @@ func (a *App) generatePreviewImage(img image.Image, previewPath string) {
 // generateMiniPreview updates mini preview if needed
 // will save fileinfo with the preview added
 func (a *App) generateMiniPreview(fi *model.FileInfo) {
-	if fi.IsImage() && fi.MiniPreview == nil {
+	if fi.IsImage() && !fi.IsSvg() && fi.MiniPreview == nil {
 		file, appErr := a.FileReader(fi.Path)
 		if appErr != nil {
 			mlog.Debug("error reading image file", mlog.Err(appErr))
@@ -1093,15 +1093,6 @@ func (a *App) generateMiniPreview(fi *model.FileInfo) {
 			mlog.Debug("generateMiniPreview: prepareImage failed", mlog.Err(err),
 				mlog.String("fileinfo_id", fi.Id), mlog.String("channel_id", fi.ChannelId),
 				mlog.String("creator_id", fi.CreatorId))
-
-			// Since this file is not a valid image (for whatever reason), prevent this fileInfo
-			// from entering generateMiniPreview in the future
-			fi.UpdateAt = model.GetMillis()
-			fi.MimeType = "invalid-" + fi.MimeType
-			if _, err = a.Srv().Store.FileInfo().Upsert(fi); err != nil {
-				mlog.Debug("Invalidating FileInfo failed", mlog.Err(err))
-			}
-
 			return
 		}
 		defer release()
diff --git a/app/import_functions.go b/app/import_functions.go
index 3dabae0bc6..14b72aeb5b 100644
--- a/app/import_functions.go
+++ b/app/import_functions.go
@@ -1233,7 +1233,7 @@ func (a *App) importAttachment(c *request.Context, data *AttachmentImportData, p
 		return nil, appErr
 	}
 
-	if fileInfo.IsImage() {
+	if fileInfo.IsImage() && !fileInfo.IsSvg() {
 		a.HandleImages([]string{fileInfo.PreviewPath}, []string{fileInfo.ThumbnailPath}, [][]byte{fileData})
 	}
 
diff --git a/app/upload.go b/app/upload.go
index ed46a107ab..06ac8965b1 100644
--- a/app/upload.go
+++ b/app/upload.go
@@ -263,7 +263,7 @@ func (a *App) UploadData(c *request.Context, us *model.UploadSession, rd io.Read
 	}
 
 	// image post-processing
-	if info.IsImage() {
+	if info.IsImage() && !info.IsSvg() {
 		if limitErr := checkImageResolutionLimit(info.Width, info.Height, *a.Config().FileSettings.MaxImageResolution); limitErr != nil {
 			return nil, model.NewAppError("uploadData", "app.upload.upload_data.large_image.app_error",
 				map[string]interface{}{"Filename": us.Filename, "Width": info.Width, "Height": info.Height}, "", http.StatusBadRequest)
diff --git a/model/file_info.go b/model/file_info.go
index 9519ef45b9..569689967d 100644
--- a/model/file_info.go
+++ b/model/file_info.go
@@ -107,6 +107,10 @@ func (fi *FileInfo) IsImage() bool {
 	return strings.HasPrefix(fi.MimeType, "image")
 }
 
+func (fi *FileInfo) IsSvg() bool {
+	return fi.MimeType == "image/svg+xml"
+}
+
 func NewInfo(name string) *FileInfo {
 	info := &FileInfo{
 		Name: name,
diff --git a/services/slackimport/slackimport.go b/services/slackimport/slackimport.go
index 57227de7e4..7aa9e13ace 100644
--- a/services/slackimport/slackimport.go
+++ b/services/slackimport/slackimport.go
@@ -791,7 +791,7 @@ func (si *SlackImporter) oldImportFile(timestamp time.Time, file io.Reader, team
 		return nil, err
 	}
 
-	if fileInfo.IsImage() && fileInfo.MimeType != "image/svg+xml" {
+	if fileInfo.IsImage() && !fileInfo.IsSvg() {
 		img, release, err := si.actions.PrepareImage(data)
 		if err != nil {
 			return nil, err