template error in issues list when poster_id is -1

[ashimokawa]

• Gitea version (or commit ref): 1.10.0+14-gc01afd584
• Git version: 2.20.1
• Operating system: Debian 10
• Database:
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL)
  • No
  • Not relevant
• Log gist:

template: repo/issue/list:236:26: executing "repo/issue/list" at <.Poster.ID>: nil pointer evaluating *models.User.ID

Description

Deleted owners issues sometimes get poster_id=-1 in issue table, leading to a template error in ./templates/repo/issue/list.tmpl in the issue list

This happened for 5 issues on codeberg.org, in an organization where users create one-time accounts for one single interaction, then delete it (I won't comment on that, not my idea), but that fluctuation seems to make the error much more likely.

I am willing to fix the template IF you tell me that -1 is expected, if it is not then we have to find out why poster_id gets set to -1. Or is it rather a bug that deleted users poster_id do NOT get to -1?

[ashimokawa]

I have a hunch:

I could happen when issues, which were created by deleted users get closed by someone in the same organization.

that's not the case 😑

[ashimokawa]

can't reproduce but happens.
Best is probably to clarify how expected behavior for deleted users is regarding -1 vs dangeling user ID.

An admin on try could also query the issue table...

select count(*) from issue where poster_id=-1

[lunny]

If users' id <= 0, then it will be replaced as a ghost user.

[ashimokawa]

@lunny
No if poster_id is -1 the above error will occur in the template

[ashimokawa]

Also dangeling ids will be replaced by ghost, only problem is -1, which keeps occuring.

When I change all -1 to 100000000 the template works

[lunny]

So this should be a bug.

[ashimokawa]

@lunny

Yes, but what is the bug?

1. The template should work with -1 as poster_id, it does not. that's the bug.

If that is true, why does poster_id of deleted users stays intact in issue table except for rare cases where it is set to -1.

OR

2. The bug is that poster_id becomes -1, that should not happen

[techknowlogick]

The poster_id should not be < 0, as template is just saying that it received nil as a user which means that what is being supplied to the template is incorrect. As when ghost is provided that is technically a valid user, so template is ok assuming everything passed to it won’t be nil. If I’m a bit rambly, the -1 id is the bug, not the template.

[ashimokawa]

@techknowlogick
This was exactly my question. I would have provided a PR to make the template work with -1, but now I know better. Thanks 👍 .

It does not happen often but it happens that poster_ids become -1, after a user deletes himself.
I had a look at the code but I did not find the cause. As I said earlier we have users on codeberg who - no kidding - create an account for every single issue, then delete it seconds after opening the issue. I also looked for -1 in the comments table but there was none - only in the issue table.

EDIT:
If you are an admin on try.gitea.io would be cool if you could query the database for -1

[techknowlogick]

@ashimokawa both options work for a solution, just one is better as with template it would just hide the underlying problem.

I just searched gitea.com and try.gitea.io DB and couldn't find any poster_id of -1.

[ashimokawa]

@techknowlogick
Thanks for doing the query!

I also did not change the template locally to be able to see if the problem re-occurs. My workaround to to set the id to some very high magic value for those five entries.

[ashimokawa]

Damn, this just happened again (the same repo from the "I delete my account for every singe interaction" guys).

https://codeberg.org/crimeflare/cloudflare-tor/issues?q=&type=all&sort=&state=closed&labels=0&milestone=0&assignee=0

`
MariaDB [gitea]> select count(*) from issue where poster_id=-1;
+----------+
| count(*) |
+----------+
| 2 |
+----------+
1 row in set (0.001 sec)

`

[mrsdizzie]

OK so the problem here is that .Poster.ID is nil. There might be some confusion over the issue value of poster_id (would be PosterID in template) with the .ID field of the pointer to a *user model contained in Poster value of the issue (or .Poster.ID in the template).

So in your error what is happening is that the ID of the Poster is nil (looks like Poster itself is nil). The template itself doesn't care (or even check) what poster_id of the issue is, however when creating a list of issues Poster ends up nil if the poster_id is <= 0 :

gitea/models/issue_list.go

Lines 98 to 105 in e80fe20

	for _, issue := range issues {
	if issue.PosterID <= 0 {
	continue
	}
	var ok bool
	if issue.Poster, ok = posterMaps[issue.PosterID]; !ok {
	issue.Poster = NewGhostUser()
	}

So you can't really change the template to "deal with -1" because it doesn't have a problem with that directly. The template expects Poster to be a valid pointer to a User model (regular or ghost) as it uses several fields from that *User model directly and if it is nil will fail.

As suggested, poster_id of an issue shouldn't get set to -1 when somebody deletes their account.

I'm unable to reproduce that happening, and it sounds like you are also unable to reproduce but are seeing it happen on the live site somehow.

Do you happen to have any combination of Gitea and SQL logs that suggest if the poster_id was changed from something valid to -1 based on a certain action?

Or if perhaps the issue itself was somehow created with a poster_id of -1 initially?

If not if you could enable that type of logging it should at least be possible to see what sets the poster_id of a particular issue to -1 and hopefully that provides more insight into where to look 👀

[lunny]

OK. Maybe we should

ghostUser := NewGhostUser() 
for _, issue := range issues { 
 	if issue.PosterID <= 0 {
+               issue.Poster = ghostUser
 		continue 
 	} 
 	var ok bool 
 	if issue.Poster, ok = posterMaps[issue.PosterID]; !ok { 
- 		issue.Poster = NewGhostUser() 
+ 		issue.Poster = ghostUser 
 	}