make avatar lookup occur at image request #10540
Conversation
speed up page generation by making avatar lookup occur at the browser not at page generation
|
This creates a new endpoint |
GiteaBot
added
the
lgtm/need 2
|
This will expose email address even if it is set to be private |
|
@lafriks that's not true.
gitea/modules/repository/commits.go Lines 111 to 141 in 82be59e Lines 223 to 235 in 82be59e gitea/templates/repo/commit_page.tmpl Lines 33 to 53 in 82be59e gitea/templates/repo/commit_page.tmpl Lines 78 to 99 in 82be59e gitea/templates/repo/commits_list.tmpl Lines 17 to 24 in 82be59e gitea/templates/repo/commits_list.tmpl Lines 52 to 61 in 82be59e gitea/templates/repo/view_list.tmpl Lines 5 to 17 in 82be59e gitea/templates/repo/view_list.tmpl Lines 24 to 33 in 82be59e |
|
We should cache the images' thumbnail on backend. |
|
We already do some caching in libravatar but it's so slow that initial page lookup for any new avatar delays rendering until it is done. |
|
And in any case, browsers are far better at caching than we will ever be. |
|
To be clearer it's not that the users thumbnail takes the time it's this code: // Finds or defaults a URL for Federation (for openid to be used, email has to be nil)
func (v *Libravatar) baseURL(email *mail.Address, openid *url.URL) (string, error) {
var service, protocol, domain string
if v.useHTTPS {
protocol = "https://"
service = v.secureServiceBase
domain = v.secureFallbackHost
} else {
protocol = "http:https://"
service = v.serviceBase
domain = v.fallbackHost
}
host := v.getDomain(email, openid)
key := cacheKey{service, host}
now := time.Now()
v.nameCacheMutex.Lock()
val, found := v.nameCache[key]
v.nameCacheMutex.Unlock()
if found && now.Sub(val.checkedAt) <= v.nameCacheDuration {
return protocol + val.target, nil
}
_, addrs, err := net.LookupSRV(service, "tcp", host)
if err != nil && err.(*net.DNSError).IsTimeout {
return "", err
}
if len(addrs) == 1 {
// select only record, if only one is available
domain = strings.TrimSuffix(addrs[0].Target, ".")
} else if len(addrs) > 1 {
// Select first record according to RFC2782 weight
// ordering algorithm (page 3)
type record struct {
srv *net.SRV
weight uint16
}
var (
totalWeight uint16
records []record
topPriority = addrs[0].Priority
topRecord *net.SRV
)
for _, rr := range addrs {
if rr.Priority > topPriority {
continue
} else if rr.Priority < topPriority {
// won't happen, because net sorts
// by priority, but just in case
totalWeight = 0
records = nil
topPriority = rr.Priority
}
totalWeight += rr.Weight
if rr.Weight > 0 {
records = append(records, record{rr, totalWeight})
} else if rr.Weight == 0 {
records = append([]record{record{srv: rr, weight: totalWeight}}, records...)
}
}
if len(records) == 1 {
topRecord = records[0].srv
} else {
randnum := uint16(rand.Intn(int(totalWeight)))
for _, rr := range records {
if rr.weight >= randnum {
topRecord = rr.srv
break
}
}
}
domain = fmt.Sprintf("%s:%d", topRecord.Target, topRecord.Port)
}
v.nameCacheMutex.Lock()
v.nameCache[key] = cacheValue{checkedAt: now, target: domain}
v.nameCacheMutex.Unlock()
return protocol + domain, nil
}This has to happen for each new domain in an email address. Without this PR this delay occurs at the time of rendering the page needlessly delaying the appearance of the page by literally seconds. |
Codecov Report
@@ Coverage Diff @@
## master #10540 +/- ##
==========================================
+ Coverage 43.53% 43.56% +0.03%
==========================================
Files 589 590 +1
Lines 82683 82772 +89
==========================================
+ Hits 35993 36058 +65
- Misses 42221 42238 +17
- Partials 4469 4476 +7
Continue to review full report at Codecov.
|
|
Wouldn't something like AES be more suitable than base64, given that the client does not need to know the email? Or maybe some other lightweight cipher which is not totally broken. |
|
The client will know the email address anyway. The obfuscation is to reduce the chance of the email address being indexed as is. |
|
I would have used the md5sum but libravatar requires the whole unhashed email address - although it could actually get away with the domain and the md5sum. |
|
Why not encrypt/decrypt it using There are existing |
GiteaBot
added
lgtm/need 1
|
I don't think email address is exposed anywhere except when cloning repo and checking commits |
Libravatar uses hash in URL: https://wiki.libravatar.org/api/ although as I am reading this you could mean libravatar library uses the full email in the backend to lookup via DNS the endpoint to use.
|
I wouldn't use |
|
@guillep2k doh 🤷♂, you're right. |
Maybe a new encryption key can be generated each time the instance starts? Caches would be invalidated when the instance restarts, but that wouldn't be so bad. |
A runtime-only secret sounds like a good solution here. It doesn't even need to be strongly secure as performance is probably more imporant, but a bit more security than base64 would be nice. |
Signed-off-by: Andrew Thornton <[email protected]>
Signed-off-by: Andrew Thornton <[email protected]>
|
But of course some unit test will fail.... |
Signed-off-by: Andrew Thornton <[email protected]>
GiteaBot
added
lgtm/done
Signed-off-by: Andrew Thornton <[email protected]>
|
Now it might be worth considering making the user's avatars go through the same hash url system. Then avatar images in emails would work. |
|
Migration to simply create the empty table done. |
Speed up page generation by making avatar lookup occur at the browser
not at page generation.