[23/33] Copying a folder: rsync where it is there, and links that survive - #412
Merged
Merged
Conversation
added 5 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.
This was referenced Sep 25, 2026
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 23 of the plan in #373: copying a folder. Stacked on #411 — merge in order; this PR's own changes: cerede2000/NextExplorer@upstream-22-thumbnail-access...upstream-23-native-copy
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 — listings, searches, other people's uploads — waits behind them.
rsyncdoes the same work in one process, off that thread entirely.What it does
FILE_TRANSFER_ENGINE=nativeor=stream; the default is unchanged — native where the image runs, JavaScript elsewhere. This is mostly about testing: frozen fromprocess.platformat load time, 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 by a test.-rlt, not-a. Recursion, links and times are what the JavaScript path gives. 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 — so owner, group and mode are left to the destination.fs.cpresolved a relative symbolic link inside the tree and wrote it out as an absolute path into the source tree, so a copied folder pointed back at the original and broke entirely once that was moved or deleted. It is kept verbatim now — as rsync keeps it, and as the trash's own copy already did.Notes for review
copyEntrychanged, which is the one primitivecopyIntoPlaceuses, so the staging-then-place behaviour from [18/33] Never let a file operation replace or merge into what holds a name #404 is untouched: a copy still lands under a hidden name and takes its real one only once whole.copyEntryWithProgressalready covers the one caller (the trash) that has it today.copyEntryis exported so a test can name the engine — the reason is written beside the export.How it was checked
On
mainat 7b07337 with #410 and #411 applied, Node 24 as the CI and the image use:mainuntouched;