Merge pull request #7034 from pixelfed/fix/avatar-upload-silent-failure

Fix silent failure in avatar upload endpoints

app/Http/Controllers/Api/BaseApiController.php

@@ -18,6 +18,7 @@ use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Log;
 use League\Fractal;
 use League\Fractal\Serializer\ArraySerializer;
 
@@ -109,6 +110,15 @@ class BaseApiController extends Controller
             Cache::forget("avatar:{$profile->id}");
             AvatarOptimize::dispatch($user->profile, $currentAvatar);
         } catch (\Exception $e) {
+            Log::error('BaseApiController@avatarUpdate failed: '.$e->getMessage(), [
+                'user_id' => $request->user()?->id,
+                'exception' => $e,
+            ]);
+
+            return response()->json([
+                'code' => 500,
+                'msg' => 'There was an error updating your avatar. Please try again.',
+            ], 500);
         }
 
         return response()->json([

app/Http/Controllers/AvatarController.php

@@ -8,6 +8,7 @@ use Illuminate\Http\JsonResponse;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Cache;
+use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Str;
 
 class AvatarController extends Controller
@@ -44,6 +45,14 @@ class AvatarController extends Controller
             Cache::forget('user:account:id:'.$user->id);
             AvatarOptimize::dispatch($user->profile, $currentAvatar);
         } catch (\Exception $e) {
+            Log::error('AvatarController@store failed: '.$e->getMessage(), [
+                'user_id' => $request->user()?->id,
+                'exception' => $e,
+            ]);
+
+            return redirect()->back()->withErrors([
+                'avatar' => 'There was an error updating your avatar. Please try again.',
+            ]);
         }
 
         return redirect()->back()->with('status', 'Avatar updated successfully. It may take a few minutes to update across the site.');

tests/Feature/Api/AvatarUpdateTest.php

@@ -0,0 +1,73 @@
+<?php
+
+use App\Models\Avatar;
+use App\Models\User;
+use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
+use Illuminate\Http\UploadedFile;
+use Illuminate\Support\Facades\Storage;
+use Laravel\Passport\Passport;
+
+uses(LazilyRefreshDatabase::class);
+
+/*
+|--------------------------------------------------------------------------
+| Avatar Update API Tests
+|--------------------------------------------------------------------------
+|
+| Regression coverage for the previously silent failure in
+| BaseApiController@avatarUpdate, where any exception during upload was
+| swallowed and the endpoint still returned a 200 "success" response.
+|
+*/
+
+describe('POST /api/v1/avatar/update', function () {
+    it('returns an error instead of a false success when processing fails', function () {
+        $user = User::factory()->create();
+        $user->refresh();
+
+        // Ensure no Avatar row exists so the internal firstOrFail() throws.
+        Avatar::whereProfileId($user->profile_id)->delete();
+
+        Storage::fake('local');
+        Passport::actingAs($user, ['write']);
+
+        $this->postJson('/api/v1/avatar/update', [
+            'upload' => UploadedFile::fake()->image('avatar.jpg', 200, 200),
+        ])
+            ->assertStatus(500)
+            ->assertJsonFragment(['code' => 500])
+            ->assertJsonMissing(['msg' => 'Avatar successfully updated']);
+    });
+
+    it('updates the avatar with a valid upload', function () {
+        $user = User::factory()->create();
+        $user->refresh();
+
+        Avatar::updateOrCreate(
+            ['profile_id' => $user->profile_id],
+            ['media_path' => 'public/avatars/default.jpg', 'change_count' => 0]
+        );
+
+        Storage::fake('local');
+        Passport::actingAs($user, ['write']);
+
+        $this->postJson('/api/v1/avatar/update', [
+            'upload' => UploadedFile::fake()->image('avatar.jpg', 200, 200),
+        ])
+            ->assertOk()
+            ->assertJsonFragment(['msg' => 'Avatar successfully updated']);
+    });
+
+    it('rejects a non-image upload with a validation error', function () {
+        $user = User::factory()->create();
+        $user->refresh();
+
+        Storage::fake('local');
+        Passport::actingAs($user, ['write']);
+
+        $this->postJson('/api/v1/avatar/update', [
+            'upload' => UploadedFile::fake()->create('malware.pdf', 100, 'application/pdf'),
+        ])
+            ->assertStatus(422);
+    });
+});