commit 3e82196d5e61dbb04a96c06684204079b191de23
parent da8954858afb4d5a3a2faf55e77d7da3be0ea3db
Author: tobi <31960611+tsmethurst@users.noreply.github.com>
Date: Thu, 24 Nov 2022 16:12:43 +0100
[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.
Diffstat:
3 files changed, 162 insertions(+), 29 deletions(-)
diff --git 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
@@ -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 <http://www.gnu.org/licenses/>.
+*/
+
+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
@@ -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 {
+ 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 {
- return nil, gtserror.NewErrorNotFound(fmt.Errorf("StatusFavedBy: error converting account to api model: %s", err))
+ err = fmt.Errorf("BoostedBy: error converting account to api model: %s", err)
+ return nil, gtserror.NewErrorInternalError(err)
}
apiAccounts = append(apiAccounts, apiAccount)
}
