From 74e6d341abb14e2e8cd288210e29d881dad9a7c1 Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Sun, 9 Aug 2026 13:29:29 -0400 Subject: [PATCH] fix: sanitize invalid UTF-8 bytes in uploaded filenames (#265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: sanitize invalid UTF-8 bytes in uploaded filenames A client-supplied filename containing a raw non-UTF-8 byte (e.g. 0x97, a Windows-1252 em dash) crashed the media insert with an uncaught QueryException: Postgres rejects invalid UTF-8 byte sequences outright under UTF8 encoding. Centralized a sanitizeOriginalFilename() helper in HasMedia and applied it to all three insert paths that store original_filename: addMedia(), addMediaFromPath(), and addMediaFromStoredPath() (the multipart cloud-upload registration path) — so every upload entry point is covered, not just the one that happened to crash in production. Fixes Nightwatch issue #24. * refactor: use mb_scrub() instead of the mb_convert_encoding same-encoding trick mb_scrub() (PHP 8.1+) is the purpose-built function for scrubbing invalid byte sequences — same behavior, clearer intent than the convert-to-same-encoding workaround it replaces. * test: cover addMediaFromStoredPath (previously untested, including sanitize fix) addMediaFromStoredPath — the multipart cloud-upload registration path — had zero test coverage before this PR, including for the invalid UTF-8 filename fix applied to it. Added a basic happy-path test plus the sanitize regression test, matching the coverage already added for addMedia() and addMediaFromPath(). Verified the regression test actually catches the bug: reverted the sanitize call for this one method locally, confirmed the test fails with the exact Nightwatch #24 QueryException, then restored the fix. --- app/Models/Traits/HasMedia.php | 16 +++++-- tests/Feature/Api/UploadControllerTest.php | 18 ++++++++ tests/Unit/Traits/HasMediaTest.php | 51 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 3 deletions(-) diff --git a/app/Models/Traits/HasMedia.php b/app/Models/Traits/HasMedia.php index 8c41e0a9..3152b89d 100644 --- a/app/Models/Traits/HasMedia.php +++ b/app/Models/Traits/HasMedia.php @@ -98,7 +98,7 @@ public function addMedia(UploadedFile $file, string $collection = 'default', arr 'collection' => $collection, 'type' => $type, 'path' => $path, - 'original_filename' => $file->getClientOriginalName(), + 'original_filename' => $this->sanitizeOriginalFilename($file->getClientOriginalName()), 'mime_type' => $normalizedMime, 'size' => strlen($normalizedBytes), 'order' => 0, @@ -137,7 +137,7 @@ public function addMediaFromPath(string $filePath, string $originalFilename, str 'collection' => $collection, 'type' => $type, 'path' => $stored['path'], - 'original_filename' => $originalFilename, + 'original_filename' => $this->sanitizeOriginalFilename($originalFilename), 'mime_type' => $stored['mime_type'], 'size' => $stored['size'], 'order' => 0, @@ -169,7 +169,7 @@ public function addMediaFromStoredPath( 'collection' => $collection, 'type' => $type, 'path' => $storagePath, - 'original_filename' => $originalFilename, + 'original_filename' => $this->sanitizeOriginalFilename($originalFilename), 'mime_type' => $mimeType, 'size' => $size, 'order' => 0, @@ -251,6 +251,16 @@ private function getMediaType(string $mimeType): string ?? throw new InvalidArgumentException("Unsupported media MIME type: {$mimeType}"))->value; } + /** + * Client-supplied filenames may contain byte sequences that aren't valid + * UTF-8 (e.g. a raw Windows-1252 byte for an em dash). Postgres rejects + * those outright on insert, so replace invalid sequences before storing. + */ + private function sanitizeOriginalFilename(string $filename): string + { + return mb_scrub($filename, 'UTF-8'); + } + private function getMediaMeta(UploadedFile $file, string $type): array { $meta = []; diff --git a/tests/Feature/Api/UploadControllerTest.php b/tests/Feature/Api/UploadControllerTest.php index dc8e66e4..bb2b0435 100644 --- a/tests/Feature/Api/UploadControllerTest.php +++ b/tests/Feature/Api/UploadControllerTest.php @@ -53,6 +53,24 @@ function signedUploadUrl(Workspace $ws, string $token, ?int $expiresInMinutes = ]); }); +test('sanitizes an invalid UTF-8 byte in the client filename instead of crashing the insert (Nightwatch #24)', function () { + $token = (string) Str::uuid(); + // 0x97 is a raw Windows-1252 em dash, not valid UTF-8 on its own — Postgres + // rejects it outright on insert unless the filename is sanitized first. + $file = UploadedFile::fake()->image("earnings \x97 report.png", 50, 50); + + $response = $this->post(signedUploadUrl($this->workspace, $token), [ + 'media' => $file, + ]); + + $response->assertCreated(); + + $media = Media::where('upload_token', $token)->first(); + expect($media)->not->toBeNull(); + expect(mb_check_encoding($media->original_filename, 'UTF-8'))->toBeTrue(); + expect($media->original_filename)->toBe('earnings ? report.png'); +}); + test('rejects unsigned request', function () { $token = (string) Str::uuid(); $file = UploadedFile::fake()->image('shot.png', 50, 50); diff --git a/tests/Unit/Traits/HasMediaTest.php b/tests/Unit/Traits/HasMediaTest.php index 87a658ca..636679a1 100644 --- a/tests/Unit/Traits/HasMediaTest.php +++ b/tests/Unit/Traits/HasMediaTest.php @@ -324,6 +324,57 @@ ->and(pathinfo($media->path, PATHINFO_EXTENSION))->toBe('jpg'); }); +test('model can add media from stored path', function () { + $workspace = Workspace::factory()->create(); + $content = file_get_contents(__DIR__.'/../../fixtures/1x1.png'); + Storage::put('medias/existing.png', $content); + + $media = $workspace->addMediaFromStoredPath('medias/existing.png', 'existing.png', 'image/png', strlen($content), 'assets'); + + expect($media)->toBeInstanceOf(Media::class); + expect($media->original_filename)->toBe('existing.png'); + expect($media->path)->toBe('medias/existing.png'); + expect($media->mime_type)->toBe('image/png'); + expect($media->size)->toBe(strlen($content)); +}); + +test('add media from stored path sanitizes invalid UTF-8 bytes in the original filename', function () { + $workspace = Workspace::factory()->create(); + $content = file_get_contents(__DIR__.'/../../fixtures/1x1.png'); + Storage::put('medias/existing.png', $content); + $invalidName = "earnings \x97 report.png"; + + $media = $workspace->addMediaFromStoredPath('medias/existing.png', $invalidName, 'image/png', strlen($content), 'assets'); + + expect(mb_check_encoding($media->original_filename, 'UTF-8'))->toBeTrue(); + expect($media->original_filename)->toBe('earnings ? report.png'); +}); + +test('add media sanitizes invalid UTF-8 bytes in the original filename', function () { + $workspace = Workspace::factory()->create(); + $invalidName = "earnings \x97 report.jpg"; + $file = UploadedFile::fake()->image($invalidName, 100, 100); + + $media = $workspace->addMedia($file, 'assets'); + + expect(mb_check_encoding($media->original_filename, 'UTF-8'))->toBeTrue(); + expect($media->original_filename)->toBe('earnings ? report.jpg'); +}); + +test('add media from path sanitizes invalid UTF-8 bytes in the original filename', function () { + $workspace = Workspace::factory()->create(); + $tempFile = tempnam(sys_get_temp_dir(), 'test'); + file_put_contents($tempFile, file_get_contents(__DIR__.'/../../fixtures/1x1.png')); + $invalidName = "earnings \x97 report.png"; + + $media = $workspace->addMediaFromPath($tempFile, $invalidName, 'assets'); + + expect(mb_check_encoding($media->original_filename, 'UTF-8'))->toBeTrue(); + expect($media->original_filename)->toBe('earnings ? report.png'); + + unlink($tempFile); +}); + test('client meta is merged into media meta', function () { $workspace = Workspace::factory()->create(); $file = UploadedFile::fake()->image('photo.jpg', 640, 480);