[29/33] Uploads that survive a proxy and a dropped connection - #422
Merged
vikramsoni2 merged 7 commits intoSep 26, 2026
Merged
Conversation
added 7 commits
September 26, 2026 10:03
The engine that keeps what a save replaces, and the API that reads a history
back, arrived with the batch before this one. Nothing showed them: no mark in a
listing, no panel, no way for an administrator to see what histories cost, and
no way to read an earlier version at all.
This is that half.
A file that has earlier versions carries a small mark in the listing, with how
many; clicking it opens the panel, which lists what was kept, who saved it and
where it came from, and offers reading a version, downloading it, putting it
back, naming it, pinning it and deleting it. Putting one back keeps what it
replaced, so nothing is lost by going back. The mark is counted once per folder
rather than once per row, and a preference turns it off — which takes the query
away as well as the icon.
An administrator gets the list of every file that has a history, wherever it
is, with what each one takes up, and can empty one. Those routes are addressed
by the history's own id rather than by a path, because the ones worth finding
include files that no longer exist: a history whose file was deleted outside
the application has no path left to ask about, and no file to authorise
against. They sit behind the administrator check for the same reason.
Reading text is now one reader, used by the editor, by a version and by a file
still in the trash — the two the batch before this one left out. That reader
fixes what the editor used to do with it:
- A UTF-16 file was answered "this file appears to be binary and cannot be
opened". In UTF-16 every letter of English is accompanied by a zero byte,
and a zero byte was exactly the test for binary. That is what `Out-File`
wrote by default until PowerShell 6 and what Notepad still offers as
"Unicode", so an export or a log from a Windows machine could not be
opened. A mark is believed when there is one; otherwise the pairing of
zeros decides, which is what tools that write UTF-16 without a mark leave
behind.
- A save wrote UTF-8 over whatever the file was. That reads perfectly well
here and breaks whatever wrote it, so a save now writes back in the
encoding the file already had.
- The size limit was checked when opening and not when saving: a paste larger
than the limit was written, and the file could then never be opened again.
It is now checked on the bytes about to be written, which in UTF-16 are
twice the characters.
- The editor opens two megabytes and saves through a JSON body, whose limit
was Express's own default of 100 kB. A file between the two opened and
could never be saved, answered "request entity too large" — which names
neither limit. The two are one decision now: the body limit is derived from
what the editor may open, and a body limit somebody set is a ceiling that
is never raised from here, so it is the editor that gives way.
The editor's read is also a GET the browser may keep and revalidate, answered
304 while the file is unchanged, so opening the editor from the Markdown
preview no longer downloads the same file twice. The identity is taken from the
file's metadata, including the inode — a save writes a new file and renames it
over the old one, so a save that comes out the same size within one clock tick
still differs.
An earlier version and a file in the trash open in the editor as text to read:
no save, no shortcut that saves, and an editor that does not take typing.
Closing goes back where it came from — the folder with the history open again,
or the trash, inside the deleted folder the file was read from.
Found while wiring this up: `marksForFolder`, `listFilesWithVersions`,
`readFileVersions` and `deleteFileVersions` all called store helpers that were
never added. Nothing called them, so nothing noticed; all four threw. The
queries they need are here, and every one of them now has a test.
Left for later, and named here so it is not lost: taking a version out as a new
file somewhere, and putting one over another file, both need the destination
dialog, which is not here yet; reading a version of an office document needs
the office editors to accept one, which comes with the batch that finishes
them; and the entry an activity log would want for a purge waits for the log.
Gates: lint and formatting on the changed files, `npm run build`, every backend
module loads, and the whole backend suite — 1,949 pass, against the two that
fail on `main` before any of this. Every claim above has a test that fails when
the change behind it is put back; the detection of UTF-16 was mutated twice,
once for the mark and once for the pairing, because they are two mechanisms.
The screens were checked in the built image: two saves, the mark showing 2 in
the listing, the panel opening from it, a version read in the read-only viewer,
a version restored from the panel with the history growing to three, a file
deleted and read from the trash by double-clicking it, and the administrator's
list showing the file, its space and its history. That is also how the last
defect here turned up: the dialog that confirms a restore teleports to the body
and stacked below the panel's own overlay, so its buttons could not be clicked.
No unit test would have seen it.
Which keys are preferences was decided twice: once in the settings service, which sanitises the value, and once in the settings route, which decides whether the key is written at all. They had drifted. The preference added here — the mark on files that have versions — reached the service's list and not the route's, so the toggle moved on screen, the save answered success, and nothing was stored. It would have read as "my setting does not stick", with two lists to find before anyone could say why. One list now, named and exported by the service; the route asks it. A test walks every key the service calls a preference and checks the route keeps it, so the two cannot drift again.
Two people editing the same document only see each other when the Document
Server was given the same key: a different key is a different document to it,
and it opens a second, independent session on the same file. Whoever saves last
then overwrites the other, with nothing to warn either of them.
The key was recomputed on every open from the file's own modification time. So
it changed under the people already editing — every save of theirs split the
session — and it did not change once they had all left, which is the one moment
it must, because the Document Server caches the prepared document under that key
and serves the stale copy on the next open.
The key is now kept: the same for as long as anyone has the document open,
including across their saves, and dropped when the Document Server reports it
has let go. Restoring a version drops it too, which is the piece the history
batch could not carry: an editor reopened after a restore was getting the copy
the restore had just replaced.
The other half is where the document is. The Document Server is handed a token
when the editor opens and returns it unchanged with every save, so the token
says where the document *was*. Renaming from the title bar makes that stale at
once. There are now editing sessions, stored rather than held in memory — a
restart mid-edit would otherwise forget the rename, and the next save would
recreate the old name beside the new one, silently. Renaming from inside the
editor moves the file through the ordinary rename, with the ordinary permission
checks, and then moves the session, the presence record and the key with it.
Presence starts when the client says the document is really open, not when the
configuration was asked for: asking for a configuration says nothing about
whether the document opened, and a file the editor then refused was shown to
everybody as being edited until it expired.
Four things were wrong in this path and are fixed here:
- **A read-only folder handed out an editing session.** The decision looked at
the requested mode and at whether the document came through a read-only
share, and never at the location's own rights. The backend token now carries
that decision, and the callback refuses a save made with a token that says
read-only.
- **The backend token was untyped and never expired.** It is signed with the
same secret as the Document Server's own tokens, so one of those would have
been accepted as ours. It now carries a type claim and a twelve-hour life.
- **Every unknown extension was announced as a text document.** The Document
Server validates documentType against the extension and refuses the config
outright, answering "the file content does not match the file extension" —
true, unhelpful, and several steps from the setting that caused it. .odg was
one of them: the Document Server counts a drawing as a presentation. The
mapping is now the one it ships, and an extension it has no editor for is
refused here, naming the setting.
- **Nothing ever expired.** Only a terminal callback released a key, so a
browser closed on the editor left the row for good — one for every document
ever opened. There is now an hourly sweep, and it takes with it the two
cleanups that were already in the tree with no caller at all:
`cleanupExpiredShares` and `cleanupExpiredSessions`, so an expired share no
longer sits on disk indefinitely.
The rename itself moved out of its route into a service, because two callers now
need exactly those checks in exactly that order. One thing changed on the way:
`ensureValidName` throws a plain Error, which the error handler could only read
as a server fault, so a name with a separator in it was answered 500 and
appeared in the logs as an unhandled failure. It is a bad request.
Gates: lint and formatting on the changed files, `npm run build`, every backend
module loads, and the whole backend suite — 1,972 pass, against the two that
fail on `main` before any of this. Twenty-three new tests; every claim above
fails when the change behind it is put back, mutated one at a time: the key back
to its inline computation, the type claim removed, the write check removed from
both ends, the save target back to the token, and the editor table back to its
catch-all.
Checked in the built image with a volume: the schema migrates, a document is
handed a key and a session, the heartbeat reports it open, opening it again
while it is open gives the same key, an unknown extension is refused by name,
renaming from the session moves the file and the folder lists the new name, and
ending the session is accepted.
Second half — editing together as the file list shows it, Save as, mentions,
force-save, the history inside the editor, and opening a document in a tab of
its own — follows under the same number.
A folder gave no sign that anybody had a document open. It was copied, moved, renamed or deleted while an editor was about to write a newer version of it, and nobody was told. A file somebody has open now carries a mark in the listing, with their names, and the folder keeps it current without asking for it: the server holds the request open until somebody joins or leaves, so presence costs nothing while nothing happens. Dragging such a document somewhere else asks first — it is allowed, and it is worth knowing that the editor will write where the file used to be. Presence is advisory and never a lock. Nothing here can stop a file operation, and nothing here survives a restart. The second half is writing. Closing the editor relied on the callback the Document Server sends when it decides the document is finished with, which arrives seconds after the last keystroke — long after the folder behind the editor has been listed again with the old content, and often after the tab is gone. The server now asks the Document Server to write what the editor holds, on the way out and on a bounded timer while editing, and the save it asked for on the way out is kept as a state of its own rather than as one of the automatic ones in between. Two requests for one session are coalesced: a close arriving while an automatic save is still assembling queues one final command behind it rather than a second one beside it, because each one costs a full document conversion. A Document Server that answers "the editor has not sent its last changes yet" is retried, four times, with increasing delays — so closing the preview stays instant and the last edits still land. Found while checking this in a browser: a listing refresh could not remove anything. The store merges a new listing into the items already on screen with `Object.assign`, which overwrites fields and never deletes them, so a mark the row had stopped carrying stayed for as long as the folder was open — a document nobody had open any more, and a history that had just been emptied. Only `supportsThumbnail` had been patched for this, one field at a time. A refresh now drops whatever the listing no longer sends, keeping only the fields the store puts there itself. Gates: lint and formatting on the changed files, `npm run build`, every backend module loads, and the whole backend suite — 1,986 pass, against the two that fail on `main` before any of this. Fourteen new tests; every claim above fails when the change behind it is put back, mutated one at a time: the listing no longer carrying presence, closing no longer flushing, the two requests no longer coalesced, and the close flush no longer counting as a deliberate save. Checked in the built image, which is also where the merge defect turned up: the mark appears when a document is reported open, without reloading the page, names the person, and goes when the session ends — and the request log shows the folder listing itself again because the held request answered, not because anything polled.
ONLYOFFICE hides the menu entries it has no answer for. Three of them were hidden because nothing here answered. **Saving a copy.** The editor does not write anything itself: it converts the document and hands the integration a URL to fetch the result from. With nothing to receive it, Download was the only way out of a document — through the browser, into the person's downloads rather than their volume. Now the server fetches it and writes the copy beside the original, never over anything: a name already taken gets the same "(1)" treatment as everywhere else, and the answer says the name actually used. A title carrying a path separator is refused rather than trimmed to its last segment — reinterpreting it would turn "../invoice.pdf" into a silent success in a folder nobody named. Saving a copy is not a change to the original, so a reader may do it; what it needs is the right to read the document and to write into the folder, exactly as an upload would. **Mentioning somebody in a comment.** The editor asks for the whole list and filters it itself as the name is typed, and it waits for an answer even when the list is empty. Only a signed-in account gets it: a visitor through a share link has no business being handed the user directory. The comment itself is already written by the editor; the separate "tell them" step has no channel to deliver on, so the mention is recorded and the answer says plainly that nothing was sent, rather than leaving the editor waiting on a handler that does nothing. **The document's own history.** The editor numbers versions from the oldest and expects the current state to be the last of them; the history this application keeps comes newest first and does not count the current state as a version. The two are reconciled here rather than in the editor. Restore is offered only to somebody who may change the document. That same piece closes what the history panel could not do: an earlier version of an office document now opens in the office editor, read only — which is what the versions panel was missing, since it could only put text in front of anybody on its own. A version gets a key of its own, so it never touches the one the people editing the document share, and it is served with no callback at all: nothing is saved from a version, and the document's own callback would release that shared key. Left out on purpose: handing the editor a file from the volume to insert or compare against. It is a short-lived read-only token over the same route, and it is useless without somewhere to choose the file — so it travels with the batch that brings the destination dialog. Gates: lint and formatting on the changed files, `npm run build`, every backend module loads, and the whole backend suite — 2,001 pass, against the two that fail on `main` before any of this. Fifteen new tests; every claim above fails when the change behind it is put back, mutated one at a time: the document URL no longer checked against the Document Server, the title trimmed instead of refused, the copy written straight over the name, the mention list handed to anybody, and a version config given a callback. Checked in the built image: a document with two versions answers a history of three entries ending in itself, one entry hands back a signed location whose token says it may never be written, and opening that version answers a viewer with its own key and no editing session.
A document had no address. Opening one filled a panel over the folder it was in, driven by a store, and the browser knew nothing about it: nothing could be linked to, nothing kept as a bookmark, the back button did not close it, and two documents could not be open at once. `/open/<path>` is that address. Everything the page does the folder listing also does — the same preview, the same plugins, the same component. What it adds is the one thing a panel over a folder cannot have: a URL. The browser then provides the tabs. Two things about leaving had to be right. Closing the document lands back in the folder it is in, with the document selected, which is where closing the panel leaves you — except in a tab opened for this document alone, where the thing the close button should close is the tab. Closing the tab has to tell the server what closing the panel tells it: a tab gives one synchronous moment on its way out, so the plugin is told it is unloading and sends what it has to send in one beacon. Without it, a document closed by closing its tab would go on being reported as open by somebody who had left. Whether a document opens in a tab of its own is an account's preference, off by default. One decision for every kind of file rather than one per plugin: a spreadsheet and a photograph open the same way, because a preference that holds for some files and not others is a preference nobody can predict. And the tab has a name. It was the last segment of the address whatever the page was, so every settings section, the search and both share lists read "Volumes"; a document opened directly read "Explorer". Now each page says what it is and which instance it belongs to — the name set in Branding, read from the settings every page already loads, so renaming the instance reaches every open tab without a reload. Four tabs all reading "Explorer" are four tabs nobody can tell apart, which is exactly what a page whose point is to be opened several times at once cannot afford. Gates: lint and formatting on the changed files, `npm run build`, every backend module loads, and the whole backend suite — 2,007 pass, against the two that fail on `main` before any of this. Checked in the built image: the folder tab reads "Projects | Explorer", the settings tab "Preferences | Explorer", the trash tab "Trash | Explorer" and the document tab "notes.md | Explorer"; `/open/Work/Projects/notes.md` shows the document; a file nothing opens says so and offers the way back; and with the preference on, a double click in the listing opens a second tab at that address.
An upload was one request. A reverse proxy refuses it outright once the body
passes whatever limit it enforces — the error names a size and never the
setting — and a connection that drops loses the whole thing, however far it had
got. Both are what somebody sending a film or a disk image over a home
connection meets first, and there was nothing to be done about either.
This adds the tus protocol beside the direct upload. The transfer becomes a
series of requests, each small enough to pass; what has arrived is remembered,
so a client that comes back asks where it got to and carries on from there.
Nothing lands in the folder until the file is whole, and when it does it takes
a free name rather than replacing anything, as every other way a file arrives
here does.
Which one is used is an administrator's decision, in a screen of its own, and
the default is unchanged: nothing about an upload moves until somebody asks.
The chunk size is theirs too, bounded by a ceiling the server states rather
than discovered by having a number refused — a chunk is held whole in memory at
each end, so an unbounded one is a way to run a server out of it.
The parts live in the cache rather than beside the destination: a part file is
not a file anybody asked for, and a volume should never show one. They are
swept once they are past their time, which covers the two ways they are left —
a client that never came back, and a data file whose record never reached the
disk.
Three things in the direct upload were wrong and are fixed here, since this is
the batch that owns it:
- **A request that announced no size skipped the space check.** The guard
takes a number and was handed nothing, so a chunked request — which is what
an API client streaming a body sends — could fill a volume already past its
reserve, on a machine where a full volume takes the database down with it.
Zero is what is honestly known about what is coming, and the reserve is
still held free.
- **One request can reach several folders, and only the first was checked.**
Each file carries its own relative path, so on a machine with more than one
disk that is several disks; the second had its free space never measured
and its dead uploads never swept.
- **A refusal left the folders it had invented behind.** No space, a file
over the limit, a client that went away: empty folders an upload created,
in somebody's tree, with nothing to say where they came from. What lies
between the topmost folder `mkdir` had to create and the destination is
exactly what that file added, and it is removed deepest first, each one
only while it is still empty — another file of the same request may have
landed in it.
And one about permissions: the destination is authorized, but the file also
carries a client-supplied relative path, so the folder it actually lands in is
authorized too. Without that, a subfolder an administrator had made read-only
still accepted uploads.
Left for later, and named here: the client learning a chunk size for itself
after a direct upload stalls, which needs the watchdog that notices the stall.
The setting for it is not here either — a setting whose half is missing is
worse than none.
Two dependencies on the server (`@tus/server`, `@tus/file-store`) and one in
the browser (`@uppy/tus`). The store and the protocol server are built on first
use rather than when the module is required: `FileStore` creates its directory
in its constructor, which would turn requiring the file into a filesystem write
— one that fails wherever the cache is not there yet, including the check that
every module loads.
Gates: lint and formatting on the changed files, `npm run build`, every backend
module loads, and the whole backend suite — 2,020 pass, against the two that
fail on `main` before any of this. Twelve new tests; every claim above fails
when the change behind it is put back: the finished file written straight over
the name, a metadata field of the literal "undefined" believed, the root
allowed, and the sweep stopped.
Checked in the built image: an upload of 300 kB was created, a third of it
sent, then stopped; the server answered that it held 100,000 bytes; the folder
showed nothing; the rest was sent from that offset and the file landed whole,
at its full size. The settings screen offers the switch and the chunk size.
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.
Stacked on #421.
An upload was one request
A reverse proxy refuses it outright once the body passes whatever limit it enforces — the error names a size and never the setting — and a connection that drops loses the whole thing, however far it had got. Both are what somebody sending a film or a disk image over a home connection meets first, and there was nothing to be done about either.
This adds the tus protocol beside the direct upload. The transfer becomes a series of requests, each small enough to pass; what has arrived is remembered, so a client that comes back asks where it got to and carries on from there. Nothing lands in the folder until the file is whole, and when it does it takes a free name rather than replacing anything, as every other way a file arrives here does.
Which one is used is an administrator's decision, in a screen of its own, and the default is unchanged: nothing about an upload moves until somebody asks. The chunk size is theirs too, bounded by a ceiling the server states rather than discovered by having a number refused — a chunk is held whole in memory at each end.
The parts live in the cache rather than beside the destination: a part file is not a file anybody asked for, and a volume should never show one. They are swept once past their time, covering both ways they are left — a client that never came back, and a data file whose record never reached the disk.
Defects in the direct upload, fixed here
mkdirhad to create and the destination is exactly what that file added; it is removed deepest first, each only while still empty.Left for later, named
The client learning a chunk size for itself after a direct upload stalls, which needs the watchdog that notices the stall. The setting for it is not here either — a setting whose half is missing is worse than none.
Dependencies
Two on the server (
@tus/server,@tus/file-store) and one in the browser (@uppy/tus). The store and the protocol server are built on first use, not when the module is required:FileStorecreates its directory in its constructor, which would turn requiring the file into a filesystem write — one that fails wherever the cache is not there yet, including the check that every module loads.Gates
npm run build; every backend module loads.mainbefore any of this.