Fix avatar negative-cache poisoning on transient LDAP errors
Lint / PHP (phpcs PSR-12) (push) Successful in 57s
Lint / JS (eslint) (push) Successful in 9s
Security / PHP Security (semgrep) (push) Failing after 41s
Lint / Deploy (push) Successful in 5s
Lint / Notify on failure (push) Has been skipped

user_avatar.php wrote a ".none" sentinel whenever it failed to obtain
avatar bytes, conflating "LDAP errored/timed out" with "user has no
avatar". A brief lldap blip (restart, slow response past the 3s timeout,
network hiccup) therefore cached a 404 for the full AVATAR_CACHE_TTL
(1h default), leaving avatars broken long after lldap recovered.

Track whether the LDAP query actually completed (`$ldapQueryOk`) and only
write the negative-cache sentinel when lldap genuinely answered with no/
invalid avatar. On error/timeout, leave no sentinel so the lookup retries
once lldap is healthy again.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-30 09:01:17 -04:00
co-authored by Claude Opus 4.8
parent 597e1b1eea
commit 5808b93cdb
+13 -4
View File
@@ -110,6 +110,7 @@ $safeUsername = ldap_escape($username, '', LDAP_ESCAPE_FILTER);
$filter = "(uid=$safeUsername)";
$avatarData = null;
$ldapQueryOk = false; // true only if the LDAP lookup completed without error
try {
$ldap = @ldap_connect("ldap://$ldapHost:$ldapPort");
@@ -137,20 +138,28 @@ try {
$avatarData = $entries[0]['avatar'][0];
}
// The query ran to completion — any "no avatar" result is authoritative.
$ldapQueryOk = true;
ldap_unbind($ldap);
} catch (Exception $e) {
error_log("user_avatar: LDAP error for username=$username: " . $e->getMessage());
// Fall through to 404
// Transient LDAP failure: do NOT poison the negative cache. Fall through to
// a plain 404 so the avatar is retried on the next request once LDAP recovers.
}
if ($avatarData === null || strlen($avatarData) < 100) {
// Write sentinel so we don't hammer LDAP for users without avatars
file_put_contents($noAvatarSentinel, '');
// Only cache "no avatar" when LDAP actually answered. On an error/timeout we
// leave no sentinel, so the lookup is retried instead of being stuck for the TTL.
if ($ldapQueryOk) {
file_put_contents($noAvatarSentinel, '');
}
http_response_code(404);
exit;
}
// Validate it's actually a JPEG (magic bytes FF D8 FF)
// Validate it's actually a JPEG (magic bytes FF D8 FF). A successful LDAP read of
// non-JPEG data is a genuine "no usable avatar", so the sentinel is appropriate here.
if (substr($avatarData, 0, 3) !== "\xFF\xD8\xFF") {
error_log("user_avatar: non-JPEG data for username=$username");
file_put_contents($noAvatarSentinel, '');