No user avatars #13159
Comments
|
Are there any earlier logs? I wouldn't be surprised if there was some earlier information with an error for this. This is likely to do with recently merged avatar minio PR. |
|
gitea/routers/routes/routes.go Lines 152 to 157 in 25f7e1c The error is appearing here because the ctx.Error handler isn't actually available when this objStorage is running. Now the problem is that L153 here is returning an error that needs to be returned. The most likely is that the object does not exist. Now that should be handled with 404 not a 500. So we have two bugs:
|
|
gitea/routers/routes/routes.go Lines 210 to 216 in 25f7e1c My suspicion is that if we moved L211-L212 to after L214 we'd probably have the error reporting correctly instead of a panic. |
|
Full log of one dashboard refresh from release/v1.13: This problem was absent in earlier master we've been testing so probably errors comes from mentioned PR. |
|
How are you running gitea? |
|
Build in IDE (vscodium) from sources. Same problem when building using cli and make in other system. No such problems in same environments with earlier master versions so my env problably is not a problem. Can't you reproduce it in current master and avatar config as above? |
|
As I said above the most likely root cause is that the avatar storage can't find the appropriate avatar but quite why that should be is another q. The reality is that you shouldn't be ending up looking up avatars unless they're in your db and thence should be stored on the disk. The most likely cause for that issue is that the default avatar location is incorrect - (but it could be that it is in the right place but that your db has incorrect data.) This leads to another issue - there is no logging anywhere or admin readable configuration stating what the configuration is - so that also needs changing. Now why does that cause the panic? Well as I show above the ctx.Error occurs before that handler is even set - so we need to adjust this code. In fact we need to adjust it slightly more - because sending the err directly back to the user will leak information. So we need to log the error and display something else. Now actually not being able to find a file shouldn't cause a 500 it should be a 404 but this isn't handled currently. So we have 4 issues:
One potential 5th issue is that the default location is incorrect. |
|
OK so here's a patch that fixes the first 2: diff --git a/routers/routes/routes.go b/routers/routes/routes.go
index a09e53efc..c59188bdf 100644
--- a/routers/routes/routes.go
+++ b/routers/routes/routes.go
@@ -7,6 +7,7 @@ package routes
import (
"bytes"
"encoding/gob"
+ "fmt"
"io"
"net/http"
"path"
@@ -125,7 +126,8 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
rPath := strings.TrimPrefix(req.RequestURI, "/"+prefix)
u, err := objStore.URL(rPath, path.Base(rPath))
if err != nil {
- ctx.Error(500, err.Error())
+ log.Error("Error whilst getting URL for %s %s. Error: %v", prefix, rPath, err)
+ ctx.Error(500, fmt.Sprintf("Error whilst getting URL for %s %s", prefix, rPath))
return
}
http.Redirect(
@@ -152,14 +154,16 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
//If we have matched and access to release or issue
fr, err := objStore.Open(rPath)
if err != nil {
- ctx.Error(500, err.Error())
+ log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err)
+ ctx.Error(500, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath))
return
}
defer fr.Close()
_, err = io.Copy(ctx.Resp, fr)
if err != nil {
- ctx.Error(500, err.Error())
+ log.Error("Error whilst rendering %s %s. Error: %v", prefix, rPath, err)
+ ctx.Error(500, fmt.Sprintf("Error whilst rendering %s %s", prefix, rPath))
return
}
}
@@ -208,10 +212,9 @@ func NewMacaron() *macaron.Macaron {
},
))
+ m.Use(templates.HTMLRenderer())
m.Use(storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars))
m.Use(storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars))
-
- m.Use(templates.HTMLRenderer())
mailer.InitMailRender(templates.Mailer())
localeNames, err := options.Dir("locale") |
|
Where are you avatars actually stored on your system in comparison to your Gitea data directory? |
* The `.Use` of storageHandler before setting up the template renderer causes a panic if there is an error to log. * The error passed to `ctx.Error` in that case may contain sensitive information and should not be rendered to the end user. We should instead log the error and render a simple error message. * There is no handling of missing avatars and this needs a 404. Minio errors need to be mapped to standard golang errors such as os.ErrNotExist. * There is no logging when storage is set up. Related go-gitea#13159 Signed-off-by: Andrew Thornton <art27@cantab.net>
* The `.Use` of storageHandler before setting up the template renderer causes a panic if there is an error to log. * The error passed to `ctx.Error` in that case may contain sensitive information and should not be rendered to the end user. We should instead log the error and render a simple error message. * There is no handling of missing avatars and this needs a 404. Minio errors need to be mapped to standard golang errors such as os.ErrNotExist. * There is no logging when storage is set up. Related #13159 Signed-off-by: Andrew Thornton <art27@cantab.net>
|
Just tested that problem still exists in current (12a1f91) master; gitea is compiled from sources and initialized from non existing data directory and absent app.ini with web install (only db set to sqlite and gitea admin user data entered - no other changes) that creates initial app.ini with sqlite3 as db and default DISABLE_GRAVATAR = false in app.ini. Data dir and app.ini are created by gitea from scratch. After logging to admin account (created in install) with no problem with displaying this user avatar but it seems not served from local file because avatars folder is empty: Then org is created and its avatar is displayed correctly and PNG file "2" is created: Then gitea is stopped and DISABLE_GRAVATAR is changed false -> true in app.ini and gitea is started; now admin avatar is not displayed (empty image frame) and debug console contains errors as below (PANIC is not generated in master seems this was fixed in the meantime)... ...and no avatar file seems present: Problem does not depend on ENABLE_FEDERATED_AVATAR value (problem exist when true or false); only DISABLE_GRAVATAR = true seems to generate this problem. Tested that problem does NOT exist in 93f7525 and user avatar is created correctly in the same environment with DISABLE_GRAVATAR = true: Tested that problem DOES exits in 80a6b0f (and this version throws PANIC in debug console). Conclusions:
|
|
The reason why the panic is no longer there is because of the linked PR #13164 and its purpose was not to fix your problem but to help with debugging it. So one thing that #13164 does is actually put a lot more logging in to inform users where Gitea is looking for, for example you should now see in your logs at start-up: Storage Start-up Logswhich tells you where the storage is being created. But it also added logging when there is an error, which you have given me here: From which I can work out that Gitea has determined that the place to store avatars is: (interestingly the 404 code doesn't appear to be working in #13164) Now a link that would point to this should only be being made if and only if there is a custom or generated avatar in place. Lines 78 to 99 in 12a1f91 OK, so the question I asked earlier still applies:
If those are the same place - then what did you previously see? |
|
By the way it appears that your logging is a little weird. What is its configuration? If you don't want colored logs set |
All not mentioned app.ini/gitea settings were default during above tests. Above ls outputs clearly show data/avatars as folder with avatars.
Yes.
No. Avatars appeared in /myhome/code/src/github.com/go-gitea/gitea/data/avatars as in mentioned ls output.
When problem occurs, folder avatars does not contain 7112ca6ffcdbc82b1a4735aec33678c6 which is avatar file that should be automatically generated by gitea when DISABLE_GRAVATAR = true but is not. Please see my ls outputs above. |
This mod fixes problem with initial avatar autogeneration and avatar autogneration after deleting previous avatar. Related: go-gitea#13159 Fixes: 80a6b0f Author-Change-Id: IB#1105243
This mod fixes problem with initial avatar autogeneration and avatar autogneration after deleting previous avatar. Related: go-gitea#13159 Fixes: 80a6b0f Author-Change-Id: IB#1105243
|
Problem is caused by non empty user.Avatar value that is generated on user create, before random avatar generation. Changed random generation procedure does not generate avatar if user.Avatar is non empty. More - deleting Avatar also don't make user.Avatar empty string. Fix works for us for both problems. |
This mod fixes problem with initial avatar autogeneration and avatar autogneration after deleting previous avatar. Related: go-gitea#13159 Fixes: 80a6b0f Author-Change-Id: IB#1105243
Gitea build from release/v1.13 and master with
do not show avatars for users and throws internal server error (500) on avatar image get with log:
No such problem with org avatars with same config.
No such problem with user avatars with default settings:
The text was updated successfully, but these errors were encountered: