vignesh/trypost
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: chunked upload session collision + workspace name i18n (#263)

Browse source 
* 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.

* fix: explicitly type upload_id when passing to receive()

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.

* 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.

* fix: require X-Upload-Id instead of falling back to the legacy identifier

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.

* 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.

* simplify: drop messages() override on workspace name validation

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.

* fix: localize StoreChunkedAssetRequest validation messages

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.

* fix: address final code review findings

- 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.
This commit is contained in:
Paulo Castellano 2026-08-09 13:06:47 -04:00 committed by GitHub
parent 80888641e5
commit 676afb15ba
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
27 changed files with 238 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
app/Http/Controllers/App/AssetController.php
View file

@ -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'),
(string) $request->validated('upload_id'),
)->toResponse();
}

8
app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php
View file

@ -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' => ['required', 'string', 'uuid'],
];
}
@ -56,11 +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.',
'total_size.max' => __('assets.upload.file_too_large', ['max' => MediaType::Video->maxSizeInMb()]),
];
}
}

8
app/Http/Requests/App/Workspace/StoreWorkspaceRequest.php
View file

@ -38,12 +38,4 @@ public function rules(): array
'logo_url' => ['nullable', 'url', 'max:1024'],
];
}
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.',
];
}
}

8
app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php
View file

@ -37,12 +37,4 @@ public function rules(): array
'logo_url' => ['nullable', 'url', 'max:1024'],
];
}
public function messages(): array
{
return [
'name.required' => 'The workspace name is required.',
'name.max' => 'The workspace name must be at most 255 characters.',
];
}
}

3
app/Services/Media/ChunkedAssetReceiver.php
View file

@ -21,8 +21,9 @@ public function receive(
int $rangeStart,
int $rangeEnd,
int $totalSize,
string $attemptId,
): ChunkReceipt {
$identifier = md5($user->id.$fileName.$totalSize);
$identifier = md5("{$user->id}{$fileName}{$totalSize}{$attemptId}");
return $this->cloud->shouldUseMultipart($fileName)
? $this->receiveViaMultipart($workspace, $identifier, $fileName, $chunk, $rangeStart, $rangeEnd, $totalSize)

2
lang/ar/assets.php
View file

@ -14,6 +14,8 @@
'formats' => 'JPEG، PNG، GIF، WebP، MP4، PDF',
'uploading' => 'جارٍ الرفع...',
'failed' => 'تعذر رفع :file. يرجى المحاولة مرة أخرى.',
'file_too_large' => 'حجم الملف يتجاوز الحد الأقصى المسموح به (:max ميجابايت).',
'cancelled' => 'تم إلغاء الرفع.',
],
'empty' => [

2
lang/de/assets.php
View file

@ -16,6 +16,8 @@
'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).',
'cancelled' => 'Upload abgebrochen.',
],
'empty' => [

2
lang/el/assets.php
View file

@ -14,6 +14,8 @@
'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF',
'uploading' => 'Μεταφόρτωση...',
'failed' => 'Δεν ήταν δυνατή η μεταφόρτωση του :file. Παρακαλούμε δοκιμάστε ξανά.',
'file_too_large' => 'Το μέγεθος του αρχείου υπερβαίνει το μέγιστο επιτρεπόμενο (:max MB).',
'cancelled' => 'Η μεταφόρτωση ακυρώθηκε.',
],
'empty' => [

2
lang/en/assets.php
View file

@ -14,6 +14,8 @@
'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).',
'cancelled' => 'Upload cancelled.',
],
'empty' => [

2
lang/es/assets.php
View file

@ -16,6 +16,8 @@
'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).',
'cancelled' => 'Subida cancelada.',
],
'empty' => [

2
lang/fr/assets.php
View file

@ -14,6 +14,8 @@
'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).',
'cancelled' => 'Import annulé.',
],
'empty' => [

2
lang/it/assets.php
View file

@ -14,6 +14,8 @@
'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).',
'cancelled' => 'Caricamento annullato.',
],
'empty' => [

2
lang/ja/assets.php
View file

@ -14,6 +14,8 @@
'formats' => 'JPEG、PNG、GIF、WebP、MP4、PDF',
'uploading' => 'アップロード中...',
'failed' => ':file をアップロードできませんでした。もう一度お試しください。',
'file_too_large' => 'ファイルサイズが許容される最大値(:max MB)を超えています。',
'cancelled' => 'アップロードをキャンセルしました。',
],
'empty' => [

2
lang/ko/assets.php
View file

@ -14,6 +14,8 @@
'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF',
'uploading' => '업로드 중...',
'failed' => ':file을(를) 업로드할 수 없습니다. 다시 시도해 주세요.',
'file_too_large' => '파일 크기가 허용된 최대값(:max MB)을 초과했습니다.',
'cancelled' => '업로드가 취소되었습니다.',
],
'empty' => [

2
lang/nl/assets.php
View file

@ -14,6 +14,8 @@
'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).',
'cancelled' => 'Upload geannuleerd.',
],
'empty' => [

2
lang/pl/assets.php
View file

@ -14,6 +14,8 @@
'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).',
'cancelled' => 'Przesyłanie anulowane.',
],
'empty' => [

2
lang/pt-BR/assets.php
View file

@ -16,6 +16,8 @@
'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).',
'cancelled' => 'Envio cancelado.',
],
'empty' => [

2
lang/ru/assets.php
View file

@ -14,6 +14,8 @@
'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF',
'uploading' => 'Загрузка...',
'failed' => 'Не удалось загрузить :file. Попробуйте ещё раз.',
'file_too_large' => 'Размер файла превышает максимально допустимый (:max МБ).',
'cancelled' => 'Загрузка отменена.',
],
'empty' => [

2
lang/tr/assets.php
View file

@ -16,6 +16,8 @@
'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).',
'cancelled' => 'Yükleme iptal edildi.',
],
'empty' => [

2
lang/uk/assets.php
View file

@ -14,6 +14,8 @@
'formats' => 'JPEG, PNG, GIF, WebP, MP4, PDF',
'uploading' => 'Завантаження...',
'failed' => 'Не вдалося завантажити :file. Спробуйте ще раз.',
'file_too_large' => 'Розмір файлу перевищує максимально допустимий (:max МБ).',
'cancelled' => 'Завантаження скасовано.',
],
'empty' => [

2
lang/zh/assets.php
View file

@ -14,6 +14,8 @@
'formats' => 'JPEG、PNG、GIF、WebP、MP4、PDF',
'uploading' => '上传中…',
'failed' => '无法上传 :file，请重试。',
'file_too_large' => '文件大小超过允许的最大值(:max MB)。',
'cancelled' => '上传已取消。',
],
'empty' => [

16
resources/js/components/assets/GalleryBrowser.vue
View file

@ -161,6 +161,7 @@ const uploadsSentinel = useTemplateRef<HTMLDivElement>('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(
@ -240,15 +241,22 @@ const handleDrop = (event: DragEvent) => {
};
const uploadFiles = async (files: File[]) => {
if (uploading.value) return;
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') {
toast.info(trans('assets.upload.cancelled'));
break;
}
toast.error(trans('assets.upload.failed', { file: file.name }));
}
}
@ -596,6 +604,7 @@ onUnmounted(() => {
uploadsObserver?.disconnect();
unsplashObserver?.disconnect();
giphyObserver?.disconnect();
uploadAbortController?.abort();
});
</script>
@ -612,7 +621,10 @@ onUnmounted(() => {
<TabsContent value="uploads" class="mt-6">
<div
class="relative mb-4 flex cursor-pointer flex-col items-center justify-center gap-2 rounded-2xl border-2 border-dashed p-8 text-center transition-colors"
:class="isDragging ? 'border-foreground bg-violet-100' : 'border-foreground/25 bg-card hover:bg-foreground/5'"
:class="[
isDragging ? 'border-foreground bg-violet-100' : 'border-foreground/25 bg-card hover:bg-foreground/5',
uploading ? 'pointer-events-none' : '',
]"
@click="triggerFileInput"
@dragover.prevent="isDragging = true"
@dragleave.prevent="isDragging = false"

6
resources/js/utils/chunkedUpload.ts
View file

@ -5,6 +5,7 @@ interface ChunkedUploadOptions {
modelId?: string;
collection?: string;
chunkSize?: number;
signal?: AbortSignal;
onProgress?: (progress: number) => void;
onComplete?: (response: any) => void;
onError?: (error: any) => void;
@ -30,6 +31,7 @@ export const uploadChunked = async (options: ChunkedUploadOptions): Promise<Chun
modelId,
collection = 'default',
chunkSize = DEFAULT_CHUNK_SIZE,
signal,
onProgress,
onComplete,
onError,
@ -38,6 +40,7 @@ export const uploadChunked = async (options: ChunkedUploadOptions): Promise<Chun
const csrfToken = document.querySelector<HTMLMetaElement>('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<Chun
'Content-Type': 'application/octet-stream',
'Content-Range': `bytes ${start}-${end - 1}/${totalSize}`,
'X-File-Name': encodeURIComponent(file.name),
'X-Upload-Id': uploadId,
'X-CSRF-TOKEN': csrfToken,
'X-Requested-With': 'XMLHttpRequest',
Accept: 'application/json',
@ -63,6 +67,7 @@ export const uploadChunked = async (options: ChunkedUploadOptions): Promise<Chun
method: 'POST',
headers,
body: chunk,
signal,
});
if (!response.ok) throw new Error(`Upload chunk failed: ${response.statusText}`);
@ -81,6 +86,7 @@ export const uploadChunked = async (options: ChunkedUploadOptions): Promise<Chun
throw new Error('Upload did not complete');
} catch (error) {
if (error instanceof DOMException && error.name === 'AbortError') throw error;
onError?.(error);
throw error;
}

8
tests/Feature/AssetControllerTest.php
View file

@ -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',
],
@ -276,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',
],
@ -283,6 +288,7 @@
);
$response->assertUnprocessable();
$response->assertJsonValidationErrors('file_name');
});
test('chunked upload rejects invalid Content-Range header', function () {
@ -293,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',
],
@ -302,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 () {

5
tests/Feature/ChunkedAssetReceiverTest.php
View file

@ -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');

179
tests/Feature/ChunkedCloudUploadTest.php
View file

@ -4,13 +4,16 @@
use App\Enums\UserWorkspace\Role;
use App\Models\Account;
use App\Models\Media;
use App\Models\User;
use App\Models\Workspace;
use App\Services\Media\ChunkedAssetReceiver;
use App\Services\Media\ChunkedCloudUploader;
use Aws\Result;
use Aws\S3\S3Client;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Storage;
use Illuminate\Support\Str;
use Illuminate\Testing\TestResponse;
beforeEach(function () {
@ -41,21 +44,27 @@ function fakeMp4Bytes(): string
return "\0\0\0\x18ftypmp42\0\0\0\0mp42isom".str_repeat("\0", 64);
}
function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0, ?int $totalSize = null): TestResponse
function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0, ?int $totalSize = null, ?string $uploadId = null): TestResponse
{
$totalSize ??= strlen($content);
$rangeEnd = $rangeStart + strlen($content) - 1;
$headers = [
'HTTP_CONTENT_RANGE' => "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,30 @@ 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 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 () {
@ -208,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']);
@ -225,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']);
@ -243,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);
@ -261,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']);
@ -291,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([
@ -316,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']);
@ -333,9 +367,124 @@ 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 —
// 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 = Str::uuid()->toString();
$attemptB = Str::uuid()->toString();
// 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)
->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 = 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();
$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);
});

3
tests/Feature/ChunkedUploadFilenameEncodingTest.php
View file

@ -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',
],