[25/40] Bring the tests for everything the last twenty batches sent - #466
Merged
Merged
Conversation
cerede2000
pushed a commit
to cerede2000/NextExplorer
that referenced
this pull request
Sep 27, 2026
The tests for everything the batches sent, and nine of ours left behind because they assert what upstream deliberately does differently.
added 25 commits
September 27, 2026 19:24
Two routes built a command line with values from the request in it and gave the line to a shell. `routes/usage.js` pasted a folder's path into `du -sb "…"` and `df -Pk "…"`; `routes/permissions.js` pasted an owner, a group, a mode and a path into `chown`, `chgrp`, `chmod -R` and two `id` lookups. A name is not a shell string. Anything that closes the quoting leaves the rest for `/bin/sh` to run, as the user the server runs as. The usage one needs no privilege at all: any account that can create a folder can name one, and where AUTH_ENABLED is false that is anybody who can reach the server. `execFile` takes the arguments as a list, so there is no line for a shell to read and no shell. The commands, their output and the answers are unchanged — this is deliberately the smallest change that closes it, and not the rewrite that stands in nxzai#450 and nxzai#453. An argument list is not a free pass on its own: `chown` reads a leading dash as an option, so `--reference=/etc/shadow` would have copied another file's ownership onto the target. An account or group name has to look like one. `tests/routes/no-shell.test.js` — three cases. Two of them name a folder, and an owner, with a payload that creates a file, and check the file is not there; the third asks for `--reference=/etc/shadow` and expects a refusal. All three fail against the routes as they are, the first two by running the command. The payload only ever touches the working directory, and it is removed whatever an assertion does, so a run that does execute leaves nothing behind.
Two things about OIDC that this fixes.
**Which of two failures it was.** A sign-in that cannot start has one of two causes:
nothing was configured, or what was configured could not be made to work. The first
is answered by filling in OIDC_ISSUER and the rest; the second is answered by looking
at the provider. Both answered 404 "OIDC is not configured", so an administrator whose
provider was unreachable was sent to change a configuration that was already right.
`configureOidc` now records what it concluded and why, and the routes read it: 404
AUTH_OIDC_NOT_CONFIGURED for the first, 503 AUTH_OIDC_PROVIDER_UNAVAILABLE for the
second, and neither carries the network's own words — no ENOTFOUND, no internal host
name, in the body or in the address bar.
A browser asking for one of those addresses is looking at a page, not reading JSON, so
it is sent back to the sign-in screen with the code beside the sentence. The screen can
then say what the code means in the reader's own language.
**Where the callback comes back to.** The return address was built from a fixed
`baseURL`, so a deployment reached through a reverse proxy on a different name sent
people back to the wrong origin. It is now resolved from the request — but only from a
forwarded host behind a trusted proxy, and only when the result exactly matches an
origin the operator configured. A redirect target that a request header could choose
is an open redirect with extra steps.
The path is still held to a relative, same-site one, as before.
Also here, because it is the same files:
- `claimsFromIdToken` in the middleware and `uniqueOrigins` in the routes were local
copies of things `utils/idToken.js` and the new `utils/oidcRedirect.js` do; both go.
- The guest-session cookie was cleared on `/api` only, at five sign-in and sign-out
paths. A guest session left on `/` outlived the sign-in that should have ended it.
- `ServiceUnavailableError` (503) and the two AUTH_OIDC_* codes, which nothing had yet.
The account lockout that `routes/auth.js` also differs in is deliberately not here: it
is a different subject and goes with releasing a locked account.
## Checks
Seven test files, 107 tests, including three this fork had and `main` did not:
`oidc-middleware`, `oidcOrigin` and `auth-oidc-routes`. Neutralising
`getOidcAvailability` so it reports one verdict for both causes turns three of them
red — the three that tell the two apart.
Whole backend suite: 2 391 passed, 2 failed, the two that fail on `main` on its own
(`auth.test.js` on the current password, `browse-hidden-files.test.js`). `npm run lint`
reports 146 against `main`'s 143, the three being the parse error on `backend/tests/**`
that 141 of `main`'s own test files already draw. Formatting clean. Frontend builds,
backend loads, documentation site builds.
One test assertion was rewritten rather than ported as it stood: it matched the wording
`express-openid-connect` uses for a callback with no sign-in in progress, and that
wording differs between 2.19 and 2.20. It now asserts the refusal.
One box, one name for it
------------------------
The sign-in box takes an email address or a username, so it is neither: it is
whatever was typed, and this calls it `identifier` from the screen to the route.
The screen and the client were renamed and the store and the route were not, so
the store passed `email` to a client expecting `identifier`. `JSON.stringify`
drops a key whose value is undefined, and the request went out carrying a
password and nobody to sign in. The answer was "invalid credentials", which is
what a wrong password looks like — so nothing about it read as a defect.
`attemptLocalLogin` already took `identifier`; the route is what had not caught
up. `email` and `username` still work, for a script or an older client that
sends them.
The identifier was the half that failed loudly. The screen also reads
`totpPending`, `oidcStatus`, `cancelTotp`, `ensureStatus` and `forgetSession`
off the store, and none of them were there: `totpPending` read undefined, so the
box for the code from the authenticator never appeared, and a correct password
on an account with a second factor landed on a screen that looked like it had
done nothing. Undefined is not an error in a template — it is a `v-if` that is
false. The store is here in full, with the code step read back from the server
on every start so a reload in the middle of one lands back on the code.
tests/routes/sign-in-identifier.test.js signs in with each of the three names,
refuses a wrong password, and reads the three frontend files to check they all
use the one name — the chain is four files long and three of them have no runner
here, which is how it broke silently in the middle.
And what a refusal says
-----------------------
A code that names the kind of refusal — FORBIDDEN, NOT_FOUND, CONFLICT,
RATE_LIMIT_EXCEEDED — is translated for the reader, and the server's own
sentence, which says *which* refusal, went underneath rather than being lost.
A lock that arrives with a duration gets a sentence of its own rather than a
placeholder in the plain one, so it can never read "{minutes}". And the
handler no longer asks vue-i18n for a key before checking it has it, which was
a console warning for every refusal the catalogue has no entry for, twice.
Two reports the server makes about itself at start, neither of which existed. **What releases up to 1.1.7 left in the cache.** The database and app-config.json lived in CACHE_DIR until 1.1.8, which moved them to CONFIG_DIR and left links behind; 2.0.3 removed that move from the entrypoint. So an installation that started on 1.1.7 or earlier and skipped the releases in between comes up on a new, empty app.db in CONFIG_DIR with its accounts, shares and settings sitting unread in the cache. Nothing said so: the server started, the sign-in page offered to create the first administrator, and the answer looked like a fresh installation rather than a lost one. It is a notice and nothing more — nothing is moved, nothing is deleted. It names what it found and where, and says what to do with it. **What the process is costing.** `services/performanceDiagnostics.js` samples CPU, resident memory as the cgroup sees it rather than as the host does, event-loop delay at p99, and the queues that can grow: thumbnails, folder sizes, transfers. Off unless PERFORMANCE_DIAGNOSTICS_ENABLED is set, and then it reports only the intervals that pass a threshold — a diagnostic that logs every interval by default is a diagnostic that fills a disk. PERFORMANCE_DIAGNOSTICS_LOG_EVERY_INTERVAL asks for all of them. An interval below its floor is held to the default: a sampler on a 1 ms interval costs more than whatever it was meant to diagnose, and 1 ms is what an emptied field sends. Each queue reports itself through an optional call. One that has no report is a queue this installation has nothing to say about, not a reason for the whole record to fail — a diagnostic that throws says nothing at the moment it is most wanted. ## Checks `legacy-cache-check.test.js`, 3 tests: a cache holding the old names is named, one holding the links 1.1.8 left is named differently, and an ordinary cache says nothing. Making the inspection always see an empty cache turns all three red. `performance-diagnostics.test.js`, 7 tests: silent unless asked for, says what it will watch and by which thresholds, holds an interval below its floor to the default, reports nothing of an ordinary interval, reports one that passes a threshold and says which kind it was, reports every interval when told to, and samples the machine rather than guessing. Making it start whether or not it was asked for turns the first red. Whole backend suite: 2 556 passed, 2 failed — the two that fail on `main` on its own.
`/api/features` already offers `preview.maxRenderBytes` to the screen, and the configuration had no such value, so it answered `null` and the preview rendered whatever it was given. A document large enough to render slowly renders slowly for everybody on the machine, and nobody could raise or lower the point at which it stops trying. PREVIEW_MAX_RENDER_SIZE sets it; 16 MB when it is not set, and a value below zero or unparseable is that default rather than a ceiling of nothing. And the listing: `GET /api/browse` answers with what is true at that moment — which documents somebody has open in an editor, what a folder weighs, whether a write would be refused. A GET with no cache header is cacheable by default, so a proxy or a browser was free to keep it and serve a folder as it was: a deleted file still listed, a document shown as open by somebody who closed it an hour ago. `private, no-store` says what it is. ## Checks `browse-caching.test.js` asserts both halves of the header, and taking the header away turns it red. The ceiling is read through `/api/features`, which has offered the field since the batch that brought the features route and was answering `null` for it. Whole backend suite: 2 556 passed, 2 failed — the two that fail on `main` on its own.
`getDb` checks whether a connection is already open and opens one when it is not. Every
caller that arrives before the first has finished sees "not open" and opens another —
and everything that starts with the server asks at once: the session store, the settings,
the trash sweep, the search index, the favourites. So a start opened app.db four times,
ran `migrate` over the same file four times in parallel, and whichever finished last
became the one everybody used.
One opening is shared now. `openDb` does the work, `getDb` hands every caller the same
promise while it is in flight and the same connection afterwards, and `closeDb` lets a
test take it away again — which is what a suite opening a database per case needs.
And `prepared(db, sql)`: `db.prepare` compiles the SQL every time it is called, and the
hot paths called it per row — a listing asking whether each of a thousand entries is a
favourite compiled the same statement a thousand times. Kept in a WeakMap keyed on the
connection, so the cache goes when the connection does and a statement is never handed to
a connection that did not compile it.
## Checks
`db-single-open.test.js`, 5 tests. Five callers in one turn get one connection — counted
on the connection objects rather than on anything the code says about itself. A later
caller gets the one already open. After `closeDb` the next caller gets a new, working one.
The same SQL twice compiles once, and the same SQL against a new connection compiles again.
Putting the per-caller opening back turns the first red.
Whole backend suite: 2 561 passed, 2 failed — the two that fail on `main` on its own.
Two files were in this batch and are not: `betterSqliteSessionStore.js` and
`bootstrap.js`. `main`'s versions are the newer ones — it opens the session database on
first use rather than when the module is required, which is what keeps
`require('./backend/src/app.js')` working where the cache directory does not exist yet.
Bringing the fork's versions over them would have broken that check.
Each account's per-folder choices were two JSON values under `user_settings`:
one map of sorts, one of views, read and rewritten whole on every change. Three
things followed, and all three are gone here.
Two tabs on different folders overwrote each other. Each save sent the whole
map, so whichever tab saved last won and the other folder's choice was lost.
A preference is now written one folder at a time, through
`PATCH /api/settings` with `{ folderSort: { path, sort } }`, and the row's
UPSERT keeps the half it was not given: setting a folder's view no longer
erases its sort.
The map had a ceiling of a hundred folders, because it shipped entire on every
load and was rewritten entire on every change; the hundred-and-first folder
silently forgot the oldest. Rows have no ceiling.
And nothing could ever clean it up. A folder deleted or renamed left its
preferences behind on every account that had ever opened it, on a path that no
longer existed. As rows they can be removed with the folder they describe.
Schema 20 creates `folder_preferences` and carries the old values over: one row
per account and folder, merged under the later of the two times, and the values
it read are removed so nothing can read them back. A value it cannot parse
costs that folder its remembered sort rather than failing the startup.
Also here, because they are the same screen and the same service:
- A default view for folders that have none of their own. `null` means the
built-in one; a mode there is no such thing as is refused rather than read as
null, which used to put every folder back to the built-in view.
- Markdown opens in the editor, for whoever mostly writes it, instead of going
through the preview and clicking Edit every time. Only markdown: it is the one
kind of file that has both, so it is the only one where opening it is a choice.
- An access rule that cannot be stored is refused with its reason instead of
being dropped from the answer. Saving `../Secret` used to answer 200 with a
list the page then adopted, and an administrator was left believing a folder
was hidden that never was. Worse, a permission that was not one of the three
became `rw`, so a mistyped `readonly` opened a folder for writing.
- Every section of a save is checked before any of it is written. A payload
carrying a valid section and a refused one used to store the first and answer
400 — a request reported as refused that had changed something.
- One list of what a preference is. It was three: the keys the route allowed,
the chain of if/else that sanitised them, and the defaults in the client store.
A key in one and missing from another was accepted, silently dropped, and
answered with its previous value, which the client applied — so the switch
flicked itself back off. `markdownOpensInEditor` did exactly that.
`PATCH /api/settings` now answers with the settings read back from storage
rather than an echo of what was sent, so what the caller applies to its own
state is what a later request would read. `tests/routes/settings-preferences.js`
was asserting the echo, and now asserts the value of each preference it saved.
`USER_SETTING_KEYS` is `WRITABLE_USER_SETTINGS`, which says what it holds.
tests/services/folder-preferences-as-rows.test.js covers the eight claims
above. Each was put back to the old behaviour to check the right one goes red:
overwriting instead of keeping the other half, deleting the account's rows on
every write, reading without the owner, a LIMIT of 100, accepting any word as a
view mode, and dropping the carry-over.
Whole suite: 2685 passed, 2 failed — the two that already fail on main
(auth.test.js on the wrong current password, browse-hidden-files.test.js on a
FOREIGN KEY). Built, started, and driven in a browser: a folder keeps the view
it was left in while its neighbour keeps the default, and markdown goes to the
editor only when the preference says so.
Moving or copying meant typing a path, or navigating to the destination first and coming back. This is the dialog the rest of the application will use for that — mounted once with the layout, opened from anywhere, and answering with a folder or with nothing. It opens on a list rather than at the root, because the folder somebody wants is nearly always one they have used before. Nobody curates that list: it is written by the transfers themselves, so it stays true to how the person really files things, and every route into a folder counts — the picker, a drag onto a favorite, a paste. Kept per account: a shared favourite is a deliberate bookmark, this is a trace of one person's habits, and showing someone else's would be both wrong and a small leak of where they work. The list is filtered against what the account can reach right now. A folder can be deleted, or have its access revoked, long after it was last used, and offering it as a destination would only produce a failure at the end of the flow. Anything gone is forgotten on the way out, so the list heals itself. Schema 21 adds `recent_destinations`, one row per account and path, trimmed to ten on write so it cannot grow for somebody who never opens the picker. A destination that fails to be remembered never fails the transfer that reached it: remembering is a convenience, and a convenience must not break the thing it is convenient for. `ModalDialog` gains a footer. Without it the dialog rendered — title, recents, the folder tree, all correct — and had no confirm button anywhere in the page: what it asked for sat inside the scrolling part, and there was no way to answer except Escape. Driving the dialog in a browser is the only thing that would have caught it, and it is why the footer is here rather than in the batch that rewrites this component. The archive preview is the first screen to use it: "Extract to…" beside "Extract here", so an archive can be unpacked somewhere other than the folder it sits in. Closing the dialog without choosing means do nothing — not the root, and not the default. tests/routes/recent-destinations.test.js covers the six claims: that a transfer is remembered without being asked to, that the most recent comes first, that using a destination again moves it up rather than listing it twice, that one account's habits stay out of another's list, that a destination since deleted is dropped, and that a write it cannot perform is swallowed rather than failing the transfer. Each was put back to the old behaviour to check the right one goes red: no recording at all, no filtering, the error rethrown, and the read without its owner. The route is described in the OpenAPI document and walked by the tour, because upstream holds every mounted route to both. Whole suite: 2691 passed, 2 failed — the two that already fail on main. Built, started, and driven in a browser end to end: two moves, the dialog opened from an archive, the folder chosen from the recents, and the entries landing there rather than beside the archive.
…tal B `archiveCacheService` reads two bounds and nothing defined either of them. In JavaScript that is not an error: it is `undefined`, and every comparison against `undefined` is false. So both guards were the wrong way round, in two different directions at once. `innerSize > archives.browseMaxBytes` was never true, so no archive was ever too large to look inside. A compound archive — a .tar.gz and its family — has to be decompressed before anything inside it can be named, and that bound is the only thing standing between "show me what is in this" and unpacking a backup of any size into the cache. It refused nothing. `total <= archives.cacheMaxBytes` was never true either, so the sweep never returned early and its loop never broke. It removed every cached copy it found, on every pass, and each archive was decompressed again from scratch the next time somebody opened it — the cache did the opposite of its job. Both are now what the code has always assumed: two gigabytes to look inside, eight for the copies altogether, either of them settable with MAX_BROWSABLE_ARCHIVE_SIZE and ARCHIVE_CACHE_MAX_SIZE. Which is where the second defect came out. `parseByteSize` rejected `5MB` for the capital B — `5m` and `5mb` were read, `5MB` and `2GB` were not — and a rejected size is null, which every setting reads as "not set" and answers with its default. Ten settings took their default from a value that had been given: the upload chunk size, the search and editor ceilings, the JSON body limit, the direct-upload limit, the storage reserve, both archive bounds, the trash quota and the preview ceiling. The README writes `5MB`. Also here, because they are the same operations: extracting from an archive and compressing into one now tell the folder-size index what they wrote. The comment above the extraction already said it queued the refresh; nothing did. And `archiveTree` reads the access section as a section — an `access` that is not an object used to read as "no rules at all", which for a zip somebody downloads is every hidden folder inside it. tests/config/archive-bounds.test.js covers both: that each bound is a finite number, that the comparisons the service writes mean something against it, that an administrator's value is taken and a value that is not a size falls back, and every written form of a size. Putting the bounds back to undefined turns five red; putting the capital B back to a refusal turns four. Whole suite: 2707 passed, 2 failed — the two that already fail on main.
How full a volume is was two commands with the path pasted into them:
du -sb "<path>"
df -Pk "<path>"
A folder name is not a shell string. A folder called
x";<anything>;echo "
closes the quote and leaves whatever follows for the shell to run, as the user
the server runs as, the moment somebody opens that folder. Any account that can
create a folder can do it; with AUTH_ENABLED=false that is anybody who can reach
the server. Reproduced against main: a file that did not exist before the
request existed after it.
`fs.statfs` answers the same question with no shell and no subprocess. It is
also the reason the answer is instant: `du -sb` walked the whole tree to report
a number the filesystem already had, on every request, for every volume — which
on a volume of any size is a full disk read to draw a bar.
The answer gains `used` and `percentUsed`, computed where the numbers are rather
than in each caller, and a path that cannot be read answers zeroes as before
rather than failing.
The bar itself comes with it: a volume in the sidebar shows how full it is when
SHOW_VOLUME_USAGE is on, which is the setting `/api/features` has always
reported and nothing has ever drawn.
tests/routes/usage-no-shell.test.js makes a folder whose name says to create a
file, asks for its usage, and checks the file is not there. Against the previous
route all three of its tests fail, the first of them by running the command.
Whole suite: 2722 passed, 2 failed — the two that already fail on main.
A share was read-only or read-write and nothing else. This gives it the five permissions the dialog has always had room for — delete, create a folder, create a file, upload, download — and makes the interface that shows them. Downloads are the one worth naming on its own. It is deliberately not tied to read-write like the other four: a read-only share is exactly where withholding downloads means something, "read this" rather than "take a copy of this". Every share that already exists keeps every permission it had; a migration decides nothing for an owner. `accessManager` is where the permissions are read, so it is where they had to be answered. A share's listing now says per file what may be done with it, rather than the route refusing at the end of a click that looked available. Counting changed with it. Every fetch of `/file/...` used to be a download, so reading a text file in the browser — which never leaves the page — was written down as a copy taken away, and an owner reading "downloaded 40 times" was reading how many times somebody had looked at it. It is counted the way the file is delivered now: inline is an opening, an attachment is a download, the same split POST /api/download already used. `tests/routes/share-counters.js` asserted the old rule and now asserts both halves of the new one. The list of shares gains what an owner actually needs: a search box, a filter, a sort, an empty state that says which of the two it is, and the activity behind each share — when it was last opened and from where, when it was last downloaded and from where. A share can be edited without being deleted and made again, which is what changing an expiry used to mean. Schema 22 adds the five permission columns, defaulting to allowed, and the two columns for the last download. `versions_visible` and `versions_download` are switched on for shares with named accounts, because those accounts could already see what they hold. One upstream test changed for a reason rather than a rename: `public-endpoints-hardening` looked at the first `guestSession=` header, and there are two now. The cookie moved from `/api` to the root — an <img> asking /static for a thumbnail cannot carry a header, and a cookie scoped to /api never reaches it — so the old one is cleared in the same answer. Whole suite: 2795 passed, 2 failed — the two that already fail on main. Putting `canDownload` back to always-true turns five red; counting every fetch as a download turns one.
The index walked past four folder names: `.git`, `node_modules`, `dist`, `build`. That is an editor's habit, and this is a file server, where those are ordinary folder names somebody may have a year of work in. A file under one of them could not be found, by name or by content, with nothing in the answer to say why (#11). The same four names were written down three times — as ripgrep globs, in the walker, and here; the first two were corrected in nxzai#419 and the index is the one that was left. Only dot-folders are walked past now, for the two reasons that are this application's own: the trash and the file versions live in one, and the rest are hidden things the search does not show unless the reader asked. What is not worth finding is the administrator's exclusion list to say, which is visible in Settings. A folder gets a row of its own, so it can be found by its name without anybody knowing what is inside it. `search-index.test.js` counted two rows leaving when a folder of two files was removed; it is three now, and says why. A result says how it was found: by its name, by what it says, or by both. It was one list with no explanation, so a file matching only its contents looked like a file whose name nearly matched. A search says when it stopped rather than pretending it finished. Three things it never said: that the term was too short to run, that it had reached its ceiling and there were more, and that it answered from the first engine alone. And it can be asked for more, in steps, instead of the answer being all there ever was. One thing this batch deliberately does not take from the fork. The fork's search answers from the catalogue and lets a row point at a file that is no longer there, because a sweep tidies the index later. Upstream stats the page of results it is about to hand out and drops what is gone, which is the better answer — a search result that opens nothing is worse than a search that missed something — and it costs one lstat per result rather than a walk. It stays, and the fork's test asserting the opposite was not ported. Whole suite: 2805 passed, 2 failed — the two that already fail on main. Putting the four names back turns three red; taking the existence check out turns four.
…de a volume
`POST /api/permissions/chown` built a command line and gave it to a shell:
chown "<owner>:<group>" "<path>"
`owner` and `group` come from the request body. It is an administrator's route,
but that is the difference between "may change ownership on this volume" and
"may run anything as the user this server runs as", which is not a difference to
leave to a pair of quotes. `chmod -R` and the two `id` lookups did the same with
the path and the numeric ids.
They take their arguments as an array now, through execFile, with no shell
anywhere. And because an argument list is not a free pass either, an account
name has to look like one: `--reference=/etc/shadow` is not a name, and chown
would have read it as an option and copied another file's ownership.
A rule that hides a folder now hides what is inside it consistently. The listing
said one thing and the rule another in several places: a read-only rule left an
administrator the Create button, a hidden rule left the folder's name in a
search result, and a symbolic link out of a share was followed as if it were
part of it. Access is one question asked in one place, and the answer carries
the reason it was refused.
A personal folder name outlives the account it belonged to, while its folder is
still on disk. Deleting an account freed its name, and the next account deriving
the same one was handed a folder full of somebody else's files. Schema 23 adds
the reservations table and, at the upgrade, removes the rows that accounts
deleted before it left behind — `folder_preferences` and `recent_destinations`
point at users through no foreign key, so nothing ever removed them.
One file this batch deliberately does not take from the fork. `routes/volumes.js`
here already drops the volumes a caller may not reach, which nxzai#425 added and
the fork has not caught up with; taking the fork's would have put every hidden
volume's name back in the sidebar. It stays.
tests/routes/permissions.test.js covers what may reach chown, including
`--reference=/etc/shadow` and `root;id`. Against the previous route ten of its
tests fail.
Whole suite: 2859 passed, 2 failed — the two that already fail on main.
A sign-in refused for too many attempts answered "invalid credentials" — the same sentence as a wrong password. Somebody locked out was told they had typed their password wrong, and typing it right did not help, and nothing said why or for how long. It says so now, with the time left, and an administrator can see which accounts are locked and lift a lock without waiting it out. The accounts screens come with it: the list says who is locked and until when, the panel for one account says whether it is and offers to unlock it, and a notice about the server's configuration can be dismissed. Two tests that were failing on main before this batch now pass, and the suite is green end to end for the first time. Neither was a product defect; both were tests that could not do what they meant to. `auth.test.js` removed app.db between apps without closing it. SQLite keeps an unlinked file alive for whoever still holds it open, so the second app in the file went on reading the first one's accounts through the old handle: `/auth/setup` answered "already configured", and a test about a wrong password failed on its first call for a reason that had nothing to do with passwords. It closes the database first — through the instance that has it open, since a fresh one has never opened anything. `closeDb` arrived with nxzai#446. `browse-hidden-files.test.js` wrote a preference for the account id `admin`, which nothing had created. A preference belongs to an account — `user_settings` says so with a foreign key — so it failed on the constraint. The account is created before the preference is written. Whole suite: 2853 passed, 0 failed.
A row in the listing showed a generic icon for everything the application had no picture for, so a .json, a .csv and a .sql looked alike. Each kind carries its own badge now, drawn from one table rather than a chain of conditions, and the extension is read from the name with a bound on what counts as one — a file called `report.2026.final.a-very-long-thing` has no extension worth showing. A thumbnail is asked for when its row comes near the viewport, not when the listing is built. A folder of two thousand pictures asked for two thousand thumbnails at once, which is the queue's whole budget spent on rows nobody had scrolled to. One that fails is retried, with the wait growing each time, instead of leaving an empty square for the rest of the session. The information panel says what a folder weighs when the index knows, and offers to work it out when it does not, saying when it is excluded from the index rather than showing nothing. A favourite can be dropped on to move or copy into it, and one whose volume has gone says so instead of failing when opened. Two things left in the fork on purpose. The inline quick-actions menu on a row was offered in nxzai#333 and closed, so the row ships without it. And `MediaPreview.vue` here has rotate and zoom that the fork's version lost; its video already sits in a grid that contains it, which is what the fork's change was for. It stays. Whole suite: 2853 passed, 0 failed. Built and driven in a browser: the badges render, and nothing is reported on the console.
… folder Three tables name a path: favorites, recent destinations, and the remembered sort and view of a folder. Nothing kept them in step with the paths they name. Deleting a folder cleaned up the favorites of whoever pressed delete. Everybody else was left pointing at a folder that is gone — and on a shared volume that is most of the people who had it. All three tables are cleaned now, for every account, along with anything that pointed inside it. Renaming or moving a folder left them behind on the old path. The favorite still named `Projects/reports` after it became `Projects/2026`, and opening it found nothing. They follow the move now, the folder and everything under it, and a folder whose name merely starts the same is left alone: `Projects/report` is not under `Projects/reports`. The rows that were already stale get a sweep of their own, and a favorite whose volume has gone says so on the screen rather than failing when it is opened — a volume can be unmounted or renamed, and nothing in the database knows. One place matched a path by prefix without the separator, which is how `Projects/reports-old` came to be treated as part of `Projects/reports`. Matching is in one file now, with the case rules of the filesystem it runs on. tests/services/path-bindings.test.js and its neighbours cover the four claims. Cleaning up only the deleter's rows turns three red; leaving the bindings behind on a move turns one. Whole suite: 2904 passed, 0 failed.
Copying a folder answered nothing until it was done. A large one looked like a page that had stopped responding, there was no way to tell a slow copy from a stuck one, and no way to change your mind — the only way out was to close the tab, which left whatever had been written where it was. Both routes stream now: what is about to be copied, how far it has got, and what landed where. Closing the connection stops the copy. The panel that shows it lists what is running, with the rate and what is left, and the same panel serves an upload, an extraction and a compression. The total is the folder-size index's to say. Without it the answer is "unknown" rather than a number that would cost a full walk of the tree to produce — which is the whole of what the copy is about to do anyway. Two defects came out of moving to a copy that reads files itself: The times were lost. `fs.cp` keeps them; a copy that reads and writes does not, unless it puts them back. A folder of photographs sorted by date came out all dated today, and there is no getting them back. `file-transfer-engines.test.js` caught it. And `capabilities` asked the service for `nativeCopyEnabled`, which is now `nativeTransferEnabled`. It threw, and the capabilities endpoint answered 500 — so the About page said nothing about the machine at all. Four upstream tests hooked `fs.cp` or `fs.copyFile` to catch a copy mid-way. Neither is on the path any more, so they hook what is: the handle a file is written through, and the rename that gives a staging copy its real name. One of them now says more than it did — at that moment there are two entries, the hidden copy and the name it will take, held since it was chosen, which is how two copies started at once become `Album` and `Album (1)`. `transferItems` stays, as the two halves composed, for everything that does not need to watch: the trash putting something back, a script, a test. The HTTP layer comes with it, because the download path needs it: posting a hidden form is a navigation, a phone suspends the page, every request in flight ends without a response — and the session probe read its own silence as proof the session was over and sent somebody who was only downloading a file to the login screen, cancelling the download on the way. Whole suite: 2904 passed, 0 failed. Driven against a running server: the stream reports as it goes, and a second copy of the same folder lands as `Source (1)`.
Chunked uploads were on or off, decided once by an administrator who cannot know what stands between the browser and the server. Left off, an upload through a reverse proxy with a body limit failed with nothing to explain it. Turned on, every upload on a local network paid for chunking that nothing there needed. The choice is made where it can be: the browser tries direct, and when that is refused it chunks — with a size that fits what got through, remembered for that address. Local stays direct; behind the proxy it chunks from the second file on. The two settings are mutually exclusive, because "chunk everything" and "chunk when you must" are different answers to the same question. The uploader is three files rather than one: what both engines share, the engine itself, and where an upload is allowed to land. A folder upload settles its destination once, before the first file goes up, so every file of it lands in the same place — two uploads of `Album` started together become `Album` and `Album (1)`, and neither is written into the other. Two things this batch does not take from the fork, both of which upstream is ahead on: `uploadService.js` here records an upload while its bytes arrive and releases the record however it ends, so a stop half way leaves the hidden file for the next start to remove. The fork's version dropped that. It stays. And the tus store and server are built on first use here, not when the module is required. The fork builds them at require time, and `FileStore` creates its directory in its constructor — so requiring the module writes to the disk, and fails outright wherever the cache directory does not exist yet, including the module-load check this repository's CI runs. Upstream's lazy construction stays; the fork's other changes to that file are here on top of it. UPLOAD_CHUNKED_AUTO_FALLBACK is read where the other upload variables are. Whole suite: 2904 passed, 0 failed. The settings screen shows the switch and what it does.
…tion
`router.push({ name: 'FolderView', params: { path } })` hands vue-router one
parameter, and vue-router encodes a parameter as one segment: every slash inside
it comes back as `%2F`, so `/browse/Stacks/data` reached the address bar as
`/browse/Stacks%2Fdata`. The route has never minded — `:path(.+)` matches real
slashes — so the only thing between the two shapes was how the push was written,
and the editor had always written it the other way.
It is not only about looks. Apache refuses an encoded slash unless
AllowEncodedSlashes is turned on, so a deep link copied out of the address bar
could come back as a 404 from somebody's reverse proxy. Addresses already saved
with `%2F` keep working: they decode to the same parameter.
The guards themselves are three files, each a function of what it is given
rather than of the store it reaches into: where to send somebody who is not
signed in, whether a settings page is theirs to open, and whether the home page
should stand aside. A guard that reads a store is a guard that cannot be tested
without one, and these had grown to where the order they ran in mattered.
A folder remembers where it was scrolled to, so going back into one lands where
you left it rather than at the top — restored only when the navigation was a
return, never when it was a fresh arrival.
Whole suite: 2904 passed, 0 failed. Driven in a browser: `/browse/Vol/Sous`
stays as it is written, and the breadcrumb leads back to `/browse/Vol`.
The trash could put something back where it came from, and nowhere else. A folder that has since been renamed, or deleted, or is now somebody else's, left the only copy sitting in the trash with no way out. "Restore to…" asks where, through the same dialog the rest of the application uses, and the restore reports as it goes and can be stopped — a folder of ten thousand files is not an instant. An earlier version could only be put back over the file it belongs to. Two more ways now: taken out as a copy beside it, and put over another file entirely, which is the one that asks before it does it. Restoring and putting a version back both tell the folder-size index what they wrote, and both record where they landed, so the folders somebody files into stay true however things get there. The versions panel says what happened in words rather than closing silently. Two files this batch leaves as they are. `trash/maintenance.js` reads the storage floor from `uploads.storageReserveBytes`; the fork's copy reads `upload.` — the section is not called that here, so the floor read as zero and the trash stopped giving space back when the volume filled. And a version read answers `private, no-store` here, where the fork says only `no-store`: a shared proxy holding somebody's earlier draft is exactly what the header is for. Whole suite: 2904 passed, 0 failed. Driven in a browser: the trash lists what was deleted, from where and by whom, and offers to restore it somewhere else.
…it was A large Markdown file was parsed and rendered whole, on every keystroke of an edit and on every open. A README of a few hundred kilobytes locked the page for as long as it took, and there was nothing on screen to say why. It is cut into slabs at the headings — where a reader's eye already expects a break — and only the slabs near the viewport are rendered. Scrolling brings the next one in. The cut is on the source text, so a fenced block or a table is never split down the middle, and the whole document is still one scroll: nothing is loaded on demand from the server, nothing changes height underneath the reader. Whole suite: 2904 passed, 0 failed.
"Delete" asked one question for two different things. With the trash on it moved the item; with the trash off, or for an item the trash cannot take, it destroyed it — and the wording was the same either way. Somebody who believed they had thirty days to change their mind sometimes had none. The dialog names the outcome for what is actually selected: to the trash, or for good, or a mixture with a count of each. When something cannot go to the trash it says which reason — on another device, larger than the trash allows, the root of a zone, a zone that cannot be written to, no zone at all, or the trash switched off. It names the shares that will go with it, whether a document is open in ONLYOFFICE, and how many earlier versions will go too. The menu itself is sections built from what the selection is, rather than a template with everything in it and half of it hidden. A section that would be empty is not drawn, so the menu no longer opens with three separators and one entry. The terminal takes what is typed as the terminal means it: a paste arrives as one write rather than a keystroke at a time, and the keys that carry meaning — the arrows, Home, End — are sent as the sequences they are rather than dropped. Whole suite: 2904 passed, 0 failed. Driven in a browser: the menu opens with its sections and shortcuts, and deleting a file says "will go to the trash, which keeps it for 30 days" with the two outcomes offered apart.
A refusal reached the screen as whatever the system had said. `EACCES: permission denied, mkdir '/mnt/Photos/2026'` is three things at once: a code nobody reads, a path from inside a container that means nothing outside it, and a 500 for something that is neither a fault of the server nor a thing a retry would change. It answers 403 now, with a sentence — "The server is not allowed to write in this folder." — and EROFS and EPERM alongside it. Every answer carries a code a client can act on rather than a sentence it would have to match. The catalogue turns the code into the reader's language, and the server's own sentence goes underneath where it says which refusal it was, rather than being thrown away. The file store is nine modules instead of one file of five hundred lines: listing, selection, sorting, renaming, transfers, thumbnails, operations, the items themselves, and what ONLYOFFICE is doing. Each one is the thing it is named after, and the store that assembles them is what it always was from the outside. Two corrections came out of it: `/healthz` was no longer mounted. It is what a container's healthcheck calls, and the description still listed it, so the route table and the description had come apart. It is mounted first, before anything else, and answers without asking who is calling. And the configuration had two sections for uploads — `upload` and `uploads` — with the tus directory and the storage floor in one and the request ceilings in the other. Everything that read one of them had to know which. One section now, named as this repository names it. The application's own in-flight files are hidden by default: `.download` while one is being fetched, `.uploading` while one is being written. Nobody put them there and nobody should open them, and they disappear when the operation ends. Whole suite: 2904 passed, 0 failed. Driven against a running server: /healthz answers 200, and /api/features reports the three patterns.
A folder of ten thousand entries put ten thousand rows in the document, each with its icon, its badge and its menu. The browser laid them all out before anything appeared, and scrolling re-laid them out. Only the rows near the viewport are drawn now; the rest are height, so the scrollbar is honest and nothing jumps. A long name is shortened in the middle rather than the end. `rapport-annuel- 2026-consolide-final.xlsx` cut at the end is every file in the folder looking identical; cut in the middle it keeps the part that tells them apart, and the extension. The toolbar says where you are and what can be done from here, and its refresh says it is refreshing rather than looking like nothing happened. The quick-actions menu on the breadcrumb is left out: it was offered in nxzai#333 and closed, so the toolbar ships without it. Whole suite: 2904 passed, 0 failed. Driven in a browser: the listing renders, scrolls, and reports nothing on the console.
…readable A document stayed marked as open by whoever had closed it. Ending an editing session removed the session and left the presence behind, so every listing went on showing the little badge and the name of somebody who had left, until the entry aged out on its own. The presence goes with the session now. The editor's code surface is a component rather than a composable holding a DOM node: it mounts, it unmounts, and it says when a file is too large for highlighting instead of locking the tab trying. A theme can be chosen, and raw text can be read as raw text. Collabora and ONLYOFFICE answer the same questions the same way: what this server can open, where its editor lives, who is holding a lock, and what to do when the Document Server is not there. The lock service answers about a file rather than about a session, which is the thing two editors actually contend for. `POST /api/onlyoffice/config` answers with `editorSessionId` as it always has, and `forceSaveSessionId` beside it — the same value under the name that says what it is for. Anything already reading either keeps working, and both are in the description. Whole suite: 2904 passed, 0 failed.
Each batch so far carried the tests for what it changed. These are the rest:
119 files covering what was already here or what those batches added, and the
eight helpers and one fixture they need — a fake 7-Zip, a soft authenticator, a
legacy database builder, a half-red-half-blue HEIC.
The suite goes from 2904 tests to 3935.
Nine of the fork's test files were left behind on purpose, each because it
asserts something this repository deliberately does differently:
- the search catalogue answering for a file that is no longer on disk, where
this repository stats the page of results and drops what is gone (nxzai#452);
- the session store opening its database when the module is required, where
this repository opens it on first use (nxzai#446);
- the schema numbers, which are this repository's own and not the fork's;
- the chunked maintenance pass, whose subject has not been sent.
And the fork's `tests/scripts/` stay in the fork: they test its own repository
tooling — a commit guard, an install script, an image pruner — not this
application.
`services/indexDb.js` says why it may remove a file directly: the index is built
from the volume and made again whenever it is missing, so it does not go through
the trash. It was the one thing in nxzai#463 that the deletion rule had not been
told about.
Whole suite: 3935 passed, 0 failed.
cerede2000
changed the title
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 #433–#465. The last commit is the one to read.
Each batch so far carried the tests for what it changed. These are the rest: 119 files covering what was already here or what those batches added, plus the eight helpers and one fixture they need — a fake 7-Zip, a soft authenticator, a legacy database builder, a half-red-half-blue HEIC.
The suite goes from 2904 tests to 3935.
Nine files left behind on purpose
Each asserts something this repository deliberately does differently:
search-name-catalogue,search-bases-through-the-indexbetterSqliteSessionStore,session-store-storagedb-migrations,db-legacy-conversions,index-dbdatabase-maintenance,editor-caching,onlyofficeAnd the fork's
tests/scripts/stay in the fork: they test its own repository tooling — a commit guard, an install script, an image pruner — not this application.One thing the deletion rule had not been told about
services/indexDb.jsremoves the index and the files SQLite keeps beside it. It says why it may: the index is built from the volume and made again whenever it is missing, so it does not go through the trash. That was the one thing in #463 the rule had not been told about.Proof
Whole suite: 3935 passed, 0 failed, 30 skipped. Prettier drops to 13 files from
main's 21; eslint's only non-parse error is the one already onmain.