diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 7dda6ded3c..709fdbc71c 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -573,103 +573,71 @@ func CheckNoError(t *testing.T, resp *model.Response) { t.Helper() if resp.Error != nil { - t.Fatal("Expected no error, got " + resp.Error.Error()) + t.Fatalf("Expected no error, got %q", resp.Error.Error()) } } -func CheckCreatedStatus(t *testing.T, resp *model.Response) { +func checkHTTPStatus(t *testing.T, resp *model.Response, expectedStatus int, expectError bool) { t.Helper() - if resp.StatusCode != http.StatusCreated { - t.Log("actual: " + strconv.Itoa(resp.StatusCode)) - t.Log("expected: " + strconv.Itoa(http.StatusCreated)) - t.Fatal("wrong status code") - } -} + switch { + case resp == nil: + t.Fatalf("Unexpected nil response, expected http:%v, expectError:%v)", expectedStatus, expectError) -func CheckForbiddenStatus(t *testing.T, resp *model.Response) { - t.Helper() + case expectError && resp.Error == nil: + t.Fatalf("Expected a non-nil error and http status:%v, got nil, %v", expectedStatus, resp.StatusCode) - if resp.Error == nil { - t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusForbidden)) - return - } + case !expectError && resp.Error != nil: + t.Fatalf("Expected no error and http status:%v, got %q, http:%v", expectedStatus, resp.Error, resp.StatusCode) - if resp.StatusCode != http.StatusForbidden { - t.Log("actual: " + strconv.Itoa(resp.StatusCode)) - t.Log("expected: " + strconv.Itoa(http.StatusForbidden)) - t.Fatal("wrong status code") - } -} - -func CheckUnauthorizedStatus(t *testing.T, resp *model.Response) { - t.Helper() - - if resp.Error == nil { - t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusUnauthorized)) - return - } - - if resp.StatusCode != http.StatusUnauthorized { - t.Log("actual: " + strconv.Itoa(resp.StatusCode)) - t.Log("expected: " + strconv.Itoa(http.StatusUnauthorized)) - t.Fatal("wrong status code") - } -} - -func CheckNotFoundStatus(t *testing.T, resp *model.Response) { - t.Helper() - - if resp.Error == nil { - t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusNotFound)) - return - } - - if resp.StatusCode != http.StatusNotFound { - t.Log("actual: " + strconv.Itoa(resp.StatusCode)) - t.Log("expected: " + strconv.Itoa(http.StatusNotFound)) - t.Fatal("wrong status code") - } -} - -func CheckBadRequestStatus(t *testing.T, resp *model.Response) { - t.Helper() - - if resp.Error == nil { - t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusBadRequest)) - return - } - - if resp.StatusCode != http.StatusBadRequest { - t.Log("actual: " + strconv.Itoa(resp.StatusCode)) - t.Log("expected: " + strconv.Itoa(http.StatusBadRequest)) - t.Fatal("wrong status code") - } -} - -func CheckNotImplementedStatus(t *testing.T, resp *model.Response) { - t.Helper() - - if resp.Error == nil { - t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusNotImplemented)) - return - } - - if resp.StatusCode != http.StatusNotImplemented { - t.Log("actual: " + strconv.Itoa(resp.StatusCode)) - t.Log("expected: " + strconv.Itoa(http.StatusNotImplemented)) - t.Fatal("wrong status code") + case resp.StatusCode != expectedStatus: + t.Fatalf("Expected http status:%v, got %v (err: %q)", expectedStatus, resp.StatusCode, resp.Error) } } func CheckOKStatus(t *testing.T, resp *model.Response) { t.Helper() + checkHTTPStatus(t, resp, http.StatusOK, false) +} - CheckNoError(t, resp) +func CheckCreatedStatus(t *testing.T, resp *model.Response) { + t.Helper() + checkHTTPStatus(t, resp, http.StatusCreated, false) +} - if resp.StatusCode != http.StatusOK { - t.Fatalf("wrong status code. expected %d got %d", http.StatusOK, resp.StatusCode) - } +func CheckForbiddenStatus(t *testing.T, resp *model.Response) { + t.Helper() + checkHTTPStatus(t, resp, http.StatusForbidden, true) +} + +func CheckUnauthorizedStatus(t *testing.T, resp *model.Response) { + t.Helper() + checkHTTPStatus(t, resp, http.StatusUnauthorized, true) +} + +func CheckNotFoundStatus(t *testing.T, resp *model.Response) { + t.Helper() + checkHTTPStatus(t, resp, http.StatusNotFound, true) +} + +func CheckBadRequestStatus(t *testing.T, resp *model.Response) { + t.Helper() + checkHTTPStatus(t, resp, http.StatusBadRequest, true) +} + +func CheckNotImplementedStatus(t *testing.T, resp *model.Response) { + t.Helper() + checkHTTPStatus(t, resp, http.StatusNotImplemented, true) +} + +func CheckRequestEntityTooLargeStatus(t *testing.T, resp *model.Response) { + t.Helper() + checkHTTPStatus(t, resp, http.StatusRequestEntityTooLarge, true) +} + +func CheckInternalErrorStatus(t *testing.T, resp *model.Response) { + t.Helper() + checkHTTPStatus(t, resp, http.StatusInternalServerError, true) } func CheckErrorMessage(t *testing.T, resp *model.Response, errorId string) { @@ -687,21 +655,6 @@ func CheckErrorMessage(t *testing.T, resp *model.Response, errorId string) { } } -func CheckInternalErrorStatus(t *testing.T, resp *model.Response) { - t.Helper() - - if resp.Error == nil { - t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusInternalServerError)) - return - } - - if resp.StatusCode != http.StatusInternalServerError { - t.Log("actual: " + strconv.Itoa(resp.StatusCode)) - t.Log("expected: " + strconv.Itoa(http.StatusInternalServerError)) - t.Fatal("wrong status code") - } -} - // Similar to s3.New() but allows initialization of signature v2 or signature v4 client. // If signV2 input is false, function always returns signature v4. // diff --git a/api4/file.go b/api4/file.go index 0b27890c0e..5d6065a4de 100644 --- a/api4/file.go +++ b/api4/file.go @@ -4,9 +4,12 @@ package api4 import ( + "bytes" "crypto/subtle" "io" "io/ioutil" + "mime" + "mime/multipart" "net/http" "net/url" "strconv" @@ -46,8 +49,11 @@ var MEDIA_CONTENT_TYPES = [...]string{ "audio/wav", } +const maxUploadDrainBytes = (10 * 1024 * 1024) // 10Mb +const maxMultipartFormDataBytes = 10 * 1024 // 10Kb + func (api *API) InitFile() { - api.BaseRoutes.Files.Handle("", api.ApiSessionRequired(uploadFile)).Methods("POST") + api.BaseRoutes.Files.Handle("", api.ApiSessionRequired(uploadFileStream)).Methods("POST") api.BaseRoutes.File.Handle("", api.ApiSessionRequiredTrustRequester(getFile)).Methods("GET") api.BaseRoutes.File.Handle("/thumbnail", api.ApiSessionRequiredTrustRequester(getFileThumbnail)).Methods("GET") api.BaseRoutes.File.Handle("/link", api.ApiSessionRequired(getFileLink)).Methods("GET") @@ -114,8 +120,9 @@ func uploadFile(c *Context, w http.ResponseWriter, r *http.Request) { return } channelId := props["channel_id"][0] - if len(channelId) == 0 { - c.SetInvalidParam("channel_id") + c.Params.ChannelId = channelId + c.RequireChannelId() + if c.Err != nil { return } @@ -143,6 +150,371 @@ func uploadFile(c *Context, w http.ResponseWriter, r *http.Request) { w.Write([]byte(resStruct.ToJson())) } +func parseMultipartRequestHeader(req *http.Request) (boundary string, err error) { + v := req.Header.Get("Content-Type") + if v == "" { + return "", http.ErrNotMultipart + } + d, params, err := mime.ParseMediaType(v) + if err != nil || d != "multipart/form-data" { + return "", http.ErrNotMultipart + } + boundary, ok := params["boundary"] + if !ok { + return "", http.ErrMissingBoundary + } + + return boundary, nil +} + +func multipartReader(req *http.Request, stream io.Reader) (*multipart.Reader, error) { + boundary, err := parseMultipartRequestHeader(req) + if err != nil { + return nil, err + } + + if stream != nil { + return multipart.NewReader(stream, boundary), nil + } else { + return multipart.NewReader(req.Body, boundary), nil + } +} + +func uploadFileStream(c *Context, w http.ResponseWriter, r *http.Request) { + // Drain any remaining bytes in the request body, up to a limit + defer io.CopyN(ioutil.Discard, r.Body, maxUploadDrainBytes) + + if !*c.App.Config().FileSettings.EnableFileAttachments { + c.Err = model.NewAppError("uploadFileStream", + "api.file.attachments.disabled.app_error", + nil, "", http.StatusNotImplemented) + return + } + + // Parse the post as a regular form (in practice, use the URL values + // since we never expect a real application/x-www-form-urlencoded + // form). + if r.Form == nil { + err := r.ParseForm() + if err != nil { + c.Err = model.NewAppError("uploadFileStream", + "api.file.upload_file.read_request.app_error", + nil, err.Error(), http.StatusBadRequest) + return + } + } + + timestamp := time.Now() + var fileUploadResponse *model.FileUploadResponse + + _, err := parseMultipartRequestHeader(r) + switch err { + case nil: + fileUploadResponse = uploadFileMultipart(c, r, nil, timestamp) + + case http.ErrNotMultipart: + fileUploadResponse = uploadFileSimple(c, r, timestamp) + + default: + c.Err = model.NewAppError("uploadFileStream", + "api.file.upload_file.read_request.app_error", + nil, err.Error(), http.StatusBadRequest) + } + if c.Err != nil { + return + } + + // Write the response values to the output upon return + w.WriteHeader(http.StatusCreated) + w.Write([]byte(fileUploadResponse.ToJson())) +} + +// uploadFileSimple uploads a file from a simple POST with the file in the request body +func uploadFileSimple(c *Context, r *http.Request, timestamp time.Time) *model.FileUploadResponse { + // Simple POST with the file in the body and all metadata in the args. + c.RequireChannelId() + c.RequireFilename() + if c.Err != nil { + return nil + } + + if !c.App.SessionHasPermissionToChannel(c.App.Session, c.Params.ChannelId, model.PERMISSION_UPLOAD_FILE) { + c.SetPermissionError(model.PERMISSION_UPLOAD_FILE) + return nil + } + + clientId := r.Form.Get("client_id") + info, appErr := c.App.UploadFileX(c.Params.ChannelId, c.Params.Filename, r.Body, + app.UploadFileSetTeamId(FILE_TEAM_ID), + app.UploadFileSetUserId(c.App.Session.UserId), + app.UploadFileSetTimestamp(timestamp), + app.UploadFileSetContentLength(r.ContentLength), + app.UploadFileSetClientId(clientId)) + if appErr != nil { + c.Err = appErr + return nil + } + + fileUploadResponse := &model.FileUploadResponse{ + FileInfos: []*model.FileInfo{info}, + } + if clientId != "" { + fileUploadResponse.ClientIds = []string{clientId} + } + return fileUploadResponse +} + +// uploadFileMultipart parses and uploads file(s) from a mime/multipart +// request. It pre-buffers up to the first part which is either the (a) +// `channel_id` value, or (b) a file. Then in case of (a) it re-processes the +// entire message recursively calling itself in stream mode. In case of (b) it +// calls to uploadFileMultipartLegacy for legacy support +func uploadFileMultipart(c *Context, r *http.Request, asStream io.Reader, timestamp time.Time) *model.FileUploadResponse { + + expectClientIds := true + var clientIds []string + resp := model.FileUploadResponse{ + FileInfos: []*model.FileInfo{}, + ClientIds: []string{}, + } + + var buf *bytes.Buffer + var mr *multipart.Reader + var err error + if asStream == nil { + // We need to buffer until we get the channel_id, or the first file. + buf = &bytes.Buffer{} + mr, err = multipartReader(r, io.TeeReader(r.Body, buf)) + } else { + mr, err = multipartReader(r, asStream) + } + if err != nil { + c.Err = model.NewAppError("uploadFileMultipart", + "api.file.upload_file.read_request.app_error", + nil, err.Error(), http.StatusBadRequest) + return nil + } + + nFiles := 0 +NEXT_PART: + for { + part, err := mr.NextPart() + if err == io.EOF { + break + } + if err != nil { + c.Err = model.NewAppError("uploadFileMultipart", + "api.file.upload_file.read_request.app_error", + nil, err.Error(), http.StatusBadRequest) + return nil + } + + // Parse any form fields in the multipart. + formname := part.FormName() + if formname == "" { + continue + } + filename := part.FileName() + if filename == "" { + var b bytes.Buffer + _, err := io.CopyN(&b, part, maxMultipartFormDataBytes) + if err != nil && err != io.EOF { + c.Err = model.NewAppError("uploadFileMultipart", + "api.file.upload_file.read_form_value.app_error", + map[string]interface{}{"Formname": formname}, + err.Error(), http.StatusBadRequest) + return nil + } + v := b.String() + + switch formname { + case "channel_id": + if c.Params.ChannelId != "" && c.Params.ChannelId != v { + c.Err = model.NewAppError("uploadFileMultipart", + "api.file.upload_file.multiple_channel_ids.app_error", + nil, "", http.StatusBadRequest) + return nil + } + if v != "" { + c.Params.ChannelId = v + } + + // Got channel_id, re-process the entire post + // in the streaming mode. + if asStream == nil { + return uploadFileMultipart(c, r, io.MultiReader(buf, r.Body), timestamp) + } + + case "client_ids": + if !expectClientIds { + c.SetInvalidParam("client_ids") + return nil + } + clientIds = append(clientIds, v) + + default: + c.SetInvalidParam(formname) + return nil + } + + continue NEXT_PART + } + + // A file part. + + if c.Params.ChannelId == "" && asStream == nil { + // Got file before channel_id, fall back to legacy buffered mode + mr, err = multipartReader(r, io.MultiReader(buf, r.Body)) + if err != nil { + c.Err = model.NewAppError("uploadFileMultipart", + "api.file.upload_file.read_request.app_error", + nil, err.Error(), http.StatusBadRequest) + return nil + } + + return uploadFileMultipartLegacy(c, mr, timestamp) + } + + c.RequireChannelId() + if c.Err != nil { + return nil + } + if !c.App.SessionHasPermissionToChannel(c.App.Session, c.Params.ChannelId, model.PERMISSION_UPLOAD_FILE) { + c.SetPermissionError(model.PERMISSION_UPLOAD_FILE) + return nil + } + + // If there's no clientIds when the first file comes, expect + // none later. + if nFiles == 0 && len(clientIds) == 0 { + expectClientIds = false + } + + // Must have a exactly one client ID for each file. + clientId := "" + if expectClientIds { + if nFiles >= len(clientIds) { + c.SetInvalidParam("client_ids") + return nil + } + + clientId = clientIds[nFiles] + } + + info, appErr := c.App.UploadFileX(c.Params.ChannelId, filename, part, + app.UploadFileSetTeamId(FILE_TEAM_ID), + app.UploadFileSetUserId(c.App.Session.UserId), + app.UploadFileSetTimestamp(timestamp), + app.UploadFileSetContentLength(-1), + app.UploadFileSetClientId(clientId)) + if appErr != nil { + c.Err = appErr + return nil + } + + // add to the response + resp.FileInfos = append(resp.FileInfos, info) + if expectClientIds { + resp.ClientIds = append(resp.ClientIds, clientId) + } + + nFiles++ + } + + // Verify that the number of ClientIds matched the number of files. + if expectClientIds && len(clientIds) != nFiles { + c.Err = model.NewAppError("uploadFileMultipart", + "api.file.upload_file.incorrect_number_of_client_ids.app_error", + map[string]interface{}{"NumClientIds": len(clientIds), "NumFiles": nFiles}, + "", http.StatusBadRequest) + return nil + } + + return &resp +} + +// uploadFileMultipartLegacy reads, buffers, and then uploads the message, +// borrowing from http.ParseMultipartForm. If successful it returns a +// *model.FileUploadResponse filled in with the individual model.FileInfo's. +func uploadFileMultipartLegacy(c *Context, mr *multipart.Reader, + timestamp time.Time) *model.FileUploadResponse { + + // Parse the entire form. + form, err := mr.ReadForm(*c.App.Config().FileSettings.MaxFileSize) + if err != nil { + c.Err = model.NewAppError("uploadFileMultipartLegacy", + "api.file.upload_file.read_request.app_error", + nil, err.Error(), http.StatusInternalServerError) + return nil + } + + // get and validate the channel Id, permission to upload there. + if len(form.Value["channel_id"]) == 0 { + c.SetInvalidParam("channel_id") + return nil + } + channelId := form.Value["channel_id"][0] + c.Params.ChannelId = channelId + c.RequireChannelId() + if c.Err != nil { + return nil + } + if !c.App.SessionHasPermissionToChannel(c.App.Session, channelId, model.PERMISSION_UPLOAD_FILE) { + c.SetPermissionError(model.PERMISSION_UPLOAD_FILE) + return nil + } + + // Check that we have either no client IDs, or one per file. + clientIds := form.Value["client_ids"] + fileHeaders := form.File["files"] + if len(clientIds) != 0 && len(clientIds) != len(fileHeaders) { + c.Err = model.NewAppError("uploadFilesMultipartBuffered", + "api.file.upload_file.incorrect_number_of_client_ids.app_error", + map[string]interface{}{"NumClientIds": len(clientIds), "NumFiles": len(fileHeaders)}, + "", http.StatusBadRequest) + return nil + } + + resp := model.FileUploadResponse{ + FileInfos: []*model.FileInfo{}, + ClientIds: []string{}, + } + + for i, fileHeader := range fileHeaders { + f, err := fileHeader.Open() + if err != nil { + c.Err = model.NewAppError("uploadFileMultipartLegacy", + "api.file.upload_file.read_request.app_error", + nil, err.Error(), http.StatusBadRequest) + return nil + } + + clientId := "" + if len(clientIds) > 0 { + clientId = clientIds[i] + } + + info, appErr := c.App.UploadFileX(c.Params.ChannelId, fileHeader.Filename, f, + app.UploadFileSetTeamId(FILE_TEAM_ID), + app.UploadFileSetUserId(c.App.Session.UserId), + app.UploadFileSetTimestamp(timestamp), + app.UploadFileSetContentLength(-1), + app.UploadFileSetClientId(clientId)) + f.Close() + if appErr != nil { + c.Err = appErr + return nil + } + + resp.FileInfos = append(resp.FileInfos, info) + if clientId != "" { + resp.ClientIds = append(resp.ClientIds, clientId) + } + } + + return &resp +} + func getFile(c *Context, w http.ResponseWriter, r *http.Request) { c.RequireFileId() if c.Err != nil { diff --git a/api4/file_test.go b/api4/file_test.go index 61483563dd..f82ba92935 100644 --- a/api4/file_test.go +++ b/api4/file_test.go @@ -4,250 +4,752 @@ package api4 import ( + "bytes" "fmt" + "io" + "io/ioutil" + "mime/multipart" "net/http" + "net/textproto" + "net/url" + "os" + "path/filepath" "strings" "testing" "time" + "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" + "github.com/mattermost/mattermost-server/utils" "github.com/mattermost/mattermost-server/utils/testutils" ) -func TestUploadFileAsMultipart(t *testing.T) { - th := Setup().InitBasic() - defer th.TearDown() - Client := th.Client +var testDir = "" - user := th.BasicUser - channel := th.BasicChannel +func init() { + testDir, _ = utils.FindDir("tests") +} - var uploadInfo *model.FileInfo - var data []byte - var err error - if data, err = testutils.ReadTestFile("test.png"); err != nil { - t.Fatal(err) - } else if fileResp, resp := Client.UploadFile(data, channel.Id, "test.png"); resp.Error != nil { - t.Fatal(resp.Error) - } else if len(fileResp.FileInfos) != 1 { - t.Fatal("should've returned a single file infos") - } else { - uploadInfo = fileResp.FileInfos[0] - } - - // The returned file info from the upload call will be missing some fields that will be stored in the database - if uploadInfo.CreatorId != user.Id { - t.Fatal("file should be assigned to user") - } else if uploadInfo.PostId != "" { - t.Fatal("file shouldn't have a post") - } else if uploadInfo.Path != "" { - t.Fatal("file path should not be set on returned info") - } else if uploadInfo.ThumbnailPath != "" { - t.Fatal("file thumbnail path should not be set on returned info") - } else if uploadInfo.PreviewPath != "" { - t.Fatal("file preview path should not be set on returned info") - } - - var info *model.FileInfo - if result := <-th.App.Srv.Store.FileInfo().Get(uploadInfo.Id); result.Err != nil { - t.Fatal(result.Err) - } else { - info = result.Data.(*model.FileInfo) - } - - if info.Id != uploadInfo.Id { - t.Fatal("file id from response should match one stored in database") - } else if info.CreatorId != user.Id { - t.Fatal("file should be assigned to user") - } else if info.PostId != "" { - t.Fatal("file shouldn't have a post") - } else if info.Path == "" { - t.Fatal("file path should be set in database") - } else if info.ThumbnailPath == "" { - t.Fatal("file thumbnail path should be set in database") - } else if info.PreviewPath == "" { - t.Fatal("file preview path should be set in database") - } - - date := time.Now().Format("20060102") - - // This also makes sure that the relative path provided above is sanitized out - expectedPath := fmt.Sprintf("%v/teams/%v/channels/%v/users/%v/%v/test.png", date, FILE_TEAM_ID, channel.Id, user.Id, info.Id) - if info.Path != expectedPath { - t.Logf("file is saved in %v", info.Path) - t.Fatalf("file should've been saved in %v", expectedPath) - } - - expectedThumbnailPath := fmt.Sprintf("%v/teams/%v/channels/%v/users/%v/%v/test_thumb.jpg", date, FILE_TEAM_ID, channel.Id, user.Id, info.Id) - if info.ThumbnailPath != expectedThumbnailPath { - t.Logf("file thumbnail is saved in %v", info.ThumbnailPath) - t.Fatalf("file thumbnail should've been saved in %v", expectedThumbnailPath) - } - - expectedPreviewPath := fmt.Sprintf("%v/teams/%v/channels/%v/users/%v/%v/test_preview.jpg", date, FILE_TEAM_ID, channel.Id, user.Id, info.Id) - if info.PreviewPath != expectedPreviewPath { - t.Logf("file preview is saved in %v", info.PreviewPath) - t.Fatalf("file preview should've been saved in %v", expectedPreviewPath) - } - - // Wait a bit for files to ready - time.Sleep(2 * time.Second) - - if err := th.cleanupTestFile(info); err != nil { - t.Fatal(err) - } - - _, resp := Client.UploadFile(data, model.NewId(), "test.png") - CheckForbiddenStatus(t, resp) - - _, resp = Client.UploadFile(data, "../../junk", "test.png") - CheckForbiddenStatus(t, resp) - - _, resp = th.SystemAdminClient.UploadFile(data, model.NewId(), "test.png") - CheckForbiddenStatus(t, resp) - - _, resp = th.SystemAdminClient.UploadFile(data, "../../junk", "test.png") - CheckForbiddenStatus(t, resp) - - _, resp = th.SystemAdminClient.UploadFile(data, channel.Id, "test.png") - CheckNoError(t, resp) - - enableFileAttachments := *th.App.Config().FileSettings.EnableFileAttachments - defer func() { - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnableFileAttachments = enableFileAttachments }) - }() - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnableFileAttachments = false }) - - _, resp = th.SystemAdminClient.UploadFile(data, channel.Id, "test.png") - if resp.StatusCode == 0 { - t.Log("file upload request failed completely") - } else if resp.StatusCode != http.StatusNotImplemented { - // This should return an HTTP 501, but it occasionally causes the http client itself to error - t.Fatalf("should've returned HTTP 501 or failed completely, got %v instead", resp.StatusCode) +func checkCond(tb testing.TB, cond bool, text string) { + if !cond { + tb.Error(text) } } -func TestUploadFileAsRequestBody(t *testing.T) { +func checkEq(tb testing.TB, v1, v2 interface{}, text string) { + checkCond(tb, fmt.Sprintf("%+v", v1) == fmt.Sprintf("%+v", v2), text) +} + +func checkNeq(tb testing.TB, v1, v2 interface{}, text string) { + checkCond(tb, fmt.Sprintf("%+v", v1) != fmt.Sprintf("%+v", v2), text) +} + +type zeroReader struct { + limit, read int +} + +func (z *zeroReader) Read(b []byte) (int, error) { + for i := range b { + if z.read == z.limit { + return i, io.EOF + } + b[i] = 0 + z.read++ + } + + return len(b), nil +} + +// File Section +var quoteEscaper = strings.NewReplacer("\\", "\\\\", `"`, "\\\"") + +func escapeQuotes(s string) string { + return quoteEscaper.Replace(s) +} + +type UploadOpener func() (io.ReadCloser, int64, error) + +func NewUploadOpenerReader(in io.Reader) UploadOpener { + return func() (io.ReadCloser, int64, error) { + rc, ok := in.(io.ReadCloser) + if ok { + return rc, -1, nil + } else { + return ioutil.NopCloser(in), -1, nil + } + } +} + +func NewUploadOpenerFile(path string) UploadOpener { + return func() (io.ReadCloser, int64, error) { + fi, err := os.Stat(path) + if err != nil { + return nil, 0, err + } + f, err := os.Open(path) + if err != nil { + return nil, 0, err + } + return f, fi.Size(), nil + } +} + +// testUploadFile and testUploadFiles have been "staged" here, eventually they +// should move back to being model.Client4 methods, once the specifics of the +// public API are sorted out. +func testUploadFile(c *model.Client4, url string, body io.Reader, contentType string, + contentLength int64) (*model.FileUploadResponse, *model.Response) { + rq, _ := http.NewRequest("POST", c.ApiUrl+c.GetFilesRoute()+url, body) + if contentLength != 0 { + rq.ContentLength = contentLength + } + rq.Header.Set("Content-Type", contentType) + + if len(c.AuthToken) > 0 { + rq.Header.Set(model.HEADER_AUTH, c.AuthType+" "+c.AuthToken) + } + + rp, err := c.HttpClient.Do(rq) + if err != nil || rp == nil { + return nil, model.BuildErrorResponse(rp, model.NewAppError(url, "model.client.connecting.app_error", nil, err.Error(), 0)) + } + defer closeBody(rp) + + if rp.StatusCode >= 300 { + return nil, model.BuildErrorResponse(rp, model.AppErrorFromJson(rp.Body)) + } + + return model.FileUploadResponseFromJson(rp.Body), model.BuildResponse(rp) +} + +func testUploadFiles( + c *model.Client4, + channelId string, + names []string, + openers []UploadOpener, + contentLengths []int64, + clientIds []string, + useMultipart, + useChunkedInSimplePost bool, +) ( + fileUploadResponse *model.FileUploadResponse, + response *model.Response, +) { + // Do not check len(clientIds), leave it entirely to the user to + // provide. The server will error out if it does not match the number + // of files, but it's not critical here. + if len(names) == 0 || len(openers) == 0 || len(names) != len(openers) { + return nil, &model.Response{ + Error: model.NewAppError("testUploadFiles", + "model.client.upload_post_attachment.file.app_error", + nil, "Empty or mismatched file data", http.StatusBadRequest), + } + } + + // emergencyResponse is a convenience wrapper to return an error response + emergencyResponse := func(err error, errCode string) *model.Response { + return &model.Response{ + Error: model.NewAppError("testUploadFiles", + "model.client."+errCode+".app_error", + nil, err.Error(), http.StatusBadRequest), + } + } + + // For multipart, start writing the request as a goroutine, and pipe + // multipart.Writer into it, otherwise generate a new request each + // time. + pipeReader, pipeWriter := io.Pipe() + mw := multipart.NewWriter(pipeWriter) + + if useMultipart { + fileUploadResponseChannel := make(chan *model.FileUploadResponse) + responseChannel := make(chan *model.Response) + closedMultipart := false + + go func() { + fur, resp := testUploadFile(c, "", pipeReader, mw.FormDataContentType(), -1) + responseChannel <- resp + fileUploadResponseChannel <- fur + }() + + defer func() { + for { + select { + // Premature response, before the entire + // multipart was sent + case response = <-responseChannel: + // Guaranteed to be there + fileUploadResponse = <-fileUploadResponseChannel + if !closedMultipart { + _ = mw.Close() + _ = pipeWriter.Close() + closedMultipart = true + } + return + + // Normal response, after the multipart was sent. + default: + if !closedMultipart { + err := mw.Close() + if err != nil { + fileUploadResponse = nil + response = emergencyResponse(err, "upload_post_attachment.writer") + return + } + err = pipeWriter.Close() + if err != nil { + fileUploadResponse = nil + response = emergencyResponse(err, "upload_post_attachment.writer") + return + } + closedMultipart = true + } + } + } + }() + + err := mw.WriteField("channel_id", channelId) + if err != nil { + return nil, emergencyResponse(err, "upload_post_attachment.channel_id") + } + } else { + fileUploadResponse = &model.FileUploadResponse{} + } + + var f io.ReadCloser + var cl int64 + var err error + data := make([]byte, 512) + + upload := func(i int, f io.ReadCloser) *model.Response { + defer f.Close() + + if len(contentLengths) > i { + cl = contentLengths[i] + } + + n, err := f.Read(data) + if err != nil && err != io.EOF { + return emergencyResponse(err, "upload_post_attachment") + } + ct := http.DetectContentType(data[:n]) + reader := io.MultiReader(bytes.NewReader(data[:n]), f) + + if useMultipart { + if len(clientIds) > i { + err := mw.WriteField("client_ids", clientIds[i]) + if err != nil { + return emergencyResponse(err, "upload_post_attachment.file") + } + } + + h := make(textproto.MIMEHeader) + h.Set("Content-Disposition", + fmt.Sprintf(`form-data; name="files"; filename="%s"`, escapeQuotes(names[i]))) + h.Set("Content-Type", ct) + + // If we error here, writing to mw, the deferred handler + part, err := mw.CreatePart(h) + if err != nil { + return emergencyResponse(err, "upload_post_attachment.writer") + } + + _, err = io.Copy(part, reader) + if err != nil { + return emergencyResponse(err, "upload_post_attachment.writer") + } + } else { + postURL := fmt.Sprintf("?channel_id=%v", url.QueryEscape(channelId)) + + fmt.Sprintf("&filename=%v", url.QueryEscape(names[i])) + if len(clientIds) > i { + postURL += fmt.Sprintf("&client_id=%v", url.QueryEscape(clientIds[i])) + } + if useChunkedInSimplePost { + cl = -1 + } + fur, resp := testUploadFile(c, postURL, reader, ct, cl) + if resp.Error != nil { + return resp + } + fileUploadResponse.FileInfos = append(fileUploadResponse.FileInfos, fur.FileInfos[0]) + if len(clientIds) > 0 { + if len(fur.ClientIds) > 0 { + fileUploadResponse.ClientIds = append(fileUploadResponse.ClientIds, fur.ClientIds[0]) + } else { + fileUploadResponse.ClientIds = append(fileUploadResponse.ClientIds, "") + } + } + response = resp + } + + return nil + } + + for i, open := range openers { + f, cl, err = open() + if err != nil { + return nil, emergencyResponse(err, "upload_post_attachment") + } + + resp := upload(i, f) + if resp != nil && resp.Error != nil { + return nil, resp + } + } + + // In case of a simple POST, the return values have been set by upload(), + // otherwise we finished writing the multipart, and the return values will + // be set in defer + return fileUploadResponse, response +} + +func TestUploadFiles(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - Client := th.Client + if *th.App.Config().FileSettings.DriverName == "" { + t.Skip("skipping because no file driver is enabled") + } - user := th.BasicUser channel := th.BasicChannel - - var uploadInfo *model.FileInfo - var data []byte - var err error - if data, err = testutils.ReadTestFile("test.png"); err != nil { - t.Fatal(err) - } else if fileResp, resp := Client.UploadFileAsRequestBody(data, channel.Id, "test.png"); resp.Error != nil { - t.Fatal(resp.Error) - } else if len(fileResp.FileInfos) != 1 { - t.Fatal("should've returned a single file infos") - } else { - uploadInfo = fileResp.FileInfos[0] - } - - // The returned file info from the upload call will be missing some fields that will be stored in the database - if uploadInfo.CreatorId != user.Id { - t.Fatal("file should be assigned to user") - } else if uploadInfo.PostId != "" { - t.Fatal("file shouldn't have a post") - } else if uploadInfo.Path != "" { - t.Fatal("file path should not be set on returned info") - } else if uploadInfo.ThumbnailPath != "" { - t.Fatal("file thumbnail path should not be set on returned info") - } else if uploadInfo.PreviewPath != "" { - t.Fatal("file preview path should not be set on returned info") - } - - var info *model.FileInfo - if result := <-th.App.Srv.Store.FileInfo().Get(uploadInfo.Id); result.Err != nil { - t.Fatal(result.Err) - } else { - info = result.Data.(*model.FileInfo) - } - - if info.Id != uploadInfo.Id { - t.Fatal("file id from response should match one stored in database") - } else if info.CreatorId != user.Id { - t.Fatal("file should be assigned to user") - } else if info.PostId != "" { - t.Fatal("file shouldn't have a post") - } else if info.Path == "" { - t.Fatal("file path should be set in database") - } else if info.ThumbnailPath == "" { - t.Fatal("file thumbnail path should be set in database") - } else if info.PreviewPath == "" { - t.Fatal("file preview path should be set in database") - } - date := time.Now().Format("20060102") - // This also makes sure that the relative path provided above is sanitized out - expectedPath := fmt.Sprintf("%v/teams/%v/channels/%v/users/%v/%v/test.png", date, FILE_TEAM_ID, channel.Id, user.Id, info.Id) - if info.Path != expectedPath { - t.Logf("file is saved in %v", info.Path) - t.Fatalf("file should've been saved in %v", expectedPath) + // Get better error messages + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableDeveloper = true }) + + op := func(name string) UploadOpener { + return NewUploadOpenerFile(filepath.Join(testDir, name)) } - expectedThumbnailPath := fmt.Sprintf("%v/teams/%v/channels/%v/users/%v/%v/test_thumb.jpg", date, FILE_TEAM_ID, channel.Id, user.Id, info.Id) - if info.ThumbnailPath != expectedThumbnailPath { - t.Logf("file thumbnail is saved in %v", info.ThumbnailPath) - t.Fatalf("file thumbnail should've been saved in %v", expectedThumbnailPath) + tests := []struct { + title string + client *model.Client4 + openers []UploadOpener + names []string + clientIds []string + + skipSuccessValidation bool + skipPayloadValidation bool + skipSimplePost bool + skipMultipart bool + channelId string + useChunkedInSimplePost bool + expectedCreatorId string + expectedPayloadNames []string + expectImage bool + expectedImageWidths []int + expectedImageHeights []int + expectedImageThumbnailNames []string + expectedImagePreviewNames []string + expectedImageHasPreview []bool + setupConfig func(a *app.App) func(a *app.App) + checkResponse func(t *testing.T, resp *model.Response) + }{ + // Upload a bunch of files, mixed images and non-images + { + title: "Happy", + names: []string{"test.png", "testgif.gif", "testplugin.tar.gz", "test-search.md"}, + expectedCreatorId: th.BasicUser.Id, + }, + // Upload a bunch of files, with clientIds + { + title: "Happy client_ids", + names: []string{"test.png", "testgif.gif", "testplugin.tar.gz", "test-search.md"}, + clientIds: []string{"1", "2", "3", "4"}, + expectedCreatorId: th.BasicUser.Id, + }, + // Upload a bunch of images. testgif.gif is an animated GIF, + // so it does not have HasPreviewImage set. + { + title: "Happy images", + names: []string{"test.png", "testgif.gif"}, + expectImage: true, + expectedCreatorId: th.BasicUser.Id, + expectedImageWidths: []int{408, 118}, + expectedImageHeights: []int{336, 118}, + expectedImageHasPreview: []bool{true, false}, + }, + { + title: "Happy invalid image", + names: []string{"testgif.gif"}, + openers: []UploadOpener{NewUploadOpenerFile(filepath.Join(testDir, "test-search.md"))}, + skipPayloadValidation: true, + expectedCreatorId: th.BasicUser.Id, + }, + // Simple POST, chunked encoding + { + title: "Happy image chunked post", + skipMultipart: true, + useChunkedInSimplePost: true, + names: []string{"test.png"}, + expectImage: true, + expectedImageWidths: []int{408}, + expectedImageHeights: []int{336}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + }, + // Image thumbnail and preview: size and orientation. Note that + // the expected image dimensions remain the same regardless of the + // orientation - what we save in FileInfo is used by the + // clients to size UI elements, so the dimensions are "actual". + { + title: "Happy image thumbnail/preview 1", + names: []string{"orientation_test_1.jpeg"}, + expectedImageThumbnailNames: []string{"orientation_test_1_expected_thumb.jpeg"}, + expectedImagePreviewNames: []string{"orientation_test_1_expected_preview.jpeg"}, + expectImage: true, + expectedImageWidths: []int{2860}, + expectedImageHeights: []int{1578}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + }, + { + title: "Happy image thumbnail/preview 2", + names: []string{"orientation_test_2.jpeg"}, + expectedImageThumbnailNames: []string{"orientation_test_2_expected_thumb.jpeg"}, + expectedImagePreviewNames: []string{"orientation_test_2_expected_preview.jpeg"}, + expectImage: true, + expectedImageWidths: []int{2860}, + expectedImageHeights: []int{1578}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + }, + { + title: "Happy image thumbnail/preview 3", + names: []string{"orientation_test_3.jpeg"}, + expectedImageThumbnailNames: []string{"orientation_test_3_expected_thumb.jpeg"}, + expectedImagePreviewNames: []string{"orientation_test_3_expected_preview.jpeg"}, + expectImage: true, + expectedImageWidths: []int{2860}, + expectedImageHeights: []int{1578}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + }, + { + title: "Happy image thumbnail/preview 4", + names: []string{"orientation_test_4.jpeg"}, + expectedImageThumbnailNames: []string{"orientation_test_4_expected_thumb.jpeg"}, + expectedImagePreviewNames: []string{"orientation_test_4_expected_preview.jpeg"}, + expectImage: true, + expectedImageWidths: []int{2860}, + expectedImageHeights: []int{1578}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + }, + { + title: "Happy image thumbnail/preview 5", + names: []string{"orientation_test_5.jpeg"}, + expectedImageThumbnailNames: []string{"orientation_test_5_expected_thumb.jpeg"}, + expectedImagePreviewNames: []string{"orientation_test_5_expected_preview.jpeg"}, + expectImage: true, + expectedImageWidths: []int{2860}, + expectedImageHeights: []int{1578}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + }, + { + title: "Happy image thumbnail/preview 6", + names: []string{"orientation_test_6.jpeg"}, + expectedImageThumbnailNames: []string{"orientation_test_6_expected_thumb.jpeg"}, + expectedImagePreviewNames: []string{"orientation_test_6_expected_preview.jpeg"}, + expectImage: true, + expectedImageWidths: []int{2860}, + expectedImageHeights: []int{1578}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + }, + { + title: "Happy image thumbnail/preview 7", + names: []string{"orientation_test_7.jpeg"}, + expectedImageThumbnailNames: []string{"orientation_test_7_expected_thumb.jpeg"}, + expectedImagePreviewNames: []string{"orientation_test_7_expected_preview.jpeg"}, + expectImage: true, + expectedImageWidths: []int{2860}, + expectedImageHeights: []int{1578}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + }, + { + title: "Happy image thumbnail/preview 8", + names: []string{"orientation_test_8.jpeg"}, + expectedImageThumbnailNames: []string{"orientation_test_8_expected_thumb.jpeg"}, + expectedImagePreviewNames: []string{"orientation_test_8_expected_preview.jpeg"}, + expectImage: true, + expectedImageWidths: []int{2860}, + expectedImageHeights: []int{1578}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + }, + { + title: "Happy admin", + client: th.SystemAdminClient, + names: []string{"test.png"}, + expectedCreatorId: th.SystemAdminUser.Id, + }, + { + title: "Happy stream", + useChunkedInSimplePost: true, + skipPayloadValidation: true, + names: []string{"50Mb-stream"}, + openers: []UploadOpener{NewUploadOpenerReader(&zeroReader{limit: 50 * 1024 * 1024})}, + setupConfig: func(a *app.App) func(a *app.App) { + maxFileSize := *a.Config().FileSettings.MaxFileSize + a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.MaxFileSize = 50 * 1024 * 1024 }) + return func(a *app.App) { + a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.MaxFileSize = maxFileSize }) + } + }, + expectedCreatorId: th.BasicUser.Id, + }, + // Error cases + { + title: "Error channel_id does not exist", + channelId: model.NewId(), + names: []string{"test.png"}, + skipSuccessValidation: true, + checkResponse: CheckForbiddenStatus, + }, + { + // on simple post this uploads the last file + // successfully, without a ClientId + title: "Error too few client_ids", + skipSimplePost: true, + names: []string{"test.png", "testplugin.tar.gz", "test-search.md"}, + clientIds: []string{"1", "4"}, + skipSuccessValidation: true, + checkResponse: CheckBadRequestStatus, + }, + { + title: "Error invalid channel_id", + channelId: "../../junk", + names: []string{"test.png"}, + skipSuccessValidation: true, + checkResponse: CheckBadRequestStatus, + }, + { + title: "Error admin channel_id does not exist", + client: th.SystemAdminClient, + channelId: model.NewId(), + names: []string{"test.png"}, + skipSuccessValidation: true, + checkResponse: CheckForbiddenStatus, + }, + { + title: "Error admin invalid channel_id", + client: th.SystemAdminClient, + channelId: "../../junk", + names: []string{"test.png"}, + skipSuccessValidation: true, + checkResponse: CheckBadRequestStatus, + }, + { + title: "Error admin disabled uploads", + client: th.SystemAdminClient, + names: []string{"test.png"}, + skipSuccessValidation: true, + checkResponse: CheckNotImplementedStatus, + setupConfig: func(a *app.App) func(a *app.App) { + enableFileAttachments := *a.Config().FileSettings.EnableFileAttachments + a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnableFileAttachments = false }) + return func(a *app.App) { + a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnableFileAttachments = enableFileAttachments }) + } + }, + }, + { + title: "Error file too large", + names: []string{"test.png"}, + skipSuccessValidation: true, + checkResponse: CheckRequestEntityTooLargeStatus, + setupConfig: func(a *app.App) func(a *app.App) { + maxFileSize := *a.Config().FileSettings.MaxFileSize + a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.MaxFileSize = 279590 }) + return func(a *app.App) { + a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.MaxFileSize = maxFileSize }) + } + }, + }, + // File too large (chunked, simple POST only, multipart would've been redundant with above) + { + title: "File too large chunked", + useChunkedInSimplePost: true, + skipMultipart: true, + names: []string{"test.png"}, + skipSuccessValidation: true, + checkResponse: CheckRequestEntityTooLargeStatus, + setupConfig: func(a *app.App) func(a *app.App) { + maxFileSize := *a.Config().FileSettings.MaxFileSize + a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.MaxFileSize = 279590 }) + return func(a *app.App) { + a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.MaxFileSize = maxFileSize }) + } + }, + }, + { + title: "Error stream too large", + skipPayloadValidation: true, + names: []string{"50Mb-stream"}, + openers: []UploadOpener{NewUploadOpenerReader(&zeroReader{limit: 50 * 1024 * 1024})}, + skipSuccessValidation: true, + checkResponse: CheckRequestEntityTooLargeStatus, + setupConfig: func(a *app.App) func(a *app.App) { + maxFileSize := *a.Config().FileSettings.MaxFileSize + a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.MaxFileSize = 100 * 1024 }) + return func(a *app.App) { + a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.MaxFileSize = maxFileSize }) + } + }, + }, } - expectedPreviewPath := fmt.Sprintf("%v/teams/%v/channels/%v/users/%v/%v/test_preview.jpg", date, FILE_TEAM_ID, channel.Id, user.Id, info.Id) - if info.PreviewPath != expectedPreviewPath { - t.Logf("file preview is saved in %v", info.PreviewPath) - t.Fatalf("file preview should've been saved in %v", expectedPreviewPath) - } + for _, useMultipart := range []bool{true, false} { + for _, tc := range tests { + if tc.skipMultipart && useMultipart || tc.skipSimplePost && !useMultipart { + continue + } - // Wait a bit for files to ready - time.Sleep(2 * time.Second) + // Set the default values and title + client := th.Client + if tc.client != nil { + client = tc.client + } + channelId := channel.Id + if tc.channelId != "" { + channelId = tc.channelId + } + if tc.checkResponse == nil { + tc.checkResponse = CheckNoError + } - if err := th.cleanupTestFile(info); err != nil { - t.Fatal(err) - } + title := "" + if useMultipart { + title = "multipart " + } else { + title = "simple " + } + if tc.title != "" { + title += tc.title + " " + } + title += fmt.Sprintf("%v", tc.names) - _, resp := Client.UploadFileAsRequestBody(data, model.NewId(), "test.png") - CheckForbiddenStatus(t, resp) + // Apply any necessary config changes + var restoreConfig func(a *app.App) + if tc.setupConfig != nil { + restoreConfig = tc.setupConfig(th.App) + } - _, resp = Client.UploadFileAsRequestBody(data, "../../junk", "test.png") - if resp.StatusCode == 0 { - t.Log("file upload request failed completely") - } else if resp.StatusCode != http.StatusBadRequest { - // This should return an HTTP 400, but it occasionally causes the http client itself to error - t.Fatalf("should've returned HTTP 400 or failed completely, got %v instead", resp.StatusCode) - } + t.Run(title, func(t *testing.T) { + if len(tc.openers) == 0 { + for _, name := range tc.names { + tc.openers = append(tc.openers, op(name)) + } + } + fileResp, resp := testUploadFiles(client, channelId, tc.names, + tc.openers, nil, tc.clientIds, useMultipart, + tc.useChunkedInSimplePost) + tc.checkResponse(t, resp) + if tc.skipSuccessValidation { + return + } - _, resp = th.SystemAdminClient.UploadFileAsRequestBody(data, model.NewId(), "test.png") - CheckForbiddenStatus(t, resp) + if fileResp == nil || len(fileResp.FileInfos) == 0 || len(fileResp.FileInfos) != len(tc.names) { + t.Fatal("Empty or mismatched actual or expected FileInfos") + } - _, resp = th.SystemAdminClient.UploadFileAsRequestBody(data, "../../junk", "test.png") - if resp.StatusCode == 0 { - t.Log("file upload request failed completely") - } else if resp.StatusCode != http.StatusBadRequest { - // This should return an HTTP 400, but it occasionally causes the http client itself to error - t.Fatalf("should've returned HTTP 400 or failed completely, got %v instead", resp.StatusCode) - } + for i, ri := range fileResp.FileInfos { + // The returned file info from the upload call will be missing some fields that will be stored in the database + checkEq(t, ri.CreatorId, tc.expectedCreatorId, "File should be assigned to user") + checkEq(t, ri.PostId, "", "File shouldn't have a post Id") + checkEq(t, ri.Path, "", "File path should not be set on returned info") + checkEq(t, ri.ThumbnailPath, "", "File thumbnail path should not be set on returned info") + checkEq(t, ri.PreviewPath, "", "File preview path should not be set on returned info") + if len(tc.clientIds) > i { + checkCond(t, len(fileResp.ClientIds) == len(tc.clientIds), + fmt.Sprintf("Wrong number of clientIds returned, expected %v, got %v", len(tc.clientIds), len(fileResp.ClientIds))) + checkEq(t, fileResp.ClientIds[i], tc.clientIds[i], + fmt.Sprintf("Wrong clientId returned, expected %v, got %v", tc.clientIds[i], fileResp.ClientIds[i])) + } - _, resp = th.SystemAdminClient.UploadFileAsRequestBody(data, channel.Id, "test.png") - CheckNoError(t, resp) + var dbInfo *model.FileInfo + result := <-th.App.Srv.Store.FileInfo().Get(ri.Id) + if result.Err != nil { + t.Error(result.Err) + } else { + dbInfo = result.Data.(*model.FileInfo) + } + checkEq(t, dbInfo.Id, ri.Id, "File id from response should match one stored in database") + checkEq(t, dbInfo.CreatorId, tc.expectedCreatorId, "F ile should be assigned to user") + checkEq(t, dbInfo.PostId, "", "File shouldn't have a post") + checkNeq(t, dbInfo.Path, "", "File path should be set in database") + _, fname := filepath.Split(dbInfo.Path) + ext := filepath.Ext(fname) + name := fname[:len(fname)-len(ext)] + expectedDir := fmt.Sprintf("%v/teams/%v/channels/%v/users/%s/%s", date, FILE_TEAM_ID, channel.Id, ri.CreatorId, ri.Id) + expectedPath := fmt.Sprintf("%s/%s", expectedDir, fname) + checkEq(t, dbInfo.Path, expectedPath, + fmt.Sprintf("File %v saved to:%q, expected:%q", dbInfo.Name, dbInfo.Path, expectedPath)) - enableFileAttachments := *th.App.Config().FileSettings.EnableFileAttachments - defer func() { - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnableFileAttachments = enableFileAttachments }) - }() - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnableFileAttachments = false }) + if tc.expectImage { + expectedThumbnailPath := fmt.Sprintf("%s/%s_thumb.jpg", expectedDir, name) + expectedPreviewPath := fmt.Sprintf("%s/%s_preview.jpg", expectedDir, name) + checkEq(t, dbInfo.ThumbnailPath, expectedThumbnailPath, + fmt.Sprintf("Thumbnail for %v saved to:%q, expected:%q", dbInfo.Name, dbInfo.ThumbnailPath, expectedThumbnailPath)) + checkEq(t, dbInfo.PreviewPath, expectedPreviewPath, + fmt.Sprintf("Preview for %v saved to:%q, expected:%q", dbInfo.Name, dbInfo.PreviewPath, expectedPreviewPath)) - _, resp = th.SystemAdminClient.UploadFileAsRequestBody(data, channel.Id, "test.png") - if resp.StatusCode == 0 { - t.Log("file upload request failed completely") - } else if resp.StatusCode != http.StatusNotImplemented { - // This should return an HTTP 501, but it occasionally causes the http client itself to error - t.Fatalf("should've returned HTTP 501 or failed completely, got %v instead", resp.StatusCode) + checkCond(t, + dbInfo.HasPreviewImage == tc.expectedImageHasPreview[i], + fmt.Sprintf("Image: HasPreviewImage should be set for %s", dbInfo.Name)) + checkCond(t, + dbInfo.Width == tc.expectedImageWidths[i] && dbInfo.Height == tc.expectedImageHeights[i], + fmt.Sprintf("Image dimensions: expected %dwx%dh, got %vwx%dh", + tc.expectedImageWidths[i], tc.expectedImageHeights[i], + dbInfo.Width, dbInfo.Height)) + } + + if !tc.skipPayloadValidation { + compare := func(get func(string) ([]byte, *model.Response), name string) { + data, resp := get(ri.Id) + if resp.Error != nil { + t.Fatal(resp.Error) + } + + expected, err := ioutil.ReadFile(filepath.Join(testDir, name)) + if err != nil { + t.Fatal(err) + } + + if bytes.Compare(data, expected) != 0 { + tf, err := ioutil.TempFile("", fmt.Sprintf("test_%v_*_%s", i, name)) + if err != nil { + t.Fatal(err) + } + io.Copy(tf, bytes.NewReader(data)) + tf.Close() + t.Errorf("Actual data mismatched %s, written to %q", name, tf.Name()) + } + } + if len(tc.expectedPayloadNames) == 0 { + tc.expectedPayloadNames = tc.names + } + + compare(client.GetFile, tc.expectedPayloadNames[i]) + if len(tc.expectedImageThumbnailNames) > i { + compare(client.GetFileThumbnail, tc.expectedImageThumbnailNames[i]) + } + if len(tc.expectedImageThumbnailNames) > i { + compare(client.GetFilePreview, tc.expectedImagePreviewNames[i]) + } + } + + th.cleanupTestFile(dbInfo) + } + }) + + if restoreConfig != nil { + restoreConfig(th.App) + } + } } } diff --git a/app/file.go b/app/file.go index 5ddfd9f412..8f85d3c5b4 100644 --- a/app/file.go +++ b/app/file.go @@ -11,7 +11,7 @@ import ( "image" "image/color" "image/draw" - _ "image/gif" + "image/gif" "image/jpeg" "io" "mime/multipart" @@ -30,6 +30,7 @@ import ( "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" "github.com/mattermost/mattermost-server/services/filesstore" + "github.com/mattermost/mattermost-server/store" "github.com/mattermost/mattermost-server/utils" ) @@ -53,7 +54,15 @@ const ( RotatedCCWMirrored = 7 RotatedCW = 8 - MaxImageSize = 6048 * 4032 // 24 megapixels, roughly 36MB as a raw image + MaxImageSize = 6048 * 4032 // 24 megapixels, roughly 36MB as a raw image + ImageThumbnailWidth = 120 + ImageThumbnailHeight = 100 + ImageThumbnailRatio = float64(ImageThumbnailHeight) / float64(ImageThumbnailWidth) + ImagePreviewWidth = 1920 + + UploadFileInitialBufferSize = 2 * 1024 * 1024 // 2Mb + + // Deprecated IMAGE_THUMBNAIL_PIXEL_WIDTH = 120 IMAGE_THUMBNAIL_PIXEL_HEIGHT = 100 IMAGE_PREVIEW_PIXEL_WIDTH = 1920 @@ -335,7 +344,8 @@ func (a *App) UploadMultipartFiles(teamId string, channelId string, userId strin for i, fileHeader := range fileHeaders { file, fileErr := fileHeader.Open() if fileErr != nil { - return nil, model.NewAppError("UploadFiles", "api.file.upload_file.bad_parse.app_error", nil, fileErr.Error(), http.StatusBadRequest) + return nil, model.NewAppError("UploadFiles", "api.file.upload_file.read_request.app_error", + map[string]interface{}{"Filename": fileHeader.Filename}, fileErr.Error(), http.StatusBadRequest) } // Will be closed after UploadFiles returns @@ -353,7 +363,7 @@ func (a *App) UploadMultipartFiles(teamId string, channelId string, userId strin // The provided files should be closed by the caller so that they are not leaked. func (a *App) UploadFiles(teamId string, channelId string, userId string, files []io.ReadCloser, filenames []string, clientIds []string, now time.Time) (*model.FileUploadResponse, *model.AppError) { if len(*a.Config().FileSettings.DriverName) == 0 { - return nil, model.NewAppError("uploadFile", "api.file.upload_file.storage.app_error", nil, "", http.StatusNotImplemented) + 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)) { @@ -421,6 +431,407 @@ func (a *App) DoUploadFile(now time.Time, rawTeamId string, rawChannelId string, return info, err } +func UploadFileSetTeamId(teamId string) func(t *uploadFileTask) { + return func(t *uploadFileTask) { + t.TeamId = filepath.Base(teamId) + } +} + +func UploadFileSetUserId(userId string) func(t *uploadFileTask) { + return func(t *uploadFileTask) { + t.UserId = filepath.Base(userId) + } +} + +func UploadFileSetTimestamp(timestamp time.Time) func(t *uploadFileTask) { + return func(t *uploadFileTask) { + t.Timestamp = timestamp + } +} + +func UploadFileSetContentLength(contentLength int64) func(t *uploadFileTask) { + return func(t *uploadFileTask) { + t.ContentLength = contentLength + } +} + +func UploadFileSetClientId(clientId string) func(t *uploadFileTask) { + return func(t *uploadFileTask) { + t.ClientId = clientId + } +} + +func UploadFileSetRaw() func(t *uploadFileTask) { + return func(t *uploadFileTask) { + t.Raw = true + } +} + +type uploadFileTask struct { + // File name. + Name string + + ChannelId string + TeamId string + UserId string + + // Time stamp to use when creating the file. + Timestamp time.Time + + // The value of the Content-Length http header, when available. + ContentLength int64 + + // The file data stream. + Input io.Reader + + // An optional, client-assigned Id field. + ClientId string + + // If Raw, do not execute special processing for images, just upload + // the file. Plugins are still invoked. + Raw bool + + //============================================================= + // Internal state + + buf *bytes.Buffer + limit int64 + limitedInput io.Reader + teeInput io.Reader + fileinfo *model.FileInfo + maxFileSize int64 + + // Cached image data that (may) get initialized in preprocessImage and + // is used in postprocessImage + decoded image.Image + imageType string + imageOrientation int + + // Testing: overrideable dependency functions + pluginsEnvironment *plugin.Environment + writeFile func(io.Reader, string) (int64, *model.AppError) + saveToDatabase func(*model.FileInfo) store.StoreChannel +} + +func (t *uploadFileTask) init(a *App) { + t.buf = &bytes.Buffer{} + t.maxFileSize = *a.Config().FileSettings.MaxFileSize + t.limit = *a.Config().FileSettings.MaxFileSize + + t.fileinfo = model.NewInfo(filepath.Base(t.Name)) + t.fileinfo.Id = model.NewId() + t.fileinfo.CreatorId = t.UserId + t.fileinfo.CreateAt = t.Timestamp.UnixNano() / int64(time.Millisecond) + t.fileinfo.Path = t.pathPrefix() + t.Name + + // Prepare to read ContentLength if it is known, otherwise limit + // ourselves to MaxFileSize. Add an extra byte to check and fail if the + // client sent too many bytes. + if t.ContentLength > 0 { + t.limit = t.ContentLength + // Over-Grow the buffer to prevent bytes.ReadFrom from doing it + // at the very end. + t.buf.Grow(int(t.limit + 1 + bytes.MinRead)) + } else { + // If we don't know the upload size, grow the buffer somewhat + // anyway to avoid extra reslicing. + t.buf.Grow(UploadFileInitialBufferSize) + } + t.limitedInput = &io.LimitedReader{ + R: t.Input, + N: t.limit + 1, + } + t.teeInput = io.TeeReader(t.limitedInput, t.buf) + + t.pluginsEnvironment = a.GetPluginsEnvironment() + t.writeFile = a.WriteFile + t.saveToDatabase = a.Srv.Store.FileInfo().Save +} + +// UploadFileX uploads a single file as specified in t. It applies the upload +// constraints, executes plugins and image processing logic as needed. It +// returns a filled-out FileInfo and an optional error. A plugin may reject the +// upload, returning a rejection error. In this case FileInfo would have +// contained the last "good" FileInfo before the execution of that plugin. +func (a *App) UploadFileX(channelId, name string, input io.Reader, + opts ...func(*uploadFileTask)) (*model.FileInfo, *model.AppError) { + + t := &uploadFileTask{ + ChannelId: filepath.Base(channelId), + Name: filepath.Base(name), + Input: input, + } + for _, o := range opts { + o(t) + } + t.init(a) + + if len(*a.Config().FileSettings.DriverName) == 0 { + return nil, t.newAppError("api.file.upload_file.storage.app_error", + "", http.StatusNotImplemented) + } + if t.ContentLength > t.maxFileSize { + return nil, t.newAppError("api.file.upload_file.too_large_detailed.app_error", + "", http.StatusRequestEntityTooLarge, "Length", t.ContentLength, "Limit", t.maxFileSize) + } + + var aerr *model.AppError + if !t.Raw && t.fileinfo.IsImage() { + aerr = t.preprocessImage() + if aerr != nil { + return t.fileinfo, aerr + } + } + + aerr = t.readAll() + if aerr != nil { + return t.fileinfo, aerr + } + + aerr = t.runPlugins() + if aerr != nil { + return t.fileinfo, aerr + } + + // Concurrently upload and update DB, and post-process the image. + wg := sync.WaitGroup{} + + if !t.Raw && t.fileinfo.IsImage() { + wg.Add(1) + go func() { + t.postprocessImage() + wg.Done() + }() + } + + _, aerr = t.writeFile(t.newReader(), t.fileinfo.Path) + if aerr != nil { + return nil, aerr + } + + if result := <-t.saveToDatabase(t.fileinfo); result.Err != nil { + return nil, result.Err + } + + wg.Wait() + + return t.fileinfo, nil +} + +func (t *uploadFileTask) readAll() *model.AppError { + _, err := t.buf.ReadFrom(t.limitedInput) + if err != nil { + return t.newAppError("api.file.upload_file.read_request.app_error", + err.Error(), http.StatusBadRequest) + } + if int64(t.buf.Len()) > t.limit { + return t.newAppError("api.file.upload_file.too_large_detailed.app_error", + "", http.StatusRequestEntityTooLarge, "Length", t.buf.Len(), "Limit", t.limit) + } + t.fileinfo.Size = int64(t.buf.Len()) + + t.limitedInput = nil + t.teeInput = nil + return nil +} + +func (t *uploadFileTask) runPlugins() *model.AppError { + if t.pluginsEnvironment == nil { + return nil + } + + pluginContext := &plugin.Context{} + var rejectionError *model.AppError + + t.pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { + buf := &bytes.Buffer{} + replacementInfo, rejectionReason := hooks.FileWillBeUploaded(pluginContext, + t.fileinfo, t.newReader(), buf) + if rejectionReason != "" { + rejectionError = t.newAppError("api.file.upload_file.read_request.app_error", + rejectionReason, http.StatusBadRequest) + return false + } + if replacementInfo != nil { + t.fileinfo = replacementInfo + } + if buf.Len() != 0 { + t.buf = buf + t.teeInput = nil + t.limitedInput = nil + t.fileinfo.Size = int64(buf.Len()) + } + + return true + }, plugin.FileWillBeUploadedId) + + if rejectionError != nil { + return rejectionError + } + + return nil +} + +func (t *uploadFileTask) preprocessImage() *model.AppError { + // If we fail to decode, return "as is". + config, _, err := image.DecodeConfig(t.newReader()) + if err != nil { + return nil + } + + t.fileinfo.Width = config.Width + t.fileinfo.Height = config.Height + + // Check dimensions before loading the whole thing into memory later on. + if t.fileinfo.Width*t.fileinfo.Height > MaxImageSize { + return t.newAppError("api.file.upload_file.large_image_detailed.app_error", + "", http.StatusBadRequest) + } + t.fileinfo.HasPreviewImage = true + nameWithoutExtension := t.Name[:strings.LastIndex(t.Name, ".")] + t.fileinfo.PreviewPath = t.pathPrefix() + nameWithoutExtension + "_preview.jpg" + t.fileinfo.ThumbnailPath = t.pathPrefix() + nameWithoutExtension + "_thumb.jpg" + + // check the image orientation with goexif; consume the bytes we + // already have first, then keep Tee-ing from input. + // TODO: try to reuse exif's .Raw buffer rather than Tee-ing + if t.imageOrientation, err = getImageOrientation(t.newReader()); err == nil && + (t.imageOrientation == RotatedCWMirrored || + t.imageOrientation == RotatedCCW || + t.imageOrientation == RotatedCCWMirrored || + t.imageOrientation == RotatedCW) { + t.fileinfo.Width, t.fileinfo.Height = t.fileinfo.Height, t.fileinfo.Width + } + + // For animated GIFs disable the preview; since we have to Decode gifs + // anyway, cache the decoded image for later. + if t.fileinfo.MimeType == "image/gif" { + gifConfig, err := gif.DecodeAll(t.newReader()) + if err == nil { + if len(gifConfig.Image) >= 1 { + t.fileinfo.HasPreviewImage = false + + } + if len(gifConfig.Image) > 0 { + t.decoded = gifConfig.Image[0] + t.imageType = "gif" + } + } + } + + return nil +} + +func (t *uploadFileTask) postprocessImage() { + decoded, typ := t.decoded, t.imageType + if decoded == nil { + var err error + decoded, typ, err = image.Decode(t.newReader()) + if err != nil { + mlog.Error(fmt.Sprintf("Unable to decode image err=%v", err)) + return + } + } + + // Fill in the background of a potentially-transparent png file as + // white. + if typ == "png" { + dst := image.NewRGBA(decoded.Bounds()) + draw.Draw(dst, dst.Bounds(), image.NewUniform(color.White), image.Point{}, draw.Src) + draw.Draw(dst, dst.Bounds(), decoded, decoded.Bounds().Min, draw.Over) + decoded = dst + } + + decoded = makeImageUpright(decoded, t.imageOrientation) + if decoded == nil { + return + } + + writeJPEG := func(img image.Image, path string) { + r, w := io.Pipe() + go func() { + _, aerr := t.writeFile(r, path) + if aerr != nil { + mlog.Error(fmt.Sprintf("Unable to upload path=%v err=%v", path, aerr)) + return + } + }() + + err := jpeg.Encode(w, img, &jpeg.Options{Quality: 90}) + if err != nil { + mlog.Error(fmt.Sprintf("Unable to encode image as jpeg path=%v err=%v", path, err)) + w.CloseWithError(err) + } else { + w.Close() + } + } + + w := decoded.Bounds().Dx() + h := decoded.Bounds().Dy() + + wg := &sync.WaitGroup{} + wg.Add(2) + go func() { + defer wg.Done() + thumb := decoded + if h > ImageThumbnailHeight || w > ImageThumbnailWidth { + if float64(h)/float64(w) < ImageThumbnailRatio { + thumb = imaging.Resize(decoded, 0, ImageThumbnailHeight, imaging.Lanczos) + } else { + thumb = imaging.Resize(decoded, ImageThumbnailWidth, 0, imaging.Lanczos) + } + } + writeJPEG(thumb, t.fileinfo.ThumbnailPath) + }() + + go func() { + defer wg.Done() + preview := decoded + if w > ImagePreviewWidth { + preview = imaging.Resize(decoded, ImagePreviewWidth, 0, imaging.Lanczos) + } + writeJPEG(preview, t.fileinfo.PreviewPath) + }() + wg.Wait() +} + +func (t uploadFileTask) newReader() io.Reader { + if t.teeInput != nil { + return io.MultiReader(bytes.NewReader(t.buf.Bytes()), t.teeInput) + } else { + return bytes.NewReader(t.buf.Bytes()) + } +} + +func (t uploadFileTask) pathPrefix() string { + return t.Timestamp.Format("20060102") + + "/teams/" + t.TeamId + + "/channels/" + t.ChannelId + + "/users/" + t.UserId + + "/" + t.fileinfo.Id + "/" +} + +func (t uploadFileTask) newAppError(id string, details interface{}, httpStatus int, extra ...interface{}) *model.AppError { + params := map[string]interface{}{ + "Name": t.Name, + "Filename": t.Name, + "ChannelId": t.ChannelId, + "TeamId": t.TeamId, + "UserId": t.UserId, + "ContentLength": t.ContentLength, + "ClientId": t.ClientId, + } + if t.fileinfo != nil { + params["Width"] = t.fileinfo.Width + params["Height"] = t.fileinfo.Height + } + for i := 0; i+1 < len(extra); i += 2 { + params[fmt.Sprintf("%v", extra[i])] = extra[i+1] + } + + return model.NewAppError("uploadFileTask", id, params, fmt.Sprintf("%v", details), httpStatus) +} + func (a *App) DoUploadFileExpectModification(now time.Time, rawTeamId string, rawChannelId string, rawUserId string, rawFilename string, data []byte) (*model.FileInfo, []byte, *model.AppError) { filename := filepath.Base(rawFilename) teamId := filepath.Base(rawTeamId) @@ -503,21 +914,21 @@ func (a *App) HandleImages(previewPathList []string, thumbnailPathList []string, img, width, height := prepareImage(fileData[i]) if img != nil { wg.Add(2) - go func(img *image.Image, path string, width int, height int) { + go func(img image.Image, path string, width int, height int) { defer wg.Done() - a.generateThumbnailImage(*img, path, width, height) + a.generateThumbnailImage(img, path, width, height) }(img, thumbnailPathList[i], width, height) - go func(img *image.Image, path string, width int) { + go func(img image.Image, path string, width int) { defer wg.Done() - a.generatePreviewImage(*img, path, width) + a.generatePreviewImage(img, path, width) }(img, previewPathList[i], width) } } wg.Wait() } -func prepareImage(fileData []byte) (*image.Image, int, int) { +func prepareImage(fileData []byte) (image.Image, int, int) { // Decode image bytes into Image object img, imgType, err := image.Decode(bytes.NewReader(fileData)) if err != nil { @@ -540,7 +951,7 @@ func prepareImage(fileData []byte) (*image.Image, int, int) { orientation, _ := getImageOrientation(bytes.NewReader(fileData)) img = makeImageUpright(img, orientation) - return &img, width, height + return img, width, height } func makeImageUpright(img image.Image, orientation int) image.Image { diff --git a/app/file_bench_test.go b/app/file_bench_test.go new file mode 100644 index 0000000000..da2d1a38aa --- /dev/null +++ b/app/file_bench_test.go @@ -0,0 +1,195 @@ +// Copyright (c) 2018-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package app + +import ( + "bytes" + "fmt" + "image" + "image/gif" + "image/jpeg" + "io" + "io/ioutil" + "math/rand" + "testing" + "time" + + "github.com/mattermost/mattermost-server/mlog" + "github.com/mattermost/mattermost-server/model" +) + +var randomJPEG []byte +var randomGIF []byte +var zero10M = make([]byte, 10*1024*1024) +var rgba *image.RGBA + +func prepareTestImages(tb testing.TB) { + if rgba != nil { + return + } + + // Create a random image (pre-seeded for predictability) + rgba = image.NewRGBA(image.Rectangle{ + image.Point{0, 0}, + image.Point{2048, 2048}, + }) + _, err := rand.New(rand.NewSource(1)).Read(rgba.Pix) + if err != nil { + tb.Fatal(err) + } + + // Encode it as JPEG and GIF + buf := &bytes.Buffer{} + err = jpeg.Encode(buf, rgba, &jpeg.Options{Quality: 50}) + if err != nil { + tb.Fatal(err) + } + randomJPEG = buf.Bytes() + + buf = &bytes.Buffer{} + err = gif.Encode(buf, rgba, nil) + if err != nil { + tb.Fatal(err) + } + randomGIF = buf.Bytes() +} + +func BenchmarkUploadFile(b *testing.B) { + prepareTestImages(b) + th := Setup().InitBasic() + defer th.TearDown() + // disable logging in the benchmark, as best we can + th.App.Log.SetConsoleLevel(mlog.LevelError) + teamId := model.NewId() + channelId := model.NewId() + userId := model.NewId() + + mb := func(i int) int { + return (i + 512*1024) / (1024 * 1024) + } + + files := []struct { + title string + ext string + data []byte + }{ + {fmt.Sprintf("random-%dMb-gif", mb(len(randomGIF))), ".gif", randomGIF}, + {fmt.Sprintf("random-%dMb-jpg", mb(len(randomJPEG))), ".jpg", randomJPEG}, + {fmt.Sprintf("zero-%dMb", mb(len(zero10M))), ".zero", zero10M}, + } + + file_benchmarks := []struct { + title string + f func(b *testing.B, n int, data []byte, ext string) + }{ + { + title: "raw-ish DoUploadFile", + f: func(b *testing.B, n int, data []byte, ext string) { + info1, err := th.App.DoUploadFile(time.Now(), teamId, channelId, + userId, fmt.Sprintf("BenchmarkDoUploadFile-%d%s", n, ext), data) + if err != nil { + b.Fatal(err) + } + <-th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id) + th.App.RemoveFile(info1.Path) + + }, + }, + { + title: "raw UploadFileX Content-Length", + f: func(b *testing.B, n int, data []byte, ext string) { + info, aerr := th.App.UploadFileX(channelId, + fmt.Sprintf("BenchmarkUploadFileTask-%d%s", n, ext), + bytes.NewReader(data), + UploadFileSetTeamId(teamId), + UploadFileSetUserId(userId), + UploadFileSetTimestamp(time.Now()), + UploadFileSetContentLength(int64(len(data))), + UploadFileSetRaw()) + if aerr != nil { + b.Fatal(aerr) + } + <-th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) + th.App.RemoveFile(info.Path) + }, + }, + { + title: "raw UploadFileX chunked", + f: func(b *testing.B, n int, data []byte, ext string) { + info, aerr := th.App.UploadFileX(channelId, + fmt.Sprintf("BenchmarkUploadFileTask-%d%s", n, ext), + bytes.NewReader(data), + UploadFileSetTeamId(teamId), + UploadFileSetUserId(userId), + UploadFileSetTimestamp(time.Now()), + UploadFileSetContentLength(-1), + UploadFileSetRaw()) + if aerr != nil { + b.Fatal(aerr) + } + <-th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) + th.App.RemoveFile(info.Path) + }, + }, + { + title: "image UploadFiles", + f: func(b *testing.B, n int, data []byte, ext string) { + resp, err := th.App.UploadFiles(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) { + info, aerr := th.App.UploadFileX(channelId, + fmt.Sprintf("BenchmarkUploadFileTask-%d%s", n, ext), + bytes.NewReader(data), + UploadFileSetTeamId(teamId), + UploadFileSetUserId(userId), + UploadFileSetTimestamp(time.Now()), + UploadFileSetContentLength(int64(len(data)))) + if aerr != nil { + b.Fatal(aerr) + } + <-th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) + th.App.RemoveFile(info.Path) + }, + }, + { + title: "image UploadFileX chunked", + f: func(b *testing.B, n int, data []byte, ext string) { + info, aerr := th.App.UploadFileX(channelId, + fmt.Sprintf("BenchmarkUploadFileTask-%d%s", n, ext), + bytes.NewReader(data), + UploadFileSetTeamId(teamId), + UploadFileSetUserId(userId), + UploadFileSetTimestamp(time.Now()), + UploadFileSetContentLength(int64(len(data)))) + if aerr != nil { + b.Fatal(aerr) + } + <-th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) + th.App.RemoveFile(info.Path) + }, + }, + } + + for _, file := range files { + for _, fb := range file_benchmarks { + b.Run(file.title+"-"+fb.title, func(b *testing.B) { + for i := 0; i < b.N; i++ { + fb.f(b, i, file.data, file.ext) + } + }) + } + } +} diff --git a/app/file_test.go b/app/file_test.go index ee1319dca2..577ca809a9 100644 --- a/app/file_test.go +++ b/app/file_test.go @@ -97,8 +97,8 @@ func TestDoUploadFile(t *testing.T) { t.Fatal(err) } else { defer func() { - <-th.App.Srv.Store.FileInfo().PermanentDelete(info3.Id) - th.App.RemoveFile(info3.Path) + <-th.App.Srv.Store.FileInfo().PermanentDelete(info4.Id) + th.App.RemoveFile(info4.Path) }() } diff --git a/app/slackimport.go b/app/slackimport.go index ba467d3572..a9863fe10b 100644 --- a/app/slackimport.go +++ b/app/slackimport.go @@ -780,8 +780,8 @@ func (a *App) OldImportFile(timestamp time.Time, file io.Reader, teamId string, if fileInfo.IsImage() && fileInfo.MimeType != "image/svg+xml" { img, width, height := prepareImage(data) if img != nil { - a.generateThumbnailImage(*img, fileInfo.ThumbnailPath, width, height) - a.generatePreviewImage(*img, fileInfo.PreviewPath, width) + a.generateThumbnailImage(img, fileInfo.ThumbnailPath, width, height) + a.generatePreviewImage(img, fileInfo.PreviewPath, width) } } diff --git a/i18n/en.json b/i18n/en.json index e652a8dc2d..a84627c693 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1201,13 +1201,29 @@ "translation": "Bad connection to S3 or minio." }, { - "id": "api.file.upload_file.bad_parse.app_error", - "translation": "Unable to upload file. Header cannot be parsed." + "id": "api.file.upload_file.read_request.app_error", + "translation": "Unable to upload file(s). Error reading or parsing request data." + }, + { + "id": "api.file.upload_file.read_form_value.app_error", + "translation": "Unable to upload file(s). Error reading the value for {{.Formname}}." + }, + { + "id": "api.file.upload_file.multiple_channel_ids.app_error", + "translation": "Unable to upload file(s). Multiple conflicting channel_ids." + }, + { + "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_detailed.app_error", + "translation": "{{.Filename}} dimensions ({{.Width}} by {{.Height}} pixels) exceed the limits" + }, { "id": "api.file.upload_file.large_image.app_error", "translation": "File above maximum dimensions could not be uploaded: {{.Filename}}" @@ -1220,6 +1236,10 @@ "id": "api.file.upload_file.too_large.app_error", "translation": "Unable to upload file. File is too large." }, + { + "id": "api.file.upload_file.too_large_detailed.app_error", + "translation": "Unable to upload file {{.Filename}}. {{.Length}} bytes exceeds the maximum allowed {{.Limit}} bytes." + }, { "id": "api.file.write_file.s3.app_error", "translation": "Encountered an error writing to S3" diff --git a/model/client4.go b/model/client4.go index 40a2ccd8be..f57a51f7f7 100644 --- a/model/client4.go +++ b/model/client4.go @@ -472,7 +472,14 @@ func (c *Client4) doApiRequestReader(method, url string, data io.Reader, etag st } func (c *Client4) DoUploadFile(url string, data []byte, contentType string) (*FileUploadResponse, *Response) { - rq, _ := http.NewRequest("POST", c.ApiUrl+url, bytes.NewReader(data)) + return c.doUploadFile(url, bytes.NewReader(data), contentType, 0) +} + +func (c *Client4) doUploadFile(url string, body io.Reader, contentType string, contentLength int64) (*FileUploadResponse, *Response) { + rq, _ := http.NewRequest("POST", c.ApiUrl+url, body) + if contentLength != 0 { + rq.ContentLength = contentLength + } rq.Header.Set("Content-Type", contentType) if len(c.AuthToken) > 0 { @@ -2290,33 +2297,33 @@ func (c *Client4) SubmitInteractiveDialog(request SubmitDialogRequest) (*SubmitD return &resp, BuildResponse(r) } -// File Section - // UploadFile will upload a file to a channel using a multipart request, to be later attached to a post. // This method is functionally equivalent to Client4.UploadFileAsRequestBody. func (c *Client4) UploadFile(data []byte, channelId string, filename string) (*FileUploadResponse, *Response) { body := &bytes.Buffer{} writer := multipart.NewWriter(body) - part, err := writer.CreateFormFile("files", filename) - if err != nil { - return nil, &Response{Error: NewAppError("UploadPostAttachment", "model.client.upload_post_attachment.file.app_error", nil, err.Error(), http.StatusBadRequest)} - } - - if _, err = io.Copy(part, bytes.NewBuffer(data)); err != nil { - return nil, &Response{Error: NewAppError("UploadPostAttachment", "model.client.upload_post_attachment.file.app_error", nil, err.Error(), http.StatusBadRequest)} - } - - part, err = writer.CreateFormField("channel_id") + part, err := writer.CreateFormField("channel_id") if err != nil { return nil, &Response{Error: NewAppError("UploadPostAttachment", "model.client.upload_post_attachment.channel_id.app_error", nil, err.Error(), http.StatusBadRequest)} } - if _, err := io.Copy(part, strings.NewReader(channelId)); err != nil { + _, err = io.Copy(part, strings.NewReader(channelId)) + if err != nil { return nil, &Response{Error: NewAppError("UploadPostAttachment", "model.client.upload_post_attachment.channel_id.app_error", nil, err.Error(), http.StatusBadRequest)} } - if err := writer.Close(); err != nil { + part, err = writer.CreateFormFile("files", filename) + if err != nil { + return nil, &Response{Error: NewAppError("UploadPostAttachment", "model.client.upload_post_attachment.file.app_error", nil, err.Error(), http.StatusBadRequest)} + } + _, err = io.Copy(part, bytes.NewBuffer(data)) + if err != nil { + return nil, &Response{Error: NewAppError("UploadPostAttachment", "model.client.upload_post_attachment.file.app_error", nil, err.Error(), http.StatusBadRequest)} + } + + err = writer.Close() + if err != nil { return nil, &Response{Error: NewAppError("UploadPostAttachment", "model.client.upload_post_attachment.writer.app_error", nil, err.Error(), http.StatusBadRequest)} } diff --git a/model/file_info.go b/model/file_info.go index 7a3e54a187..65555b6b93 100644 --- a/model/file_info.go +++ b/model/file_info.go @@ -112,6 +112,24 @@ func (o *FileInfo) IsImage() bool { return strings.HasPrefix(o.MimeType, "image") } +func NewInfo(name string) *FileInfo { + info := &FileInfo{ + Name: name, + } + + extension := strings.ToLower(filepath.Ext(name)) + info.MimeType = mime.TypeByExtension(extension) + + if extension != "" && extension[0] == '.' { + // The client expects a file extension without the leading period + info.Extension = extension[1:] + } else { + info.Extension = extension + } + + return info +} + func GetInfoForBytes(name string, data []byte) (*FileInfo, *AppError) { info := &FileInfo{ Name: name, diff --git a/plugin/environment.go b/plugin/environment.go index d64a92ea08..5951c122b5 100644 --- a/plugin/environment.go +++ b/plugin/environment.go @@ -19,11 +19,6 @@ import ( type apiImplCreatorFunc func(*model.Manifest) API -// multiPluginHookRunnerFunc is a callback function to invoke as part of RunMultiPluginHook. -// -// Return false to stop the hook from iterating to subsequent plugins. -type multiPluginHookRunnerFunc func(hooks Hooks) bool - type activePlugin struct { BundleInfo *model.BundleInfo State int @@ -288,7 +283,7 @@ func (env *Environment) HooksForPlugin(id string) (Hooks, error) { // // If hookRunnerFunc returns false, iteration will not continue. The iteration order among active // plugins is not specified. -func (env *Environment) RunMultiPluginHook(hookRunnerFunc multiPluginHookRunnerFunc, hookId int) { +func (env *Environment) RunMultiPluginHook(hookRunnerFunc func(hooks Hooks) bool, hookId int) { env.activePlugins.Range(func(key, value interface{}) bool { activePlugin := value.(activePlugin) diff --git a/tests/orientation_test.jpeg b/tests/orientation_test.jpeg new file mode 100644 index 0000000000..dd513ce6b6 Binary files /dev/null and b/tests/orientation_test.jpeg differ diff --git a/tests/orientation_test_1.jpeg b/tests/orientation_test_1.jpeg new file mode 100644 index 0000000000..d4aa4ec7c9 Binary files /dev/null and b/tests/orientation_test_1.jpeg differ diff --git a/tests/orientation_test_1_expected_preview.jpeg b/tests/orientation_test_1_expected_preview.jpeg new file mode 100644 index 0000000000..383b7b4807 Binary files /dev/null and b/tests/orientation_test_1_expected_preview.jpeg differ diff --git a/tests/orientation_test_1_expected_thumb.jpeg b/tests/orientation_test_1_expected_thumb.jpeg new file mode 100644 index 0000000000..431e30e976 Binary files /dev/null and b/tests/orientation_test_1_expected_thumb.jpeg differ diff --git a/tests/orientation_test_2.jpeg b/tests/orientation_test_2.jpeg new file mode 100644 index 0000000000..f2c5f970d6 Binary files /dev/null and b/tests/orientation_test_2.jpeg differ diff --git a/tests/orientation_test_2_expected_preview.jpeg b/tests/orientation_test_2_expected_preview.jpeg new file mode 100644 index 0000000000..ff6d4d561f Binary files /dev/null and b/tests/orientation_test_2_expected_preview.jpeg differ diff --git a/tests/orientation_test_2_expected_thumb.jpeg b/tests/orientation_test_2_expected_thumb.jpeg new file mode 100644 index 0000000000..73147fdb56 Binary files /dev/null and b/tests/orientation_test_2_expected_thumb.jpeg differ diff --git a/tests/orientation_test_3.jpeg b/tests/orientation_test_3.jpeg new file mode 100644 index 0000000000..3baf2028b5 Binary files /dev/null and b/tests/orientation_test_3.jpeg differ diff --git a/tests/orientation_test_3_expected_preview.jpeg b/tests/orientation_test_3_expected_preview.jpeg new file mode 100644 index 0000000000..39776668c3 Binary files /dev/null and b/tests/orientation_test_3_expected_preview.jpeg differ diff --git a/tests/orientation_test_3_expected_thumb.jpeg b/tests/orientation_test_3_expected_thumb.jpeg new file mode 100644 index 0000000000..e5c4e31eca Binary files /dev/null and b/tests/orientation_test_3_expected_thumb.jpeg differ diff --git a/tests/orientation_test_4.jpeg b/tests/orientation_test_4.jpeg new file mode 100644 index 0000000000..6f8f8f4c61 Binary files /dev/null and b/tests/orientation_test_4.jpeg differ diff --git a/tests/orientation_test_4_expected_preview.jpeg b/tests/orientation_test_4_expected_preview.jpeg new file mode 100644 index 0000000000..6008ab1b73 Binary files /dev/null and b/tests/orientation_test_4_expected_preview.jpeg differ diff --git a/tests/orientation_test_4_expected_thumb.jpeg b/tests/orientation_test_4_expected_thumb.jpeg new file mode 100644 index 0000000000..27a03a0d55 Binary files /dev/null and b/tests/orientation_test_4_expected_thumb.jpeg differ diff --git a/tests/orientation_test_5.jpeg b/tests/orientation_test_5.jpeg new file mode 100644 index 0000000000..d6e8fb1cc7 Binary files /dev/null and b/tests/orientation_test_5.jpeg differ diff --git a/tests/orientation_test_5_expected_preview.jpeg b/tests/orientation_test_5_expected_preview.jpeg new file mode 100644 index 0000000000..9a7a32a470 Binary files /dev/null and b/tests/orientation_test_5_expected_preview.jpeg differ diff --git a/tests/orientation_test_5_expected_thumb.jpeg b/tests/orientation_test_5_expected_thumb.jpeg new file mode 100644 index 0000000000..35df9ae7a5 Binary files /dev/null and b/tests/orientation_test_5_expected_thumb.jpeg differ diff --git a/tests/orientation_test_6.jpeg b/tests/orientation_test_6.jpeg new file mode 100644 index 0000000000..e672709d69 Binary files /dev/null and b/tests/orientation_test_6.jpeg differ diff --git a/tests/orientation_test_6_expected_preview.jpeg b/tests/orientation_test_6_expected_preview.jpeg new file mode 100644 index 0000000000..a63b8da81d Binary files /dev/null and b/tests/orientation_test_6_expected_preview.jpeg differ diff --git a/tests/orientation_test_6_expected_thumb.jpeg b/tests/orientation_test_6_expected_thumb.jpeg new file mode 100644 index 0000000000..d0f4ff50d9 Binary files /dev/null and b/tests/orientation_test_6_expected_thumb.jpeg differ diff --git a/tests/orientation_test_7.jpeg b/tests/orientation_test_7.jpeg new file mode 100644 index 0000000000..59d38698b9 Binary files /dev/null and b/tests/orientation_test_7.jpeg differ diff --git a/tests/orientation_test_7_expected_preview.jpeg b/tests/orientation_test_7_expected_preview.jpeg new file mode 100644 index 0000000000..0ad78ae869 Binary files /dev/null and b/tests/orientation_test_7_expected_preview.jpeg differ diff --git a/tests/orientation_test_7_expected_thumb.jpeg b/tests/orientation_test_7_expected_thumb.jpeg new file mode 100644 index 0000000000..1b1dc2af8e Binary files /dev/null and b/tests/orientation_test_7_expected_thumb.jpeg differ diff --git a/tests/orientation_test_8.jpeg b/tests/orientation_test_8.jpeg new file mode 100644 index 0000000000..590eb639c4 Binary files /dev/null and b/tests/orientation_test_8.jpeg differ diff --git a/tests/orientation_test_8_expected_preview.jpeg b/tests/orientation_test_8_expected_preview.jpeg new file mode 100644 index 0000000000..42ebd55ab0 Binary files /dev/null and b/tests/orientation_test_8_expected_preview.jpeg differ diff --git a/tests/orientation_test_8_expected_thumb.jpeg b/tests/orientation_test_8_expected_thumb.jpeg new file mode 100644 index 0000000000..7ef67f7e8e Binary files /dev/null and b/tests/orientation_test_8_expected_thumb.jpeg differ