matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.82k stars 2.13k forks source link

`matrix.org` exposes unstable endpoint `https://matrix.org/_matrix/client/unstable/im.nheko.summary/rooms/{roomId}/summary` #12562

Open richvdh opened 2 years ago

richvdh commented 2 years ago

This is part of MSC3266, but exposing it on matrix.org encourages clients to use it (see https://github.com/matrix-org/matrix.to/issues/266)

We should plan to disable it, to stop clients being tempted to depend on it.

richvdh commented 2 years ago

according to @t3chguy it's only a fallback, so it should be safe to disable it.

t3chguy commented 2 years ago

No, I said it has a fallback. It is the primary, if you disable it you'll instead have a publicRooms request for iirc 10000 rooms for each matrix.to load

t3chguy commented 2 years ago

This was done as part of delight, this issue must be raised with them. The /publicRooms fallback doesn't contain is_space so will regress the matrix.to behaviour for space links.

richvdh commented 2 years ago

argh. So it's a critical feature for matrix.to?

We can't have a shipped product relying on unstable endpoints forever. We need to at least have a plan to fix this. I'll raise it internally.

t3chguy commented 2 years ago

Only critical for space link previews, not for the other 95% of functionality. Without it spaces will look just like regular rooms

richvdh commented 2 years ago

Having clarified with @t3chguy, it seems that without this endpoint, the following will happen on matrix.to:

  1. No preview (room name/topic/membership count) will be available for many rooms and spaces. (Only those exposed via the room directory (/publicRooms) will have a preview. In particular, Element clients don't let you publish spaces to /publicRooms (due to it not yet being space-aware).)
  2. Performance will be worse, since matrix.to will paginates through the entire /publicRooms list looking for a room. Empirically, it doesn't seem too bad.
  3. Invite links to spaces will look identical to rooms. (The avatar shape is different: spaces have a rounded square; rooms have a circle. This is only visible for spaces/rooms which have an avatar with a non-white background.)

Example: currently: image

Without this endpoint: image

Note that all these things are already true for anyone who chooses to configure matrix.to to use a homeserver other than matrix.org.

richvdh commented 2 years ago

It's also worth noting that Element Android has code that refers to this endpoint, but given EA is used regularly against non-matrix.org servers, I can only assume it has a sensible fallback.

deepbluev7 commented 2 years ago

I updated https://github.com/matrix-org/synapse/pull/11507 and https://github.com/matrix-org/matrix-spec-proposals/pull/3266 now, so an alternative to removing it would be to FCP the MSC and then either stabilize it or remove it depending on the outcome of the FCP.

ShadowJonathan commented 1 year ago

I don't see the problem with this: This would be fine as long as there is code that falls back when this endpoint is not present, it has "unstable" in the endpoint path, and at this stage in matrix development, with global versions being properly underway, there should be no ambiguity as to how clients handle this.

richvdh commented 1 year ago

I don't see the problem with this:

You don't see a problem with matrix.org exposing unstable endpoints? The problem is that it encourages clients to rely on it, which means that other servers have to implement it, which means that we have effectively specced an endpoint with unstable in the name.

ShadowJonathan commented 1 year ago

I don't see a problem in it in that clients should know there is a proper procedure around unstable these days, and if they aren't following it, or the expectation is that they can flout the rules, then that's a bigger problem than just matrix.org, one matrix.org alone can't fix by removing this unstable endpoint.

By not exposing this it might appear to fix this specific problem, but it does't fix the wider problem (not regarding proper procedure around unstable)