Conversation
Nothing consumes a caption. It arrived on an incoming protobuf pointer, went into a column, came back out as a struct field, and went back out on the wire unchanged: no client renders it, nothing searches it, no index or constraint refers to it, and the conversation-preview query already skips it deliberately. A field that only round-trips is one more column to keep positionally correct in four statements for no behaviour. Field 11 is reserved rather than deleted: clients on the network keep sending it, and the number must never be handed to anything else. The checked-in protobuf output and the generated debug printer are regenerated to match, with protoc 3.21.12 - the version that produced the existing files.
A BlurHash encodes a tiny blurred preview of an image as a short string, so a recipient can draw something in the attachment's place the moment the message arrives rather than an empty box that resizes when the file lands. Field 12: 11 is the next free number but is being reserved by the caption removal, so this takes the one after it. Generated output regenerated with protoc 3.21.12, the version the committed files were produced with; debug_print.cpp comes from gen_debug_print.py over the same descriptors.
Threaded through the same path as the other descriptive fields: off the arriving pointer into message_attachments, back out onto Attachment, and onto the pointer a send builds. Advisory and opaque throughout -- nothing here parses the string or checks it against the file, which on an incoming attachment has usually not been fetched at all. The sender supplies it for the reason width and height are already the caller's to supply: encoding a blurhash needs an image decoder, and taking that dependency on is the decision the comment beside those two declines to make. Length is the one thing checked, and only as a bound on what a peer can put in the database: a blurhash is `4 + 2 * numX * numY` characters, so 28 admits everything up to 4x3 components. An over-long one is dropped on the way in and throws on the way out -- incoming is a remote peer's data, where losing the placeholder must not cost the attachment, and outgoing is our own caller's mistake, reported where it was made. Nothing sends one yet, so the column is null for every message that exists today; a reader that finds it unset is in the ordinary case, not a degraded one.
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
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.
Adds an optional per-attachment blurhash: a short string encoding a blurred preview, so a recipient can draw a placeholder with the right colours before the file arrives. No Session client has one today.
libsession carries it, it does not compute it
This is the crux, and it is the existing argument rather than a new one.
attachment.hppalready says width and height are the caller's to supply "because libsession cannot read them - deriving them means either an image library or hand-written header parsing of untrusted files". Encoding a blurhash needs exactly that decoder, so the field sits beside those two for the identical stated reason.Open question for maintainers, raised deliberately rather than left implicit: nothing produces a blurhash today, so the field will be null for every real message until some sending client fills it. Whether libsession should eventually take on an image dependency and generate these itself - which would also settle thumbnailing, the other thing that comment defers - is your call, not this PR's. It only makes the field exist.
What it adds
blurhashonOutgoingAttachmentandAttachment(include/session/client/attachment.hpp)blurhash TEXTcolumn onmessage_attachments, appended at the end of the table, plussrc/client/schema/007_attachment_blurhash.sqlsrc/client/client.cpp: the attachment read, the outgoing write, the outgoing protobuf build, and the incoming writeoptional string blurhash = 12onAttachmentPointertests/test_client/attachments.cppField number
12, because 11 is
captionand is reserved by the PR below this one. Nothing renumbers.Length is capped at 28
A blurhash's length is fixed by its component counts as
4 + 2 * numX * numY, so 28 characters is a 4x3-component hash and the cap admits everything up to that. Deliberately conservative: 4x3 is what is wanted now, and raising it later is a one-line change that costs nothing on the wire, since the column isTEXTand the protobuf field is an unbounded string either way.It is a bound on what a remote peer can put in the database, not a parse. The string stays opaque: nothing checks its base83 alphabet, and nothing checks that its length agrees with the size flag it declares. There is a comment saying so, because the obvious "improvement" is to turn it into a validator and that would contradict the point of the field.
The two directions differ on purpose. An over-long blurhash arriving from a peer is dropped - the attachment is stored intact without it, because a placeholder is decoration and losing it must not cost someone the file. An over-long one handed to
send_messagethrowsstd::invalid_argumentfrom_require_readable, on the calling thread, for the reason already written above that function: a caller's mistake belongs where it was made rather than in a message that has been stored and shown.Verification
testAll: 407 cases, 25,758,679 assertions, all pass.utils/format.sh verifyclean. Protobuf regenerated with protoc 3.21.12, matching the stamp in the committed.pb.h, so the generated diff is the new field and the_has_bits_shift and nothing else.