From 1a35aee3b9510beeeb4f72503857e09b49d3fecb Mon Sep 17 00:00:00 2001 From: albertlast Date: Sun, 30 Aug 2026 18:48:59 +0200 Subject: [PATCH] Looks for a gallery avatar where the gallery is 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 --- Sources/Avatar.php | 32 +++++- tests/Unit/AvatarGalleryDirectoryTest.php | 130 ++++++++++++++++++++++ 2 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 tests/Unit/AvatarGalleryDirectoryTest.php diff --git a/Sources/Avatar.php b/Sources/Avatar.php index 7df9e0605e..60c1d10607 100644 --- a/Sources/Avatar.php +++ b/Sources/Avatar.php @@ -501,14 +501,14 @@ public function __construct( break; - // Is the file in the prepackaged avatar directory? + // Is the file in the avatar gallery directory? case 2: if ( !empty($this->filename) && !empty(Config::$modSettings['avatar_url']) - && is_file(Config::$boarddir . '/avatars/' . ltrim($this->filename, '\\/')) + && is_file(self::getGalleryDir() . '/' . ltrim($this->filename, '\\/')) && Utils::checkMimeType( - Config::$boarddir . '/avatars/' . ltrim($this->filename, '\\/'), + self::getGalleryDir() . '/' . ltrim($this->filename, '\\/'), 'image/', true, ) @@ -558,7 +558,7 @@ public function __construct( fn($path) => Sapi::canonicalPath($path) . DIRECTORY_SEPARATOR, array_filter([ Config::$modSettings['custom_avatar_dir'] ?? '', - Config::$boarddir . '/avatars', + self::getGalleryDir(), ]), ); @@ -579,7 +579,7 @@ public function __construct( case 5: if ( !empty(Config::$modSettings['avatar_url']) - && is_file(Config::$boarddir . '/avatars/default.png') + && is_file(self::getGalleryDir() . '/default.png') ) { $url = new Url(Config::$modSettings['avatar_url'] . '/default.png'); } @@ -804,4 +804,26 @@ private function getExternal(): string default => 'http://', }; } + + /************************* + * Internal static methods + *************************/ + + /** + * The directory that holds the gallery of avatars members can choose from. + * + * This is the directory the admin nominated, which is the one the gallery + * is listed from and the one a chosen avatar is checked against before it + * is saved. Config::$modSettings['avatar_url'] is the address of the same + * directory, so the two have to name the same place or a member's choice + * resolves to an address with nothing behind it. + * + * @return string Path to the avatar gallery directory. + */ + private static function getGalleryDir(): string + { + return !empty(Config::$modSettings['avatar_directory']) + ? Config::$modSettings['avatar_directory'] + : Config::$boarddir . '/avatars'; + } } diff --git a/tests/Unit/AvatarGalleryDirectoryTest.php b/tests/Unit/AvatarGalleryDirectoryTest.php new file mode 100644 index 0000000000..0f2d524c97 --- /dev/null +++ b/tests/Unit/AvatarGalleryDirectoryTest.php @@ -0,0 +1,130 @@ +setUpForum(Config::$boarddir . '/Themes/default/images'); + + $avatar = new Avatar(url: 'cake.png', id_member: 1); + + $this->assertSame('https://example.com/gallery/cake.png', (string) $avatar->url); + } + + public function testTheSameForAFileInASubdirectoryOfTheGallery(): void + { + $this->setUpForum(Config::$boarddir . '/Themes/default/images'); + + $avatar = new Avatar(url: 'icons/bell.png', id_member: 1); + + $this->assertSame('https://example.com/gallery/icons/bell.png', (string) $avatar->url); + } + + /** + * A forum whose admin never touched the setting keeps the gallery SMF + * ships, so the value the installer would have written is the fallback. + */ + public function testTheShippedGalleryIsUsedWhenNothingSaysOtherwise(): void + { + $this->setUpForum(null); + + $avatar = new Avatar(url: 'Oxygen/beagle.png', id_member: 1); + + $this->assertSame('https://example.com/gallery/Oxygen/beagle.png', (string) $avatar->url); + } + + /****************** + * Internal methods + ******************/ + + protected function setUp(): void + { + $this->boardurl = Config::$boardurl ?? ''; + + foreach (['avatar_url', 'avatar_directory', 'gravatarEnabled'] as $key) { + if (isset(Config::$modSettings[$key])) { + $this->backup[$key] = Config::$modSettings[$key]; + } + } + } + + /** + * PHPUnit does not reset SMF's statics between tests, so a setting left + * behind here would leak into every test that follows. + */ + protected function tearDown(): void + { + Config::$boardurl = $this->boardurl; + + foreach (['avatar_url', 'avatar_directory', 'gravatarEnabled'] as $key) { + unset(Config::$modSettings[$key]); + + if (isset($this->backup[$key])) { + Config::$modSettings[$key] = $this->backup[$key]; + } + } + + $this->backup = []; + } + + /** + * Puts the gallery in $directory, or leaves the setting off entirely when + * it is null. The URL is deliberately not the shipped one, so that an + * address built from it cannot be mistaken for a lucky guess. + */ + private function setUpForum(?string $directory): void + { + Config::$boardurl = 'https://example.com'; + Config::$modSettings['avatar_url'] = 'https://example.com/gallery'; + Config::$modSettings['gravatarEnabled'] = false; + + if ($directory === null) { + unset(Config::$modSettings['avatar_directory']); + } else { + Config::$modSettings['avatar_directory'] = $directory; + } + } +}