Fix minio client breakage with bifrost round 2 (#20758)

* Revert "Revert "MM-43828: Pass object length for some image operations (#20711)" (#20740)"

This reverts commit c7ae090dad.

* Fix to work in bifrost mode

```release-note
NONE
```

app/app_iface.go

@@ -381,10 +381,6 @@ type AppIface interface {
 	// upload, returning a rejection error. In this case FileInfo would have
 	// contained the last "good" FileInfo before the execution of that plugin.
 	UploadFileX(c *request.Context, channelID, name string, input io.Reader, opts ...func(*UploadFileTask)) (*model.FileInfo, *model.AppError)
-	// Uploads some files to the given team and channel as the given user. files and filenames should have
-	// the same length. clientIds should either not be provided or have the same length as files and filenames.
-	// The provided files should be closed by the caller so that they are not leaked.
-	UploadFiles(c *request.Context, teamID string, channelID string, userID string, files []io.ReadCloser, filenames []string, clientIds []string, now time.Time) (*model.FileUploadResponse, *model.AppError)
 	// UserIsInAdminRoleGroup returns true at least one of the user's groups are configured to set the members as
 	// admins in the given syncable.
 	UserIsInAdminRoleGroup(userID, syncableID string, syncableType model.GroupSyncableType) (bool, *model.AppError)
@@ -1132,7 +1128,6 @@ type AppIface interface {
 	UpdateUserRolesWithUser(c request.CTX, user *model.User, newRoles string, sendWebSocketEvent bool) (*model.User, *model.AppError)
 	UploadData(c *request.Context, us *model.UploadSession, rd io.Reader) (*model.FileInfo, *model.AppError)
 	UploadEmojiImage(id string, imageData *multipart.FileHeader) *model.AppError
-	UploadMultipartFiles(c *request.Context, teamID string, channelID string, userID string, fileHeaders []*multipart.FileHeader, clientIds []string, now time.Time) (*model.FileUploadResponse, *model.AppError)
 	UpsertGroupMember(groupID string, userID string) (*model.GroupMember, *model.AppError)
 	UpsertGroupMembers(groupID string, userIDs []string) ([]*model.GroupMember, *model.AppError)
 	UpsertGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError)

app/file.go

@@ -12,7 +12,6 @@ import (
 	"fmt"
 	"image"
 	"io"
-	"mime/multipart"
 	"net/http"
 	"net/url"
 	"os"
@@ -453,76 +452,6 @@ func GeneratePublicLinkHash(fileID, salt string) string {
 	return base64.RawURLEncoding.EncodeToString(hash.Sum(nil))
 }
 
-func (a *App) UploadMultipartFiles(c *request.Context, teamID string, channelID string, userID string, fileHeaders []*multipart.FileHeader, clientIds []string, now time.Time) (*model.FileUploadResponse, *model.AppError) {
-	files := make([]io.ReadCloser, len(fileHeaders))
-	filenames := make([]string, len(fileHeaders))
-
-	for i, fileHeader := range fileHeaders {
-		file, fileErr := fileHeader.Open()
-		if fileErr != nil {
-			return nil, model.NewAppError("UploadFiles", "api.file.upload_file.read_request.app_error",
-				map[string]any{"Filename": fileHeader.Filename}, fileErr.Error(), http.StatusBadRequest)
-		}
-
-		// Will be closed after UploadFiles returns
-		defer file.Close()
-
-		files[i] = file
-		filenames[i] = fileHeader.Filename
-	}
-
-	return a.UploadFiles(c, teamID, channelID, userID, files, filenames, clientIds, now)
-}
-
-// Uploads some files to the given team and channel as the given user. files and filenames should have
-// the same length. clientIds should either not be provided or have the same length as files and filenames.
-// The provided files should be closed by the caller so that they are not leaked.
-func (a *App) UploadFiles(c *request.Context, teamID string, channelID string, userID string, files []io.ReadCloser, filenames []string, clientIds []string, now time.Time) (*model.FileUploadResponse, *model.AppError) {
-	if *a.Config().FileSettings.DriverName == "" {
-		return nil, model.NewAppError("UploadFiles", "api.file.upload_file.storage.app_error", nil, "", http.StatusNotImplemented)
-	}
-
-	if len(filenames) != len(files) || (len(clientIds) > 0 && len(clientIds) != len(files)) {
-		return nil, model.NewAppError("UploadFiles", "api.file.upload_file.incorrect_number_of_files.app_error", nil, "", http.StatusBadRequest)
-	}
-
-	resStruct := &model.FileUploadResponse{
-		FileInfos: []*model.FileInfo{},
-		ClientIds: []string{},
-	}
-
-	previewPathList := []string{}
-	thumbnailPathList := []string{}
-	imageDataList := [][]byte{}
-
-	for i, file := range files {
-		buf := bytes.NewBuffer(nil)
-		io.Copy(buf, file)
-		data := buf.Bytes()
-
-		info, data, err := a.DoUploadFileExpectModification(c, now, teamID, channelID, userID, filenames[i], data)
-		if err != nil {
-			return nil, err
-		}
-
-		if info.PreviewPath != "" || info.ThumbnailPath != "" {
-			previewPathList = append(previewPathList, info.PreviewPath)
-			thumbnailPathList = append(thumbnailPathList, info.ThumbnailPath)
-			imageDataList = append(imageDataList, data)
-		}
-
-		resStruct.FileInfos = append(resStruct.FileInfos, info)
-
-		if len(clientIds) > 0 {
-			resStruct.ClientIds = append(resStruct.ClientIds, clientIds[i])
-		}
-	}
-
-	a.HandleImages(previewPathList, thumbnailPathList, imageDataList)
-
-	return resStruct, nil
-}
-
 // UploadFile uploads a single file in form of a completely constructed byte array for a channel.
 func (a *App) UploadFile(c request.CTX, data []byte, channelID string, filename string) (*model.FileInfo, *model.AppError) {
 	_, err := a.GetChannel(c, channelID)

app/file_bench_test.go

@@ -9,8 +9,6 @@ import (
 	"image"
 	"image/gif"
 	"image/jpeg"
-	"io"
-	"io/ioutil"
 	"math/rand"
 	"testing"
 	"time"
@@ -130,21 +128,6 @@ func BenchmarkUploadFile(b *testing.B) {
 				th.App.RemoveFile(info.Path)
 			},
 		},
-		{
-			title: "image UploadFiles",
-			f: func(b *testing.B, n int, data []byte, ext string) {
-				resp, err := th.App.UploadFiles(th.Context, teamID, channelID, userID,
-					[]io.ReadCloser{ioutil.NopCloser(bytes.NewReader(data))},
-					[]string{fmt.Sprintf("BenchmarkDoUploadFiles-%d%s", n, ext)},
-					[]string{},
-					time.Now())
-				if err != nil {
-					b.Fatal(err)
-				}
-				th.App.Srv().Store.FileInfo().PermanentDelete(resp.FileInfos[0].Id)
-				th.App.RemoveFile(resp.FileInfos[0].Path)
-			},
-		},
 		{
 			title: "image UploadFileX Content-Length",
 			f: func(b *testing.B, n int, data []byte, ext string) {

app/opentracing/opentracing_layer.go

@@ -17813,50 +17813,6 @@ func (a *OpenTracingAppLayer) UploadFileX(c *request.Context, channelID string,
 	return resultVar0, resultVar1
 }
 
-func (a *OpenTracingAppLayer) UploadFiles(c *request.Context, teamID string, channelID string, userID string, files []io.ReadCloser, filenames []string, clientIds []string, now time.Time) (*model.FileUploadResponse, *model.AppError) {
-	origCtx := a.ctx
-	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UploadFiles")
-
-	a.ctx = newCtx
-	a.app.Srv().Store.SetContext(newCtx)
-	defer func() {
-		a.app.Srv().Store.SetContext(origCtx)
-		a.ctx = origCtx
-	}()
-
-	defer span.Finish()
-	resultVar0, resultVar1 := a.app.UploadFiles(c, teamID, channelID, userID, files, filenames, clientIds, now)
-
-	if resultVar1 != nil {
-		span.LogFields(spanlog.Error(resultVar1))
-		ext.Error.Set(span, true)
-	}
-
-	return resultVar0, resultVar1
-}
-
-func (a *OpenTracingAppLayer) UploadMultipartFiles(c *request.Context, teamID string, channelID string, userID string, fileHeaders []*multipart.FileHeader, clientIds []string, now time.Time) (*model.FileUploadResponse, *model.AppError) {
-	origCtx := a.ctx
-	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UploadMultipartFiles")
-
-	a.ctx = newCtx
-	a.app.Srv().Store.SetContext(newCtx)
-	defer func() {
-		a.app.Srv().Store.SetContext(origCtx)
-		a.ctx = origCtx
-	}()
-
-	defer span.Finish()
-	resultVar0, resultVar1 := a.app.UploadMultipartFiles(c, teamID, channelID, userID, fileHeaders, clientIds, now)
-
-	if resultVar1 != nil {
-		span.LogFields(spanlog.Error(resultVar1))
-		ext.Error.Set(span, true)
-	}
-
-	return resultVar0, resultVar1
-}
-
 func (a *OpenTracingAppLayer) UpsertGroupMember(groupID string, userID string) (*model.GroupMember, *model.AppError) {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UpsertGroupMember")

app/plugin_hooks_test.go

@@ -462,15 +462,12 @@ func TestHookFileWillBeUploaded(t *testing.T) {
 		}, th.App, func(*model.Manifest) plugin.API { return &mockAPI })
 		defer tearDown()
 
-		_, err := th.App.UploadFiles(th.Context,
+		_, err := th.App.UploadFile(th.Context,
-			"noteam",
+			[]byte("inputfile"),
 			th.BasicChannel.Id,
-			th.BasicUser.Id,
+			"testhook.txt",
-			[]io.ReadCloser{ioutil.NopCloser(bytes.NewBufferString("inputfile"))},
-			[]string{"testhook.txt"},
-			[]string{},
-			time.Now(),
 		)
+
 		if assert.NotNil(t, err) {
 			assert.Equal(t, "File rejected by plugin. rejected", err.Message)
 		}
@@ -515,15 +512,12 @@ func TestHookFileWillBeUploaded(t *testing.T) {
 		}, th.App, func(*model.Manifest) plugin.API { return &mockAPI })
 		defer tearDown()
 
-		_, err := th.App.UploadFiles(th.Context,
+		_, err := th.App.UploadFile(th.Context,
-			"noteam",
+			[]byte("inputfile"),
 			th.BasicChannel.Id,
-			th.BasicUser.Id,
+			"testhook.txt",
-			[]io.ReadCloser{ioutil.NopCloser(bytes.NewBufferString("inputfile"))},
-			[]string{"testhook.txt"},
-			[]string{},
-			time.Now(),
 		)
+
 		if assert.NotNil(t, err) {
 			assert.Equal(t, "File rejected by plugin. rejected", err.Message)
 		}
@@ -562,20 +556,16 @@ func TestHookFileWillBeUploaded(t *testing.T) {
 		}, th.App, func(*model.Manifest) plugin.API { return &mockAPI })
 		defer tearDown()
 
-		response, err := th.App.UploadFiles(th.Context,
+		response, err := th.App.UploadFile(th.Context,
-			"noteam",
+			[]byte("inputfile"),
 			th.BasicChannel.Id,
-			th.BasicUser.Id,
+			"testhook.txt",
-			[]io.ReadCloser{ioutil.NopCloser(bytes.NewBufferString("inputfile"))},
-			[]string{"testhook.txt"},
-			[]string{},
-			time.Now(),
 		)
+
 		assert.Nil(t, err)
 		assert.NotNil(t, response)
-		assert.Equal(t, 1, len(response.FileInfos))
 
-		fileID := response.FileInfos[0].Id
+		fileID := response.Id
 		fileInfo, err := th.App.GetFileInfo(fileID)
 		assert.Nil(t, err)
 		assert.NotNil(t, fileInfo)
@@ -638,19 +628,14 @@ func TestHookFileWillBeUploaded(t *testing.T) {
 		}, th.App, func(*model.Manifest) plugin.API { return &mockAPI })
 		defer tearDown()
 
-		response, err := th.App.UploadFiles(th.Context,
+		response, err := th.App.UploadFile(th.Context,
-			"noteam",
+			[]byte("inputfile"),
 			th.BasicChannel.Id,
-			th.BasicUser.Id,
+			"testhook.txt",
-			[]io.ReadCloser{ioutil.NopCloser(bytes.NewBufferString("inputfile"))},
-			[]string{"testhook.txt"},
-			[]string{},
-			time.Now(),
 		)
 		assert.Nil(t, err)
 		assert.NotNil(t, response)
-		assert.Equal(t, 1, len(response.FileInfos))
+		fileID := response.Id
-		fileID := response.FileInfos[0].Id
 
 		fileInfo, err := th.App.GetFileInfo(fileID)
 		assert.Nil(t, err)

i18n/en.json

@@ -1857,10 +1857,6 @@
     "id": "api.file.upload_file.incorrect_number_of_client_ids.app_error",
     "translation": "Unable to upload file(s). Have {{.NumClientIds}} client_ids for {{.NumFiles}} files."
   },
-  {
-    "id": "api.file.upload_file.incorrect_number_of_files.app_error",
-    "translation": "Unable to upload files. Incorrect number of files specified."
-  },
   {
     "id": "api.file.upload_file.large_image.app_error",
     "translation": "File above maximum dimensions could not be uploaded: {{.Filename}}"

shared/filestore/s3store.go

@@ -4,6 +4,7 @@
 package filestore
 
 import (
+	"bytes"
 	"context"
 	"crypto/tls"
 	"io"
@@ -365,7 +366,16 @@ func (b *S3FileBackend) WriteFile(fr io.Reader, path string) (int64, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), b.timeout)
 	defer cancel()
 	options := s3PutOptions(b.encrypt, contentType)
-	info, err := b.client.PutObject(ctx, b.bucket, path, fr, -1, options)
+
+	objSize := -1
+	isCloud := os.Getenv("MM_CLOUD_FILESTORE_BIFROST") != ""
+	// We pass an object size only in situations where bifrost is not
+	// used. Bifrost needs to run in HTTPS, which is not yet deployed.
+	if buf, ok := fr.(*bytes.Buffer); ok && !isCloud {
+		objSize = buf.Len()
+	}
+
+	info, err := b.client.PutObject(ctx, b.bucket, path, fr, int64(objSize), options)
 	if err != nil {
 		return info.Size, errors.Wrapf(err, "unable write the data in the file %s", path)
 	}
@@ -393,7 +403,14 @@ func (b *S3FileBackend) AppendFile(fr io.Reader, path string) (int64, error) {
 	partName := fp + ".part"
 	ctx2, cancel2 := context.WithTimeout(context.Background(), b.timeout)
 	defer cancel2()
-	info, err := b.client.PutObject(ctx2, b.bucket, partName, fr, -1, options)
+	objSize := -1
+	isCloud := os.Getenv("MM_CLOUD_FILESTORE_BIFROST") != ""
+	// We pass an object size only in situations where bifrost is not
+	// used. Bifrost needs to run in HTTPS, which is not yet deployed.
+	if buf, ok := fr.(*bytes.Buffer); ok && !isCloud {
+		objSize = buf.Len()
+	}
+	info, err := b.client.PutObject(ctx2, b.bucket, partName, fr, int64(objSize), options)
 	if err != nil {
 		return 0, errors.Wrapf(err, "unable append the data in the file %s", path)
 	}