matrix-spec: Inconsistency for edits to encrypted events

It sounds like there is general support for option 2 in the list above: stop servers performing edits, and put the whole of the edit event into the relationship aggregation.

This is how I was expecting edits to work until I implemented them in the Rust SDK. It makes no sense to me that “relation aggregation” would touch the content of an event, or that a homeserver would ever communicates a different event content over federation vs. CS-API.

I am happy that doing a breaking change to clean this up is being considered 😃

It sounds like there is general support for option 2 in the list above: stop servers performing edits, and put the whole of the edit event into the relationship aggregation.

In this scenario, an event which had been edited would look like this, when returned by the server:

{
  "event_id": "$original_event_id",
  "type": "m.room.message",
  "sender": "@sender:localhost",
  "origin_server_ts": 1649772301230,
  "room_id": "!room:example.org",
  "content": {
    "body": "Original message",
    "msgtype": "m.text",
  },
  "unsigned": {
    "m.relations": {
      "m.replace": {
        "event_id": "$latest_edit_event_id",
        "type": "m.room.message",
        "sender": "@sender:localhost",
        "origin_server_ts": 1649772304313,
        "room_id": "!room:example.org",
        "content": {
          "body": " * Edited message",
          "msgtype": "m.text",
          "m.new_content": {
              "body": "Edited message",
              "msgtype": "m.text"
          },
          "m.relates_to": {
            "rel_type": "m.replace",
            "event_id": "$original_event_id"
          }
        }
      }
    }
  }
}

Note that the implication is that all clients must now inspect the bundled aggregation, and extract the m.new_content, otherwise they may incorrectly show the original event when back-paginating.

An encrypted event with edits would look like:

{
  "event_id": "$original_event_id",
  "type": "m.room.encrypted",
  "sender": "@sender:localhost",
  "origin_server_ts": 1649772301230,
  "room_id": "!room:example.org",
  "content": {
    "algorithm": "m.megolm.v1.aes-sha2",
    "session_id": "<outbound_group_session_id>",
    "ciphertext": "<original_encrypted_payload_base_64>"
  },
  "unsigned": {
    "m.relations": {
      "m.replace": {
        "event_id": "$latest_edit_event_id",
        "type": "m.room.encrypted",
        "sender": "@sender:localhost",
        "origin_server_ts": 1649772304313,
        "room_id": "!room:example.org",
        "content": {
          "algorithm": "m.megolm.v1.aes-sha2",
          "session_id": "<outbound_group_session_id>",
          "ciphertext": "<encrypted_replacement_payload_base_64>"
          "m.relates_to": {
            "rel_type": "m.replace",
            "event_id": "$original_event_id"
          }
        }
      }
    }
  }
}

As before, the encrypted_replacement_payload would be simply:

{
  "type": "m.room.message",
  "room_id": "!some_room_id",
  "content": {
    "body": "* Edited message",
    "msgtype": "m.text",
    "m.new_content": {
      "body": "Edited message",
      "msgtype": "m.text"
    }
  }
}

My concern is that this could be a breaking change for clients which now expect servers to apply edits before returning them. I would welcome input from client authors on this point.

[have servers not apply edits] My concern is that this could be a breaking change for clients which now expect servers to apply edits before returning them.

Incidentally, I polled a couple of client developers on this:

• @BillCarsonFr said that Element-Android will apply any edits it happens to receive in its timeline, and otherwise just displays any content it receives from the server. This is pretty broken as it is, because (a) it doesn’t work at all for encrypted rooms, and (b) if the client sees edit events, there is no guarantee that it can see the most recent edit event - so it may actually be overwriting the “latest” content as returned by the server with a less-recent edit.
• @t3chguy said that Element-Web relies on the server to return the modified content, and not doing so will produce incorrect results for permalink views, etc. The exception is encrypted rooms, because EW special-cases encrypted rooms/events.

So it probably would cause breakage that we should avoid if possible. One idea is to make the change in three steps: first servers start returning the entire edit event in unsigned, then clients switch over to it, and finally servers stop replacing content.

@kegsay:

Is it not possible to flip the event stack on it’s head and have the outer event be the edit and then in unsigned have the base event being edited? Currently it is the inverse where we have the original event and then in unsigned the newer event sits. That way, if you don’t care about edits (because you don’t implement them) then you see the right thing, and when you do care about them you can just inspect unsigned to present the “edits” dialog.

I’ve been thinking a bit more about this. I’m still worried that, whilst it sounds attractive on the surface, there’s complexity here.

For instance: suppose you want to react to an event that has been edited. Reactions are supposed to be applied only to the original event, so as a client you still have to remember to go and dig in the unsigned object to figure out how to build the reaction.

I’m very much inclined to leave this for another day.

(I said “event types” above but actually meant “relationship type”.)

The presence of m.new_content leaks the fact that an event is an edit. In today’s world this doesn’t seem too terrible, since we’re currently leaking relationship types in the clear anyway, but it’s a move in the wrong direction since we’d ideally want to be able to encrypt relationship types too (as talked about in https://github.com/matrix-org/matrix-spec/issues/660).

put the encrypted content inside m.new_content, rather than the other way around.

I’d rather we avoid this, because it would be yet another metadata leak and would add another hurdle to hiding <del>event</del> relationship types (of encrypted events) in the future.