Commit Graph

9 Commits

Author SHA1 Message Date
denkfabrik-li cd8da6a117 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.
2026-08-28 06:40:43 +02:00
ignacionelson 835943e1b6 Merge pull request #1686 from denkfabrik-li/fix/chunked-upload-complete-lock
Finalise each chunked upload once, under a per-session lock

Resolved a trivial conflict in ChunkedUploadsTest: this branch and
d7e639b both append tests to the end of the file, so both are kept.
2026-08-26 22:26:28 -03:00
Ignacio Nelson d36abd73ba Merge pull request #1682 from denkfabrik-li/fix/chunked-upload-max-size
Enforce the max file size against the bytes a chunked upload assembles
2026-08-26 22:23:50 -03:00
ignacionelson d7e639b7af Close the two-request version of the deleted-folder target, and say why it failed
Follow-up to #1703, which made `exists:folders,id` mean what its ten
readers already assumed. Two things it named and deliberately left.

**The chunked upload is two requests.** store()'s rule only ever sees the
first: POST /uploads records the resolved folder on the UploadSession and
complete() reads it back from the session rather than from the caller, so
deleting the folder while the bytes are in flight still files the
assembled file into it -- the same orphan state #1703 removes, reached by
a door a validation rule cannot watch. complete() now re-resolves through
Folder::query() and files at the root when the folder has gone.

Root rather than a refusal, because the two moments cost different
things. At store() nothing has been sent, so refusing is free and honest,
which is the call #1703 made. Here the bytes are already uploaded, and
discarding somebody's finished transfer over a folder that vanished
underneath them is the harsher of the two surprises. The file lands
somewhere they can see it and move it.

**The refusal now explains itself.** "The selected folder id is invalid"
says nothing when the answer is that the folder has been deleted -- and
that is the usual way to meet this rule, since a live id picked from a
list is how anybody gets here. It matters most on the chunked path, the
one place #1703 makes a previously-working request fail. A small
ValidationRule object carries the message, which keeps the single
definition Rules::folderId() exists for: a messages() array would have to
be repeated at all ten call sites, and rules meaning different things in
ten places is what went wrong in the first place.

One note for whoever writes the next test here. Upload parts live in
storage_path('app/uploads-tmp/{session_id}'), which is a real shared
directory rather than a faked disk, and each parallel worker's database
restarts session ids at 1 -- so two files writing parts on two workers
collide, and ChunkedUploadsTest's afterEach deletes the whole tree for
everybody. Six test files write parts today. These two cases live in
ChunkedUploadsTest rather than beside the rest of their subject so this
change does not add a seventh racer; the underlying isolation problem
predates it and is worth its own fix.
2026-08-26 17:41:07 -03:00
denkfabrik-li 7727ad7616 Say what exists:folders,id was already being read as
Folder uses SoftDeletes. The `exists` rule runs against the table, so a
folder in the trash passes it -- while every resolution that follows goes
through Folder::query(), which honours the soft delete and finds nothing.
Ten rules across five controllers rely on that check, and each one reads
it as "this folder exists".

Two of them then wrote the id anyway. Api\FilesController::store()
resolves the folder, hands the null to Folder::uploadableBy(), is told
yes -- correctly, that is the rule for a root upload -- and passes
$validated['folder_id'] to the write. FilesController::store() is the
same shape once #1694 gives it the guard. FilesController::update() and
its API twin write it straight through with nothing in between.

The result is a live file inside a deleted folder, which is a state
nothing else in the application produces: FolderService::delete() deletes
every file in the subtree along with it. The row is reachable by id, in
search and over the API, and missing from the listing its uploader would
look in.

Rules::folderId() makes the check mean what its readers assume, once,
where the reasoning can be written down -- the same argument slug() makes
for itself one method above. Every site takes it, so the file cannot end
up with two spellings of the same rule and no way to tell which is the
safe one.

What changes, path by path:

  - POST /files, POST /api/v1/files, PATCH /files/{file} and
    PATCH /api/v1/files/{file} refuse a folder in the trash instead of
    writing its id. This is the fix.
  - POST /uploads used to accept it and quietly file the upload at the
    root -- its guard and its write already agreed, on null. It now says
    so instead, which is what the other upload paths do.
  - files/{file}/move, files/bulk-edit, folders, folders/{folder}/move
    and the portal's my-folders already refused, through
    StaffLibraryScope::folders() or Folder::scopeVisibleToClient(), both
    of which drop trashed rows. They still refuse; the answer is now 422
    naming folder_id rather than a bare 404. Those two guards are asking
    a different question -- "is this folder yours" -- and they keep
    asking it.

No live folder id behaves differently anywhere, and the root (a null
folder_id) is untouched.

The published API document is unchanged: `exists` renders the same either
way. Regenerated with php artisan scramble:export and byte-identical.
2026-08-26 06:01:06 +02:00
denkfabrik-li 329aef98e0 Finalise each chunked upload once, under a per-session lock
complete() assembled the received parts into the one target file and
created the File row with no guard against a second complete() for the
same session running at the same time -- an Uppy retry, a double submit,
a resend after a lost connection. Two of them would interleave writes
into the session's single `assembled` file (the stored bytes then no
longer match the checksum computed from the in-memory buffers) and could
each create a File row.

Take a per-session lock around the finalisation and fail a second caller
fast; the lock's TTL releases the claim if a completion dies mid-flight,
so a genuine retry still works. The body moves to a finalise() helper so
complete() reads as auth + lock + finalise.
2026-08-25 22:13:46 +02:00
denkfabrik-li 71d6b8937e Enforce the max file size against the bytes a chunked upload assembles
store() checks Setting::MaxFileSizeMb against the size the client declares
when it opens the session, and complete() re-checks the storage quota
against the real assembled byte count -- but nothing re-checked the size
limit itself. A client that declared a one-byte upload and then streamed
gigabytes of parts passed store()'s check and was never stopped, so the
configured limit (which store() applies to everyone, staff included) did
not hold for the resumable path that real uploads use.

Re-check the assembled byte count against MaxFileSizeMb in complete(),
cleaning up the assembled bytes and the session exactly as the quota
branch already does.
2026-08-25 20:32:13 +02:00
Ignacio Nelson 9b265e3b87 Say what a stored upload part's receipt actually is (#1617)
An empty response gets Symfony's default Content-Type, text/html, and a
CDN in front of the app takes that at its word: Cloudflare's Email
Obfuscation and Automatic HTTPS Rewrites both rewrite HTML bodies, so
they drop the origin's ETag from the response — a rewritten body would
no longer match it.

That ETag is the client's only signal that a part landed. Nothing on
this side notices its loss, because LocalPartStore keeps its own record
of every part and complete() never reads a client-supplied one; the
upload simply reaches 100% and stops, with no error at either end.
Reported from a Cloudflare-fronted install (#1616), where the visible
symptom was Uppy's "Could not read the ETag header" — which names CORS,
and sends you chasing a preflight that same-origin requests never make.

Naming the content type accurately keeps the response out of every
HTML-rewriting path there is, rather than asking each CDN-fronted
install to discover this one for itself.

The rationale sits inside the header array rather than above the return:
Scramble reads a comment attached to a return statement as that
response's description in the published OpenAPI document, and this
controller is mounted on the API routes too.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-14 11:51:46 -03:00
ignacionelson 6e47d76ba6 ProjectSend 2.0.0
Client file sharing, rebuilt from the ground up: a private area per
client, resumable uploads, folders, groups and categories, sharing with
expiry dates and download limits, comments, file versions, an activity
log, a REST API, and sixteen languages.

This repository begins here. ProjectSend 2 was developed privately, and
that development history is not published — the previous generation
remains available, with its own history, at projectsend/legacy.

Free software under the GNU General Public License v2, or (at your
option) any later version.
2026-08-14 01:38:12 -03:00