From 3e82196d5e61dbb04a96c06684204079b191de23 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Thu, 24 Nov 2022 16:12:43 +0100 Subject: [PATCH] [bugfix] Fix status boosts giving 404 (#1137) We broke this at some point recently, and i'm not sure when. In any case, i updated some of the logic in there + added a test for it. --- internal/api/client/status/statusboostedby.go | 20 +--- .../api/client/status/statusboostedby_test.go | 112 ++++++++++++++++++ internal/processing/status/boostedby.go | 59 ++++++--- 3 files changed, 162 insertions(+), 29 deletions(-) create mode 100644 internal/api/client/status/statusboostedby_test.go diff --git a/internal/api/client/status/statusboostedby.go b/internal/api/client/status/statusboostedby.go index 883f07fac..4a175f6e9 100644 --- a/internal/api/client/status/statusboostedby.go +++ b/internal/api/client/status/statusboostedby.go @@ -19,12 +19,12 @@ package status import ( + "errors" "net/http" - "codeberg.org/gruf/go-kv" "github.com/gin-gonic/gin" "github.com/superseriousbusiness/gotosocial/internal/api" - "github.com/superseriousbusiness/gotosocial/internal/log" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/oauth" ) @@ -66,24 +66,16 @@ import ( // '404': // description: not found func (m *Module) StatusBoostedByGETHandler(c *gin.Context) { - l := log.WithFields(kv.Fields{ - - {"request_uri", c.Request.RequestURI}, - {"user_agent", c.Request.UserAgent()}, - {"origin_ip", c.ClientIP()}, - }...) - l.Debugf("entering function") - - authed, err := oauth.Authed(c, true, true, true, true) // we don't really need an app here but we want everything else + authed, err := oauth.Authed(c, true, true, true, true) if err != nil { - l.Errorf("error authing status boosted by request: %s", err) - c.JSON(http.StatusBadRequest, gin.H{"error": "not authed"}) + api.ErrorHandler(c, gtserror.NewErrorUnauthorized(err, err.Error()), m.processor.InstanceGet) return } targetStatusID := c.Param(IDKey) if targetStatusID == "" { - c.JSON(http.StatusBadRequest, gin.H{"error": "no status id provided"}) + err := errors.New("no status id specified") + api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet) return } diff --git a/internal/api/client/status/statusboostedby_test.go b/internal/api/client/status/statusboostedby_test.go new file mode 100644 index 000000000..0d7c9f7ab --- /dev/null +++ b/internal/api/client/status/statusboostedby_test.go @@ -0,0 +1,112 @@ +/* + 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 status_test + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/api/client/status" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/oauth" + "github.com/superseriousbusiness/gotosocial/testrig" +) + +type StatusBoostedByTestSuite struct { + StatusStandardTestSuite +} + +func (suite *StatusBoostedByTestSuite) TestRebloggedByOK() { + t := suite.testTokens["local_account_1"] + oauthToken := oauth.DBTokenToToken(t) + targetStatus := suite.testStatuses["local_account_1_status_1"] + + recorder := httptest.NewRecorder() + ctx, _ := testrig.CreateGinTestContext(recorder, nil) + 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"]) + ctx.Request = httptest.NewRequest(http.MethodGet, fmt.Sprintf("http://localhost:8080%s", strings.Replace(status.RebloggedPath, ":id", targetStatus.ID, 1)), nil) // the endpoint we're hitting + ctx.Request.Header.Set("accept", "application/json") + ctx.AddParam("id", targetStatus.ID) + + suite.statusModule.StatusBoostedByGETHandler(ctx) + + suite.EqualValues(http.StatusOK, recorder.Code) + + result := recorder.Result() + defer result.Body.Close() + + b, err := ioutil.ReadAll(result.Body) + suite.NoError(err) + + accounts := []*gtsmodel.Account{} + err = json.Unmarshal(b, &accounts) + suite.NoError(err) + + if !suite.Len(accounts, 1) { + suite.FailNow("should have had 1 account") + } + + suite.Equal(accounts[0].ID, suite.testAccounts["admin_account"].ID) +} + +func (suite *StatusBoostedByTestSuite) TestRebloggedByUseBoostWrapperID() { + t := suite.testTokens["local_account_1"] + oauthToken := oauth.DBTokenToToken(t) + targetStatus := suite.testStatuses["admin_account_status_4"] // admin_account_status_4 is a boost of local_account_1_status_1 + + recorder := httptest.NewRecorder() + ctx, _ := testrig.CreateGinTestContext(recorder, nil) + 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"]) + ctx.Request = httptest.NewRequest(http.MethodGet, fmt.Sprintf("http://localhost:8080%s", strings.Replace(status.RebloggedPath, ":id", targetStatus.ID, 1)), nil) // the endpoint we're hitting + ctx.Request.Header.Set("accept", "application/json") + ctx.AddParam("id", targetStatus.ID) + + suite.statusModule.StatusBoostedByGETHandler(ctx) + + suite.EqualValues(http.StatusOK, recorder.Code) + + result := recorder.Result() + defer result.Body.Close() + + b, err := ioutil.ReadAll(result.Body) + suite.NoError(err) + + accounts := []*gtsmodel.Account{} + err = json.Unmarshal(b, &accounts) + suite.NoError(err) + + if !suite.Len(accounts, 1) { + suite.FailNow("should have had 1 account") + } + + suite.Equal(accounts[0].ID, suite.testAccounts["admin_account"].ID) +} + +func TestStatusBoostedByTestSuite(t *testing.T) { + suite.Run(t, new(StatusBoostedByTestSuite)) +} diff --git a/internal/processing/status/boostedby.go b/internal/processing/status/boostedby.go index 39a97aff0..9edbef938 100644 --- a/internal/processing/status/boostedby.go +++ b/internal/processing/status/boostedby.go @@ -24,6 +24,7 @@ import ( "fmt" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" + "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" ) @@ -31,45 +32,73 @@ import ( func (p *processor) BoostedBy(ctx context.Context, requestingAccount *gtsmodel.Account, targetStatusID string) ([]*apimodel.Account, gtserror.WithCode) { targetStatus, err := p.db.GetStatusByID(ctx, targetStatusID) if err != nil { - return nil, gtserror.NewErrorNotFound(fmt.Errorf("error fetching status %s: %s", targetStatusID, err)) + wrapped := fmt.Errorf("BoostedBy: error fetching status %s: %s", targetStatusID, err) + if !errors.Is(err, db.ErrNoEntries) { + return nil, gtserror.NewErrorInternalError(wrapped) + } + return nil, gtserror.NewErrorNotFound(wrapped) } - if targetStatus.Account == nil { - return nil, gtserror.NewErrorNotFound(fmt.Errorf("no status owner for status %s", targetStatusID)) + + if boostOfID := targetStatus.BoostOfID; boostOfID != "" { + // the target status is a boost wrapper, redirect this request to the status it boosts + boostedStatus, err := p.db.GetStatusByID(ctx, boostOfID) + if err != nil { + wrapped := fmt.Errorf("BoostedBy: error fetching status %s: %s", boostOfID, err) + if !errors.Is(err, db.ErrNoEntries) { + return nil, gtserror.NewErrorInternalError(wrapped) + } + return nil, gtserror.NewErrorNotFound(wrapped) + } + targetStatus = boostedStatus } visible, err := p.filter.StatusVisible(ctx, targetStatus, requestingAccount) if err != nil { - return nil, gtserror.NewErrorNotFound(fmt.Errorf("error seeing if status %s is visible: %s", targetStatus.ID, err)) + err = fmt.Errorf("BoostedBy: error seeing if status %s is visible: %s", targetStatus.ID, err) + return nil, gtserror.NewErrorNotFound(err) } if !visible { - return nil, gtserror.NewErrorNotFound(errors.New("status is not visible")) + err = errors.New("BoostedBy: status is not visible") + return nil, gtserror.NewErrorNotFound(err) } statusReblogs, err := p.db.GetStatusReblogs(ctx, targetStatus) if err != nil { - return nil, gtserror.NewErrorNotFound(fmt.Errorf("StatusBoostedBy: error seeing who boosted status: %s", err)) + err = fmt.Errorf("BoostedBy: error seeing who boosted status: %s", err) + return nil, gtserror.NewErrorNotFound(err) } - // filter the list so the user doesn't see accounts they blocked or which blocked them - filteredAccounts := []*gtsmodel.Account{} + // filter account IDs so the user doesn't see accounts they blocked or which blocked them + accountIDs := make([]string, 0, len(statusReblogs)) for _, s := range statusReblogs { blocked, err := p.db.IsBlocked(ctx, requestingAccount.ID, s.AccountID, true) if err != nil { - return nil, gtserror.NewErrorNotFound(fmt.Errorf("StatusBoostedBy: error checking blocks: %s", err)) + err = fmt.Errorf("BoostedBy: error checking blocks: %s", err) + return nil, gtserror.NewErrorNotFound(err) } if !blocked { - filteredAccounts = append(filteredAccounts, s.Account) + accountIDs = append(accountIDs, s.AccountID) } } // TODO: filter other things here? suspended? muted? silenced? - // now we can return the api representation of those accounts - apiAccounts := []*apimodel.Account{} - for _, acc := range filteredAccounts { - apiAccount, err := p.tc.AccountToAPIAccountPublic(ctx, acc) + // fetch accounts + create their API representations + apiAccounts := make([]*apimodel.Account, 0, len(accountIDs)) + for _, accountID := range accountIDs { + account, err := p.db.GetAccountByID(ctx, accountID) if err != nil { - return nil, gtserror.NewErrorNotFound(fmt.Errorf("StatusFavedBy: error converting account to api model: %s", err)) + wrapped := fmt.Errorf("BoostedBy: error fetching account %s: %s", accountID, err) + if !errors.Is(err, db.ErrNoEntries) { + return nil, gtserror.NewErrorInternalError(wrapped) + } + return nil, gtserror.NewErrorNotFound(wrapped) + } + + apiAccount, err := p.tc.AccountToAPIAccountPublic(ctx, account) + if err != nil { + err = fmt.Errorf("BoostedBy: error converting account to api model: %s", err) + return nil, gtserror.NewErrorInternalError(err) } apiAccounts = append(apiAccounts, apiAccount) }