Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check Mirror exists before linking its Repo #20840

Merged
merged 2 commits into from
Aug 18, 2022
Merged

Check Mirror exists before linking its Repo #20840

merged 2 commits into from
Aug 18, 2022

Conversation

zeripath
Copy link
Contributor

In MirrorRepositoryList.loadAttributes there is some code to load the Mirror entries
from the database. This assumes that every Repository which has IsMirror set has
a Mirror associated in the DB. This association is incorrect in the case of
Mirror repository under creation when there is no Mirror entry in the DB until
completion.

Unfortunately LoadAttributes makes this incorrect assumption and presumes that a
Mirror will always be loaded. This then causes a panic.

This PR simply double checks if there a Mirror before attempting to link back to
its Repo. Unfortunately it should be expected that there may be other cases where
this incorrect assumption causes further problems.

Fix #20804

Signed-off-by: Andrew Thornton art27@cantab.net

@zeripath
Check Mirror exists before linking its Repo
9bacd90 
In MirrorRepositoryList.loadAttributes there is some code to load the Mirror entries
from the database. This assumes that every Repository which has IsMirror set has
a Mirror associated in the DB. This association is incorrect in the case of
Mirror repository under creation when there is no Mirror entry in the DB until
completion.

Unfortunately LoadAttributes makes this incorrect assumption and presumes that a
Mirror will always be loaded. This then causes a panic.

This PR simply double checks if there a Mirror before attempting to link back to
its Repo. Unfortunately it should be expected that there may be other cases where
this incorrect assumption causes further problems.

Fix go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added this to the 1.18.0 milestone Aug 17, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Aug 18, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 18, 2022
@lunny lunny merged commit 03df7d0 into go-gitea:main Aug 18, 2022
lunny added a commit to lunny/gitea that referenced this pull request Aug 18, 2022
@zeripath @lunny
Check Mirror exists before linking its Repo (go-gitea#20840)
cc347b1 
In MirrorRepositoryList.loadAttributes there is some code to load the Mirror entries
from the database. This assumes that every Repository which has IsMirror set has
a Mirror associated in the DB. This association is incorrect in the case of
Mirror repository under creation when there is no Mirror entry in the DB until
completion.

Unfortunately LoadAttributes makes this incorrect assumption and presumes that a
Mirror will always be loaded. This then causes a panic.

This PR simply double checks if there a Mirror before attempting to link back to
its Repo. Unfortunately it should be expected that there may be other cases where
this incorrect assumption causes further problems.

Fix go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@lunny lunny mentioned this pull request Aug 18, 2022
Merged
@lunny lunny added the backport/done All backports for this PR have been created label Aug 18, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 18, 2022
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
d3256eb 
* giteaofficial/main:
  Fix migration file name (go-gitea#20843)
  Check Mirror exists before linking its Repo (go-gitea#20840)
  [skip ci] Updated translations via Crowdin
  Add badge capabilities to users (go-gitea#20607)
  docs[zh-cn]: Managing Deployments With Environment Variables (go-gitea#20817)
  Correctly escape within tribute.js (go-gitea#20831)
  Fix panic when an invalid oauth2 name is passed (go-gitea#20820)
  Use the total issue count for UI (go-gitea#20785)
lafriks pushed a commit that referenced this pull request Aug 18, 2022
@lunny @zeripath
Check Mirror exists before linking its Repo (#20840) (#20842)
c40c753 
In MirrorRepositoryList.loadAttributes there is some code to load the Mirror entries
from the database. This assumes that every Repository which has IsMirror set has
a Mirror associated in the DB. This association is incorrect in the case of
Mirror repository under creation when there is no Mirror entry in the DB until
completion.

Unfortunately LoadAttributes makes this incorrect assumption and presumes that a
Mirror will always be loaded. This then causes a panic.

This PR simply double checks if there a Mirror before attempting to link back to
its Repo. Unfortunately it should be expected that there may be other cases where
this incorrect assumption causes further problems.

Fix #20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: zeripath <art27@cantab.net>
zeripath added a commit to zeripath/gitea that referenced this pull request Aug 18, 2022
@zeripath
Remove calls to load Mirrors in user.Dashboard
6226355 
Whilst looking at go-gitea#20840 I noticed that the Mirrors data doesn't appear
to be being used therefore we can remove this and in fact none of the
related code is used elsewhere so it can also be removed.

Related go-gitea#20840
Related go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Aug 18, 2022
Merged
@zeripath zeripath deleted the fix-20804-prevent-panic-for-non-existent-mirror branch August 18, 2022 18:40
techknowlogick pushed a commit that referenced this pull request Aug 19, 2022
@zeripath
Remove calls to load Mirrors in user.Dashboard (#20855)
68f3aae 
Whilst looking at #20840 I noticed that the Mirrors data doesn't appear
to be being used therefore we can remove this and in fact none of the
related code is used elsewhere so it can also be removed.

Related #20840
Related #20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit to zeripath/gitea that referenced this pull request Aug 21, 2022
@zeripath
Remove calls to load Mirrors in user.Dashboard (go-gitea#20855)
9192e3d 
Backport go-gitea#20855

Whilst looking at go-gitea#20840 I noticed that the Mirrors data doesn't appear
to be being used therefore we can remove this and in fact none of the
related code is used elsewhere so it can also be removed.

Related go-gitea#20840
Related go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Aug 21, 2022
Merged
lunny added a commit that referenced this pull request Aug 22, 2022
@zeripath @lunny
Remove calls to load Mirrors in user.Dashboard (#20855) (#20897)
ec9b43b 
Backport #20855

Whilst looking at #20840 I noticed that the Mirrors data doesn't appear
to be being used therefore we can remove this and in fact none of the
related code is used elsewhere so it can also be removed.

Related #20840
Related #20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 28, 2022
@zeripath @lunny
Check Mirror exists before linking its Repo (go-gitea#20840)
1eb6443 
In MirrorRepositoryList.loadAttributes there is some code to load the Mirror entries
from the database. This assumes that every Repository which has IsMirror set has
a Mirror associated in the DB. This association is incorrect in the case of
Mirror repository under creation when there is no Mirror entry in the DB until
completion.

Unfortunately LoadAttributes makes this incorrect assumption and presumes that a
Mirror will always be loaded. This then causes a panic.

This PR simply double checks if there a Mirror before attempting to link back to
its Repo. Unfortunately it should be expected that there may be other cases where
this incorrect assumption causes further problems.

Fix go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 28, 2022
@zeripath
Remove calls to load Mirrors in user.Dashboard (go-gitea#20855)
1c3b60e 
Whilst looking at go-gitea#20840 I noticed that the Mirrors data doesn't appear
to be being used therefore we can remove this and in fact none of the
related code is used elsewhere so it can also be removed.

Related go-gitea#20840
Related go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Gusted Gusted Gusted approved these changes

@lunny lunny lunny approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Milestone
1.18.0
Development

Successfully merging this pull request may close these issues.

Clicking on an organization dashboard during a warehouse migration will report a 500 error
4 participants
@zeripath @lunny @Gusted @GiteaBot