[3.0] Look for a gallery avatar where the gallery is - #9588
Open
albertlast wants to merge 1 commit into
Open
Conversation
Config::$modSettings['avatar_directory'] is where the avatar gallery lives. The admin nominates it, and Attachments warns when the directory it names is not there. Profile lists the gallery out of it, and refuses to save a choice that resolves outside it, so a stored gallery avatar is a path into that directory and nowhere else. Avatar looked for the file under Config::$boarddir . '/avatars' instead, while building the address from Config::$modSettings['avatar_url']. The two agree until an admin moves the gallery, and then every member's chosen avatar is looked for in a directory it was never in: not found at the gallery step, not found at the custom_avatar step, and answered with default.png. A forum that moved its avatars directory shows the default image for everybody. Reading the setting in one place keeps the file and its address naming the same directory, and keeps the value the admin panel writes when the field is left empty as the fallback. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This change was produced by an LLM. The fix, the tests, the commit message and
this description were all written by Claude (Anthropic), driven by @albertlast. It
has not yet had human code review.
Description
Config::$modSettings['avatar_directory']is where the avatar gallery lives. It isa setting the admin owns:
Actions/Admin/Attachments.phpoffers the field, defaults it toConfig::$boarddir . '/avatars'when it is submitted empty, and showsavatar_directory_wrongwhen the directory it names is not a directory.Profile::getAvatars()lists the gallery out of it.Profilerefuses to save a chosen avatar whoserealpath()does not start withit.
So a stored gallery avatar is a path into that directory, and the forum has already
said so twice by the time
Avatarsees it.Avatarlooked underConfig::$boarddir . '/avatars', hard-coded, in the threeplaces that find the file — while building the address from
Config::$modSettings['avatar_url'], which is the URL of the configureddirectory. The path and the address only name the same place while the admin leaves
the setting alone.
When they do not, every member's chosen avatar is looked for somewhere it has never
been: not found at the gallery step, not found in
custom_avatar, and answered withdefault.png. A forum that moved its avatars directory shows the default image foreverybody, with nothing in the error log to say why.
The fix reads the setting in one place,
getGalleryDir(), and keeps the adminpanel's own default as the fallback for a forum that never wrote the setting.
These are regression tests
tests/Unit/AvatarGalleryDirectoryTest.phppointsavatar_directoryatThemes/default/images, which stands in for a moved gallery: full of images, notthe directory SMF ships the gallery in, and nothing has to be written to disk to
use it.
Against the unfixed constructor, two of the three fail, and the failure is the
symptom rather than an abstraction of it:
The third passes either side, which is the point of it: a forum whose admin never
touched the setting keeps the gallery SMF ships.
Notes for review
case 2said "prepackaged avatar directory". The gallery isprepackaged only until someone moves it, so it now says what the case looks in.
case 4, which works a filename back out of a URL path, is one of the three. Itis checking that a resolved path sits inside a directory avatars are allowed to
be in, and after this change the set of those directories is the configured one
plus
custom_avatar_dir— which is the same setProfilevalidates against.Config::$boarddir . '/avatars'is deliberately not kept alongside it: on a forumthat moved the gallery, it is no longer an avatar directory.
AvatarTest.php, because [3.0][Testing] Cover four more merged fixes in the unit suite #9586and [3.0] End the search for an avatar at the last resort #9587 each add an Avatar test file of their own and I would rather they merge
in any order than have three PRs conflict on one file.
is the more urgent of the two.
Issues References (Fixes|Related|Closes)
Related to #9586, #9587.