GetFeeds must always discard actions with dangling repo_id #19598
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6543
reviewed
May 3, 2022
GiteaBot
added
the
lgtm/need 2
6543
approved these changes
May 3, 2022
GiteaBot
added
lgtm/need 1
Gusted
approved these changes
May 3, 2022
GiteaBot
added
lgtm/done
6543
requested changes
May 3, 2022
singuliere
force-pushed
the
forgefriends-mr48
branch
from
b41ab10 to
bf728b8
Compare
I agree that it feels like a hack. Now that the tests are passing I'll try to follow your suggestion. My concern is that returning an action with a nil Repo member is likely to set a trap for future code using the Action struct that goes undetected because it is a rare occurrence and not covered by the current tests. |
singuliere
force-pushed
the
forgefriends-mr48
branch
from
a691314 to
4b4da6b
Compare
|
@Gusted Added a safeguard so loadRepoOwner is no longer susceptible to panic on action.Repo == nil |
singuliere
force-pushed
the
forgefriends-mr48
branch
from
fae8c19 to
3fc19d8
Compare
|
please |
|
@singuliere please refrain from force pushes, it makes it hard to review PRs |
This comment was marked as off-topic.
This comment was marked as off-topic.
lunny
approved these changes
May 4, 2022
6543
approved these changes
May 4, 2022
|
@singuliere please send backport :) |
singuliere
added a commit
to singuliere/gitea
that referenced
this pull request
May 6, 2022
…19598) Co-authored-by: Loïc Dachary <[email protected]> (cherry picked from commit b536b65) Conflicts: models/action_test.go The GetFeeds function does not have a Context argument in 1.16. models/action.go The SQL statement is essentially the same in 1.16 but structured differently. The Join() was copied and the created_unix field prefixed with `action`. models/action_list.go in 1.16 the loadRepoOwner method did not exist and it was done in the RetrieveFeeds method of web/feed/profile.go. The safeguard to skip when act.Repo == nil was moved there.
zjjhot
added a commit
to zjjhot/gitea
that referenced
this pull request
May 6, 2022
* giteaofficial/main: Simplify `IsVendor` (go-gitea#19626) Prevent NPE when checking repo units if the user is nil (go-gitea#19625) Skip duplicated layers. (go-gitea#19624) Add "Reference" section to Issue view sidebar (go-gitea#19609) GetFeeds must always discard actions with dangling repo_id (go-gitea#19598)
6543
pushed a commit
that referenced
this pull request
May 8, 2022
…19629) Co-authored-by: Loïc Dachary <[email protected]> (cherry picked from commit b536b65) Conflicts: models/action_test.go The GetFeeds function does not have a Context argument in 1.16. models/action.go The SQL statement is essentially the same in 1.16 but structured differently. The Join() was copied and the created_unix field prefixed with `action`. models/action_list.go in 1.16 the loadRepoOwner method did not exist and it was done in the RetrieveFeeds method of web/feed/profile.go. The safeguard to skip when act.Repo == nil was moved there.
AbdulrhmnGhanem
pushed a commit
to kitspace/gitea
that referenced
this pull request
Aug 24, 2022
…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 <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See original merge request from Loïc Dachary @ forgefriends
See https://discourse.gitea.io/t/blank-page-after-login/5051/12
for a panic in 1.16.6.
Fix #19631