Fix premature 413 responses for concurrent upload chunks

app/Modules/Files/Http/Controllers/ChunkedUploadsController.php

@@ -235,7 +235,10 @@ class ChunkedUploadsController extends Controller
         // chooses its own chunking and only the last part is short.
         $limit = $maxPartBytes * 2;
 
-        if ($request->header('Content-Length') !== null && (int) $request->header('Content-Length') > $limit) {
+        $contentLength = $request->header('Content-Length');
+        $reservationLimit = $contentLength !== null ? (int) $contentLength : $limit;
+
+        if ($reservationLimit < 1 || $reservationLimit > $limit) {
             abort(413);
         }
 
@@ -254,7 +257,12 @@ class ChunkedUploadsController extends Controller
         // what a part gets is whatever the session has left, and the write
         // is then capped at exactly that — an over-long body is cut off
         // mid-stream as it always was, just against a smaller number.
-        $reserve = $this->reservePartRoom($session, $part, $limit);
+        // Reserve the declared request length when available. Reserving the
+        // full per-part ceiling (40 MiB for a normal 20 MiB chunk) makes
+        // concurrent final parts exhaust the session allowance prematurely.
+        // Unknown-length requests retain the conservative ceiling, and the
+        // streamed byte count is still enforced against the reservation.
+        $reserve = $this->reservePartRoom($session, $part, $reservationLimit);
 
         if ($reserve < 1) {
             // 413 rather than 422: this is about the size of what is being
@@ -264,9 +272,10 @@ class ChunkedUploadsController extends Controller
             abort(413);
         }
 
-        $stream = $request->getContent(true);
+        $stream = null;
 
         try {
+            $stream = $request->getContent(true);
             $etag = $this->parts->storePart($session, $part, $stream, $reserve);
         } catch (PartTooLargeException) {
             abort(413);

tests/Feature/Files/ChunkedUploadsTest.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 use App\Models\User;
 use App\Modules\Audit\Action;
 use App\Modules\Audit\ActivityLog;
+use App\Modules\Files\Http\Controllers\ChunkedUploadsController;
 use App\Modules\Files\Models\File;
 use App\Modules\Files\Uploads\UploadSession;
 use App\Modules\Identity\Models\RolePermission;
@@ -12,6 +13,7 @@ use App\Modules\Identity\Permissions\Permission;
 use App\Modules\Identity\Permissions\SystemRole;
 use App\Modules\Platform\Settings\Setting;
 use App\Modules\Platform\Settings\Settings;
+use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\Storage;
 use Illuminate\Testing\TestResponse;
@@ -534,6 +536,92 @@ test('staged bytes are released when a part is replaced, refused or falls short'
     expect($session()->staged_bytes)->toBe(10);
 });
 
+test('concurrent final parts reserve their declared lengths without exhausting the session', function () {
+    $this->actingAs($this->admin);
+    config(['projectsend.upload_part_size_mb' => 1]);
+
+    $contents = [
+        str_repeat('a', 1024 * 1024),
+        str_repeat('b', 1024 * 1024),
+        str_repeat('c', 1024 * 1024),
+        str_repeat('d', 1024 * 1024),
+        'last-part',
+    ];
+    $size = array_sum(array_map(strlen(...), $contents));
+    $sessionId = createSession($size, 'concurrent.txt');
+    $fibers = [];
+
+    foreach ($contents as $index => $content) {
+        $request = new class extends Request
+        {
+            public function getContent($asResource = false)
+            {
+                // All requests reserve room before any request finishes
+                // reading. Fibers make this interleaving deterministic
+                // without separate processes or a shared test database.
+                Fiber::suspend();
+
+                return parent::getContent($asResource);
+            }
+        };
+        $request->initialize([], [], [], [], [], ['CONTENT_LENGTH' => (string) strlen($content)], $content);
+        $request->setUserResolver(fn () => $this->admin);
+        $session = UploadSession::query()->findOrFail($sessionId);
+        $fiber = new Fiber(fn () => app(ChunkedUploadsController::class)->putPart($request, $session, $index + 1));
+        $fiber->start();
+        expect($fiber->isSuspended())->toBeTrue();
+        $fibers[] = $fiber;
+    }
+
+    expect(UploadSession::query()->findOrFail($sessionId)->staged_bytes)->toBe($size);
+
+    foreach ($fibers as $fiber) {
+        $fiber->resume();
+        expect($fiber->getReturn()->getStatusCode())->toBe(200);
+    }
+
+    expect(UploadSession::query()->findOrFail($sessionId)->staged_bytes)->toBe($size);
+
+    $response = $this->postJson("/uploads/{$sessionId}/complete")->assertOk();
+    $file = File::query()->findOrFail($response->json('file_id'));
+    expect($file->size)->toBe($size)
+        ->and($file->checksum)->toBe(hash('sha256', implode('', $contents)))
+        ->and(Storage::disk('files')->get($file->path))->toBe(implode('', $contents));
+});
+
+test('a declared part length is enforced while streaming and refunded after rejection', function () {
+    $this->actingAs($this->admin);
+    $sessionId = createSession(10);
+    $url = $this->getJson("/uploads/{$sessionId}/parts/1/sign")->assertOk()->json('url');
+
+    $this->call('PUT', $url, [], [], [], ['CONTENT_LENGTH' => '2'], 'four')->assertStatus(413);
+
+    expect(UploadSession::query()->findOrFail($sessionId)->staged_bytes)->toBe(0)
+        ->and(is_file(partsRoot().'/'.$sessionId.'/1.part'))->toBeFalse();
+
+    $this->call('PUT', $url, [], [], [], ['CONTENT_LENGTH' => '4'], 'four')->assertOk();
+    expect(UploadSession::query()->findOrFail($sessionId)->staged_bytes)->toBe(4);
+});
+
+test('a failure opening the request stream releases its reservation', function () {
+    $this->actingAs($this->admin);
+    $sessionId = createSession(10);
+    $session = UploadSession::query()->findOrFail($sessionId);
+    $request = new class extends Request
+    {
+        public function getContent($asResource = false)
+        {
+            throw new RuntimeException('Could not open request stream.');
+        }
+    };
+    $request->headers->set('Content-Length', '4');
+    $request->setUserResolver(fn () => $this->admin);
+
+    expect(fn () => app(ChunkedUploadsController::class)->putPart($request, $session, 1))
+        ->toThrow(RuntimeException::class, 'Could not open request stream.');
+    expect($session->fresh()->staged_bytes)->toBe(0);
+});
+
 test('open sessions count against a client quota, so it cannot be spent twice', function () {
     $client = User::factory()->client()->create(['storage_quota_mb' => 1]);
     grantChunkedUploadPermission($client);