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) } }, + ) + } }