From fcecd0c952d6aa286ab326c4fecd06a42de06977 Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Thu, 29 Feb 2024 14:20:57 +0000 Subject: [PATCH] [bugfix] unwrap boosts when checking in-reply-to status (#2702) * add stronger checks on status being replied to * update error code test is expecting --- .../api/client/statuses/statuscreate_test.go | 4 +- internal/processing/status/create.go | 83 ++++++++++--------- 2 files changed, 46 insertions(+), 41 deletions(-) diff --git a/internal/api/client/statuses/statuscreate_test.go b/internal/api/client/statuses/statuscreate_test.go index d936cb656..881943450 100644 --- a/internal/api/client/statuses/statuscreate_test.go +++ b/internal/api/client/statuses/statuscreate_test.go @@ -284,13 +284,13 @@ func (suite *StatusCreateTestSuite) TestReplyToNonexistentStatus() { // check response - suite.EqualValues(http.StatusBadRequest, recorder.Code) + suite.EqualValues(http.StatusNotFound, recorder.Code) result := recorder.Result() defer result.Body.Close() b, err := ioutil.ReadAll(result.Body) suite.NoError(err) - suite.Equal(`{"error":"Bad Request: cannot reply to status that does not exist"}`, string(b)) + suite.Equal(`{"error":"Not Found: target status not found"}`, string(b)) } // Post a reply to the status of a local user that allows replies. diff --git a/internal/processing/status/create.go b/internal/processing/status/create.go index 737755852..01ded74bd 100644 --- a/internal/processing/status/create.go +++ b/internal/processing/status/create.go @@ -30,6 +30,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/id" + "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/messages" "github.com/superseriousbusiness/gotosocial/internal/text" "github.com/superseriousbusiness/gotosocial/internal/typeutils" @@ -40,12 +41,20 @@ import ( // Create processes the given form to create a new status, returning the api model representation of that status if it's OK. // // Precondition: the form's fields should have already been validated and normalized by the caller. -func (p *Processor) Create(ctx context.Context, requestingAccount *gtsmodel.Account, application *gtsmodel.Application, form *apimodel.AdvancedStatusCreateForm) (*apimodel.Status, gtserror.WithCode) { +func (p *Processor) Create( + ctx context.Context, + requester *gtsmodel.Account, + application *gtsmodel.Application, + form *apimodel.AdvancedStatusCreateForm, +) ( + *apimodel.Status, + gtserror.WithCode, +) { // Generate new ID for status. statusID := id.NewULID() // Generate necessary URIs for username, to build status URIs. - accountURIs := uris.GenerateURIsForAccount(requestingAccount.Username) + accountURIs := uris.GenerateURIsForAccount(requester.Username) // Get current time. now := time.Now() @@ -57,9 +66,9 @@ func (p *Processor) Create(ctx context.Context, requestingAccount *gtsmodel.Acco CreatedAt: now, UpdatedAt: now, Local: util.Ptr(true), - Account: requestingAccount, - AccountID: requestingAccount.ID, - AccountURI: requestingAccount.URI, + Account: requester, + AccountID: requester.ID, + AccountURI: requester.URI, ActivityStreamsType: ap.ObjectNote, Sensitive: &form.Sensitive, CreatedWithApplicationID: application.ID, @@ -86,7 +95,12 @@ func (p *Processor) Create(ctx context.Context, requestingAccount *gtsmodel.Acco status.PollID = status.Poll.ID } - if errWithCode := p.processReplyToID(ctx, form, requestingAccount.ID, status); errWithCode != nil { + // Check + attach in-reply-to status. + if errWithCode := p.processInReplyTo(ctx, + requester, + status, + form.InReplyToID, + ); errWithCode != nil { return nil, errWithCode } @@ -94,15 +108,15 @@ func (p *Processor) Create(ctx context.Context, requestingAccount *gtsmodel.Acco return nil, errWithCode } - if errWithCode := p.processMediaIDs(ctx, form, requestingAccount.ID, status); errWithCode != nil { + if errWithCode := p.processMediaIDs(ctx, form, requester.ID, status); errWithCode != nil { return nil, errWithCode } - if err := processVisibility(form, requestingAccount.Privacy, status); err != nil { + if err := processVisibility(form, requester.Privacy, status); err != nil { return nil, gtserror.NewErrorInternalError(err) } - if err := processLanguage(form, requestingAccount.Language, status); err != nil { + if err := processLanguage(form, requester.Language, status); err != nil { return nil, gtserror.NewErrorInternalError(err) } @@ -128,58 +142,49 @@ func (p *Processor) Create(ctx context.Context, requestingAccount *gtsmodel.Acco APObjectType: ap.ObjectNote, APActivityType: ap.ActivityCreate, GTSModel: status, - OriginAccount: requestingAccount, + OriginAccount: requester, }) if status.Poll != nil { // Now that the status is inserted, and side effects queued, // attempt to schedule an expiry handler for the status poll. if err := p.polls.ScheduleExpiry(ctx, status.Poll); err != nil { - err := gtserror.Newf("error scheduling poll expiry: %w", err) - return nil, gtserror.NewErrorInternalError(err) + log.Errorf(ctx, "error scheduling poll expiry: %v", err) } } - return p.c.GetAPIStatus(ctx, requestingAccount, status) + return p.c.GetAPIStatus(ctx, requester, status) } -func (p *Processor) processReplyToID(ctx context.Context, form *apimodel.AdvancedStatusCreateForm, thisAccountID string, status *gtsmodel.Status) gtserror.WithCode { - if form.InReplyToID == "" { +func (p *Processor) processInReplyTo(ctx context.Context, requester *gtsmodel.Account, status *gtsmodel.Status, inReplyToID string) gtserror.WithCode { + if inReplyToID == "" { return nil } - // If this status is a reply to another status, we need to do a bit of work to establish whether or not this status can be posted: - // - // 1. Does the replied status exist in the database? - // 2. Is the replied status marked as replyable? - // 3. Does a block exist between either the current account or the account that posted the status it's replying to? - // - // If this is all OK, then we fetch the repliedStatus and the repliedAccount for later processing. - - inReplyTo, err := p.state.DB.GetStatusByID(ctx, form.InReplyToID) - if err != nil && !errors.Is(err, db.ErrNoEntries) { - err := gtserror.Newf("error fetching status %s from db: %w", form.InReplyToID, err) - return gtserror.NewErrorInternalError(err) + // Fetch target in-reply-to status (checking visibility). + inReplyTo, errWithCode := p.c.GetVisibleTargetStatus(ctx, + requester, + inReplyToID, + nil, + ) + if errWithCode != nil { + return errWithCode } - if inReplyTo == nil { - const text = "cannot reply to status that does not exist" - return gtserror.NewErrorBadRequest(errors.New(text), text) + // If this is a boost, unwrap it to get source status. + inReplyTo, errWithCode = p.c.UnwrapIfBoost(ctx, + requester, + inReplyTo, + ) + if errWithCode != nil { + return errWithCode } if !*inReplyTo.Replyable { - text := fmt.Sprintf("status %s is marked as not replyable", form.InReplyToID) + const text = "in-reply-to status marked as not replyable" return gtserror.NewErrorForbidden(errors.New(text), text) } - if blocked, err := p.state.DB.IsEitherBlocked(ctx, thisAccountID, inReplyTo.AccountID); err != nil { - err := gtserror.Newf("error checking block in db: %w", err) - return gtserror.NewErrorInternalError(err) - } else if blocked { - text := fmt.Sprintf("status %s is not replyable", form.InReplyToID) - return gtserror.NewErrorNotFound(errors.New(text), text) - } - // Set status fields from inReplyTo. status.InReplyToID = inReplyTo.ID status.InReplyTo = inReplyTo