Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

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

Open
richvdh opened this issue Apr 26, 2022 · 11 comments
Labels
A-Spec-Compliance places where synapse does not conform to the spec T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@richvdh
Copy link
Member

richvdh commented Apr 26, 2022

This is part of MSC3266, but exposing it on matrix.org encourages clients to use it (see matrix-org/matrix.to#266)

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

@richvdh richvdh added A-Spec-Compliance places where synapse does not conform to the spec T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Apr 26, 2022
@richvdh
Copy link
Member Author

richvdh commented Apr 26, 2022

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

@t3chguy
Copy link
Member

t3chguy commented Apr 26, 2022

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
Copy link
Member

t3chguy commented Apr 26, 2022

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
Copy link
Member Author

richvdh commented Apr 26, 2022

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
Copy link
Member

t3chguy commented Apr 26, 2022

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

@richvdh
Copy link
Member Author

richvdh commented Apr 27, 2022

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
Copy link
Member Author

richvdh commented Apr 27, 2022

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
Copy link
Contributor

I updated #11507 and matrix-org/matrix-spec-proposals#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
Copy link
Contributor

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
Copy link
Member Author

richvdh commented Nov 7, 2022

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
Copy link
Contributor

ShadowJonathan commented Nov 7, 2022

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)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Spec-Compliance places where synapse does not conform to the spec T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

4 participants