From cd8da6a117c27d14d9b7390cc3813c624438b82f Mon Sep 17 00:00:00 2001 From: denkfabrik-li <274324701+denkfabrik-li@users.noreply.github.com> Date: Fri, 28 Aug 2026 02:25:16 +0200 Subject: [PATCH] Name the quota a client is actually held to when an upload is refused Both chunked-upload quota checks resolve the limit through ClientStorageUsage::quotaBytes(), which falls back to the site default when a client has no quota of their own -- and then print `$user->storage_quota_mb` in the rejection. For every client who was never given an explicit quota that column is 0, so the message reads "This upload would exceed your storage quota of 0 MB." at the one moment somebody is trying to find out what their limit is. The API's single-request upload already prints `$this->storageUsage->quotaMb($user)` for the same sentence (Api/FilesController.php:208). The two chunked copies now do the same. Three tests: the inherited default is named at session creation and again at completion, and a client with a quota of their own still sees their own number. Without the fix the first two go red, the third stays green. --- .../Controllers/ChunkedUploadsController.php | 13 ++++- tests/Feature/Clients/StorageQuotaTest.php | 52 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php b/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php index d44ac85c..ae6ae2f2 100644 --- a/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php +++ b/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php @@ -98,7 +98,14 @@ class ChunkedUploadsController extends Controller if ($quotaBytes > 0 && $this->storageUsage->usedBytes($user) + (int) $validated['size'] > $quotaBytes) { throw ValidationException::withMessages([ - 'size' => __('This upload would exceed your storage quota of :quota MB.', ['quota' => (string) $user->storage_quota_mb]), + 'size' => __('This upload would exceed your storage quota of :quota MB.', [ + // The resolved quota, not the column: a client who + // was never given one of their own carries 0 there + // and inherits the site default, so printing the + // column reads "your storage quota of 0 MB" at the + // moment somebody is asking what their limit is. + 'quota' => (string) $this->storageUsage->quotaMb($user), + ]), ]); } } @@ -306,7 +313,9 @@ class ChunkedUploadsController extends Controller $session->delete(); throw ValidationException::withMessages([ - 'size' => __('This upload would exceed your storage quota of :quota MB.', ['quota' => (string) $user->storage_quota_mb]), + 'size' => __('This upload would exceed your storage quota of :quota MB.', [ + 'quota' => (string) $this->storageUsage->quotaMb($user), + ]), ]); } } diff --git a/tests/Feature/Clients/StorageQuotaTest.php b/tests/Feature/Clients/StorageQuotaTest.php index 37ebab95..b49dd61f 100644 --- a/tests/Feature/Clients/StorageQuotaTest.php +++ b/tests/Feature/Clients/StorageQuotaTest.php @@ -191,6 +191,58 @@ test('a client with no custom quota is limited by the site default once one is s ])->assertJsonValidationErrors('size'); }); +test('the rejection names the quota the client is actually held to', function () { + // storage_quota_mb of 0 means "no quota of their own", and the site + // default is what is then enforced — so the column is the one number + // the message must not print. + app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 1); + $client = User::factory()->client()->create(['storage_quota_mb' => 0]); + grantUploadPermission($client); + makeClientFile($client, 1000 * 1024); + $this->actingAs($client); + + $response = $this->postJson('/uploads', [ + 'filename' => 'over-the-default.pdf', + 'size' => 200 * 1024, + 'type' => 'application/pdf', + ])->assertJsonValidationErrors('size'); + + expect($response->json('errors.size.0'))->toBe('This upload would exceed your storage quota of 1 MB.'); +}); + +test('the same is true when the real byte count is what pushes them over', function () { + // The completion check is a second copy of the same sentence, and had + // the same bug. + app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 1); + $client = User::factory()->client()->create(['storage_quota_mb' => 0]); + grantUploadPermission($client); + makeClientFile($client, 1000 * 1024); + $this->actingAs($client); + + $sessionId = createChunkedSession(11, 'lied-about-size.txt'); + putChunkedPart($sessionId, 1, str_repeat('a', 50 * 1024)); + + $response = $this->postJson("/uploads/{$sessionId}/complete")->assertJsonValidationErrors('size'); + + expect($response->json('errors.size.0'))->toBe('This upload would exceed your storage quota of 1 MB.'); +}); + +test('a client with a quota of their own still sees their own number', function () { + app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 250); + $client = User::factory()->client()->create(['storage_quota_mb' => 1]); + grantUploadPermission($client); + makeClientFile($client, 1000 * 1024); + $this->actingAs($client); + + $response = $this->postJson('/uploads', [ + 'filename' => 'over-their-own.pdf', + 'size' => 200 * 1024, + 'type' => 'application/pdf', + ])->assertJsonValidationErrors('size'); + + expect($response->json('errors.size.0'))->toBe('This upload would exceed your storage quota of 1 MB.'); +}); + test('a client\'s own custom quota is unaffected by later changes to the site default', function () { app(Settings::class)->set(Setting::DefaultClientStorageQuotaMb, 1); $client = User::factory()->client()->create(['storage_quota_mb' => 500]); // an explicit, much larger override