Timeline bugfix (#60)

* fix a stack overflow in the timeline

* go fmt
This commit is contained in:
Tobi Smethurst 2021-06-23 18:42:20 +02:00 committed by GitHub
parent 8c9a853343
commit 16e486ad96
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 100 additions and 26 deletions

View file

@ -13,7 +13,12 @@ func (t *timeline) Get(amount int, maxID string, sinceID string, minID string) (
l := t.log.WithFields(logrus.Fields{ l := t.log.WithFields(logrus.Fields{
"func": "Get", "func": "Get",
"accountID": t.accountID, "accountID": t.accountID,
"amount": amount,
"maxID": maxID,
"sinceID": sinceID,
"minID": minID,
}) })
l.Debug("entering get")
var statuses []*apimodel.Status var statuses []*apimodel.Status
var err error var err error

View file

@ -10,6 +10,45 @@
) )
func (t *timeline) IndexBefore(statusID string, include bool, amount int) error { func (t *timeline) IndexBefore(statusID string, include bool, amount int) error {
filtered := []*gtsmodel.Status{}
offsetStatus := statusID
if include {
s := &gtsmodel.Status{}
if err := t.db.GetByID(statusID, s); err != nil {
return fmt.Errorf("IndexBefore: error getting initial status with id %s: %s", statusID, err)
}
filtered = append(filtered, s)
}
grabloop:
for len(filtered) < amount {
statuses, err := t.db.GetStatusesWhereFollowing(t.accountID, "", offsetStatus, "", amount, false)
if err != nil {
if _, ok := err.(db.ErrNoEntries); ok {
break grabloop // we just don't have enough statuses left in the db so index what we've got and then bail
}
return fmt.Errorf("IndexBefore: error getting statuses from db: %s", err)
}
for _, s := range statuses {
timelineable, err := t.filter.StatusHometimelineable(s, t.account)
if err != nil {
continue
}
if timelineable {
filtered = append(filtered, s)
}
offsetStatus = s.ID
}
}
for _, s := range filtered {
if _, err := t.IndexOne(s.CreatedAt, s.ID, s.BoostOfID); err != nil {
return fmt.Errorf("IndexBefore: error indexing status with id %s: %s", s.ID, err)
}
}
return nil return nil
} }
@ -41,7 +80,7 @@ func (t *timeline) IndexBehind(statusID string, amount int) error {
for _, s := range filtered { for _, s := range filtered {
if _, err := t.IndexOne(s.CreatedAt, s.ID, s.BoostOfID); err != nil { if _, err := t.IndexOne(s.CreatedAt, s.ID, s.BoostOfID); err != nil {
return fmt.Errorf("IndexBehindAndIncluding: error indexing status with id %s: %s", s.ID, err) return fmt.Errorf("IndexBehind: error indexing status with id %s: %s", s.ID, err)
} }
} }

View file

@ -106,7 +106,10 @@ func (m *manager) Ingest(status *gtsmodel.Status, timelineAccountID string) (boo
"statusID": status.ID, "statusID": status.ID,
}) })
t := m.getOrCreateTimeline(timelineAccountID) t, err := m.getOrCreateTimeline(timelineAccountID)
if err != nil {
return false, err
}
l.Trace("ingesting status") l.Trace("ingesting status")
return t.IndexOne(status.CreatedAt, status.ID, status.BoostOfID) return t.IndexOne(status.CreatedAt, status.ID, status.BoostOfID)
@ -119,7 +122,10 @@ func (m *manager) IngestAndPrepare(status *gtsmodel.Status, timelineAccountID st
"statusID": status.ID, "statusID": status.ID,
}) })
t := m.getOrCreateTimeline(timelineAccountID) t, err := m.getOrCreateTimeline(timelineAccountID)
if err != nil {
return false, err
}
l.Trace("ingesting status") l.Trace("ingesting status")
return t.IndexAndPrepareOne(status.CreatedAt, status.ID) return t.IndexAndPrepareOne(status.CreatedAt, status.ID)
@ -132,7 +138,10 @@ func (m *manager) Remove(statusID string, timelineAccountID string) (int, error)
"statusID": statusID, "statusID": statusID,
}) })
t := m.getOrCreateTimeline(timelineAccountID) t, err := m.getOrCreateTimeline(timelineAccountID)
if err != nil {
return 0, err
}
l.Trace("removing status") l.Trace("removing status")
return t.Remove(statusID) return t.Remove(statusID)
@ -144,7 +153,10 @@ func (m *manager) HomeTimeline(timelineAccountID string, maxID string, sinceID s
"timelineAccountID": timelineAccountID, "timelineAccountID": timelineAccountID,
}) })
t := m.getOrCreateTimeline(timelineAccountID) t, err := m.getOrCreateTimeline(timelineAccountID)
if err != nil {
return nil, err
}
statuses, err := t.Get(limit, maxID, sinceID, minID) statuses, err := t.Get(limit, maxID, sinceID, minID)
if err != nil { if err != nil {
@ -154,7 +166,10 @@ func (m *manager) HomeTimeline(timelineAccountID string, maxID string, sinceID s
} }
func (m *manager) GetIndexedLength(timelineAccountID string) int { func (m *manager) GetIndexedLength(timelineAccountID string) int {
t := m.getOrCreateTimeline(timelineAccountID) t, err := m.getOrCreateTimeline(timelineAccountID)
if err != nil {
return 0
}
return t.PostIndexLength() return t.PostIndexLength()
} }
@ -164,13 +179,19 @@ func (m *manager) GetDesiredIndexLength() int {
} }
func (m *manager) GetOldestIndexedID(timelineAccountID string) (string, error) { func (m *manager) GetOldestIndexedID(timelineAccountID string) (string, error) {
t := m.getOrCreateTimeline(timelineAccountID) t, err := m.getOrCreateTimeline(timelineAccountID)
if err != nil {
return "", err
}
return t.OldestIndexedPostID() return t.OldestIndexedPostID()
} }
func (m *manager) PrepareXFromTop(timelineAccountID string, limit int) error { func (m *manager) PrepareXFromTop(timelineAccountID string, limit int) error {
t := m.getOrCreateTimeline(timelineAccountID) t, err := m.getOrCreateTimeline(timelineAccountID)
if err != nil {
return err
}
return t.PrepareFromTop(limit) return t.PrepareFromTop(limit)
} }
@ -198,11 +219,15 @@ func (m *manager) WipeStatusFromAllTimelines(statusID string) error {
return err return err
} }
func (m *manager) getOrCreateTimeline(timelineAccountID string) Timeline { func (m *manager) getOrCreateTimeline(timelineAccountID string) (Timeline, error) {
var t Timeline var t Timeline
i, ok := m.accountTimelines.Load(timelineAccountID) i, ok := m.accountTimelines.Load(timelineAccountID)
if !ok { if !ok {
t = NewTimeline(timelineAccountID, m.db, m.tc, m.log) var err error
t, err = NewTimeline(timelineAccountID, m.db, m.tc, m.log)
if err != nil {
return nil, err
}
m.accountTimelines.Store(timelineAccountID, t) m.accountTimelines.Store(timelineAccountID, t)
} else { } else {
t, ok = i.(Timeline) t, ok = i.(Timeline)
@ -211,5 +236,5 @@ func (m *manager) getOrCreateTimeline(timelineAccountID string) Timeline {
} }
} }
return t return t, nil
} }

View file

@ -125,16 +125,22 @@ type timeline struct {
} }
// NewTimeline returns a new Timeline for the given account ID // NewTimeline returns a new Timeline for the given account ID
func NewTimeline(accountID string, db db.DB, typeConverter typeutils.TypeConverter, log *logrus.Logger) Timeline { func NewTimeline(accountID string, db db.DB, typeConverter typeutils.TypeConverter, log *logrus.Logger) (Timeline, error) {
timelineOwnerAccount := &gtsmodel.Account{}
if err := db.GetByID(accountID, timelineOwnerAccount); err != nil {
return nil, err
}
return &timeline{ return &timeline{
postIndex: &postIndex{}, postIndex: &postIndex{},
preparedPosts: &preparedPosts{}, preparedPosts: &preparedPosts{},
accountID: accountID, accountID: accountID,
account: timelineOwnerAccount,
db: db, db: db,
filter: visibility.NewFilter(db, log), filter: visibility.NewFilter(db, log),
tc: typeConverter, tc: typeConverter,
log: log, log: log,
} }, nil
} }
func (t *timeline) Reset() error { func (t *timeline) Reset() error {

View file

@ -12,9 +12,8 @@
func (f *filter) StatusVisible(targetStatus *gtsmodel.Status, requestingAccount *gtsmodel.Account) (bool, error) { func (f *filter) StatusVisible(targetStatus *gtsmodel.Status, requestingAccount *gtsmodel.Account) (bool, error) {
l := f.log.WithFields(logrus.Fields{ l := f.log.WithFields(logrus.Fields{
"func": "StatusVisible", "func": "StatusVisible",
"statusID": targetStatus.ID, "statusID": targetStatus.ID,
"requestingAccountID": requestingAccount.ID,
}) })
relevantAccounts, err := f.pullRelevantAccountsFromStatus(targetStatus) relevantAccounts, err := f.pullRelevantAccountsFromStatus(targetStatus)
@ -49,6 +48,16 @@ func (f *filter) StatusVisible(targetStatus *gtsmodel.Status, requestingAccount
} }
} }
// If requesting account is nil, that means whoever requested the status didn't auth, or their auth failed.
// In this case, we can still serve the status if it's public, otherwise we definitely shouldn't.
if requestingAccount == nil {
if targetStatus.Visibility == gtsmodel.VisibilityPublic {
return true, nil
}
l.Trace("requesting account is nil but the target status isn't public")
return false, nil
}
// if the requesting user doesn't exist (anymore) then the status also shouldn't be visible // if the requesting user doesn't exist (anymore) then the status also shouldn't be visible
// note: we only do this for local users // note: we only do this for local users
if requestingAccount.Domain == "" { if requestingAccount.Domain == "" {
@ -68,16 +77,6 @@ func (f *filter) StatusVisible(targetStatus *gtsmodel.Status, requestingAccount
} }
} }
// If requesting account is nil, that means whoever requested the status didn't auth, or their auth failed.
// In this case, we can still serve the status if it's public, otherwise we definitely shouldn't.
if requestingAccount == nil {
if targetStatus.Visibility == gtsmodel.VisibilityPublic {
return true, nil
}
l.Trace("requesting account is nil but the target status isn't public")
return false, nil
}
// if requesting account is suspended then don't show the status -- although they probably shouldn't have gotten // if requesting account is suspended then don't show the status -- although they probably shouldn't have gotten
// this far (ie., been authed) in the first place: this is just for safety. // this far (ie., been authed) in the first place: this is just for safety.
if !requestingAccount.SuspendedAt.IsZero() { if !requestingAccount.SuspendedAt.IsZero() {