From d1cac53cbb69ab7df5522aa2fa52df4c9333063a Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Fri, 1 Dec 2023 10:53:53 +0000 Subject: [PATCH] [bugfix] in fedi API CreateStatus(), handle case of data-race and return early (#2403) --- internal/processing/workers/fromfediapi.go | 124 +++++++++------------ 1 file changed, 52 insertions(+), 72 deletions(-) diff --git a/internal/processing/workers/fromfediapi.go b/internal/processing/workers/fromfediapi.go index a8da50819..4c5c85666 100644 --- a/internal/processing/workers/fromfediapi.go +++ b/internal/processing/workers/fromfediapi.go @@ -151,21 +151,62 @@ func (p *Processor) ProcessFromFediAPI(ctx context.Context, fMsg messages.FromFe func (p *fediAPI) CreateStatus(ctx context.Context, fMsg messages.FromFediAPI) error { var ( - status *gtsmodel.Status - err error + status *gtsmodel.Status + statusable ap.Statusable + err error ) - if fMsg.APObjectModel == nil /* i.e. forwarded */ { + var ok bool + + switch { + case fMsg.APObjectModel != nil: + // A model was provided, extract this from message. + statusable, ok = fMsg.APObjectModel.(ap.Statusable) + if !ok { + return gtserror.Newf("cannot cast %T -> ap.Statusable", fMsg.APObjectModel) + } + + // Create bare-bones model to pass + // into RefreshStatus(), which it will + // further populate and insert as new. + bareStatus := new(gtsmodel.Status) + bareStatus.Local = util.Ptr(false) + bareStatus.URI = ap.GetJSONLDId(statusable).String() + + // Call RefreshStatus() to parse and process the provided + // statusable model, which it will use to further flesh out + // the bare bones model and insert it into the database. + status, statusable, err = p.federate.RefreshStatus(ctx, + fMsg.ReceivingAccount.Username, + bareStatus, + statusable, + true, + ) + if err != nil { + return gtserror.Newf("error processing new status %s: %w", bareStatus.URI, err) + } + + case fMsg.APIri != nil: // Model was not set, deref with IRI (this is a forward). // This will also cause the status to be inserted into the db. - status, err = p.statusFromAPIRI(ctx, fMsg) - } else { - // Model is set, ensure we have the most up-to-date model. - status, err = p.statusFromAPModel(ctx, fMsg) + status, statusable, err = p.federate.GetStatusByURI(ctx, + fMsg.ReceivingAccount.Username, + fMsg.APIri, + ) + if err != nil { + return gtserror.Newf("error dereferencing forwarded status %s: %w", fMsg.APIri, err) + } + + default: + return gtserror.New("neither APObjectModel nor APIri set") } - if err != nil { - return gtserror.Newf("error extracting status from federatorMsg: %w", err) + if statusable == nil { + // Another thread beat us to + // creating this status! Return + // here and let the other thread + // handle timelining + notifying. + return nil } if status.InReplyToID != "" { @@ -227,66 +268,6 @@ func (p *fediAPI) CreatePollVote(ctx context.Context, fMsg messages.FromFediAPI) return nil } -func (p *fediAPI) statusFromAPModel(ctx context.Context, fMsg messages.FromFediAPI) (*gtsmodel.Status, error) { - // AP statusable representation MUST have been set. - statusable, ok := fMsg.APObjectModel.(ap.Statusable) - if !ok { - return nil, gtserror.Newf("cannot cast %T -> ap.Statusable", fMsg.APObjectModel) - } - - // Status may have been set (no problem if not). - status, _ := fMsg.GTSModel.(*gtsmodel.Status) - - if status == nil { - // No status was set, create a bare-bones - // model for the deferencer to flesh-out, - // this indicates it is a new (to us) status. - status = >smodel.Status{ - - // if coming in here status will ALWAYS be remote. - Local: util.Ptr(false), - URI: ap.GetJSONLDId(statusable).String(), - } - } - - // Call refresh on status to either update existing - // model, or parse + insert status from statusable data. - status, _, err := p.federate.RefreshStatus( - ctx, - fMsg.ReceivingAccount.Username, - status, - statusable, - false, // Don't force refresh. - ) - if err != nil { - return nil, gtserror.Newf("error refreshing status: %w", err) - } - - return status, nil -} - -func (p *fediAPI) statusFromAPIRI(ctx context.Context, fMsg messages.FromFediAPI) (*gtsmodel.Status, error) { - // There should be a status IRI pinned to - // the federatorMsg for us to dereference. - if fMsg.APIri == nil { - const text = "neither APObjectModel nor APIri set" - return nil, gtserror.New(text) - } - - // Get the status + ensure we have - // the most up-to-date version. - status, _, err := p.federate.GetStatusByURI( - ctx, - fMsg.ReceivingAccount.Username, - fMsg.APIri, - ) - if err != nil { - return nil, gtserror.Newf("error getting status by uri %s: %w", fMsg.APIri, err) - } - - return status, nil -} - func (p *fediAPI) CreateFollowReq(ctx context.Context, fMsg messages.FromFediAPI) error { followRequest, ok := fMsg.GTSModel.(*gtsmodel.FollowRequest) if !ok { @@ -359,8 +340,7 @@ func (p *fediAPI) CreateAnnounce(ctx context.Context, fMsg messages.FromFediAPI) // Dereference status that this boosts, note // that this will handle dereferencing the status // ancestors / descendants where appropriate. - if err := p.federate.DereferenceAnnounce( - ctx, + if err := p.federate.DereferenceAnnounce(ctx, status, fMsg.ReceivingAccount.Username, ); err != nil { @@ -388,7 +368,7 @@ func (p *fediAPI) CreateAnnounce(ctx context.Context, fMsg messages.FromFediAPI) log.Errorf(ctx, "error notifying announce: %v", err) } - // Interaction counts changed on the boosted status; + // Interaction counts changed on the original status; // uncache the prepared version from all timelines. p.surface.invalidateStatusFromTimelines(ctx, status.ID)