MSC2285 hidden/private read receipts setting no longer functioning on Synapse 1.59 or later

[syldrathecat]

MSC2285 for private read receipts was updated (https://github.com/matrix-org/matrix-spec-proposals/pull/2285/files) to use a new receipt type (org.matrix.msc2285.read.private) instead of the previously proposed read_fully flag (org.matrix.msc2285.hidden).

As of Synapse 1.59, support for the org.matrix.msc2285.hidden flag was ripped out (https://github.com/matrix-org/synapse/commit/116a4c8340b729ffde43be33df24d417384cb28b) and replaced with support for the new read receipt type org.matrix.msc2285.read.private. This means all clients sending org.matrix.msc2285.hidden are now having their privacy request dropped by the server and are leaking read receipts once again.

Unfortunately both versions of the proposal share the same org.matrix.msc2285 unstable feature flag, making it effectively impossible to detect which servers will respect the org.matrix.msc2285.hidden flag.

[deepbluev7]

Yeah, I was meaning to update that for a while, but I always forget. I think specifically https://github.com/Nheko-Reborn/mtxclient/blob/master/lib/http/client.cpp#L932 will need to be updated.

[syldrathecat]

I was able to get a Matrix 1.59 server with the feature enabled to accept the read receipts and clear notifications with this change.

diff --git a/lib/http/client.cpp b/lib/http/client.cpp
index cd8a294..3f843c0 100644
--- a/lib/http/client.cpp
+++ b/lib/http/client.cpp
@@ -930,7 +930,7 @@ Client::read_event(const std::string &room_id,
       "/client/r0/rooms/" + mtx::client::utils::url_encode(room_id) + "/read_markers";

     nlohmann::json body = {
-      {"m.fully_read", event_id}, {"m.read", event_id}, {"org.matrix.msc2285.hidden", hidden}};
+      {"m.fully_read", event_id}, {hidden ? "org.matrix.msc2285.read.private" : "m.read", event_id}};

     post<nlohmann::json, mtx::responses::Empty>(
       api_path,

On a server without MSC2285 enabled this totally breaks notification clearing however, so experimental feature detection is probably required. Unfortunately even with feature detection, pre-1.59 Synapse servers will just have broken notifications, because they support the old protocol advertised under the same feature name.

It also seems like there's other code that needs updating as well, because this warning starts to appear:

[2022-05-27 09:32:37.827] [mtx] [warning] Error parsing events: [json.exception.out_of_range.403] key 'm.read' not found, {
  "content": {
    "<redacted>": {
      "org.matrix.msc2285.read.private": {
        "@<redacted>": {
          "ts": 1653609758371
        }
      }
    }
  },
  "type": "m.receipt"
}

I guess originating from here https://github.com/Nheko-Reborn/mtxclient/blob/a4f4c79153cc139c44aaf7bd5bf47fef6c0712b3/lib/structs/events/ephemeral/receipt.cpp

I'm not totally sure how the logic should look here since I don't understand the spec very well, but I'm guessing you just want the greater of either available timestamp.

[deepbluev7]

We probably need to change the receipts to make the different receipts optional so that clients can decide to store them separately, but it probably makes sense to just store one of them in Nheko rn. And I guess we will have to live with read receipts being broken on old servers then. But I'll comment on the MSC.

[deepbluev7]

I guess it is fine to just have it subtly broken and just implement the newest iteration. We still need to figure out how to store the different receipt now though.

[symphorien]

I tested this patch and it does not mark the messages as read in element android so I suspect it does not send valid receipts at all :)

[deepbluev7]

Fixed in a6ca9714490ad0bb0c032e00be4f7d7ebd683378

Other clients might not accept those events as read, but that's a limitation of that MSC.