From 7ab5c774cbb946a6398b5517492713f3a9a1fc9f Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Thu, 10 Sep 2026 19:29:31 -0400 Subject: [PATCH] A file in a chat keeps its name and its label `Ref.toJson` is what the vault persists next to a message, and it never wrote `filename`, `classification` or `compartments`. They reach the far end correctly, as additive top-level keys on the envelope, so a received document ARRIVED named. It was the local copy that lost them, on the first read back out of storage. A file in a chat therefore had a name until the app was restarted and never again, which reads as a damaged file rather than as the app forgetting. `fromJson` now defaults those three to null rather than to empty. Passing nothing means "not a wire parse, read what was stored"; passing a value, including an empty one, means the envelope said so and wins. Empty defaults could not tell those two apart, and answering "" for both is exactly what made a stored file nameless. The same rule fixes `expires`. The history path hands in 0.0, and 0 means "does not expire", so every stored story read back as immortal: its bytes were never dropped and it rendered forever. A caller that names an expiry still means it; one that does not now gets what the vault kept. They are stored under the same names the shared backup format uses, so `BackupPayload` carries them straight through. A document restored from a backup arrived with no name and no label before this, which is worse than not restoring it. Four tests on the storage round trip, one on the envelope still winning, one that an attachment which never had a name does not invent one, and one carrying a document through a backup whole. Full suite: 1276 tests, no failures. No new ktlint violations in Content.kt (3 at HEAD, 3 now). Co-Authored-By: Claude Opus 5 --- .../kotlin/io/flamenet/e2e/BackupPayload.kt | 10 ++ .../main/kotlin/io/flamenet/e2e/Content.kt | 51 ++++++++-- .../flamenet/e2e/AttachmentNameAtRestTest.kt | 92 +++++++++++++++++++ .../io/flamenet/e2e/BackupPayloadTest.kt | 52 +++++++++++ 4 files changed, 195 insertions(+), 10 deletions(-) create mode 100644 engine/src/test/kotlin/io/flamenet/e2e/AttachmentNameAtRestTest.kt diff --git a/engine/src/main/kotlin/io/flamenet/e2e/BackupPayload.kt b/engine/src/main/kotlin/io/flamenet/e2e/BackupPayload.kt index 93d81a1..fc1cfc0 100644 --- a/engine/src/main/kotlin/io/flamenet/e2e/BackupPayload.kt +++ b/engine/src/main/kotlin/io/flamenet/e2e/BackupPayload.kt @@ -185,6 +185,13 @@ object BackupPayload { a.optDouble("dur", 0.0).takeIf { it > 0.0 }?.let { out.put("duration", it) } // Seconds here, milliseconds there. a.optDouble("exp", 0.0).takeIf { it > 0.0 }?.let { out.put("expires", secondsToMillis(it)) } + // Stored under the same names the shared shape uses, so these three + // pass straight through. They were not carried before because this app + // did not keep them at all; a restored document arrived with no name + // and no label, which is worse than not restoring it. + a.optString("filename").takeIf { it.isNotEmpty() }?.let { out.put("filename", it) } + a.optInt("classification", 0).takeIf { it != 0 }?.let { out.put("classification", it) } + a.optJSONArray("compartments")?.takeIf { it.length() > 0 }?.let { out.put("compartments", it) } return out } @@ -330,6 +337,9 @@ object BackupPayload { .put("h", a.optInt("height")) a.optDouble("duration", 0.0).takeIf { it > 0.0 }?.let { out.put("dur", it) } a.optDouble("expires", 0.0).takeIf { it > 0.0 }?.let { out.put("exp", millisToSeconds(it)) } + a.optString("filename").takeIf { it.isNotEmpty() }?.let { out.put("filename", it) } + a.optInt("classification", 0).takeIf { it != 0 }?.let { out.put("classification", it) } + a.optJSONArray("compartments")?.takeIf { it.length() > 0 }?.let { out.put("compartments", it) } return out } diff --git a/engine/src/main/kotlin/io/flamenet/e2e/Content.kt b/engine/src/main/kotlin/io/flamenet/e2e/Content.kt index 9916c1b..fa51cbb 100644 --- a/engine/src/main/kotlin/io/flamenet/e2e/Content.kt +++ b/engine/src/main/kotlin/io/flamenet/e2e/Content.kt @@ -863,21 +863,45 @@ object Attachment { // isActive treats 0 as immortal, means an expired story's bytes // were never dropped on write. .also { if (expires > 0) it.put("exp", expires) } + // A document's name and its label are what it IS once it + // leaves Files. They reach the far end as top-level keys on the + // envelope, so a received file arrived named; it was the copy + // kept HERE that lost them, because this is what the vault + // persists and it never wrote them. A file in a chat had a name + // until the app restarted and never again, which reads as a + // damaged file rather than as the app forgetting. + .also { if (filename.isNotEmpty()) it.put("filename", filename) } + .also { if (classification != 0) it.put("classification", classification) } + .also { + if (compartments.isNotEmpty()) { + it.put("compartments", org.json.JSONArray(compartments)) + } + } companion object { /** * `filename`, `classification` and `compartments` come from the - * ENVELOPE, not from `att`, they are additive top-level keys, so an - * older parser ignores them and a newer one has to be handed them - * explicitly rather than reading them off the attachment. + * ENVELOPE when a message is being parsed off the wire: they are + * additive top-level keys there, not part of `att`, so an older + * parser ignores them and a newer one is handed them explicitly. + * + * They also come from `att` itself when a message is being read + * back out of the vault, because that is where [toJson] put them + * and there is no envelope left by then. + * + * Hence null rather than empty defaults. Passing nothing means "not + * a wire parse, read what was stored"; passing a value, including + * an empty one, means the envelope said so and wins. Empty defaults + * could not tell those apart, and answering "" for both is what + * made a stored file nameless. */ fun fromJson( kind: String, json: JSONObject, expires: Double, - filename: String = "", - classification: Int = 0, - compartments: List = emptyList(), + filename: String? = null, + classification: Int? = null, + compartments: List? = null, ): Ref? { val key = runCatching { Bytes.unb64(json.optString("key")) }.getOrNull() val digest = runCatching { Bytes.unb64(json.optString("digest")) }.getOrNull() @@ -894,10 +918,17 @@ object Attachment { width = json.optInt("w"), height = json.optInt("h"), duration = json.optDouble("dur", 0.0), - expires = expires, - filename = filename, - classification = classification, - compartments = compartments, + // Same rule: a caller that names an expiry means it, and + // one that does not gets what the vault kept. Without this + // the history path read every stored story as immortal, + // because it hands in 0.0 and 0 means "does not expire". + expires = if (expires > 0.0) expires else json.optDouble("exp", 0.0), + filename = filename ?: json.optString("filename"), + classification = classification ?: json.optInt("classification"), + compartments = + compartments ?: json.optJSONArray("compartments")?.let { array -> + (0 until array.length()).map { i -> array.optInt(i) } + } ?: emptyList(), ) } } diff --git a/engine/src/test/kotlin/io/flamenet/e2e/AttachmentNameAtRestTest.kt b/engine/src/test/kotlin/io/flamenet/e2e/AttachmentNameAtRestTest.kt new file mode 100644 index 0000000..38eaf53 --- /dev/null +++ b/engine/src/test/kotlin/io/flamenet/e2e/AttachmentNameAtRestTest.kt @@ -0,0 +1,92 @@ +package io.flamenet.e2e + +import org.json.JSONObject +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * What a document is once it leaves Files: a name and a label. + * + * `Ref.toJson` is how an attachment is persisted alongside a message, and it + * did not write `filename`, `classification` or `compartments`. They reached + * the wire correctly, so a file ARRIVED with its name; it was the local copy + * that lost it, on the first read back from the vault. A document in a chat + * therefore had a name until the app was restarted and never again, which + * looks like the file was damaged rather than like the app forgot. + */ +class AttachmentNameAtRestTest { + private fun fileRef() = + Attachment.Ref( + kind = Attachment.FILE, + id = "att-1", + key = ByteArray(32) { 1 }, + digest = ByteArray(32) { 2 }, + bytes = 4096, + mime = "application/octet-stream", + filename = "contract.pdf", + classification = 3, + compartments = listOf(2, 7), + ) + + @Test + fun `a stored file keeps its name, its label and its compartments`() { + val back = Attachment.Ref.fromJson(Attachment.FILE, fileRef().toJson(), 0.0) + assertEquals("contract.pdf", back?.filename) + assertEquals(3, back?.classification) + assertEquals(listOf(2, 7), back?.compartments) + } + + @Test + fun `a story keeps its expiry through the vault, not only through the strip`() { + val story = + Attachment.Ref( + kind = Attachment.STORY, + id = "s-1", + key = ByteArray(32) { 1 }, + digest = ByteArray(32) { 2 }, + bytes = 10, + mime = "image/jpeg", + expires = 1_700_600_000.0, + ) + // Read back with no expiry handed in, as the history path does. + val back = Attachment.Ref.fromJson(Attachment.STORY, story.toJson(), 0.0) + assertEquals(1_700_600_000.0, back?.expires) + } + + @Test + fun `the envelope still wins, because that is where an inbound file names itself`() { + // `filename`, `classification` and `compartments` are additive + // top-level keys on the WIRE, not part of `att`. An inbound message is + // parsed with them handed in explicitly, and that must keep overriding + // whatever the attachment object does or does not carry. + val fromWire = + Attachment.Ref.fromJson( + Attachment.FILE, + fileRef().toJson(), + 0.0, + filename = "renamed-by-sender.pdf", + classification = 9, + compartments = listOf(1), + ) + assertEquals("renamed-by-sender.pdf", fromWire?.filename) + assertEquals(9, fromWire?.classification) + assertEquals(listOf(1), fromWire?.compartments) + } + + @Test + fun `an attachment that never had a name does not invent one`() { + val plain = + Attachment.Ref( + kind = Attachment.IMAGE, + id = "i-1", + key = ByteArray(32) { 1 }, + digest = ByteArray(32) { 2 }, + bytes = 10, + mime = "image/jpeg", + ) + val back = Attachment.Ref.fromJson(Attachment.IMAGE, plain.toJson(), 0.0) + assertEquals("", back?.filename) + assertEquals(0, back?.classification) + assertEquals(emptyList(), back?.compartments) + } +} diff --git a/engine/src/test/kotlin/io/flamenet/e2e/BackupPayloadTest.kt b/engine/src/test/kotlin/io/flamenet/e2e/BackupPayloadTest.kt index a107c88..c0f51f4 100644 --- a/engine/src/test/kotlin/io/flamenet/e2e/BackupPayloadTest.kt +++ b/engine/src/test/kotlin/io/flamenet/e2e/BackupPayloadTest.kt @@ -398,4 +398,56 @@ class BackupPayloadTest { assertEquals("t", out.getJSONObject("groupSecrets").getJSONObject("gid-1").getString("tag")) assertTrue(out.getJSONArray("callLog").getJSONObject(0).getBoolean("ok")) } + + @Test + fun `a document keeps its name and its label across a backup`() { + val fileAttachment = + JSONObject() + .put("id", "att-2") + .put("key", "a".repeat(43)) + .put("digest", "b".repeat(43)) + .put("bytes", 900) + .put("mime", "application/octet-stream") + .put("filename", "contract.pdf") + .put("classification", 3) + .put("compartments", JSONArray(listOf(2, 7))) + val internal = + JSONObject().put( + "threads", + JSONObject().put( + "7", + JSONArray().put( + JSONObject() + .put("text", "") + .put("at", 1L) + .put("kind", Attachment.FILE) + .put("att", fileAttachment), + ), + ), + ) + val shared = BackupPayload.toShared(internal) + val sharedAtt = + shared + .getJSONObject("threads") + .getJSONArray("7") + .getJSONObject(0) + .getJSONObject("att") + assertEquals(Attachment.FILE, sharedAtt.getString("kind")) + assertEquals("contract.pdf", sharedAtt.getString("filename")) + assertEquals(3, sharedAtt.getInt("classification")) + + val back = + BackupPayload + .fromShared(shared) + .getJSONObject("threads") + .getJSONArray("7") + .getJSONObject(0) + .getJSONObject("att") + assertEquals("contract.pdf", back.getString("filename")) + assertEquals(3, back.getInt("classification")) + assertEquals( + listOf(2, 7), + back.getJSONArray("compartments").let { a -> (0 until a.length()).map { a.getInt(it) } }, + ) + } }