Skip to content

Repository owner can be nil if deleted and that breaks the listing of repositories. #24857

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.

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

Open
sergiotarxz opened this issue May 22, 2023 · 6 comments
Labels

Comments

@sergiotarxz
Copy link

sergiotarxz commented May 22, 2023

Description

I do not know if it is still posible to reach a point where deleting a owner does not delete its repositories, but is was once possible and failure to handle that breaks the lists of repositories.

Before patching it myself to avoid the bug.- By not listing repositories without owner.- this url would throw a 500 error.
https://git.owlcode.tech/explore/repos?page=2&sort=recentupdate&q=&topic=false&language=&only_show_relevant=false

2023/05/16 18:17:38 ...s/context/context.go:263:HTML() [E] [6463aca2] Render failed: template: explore/repo_list:29:19: executing "explore/repo_list" at <.Owner.Visibility.IsPrivate>: nil pointer evaluating *user.User.Visibility
        in template file (builtin) templates/explore/repo_list.tmpl (subtemplate of explore/repos):
        ----------------------------------------------------------------------
                                                                        <span class="ui basic label">{{$.locale.Tr "repo.desc.private"}}</span>
                                                                {{else}}
                                                                        {{if .Owner.Visibility.IsPrivate}}
                                                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
        ----------------------------------------------------------------------

Gitea Version

1.19.3

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

2.39.3

Operating System

Gentoo GNU/Linux

How are you running Gitea?

Built from portage renaming the ebuild to point the lastest upstream version to see if that solved the bug, it did not.

I have added this patch to /etc/portage/patches to avoid the bug:

diff --git a/routers/web/explore/repo.go b/routers/web/explore/repo.go
index be5ad1b..79db4a9 100644
--- a/routers/web/explore/repo.go
+++ b/routers/web/explore/repo.go
@@ -113,6 +113,14 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) {
 		IncludeDescription: setting.UI.SearchRepoDescription,
 		OnlyShowRelevant:   opts.OnlyShowRelevant,
 	})
+	old_repos := repos
+	repos = nil
+	for _, item := range old_repos {
+		if item.Owner != nil {
+			repos = append(repos, item)
+		}
+	}
+
 	if err != nil {
 		ctx.ServerError("SearchRepository", err)
 		return

Database

PostgreSQL

@sergiotarxz
Copy link
Author

I can make a pull request with this patch or one avoiding to retrieve in RenderRepoSearch a empty owner repo in the first place whatever you do think is more elegant.

@sergiotarxz
Copy link
Author

sergiotarxz commented May 22, 2023

I think a user containing all the orphan repositories would be the ideal thing, but I have deleted users because many of them where spam accounts and in those cases their repositories are spam also.

PD: I saw that such thing exist and it is called ghostuser, maybe RenderRepoSearch should return ghostuser for Owner = nil, althought I am not really sure if that makes sense.

@lunny
Copy link
Member

lunny commented May 23, 2023

It's impossible because there is a transaction on Gitea side. I think the correct method is to run ./gitea doctor to fix the broken problem but not patch and patch. Otherwise, many objects could be nil.

@JakobDev
Copy link
Contributor

I agree that gitea doctor should run to fix the Problem, but I also think that Gitea should not crash in such cases. This could mean, that the Explore Page on an Gitea instance becomes unusable for all until a Admin realises the problem and steps in. There might be also cases, where this happens on a larger Instance and the Broken Repo is at Page 10 or can be found with a Search, so the Admin don't realises this immediately. Many Users will not report to the Admin, if the Site crashes when searching for something, so avoiding a rash is always a good thing.

@lunny
Copy link
Member

lunny commented May 23, 2023

I agree that gitea doctor should run to fix the Problem, but I also think that Gitea should not crash in such cases. This could mean, that the Explore Page on an Gitea instance becomes unusable for all until a Admin realises the problem and steps in. There might be also cases, where this happens on a larger Instance and the Broken Repo is at Page 10 or can be found with a Search, so the Admin don't realises this immediately. Many Users will not report to the Admin, if the Site crashes when searching for something, so avoiding a rash is always a good thing.

This situation is not by design, so it should return 500. Other problems, like issues' author deleted, it will be displayed as ghost which is by design.

I don't think it can be resolved simply. If the UI isn't crash, the problem cannot be found. And even if you ignore that on search or list UI, when click in the detail page or any other repo page, it will return 500 still. If you want to fix all of them, it's a nightmare.

@sergiotarxz
Copy link
Author

I will run gitea doctor. Where is this documented? How could we make admins more aware that the real issue is the failure to run gitea doctor?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants