Attachment availability: say what we hold of a file, and tell everyone showing it by jagerman · Pull Request #168 · session-foundation/libsession-util

jagerman · 2026-09-18T00:17:56Z

A client with auto-download off cannot currently draw an attachment without guessing. It knows the file exists and it knows how to ask for the bytes, but not whether asking means a disk read or a download — so it either fetches everything to find out, or offers a download button over a file that is already here. if (already_have_it) show_it(); else show_download_button(); was not answerable.

This adds that, as fields on Attachment rather than as a separate query: a page of fifty attachments should not cost fifty round trips across a language boundary to find out what to draw.

This PR supercedes/includes parts of #166 and #164.

What a client gets

Attachment::availability is cached, fetching, or absent — respectively "a read", "join the transfer already running", and "the decision is the user's". fetch_done/fetch_total say how far a running transfer has reached, in the same encrypted bytes AttachmentProgress counts, so a conversation opened midway can draw a bar that continues rather than jumps. unavailable records a fetch that failed in a way that trying again will not fix, and says which: not_found (404, which is also how an expired upload answers) or unreadable (the bytes arrived and did not authenticate). Only the first is worth telling a user to ask for a resend about.

unavailable is a cached answer, not a fact about the url, and that distinction is load-bearing. An attachment url is a hash of the encrypted body and the encryption is deterministic, so the same file sent again by the same account lands at the same url. That is the repair path — the recipient is told it could not be fetched, asks for it again, the sender's re-upload puts those bytes back where they were — and a flag that never cleared would block exactly the action that fixes the problem. So it is set across every row naming a url when a fetch of it fails, and cleared across every row naming that url when a new attachment row quoting it arrives.

What we send is kept under the same rule that would have fetched it had it arrived, plus anything on a gallery-viewable message whatever that rule says: a gallery is drawn as its pictures, so those bytes have to be here or the conversation shows placeholders over files that came off this disk. The gallery rule decides it — the same rule the display applies — rather than an attachment happening to be an image, so a message mixing a picture with a document draws as a list like any other and neither file is kept.

Breaking change

message_added and message_updated become messages_added and messages_updated, taking std::vector<Message>&&, and both lose their ConversationId&& parameter. Message::conversation already carried it, and passing it alongside stops making sense once a batch can span conversations. Anything implementing these needs updating — session-cli and the session-app bridge at least.

One cause routinely changes several messages, and reporting them one call at a time made the application reassemble what libsession had just taken apart. A file is the clearest case: it is one file, so evicting it, fetching it, or finding it unfetchable changes every message showing it at once — and reporting a message also reports its replies, since a reply draws a preview of its target. Two things are now promised that could not be before:

ordered oldest first, by the timestamp history is ordered on and then by id, so a handler applying a batch in order lands where a reload would put it. That is the reverse of Conversation::messages(), which pages backwards from the newest.
a batch may span conversations, which is what the fan-out cases need.

Schema

One migration, 005_attachment_availability, in which attachment_cache is dropped and recreated: cached files become orphans that the reconcile sweep clears on its next pass. The cache is disposable by construction, its filenames are keyed now so the old ones name files nothing will look up again, and a surrogate key cannot be added to an existing table without rebuilding it anyway. Nothing else loses data. It also adds the unavailable column, the cached reference, and a partial index for each.

The schema grew over four migrations as the branch was built, one per commit that needed it — right while the commits have to stand on their own, wrong once they merge, since a migration is permanent and numbered and those four describe states that have never existed anywhere but this branch. They are collapsed in the last commit and the history still shows how it was arrived at.

That makes the branch's own intermediate commits the one thing that cannot take the replacement — they have some of the four recorded already — so they go on tests/schema_history_skip.txt, every commit of the branch but the last. A rebase after this point invalidates those hashes and the list has to be regenerated; merging keeps them reachable and it stays valid. Verified separately from the walk: a database at client takes the new 005 cleanly.

Two things worth a reviewer's attention

The cache directory was a membership oracle. Files were named by an unkeyed hash of the url, so anyone who could list the directory could test whether this account had downloaded a particular file — they need only hash the url they are interested in and look. Names are now keyed on the account's cache key with a personalisation, which is half of why the migration discards the cache.

Eviction said nothing. It deleted the row, unlinked the file, and left every message that was drawing that file silently claiming to have it. AttachmentAvailability documents that changes are reported, so this was a promise the code did not keep. It could not: eviction holds a file name, the name is a keyed hash, and a keyed hash does not run backwards — reaching the messages meant hashing every url in the table. The cached reference is what makes the question answerable, and it also takes the hash off the read path entirely: finding an existing copy now goes through the stored name rather than recomputing it, so a future change to the naming costs nothing already downloaded instead of costing the whole cache again.

Supersedes #164

#164 — Report how full the attachment cache is — is cherry-picked here with its authorship intact, so it should be closed rather than merged. It is carried here rather than left to land on its own because the two do not survive each other: keying the cache filenames changed cache::path_for to take the account's key, and #164's test was written against the old signature. Since that test is an insertion into a region this branch never touches, git merges the two without a single conflict and produces a tree that does not compile — a silent break for whoever merges second. The one call is fixed in that commit rather than after it, so no commit here is left unbuildable.

A follow-up commit then removes what having both in one tree exposed: eviction and attachment_cache_size were the same query written twice, along with two copies of the answer to "why is this an optional". SQL sums an empty set to NULL, so an empty cache came back as no answer rather than as no bytes; coalesce settles that in the query, where the NULL is produced, and the C++ stops advertising a state that cannot occur.

Also here

One commit here fixes a download-cancellation bug that is independent of the rest but collides with it directly. FileTransferRequest::cancelled was consulted on every upload path and on none of the streaming download ones, so cancelling a download only stopped us from using what arrived — the rest of the file came down the stream anyway and was dropped a chunk at a time. That is the common case rather than an unusual one: the stream scheme authenticates each chunk as it arrives, so a tampered file is known to be unusable from the bad chunk onwards, and a large attachment can be most of the way through when that happens. Shutdown had the same shape, cancelling its active downloads and then closing with the transfers still running.

It collides with this branch because, once cancellation works, a download we abandoned completes with the cancellation we asked for — so the reason we abandoned it has to be preferred over the status. Otherwise a decryption failure reports itself as download failed with status -10200 and the code that says a retry is pointless is lost, which is precisely the unavailable machinery above.

Testing

405 cases pass, clang-format clean, no compiler warnings.

The transition coverage is the part worth looking at: one file shown by two messages, driven through absent → fetching → absent (transient failure) → fetching → cached → absent (eviction) → unavailable (404) → cleared (resend), asserting at each step which message ids were reported and what state they were told, rather than counting events. Separate tests cover a batch spanning two conversations in the promised order, and the upload case where the message reported is in a different conversation from the send.

Two emission sites have no test of their own, deliberately: the reconcile sweep's stale rows and save_attachment's permanent-failure path. Both are shared calls into paths that are covered, so a test there would pin the call site rather than the behaviour.

tests/schema_history_check.sh fails upgrading from 3df4ae52, on a group state/kicked_timestamp check constraint that 84327685 did not recreate. That predates this branch and is unrelated to it; the migration here upgrades cleanly from client, which the walk now reaches as its first real starting point. CI does not run that script.

A client with auto-download off has to decide between drawing a file and drawing something to press to fetch it, and had no way to tell which: the only question it could ask was `attachment_data`, which fetches on a miss -- so asking was indistinguishable from downloading. `Attachment::availability` answers it, as a field rather than a call, because the caller that needs it most is rendering a page of them across a language boundary and a round trip each would cost more than the whole read. It is a hint about what `attachment_data` will *do* -- read, join, or fetch -- rather than a promise about the file: a `cached` that is evicted first just means the call fetches, which is correct and merely slower. `cached` comes from the `attachment_cache` row rather than a stat, since that row exists precisely so eviction need not walk the directory, and `fetching` from `_in_flight`, checked first: a transfer under way has no cache row yet, and answering `absent` while the bytes are arriving is what puts a download button over the top of a progress bar. The message builders become members to reach it. They were file-local statics because they needed nothing but the database, which is no longer true -- the answer depends on the cache directory and on which transfers are running, and both of those are Client's. Threading it in as an argument was the alternative and a worse one: it is needed one level down, inside the reply targets a read builds with the same code, so every signature in between would have carried a parameter it does not use.

Once availability is part of the message, a change to it is a change to the message: an application holding one now holds something we know is wrong, and `message_updated` is how everything else in here says so. Fired on the three transitions a transfer makes -- `absent` to `fetching` when one starts, and back to `cached` or `absent` when it finishes or fails -- and for every message showing that file, since more than one routinely does. Not display pictures: one belongs to a conversation rather than to a message, and has its own progress handler to report itself with. The completion emit goes before the waiters are told, so that a waiter which reads a message in its callback finds the settled state rather than the one it was called about.

A cached file was named `blake2b(base_url)` -- unkeyed, unpersonalised, and so a function of information the adversary already has. That makes the cache directory a membership oracle: hash a url you are curious about, look for that filename, and you know whether this account downloaded it. Encrypting the contents does nothing about it, because the question is answered by the name before any file is opened. It applied to display pictures equally, so "does this account have a copy of that contact's avatar" was testable the same way. Which defeats the reason the cache is encrypted at all, stated in its own header: the disk is not a trusted place, it ends up in backups, in disk images, and in whoever's hands the machine does. So the name is a MAC rather than a digest, keyed on `client:cache_key` -- the secret the files are already encrypted under, which lives only in the encrypted database -- and personalised, so that one key serving two purposes is kept apart by construction rather than by the primitives happening not to meet. A listing now says how many files there are and nothing else. The key is applied in `Client::_cache_path`/`_cache_name` rather than at each call site: a caller that forgot it would silently get the guessable naming back, so the way to get a name is to ask the object that has the key. Every existing name changes, so the migration drops the cache rows: the files they name are then referenced by nothing, which is what a sweep collects. A cache is the one thing that costs nothing to lose.

Brings binding and retrieval of scoped enums, which the attachment code wants so a column whose values are a named set can be read back as that enum rather than as a bare int the caller has to recognise.

Every question the attachment cache asks of this table is "which messages show this file": which ones to report as fetching when a transfer starts, as cached when it finishes, and as absent again when the file is evicted. More than one message routinely quotes one file, since an attachment url is a hash of the encrypted body and so the same file sent twice by the same account lands at the same url. Without an index each of those answers is a scan of every attachment row in the account to touch a handful of them. Partial on url because a row without one has nothing to fetch and is never the subject of any of it.

A download that fails because the file server does not hold it, or because what arrived did not authenticate, says something about the file rather than about the attempt. Until now that was reported once, to whoever asked, and forgotten: the next thing to look at the message offered the same fetch again, and the only way to find out it could not work was to run it. Record it on the attachment instead, as `Attachment::unavailable`, so a display can say the file cannot be had rather than invite an attempt that will fail. The value says which of the two it was, because they are different things to tell a user: 404 -- which is how an expired upload answers -- means asking the sender for it again will work, and ATTACHMENT_UNREADABLE means it will not, since an attachment url is a hash of the encrypted body and a resend of the same file reproduces the same bytes. Nothing transient is recorded: a timeout or a server error says nothing about the file and the next attempt may well succeed. Set and cleared across every row naming the url, not just the row that was being fetched, since it is one file and a transcript must not show one message's copy as broken and another's as fine. Clearing happens when a new attachment row quoting that url arrives, which is the resend the user was told to ask for; the verdict has to be a cached answer rather than a permanent one, or it would block exactly the action that repairs it. Both directions emit message_updated for the messages they change.

`availability` says a download is running but not where it has reached, which leaves a conversation opened mid-transfer with a bar it cannot position: the progress reports went to whoever started the fetch, and a display that was not listening has no other way back to the figures. Carry them on the attachment alongside the state they qualify, counting the same encrypted bytes `AttachmentProgress` does, so a bar seeded from the message and then fed by reports continues rather than jumping. Also covers the availability states themselves, which went in untested: absent before anyone asks, fetching from the moment the transfer starts, cached once it finishes, and absent again when the cache gives that copy up for another file.

A message we just sent drew as something to press, and pressing it fetched back a file that had come off this disk a moment earlier. Keep what we upload in the attachment cache so an outgoing attachment answers "is this here" the same way an incoming one does. Under the rule that would have applied to the same file arriving -- shared with the auto-download path rather than restated -- plus anything on a gallery-viewable message whatever that rule says: a gallery is drawn as its pictures, so those bytes have to be here or the conversation shows placeholders over files it could read directly. The gallery rule decides that, the same rule the display applies, rather than an attachment happening to be an image: a message mixing a picture with a document draws as a list like any other. The source is re-read, since the upload streams the file rather than holding it, and only when its length still matches what was uploaded: a url names one particular encrypted body, and storing anything else under it would leave the cache answering for that url with the wrong bytes. Everything else about it is best effort -- a file that has since moved just means a download if the message is ever drawn again.

A cached file is named by a keyed hash of its url, and that hash was how everything found it: "is this file here" hashed the url and looked the result up. Two things follow from that, and both are worth being rid of. The hash becomes load-bearing for every file already downloaded rather than only for the ones being written, so changing it cannot be done without invalidating the lot -- which is exactly what 005 had to do. And it does not run backwards, so eviction, which holds a file name and needs the messages drawing that file, could not get to them at all: it deleted the row and the file and said nothing, leaving conversations showing pictures whose files had gone. `AttachmentAvailability` already promises those changes are reported. Give `attachment_cache` a surrogate key and have an attachment row reference it, `ON DELETE SET NULL`. Nothing is duplicated -- the row stores an integer, not a second copy of the name or the url -- and the reference runs the way the questions do: - availability comes back with the row, so reading a page no longer hashes a url or looks anything up per attachment; - finding an existing copy goes through the reference and uses the name the entry was stored with, so the hash applies only to a file being created and can change freely; - eviction reads the messages off an index before the delete, and the foreign key clears their reference on the way out. The reconcile sweep drops its stale rows through the same path, so a file that vanished behind our back is reported like one we deleted on purpose. `_in_flight` moves to being keyed by url at the same time: the hashed name is about what a directory listing reveals, which is not a question a map in this process has.

Caching a file said nothing. On the download path that was hidden: the fetch's completion emitted right after storing, so the messages showing the file were told. An upload has no such completion, so keeping a copy of a file we sent -- which makes it drawable for every message quoting that url, and an attachment url is a hash of the encrypted body, so somebody having sent us the same file is the ordinary case -- told nobody but the message being sent. Move the emit to `_cache_attachment`, which is where a file becomes cached and is reached both ways. `store` then says whether it got that far, and the fetch reports the transfer ending only when it did not: a failure, a file that could not be written, and no cache at all are the same event to a reader, and all three leave nothing behind. The entry also has to leave `_in_flight` before any of that runs, or the report of a file arriving says it is still arriving. The test drives one file, shown by two messages, through every transition -- absent to fetching, back to absent for a transient failure, to cached, out again by eviction, to unavailable on a 404 and clear again on a resend -- and asserts which messages were told and what they were told the state was, rather than counting events. A second covers the upload case across conversations.

One cause routinely changes several messages, and reporting them one call at a time made the application reassemble what libsession had just taken apart. A file is the clearest case: it is one file, so evicting it, fetching it, or finding it unfetchable changes every message showing it at once -- and reporting a message also reports its replies, since a reply draws a preview of its target. So `message_added` and `message_updated` become `messages_added` and `messages_updated`, taking a vector. Two things are now promised that could not be before: - **ordered oldest first**, by the timestamp history is ordered on and then by id, so a handler applying a batch in order lands where a reload would put it. That is the reverse of `messages()`, which pages backwards from the newest; - **a batch may span conversations**, which is what the fan-out cases need. Nothing is lost by allowing it: `Message` already carries its own `conversation`, which is why the callbacks no longer take one. Eviction and the reconcile sweep now collect across the whole pass, so a message showing two of the files being dropped is told once rather than twice, and the ids are resolved to Messages only at the end -- one build per message, in its settled state rather than in whatever state it was in partway through the pass.

`FileTransferRequest::cancelled` was consulted on every upload path and on none of the streaming download ones, so cancelling a download only stopped us from using what arrived: the rest of the file came down the stream anyway and was dropped a chunk at a time. That is the common case rather than an unusual one. The stream scheme authenticates each chunk as it arrives, so a file that has been tampered with, or one that runs past the length its sender claimed, is known to be unusable from the bad chunk onwards -- and a large attachment can be most of the way through when that happens. Shutdown was the same: the routers cancel their active downloads and then close, with the transfers still running. Check the flag as each chunk arrives, which is the only moment this end of a transfer is given -- a download is driven by the server -- and abort the stream when it is set. A stalled stream still notices nothing, and the caller's timeouts remain what end those. The layer above needed one correction to suit it: a download we abandoned now completes with the cancellation we asked for, so the reason we abandoned it has to be preferred over the status when there is one. Otherwise a decryption failure would report itself as "download failed with status -10200", and the code that says a retry is pointless would be lost.

There was a getter for the cache limit and none for what is in it, so a client could show the ceiling but not how close to it the cache was. Summed from the cache index, in bytes on disk, which is the measure the limit is in and the one eviction compares against.

Eviction and `attachment_cache_size` were the same query written twice, which is also two copies of the answer to "why is this an optional" -- SQL sums an empty set to NULL, so an empty cache came back as no answer rather than as no bytes. Answer it once, in SQL, where the NULL is produced: `coalesce` makes the query incapable of returning one, so the C++ type stops advertising a state that cannot occur. That is how the rest of this file defaults a NULL already. The connection-taking overload is for eviction, which holds one and is about to write through it.

The schema grew over four migrations as this branch was built, one per commit that needed it, which is right while the commits have to stand on their own and wrong once they are merged: a migration is permanent, numbered and ordered, and these four describe intermediate states that have never existed anywhere but here. One migration per merged change instead. 005 now does what all four did, and the commits still show how it was arrived at. Its predecessors are exactly the databases that cannot take it -- they have some of the four recorded already -- so the branch goes on the schema-history skiplist: every commit of it but this one, since no state of it before this is something anything can have started from.

jagerman and others added 15 commits September 17, 2026 20:18

Update session-sqlite

73dc04a

Brings binding and retrieval of scoped enums, which the attachment code wants so a column whose values are a named set can be read back as that enum rather than as a bare int the caller has to recognise.

Navigation Menu

Attachment availability: say what we hold of a file, and tell everyone showing it - #168

Attachment availability: say what we hold of a file, and tell everyone showing it#168
jagerman wants to merge 15 commits into
session-foundation:clientfrom
jagerman:attachment-availability

jagerman commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants