Skip to content

Commit 74e6d34

Browse files
fix: sanitize invalid UTF-8 bytes in uploaded filenames (#265)
* 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.
1 parent 676afb1 commit 74e6d34

3 files changed

Lines changed: 82 additions & 3 deletions

File tree

app/Models/Traits/HasMedia.php

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public function addMedia(UploadedFile $file, string $collection = 'default', arr
9898
'collection' => $collection,
9999
'type' => $type,
100100
'path' => $path,
101-
'original_filename' => $file->getClientOriginalName(),
101+
'original_filename' => $this->sanitizeOriginalFilename($file->getClientOriginalName()),
102102
'mime_type' => $normalizedMime,
103103
'size' => strlen($normalizedBytes),
104104
'order' => 0,
@@ -137,7 +137,7 @@ public function addMediaFromPath(string $filePath, string $originalFilename, str
137137
'collection' => $collection,
138138
'type' => $type,
139139
'path' => $stored['path'],
140-
'original_filename' => $originalFilename,
140+
'original_filename' => $this->sanitizeOriginalFilename($originalFilename),
141141
'mime_type' => $stored['mime_type'],
142142
'size' => $stored['size'],
143143
'order' => 0,
@@ -169,7 +169,7 @@ public function addMediaFromStoredPath(
169169
'collection' => $collection,
170170
'type' => $type,
171171
'path' => $storagePath,
172-
'original_filename' => $originalFilename,
172+
'original_filename' => $this->sanitizeOriginalFilename($originalFilename),
173173
'mime_type' => $mimeType,
174174
'size' => $size,
175175
'order' => 0,
@@ -251,6 +251,16 @@ private function getMediaType(string $mimeType): string
251251
?? throw new InvalidArgumentException("Unsupported media MIME type: {$mimeType}"))->value;
252252
}
253253

254+
/**
255+
* Client-supplied filenames may contain byte sequences that aren't valid
256+
* UTF-8 (e.g. a raw Windows-1252 byte for an em dash). Postgres rejects
257+
* those outright on insert, so replace invalid sequences before storing.
258+
*/
259+
private function sanitizeOriginalFilename(string $filename): string
260+
{
261+
return mb_scrub($filename, 'UTF-8');
262+
}
263+
254264
private function getMediaMeta(UploadedFile $file, string $type): array
255265
{
256266
$meta = [];

tests/Feature/Api/UploadControllerTest.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,24 @@ function signedUploadUrl(Workspace $ws, string $token, ?int $expiresInMinutes =
5353
]);
5454
});
5555

56+
test('sanitizes an invalid UTF-8 byte in the client filename instead of crashing the insert (Nightwatch #24)', function () {
57+
$token = (string) Str::uuid();
58+
// 0x97 is a raw Windows-1252 em dash, not valid UTF-8 on its own — Postgres
59+
// rejects it outright on insert unless the filename is sanitized first.
60+
$file = UploadedFile::fake()->image("earnings \x97 report.png", 50, 50);
61+
62+
$response = $this->post(signedUploadUrl($this->workspace, $token), [
63+
'media' => $file,
64+
]);
65+
66+
$response->assertCreated();
67+
68+
$media = Media::where('upload_token', $token)->first();
69+
expect($media)->not->toBeNull();
70+
expect(mb_check_encoding($media->original_filename, 'UTF-8'))->toBeTrue();
71+
expect($media->original_filename)->toBe('earnings ? report.png');
72+
});
73+
5674
test('rejects unsigned request', function () {
5775
$token = (string) Str::uuid();
5876
$file = UploadedFile::fake()->image('shot.png', 50, 50);

tests/Unit/Traits/HasMediaTest.php

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,57 @@
324324
->and(pathinfo($media->path, PATHINFO_EXTENSION))->toBe('jpg');
325325
});
326326

327+
test('model can add media from stored path', function () {
328+
$workspace = Workspace::factory()->create();
329+
$content = file_get_contents(__DIR__.'/../../fixtures/1x1.png');
330+
Storage::put('medias/existing.png', $content);
331+
332+
$media = $workspace->addMediaFromStoredPath('medias/existing.png', 'existing.png', 'image/png', strlen($content), 'assets');
333+
334+
expect($media)->toBeInstanceOf(Media::class);
335+
expect($media->original_filename)->toBe('existing.png');
336+
expect($media->path)->toBe('medias/existing.png');
337+
expect($media->mime_type)->toBe('image/png');
338+
expect($media->size)->toBe(strlen($content));
339+
});
340+
341+
test('add media from stored path sanitizes invalid UTF-8 bytes in the original filename', function () {
342+
$workspace = Workspace::factory()->create();
343+
$content = file_get_contents(__DIR__.'/../../fixtures/1x1.png');
344+
Storage::put('medias/existing.png', $content);
345+
$invalidName = "earnings \x97 report.png";
346+
347+
$media = $workspace->addMediaFromStoredPath('medias/existing.png', $invalidName, 'image/png', strlen($content), 'assets');
348+
349+
expect(mb_check_encoding($media->original_filename, 'UTF-8'))->toBeTrue();
350+
expect($media->original_filename)->toBe('earnings ? report.png');
351+
});
352+
353+
test('add media sanitizes invalid UTF-8 bytes in the original filename', function () {
354+
$workspace = Workspace::factory()->create();
355+
$invalidName = "earnings \x97 report.jpg";
356+
$file = UploadedFile::fake()->image($invalidName, 100, 100);
357+
358+
$media = $workspace->addMedia($file, 'assets');
359+
360+
expect(mb_check_encoding($media->original_filename, 'UTF-8'))->toBeTrue();
361+
expect($media->original_filename)->toBe('earnings ? report.jpg');
362+
});
363+
364+
test('add media from path sanitizes invalid UTF-8 bytes in the original filename', function () {
365+
$workspace = Workspace::factory()->create();
366+
$tempFile = tempnam(sys_get_temp_dir(), 'test');
367+
file_put_contents($tempFile, file_get_contents(__DIR__.'/../../fixtures/1x1.png'));
368+
$invalidName = "earnings \x97 report.png";
369+
370+
$media = $workspace->addMediaFromPath($tempFile, $invalidName, 'assets');
371+
372+
expect(mb_check_encoding($media->original_filename, 'UTF-8'))->toBeTrue();
373+
expect($media->original_filename)->toBe('earnings ? report.png');
374+
375+
unlink($tempFile);
376+
});
377+
327378
test('client meta is merged into media meta', function () {
328379
$workspace = Workspace::factory()->create();
329380
$file = UploadedFile::fake()->image('photo.jpg', 640, 480);

0 commit comments

Comments
 (0)