Harden upload filename handling and add edge-case tests

app/Services/FileService.php

@@ -18,16 +18,81 @@ class FileService
      */
     public function saveUploadedFile($path, $file): File
     {
-        Storage::disk('public')->put($path . '/' .$file->getClientOriginalName(), $file->getContent());
-        return File::query()->updateOrCreate([
-            'link' => url('/storage/') . '/' . $path . '/' .$file->getClientOriginalName(),
-            'path' => $path . '/' .$file->getClientOriginalName(),
+        $originalName = $this->sanitizeOriginalName((string)$file->getClientOriginalName());
+        $storedFilename = $this->buildStoredFilename($originalName);
+        $relativePath = $this->buildUniquePath($path, $storedFilename);
+
+        Storage::disk('public')->put($relativePath, $file->getContent());
+
+        return File::query()->create([
+            'link' => url('/storage/' . $relativePath),
+            'path' => $relativePath,
             'user_id' => auth()->user()->id,
-            'original_name' => $file->getClientOriginalName(),
+            'original_name' => $originalName,
             'mime_type' => $file->getClientMimeType(),
         ]);
     }
 
+    private function sanitizeOriginalName(string $originalName): string
+    {
+        $name = basename(str_replace('\\', '/', $originalName));
+        $name = preg_replace('/[\x00-\x1F\x7F]+/u', '', $name) ?? '';
+        $name = trim($name);
+
+        return $name === '' ? 'file' : $name;
+    }
+
+    private function buildStoredFilename(string $originalName): string
+    {
+        $baseName = pathinfo($originalName, PATHINFO_FILENAME);
+        $extension = pathinfo($originalName, PATHINFO_EXTENSION);
+
+        $baseName = preg_replace('/[<>:|?*\/\\\\]+/u', '_', $baseName) ?? '';
+        $baseName = preg_replace('/\s+/u', ' ', $baseName) ?? '';
+        $baseName = trim($baseName, " .\t\n\r\0\x0B");
+
+        if ($baseName === '' || $baseName === '.' || $baseName === '..') {
+            $baseName = 'file';
+        }
+
+        if (function_exists('mb_substr')) {
+            $baseName = mb_substr($baseName, 0, 150);
+        } else {
+            $baseName = substr($baseName, 0, 150);
+        }
+
+        $extension = preg_replace('/[^A-Za-z0-9]+/', '', $extension) ?? '';
+
+        return $extension === '' ? $baseName : $baseName . '.' . $extension;
+    }
+
+    private function buildUniquePath(string $directory, string $filename): string
+    {
+        $directory = trim($directory, '/');
+        $basePath = $directory === '' ? $filename : $directory . '/' . $filename;
+
+        if (!Storage::disk('public')->exists($basePath)) {
+            return $basePath;
+        }
+
+        $baseName = pathinfo($filename, PATHINFO_FILENAME);
+        $extension = pathinfo($filename, PATHINFO_EXTENSION);
+
+        for ($index = 1; $index < 10000; $index++) {
+            $candidateName = $baseName . ' (' . $index . ')';
+            if ($extension !== '') {
+                $candidateName .= '.' . $extension;
+            }
+
+            $candidatePath = $directory === '' ? $candidateName : $directory . '/' . $candidateName;
+            if (!Storage::disk('public')->exists($candidatePath)) {
+                return $candidatePath;
+            }
+        }
+
+        return $basePath;
+    }
+
 
     /**
      * @param string $path
@@ -76,4 +141,4 @@ class FileService
     }
 
 
-}
+}

tests/Feature/OrderControllerTest.php

@@ -404,6 +404,50 @@ class OrderControllerTest extends TestCase
         $this->assertCount(1, $order->fresh()->documents);
     }
 
+    public function test_upload_document_preserves_unicode_and_quotes_filename(): void
+    {
+        Storage::fake('public');
+
+        $order = Order::factory()->create();
+        $filename = "Документ «смета» 'финал' \"v2\".pdf";
+        $document = UploadedFile::fake()->create($filename, 100, 'application/pdf');
+
+        $response = $this->actingAs($this->managerUser)
+            ->post(route('order.upload-document', $order), [
+                'document' => [$document],
+            ]);
+
+        $response->assertRedirect(route('order.show', $order));
+
+        $saved = $order->fresh()->documents->first();
+        $this->assertNotNull($saved);
+        $this->assertEquals($filename, $saved->original_name);
+        $this->assertEquals('orders/' . $order->id . '/document/' . $filename, $saved->path);
+        Storage::disk('public')->assertExists($saved->path);
+    }
+
+    public function test_upload_statement_preserves_unicode_and_quotes_filename(): void
+    {
+        Storage::fake('public');
+
+        $order = Order::factory()->create();
+        $filename = "Акт «приемки» 'этап 1' \"финал\".pdf";
+        $statement = UploadedFile::fake()->create($filename, 100, 'application/pdf');
+
+        $response = $this->actingAs($this->managerUser)
+            ->post(route('order.upload-statement', $order), [
+                'statement' => [$statement],
+            ]);
+
+        $response->assertRedirect(route('order.show', $order));
+
+        $saved = $order->fresh()->statements->first();
+        $this->assertNotNull($saved);
+        $this->assertEquals($filename, $saved->original_name);
+        $this->assertEquals('orders/' . $order->id . '/statement/' . $filename, $saved->path);
+        Storage::disk('public')->assertExists($saved->path);
+    }
+
     public function test_upload_document_limits_to_5_files(): void
     {
         Storage::fake('public');

tests/Feature/ReclamationControllerTest.php

@@ -186,6 +186,28 @@ class ReclamationControllerTest extends TestCase
         $this->assertCount(1, $reclamation->fresh()->photos_before);
     }
 
+    public function test_upload_photo_before_preserves_unicode_and_quotes_filename(): void
+    {
+        Storage::fake('public');
+
+        $reclamation = Reclamation::factory()->create();
+        $filename = "Фото «до» 'левая' \"камера\".jpg";
+        $photo = UploadedFile::fake()->create($filename, 100, 'image/jpeg');
+
+        $response = $this->actingAs($this->managerUser)
+            ->post(route('reclamations.upload-photo-before', $reclamation), [
+                'photo' => [$photo],
+            ]);
+
+        $response->assertRedirect();
+
+        $saved = $reclamation->fresh()->photos_before->first();
+        $this->assertNotNull($saved);
+        $this->assertEquals($filename, $saved->original_name);
+        $this->assertEquals('reclamations/' . $reclamation->id . '/photo_before/' . $filename, $saved->path);
+        Storage::disk('public')->assertExists($saved->path);
+    }
+
     public function test_can_delete_photo_before(): void
     {
         Storage::fake('public');
@@ -254,6 +276,28 @@ class ReclamationControllerTest extends TestCase
         $this->assertCount(1, $reclamation->fresh()->documents);
     }
 
+    public function test_upload_document_preserves_unicode_and_quotes_filename(): void
+    {
+        Storage::fake('public');
+
+        $reclamation = Reclamation::factory()->create();
+        $filename = "Рекламация «док» 'версия' \"A\".pdf";
+        $document = UploadedFile::fake()->create($filename, 100, 'application/pdf');
+
+        $response = $this->actingAs($this->managerUser)
+            ->post(route('reclamations.upload-document', $reclamation), [
+                'document' => [$document],
+            ]);
+
+        $response->assertRedirect();
+
+        $saved = $reclamation->fresh()->documents->first();
+        $this->assertNotNull($saved);
+        $this->assertEquals($filename, $saved->original_name);
+        $this->assertEquals('reclamations/' . $reclamation->id . '/document/' . $filename, $saved->path);
+        Storage::disk('public')->assertExists($saved->path);
+    }
+
     public function test_can_delete_document(): void
     {
         Storage::fake('public');
@@ -288,6 +332,28 @@ class ReclamationControllerTest extends TestCase
         $this->assertCount(1, $reclamation->fresh()->acts);
     }
 
+    public function test_upload_act_preserves_unicode_and_quotes_filename(): void
+    {
+        Storage::fake('public');
+
+        $reclamation = Reclamation::factory()->create();
+        $filename = "Акт «сервис» 'этап' \"01\".pdf";
+        $act = UploadedFile::fake()->create($filename, 100, 'application/pdf');
+
+        $response = $this->actingAs($this->managerUser)
+            ->post(route('reclamations.upload-act', $reclamation), [
+                'acts' => [$act],
+            ]);
+
+        $response->assertRedirect();
+
+        $saved = $reclamation->fresh()->acts->first();
+        $this->assertNotNull($saved);
+        $this->assertEquals($filename, $saved->original_name);
+        $this->assertEquals('reclamations/' . $reclamation->id . '/act/' . $filename, $saved->path);
+        Storage::disk('public')->assertExists($saved->path);
+    }
+
     public function test_can_delete_act(): void
     {
         Storage::fake('public');

tests/Unit/Services/FileServiceTest.php

@@ -0,0 +1,81 @@
+<?php
+
+namespace Tests\Unit\Services;
+
+use App\Models\User;
+use App\Services\FileService;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Http\UploadedFile;
+use Illuminate\Support\Facades\Storage;
+use Tests\TestCase;
+
+class FileServiceTest extends TestCase
+{
+    use RefreshDatabase;
+
+    public function test_save_uploaded_file_supports_unicode_and_quotes_filename(): void
+    {
+        Storage::fake('public');
+        $user = User::factory()->create();
+        $this->actingAs($user);
+
+        $filename = "Файл «отчет» 'смена' \"v3\".pdf";
+        $uploaded = UploadedFile::fake()->create($filename, 64, 'application/pdf');
+
+        $saved = app(FileService::class)->saveUploadedFile('tests/uploads', $uploaded);
+
+        $this->assertEquals($filename, $saved->original_name);
+        $this->assertEquals('tests/uploads/' . $filename, $saved->path);
+        $this->assertEquals(url('/storage/tests/uploads/' . $filename), $saved->link);
+        Storage::disk('public')->assertExists($saved->path);
+    }
+
+    public function test_save_uploaded_file_sanitizes_dangerous_name_parts(): void
+    {
+        Storage::fake('public');
+        $user = User::factory()->create();
+        $this->actingAs($user);
+
+        $uploaded = UploadedFile::fake()->create('../../evil/..\\..\\doc?.pdf', 64, 'application/pdf');
+
+        $saved = app(FileService::class)->saveUploadedFile('tests/uploads', $uploaded);
+
+        $this->assertEquals('doc?.pdf', $saved->original_name);
+        $this->assertEquals('tests/uploads/doc_.pdf', $saved->path);
+        $this->assertStringNotContainsString('..', $saved->path);
+        Storage::disk('public')->assertExists($saved->path);
+    }
+
+    public function test_save_uploaded_file_avoids_overwrite_for_duplicate_names(): void
+    {
+        Storage::fake('public');
+        $user = User::factory()->create();
+        $this->actingAs($user);
+
+        $firstUpload = UploadedFile::fake()->create('одинаковое имя.pdf', 64, 'application/pdf');
+        $secondUpload = UploadedFile::fake()->create('одинаковое имя.pdf', 64, 'application/pdf');
+
+        $first = app(FileService::class)->saveUploadedFile('tests/uploads', $firstUpload);
+        $second = app(FileService::class)->saveUploadedFile('tests/uploads', $secondUpload);
+
+        $this->assertEquals('tests/uploads/одинаковое имя.pdf', $first->path);
+        $this->assertEquals('tests/uploads/одинаковое имя (1).pdf', $second->path);
+        Storage::disk('public')->assertExists($first->path);
+        Storage::disk('public')->assertExists($second->path);
+    }
+
+    public function test_save_uploaded_file_uses_fallback_when_basename_is_empty(): void
+    {
+        Storage::fake('public');
+        $user = User::factory()->create();
+        $this->actingAs($user);
+
+        $uploaded = UploadedFile::fake()->create('.pdf', 64, 'application/pdf');
+
+        $saved = app(FileService::class)->saveUploadedFile('tests/uploads', $uploaded);
+
+        $this->assertEquals('.pdf', $saved->original_name);
+        $this->assertEquals('tests/uploads/file.pdf', $saved->path);
+        Storage::disk('public')->assertExists($saved->path);
+    }
+}