[25/33] Shares: count opening a link apart from downloading from it - #414
Merged
Merged
Conversation
added 7 commits
September 25, 2026 09:24
…ument The Document Server does not send the document back: it sends a URL to fetch it from. The file was emptied first and then written as the answer arrived, so a stream that stopped arriving — a dropped network, a Document Server restarting mid-answer — left an empty file where the work had been. The state the save replaced was gone in every case anyway, since nothing kept it. Both are the same change. The document is pulled into a file of its own beside it and handed to the versions, which keep what it replaces and put the new content in place only once it is whole. A save nobody could finish therefore leaves the document exactly as it was, and the history the engine has been keeping since it arrived is finally written by somebody: the versions have been inert until now, because nothing in the application saved over a file. A save on purpose — the editor's own Save, or the last one made once everybody has left — is marked as such, so the automatic saves in between do not each stand as a state of their own; everyone editing together shares the document key, which is the session those saves belong to. The person credited is the one whose changes the callback carries, or the account the editing session was opened for when it carries none; somebody who came through a share link is credited as the link, since there is no account to name. Seven tests against a Document Server that hands the document over, refuses to, or dies halfway through. Five of them fail on the previous write: the document is emptied by a download that never finishes, and nothing is kept.
The callback says where the saved document is, and the server fetched whatever it was told to. The callback is answered without a session — it is how a separate service reaches us — so anybody able to call it could name any address the server can reach: one on the machine itself, one inside the container's network, a metadata service. The answer was then written into a file in the volume, where it could be read back. The URL now has to come from the Document Server the document was handed to. `ONLYOFFICE_DOWNLOAD_ORIGINS` declares the others it may report itself under, which happens behind a proxy or inside a container network; the configured address is always allowed, and a refusal says in the log which origin was turned away and which are allowed, since that is the only way to tell this apart from a Document Server that cannot be reached. The test answers perfectly well from a server that is not the Document Server, so a refusal cannot be mistaken for a network failure: without the check the document is replaced by what that server handed over.
Collabora hands the document over itself, so nothing here could lose a file the way an unfinished download could. What was missing is the other half: the state each save replaced was dropped, so a document edited all afternoon in Collabora had no history at all, while the same document saved from anywhere else now does. The save goes through the versions. Who saved it is the account the editing session was opened for, and the editor is recorded beside it, so a history shows where each state came from. Collabora saves on its own every few minutes. Those saves belong to the session rather than standing as states of their own, and the session is the lock every co-editor of the document holds — one per open document, which is exactly what a session is. A save somebody asked for, or the one made on closing, is marked as such and is kept even when the timer saves over it a minute later. Five tests, all failing on the plain rename: what a save replaced is gone, and nothing says who saved or how.
`/api/thumbnails` resolves the path, checks the access, and hands back a URL under `/static/thumbnails`. That directory is served by express.static, which the authentication middleware does not cover, and a thumbnail's cache name is derived from the file's path — so the name is guessable, and anybody who could reach the server could fetch the picture of a file they may not open. The 200 against a 404 answered "does this file exist" besides. A session would not settle it: it says who is asking, not what they were cleared to see. A visitor holding a perfectly valid session for one share could name a thumbnail belonging to another share, or to a private folder, and it would be served. So the decision travels with the URL. `/api/thumbnails` signs the one filename it has just cleared, and the static handler verifies that signature — no database, no session, nothing else to get wrong. The signature lasts a day, which is the guest session's own lifetime; a page left open longer asks the API again, and the access check runs again with it. The key is derived from the session secret, so nothing new has to be configured, and a restart that loses it only means open pages fetch their thumbnails again. Nothing else produces such a URL — the service is called from that route and nowhere else — so no other path had to learn about the token. Eight tests: seven on the static handler (no token, a token for another file, an expired one, one whose expiry was pushed out by hand, a nested path with the same basename, a malformed URL, and authentication switched off), and one that walks the whole way — ask the API, follow the URL it gives, then ask for the same picture without what the API attached. Five of the first seven fail without the handler; the last one fails without the signature.
Copying a tree in JavaScript walks it one entry at a time on the only thread the server has. A folder of a hundred thousand files is a hundred thousand trips through the event loop, and everything else the server was doing waits behind them — listings, searches, other people's uploads. rsync does the same work in one process, off that thread entirely, and the image now carries it. Which engine runs is a setting rather than a constant read from the platform at load time. That mattered less for configuring it than for testing it: frozen from `process.platform`, the native path could never run on a developer's machine and the JavaScript path could never run in the container, so each half was only ever exercised where it was chosen and neither was exercised by a test. `FILE_TRANSFER_ENGINE=native` or `=stream` names either one; the default is what it was. rsync is asked for before anything is written, so an image without it copies in JavaScript from the start rather than discovering it is missing halfway through a tree. It is called with `-rlt` rather than `-a`: recursion, links and times are what the JavaScript path gives, and asking for permissions as well makes rsync fail outright on a filesystem that refuses to set them — an SMB or FUSE mount, where the copy used to succeed. Found while making the two engines agree: `fs.cp` resolved a relative symbolic link inside the tree and wrote it out as an absolute path into the *source* tree, so the copy pointed back at the original and broke entirely once that was moved or deleted. Kept verbatim now, as rsync keeps it and as the trash's own copy already did. Three tests run the same tree through both engines — files, nested folders, an empty one, a link — and require the same tree out of each, with rsync taken off the PATH for the third. Two fail without the link fix; all three fail with the native engine taken away.
An upload wrote until the filesystem refused it. That is not only a failed upload: where `/config` sits on the same filesystem, which is the ordinary single-volume deployment, SQLite stops being able to write at that moment and the whole application stops working — for everybody, not for the person uploading. `UPLOAD_STORAGE_RESERVE` has been in the configuration since the trash landed, as the floor the trash keeps free. Nothing asked about it before writing. The upload now does, before a byte goes down: what is coming is only known from the request's own declaration, and a client that declares nothing is still held to the reserve, which is the number that matters here. The trash gives its space back first. It holds room that is recoverable, so the volume's trash purges its oldest items — but only when that is enough to let the upload through: emptying somebody's trash for an upload that is refused anyway would destroy their deleted files for nothing. Another volume's trash is never touched. A filesystem whose free space cannot be read — `statfs` is not everywhere — is not a reason to refuse anybody, so the guard stays silent there. It narrows a window rather than closing it: two uploads racing can each be told there is room, and the reserve absorbs what gets through. `InsufficientStorageError` is new, and answers 507. Eight tests: four on the guard with the trash behind it (it fits once the oldest items go, it is refused with the trash untouched when they would not be enough, another volume's trash is left alone, and nothing is taken when the upload fits anyway), and four through the upload route itself — refused with nothing written, accepted when there is room, refused for the reserve alone, and allowed through where the free space cannot be read. The two that name the refusal fail when the guard is taken out of the upload.
The number an owner was shown as a share's downloads counted every page load. Opening the link raised it, and so did reloading the page, because opening and downloading went through the same counter — a link opened twenty times and never downloaded from read as twenty downloads, and nothing at all said how often it had been opened. They are two numbers now. Opening the link counts an open; a file leaving through it, or a folder leaving as a zip, counts a download. Either way the share remembers when it was last reached, and from where — the address is kept beside the count, which is what makes a link somebody has passed around recognisable as one. Schema v14 adds the two columns to the shares, defaulting to nothing, so an existing installation reads zero opens until it is opened again and keeps every download it had already counted. Both numbers are in what the API answers, and the old field keeps its name and its meaning — it is simply true now.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Batch 25 of the plan in #373: what a share's numbers mean. Stacked on #413 — merge in order; this PR's own changes: cerede2000/NextExplorer@upstream-24-upload-space...upstream-25-share-counters
The number an owner is shown as a share's downloads counts every page load. Opening the link raises it, and so does reloading the page, because opening and downloading go through the same counter: a link opened twenty times and never downloaded from reads as twenty downloads. Nothing says how often it has been opened.
What it does
Notes for review
accessCountbesidedownloadCount), and an interface that reads the old field keeps working.addColumnIfMissingthe earlier ones use, so a/configshared with another build is safe.How it was checked
On
mainat 7b07337 with #410, #411, #412 and #413 applied, Node 24 as the CI and the image use:mainuntouched;