Address review decision: the post-commit avatar sync still blocked the
LDAP/OIDC login response for the download and up to 30s of decode
queueing. It now runs in a goroutine on a context that outlives the
request, with panics recovered and failures logged.
Address pr-swarm findings: avatar GETs queued for decode capacity on
context.Background(), so an abandoned request still waited 30s. The
request context now reaches the providers; gravatar's shared fetch keeps
running for other waiters when one disconnects. Busy shedding is logged
as a warning instead of an error.
Address pr-swarm finding: the flush ran before commit while the old blob
stays readable until then, so a queued avatar GET could re-cache the old
image with no expiry. The flush now runs on commit and the cache key
carries the file id.
Address pr-swarm finding: users start on defaultsettings.avatar_file_id,
so the first avatar upload or provider sync deleted that shared file for
everyone. Replacing now skips it, through the same check account
deletion uses.
Address review decision: SaveAvatar removed the old file's blob from
storage inside the transaction, so a later rollback left the user on an
avatar whose content was gone. The files row is still deleted in the
session; the blob and its previews go once the session commits.
Address review decision: the provider avatar was decoded inside the login
transaction after the user row was written, holding it while queued for
decode capacity, and a later rollback (TOTP, disabled user) discarded the
work. Logins now commit first; the avatar is decoded without a session
and saved in its own short transaction.
Address review decision: a decode queued for capacity held the request's
transaction for up to 30s, so on SQLite a concurrent write made the
upload fail with database is locked. Uploads and avatar reads now decode
and encode first; only the encoded PNG enters the transaction.
Address review decision: uploads skipped the square crop synced avatars
got, so non-square API uploads were stored as-is although avatars render
square. Non-square uploads are now center-cropped.
Address pr-swarm finding: both logins copied crop, store and flush, had
diverged, decoded every avatar twice, and imaging.Fill cloned images
thinner than 100 px at full resolution outside the decode budget.
Address pr-swarm finding: StoreUploadedAvatar ran its own header check
without the read limit, and anything it let through that Decode then
rejected (truncated data, header past 12 MiB) answered 500.
Uploaded avatars are decoded through imageutils.Decode on upload and when
resizing per size, so a busy server answers 503 instead of decoding in
parallel. The old avatar file is removed only after the new one decoded,
since a storage delete is not rolled back with the session.
A file row whose blob is gone from storage (Sentry API-OSS-50) made
LoadFileByID return the raw storage path error, which the error handler
turned into a 500 and reported to Sentry. Return ErrFileDoesNotExist
instead and update the callers that matched on the path error.
Decoding now validates image dimensions before allocating pixels
(imaging.Fit replaces unbounded Resize so neither side can blow up),
backgrounds decode once and reuse the image for the blur hash, and
DownloadImage is bounded at the configured max file size. Small sources
now keep their native size (Fit does not upscale); the frontend scales
via CSS.
Querying the global engine while the request already holds an open
transaction acquires a second pool connection. Under concurrent
attachment downloads (e.g. a kanban board full of image previews) all
pool connections end up held by transactions that each wait for an
extra connection that can never be freed, hanging every request until
restart.
The redundant meta load in the upload avatar provider is removed
entirely; the decoded image is all that path uses.
GetAvatar converted a caller-supplied size int64 directly to int via
int(size) for imaging.Resize, with no check that the value was
non-negative or within int's range. size traces back to an
unauthenticated query parameter; on 32-bit builds a large int64
would silently truncate/wrap when narrowed, and a negative value
would feed straight into the resize call. Reject out-of-range values
before the conversion instead of letting it wrap silently.
avatar.gravatarbaseurl is operator-configurable, so the request destination was
never a fixed constant - pkg/utils/avatar.go already uses the SSRF-safe client
for the same job. The previous 5s timeout is kept as a context deadline so the
configured (30s by default) client timeout does not apply here.
Extract the duplicated user-lookup, provider-selection and size-clamping
logic from the v1 GetAvatar and v2 avatarGet handlers into a single
avatar.GetAvatarForUsername helper. Both handlers now call it and keep
only their transport-specific code (v1: echo size parse + c.Blob, v2:
huma input/response). Pure refactor, behavior is unchanged.
Bot users now render with a cool-toned (blue/cyan/violet/teal/indigo)
marble variant so they're visually distinguishable from human users.
Marble's rendering logic is parameterized with a palette; the route
forces the bot palette whenever the resolved user is a bot, overriding
whatever avatar provider they'd otherwise inherit.
Register CachedAvatar and Photo with encoding/gob so Redis can properly
deserialize them. Migrate both to use RememberValue[T] which calls
GetWithValue() internally, fixing the broken type assertion when Redis
is the keyvalue backend.
Also removes the recursion-depth fallback in upload.go since
RememberValue eliminates the type mismatch failure mode entirely.
files.Create() and files.CreateWithMime() internally create their own
sessions and transactions. When called from within an existing
transaction (now that db.NewSession() auto-begins), this creates nested
transactions that deadlock on SQLite.
Switch to files.CreateWithSession() and files.CreateWithMimeAndSession()
to participate in the caller's existing transaction instead.
FlushCache was using keyvalue.Del with the base key
(avatar_upload_{userID}) but the actual cache entries are stored with
size suffixes (avatar_upload_{userID}_{size}). The Del call targeted a
key that never existed, so cached avatars were never invalidated.
Switch to keyvalue.DelPrefix to delete all size variants at once,
matching the pattern the gravatar provider already uses correctly.
The test populates the cache with multiple size-suffixed keys
and verifies that FlushCache removes all of them. Currently fails
because FlushCache uses Del with the base key which doesn't match
the actual size-suffixed cache keys.
Update all code paths that pass file content to the storage layer to
provide io.ReadSeeker instead of io.Reader:
- Avatar upload: use bytes.NewReader instead of bytes.Buffer
- Background upload handler: use bytes.NewReader instead of bytes.Buffer
- Unsplash background: buffer response body into bytes.NewReader
- Dump restore: buffer zip entry into bytes.NewReader
- Migration structure: pass bytes.NewReader directly instead of wrapping
in io.NopCloser
- Task attachment: change NewAttachment parameter from io.ReadCloser to
io.ReadSeeker
Fixes a bug where the webhook HTTP client was mutating `http.DefaultClient` (the global singleton), causing ALL HTTP requests in the application to use the webhook proxy. This broke OIDC authentication and other external HTTP calls when webhook proxy was configured.
Fixes#2144
This adds the ability to set a base URL for a Gravatar-compatible avatar
service (Gravatar itself, or Libravatar, for instance). The default will
be www.gravatar.com, so nothing will change from current behaviour unless
the user explicitly configures another URL.
Resolves#2082
Email notifications now display user mentions with inline avatar images for improved visual recognition and easier identification. Mentions gracefully fall back to display names if avatars are unavailable.
kolaente
Mauandkolaente
Rhys McNeillandkolaente
rhclaytoandGitHub
Weijie ZhaoandGitHub
Dominik PschenitschniandGitHub