[22/33] Thumbnails: serve one only to somebody who was cleared to see it - #411
Merged
Merged
Conversation
added 4 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.
vikramsoni2
pushed a commit
that referenced
this pull request
Sep 26, 2026
A thumbnail was made inside the request that asked for it. A folder of five hundred pictures is five hundred held requests; a video on a slow disk holds one for minutes; and nothing could be said about which of them mattered, so the tile somebody was looking at waited behind the one they had scrolled past. The request now answers with what is already there, or says it has queued the work and asks the caller to come back. What is worth doing first is decided in the queue, where it can be: a thumbnail somebody is waiting for goes ahead of one being fetched in advance, and a prefetch is admitted only while nothing interactive is running. The browser asks again, with increasing delays, rather than holding a connection open per tile. Making them is the other half. Video thumbnails go through ffmpeg with their own concurrency, their own seek point, their own thread count and a generous ceiling — a thumbnail killed early is a thumbnail that never appears. RAW files hand over the full-size preview their own metadata carries, which is far cheaper than decoding the raw sensor data and is what every other viewer shows. Both are bounded: sharp's cache is held to a size, the external processes are counted and can be given a lower priority, and a job that takes too long says so in the log rather than disappearing into it. What they leave behind is bounded too. The cache has a file count, an age and a cleanup that walks it in batches, shared with the RAW previews so the two are held to one reading of the same settings rather than two copies that drift. A temporary file an interrupted write left behind is recognised and removed, under both the name this version writes and the ones earlier versions did — and a file this process is still writing is never one of them. A failure is remembered for a while, so a file that cannot be made into a thumbnail is not attempted again on every listing; and the cache key carries a version, so a change in how thumbnails are made invalidates what is there rather than serving it for ever. The settings screen was saving something other than what it showed: a value outside the bounds the server holds it to was sent as typed and brought within them there, and an emptied field was dropped. The bounds are now the page's as well, and it will not offer to save what the server would change. It also offered to save before the settings had loaded, and saving then wrote its own defaults over what was stored. Gates: lint and formatting on the changed files, `npm run build`, every backend module loads, and the whole backend suite — 2,026 pass, against the two that fail on `main` before any of this. Six new tests, and the thumbnail-token test from #411 now waits for the picture the way the browser does. Every claim fails when the change behind it is put back: the request held again, and thumbnails that cannot be switched off. One claim was dropped rather than left passing for the wrong reason — which of the two short-circuits answers a second ask cannot be told apart from outside, so nothing here says it can. Checked in the built image: the first ask answered 202 and said so, the picture appeared on a later ask, the next ask answered at once, a prefetch was taken, the picture opened with the proof on its URL and was refused 401 without it, and the folder drew all three.
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 22 of the plan in #373: thumbnails, and who may see one. Stacked on #410 — merge in order; this PR's own changes: cerede2000/NextExplorer@upstream-21-office-saves...upstream-22-thumbnail-access
/api/thumbnailsresolves the path and checks the access, then hands back a URL under/static/thumbnails. That directory is served byexpress.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 the picture of a file somebody may not open can be fetched by anybody who can reach the server. A 200 against a 404 answers "does this file exist" besides.A session cannot settle it: it says who is asking, not what they were cleared to see. A visitor holding a perfectly valid session for one share can name a thumbnail belonging to another share, or to a private folder, and it is served.
What it does
/api/thumbnailssigns the one filename it has just cleared; the static handler verifies that signature and nothing else — no database read, no session, nothing to confuse. A request without it is answered 401, so existence is not disclosed either.AUTH_MODE=disabledthe handler steps aside.Notes for review
How it was checked
On
mainat 7b07337 with #410 applied, Node 24 as the CI and the image use:mainuntouched (auth.test.js"current password is incorrect",browse-hidden-files.test.js);