From 187f3dba22a416f1c04d8f15877c55b3b1506e1f Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Sun, 9 Aug 2026 12:53:32 -0300 Subject: [PATCH 1/8] fix: give each chunked upload attempt a unique server-side identifier The upload session identifier was derived only from user+filename+size (ChunkedAssetReceiver::receive), with no per-attempt nonce. Two genuinely concurrent attempts of the same file (e.g. closing and reopening the media picker mid-upload, then re-uploading the same file) collided on the same Redis cache key / temp file, producing RuntimeException("Chunked cloud upload session expired or missing.") on the multipart/cloud path and silent byte corruption on the local-assemble path. The frontend now mints a UUID per upload attempt (X-Upload-Id header) that gets folded into the identifier. Falls back to the old formula when the header is absent, so any already-loaded frontend bundle keeps working. Also guards the media picker's dropzone against re-triggering an upload while one is in flight, and aborts the in-flight fetch when the dialog unmounts mid-upload. Fixes Nightwatch issue #23. --- app/Http/Controllers/App/AssetController.php | 1 + .../App/Asset/StoreChunkedAssetRequest.php | 2 + app/Services/Media/ChunkedAssetReceiver.php | 3 +- .../js/components/assets/GalleryBrowser.vue | 22 ++- resources/js/utils/chunkedUpload.ts | 6 + tests/Feature/ChunkedCloudUploadTest.php | 174 +++++++++++++++++- 6 files changed, 197 insertions(+), 11 deletions(-) diff --git a/app/Http/Controllers/App/AssetController.php b/app/Http/Controllers/App/AssetController.php index 590a2615f..72f4de961 100644 --- a/app/Http/Controllers/App/AssetController.php +++ b/app/Http/Controllers/App/AssetController.php @@ -83,6 +83,7 @@ public function storeChunked(StoreChunkedAssetRequest $request, ChunkedAssetRece (int) $request->validated('range_start'), (int) $request->validated('range_end'), (int) $request->validated('total_size'), + $request->validated('upload_id'), )->toResponse(); } diff --git a/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php b/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php index fbf100c61..ba26d017a 100644 --- a/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php +++ b/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php @@ -29,6 +29,7 @@ protected function prepareForValidation(): void 'range_end' => $parsed[1] ?? null, 'total_size' => $parsed[2] ?? null, 'file_name' => strtolower(rawurldecode((string) $this->header('X-File-Name', 'upload'))), + 'upload_id' => $this->header('X-Upload-Id'), ]); } @@ -47,6 +48,7 @@ public function rules(): array 'range_end' => ['required', 'integer', 'gte:range_start'], 'total_size' => ['required', 'integer', 'min:1', 'max:'.MediaType::Video->maxSizeInBytes()], 'file_name' => ['required', 'string', 'ends_with:'.implode(',', $allowedSuffixes)], + 'upload_id' => ['nullable', 'string', 'uuid'], ]; } diff --git a/app/Services/Media/ChunkedAssetReceiver.php b/app/Services/Media/ChunkedAssetReceiver.php index fa09a5954..5ba73126e 100644 --- a/app/Services/Media/ChunkedAssetReceiver.php +++ b/app/Services/Media/ChunkedAssetReceiver.php @@ -21,8 +21,9 @@ public function receive( int $rangeStart, int $rangeEnd, int $totalSize, + ?string $attemptId = null, ): ChunkReceipt { - $identifier = md5($user->id.$fileName.$totalSize); + $identifier = md5($user->id.$fileName.$totalSize.(string) $attemptId); return $this->cloud->shouldUseMultipart($fileName) ? $this->receiveViaMultipart($workspace, $identifier, $fileName, $chunk, $rangeStart, $rangeEnd, $totalSize) diff --git a/resources/js/components/assets/GalleryBrowser.vue b/resources/js/components/assets/GalleryBrowser.vue index a49d2d434..cd7abd810 100644 --- a/resources/js/components/assets/GalleryBrowser.vue +++ b/resources/js/components/assets/GalleryBrowser.vue @@ -161,6 +161,7 @@ const uploadsSentinel = useTemplateRef('uploadsSentinel'); const isDragging = ref(false); const uploading = ref(false); let uploadsObserver: IntersectionObserver | null = null; +let uploadAbortController: AbortController | null = null; const fetchUploads = async (page: number, term: string) => { const response = await fetch( @@ -224,9 +225,16 @@ watch(uploadsSentinel, async () => { setupUploadsObserver(); }); -const triggerFileInput = () => fileInput.value?.click(); +const triggerFileInput = () => { + if (uploading.value) return; + fileInput.value?.click(); +}; const handleFileSelect = (event: Event) => { const target = event.target as HTMLInputElement; + if (uploading.value) { + target.value = ''; + return; + } if (target.files) { void uploadFiles(Array.from(target.files)); target.value = ''; @@ -234,6 +242,7 @@ const handleFileSelect = (event: Event) => { }; const handleDrop = (event: DragEvent) => { isDragging.value = false; + if (uploading.value) return; if (event.dataTransfer?.files) { void uploadFiles(Array.from(event.dataTransfer.files)); } @@ -241,14 +250,17 @@ const handleDrop = (event: DragEvent) => { const uploadFiles = async (files: File[]) => { uploading.value = true; + uploadAbortController = new AbortController(); for (const file of files) { try { await uploadChunked({ file, url: assetsStoreChunked.url(), collection: 'assets', + signal: uploadAbortController.signal, }); - } catch { + } catch (error) { + if (error instanceof DOMException && error.name === 'AbortError') break; toast.error(trans('assets.upload.failed', { file: file.name })); } } @@ -596,6 +608,7 @@ onUnmounted(() => { uploadsObserver?.disconnect(); unsplashObserver?.disconnect(); giphyObserver?.disconnect(); + uploadAbortController?.abort(); }); @@ -612,7 +625,10 @@ onUnmounted(() => {
void; onComplete?: (response: any) => void; onError?: (error: any) => void; @@ -30,6 +31,7 @@ export const uploadChunked = async (options: ChunkedUploadOptions): Promise('meta[name="csrf-token"]')?.content ?? ''; const totalSize = file.size; const totalChunks = Math.ceil(totalSize / chunkSize); + const uploadId = crypto.randomUUID(); let uploadedBytes = 0; try { @@ -50,6 +53,7 @@ export const uploadChunked = async (options: ChunkedUploadOptions): Promise "bytes {$rangeStart}-{$rangeEnd}/{$totalSize}", + 'HTTP_X_FILE_NAME' => rawurlencode($fileName), + 'HTTP_ACCEPT' => 'application/json', + 'CONTENT_TYPE' => 'application/octet-stream', + ]; + + if ($uploadId !== null) { + $headers['HTTP_X_UPLOAD_ID'] = $uploadId; + } + return test()->actingAs(test()->user)->call( 'POST', route('app.assets.store-chunked'), [], [], [], - [ - 'HTTP_CONTENT_RANGE' => "bytes {$rangeStart}-{$rangeEnd}/{$totalSize}", - 'HTTP_X_FILE_NAME' => rawurlencode($fileName), - 'HTTP_ACCEPT' => 'application/json', - 'CONTENT_TYPE' => 'application/octet-stream', - ], + $headers, $content, ); } @@ -200,6 +209,54 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 expect($retry)->toMatchArray(['done' => false, 'progress' => $first['progress']]); }); +// ─── Per-attempt identifier (concurrent duplicate uploads) ─────── + +test('receive derives the legacy identifier when no upload id is provided', function () { + seedChunkedUploadWorkspace(); + + $totalSize = 1000; + $expected = md5(test()->user->id.'video.mp4'.$totalSize); + + $cloud = Mockery::mock(ChunkedCloudUploader::class); + $cloud->shouldReceive('shouldUseMultipart')->andReturn(true); + $cloud->shouldReceive('receiveChunk') + ->once() + ->withArgs(fn (string $identifier) => $identifier === $expected) + ->andReturn(['done' => false, 'progress' => 10]); + + (new ChunkedAssetReceiver($cloud))->receive( + test()->workspace, + test()->user, + 'video.mp4', + 'chunk', + 0, + 99, + 1000, + ); +}); + +test('receive derives a distinct identifier per upload attempt', function () { + seedChunkedUploadWorkspace(); + + $seen = []; + $cloud = Mockery::mock(ChunkedCloudUploader::class); + $cloud->shouldReceive('shouldUseMultipart')->andReturn(true); + $cloud->shouldReceive('receiveChunk') + ->twice() + ->withArgs(function (string $identifier) use (&$seen) { + $seen[] = $identifier; + + return true; + }) + ->andReturn(['done' => false, 'progress' => 10]); + + $receiver = new ChunkedAssetReceiver($cloud); + $receiver->receive(test()->workspace, test()->user, 'video.mp4', 'chunk', 0, 99, 1000, 'attempt-a'); + $receiver->receive(test()->workspace, test()->user, 'video.mp4', 'chunk', 0, 99, 1000, 'attempt-b'); + + expect($seen[0])->not->toBe($seen[1]); +}); + // ─── HTTP: local / public assemble path ────────────────────────── test('chunked upload stores video on the local disk via assemble path', function () { @@ -339,3 +396,106 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 $response->assertJson(['done' => true, 'type' => 'video']); Storage::disk('local')->assertExists(test()->workspace->getMedia('assets')->first()->path); }); + +// ─── Concurrent duplicate uploads (regression for Nightwatch #23) ─ +// +// Same user, same filename, same total size, in flight at the same time — +// e.g. the media picker dialog is closed mid-upload and reopened, then the +// same file is uploaded again. Pre-fix these two attempts shared a single +// server-side identifier and stepped on each other's state. + +test('a second attempt completing does not corrupt or crash an in-flight sibling attempt on the multipart cloud path', function () { + config(['filesystems.default' => 'r2', 'filesystems.disks.r2.driver' => 's3']); + Storage::fake('r2'); + seedChunkedUploadWorkspace(); + + $client = Mockery::mock(S3Client::class); + $client->shouldReceive('createMultipartUpload') + ->twice() + ->andReturn(new Result(['UploadId' => 'upload-a']), new Result(['UploadId' => 'upload-b'])); + $client->shouldReceive('uploadPart') + ->times(4) + ->andReturn( + new Result(['ETag' => '"etag-a1"']), + new Result(['ETag' => '"etag-b1"']), + new Result(['ETag' => '"etag-b2"']), + new Result(['ETag' => '"etag-a2"']), + ); + $client->shouldReceive('completeMultipartUpload') + ->twice() + ->andReturn(new Result([])); + + app()->instance( + ChunkedCloudUploader::class, + new ChunkedCloudUploader(Cache::store(), $client, 'test-bucket', 'r2'), + ); + + $total = ChunkedCloudUploader::MIN_PART_BYTES + 50; + $attemptA = (string) Str::uuid(); + $attemptB = (string) Str::uuid(); + + // Random bytes so finfo sniffs "application/octet-stream" on the first + // chunk, which makes detectMimeType() fall back to the .mp4 allow-list + // mime instead of misdetecting a text mime type from repeated bytes. + $firstPartA = random_bytes(ChunkedCloudUploader::MIN_PART_BYTES); + $firstPartB = random_bytes(ChunkedCloudUploader::MIN_PART_BYTES); + + // A0: attempt A starts, first (non-final) part. + postChunkedAsset('clip.mp4', $firstPartA, 0, $total, uploadId: $attemptA) + ->assertSuccessful(); + + // B0: attempt B, identical filename+size, first part. Pre-fix this shares + // A's cache key; since A's next_offset is already > 0 it becomes a no-op + // idempotent replay that silently reuses A's session instead of starting + // its own. + postChunkedAsset('clip.mp4', $firstPartB, 0, $total, uploadId: $attemptB) + ->assertSuccessful(); + + // B1: attempt B's final chunk. Pre-fix this matches A's next_offset + // exactly, so it completes A's own multipart upload using B's bytes as + // part 2 (silent corruption), then forgets the shared cache key. + $doneB = postChunkedAsset('clip.mp4', str_repeat('b', 50), ChunkedCloudUploader::MIN_PART_BYTES, $total, uploadId: $attemptB); + + // A1: attempt A's own final chunk. Pre-fix the cache key is now gone, so + // this throws RuntimeException("Chunked cloud upload session expired or + // missing.") — the exact Nightwatch #23 crash. + $doneA = postChunkedAsset('clip.mp4', str_repeat('a', 50), ChunkedCloudUploader::MIN_PART_BYTES, $total, uploadId: $attemptA); + + $doneA->assertSuccessful(); + $doneA->assertJson(['done' => true]); + $doneB->assertSuccessful(); + $doneB->assertJson(['done' => true]); + + expect($doneA->json('id'))->not->toBe($doneB->json('id')); + expect($doneA->json('path'))->not->toBe($doneB->json('path')); +}); + +test('two concurrent attempts of the same file do not corrupt each other on the local assemble path', function () { + config(['filesystems.default' => 'local']); + Storage::fake('local'); + seedChunkedUploadWorkspace(); + + $header = fakeMp4Bytes(); + $tailA = 'AAAA'; + $tailB = 'BBBB'; + $total = strlen($header) + 4; + $attemptA = (string) Str::uuid(); + $attemptB = (string) Str::uuid(); + + postChunkedAsset('clip.mp4', $header, 0, $total, uploadId: $attemptA)->assertSuccessful(); + postChunkedAsset('clip.mp4', $header, 0, $total, uploadId: $attemptB)->assertSuccessful(); + + $doneA = postChunkedAsset('clip.mp4', $tailA, strlen($header), $total, uploadId: $attemptA); + $doneB = postChunkedAsset('clip.mp4', $tailB, strlen($header), $total, uploadId: $attemptB); + + $doneA->assertSuccessful(); + $doneA->assertJson(['done' => true]); + $doneB->assertSuccessful(); + $doneB->assertJson(['done' => true]); + + $mediaA = Media::find($doneA->json('id')); + $mediaB = Media::find($doneB->json('id')); + + expect(Storage::disk('local')->get($mediaA->path))->toBe($header.$tailA); + expect(Storage::disk('local')->get($mediaB->path))->toBe($header.$tailB); +}); From ba886f9bf5a2b3dca98a659d76537fd058e3abbd Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Sun, 9 Aug 2026 12:57:43 -0300 Subject: [PATCH 2/8] fix: explicitly type upload_id when passing to receive() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matches the existing explicit (int) casts on the sibling validated() calls in the same method — validated() returns mixed, so this keeps the nullable-string contract explicit instead of relying on an implicit runtime type. --- app/Http/Controllers/App/AssetController.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/App/AssetController.php b/app/Http/Controllers/App/AssetController.php index 72f4de961..0f965d01c 100644 --- a/app/Http/Controllers/App/AssetController.php +++ b/app/Http/Controllers/App/AssetController.php @@ -75,6 +75,8 @@ public function storeChunked(StoreChunkedAssetRequest $request, ChunkedAssetRece $this->authorize('createPost', $workspace); + $uploadId = $request->validated('upload_id'); + return $receiver->receive( $workspace, $request->user(), @@ -83,7 +85,7 @@ public function storeChunked(StoreChunkedAssetRequest $request, ChunkedAssetRece (int) $request->validated('range_start'), (int) $request->validated('range_end'), (int) $request->validated('total_size'), - $request->validated('upload_id'), + $uploadId === null ? null : (string) $uploadId, )->toResponse(); } From e83771f69c039824c6f6a6b47fee32278091bafe Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Sun, 9 Aug 2026 13:01:30 -0300 Subject: [PATCH 3/8] style: inline the upload_id null-safe cast Drop the intermediate variable so all receive() arguments read as a single expression each, matching the sibling validated() casts. --- app/Http/Controllers/App/AssetController.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/Http/Controllers/App/AssetController.php b/app/Http/Controllers/App/AssetController.php index 0f965d01c..9c49980b6 100644 --- a/app/Http/Controllers/App/AssetController.php +++ b/app/Http/Controllers/App/AssetController.php @@ -75,8 +75,6 @@ public function storeChunked(StoreChunkedAssetRequest $request, ChunkedAssetRece $this->authorize('createPost', $workspace); - $uploadId = $request->validated('upload_id'); - return $receiver->receive( $workspace, $request->user(), @@ -85,7 +83,7 @@ public function storeChunked(StoreChunkedAssetRequest $request, ChunkedAssetRece (int) $request->validated('range_start'), (int) $request->validated('range_end'), (int) $request->validated('total_size'), - $uploadId === null ? null : (string) $uploadId, + $request->validated('upload_id') === null ? null : (string) $request->validated('upload_id'), )->toResponse(); } From 46ca9b69fffb863d5d59fa18f5e71b564eb1fc78 Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Sun, 9 Aug 2026 13:16:11 -0300 Subject: [PATCH 4/8] fix: require X-Upload-Id instead of falling back to the legacy identifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nullable upload_id only preserved the old (collision-prone) formula for clients that omit the header — it didn't actually protect them. Making it required closes that gap outright: a request without the header now fails loud (422) instead of silently falling back to the vulnerable identifier. ChunkedAssetReceiver::receive() now takes a required $attemptId. Updated every existing test hitting app.assets.store-chunked (ChunkedCloudUploadTest, ChunkedAssetReceiverTest, ChunkedUploadFilenameEncodingTest, AssetControllerTest) to send a real upload id, and added a regression test asserting the endpoint rejects a request with no X-Upload-Id header. --- app/Http/Controllers/App/AssetController.php | 2 +- .../App/Asset/StoreChunkedAssetRequest.php | 3 +- app/Services/Media/ChunkedAssetReceiver.php | 4 +- tests/Feature/AssetControllerTest.php | 4 ++ tests/Feature/ChunkedAssetReceiverTest.php | 5 ++ tests/Feature/ChunkedCloudUploadTest.php | 60 ++++++++----------- .../ChunkedUploadFilenameEncodingTest.php | 3 + 7 files changed, 41 insertions(+), 40 deletions(-) diff --git a/app/Http/Controllers/App/AssetController.php b/app/Http/Controllers/App/AssetController.php index 9c49980b6..e6a772cee 100644 --- a/app/Http/Controllers/App/AssetController.php +++ b/app/Http/Controllers/App/AssetController.php @@ -83,7 +83,7 @@ public function storeChunked(StoreChunkedAssetRequest $request, ChunkedAssetRece (int) $request->validated('range_start'), (int) $request->validated('range_end'), (int) $request->validated('total_size'), - $request->validated('upload_id') === null ? null : (string) $request->validated('upload_id'), + (string) $request->validated('upload_id'), )->toResponse(); } diff --git a/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php b/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php index ba26d017a..8c533f5f4 100644 --- a/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php +++ b/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php @@ -48,7 +48,7 @@ public function rules(): array 'range_end' => ['required', 'integer', 'gte:range_start'], 'total_size' => ['required', 'integer', 'min:1', 'max:'.MediaType::Video->maxSizeInBytes()], 'file_name' => ['required', 'string', 'ends_with:'.implode(',', $allowedSuffixes)], - 'upload_id' => ['nullable', 'string', 'uuid'], + 'upload_id' => ['required', 'string', 'uuid'], ]; } @@ -63,6 +63,7 @@ public function messages(): array 'total_size.required' => 'Invalid Content-Range header', 'total_size.max' => 'File size exceeds the maximum allowed ('.MediaType::Video->maxSizeInMb().' MB).', 'file_name.ends_with' => 'File type not supported.', + 'upload_id.required' => 'Missing X-Upload-Id header', ]; } } diff --git a/app/Services/Media/ChunkedAssetReceiver.php b/app/Services/Media/ChunkedAssetReceiver.php index 5ba73126e..dc4e3ea01 100644 --- a/app/Services/Media/ChunkedAssetReceiver.php +++ b/app/Services/Media/ChunkedAssetReceiver.php @@ -21,9 +21,9 @@ public function receive( int $rangeStart, int $rangeEnd, int $totalSize, - ?string $attemptId = null, + string $attemptId, ): ChunkReceipt { - $identifier = md5($user->id.$fileName.$totalSize.(string) $attemptId); + $identifier = md5($user->id.$fileName.$totalSize.$attemptId); return $this->cloud->shouldUseMultipart($fileName) ? $this->receiveViaMultipart($workspace, $identifier, $fileName, $chunk, $rangeStart, $rangeEnd, $totalSize) diff --git a/tests/Feature/AssetControllerTest.php b/tests/Feature/AssetControllerTest.php index 0db068bb1..be9ae5676 100644 --- a/tests/Feature/AssetControllerTest.php +++ b/tests/Feature/AssetControllerTest.php @@ -11,6 +11,7 @@ use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Storage; +use Illuminate\Support\Str; beforeEach(function () { Storage::fake(); @@ -214,6 +215,7 @@ [ 'HTTP_CONTENT_RANGE' => 'bytes 0-'.($size - 1).'/'.$size, 'HTTP_X_FILE_NAME' => 'test.png', + 'HTTP_X_UPLOAD_ID' => Str::uuid()->toString(), 'HTTP_ACCEPT' => 'application/json', 'CONTENT_TYPE' => 'application/octet-stream', ], @@ -238,6 +240,7 @@ [ 'HTTP_CONTENT_RANGE' => 'bytes 0-'.($size - 1).'/'.$size, 'HTTP_X_FILE_NAME' => 'deck.pdf', + 'HTTP_X_UPLOAD_ID' => Str::uuid()->toString(), 'HTTP_ACCEPT' => 'application/json', 'CONTENT_TYPE' => 'application/octet-stream', ], @@ -257,6 +260,7 @@ [ 'HTTP_CONTENT_RANGE' => 'bytes 0-499/1000', 'HTTP_X_FILE_NAME' => 'test-video.mp4', + 'HTTP_X_UPLOAD_ID' => Str::uuid()->toString(), 'HTTP_ACCEPT' => 'application/json', 'CONTENT_TYPE' => 'application/octet-stream', ], diff --git a/tests/Feature/ChunkedAssetReceiverTest.php b/tests/Feature/ChunkedAssetReceiverTest.php index 50d6a8473..affdb5d33 100644 --- a/tests/Feature/ChunkedAssetReceiverTest.php +++ b/tests/Feature/ChunkedAssetReceiverTest.php @@ -73,6 +73,7 @@ 0, strlen($bytes) - 1, strlen($bytes), + 'attempt-1', ); expect($receipt->done)->toBeTrue(); @@ -97,6 +98,7 @@ 0, 99, $total, + 'attempt-1', ); expect($receipt->done)->toBeFalse(); @@ -127,6 +129,7 @@ 0, 11, 12, + 'attempt-1', ); expect($receipt->done)->toBeTrue(); @@ -151,6 +154,7 @@ 0, 99, 200, + 'attempt-1', ); expect($receipt->done)->toBeFalse(); @@ -183,6 +187,7 @@ 0, 13, 14, + 'attempt-1', ))->toThrow(InvalidArgumentException::class); Storage::assertMissing('medias/orphan.mp4'); diff --git a/tests/Feature/ChunkedCloudUploadTest.php b/tests/Feature/ChunkedCloudUploadTest.php index b2d0fadba..b49a7c50e 100644 --- a/tests/Feature/ChunkedCloudUploadTest.php +++ b/tests/Feature/ChunkedCloudUploadTest.php @@ -211,30 +211,6 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 // ─── Per-attempt identifier (concurrent duplicate uploads) ─────── -test('receive derives the legacy identifier when no upload id is provided', function () { - seedChunkedUploadWorkspace(); - - $totalSize = 1000; - $expected = md5(test()->user->id.'video.mp4'.$totalSize); - - $cloud = Mockery::mock(ChunkedCloudUploader::class); - $cloud->shouldReceive('shouldUseMultipart')->andReturn(true); - $cloud->shouldReceive('receiveChunk') - ->once() - ->withArgs(fn (string $identifier) => $identifier === $expected) - ->andReturn(['done' => false, 'progress' => 10]); - - (new ChunkedAssetReceiver($cloud))->receive( - test()->workspace, - test()->user, - 'video.mp4', - 'chunk', - 0, - 99, - 1000, - ); -}); - test('receive derives a distinct identifier per upload attempt', function () { seedChunkedUploadWorkspace(); @@ -265,7 +241,7 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 seedChunkedUploadWorkspace(); $content = fakeMp4Bytes(); - $response = postChunkedAsset('clip.mp4', $content); + $response = postChunkedAsset('clip.mp4', $content, uploadId: Str::uuid()->toString()); $response->assertSuccessful(); $response->assertJson(['done' => true, 'type' => 'video']); @@ -282,7 +258,7 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 seedChunkedUploadWorkspace(); $content = fakeMp4Bytes(); - $response = postChunkedAsset('clip.mp4', $content); + $response = postChunkedAsset('clip.mp4', $content, uploadId: Str::uuid()->toString()); $response->assertSuccessful(); $response->assertJson(['done' => true, 'type' => 'video']); @@ -300,13 +276,14 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 $part1 = fakeMp4Bytes(); $part2 = str_repeat("\0", 50); $total = strlen($part1) + strlen($part2); + $uploadId = Str::uuid()->toString(); - $mid = postChunkedAsset('clip.mp4', $part1, 0, $total); + $mid = postChunkedAsset('clip.mp4', $part1, 0, $total, uploadId: $uploadId); $mid->assertSuccessful(); $mid->assertJson(['done' => false]); expect(test()->workspace->getMedia('assets')->count())->toBe(0); - $done = postChunkedAsset('clip.mp4', $part2, strlen($part1), $total); + $done = postChunkedAsset('clip.mp4', $part2, strlen($part1), $total, uploadId: $uploadId); $done->assertSuccessful(); $done->assertJson(['done' => true, 'type' => 'video']); expect(test()->workspace->getMedia('assets')->count())->toBe(1); @@ -318,7 +295,7 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 seedChunkedUploadWorkspace(); $content = file_get_contents(__DIR__.'/../fixtures/1x1.png'); - $response = postChunkedAsset('photo.png', $content); + $response = postChunkedAsset('photo.png', $content, uploadId: Str::uuid()->toString()); $response->assertSuccessful(); $response->assertJson(['done' => true, 'type' => 'image']); @@ -348,7 +325,7 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 ]); app()->instance(ChunkedCloudUploader::class, $fake); - $response = postChunkedAsset('clip.mp4', 'fake-video!!'); + $response = postChunkedAsset('clip.mp4', 'fake-video!!', uploadId: Str::uuid()->toString()); $response->assertSuccessful(); $response->assertJson([ @@ -373,7 +350,7 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 app()->instance(ChunkedCloudUploader::class, $mock); $content = file_get_contents(__DIR__.'/../fixtures/1x1.png'); - $response = postChunkedAsset('photo.png', $content); + $response = postChunkedAsset('photo.png', $content, uploadId: Str::uuid()->toString()); $response->assertSuccessful(); $response->assertJson(['done' => true, 'type' => 'image']); @@ -390,13 +367,24 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 $mock->shouldNotReceive('receiveChunk'); app()->instance(ChunkedCloudUploader::class, $mock); - $response = postChunkedAsset('clip.mp4', fakeMp4Bytes()); + $response = postChunkedAsset('clip.mp4', fakeMp4Bytes(), uploadId: Str::uuid()->toString()); $response->assertSuccessful(); $response->assertJson(['done' => true, 'type' => 'video']); Storage::disk('local')->assertExists(test()->workspace->getMedia('assets')->first()->path); }); +test('chunked upload rejects a request with no X-Upload-Id header', function () { + config(['filesystems.default' => 'local']); + Storage::fake('local'); + seedChunkedUploadWorkspace(); + + $response = postChunkedAsset('clip.mp4', fakeMp4Bytes()); + + $response->assertStatus(422); + $response->assertJsonValidationErrors('upload_id'); +}); + // ─── Concurrent duplicate uploads (regression for Nightwatch #23) ─ // // Same user, same filename, same total size, in flight at the same time — @@ -431,8 +419,8 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 ); $total = ChunkedCloudUploader::MIN_PART_BYTES + 50; - $attemptA = (string) Str::uuid(); - $attemptB = (string) Str::uuid(); + $attemptA = Str::uuid()->toString(); + $attemptB = Str::uuid()->toString(); // Random bytes so finfo sniffs "application/octet-stream" on the first // chunk, which makes detectMimeType() fall back to the .mp4 allow-list @@ -479,8 +467,8 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 $tailA = 'AAAA'; $tailB = 'BBBB'; $total = strlen($header) + 4; - $attemptA = (string) Str::uuid(); - $attemptB = (string) Str::uuid(); + $attemptA = Str::uuid()->toString(); + $attemptB = Str::uuid()->toString(); postChunkedAsset('clip.mp4', $header, 0, $total, uploadId: $attemptA)->assertSuccessful(); postChunkedAsset('clip.mp4', $header, 0, $total, uploadId: $attemptB)->assertSuccessful(); diff --git a/tests/Feature/ChunkedUploadFilenameEncodingTest.php b/tests/Feature/ChunkedUploadFilenameEncodingTest.php index 5d0b68861..8dc7ca675 100644 --- a/tests/Feature/ChunkedUploadFilenameEncodingTest.php +++ b/tests/Feature/ChunkedUploadFilenameEncodingTest.php @@ -7,6 +7,7 @@ use App\Models\User; use App\Models\Workspace; use Illuminate\Support\Facades\Storage; +use Illuminate\Support\Str; use Illuminate\Testing\TestResponse; beforeEach(function () { @@ -43,6 +44,7 @@ function postEncodedChunkedUpload(string $fileName, string $content): TestRespon [ 'HTTP_CONTENT_RANGE' => 'bytes 0-'.($size - 1).'/'.$size, 'HTTP_X_FILE_NAME' => rawurlencode($fileName), + 'HTTP_X_UPLOAD_ID' => Str::uuid()->toString(), 'HTTP_ACCEPT' => 'application/json', 'CONTENT_TYPE' => 'application/octet-stream', ], @@ -95,6 +97,7 @@ function postEncodedChunkedUpload(string $fileName, string $content): TestRespon [ 'HTTP_CONTENT_RANGE' => 'bytes 0-'.($size - 1).'/'.$size, 'HTTP_X_FILE_NAME' => 'plain-ascii.png', + 'HTTP_X_UPLOAD_ID' => Str::uuid()->toString(), 'HTTP_ACCEPT' => 'application/json', 'CONTENT_TYPE' => 'application/octet-stream', ], From f11dfd1c16708f4778b949a9d4afa24d654a03e8 Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Sun, 9 Aug 2026 13:22:55 -0300 Subject: [PATCH 5/8] fix: localize hardcoded workspace name validation messages StoreWorkspaceRequest had its custom messages() hardcoded in pt-BR regardless of the user's locale; UpdateWorkspaceRequest had the same bug hardcoded in English. Both now go through __('validation.required' / 'validation.max.string') with the already-localized workspaces.create.name attribute label (present in all 16 lang/ directories), matching the pattern already used by StoreWorkspaceInviteRequest. Unrelated to the chunked upload fix, but caught while reviewing this file's messages() convention. --- app/Http/Requests/App/Workspace/StoreWorkspaceRequest.php | 4 ++-- app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/Http/Requests/App/Workspace/StoreWorkspaceRequest.php b/app/Http/Requests/App/Workspace/StoreWorkspaceRequest.php index e8cda84b5..e87eaae1d 100644 --- a/app/Http/Requests/App/Workspace/StoreWorkspaceRequest.php +++ b/app/Http/Requests/App/Workspace/StoreWorkspaceRequest.php @@ -42,8 +42,8 @@ public function rules(): array public function messages(): array { return [ - 'name.required' => 'O nome do workspace é obrigatório.', - 'name.max' => 'O nome do workspace deve ter no máximo 255 caracteres.', + 'name.required' => __('validation.required', ['attribute' => __('workspaces.create.name')]), + 'name.max' => __('validation.max.string', ['attribute' => __('workspaces.create.name'), 'max' => 255]), ]; } } diff --git a/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php b/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php index 6bb20ef0a..90765ccf0 100644 --- a/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php +++ b/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php @@ -41,8 +41,8 @@ public function rules(): array public function messages(): array { return [ - 'name.required' => 'The workspace name is required.', - 'name.max' => 'The workspace name must be at most 255 characters.', + 'name.required' => __('validation.required', ['attribute' => __('workspaces.create.name')]), + 'name.max' => __('validation.max.string', ['attribute' => __('workspaces.create.name'), 'max' => 255]), ]; } } From 5fd7de0aead4a83707e32b5ec1f851bc7911933a Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Sun, 9 Aug 2026 13:26:02 -0300 Subject: [PATCH 6/8] simplify: drop messages() override on workspace name validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Laravel already localizes the generic required/max messages from lang/{locale}/validation.php automatically — no need to hand-roll messages() for standard rules with no custom copy. --- app/Http/Requests/App/Workspace/StoreWorkspaceRequest.php | 8 -------- .../Requests/App/Workspace/UpdateWorkspaceRequest.php | 8 -------- 2 files changed, 16 deletions(-) diff --git a/app/Http/Requests/App/Workspace/StoreWorkspaceRequest.php b/app/Http/Requests/App/Workspace/StoreWorkspaceRequest.php index e87eaae1d..95265442f 100644 --- a/app/Http/Requests/App/Workspace/StoreWorkspaceRequest.php +++ b/app/Http/Requests/App/Workspace/StoreWorkspaceRequest.php @@ -38,12 +38,4 @@ public function rules(): array 'logo_url' => ['nullable', 'url', 'max:1024'], ]; } - - public function messages(): array - { - return [ - 'name.required' => __('validation.required', ['attribute' => __('workspaces.create.name')]), - 'name.max' => __('validation.max.string', ['attribute' => __('workspaces.create.name'), 'max' => 255]), - ]; - } } diff --git a/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php b/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php index 90765ccf0..ad8add4ee 100644 --- a/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php +++ b/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php @@ -37,12 +37,4 @@ public function rules(): array 'logo_url' => ['nullable', 'url', 'max:1024'], ]; } - - public function messages(): array - { - return [ - 'name.required' => __('validation.required', ['attribute' => __('workspaces.create.name')]), - 'name.max' => __('validation.max.string', ['attribute' => __('workspaces.create.name'), 'max' => 255]), - ]; - } } From 218aadc639151b6b2f9cb906f21514e711a297f8 Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Sun, 9 Aug 2026 13:31:51 -0300 Subject: [PATCH 7/8] fix: localize StoreChunkedAssetRequest validation messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the hardcoded English messages for required/ends_with rules — Laravel's own localized validation.php messages already cover them adequately (ends_with's generic message is actually more useful, since it lists the accepted extensions). total_size.max still needs a custom message (the rule is in raw bytes, unreadable without MB conversion), so it now goes through __('assets.upload.file_too_large') with the key added to all 16 lang/ locales. Also fixed test flakiness discovered while touching this file: ChunkedCloudUploadTest used random_bytes() for the first mp4 chunk, which occasionally collides with an unrelated magic number (MZ/PE, SIMH tape, ...) and makes finfo misdetect the mime type. Replaced with real mp4 header bytes padded with nulls, so detection is deterministic. --- .../Requests/App/Asset/StoreChunkedAssetRequest.php | 7 +------ lang/ar/assets.php | 1 + lang/de/assets.php | 1 + lang/el/assets.php | 1 + lang/en/assets.php | 1 + lang/es/assets.php | 1 + lang/fr/assets.php | 1 + lang/it/assets.php | 1 + lang/ja/assets.php | 1 + lang/ko/assets.php | 1 + lang/nl/assets.php | 1 + lang/pl/assets.php | 1 + lang/pt-BR/assets.php | 1 + lang/ru/assets.php | 1 + lang/tr/assets.php | 1 + lang/uk/assets.php | 1 + lang/zh/assets.php | 1 + tests/Feature/ChunkedCloudUploadTest.php | 11 ++++++----- 18 files changed, 23 insertions(+), 11 deletions(-) diff --git a/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php b/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php index 8c533f5f4..d8740a55a 100644 --- a/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php +++ b/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php @@ -58,12 +58,7 @@ public function rules(): array public function messages(): array { return [ - 'range_start.required' => 'Invalid Content-Range header', - 'range_end.required' => 'Invalid Content-Range header', - 'total_size.required' => 'Invalid Content-Range header', - 'total_size.max' => 'File size exceeds the maximum allowed ('.MediaType::Video->maxSizeInMb().' MB).', - 'file_name.ends_with' => 'File type not supported.', - 'upload_id.required' => 'Missing X-Upload-Id header', + 'total_size.max' => __('assets.upload.file_too_large', ['max' => MediaType::Video->maxSizeInMb()]), ]; } } diff --git a/lang/ar/assets.php b/lang/ar/assets.php index fe4108d9c..372f81409 100644 --- a/lang/ar/assets.php +++ b/lang/ar/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG، PNG، GIF، WebP، MP4، PDF', 'uploading' => 'جارٍ الرفع...', 'failed' => 'تعذر رفع :file. يرجى المحاولة مرة أخرى.', + 'file_too_large' => 'حجم الملف يتجاوز الحد الأقصى المسموح به (:max ميجابايت).', ], 'empty' => [ diff --git a/lang/de/assets.php b/lang/de/assets.php index 8386c33de..09b620558 100644 --- a/lang/de/assets.php +++ b/lang/de/assets.php @@ -16,6 +16,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Wird hochgeladen...', 'failed' => ':file konnte nicht hochgeladen werden. Bitte versuche es erneut.', + 'file_too_large' => 'Die Dateigröße überschreitet das zulässige Maximum (:max MB).', ], 'empty' => [ diff --git a/lang/el/assets.php b/lang/el/assets.php index 9b971ea03..0e94fa7bf 100644 --- a/lang/el/assets.php +++ b/lang/el/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Μεταφόρτωση...', 'failed' => 'Δεν ήταν δυνατή η μεταφόρτωση του :file. Παρακαλούμε δοκιμάστε ξανά.', + 'file_too_large' => 'Το μέγεθος του αρχείου υπερβαίνει το μέγιστο επιτρεπόμενο (:max MB).', ], 'empty' => [ diff --git a/lang/en/assets.php b/lang/en/assets.php index 57483f4ac..50cf03ae6 100644 --- a/lang/en/assets.php +++ b/lang/en/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Uploading...', 'failed' => 'Could not upload :file. Please try again.', + 'file_too_large' => 'File size exceeds the maximum allowed (:max MB).', ], 'empty' => [ diff --git a/lang/es/assets.php b/lang/es/assets.php index 0945d7376..9565078af 100644 --- a/lang/es/assets.php +++ b/lang/es/assets.php @@ -16,6 +16,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Subiendo...', 'failed' => 'No se pudo subir :file. Inténtalo de nuevo.', + 'file_too_large' => 'El tamaño del archivo supera el máximo permitido (:max MB).', ], 'empty' => [ diff --git a/lang/fr/assets.php b/lang/fr/assets.php index 6787392d6..b6cc6d130 100644 --- a/lang/fr/assets.php +++ b/lang/fr/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Import en cours...', 'failed' => 'Impossible d\'importer :file. Veuillez réessayer.', + 'file_too_large' => 'La taille du fichier dépasse le maximum autorisé (:max Mo).', ], 'empty' => [ diff --git a/lang/it/assets.php b/lang/it/assets.php index be12bdeb5..c8bf2cc6a 100644 --- a/lang/it/assets.php +++ b/lang/it/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Caricamento in corso...', 'failed' => 'Impossibile caricare :file. Riprova.', + 'file_too_large' => 'La dimensione del file supera il massimo consentito (:max MB).', ], 'empty' => [ diff --git a/lang/ja/assets.php b/lang/ja/assets.php index f953e2c08..cbc18912b 100644 --- a/lang/ja/assets.php +++ b/lang/ja/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG、PNG、GIF、WebP、MP4、PDF', 'uploading' => 'アップロード中...', 'failed' => ':file をアップロードできませんでした。もう一度お試しください。', + 'file_too_large' => 'ファイルサイズが許容される最大値(:max MB)を超えています。', ], 'empty' => [ diff --git a/lang/ko/assets.php b/lang/ko/assets.php index 328b595e6..1f9881729 100644 --- a/lang/ko/assets.php +++ b/lang/ko/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => '업로드 중...', 'failed' => ':file을(를) 업로드할 수 없습니다. 다시 시도해 주세요.', + 'file_too_large' => '파일 크기가 허용된 최대값(:max MB)을 초과했습니다.', ], 'empty' => [ diff --git a/lang/nl/assets.php b/lang/nl/assets.php index 2261883dc..af0b8feb1 100644 --- a/lang/nl/assets.php +++ b/lang/nl/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Uploaden...', 'failed' => ':file kon niet worden geüpload. Probeer het opnieuw.', + 'file_too_large' => 'Bestandsgrootte overschrijdt het toegestane maximum (:max MB).', ], 'empty' => [ diff --git a/lang/pl/assets.php b/lang/pl/assets.php index 9fbbf7e24..e5e27d75a 100644 --- a/lang/pl/assets.php +++ b/lang/pl/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Przesyłanie...', 'failed' => 'Nie udało się przesłać :file. Spróbuj ponownie.', + 'file_too_large' => 'Rozmiar pliku przekracza dozwolone maksimum (:max MB).', ], 'empty' => [ diff --git a/lang/pt-BR/assets.php b/lang/pt-BR/assets.php index 5da241323..8e5c4494e 100644 --- a/lang/pt-BR/assets.php +++ b/lang/pt-BR/assets.php @@ -16,6 +16,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Enviando...', 'failed' => 'Não foi possível enviar :file. Tente novamente.', + 'file_too_large' => 'O tamanho do arquivo excede o máximo permitido (:max MB).', ], 'empty' => [ diff --git a/lang/ru/assets.php b/lang/ru/assets.php index 5722de788..6649f1e0f 100644 --- a/lang/ru/assets.php +++ b/lang/ru/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Загрузка...', 'failed' => 'Не удалось загрузить :file. Попробуйте ещё раз.', + 'file_too_large' => 'Размер файла превышает максимально допустимый (:max МБ).', ], 'empty' => [ diff --git a/lang/tr/assets.php b/lang/tr/assets.php index f783f101c..4598c9c53 100644 --- a/lang/tr/assets.php +++ b/lang/tr/assets.php @@ -16,6 +16,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Yükleniyor...', 'failed' => ':file yüklenemedi. Lütfen tekrar deneyin.', + 'file_too_large' => 'Dosya boyutu izin verilen maksimumu aşıyor (:max MB).', ], 'empty' => [ diff --git a/lang/uk/assets.php b/lang/uk/assets.php index 774f7d5f9..dd2198e92 100644 --- a/lang/uk/assets.php +++ b/lang/uk/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF', 'uploading' => 'Завантаження...', 'failed' => 'Не вдалося завантажити :file. Спробуйте ще раз.', + 'file_too_large' => 'Розмір файлу перевищує максимально допустимий (:max МБ).', ], 'empty' => [ diff --git a/lang/zh/assets.php b/lang/zh/assets.php index e3cecc474..dcf8378bf 100644 --- a/lang/zh/assets.php +++ b/lang/zh/assets.php @@ -14,6 +14,7 @@ 'formats' => 'JPEG、PNG、GIF、WebP、MP4、PDF', 'uploading' => '上传中…', 'failed' => '无法上传 :file,请重试。', + 'file_too_large' => '文件大小超过允许的最大值(:max MB)。', ], 'empty' => [ diff --git a/tests/Feature/ChunkedCloudUploadTest.php b/tests/Feature/ChunkedCloudUploadTest.php index b49a7c50e..ae35fd75e 100644 --- a/tests/Feature/ChunkedCloudUploadTest.php +++ b/tests/Feature/ChunkedCloudUploadTest.php @@ -422,11 +422,12 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 $attemptA = Str::uuid()->toString(); $attemptB = Str::uuid()->toString(); - // Random bytes so finfo sniffs "application/octet-stream" on the first - // chunk, which makes detectMimeType() fall back to the .mp4 allow-list - // mime instead of misdetecting a text mime type from repeated bytes. - $firstPartA = random_bytes(ChunkedCloudUploader::MIN_PART_BYTES); - $firstPartB = random_bytes(ChunkedCloudUploader::MIN_PART_BYTES); + // Real mp4 magic bytes padded with nulls, so finfo reliably detects + // video/mp4 on the first chunk regardless of libmagic's signature + // database — random/arbitrary byte patterns occasionally collide with + // an unrelated magic number (MZ/PE, SIMH tape, ...) and flake. + $firstPartA = str_pad(fakeMp4Bytes(), ChunkedCloudUploader::MIN_PART_BYTES, "\0"); + $firstPartB = str_pad(fakeMp4Bytes(), ChunkedCloudUploader::MIN_PART_BYTES, "\0"); // A0: attempt A starts, first (non-final) part. postChunkedAsset('clip.mp4', $firstPartA, 0, $total, uploadId: $attemptA) From 13fb95b2093d908bad503e8653943f91643007fa Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Sun, 9 Aug 2026 14:03:23 -0300 Subject: [PATCH 8/8] fix: address final code review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ChunkedAssetReceiver: use double-quoted interpolation instead of concatenation for the identifier hash, per project convention. - AssetControllerTest: two chunked-upload rejection tests didn't send X-Upload-Id, so their 422 assertions could pass for the wrong reason (upload_id.required) instead of the field they claim to cover. Added the header and asserted the specific validation error field. - GalleryBrowser: centralize the upload-in-progress guard as a single check at the top of uploadFiles() instead of three separate checks at each entry point (click/select/drop) — matches the single-source- of-truth pattern already used in PhotoUpload.vue. - GalleryBrowser: show a toast when an in-flight upload is aborted (dialog closed mid-upload) instead of silently discarding it with no feedback. New assets.upload.cancelled key added to all 16 lang/ locales. --- app/Services/Media/ChunkedAssetReceiver.php | 2 +- lang/ar/assets.php | 1 + lang/de/assets.php | 1 + lang/el/assets.php | 1 + lang/en/assets.php | 1 + lang/es/assets.php | 1 + lang/fr/assets.php | 1 + lang/it/assets.php | 1 + lang/ja/assets.php | 1 + lang/ko/assets.php | 1 + lang/nl/assets.php | 1 + lang/pl/assets.php | 1 + lang/pt-BR/assets.php | 1 + lang/ru/assets.php | 1 + lang/tr/assets.php | 1 + lang/uk/assets.php | 1 + lang/zh/assets.php | 1 + .../js/components/assets/GalleryBrowser.vue | 16 ++++++---------- tests/Feature/AssetControllerTest.php | 4 ++++ 19 files changed, 27 insertions(+), 11 deletions(-) diff --git a/app/Services/Media/ChunkedAssetReceiver.php b/app/Services/Media/ChunkedAssetReceiver.php index dc4e3ea01..d09949d77 100644 --- a/app/Services/Media/ChunkedAssetReceiver.php +++ b/app/Services/Media/ChunkedAssetReceiver.php @@ -23,7 +23,7 @@ public function receive( int $totalSize, string $attemptId, ): ChunkReceipt { - $identifier = md5($user->id.$fileName.$totalSize.$attemptId); + $identifier = md5("{$user->id}{$fileName}{$totalSize}{$attemptId}"); return $this->cloud->shouldUseMultipart($fileName) ? $this->receiveViaMultipart($workspace, $identifier, $fileName, $chunk, $rangeStart, $rangeEnd, $totalSize) diff --git a/lang/ar/assets.php b/lang/ar/assets.php index 372f81409..1c59d9b03 100644 --- a/lang/ar/assets.php +++ b/lang/ar/assets.php @@ -15,6 +15,7 @@ 'uploading' => 'جارٍ الرفع...', 'failed' => 'تعذر رفع :file. يرجى المحاولة مرة أخرى.', 'file_too_large' => 'حجم الملف يتجاوز الحد الأقصى المسموح به (:max ميجابايت).', + 'cancelled' => 'تم إلغاء الرفع.', ], 'empty' => [ diff --git a/lang/de/assets.php b/lang/de/assets.php index 09b620558..9ad7195f3 100644 --- a/lang/de/assets.php +++ b/lang/de/assets.php @@ -17,6 +17,7 @@ 'uploading' => 'Wird hochgeladen...', 'failed' => ':file konnte nicht hochgeladen werden. Bitte versuche es erneut.', 'file_too_large' => 'Die Dateigröße überschreitet das zulässige Maximum (:max MB).', + 'cancelled' => 'Upload abgebrochen.', ], 'empty' => [ diff --git a/lang/el/assets.php b/lang/el/assets.php index 0e94fa7bf..6cae0e14e 100644 --- a/lang/el/assets.php +++ b/lang/el/assets.php @@ -15,6 +15,7 @@ 'uploading' => 'Μεταφόρτωση...', 'failed' => 'Δεν ήταν δυνατή η μεταφόρτωση του :file. Παρακαλούμε δοκιμάστε ξανά.', 'file_too_large' => 'Το μέγεθος του αρχείου υπερβαίνει το μέγιστο επιτρεπόμενο (:max MB).', + 'cancelled' => 'Η μεταφόρτωση ακυρώθηκε.', ], 'empty' => [ diff --git a/lang/en/assets.php b/lang/en/assets.php index 50cf03ae6..91f2c9502 100644 --- a/lang/en/assets.php +++ b/lang/en/assets.php @@ -15,6 +15,7 @@ 'uploading' => 'Uploading...', 'failed' => 'Could not upload :file. Please try again.', 'file_too_large' => 'File size exceeds the maximum allowed (:max MB).', + 'cancelled' => 'Upload cancelled.', ], 'empty' => [ diff --git a/lang/es/assets.php b/lang/es/assets.php index 9565078af..e99be7bcf 100644 --- a/lang/es/assets.php +++ b/lang/es/assets.php @@ -17,6 +17,7 @@ 'uploading' => 'Subiendo...', 'failed' => 'No se pudo subir :file. Inténtalo de nuevo.', 'file_too_large' => 'El tamaño del archivo supera el máximo permitido (:max MB).', + 'cancelled' => 'Subida cancelada.', ], 'empty' => [ diff --git a/lang/fr/assets.php b/lang/fr/assets.php index b6cc6d130..9068a3914 100644 --- a/lang/fr/assets.php +++ b/lang/fr/assets.php @@ -15,6 +15,7 @@ 'uploading' => 'Import en cours...', 'failed' => 'Impossible d\'importer :file. Veuillez réessayer.', 'file_too_large' => 'La taille du fichier dépasse le maximum autorisé (:max Mo).', + 'cancelled' => 'Import annulé.', ], 'empty' => [ diff --git a/lang/it/assets.php b/lang/it/assets.php index c8bf2cc6a..68ea2c122 100644 --- a/lang/it/assets.php +++ b/lang/it/assets.php @@ -15,6 +15,7 @@ 'uploading' => 'Caricamento in corso...', 'failed' => 'Impossibile caricare :file. Riprova.', 'file_too_large' => 'La dimensione del file supera il massimo consentito (:max MB).', + 'cancelled' => 'Caricamento annullato.', ], 'empty' => [ diff --git a/lang/ja/assets.php b/lang/ja/assets.php index cbc18912b..851a90f81 100644 --- a/lang/ja/assets.php +++ b/lang/ja/assets.php @@ -15,6 +15,7 @@ 'uploading' => 'アップロード中...', 'failed' => ':file をアップロードできませんでした。もう一度お試しください。', 'file_too_large' => 'ファイルサイズが許容される最大値(:max MB)を超えています。', + 'cancelled' => 'アップロードをキャンセルしました。', ], 'empty' => [ diff --git a/lang/ko/assets.php b/lang/ko/assets.php index 1f9881729..b22d5cdea 100644 --- a/lang/ko/assets.php +++ b/lang/ko/assets.php @@ -15,6 +15,7 @@ 'uploading' => '업로드 중...', 'failed' => ':file을(를) 업로드할 수 없습니다. 다시 시도해 주세요.', 'file_too_large' => '파일 크기가 허용된 최대값(:max MB)을 초과했습니다.', + 'cancelled' => '업로드가 취소되었습니다.', ], 'empty' => [ diff --git a/lang/nl/assets.php b/lang/nl/assets.php index af0b8feb1..77e6fcc90 100644 --- a/lang/nl/assets.php +++ b/lang/nl/assets.php @@ -15,6 +15,7 @@ 'uploading' => 'Uploaden...', 'failed' => ':file kon niet worden geüpload. Probeer het opnieuw.', 'file_too_large' => 'Bestandsgrootte overschrijdt het toegestane maximum (:max MB).', + 'cancelled' => 'Upload geannuleerd.', ], 'empty' => [ diff --git a/lang/pl/assets.php b/lang/pl/assets.php index e5e27d75a..c3a6beab3 100644 --- a/lang/pl/assets.php +++ b/lang/pl/assets.php @@ -15,6 +15,7 @@ 'uploading' => 'Przesyłanie...', 'failed' => 'Nie udało się przesłać :file. Spróbuj ponownie.', 'file_too_large' => 'Rozmiar pliku przekracza dozwolone maksimum (:max MB).', + 'cancelled' => 'Przesyłanie anulowane.', ], 'empty' => [ diff --git a/lang/pt-BR/assets.php b/lang/pt-BR/assets.php index 8e5c4494e..a402f9768 100644 --- a/lang/pt-BR/assets.php +++ b/lang/pt-BR/assets.php @@ -17,6 +17,7 @@ 'uploading' => 'Enviando...', 'failed' => 'Não foi possível enviar :file. Tente novamente.', 'file_too_large' => 'O tamanho do arquivo excede o máximo permitido (:max MB).', + 'cancelled' => 'Envio cancelado.', ], 'empty' => [ diff --git a/lang/ru/assets.php b/lang/ru/assets.php index 6649f1e0f..0413b7092 100644 --- a/lang/ru/assets.php +++ b/lang/ru/assets.php @@ -15,6 +15,7 @@ 'uploading' => 'Загрузка...', 'failed' => 'Не удалось загрузить :file. Попробуйте ещё раз.', 'file_too_large' => 'Размер файла превышает максимально допустимый (:max МБ).', + 'cancelled' => 'Загрузка отменена.', ], 'empty' => [ diff --git a/lang/tr/assets.php b/lang/tr/assets.php index 4598c9c53..45578fc81 100644 --- a/lang/tr/assets.php +++ b/lang/tr/assets.php @@ -17,6 +17,7 @@ 'uploading' => 'Yükleniyor...', 'failed' => ':file yüklenemedi. Lütfen tekrar deneyin.', 'file_too_large' => 'Dosya boyutu izin verilen maksimumu aşıyor (:max MB).', + 'cancelled' => 'Yükleme iptal edildi.', ], 'empty' => [ diff --git a/lang/uk/assets.php b/lang/uk/assets.php index dd2198e92..a2dcea8c9 100644 --- a/lang/uk/assets.php +++ b/lang/uk/assets.php @@ -15,6 +15,7 @@ 'uploading' => 'Завантаження...', 'failed' => 'Не вдалося завантажити :file. Спробуйте ще раз.', 'file_too_large' => 'Розмір файлу перевищує максимально допустимий (:max МБ).', + 'cancelled' => 'Завантаження скасовано.', ], 'empty' => [ diff --git a/lang/zh/assets.php b/lang/zh/assets.php index dcf8378bf..142dccec4 100644 --- a/lang/zh/assets.php +++ b/lang/zh/assets.php @@ -15,6 +15,7 @@ 'uploading' => '上传中…', 'failed' => '无法上传 :file,请重试。', 'file_too_large' => '文件大小超过允许的最大值(:max MB)。', + 'cancelled' => '上传已取消。', ], 'empty' => [ diff --git a/resources/js/components/assets/GalleryBrowser.vue b/resources/js/components/assets/GalleryBrowser.vue index cd7abd810..150afdd45 100644 --- a/resources/js/components/assets/GalleryBrowser.vue +++ b/resources/js/components/assets/GalleryBrowser.vue @@ -225,16 +225,9 @@ watch(uploadsSentinel, async () => { setupUploadsObserver(); }); -const triggerFileInput = () => { - if (uploading.value) return; - fileInput.value?.click(); -}; +const triggerFileInput = () => fileInput.value?.click(); const handleFileSelect = (event: Event) => { const target = event.target as HTMLInputElement; - if (uploading.value) { - target.value = ''; - return; - } if (target.files) { void uploadFiles(Array.from(target.files)); target.value = ''; @@ -242,13 +235,13 @@ const handleFileSelect = (event: Event) => { }; const handleDrop = (event: DragEvent) => { isDragging.value = false; - if (uploading.value) return; if (event.dataTransfer?.files) { void uploadFiles(Array.from(event.dataTransfer.files)); } }; const uploadFiles = async (files: File[]) => { + if (uploading.value) return; uploading.value = true; uploadAbortController = new AbortController(); for (const file of files) { @@ -260,7 +253,10 @@ const uploadFiles = async (files: File[]) => { signal: uploadAbortController.signal, }); } catch (error) { - if (error instanceof DOMException && error.name === 'AbortError') break; + if (error instanceof DOMException && error.name === 'AbortError') { + toast.info(trans('assets.upload.cancelled')); + break; + } toast.error(trans('assets.upload.failed', { file: file.name })); } } diff --git a/tests/Feature/AssetControllerTest.php b/tests/Feature/AssetControllerTest.php index be9ae5676..f124ed3dc 100644 --- a/tests/Feature/AssetControllerTest.php +++ b/tests/Feature/AssetControllerTest.php @@ -280,6 +280,7 @@ [ 'HTTP_CONTENT_RANGE' => 'bytes 0-99/100', 'HTTP_X_FILE_NAME' => 'malware.exe', + 'HTTP_X_UPLOAD_ID' => Str::uuid()->toString(), 'HTTP_ACCEPT' => 'application/json', 'CONTENT_TYPE' => 'application/octet-stream', ], @@ -287,6 +288,7 @@ ); $response->assertUnprocessable(); + $response->assertJsonValidationErrors('file_name'); }); test('chunked upload rejects invalid Content-Range header', function () { @@ -297,6 +299,7 @@ [ 'HTTP_CONTENT_RANGE' => 'invalid', 'HTTP_X_FILE_NAME' => 'test.jpg', + 'HTTP_X_UPLOAD_ID' => Str::uuid()->toString(), 'HTTP_ACCEPT' => 'application/json', 'CONTENT_TYPE' => 'application/octet-stream', ], @@ -306,6 +309,7 @@ // FormRequest validation surfaces parse failures as 422 // (range_start / range_end / total_size all required). $response->assertUnprocessable(); + $response->assertJsonValidationErrors(['range_start', 'range_end', 'total_size']); }); test('chunked upload rejects unauthenticated', function () {