Files
projectsend/app/Modules/Comments/Http/Controllers/FileCommentsController.php
T
ignacionelson 7c7ba7cd53 Stop a typed-in storage quota from 500ing when a client is created
Filling the "Storage quota (MB)" field on the new-client form raised a
TypeError and the request died with a 500. Leaving it blank worked, which
is why it reached a release: that path goes through `null ?? 0`, and the
0 is an int.

The `integer` validation rule checks that a value looks like an integer.
It does not convert it. `$request->validate()` returns the raw input, so
the form field arrives as the string "2048" -- and the create form types
that field as a string in React, so it is a string even over JSON. Both
controllers declare strict_types, so handing it to
`ClientAccounts::create()`'s `int $storageQuotaMb` is a TypeError.

Fixed on both surfaces that call create(): the staff screen and
/api/v1/clients. The API twin had the same defect, reachable by sending
the quota as a quoted JSON value or a form-encoded body -- its own create
test only ever sent a JSON number.

Two more call sites had the same shape and are cast too, though nothing
sends them a string today: the share-link download cap and a comment's
reply_to. Both are safe only because a frontend file happens to call
Number() first, which is a fact about that file rather than anything the
signature guarantees. The null in each is preserved rather than collapsed
to 0 -- "no cap" is not a cap of zero.

`storage_quota_mb` is also cast on User and Invitation. The column is an
unsignedInteger and both docblocks already promise int; it is read
straight into provision()'s typed parameter when an invitation is
redeemed, and which type a driver hands back is not something that call
site should depend on.

Found on the new files-test rehearsal instance, on its first real use,
against the same build the whole fleet is running.
2026-09-12 21:16:49 -03:00

147 lines
5.0 KiB
PHP

<?php
declare(strict_types=1);
namespace App\Modules\Comments\Http\Controllers;
use App\Http\Controllers\Controller;
use App\Models\User;
use App\Modules\Comments\Access\VisibleCommentScope;
use App\Modules\Comments\CommentPresenter;
use App\Modules\Comments\CommentVisibility;
use App\Modules\Comments\FileComments;
use App\Modules\Comments\Models\FileComment;
use App\Modules\Files\Models\File;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Gate;
use Illuminate\Validation\Rule;
/**
* The comment thread as JSON, for both staff and clients — one endpoint,
* because the two differ only in what the scope returns them, not in
* anything this controller does.
*
* Note what this endpoint will not accept: a client id. Answering one
* client is `reply_to`, a comment this viewer can already see, so the
* worst a hand-rolled request can do is answer a conversation it was
* shown. Nothing here can point a comment at an arbitrary client.
*/
class FileCommentsController extends Controller
{
public function __construct(
private readonly FileComments $comments,
private readonly CommentPresenter $presenter,
private readonly VisibleCommentScope $scope,
) {}
public function index(Request $request, File $file): JsonResponse
{
$viewer = $request->user();
assert($viewer !== null);
Gate::forUser($viewer)->authorize('view', $file);
return response()->json($this->payload($viewer, $file));
}
public function store(Request $request, File $file): JsonResponse
{
$viewer = $request->user();
assert($viewer !== null);
Gate::forUser($viewer)->authorize('view', $file);
$validated = $request->validate([
'body' => ['required', 'string', 'max:5000'],
'visibility' => ['required', Rule::enum(CommentVisibility::class)],
'reply_to' => ['nullable', 'integer'],
]);
$this->comments->post(
$file,
$viewer,
CommentVisibility::from($validated['visibility']),
$validated['body'],
// Cast for the reason ShareLinksController gives: `integer`
// does not convert, and replyTarget() takes a strict ?int.
$this->replyTarget($viewer, $file, isset($validated['reply_to']) ? (int) $validated['reply_to'] : null),
);
return response()->json($this->payload($viewer, $file), 201);
}
public function update(Request $request, FileComment $comment): JsonResponse
{
$viewer = $request->user();
assert($viewer !== null);
Gate::forUser($viewer)->authorize('update', $comment);
$validated = $request->validate(['body' => ['required', 'string', 'max:5000']]);
$this->comments->edit($comment, $validated['body']);
return response()->json($this->payloadAfterChange($viewer, $comment->file));
}
public function destroy(Request $request, FileComment $comment): JsonResponse
{
$viewer = $request->user();
assert($viewer !== null);
Gate::forUser($viewer)->authorize('delete', $comment);
$file = $comment->file;
$this->comments->remove($comment);
return response()->json($this->payloadAfterChange($viewer, $file));
}
/**
* The thread for the two routes that bind a file, after their own
* `view` authorization has passed.
*
* @return array<string, mixed>
*/
private function payload(User $viewer, File $file): array
{
return $this->presenter->thread($viewer, $file);
}
/**
* The thread that goes back with a change to one comment.
*
* update() and destroy() bind a comment rather than a file, so nothing
* in the request has established that this viewer may read the file's
* conversation — only that this one comment is theirs to change.
* Somebody who commented through the public listing is exactly that
* person, and refusing them on their own edit would be wrong, so the
* reading they get back is the one the file's own gate allows them:
* the public page's, if that is how they arrived.
*
* @return array<string, mixed>
*/
private function payloadAfterChange(User $viewer, File $file): array
{
return $this->presenter->thread(
$viewer,
$file,
viewerMaySeeFile: Gate::forUser($viewer)->allows('view', $file),
);
}
/**
* The comment being answered, resolved through the same scope that
* decided what this viewer may read. A reply can therefore only ever
* join a conversation they were already shown — an id they were not
* given resolves to null and the comment is treated as a fresh one,
* rather than 403ing on something they cannot be told exists.
*/
private function replyTarget(User $viewer, File $file, ?int $id): ?FileComment
{
if ($id === null) {
return null;
}
return $this->scope->for($viewer, $file)->whereKey($id)->first();
}
}