From 66175c8ad94485338068c264e6b0fb00044e9fd6 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Sun, 30 Jan 2022 16:10:53 +0100 Subject: [PATCH] [bug] Fix minimum description length check (#369) * add/update tests * don't check min description length on media create --- internal/api/client/media/mediacreate.go | 4 +- internal/api/client/media/mediacreate_test.go | 105 +++++++- internal/api/client/media/mediaupdate_test.go | 235 ++++++++++++++++++ 3 files changed, 329 insertions(+), 15 deletions(-) create mode 100644 internal/api/client/media/mediaupdate_test.go diff --git a/internal/api/client/media/mediacreate.go b/internal/api/client/media/mediacreate.go index 7887461ee..5946ed398 100644 --- a/internal/api/client/media/mediacreate.go +++ b/internal/api/client/media/mediacreate.go @@ -149,11 +149,9 @@ func validateCreateMedia(form *model.AttachmentRequest) error { return fmt.Errorf("file size limit exceeded: limit is %d bytes but attachment was %d bytes", maxSize, form.File.Size) } - if len(form.Description) < minDescriptionChars || len(form.Description) > maxDescriptionChars { + if len(form.Description) > maxDescriptionChars { return fmt.Errorf("image description length must be between %d and %d characters (inclusive), but provided image description was %d chars", minDescriptionChars, maxDescriptionChars, len(form.Description)) } - // TODO: validate focus here - return nil } diff --git a/internal/api/client/media/mediacreate_test.go b/internal/api/client/media/mediacreate_test.go index 2897d786b..2d8042e33 100644 --- a/internal/api/client/media/mediacreate_test.go +++ b/internal/api/client/media/mediacreate_test.go @@ -21,6 +21,8 @@ package media_test import ( "bytes" "context" + "crypto/rand" + "encoding/base64" "encoding/json" "fmt" "io/ioutil" @@ -31,10 +33,11 @@ import ( "codeberg.org/gruf/go-store/kv" "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" - "github.com/stretchr/testify/assert" + "github.com/spf13/viper" "github.com/stretchr/testify/suite" mediamodule "github.com/superseriousbusiness/gotosocial/internal/api/client/media" "github.com/superseriousbusiness/gotosocial/internal/api/model" + "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/email" "github.com/superseriousbusiness/gotosocial/internal/federation" @@ -117,7 +120,7 @@ func (suite *MediaCreateTestSuite) TearDownTest() { ACTUAL TESTS */ -func (suite *MediaCreateTestSuite) TestStatusCreatePOSTImageHandlerSuccessful() { +func (suite *MediaCreateTestSuite) TestMediaCreateSuccessful() { // set up the context for the request t := suite.testTokens["local_account_1"] oauthToken := oauth.DBTokenToToken(t) @@ -171,16 +174,16 @@ func (suite *MediaCreateTestSuite) TestStatusCreatePOSTImageHandlerSuccessful() result := recorder.Result() defer result.Body.Close() b, err := ioutil.ReadAll(result.Body) - assert.NoError(suite.T(), err) + suite.NoError(err) fmt.Println(string(b)) attachmentReply := &model.Attachment{} err = json.Unmarshal(b, attachmentReply) - assert.NoError(suite.T(), err) + suite.NoError(err) - assert.Equal(suite.T(), "this is a test image -- a cool background from somewhere", attachmentReply.Description) - assert.Equal(suite.T(), "image", attachmentReply.Type) - assert.EqualValues(suite.T(), model.MediaMeta{ + suite.Equal("this is a test image -- a cool background from somewhere", attachmentReply.Description) + suite.Equal("image", attachmentReply.Type) + suite.EqualValues(model.MediaMeta{ Original: model.MediaDimensions{ Width: 1920, Height: 1080, @@ -198,11 +201,89 @@ func (suite *MediaCreateTestSuite) TestStatusCreatePOSTImageHandlerSuccessful() Y: 0.5, }, }, attachmentReply.Meta) - assert.Equal(suite.T(), "LjBzUo#6RQR._NvzRjWF?urqV@a$", attachmentReply.Blurhash) - assert.NotEmpty(suite.T(), attachmentReply.ID) - assert.NotEmpty(suite.T(), attachmentReply.URL) - assert.NotEmpty(suite.T(), attachmentReply.PreviewURL) - assert.Equal(suite.T(), len(storageKeysBeforeRequest)+2, len(storageKeysAfterRequest)) // 2 images should be added to storage: the original and the thumbnail + suite.Equal("LjBzUo#6RQR._NvzRjWF?urqV@a$", attachmentReply.Blurhash) + suite.NotEmpty(attachmentReply.ID) + suite.NotEmpty(attachmentReply.URL) + suite.NotEmpty(attachmentReply.PreviewURL) + suite.Equal(len(storageKeysBeforeRequest)+2, len(storageKeysAfterRequest)) // 2 images should be added to storage: the original and the thumbnail +} + +func (suite *MediaCreateTestSuite) TestMediaCreateLongDescription() { + // set up the context for the request + t := suite.testTokens["local_account_1"] + oauthToken := oauth.DBTokenToToken(t) + recorder := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(recorder) + ctx.Set(oauth.SessionAuthorizedApplication, suite.testApplications["application_1"]) + ctx.Set(oauth.SessionAuthorizedToken, oauthToken) + ctx.Set(oauth.SessionAuthorizedUser, suite.testUsers["local_account_1"]) + ctx.Set(oauth.SessionAuthorizedAccount, suite.testAccounts["local_account_1"]) + + // read a random string of a really long description + descriptionBytes := make([]byte, 5000) + if _, err := rand.Read(descriptionBytes); err != nil { + panic(err) + } + description := base64.RawStdEncoding.EncodeToString(descriptionBytes) + + // create the request + buf, w, err := testrig.CreateMultipartFormData("file", "../../../../testrig/media/test-jpeg.jpg", map[string]string{ + "description": description, + "focus": "-0.5,0.5", + }) + if err != nil { + panic(err) + } + ctx.Request = httptest.NewRequest(http.MethodPost, fmt.Sprintf("http://localhost:8080/%s", mediamodule.BasePath), bytes.NewReader(buf.Bytes())) // the endpoint we're hitting + ctx.Request.Header.Set("Content-Type", w.FormDataContentType()) + ctx.Request.Header.Set("accept", "application/json") + + // do the actual request + suite.mediaModule.MediaCreatePOSTHandler(ctx) + + // check response + suite.EqualValues(http.StatusUnprocessableEntity, recorder.Code) + + result := recorder.Result() + defer result.Body.Close() + b, err := ioutil.ReadAll(result.Body) + suite.NoError(err) + + expectedErr := fmt.Sprintf(`{"error":"image description length must be between 0 and 500 characters (inclusive), but provided image description was %d chars"}`, len(description)) + suite.Equal(expectedErr, string(b)) +} + +func (suite *MediaCreateTestSuite) TestMediaCreateTooShortDescription() { + // set the min description length + viper.Set(config.Keys.MediaDescriptionMinChars, 500) + + // set up the context for the request + t := suite.testTokens["local_account_1"] + oauthToken := oauth.DBTokenToToken(t) + recorder := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(recorder) + ctx.Set(oauth.SessionAuthorizedApplication, suite.testApplications["application_1"]) + ctx.Set(oauth.SessionAuthorizedToken, oauthToken) + ctx.Set(oauth.SessionAuthorizedUser, suite.testUsers["local_account_1"]) + ctx.Set(oauth.SessionAuthorizedAccount, suite.testAccounts["local_account_1"]) + + // create the request + buf, w, err := testrig.CreateMultipartFormData("file", "../../../../testrig/media/test-jpeg.jpg", map[string]string{ + "description": "", // provide an empty description + "focus": "-0.5,0.5", + }) + if err != nil { + panic(err) + } + ctx.Request = httptest.NewRequest(http.MethodPost, fmt.Sprintf("http://localhost:8080/%s", mediamodule.BasePath), bytes.NewReader(buf.Bytes())) // the endpoint we're hitting + ctx.Request.Header.Set("Content-Type", w.FormDataContentType()) + ctx.Request.Header.Set("accept", "application/json") + + // do the actual request + suite.mediaModule.MediaCreatePOSTHandler(ctx) + + // check response -- there should be no error because minimum description length is checked on *UPDATE*, not initial upload + suite.EqualValues(http.StatusOK, recorder.Code) } func TestMediaCreateTestSuite(t *testing.T) { diff --git a/internal/api/client/media/mediaupdate_test.go b/internal/api/client/media/mediaupdate_test.go new file mode 100644 index 000000000..cac6c304e --- /dev/null +++ b/internal/api/client/media/mediaupdate_test.go @@ -0,0 +1,235 @@ +/* + GoToSocial + Copyright (C) 2021-2022 GoToSocial Authors admin@gotosocial.org + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with this program. If not, see . +*/ + +package media_test + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" + + "codeberg.org/gruf/go-store/kv" + "github.com/gin-gonic/gin" + "github.com/sirupsen/logrus" + "github.com/spf13/viper" + "github.com/stretchr/testify/suite" + mediamodule "github.com/superseriousbusiness/gotosocial/internal/api/client/media" + "github.com/superseriousbusiness/gotosocial/internal/api/model" + "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/db" + "github.com/superseriousbusiness/gotosocial/internal/email" + "github.com/superseriousbusiness/gotosocial/internal/federation" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/media" + "github.com/superseriousbusiness/gotosocial/internal/oauth" + "github.com/superseriousbusiness/gotosocial/internal/processing" + "github.com/superseriousbusiness/gotosocial/internal/typeutils" + "github.com/superseriousbusiness/gotosocial/testrig" +) + +type MediaUpdateTestSuite struct { + // standard suite interfaces + suite.Suite + db db.DB + storage *kv.KVStore + federator federation.Federator + tc typeutils.TypeConverter + mediaHandler media.Handler + oauthServer oauth.Server + emailSender email.Sender + processor processing.Processor + + // standard suite models + testTokens map[string]*gtsmodel.Token + testClients map[string]*gtsmodel.Client + testApplications map[string]*gtsmodel.Application + testUsers map[string]*gtsmodel.User + testAccounts map[string]*gtsmodel.Account + testAttachments map[string]*gtsmodel.MediaAttachment + + // item being tested + mediaModule *mediamodule.Module +} + +/* + TEST INFRASTRUCTURE +*/ + +func (suite *MediaUpdateTestSuite) SetupSuite() { + // setup standard items + testrig.InitTestConfig() + testrig.InitTestLog() + suite.db = testrig.NewTestDB() + suite.storage = testrig.NewTestStorage() + suite.tc = testrig.NewTestTypeConverter(suite.db) + suite.mediaHandler = testrig.NewTestMediaHandler(suite.db, suite.storage) + suite.oauthServer = testrig.NewTestOauthServer(suite.db) + suite.federator = testrig.NewTestFederator(suite.db, testrig.NewTestTransportController(testrig.NewMockHTTPClient(nil), suite.db), suite.storage) + suite.emailSender = testrig.NewEmailSender("../../../../web/template/", nil) + suite.processor = testrig.NewTestProcessor(suite.db, suite.storage, suite.federator, suite.emailSender) + + // setup module being tested + suite.mediaModule = mediamodule.New(suite.processor).(*mediamodule.Module) +} + +func (suite *MediaUpdateTestSuite) TearDownSuite() { + if err := suite.db.Stop(context.Background()); err != nil { + logrus.Panicf("error closing db connection: %s", err) + } +} + +func (suite *MediaUpdateTestSuite) SetupTest() { + testrig.StandardDBSetup(suite.db, nil) + testrig.StandardStorageSetup(suite.storage, "../../../../testrig/media") + suite.testTokens = testrig.NewTestTokens() + suite.testClients = testrig.NewTestClients() + suite.testApplications = testrig.NewTestApplications() + suite.testUsers = testrig.NewTestUsers() + suite.testAccounts = testrig.NewTestAccounts() + suite.testAttachments = testrig.NewTestAttachments() +} + +func (suite *MediaUpdateTestSuite) TearDownTest() { + testrig.StandardDBTeardown(suite.db) + testrig.StandardStorageTeardown(suite.storage) +} + +/* + ACTUAL TESTS +*/ + +func (suite *MediaUpdateTestSuite) TestUpdateImage() { + toUpdate := suite.testAttachments["local_account_1_unattached_1"] + + // set up the context for the request + t := suite.testTokens["local_account_1"] + oauthToken := oauth.DBTokenToToken(t) + recorder := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(recorder) + ctx.Set(oauth.SessionAuthorizedApplication, suite.testApplications["application_1"]) + ctx.Set(oauth.SessionAuthorizedToken, oauthToken) + ctx.Set(oauth.SessionAuthorizedUser, suite.testUsers["local_account_1"]) + ctx.Set(oauth.SessionAuthorizedAccount, suite.testAccounts["local_account_1"]) + + // create the request + buf, w, err := testrig.CreateMultipartFormData("", "", map[string]string{ + "id": toUpdate.ID, + "description": "new description!", + "focus": "-0.1,0.3", + }) + if err != nil { + panic(err) + } + ctx.Request = httptest.NewRequest(http.MethodPut, fmt.Sprintf("http://localhost:8080/%s/%s", mediamodule.BasePath, toUpdate.ID), bytes.NewReader(buf.Bytes())) // the endpoint we're hitting + ctx.Request.Header.Set("Content-Type", w.FormDataContentType()) + ctx.Request.Header.Set("accept", "application/json") + ctx.Params = gin.Params{ + gin.Param{ + Key: mediamodule.IDKey, + Value: toUpdate.ID, + }, + } + + // do the actual request + suite.mediaModule.MediaPUTHandler(ctx) + + // check response + suite.EqualValues(http.StatusOK, recorder.Code) + + result := recorder.Result() + defer result.Body.Close() + b, err := ioutil.ReadAll(result.Body) + suite.NoError(err) + + // reply should be an attachment + attachmentReply := &model.Attachment{} + err = json.Unmarshal(b, attachmentReply) + suite.NoError(err) + + // the reply should contain the updated fields + suite.Equal("new description!", attachmentReply.Description) + suite.EqualValues("gif", attachmentReply.Type) + suite.EqualValues(model.MediaMeta{ + Original: model.MediaDimensions{Width: 800, Height: 450, FrameRate: "", Duration: 0, Bitrate: 0, Size: "800x450", Aspect: 1.7777778}, + Small: model.MediaDimensions{Width: 256, Height: 144, FrameRate: "", Duration: 0, Bitrate: 0, Size: "256x144", Aspect: 1.7777778}, + Focus: model.MediaFocus{X: -0.1, Y: 0.3}, + }, attachmentReply.Meta) + suite.Equal(toUpdate.Blurhash, attachmentReply.Blurhash) + suite.Equal(toUpdate.ID, attachmentReply.ID) + suite.Equal(toUpdate.URL, attachmentReply.URL) + suite.NotEmpty(toUpdate.Thumbnail.URL, attachmentReply.PreviewURL) +} + +func (suite *MediaUpdateTestSuite) TestUpdateImageShortDescription() { + // set the min description length + viper.Set(config.Keys.MediaDescriptionMinChars, 50) + + toUpdate := suite.testAttachments["local_account_1_unattached_1"] + + // set up the context for the request + t := suite.testTokens["local_account_1"] + oauthToken := oauth.DBTokenToToken(t) + recorder := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(recorder) + ctx.Set(oauth.SessionAuthorizedApplication, suite.testApplications["application_1"]) + ctx.Set(oauth.SessionAuthorizedToken, oauthToken) + ctx.Set(oauth.SessionAuthorizedUser, suite.testUsers["local_account_1"]) + ctx.Set(oauth.SessionAuthorizedAccount, suite.testAccounts["local_account_1"]) + + // create the request + buf, w, err := testrig.CreateMultipartFormData("", "", map[string]string{ + "id": toUpdate.ID, + "description": "new description!", + "focus": "-0.1,0.3", + }) + if err != nil { + panic(err) + } + ctx.Request = httptest.NewRequest(http.MethodPut, fmt.Sprintf("http://localhost:8080/%s/%s", mediamodule.BasePath, toUpdate.ID), bytes.NewReader(buf.Bytes())) // the endpoint we're hitting + ctx.Request.Header.Set("Content-Type", w.FormDataContentType()) + ctx.Request.Header.Set("accept", "application/json") + ctx.Params = gin.Params{ + gin.Param{ + Key: mediamodule.IDKey, + Value: toUpdate.ID, + }, + } + + // do the actual request + suite.mediaModule.MediaPUTHandler(ctx) + + // check response + suite.EqualValues(http.StatusBadRequest, recorder.Code) + + result := recorder.Result() + defer result.Body.Close() + b, err := ioutil.ReadAll(result.Body) + suite.NoError(err) + + // reply should be an error message + suite.Equal(`{"error":"image description length must be between 50 and 500 characters (inclusive), but provided image description was 16 chars"}`, string(b)) +} + +func TestMediaUpdateTestSuite(t *testing.T) { + suite.Run(t, new(MediaUpdateTestSuite)) +}