Prevent NPE when cache service is disabled

[zeripath]

The cache service can be disabled - at which point ctx.Cache will be nil
and the use of it will cause an NPE.

The main part of this PR is that the cache is used for restricting
resending of activation mails and without this we cache we cannot
restrict this. Whilst this code could be re-considered to use the db and
probably should be, I think we can simply disable this code in the case
that the cache is disabled.

There are also several bug fixes in the /nodeinfo API endpoint.

Fix #18749

Signed-off-by: Andrew Thornton [email protected]

[6543]

I think we should create some "noop cache" that we would just can use instead if option is filped

as fix it's ok

[Gusted]

I don't feel comfortable approving this PR, this feels too error-prone to have such NPE against because we forgot we need to check if CacheService is enabled. @6543 suggestions seems reasonable.

[lunny]

Maybe you mean #19708

[wxiaoguang]

IIRC this problem has been discussed in discord long time ago.

The agreement at that time was: always use a builtin cache if there is no external cache (eg: use the builtin cache called twoqueue) , or make some logic (email sending protection) work correctly without cache.

Around 02/14/2022:

02/14/2022
Oh, that's reasonable, but it might become an non-obvious knowledge to end users.
That's why I propose to force the cache to be enabled with fallback.
Otherwise if someone disables the cache by mistake, then spam emails would become a security problem. I would be more careful about such case.

02/14/2022
yeah ... then we do the rest of your plan - 2. 2Q as default + (memory = twoqueue with memory-legacy for the old memory), and 3. we make this work properly

[6543]

we do have https://gitea.com/go-chi/cache/src/branch/master/memcache/memcache.go

[6543]

@wxiaoguang we do have memcache enabled by default! https://docs.gitea.io/en-us/config-cheat-sheet/#cache-cache

[wxiaoguang]

@wxiaoguang we do have memcache enabled by default! https://docs.gitea.io/en-us/config-cheat-sheet/#cache-cache

Yes, but the panic is caused by some users who disable the cache manually. The related issue has the details:

• Setting cache.ENABLED=false causes 500 panic #18749
• Cannot access <domain>/user/settings/account on fresh installation #19240
• Panic (crash) when requesting password without configured cache #19701

That's why the agreement on 02/14/2022 came: either always enable the cache (remove the option from app.ini), or make all logic correct without cache (protect users from email spam attacking even if there is no cache)

---

(no blocker from my side, I just try to mention the plan and risk which has been discussed before, while always using a builtin cache as default is the best choice in my mind).

[lunny]

@wxiaoguang we do have memcache enabled by default! https://docs.gitea.io/en-us/config-cheat-sheet/#cache-cache

Yes, but the panic is caused by some users who disable the cache manually. The related issue has the details:

• Setting cache.ENABLED=false causes 500 panic #18749
• Cannot access /user/settings/account on fresh installation #19240
• Panic (crash) when requesting password without configured cache #19701

That's why the agreement on 02/14/2022 came: either always enable the cache (remove the option from app.ini), or make all logic correct without cache (protect users from email spam attacking even if there is no cache)

(no blocker from my side, I just try to mention the plan and risk which has been discussed before, while always using a builtin cache as default is the best choice in my mind).

Hm, the email part should be refactor.

[zeripath]

The NPEs have been useful for spotting bugs in the past.

For example the problems in nodeinfo