Skip to content

Conversation

@sandervanhooft
Copy link

Bug

In app/Filament/Pages/Colors.php, both the logo and favicon FileUpload fields have duplicate getUploadedFileNameForStorageUsing() callbacks chained on them. Since PHP method chaining means the last call wins, this causes:

Logo field

The second (incorrect) callback overrides the first and returns storage_path("app/public/...") — an absolute server path like /var/www/html/storage/app/public/logo-foo.png — instead of a relative filename. This gets stored as the filename, resulting in broken image URLs.

Favicon field

The first (incorrect) callback returns storage_path("app/public/favicon.png"), but is overridden by the second (correct) one returning "favicon.png". The broken callback is dead code but misleading.

Fix

Remove the duplicate getUploadedFileNameForStorageUsing() callbacks, keeping only the correct ones:

  • Logo: logo-{originalname} (relative filename)
  • Favicon: favicon.png (relative filename)

Before (logo example)

->getUploadedFileNameForStorageUsing(
    function (TemporaryUploadedFile $file): string {
        return (string)str($file->getClientOriginalName())->prepend('logo-');
    }
)
->getUploadedFileNameForStorageUsing(  // ❌ overrides the above
    function ($record) {
        return storage_path('app/public/' . app(ColorSettings::class)->logo);
    }
)

After

->getUploadedFileNameForStorageUsing(
    function (TemporaryUploadedFile $file): string {
        return (string)str($file->getClientOriginalName())->prepend('logo-');
    }
)

Both the logo and favicon FileUpload fields had two getUploadedFileNameForStorageUsing
callbacks. The second callback overrides the first (PHP method chaining), causing:

- Logo: stored with an absolute server path (storage_path(...)) instead of 'logo-{name}'
- Favicon: the correct callback was second, but the first (broken) one was redundant

This removes the duplicate callbacks, keeping only the correct ones:
- Logo: 'logo-{originalname}'
- Favicon: 'favicon.png'
@Cannonb4ll Cannonb4ll merged commit 6c657df into ploi:main Feb 4, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants