Optimization of labels handling in issue_search

[xlii-chl]

This PR enhances the labels handling in issue_search by optimizing the SQL query and de-duplicating the ids when generating the query string.

Those patches are backportable to v1.20.2 as is.

---

Background

For some time now, BingBot and some other crawlers have been putting my Gitea instance on its knees with requests containing a lot of label ids, like this one :

[07/Aug/2023:11:28:37 +0200] "GET /Dolibarr/sendrecurringinvoicebymail/issues?q=&type=all&sort=&state=closed&labels=1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c2%2c10%2c2%2c1%2c1%2c10%2c10%2c7%2c6%2c10%2c10%2c3%2c2%2c1%2c5%2c10%2c1%2c6%2c2%2c7%2c3%2c7%2c6%2c10%2c1%2c10%2c1%2c1%2c7%2c7%2c1%2c1%2c1%2c1%2c10%2c10%2c1%2c2%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c1%2c2%2c1%2c12%2c6%2c6%2c10&milestone=0&project=-1&poster=0 HTTP/1.1" 499 0 "-" "Mozilla/5.0 AppleWebKit/537.36 (KHTML, like Gecko; compatible; bingbot/2.0; +http://www.bing.com/bingbot.htm) Chrome/103.0.5060.134 Safari/537.36"

Since each of the label ids implies a join, it grows exponentially expensive for the database engine (at least on PostgreSQL but SQLite suffers a little too).

Thus this PR proposes two enhancements:

• rewrite the database query to use only one squashed condition,
• deduplicate the label ids when generating the URL.

This is my first try at Golang so don't hesitate to point mistakes or inelegant code.

As a side note, I unfortunately don't have the start of this snowball in my webserver logs anymore, and couldn't really reproduce it (the labels in the above project don't have scopes, and the label id n°1 that we can see in the query string is linked to a deleted repository (but its labels still exist in database apparently)).

[xlii-chl]

Rewrote using new Slices.Sort([]int64) & Compact([]int64) functions from Go 1.21.

[KN4CK3R]

Just use code.gitea.io/gitea/modules/container.Set. Then you don't need to sort and compact.

[xlii-chl]

Thanks for the review ! However, I used container.Set in my previous iteration and the code was less readable and the output's ordering was random (container.Set uses map).
I still use it in models/issues/issue_search.go where its Add() method and the on-the-fly de-duplication are quite handy.

[KN4CK3R]

Didn't see that the output is not just used in the backend. Then you could just use the set with slices.Sort at the end but that is only better if labelQuerySlice is very large.

[wxiaoguang]

I think KN4CK3R 's suggestion is right. Managed to do some rewriting: #31497

[xlii-chl]

This pull-request doesn't seem to draw interest so I guess I'll be closing it in a few weeks.

However, while not a security issue strictly speaking, I believe it can become a problem on loosely configured instances, and even help for a DoS. For example, on the quite-strictly-configured try.gitea.io instance with a limit at ~60 joins (sqlite ?) :

~$ time curl -s -o /dev/null 'https://try.gitea.io/czygsx/Gitea_2/issues?q=&type=all&sort=&state=open&labels=15516%2c15515&milestone=0&project=0&assignee=0&poster=0'

real	0m0,444s
user	0m0,105s
sys	0m0,021s


~$ time curl -s -o /dev/null 'https://try.gitea.io/czygsx/Gitea_2/issues?q=&type=all&sort=&state=open&labels=15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516%2c15515%2c15516&milestone=0&project=0&assignee=0&poster=0'
real	0m1,431s
user	0m0,116s
sys	0m0,013s

I hope you'll review it once more, and even if not, thanks for this awesome forge.

[lunny]

Sorry missed this one. It seems interesting. I will take a look at next week.

[techknowlogick]

Sorry, this PR got lost in the backlog. I'm currently going through the list of PRs still open and looking at getting them in.

[wxiaoguang]

Sorry, this PR got lost in the backlog. I'm currently going through the list of PRs still open and looking at getting them in.

But it seems that there are still some nits unresolved, including @KN4CK3R 's concern. #26460 (review)

[wxiaoguang]

-> Refactor issue label selection #31497

[wxiaoguang]

@xlii-chl I saw that you said "weird ? The PR was sleeping for a year".

Since the hard-fork community is quite hostility to me, I could only answer your question here.

Why this PR becomes stale?

Because the review suggestion is not resolved and you didn't respond for long time, and the code style doesn't quite match Golang standard. Indeed it needs more work.

If you could answer the review suggestion (either apply it or explain why not doing that), it could move on easily.

What happened to the hard-fork?

Well, that's a long story, in short:

• They keeps criticizing Gitea community/maintainers and made a lot of lies, and they copied a lot of Gitea's feature and publicize that "they are their work" (for example: Gitea Actions).
• And be careful about their cherry-picks and PRs, I am sure there are not only one security problems (include XSS, permission bypass) in their codebase but not in Gitea.

---

• They keeps making false statements, for example: rework long-term authentication #27455 (comment) , for the contributing guideline, it has been there for 7 years, they have been kindly warned not only one time, but they keeps saying that "Oh I don't know" and try to catch every chance to criticize Gitea.

• They keeps claiming that "their test is good" but "Gitea merge large untested code", but their PRs to Gitea didn't have tests, eg: Use correct tab value for repo search #27753 and MinIO > 2021 interprets a / prefix differently #26841, and there are even more. There was no test, and even worse, they are incorrect.

And here are why many of theirs PRs to Gitea were not merged mainly because there are bugs and they do not respond.

Let's see why some of them are not merged (I only picked some my reviews):

Really suggest everyone to take a deep look ........

• Downgrade @github/combobox-nav #28541 (comment)
• Improve display of Webauthn keys' creation time #28262 (comment)
• Add cancel button to wiki #28261 (comment)
• Prevent multiple codecomments to be submitted #28139 (review)
• Drop sha256-simd in favor of stdlib #27895 (comment)
• Use correct tab value for repo search #27753 (review)
• rework long-term authentication #27455 (comment)
• tidy up archive modal #27131 (comment)
• MinIO > 2021 interprets a / prefix differently #26841 (comment)
• maintain aspect ratio for auth icons #26488 (comment)
• make filename accessible in dropzone #26487 (comment)

[lunny]

Let's not backport this since it's not a bug fix.