mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2024-08-29 22:06:06 +02:00
GetFeeds must always discard actions with dangling repo_id (#19598)
* GetFeeds must always discard actions with dangling repo_id See https://discourse.gitea.io/t/blank-page-after-login/5051/12 for a panic in 1.16.6. * add comment to explain the dangling ID in the fixture * loadRepoOwner must not attempt to use a nil action.Repo * make fmt Co-authored-by: Loïc Dachary <loic@dachary.org>
This commit is contained in:
parent
04fc4b7e05
commit
b536b65189
5 changed files with 37 additions and 7 deletions
|
@ -340,14 +340,14 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, error) {
|
|||
}
|
||||
|
||||
e := db.GetEngine(ctx)
|
||||
sess := e.Where(cond)
|
||||
sess := e.Where(cond).Join("INNER", "repository", "`repository`.id = `action`.repo_id")
|
||||
|
||||
opts.SetDefaultValues()
|
||||
sess = db.SetSessionPagination(sess, &opts)
|
||||
|
||||
actions := make([]*Action, 0, opts.PageSize)
|
||||
|
||||
if err := sess.Desc("created_unix").Find(&actions); err != nil {
|
||||
if err := sess.Desc("`action`.created_unix").Find(&actions); err != nil {
|
||||
return nil, fmt.Errorf("Find: %v", err)
|
||||
}
|
||||
|
||||
|
@ -417,7 +417,7 @@ func activityQueryCondition(opts GetFeedsOptions) (builder.Cond, error) {
|
|||
}
|
||||
|
||||
if !opts.IncludePrivate {
|
||||
cond = cond.And(builder.Eq{"is_private": false})
|
||||
cond = cond.And(builder.Eq{"`action`.is_private": false})
|
||||
}
|
||||
if !opts.IncludeDeleted {
|
||||
cond = cond.And(builder.Eq{"is_deleted": false})
|
||||
|
@ -430,8 +430,8 @@ func activityQueryCondition(opts GetFeedsOptions) (builder.Cond, error) {
|
|||
} else {
|
||||
dateHigh := dateLow.Add(86399000000000) // 23h59m59s
|
||||
|
||||
cond = cond.And(builder.Gte{"created_unix": dateLow.Unix()})
|
||||
cond = cond.And(builder.Lte{"created_unix": dateHigh.Unix()})
|
||||
cond = cond.And(builder.Gte{"`action`.created_unix": dateLow.Unix()})
|
||||
cond = cond.And(builder.Lte{"`action`.created_unix": dateHigh.Unix()})
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -80,6 +80,9 @@ func (actions ActionList) loadRepoOwner(e db.Engine, userMap map[int64]*user_mod
|
|||
}
|
||||
|
||||
for _, action := range actions {
|
||||
if action.Repo == nil {
|
||||
continue
|
||||
}
|
||||
repoOwner, ok := userMap[action.Repo.OwnerID]
|
||||
if !ok {
|
||||
repoOwner, err = user_model.GetUserByID(action.Repo.OwnerID)
|
||||
|
|
|
@ -211,3 +211,20 @@ func TestNotifyWatchers(t *testing.T) {
|
|||
OpType: action.OpType,
|
||||
})
|
||||
}
|
||||
|
||||
func TestGetFeedsCorrupted(t *testing.T) {
|
||||
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}).(*user_model.User)
|
||||
unittest.AssertExistsAndLoadBean(t, &Action{
|
||||
ID: 8,
|
||||
RepoID: 1700,
|
||||
})
|
||||
|
||||
actions, err := GetFeeds(db.DefaultContext, GetFeedsOptions{
|
||||
RequestedUser: user,
|
||||
Actor: user,
|
||||
IncludePrivate: true,
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
assert.Len(t, actions, 0)
|
||||
}
|
||||
|
|
|
@ -56,3 +56,11 @@
|
|||
repo_id: 8 # public
|
||||
is_private: false
|
||||
created_unix: 1603011540 # grouped with id:7
|
||||
|
||||
- id: 8
|
||||
user_id: 1
|
||||
op_type: 12 # close issue
|
||||
act_user_id: 1
|
||||
repo_id: 1700 # dangling intentional
|
||||
is_private: false
|
||||
created_unix: 1603011541
|
||||
|
|
|
@ -175,9 +175,11 @@ func init() {
|
|||
|
||||
checkForActionConsistency := func(t assert.TestingT, bean interface{}) {
|
||||
action := reflectionWrap(bean)
|
||||
if action.int("RepoID") != 1700 { // dangling intentional
|
||||
repoRow := AssertExistsAndLoadMap(t, "repository", builder.Eq{"id": action.int("RepoID")})
|
||||
assert.Equal(t, parseBool(repoRow["is_private"]), action.bool("IsPrivate"), "action: %+v", action)
|
||||
}
|
||||
}
|
||||
|
||||
consistencyCheckMap["user"] = checkForUserConsistency
|
||||
consistencyCheckMap["repository"] = checkForRepoConsistency
|
||||
|
|
Loading…
Reference in a new issue