Skip to content

MSC3911: Linking media to events #3911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Oct 16, 2022

@turt2live turt2live added proposal A matrix spec change proposal s2s Server-to-Server API (federation) client-server Client-Server API security kind:core MSC which is critical to the protocol's success hacktoberfest-accepted needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Oct 16, 2022

### Overview

After an item of media is uploaded, it must be linked to an event (via
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For MLS, we may need to reference media within a to-device event, but we might be able to link it to an in-room event. I'll have to look into this more, but flagging this as something that will need to be considered.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's also ephemeral-ish things like user profiles which aren't directly related to events, though are duplicated/copied to membership events in most cases.

@turt2live turt2live self-requested a review October 18, 2022 17:52
Copy link

@Munna231 Munna231 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • #

@richvdh
Copy link
Member Author

richvdh commented Oct 23, 2022

I've moved a bunch of the changes here out to MSC3716 MSC3916, in the hope of making them a little more digestible.

@richvdh richvdh force-pushed the rav/proposal/linking_media_to_events branch from 464e5a4 to cc853ef Compare October 23, 2022 23:17
@richvdh richvdh closed this Oct 23, 2022
@deepbluev7
Copy link
Contributor

deepbluev7 commented Oct 23, 2022

I've moved a bunch of the changes here out to MSC3716, in the hope of making them a little more digestible.

You meant MSC3916 (if anyone else has issues following that link :D)

@richvdh richvdh reopened this Oct 23, 2022
@richvdh
Copy link
Member Author

richvdh commented Oct 23, 2022

I've moved a bunch of the changes here out to MSC3716, in the hope of making them a little more digestible.

You meant MSC3916 (if anyone else has issues following that link :D)

Thank you, yes I did 🤦. Edited.

Comment on lines +233 to +235
In addition, this could cause duplication of media in the remote media cache,
if the implementation does not take steps to deduplicate (eg, storing media
by content hash rather than media id).
Copy link
Contributor

@reivilibre reivilibre Mar 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we spec a response header for the media download that provides a suitable hash of the object?
e.g. X-Matrix-Media-Hash: blake2,AAAAAAAAAAAAAAAAAAAAAAAAAAA....
That way, a homeserver (or client!) which is downloading a copy of the media can abort the download if it finds the hash already in its media store. This means that copies of large media do not lead to each server downloading it for each copy.
(Further, for bandwidth-sensitive clients, a HEAD could be used to get the hash before even initiating the download?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

De-duplication is already possible at an implementation level, and would probably be handled by a different MSC.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

De-duplication is already possible at an implementation level

I think that reivilibre wasn't talking about the way clients/homeservers are deduplicating files after download, but rather before they're downloaded. I agree that former is an implementation detail that doesn't require uniformity across software, and as such doesn't need to be covered in protocol spec. But if the intention here is to avoid the very process of downloading a file multiple times (from a different linked MXC each time), can this be done without protocol declaring one well known way to discover e.g. that hash of underlying file by HEAD request?

I'm specifically thinking of a situation where you've got a user looking at sticker selector and not-so-maliciously spamming them by clicking them 100s of times. Some people do that, it isn't really a bad behavior in some rooms and contexts (slightly distant example would be custom emote spam on Twitch chats). Since the stickers sent in each event are no longer sharing the same MXC, AFAIK client (and homeserver, if sender is remote) will have to download the sticker 100s of times, and thus generate a lot of unnecessary traffic. Only then it will be able to deduplicate file for local storage/cache.

So my (and I assume reivilibre's) concern here is that with the proposal as it is right now, there's no way for client/homeserver to avoid that "traffic duplication", so to speak. Or is there? 😅

Copy link

@rom4nik rom4nik Jan 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, on the other hand, that choice of hash type used for deduplication before download, might eventually have an effect on future implementations of dedupe after download. For example, iirc currently in Synapse there's a mapping of MXC URI to local filename which is a random string. With this MSC, each server would have to store a mapping from the new generated IDs to the local file (in Synapse's case, again, a random name). If protocol adds a way to map MXC IDs to file hash early, is there a reason for servers to not store files by naming them after their hashes (as received over e.g. federation)? This would save you one more lookup: MXC IDs -> file hash -> randomized local name.

Oh, and I haven't thought how/if this "early dedupe" could realistically work with files attached to E2EE events, and whether it would add any additional metadata leak other than what's caused by this MSC as is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a way to map MXC IDs to file hash early

This makes a lot of sense to me. There's no real benefit in making up garbage strings to then use as the MXC ID. This was useful pre-authorisation because it meant you could hypothetically drop references to the same piece of content which was used elsewhere, but if we're adding in authorisation for that link then there's little reason to keep the random MXC IDs around imo.

Comment on lines +35 to +39
1. A new "media upload" endpoint is defined, `POST
/_matrix/client/v1/media/upload`. It is based on the existing
[`/_matrix/media/v3/upload`](https://spec.matrix.org/v1.4/client-server-api/#post_matrixmediav3upload)
endpoint, but media uploaded this way is not initially viewable (except to
the user that uploaded it). This is referred to as a "restricted" media item.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: extend this to /_matrix/media/v1/create, as added by MSC2246.

Comment on lines +148 to +149
This "copy" api is to be used by clients when forwarding events with media
attachments.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd need to use this for m.sticker events too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and custom emoji, when that exists)

--gc0p4Jq0M2Yt08jU534c0p
```

5. New "media copy" API
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A downside of the media copy API is clients would no longer be able to reliably cache media. For the example of stickers or emoji, popular/busy rooms could have tens of references to the same media object, which causes tens of downloads because the client doesn't know different.

Possible alternatives (for future MSCs, imo) would be:

  1. Allow some media objects to be reference counted/linked to multiple events.
  2. Include a hash of the file in the event, so clients can locally realize that the objects are all the same content-wise.

I'm more preferential to the second.

--gc0p4Jq0M2Yt08jU534c0p
```

5. New "media copy" API
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

going down the rabbit hole of custom emoji, there's an interaction with MSC4027 we will have to figure out.

@AndrewRyanChama

This comment was marked as duplicate.


Fixes [synapse#1263](https://github.com/matrix-org/synapse/issues/1263).

## Potential issues
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndrewRyanChama says:

How does this interact with m.replace? Will the client be expected to /copy all of the medias or will it be handled automagically?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the replacing event contains the same media, /copy seems reasonable to me because it keeps with the idea of attaching media to a single event only.

#### Redacting events

Under this proposal, servers can determine which media is referenced by an
event that is redacted, and add that media to a list to be cleaned up.
Copy link

@AndrewRyanChama AndrewRyanChama Dec 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this is that if the user who sent the media leaves the room, the homeserver may no longer get the reaction events. That means there will be no mechanism for those events to be removed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the server doesn't have a local user in the room anymore, I suppose it should remove the room, the events and any linked media anyway?

Content-Type: application/json

{ "restrictions": {
"event_id": "$Rqnc-F-dvnEYJTyHq_iKxU2bZ1CI92-kuZq3a5lr5Zg"
Copy link

@AndrewRyanChama AndrewRyanChama Dec 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that the association between (encrypted) media and event ids will be plainly accessible to the homeserver, and federated servers. I'd imagine that isn't something we actually care about but something that others may complain about.

Given that /copy presumably can't actually decrypt the media, that also means that the homeserver can associate any copied encrypted media as well

Comment on lines +90 to +92
The `/_matrix/federation/v1/media/download` and `/_matrix/federation/v1/media/thumbnail`
endpoints specified by [MSC3916](https://github.com/matrix-org/matrix-spec-proposals/pull/3916)
are extended: the returned json object may have a property `restrictions`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are several updates required to this MSC - the priority has been to get 3916 into working order, then the team will move onto this MSC afterwards.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MSC3916 was merged 🥳

Copy link
Contributor

@HarHarLinks HarHarLinks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewing with my Nordeck hat on.

The existing endpoint is deprecated. Media uploaded via the existing endpoint
is "unrestricted".

2. Attaching media
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the proposal is called "Linking media to events" and "linking" terminology is used outside of this list item. is this intentional? can we agree on a single term otherwise? "attaching" seems better suited.


If any of the `attach_media` parameters do not correspond to known
restricted media items, or they refer to restricted media items that have
already been attached, the server responds with a 400 error with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reading this proposal top down, it is not entirely clear whether "already attached" means a different event or a duplicate query parameter.

Comment on lines +76 to +77
If the media is not attached to either an event or a profile within a reasonable period
(say, ten minutes), then the server is free to assume that the user has changed their
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for clarity: the period starts as the media upload has finished, which is particularly relevant with async uploads?


The new `/download` and `/thumbnail` endpoints added in
[MSC3916](https://github.com/matrix-org/matrix-spec-proposals/pull/3916) are
updated the server must check if the requesting user or server has
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
updated the server must check if the requesting user or server has
updated such that the server must check if the requesting user or server has

cache the restrictions list.

If neither `event_id` nor `profile_user_id` are present, the requesting
user should assume that an unknown restriction is present, and not allow access
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
user should assume that an unknown restriction is present, and not allow access
homeserver should assume that an unknown restriction is present, and not allow access

* The only protection for media is the obscurity of the URL, and URLs are
easily leaked (eg accidental sharing, access
logs). [synapse#2150](https://github.com/matrix-org/synapse/issues/2150)
* Anybody (including non-matrix users) can cause a homeserver to copy media
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since authenticated media landed, I think

Suggested change
* Anybody (including non-matrix users) can cause a homeserver to copy media
* Any locally authenticated user can cause a homeserver to copy media

Comment on lines +177 to +180
It is expected that servers will continue to treat such media as unrestricted
(at least for local users), but it would be legitimate for them to, for example,
return a different `mxc:` URI for each requesting user, and only allow each user
access to the corresponding `mxc:` URI.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leaving url previews as a way to keep "uploading" unrestricted media would leave open the attack vector this MSC is trying to close.

Comment on lines +200 to +201
* special-casing the bridge AS user to permit it to upload media without
expiry.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* special-casing the bridge AS user to permit it to upload media without
expiry.
* special-casing the bridge AS user to permit it to upload unrestricted media.

other ideas:

  • attaching media to messages retroactively (e.g. the pastebin copy to the original long message)
  • tracking media origin room and/or user so AS don't upload entirely unrestricted media


Fixes [synapse#1263](https://github.com/matrix-org/synapse/issues/1263).

## Potential issues
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please consider that widgets also use media, i.e. #4039

to summarize, linking media is not a big issue for events of a well-known structure referencing them - with regards to widgets, that means that clients can parse the payload and handle it automatically. however especially for widgets it is often attractive to send nonstandard events (let alone ones with some custom encoding), that prohibit this.

there can be use cases (with widgets or without), where multiple media are attached to an event that represents a certain state at a point in time and which is edited collaboratively. however editing would usually involve changing only one of the referenced media items at most, resulting in an immense overhead when having to copy all the other attached media as well when sending an update (example implementation).

(This mechanism, rather than just allowing clients to attach media to multiple
events, is necessary to ensure that the list of events attached a given piece of
media does not grow over time, which would make it difficult for servers to
reliably cache media and impose the correct access restrictions.)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this would be a problem? Don't servers have to do this anyway if they store copies as references?


4. Federation API returns a `restrictions` property

The `/_matrix/federation/v1/media/download` and `/_matrix/federation/v1/media/thumbnail`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of illegal content, users may want clients to delete the content from local caches as soon as possible, hence recommending clients purge media from redacted events would make sense, either by them remembering which event caused them to download the media, or sending the restrictions in the client/server API endpoints.

return a different `mxc:` URI for each requesting user, and only allow each user
access to the corresponding `mxc:` URI.

### Applications
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Encrypted-history sharing (currently in progress, but tracked at element-hq/element-meta#39, MSC4268) adds another application for media blobs which are not tied to a specific event. It might be that a separate mechanism which creates media specifically for sharing with one other user/device would help.


If the media is not attached to either an event or a profile within a reasonable period
(say, ten minutes), then the server is free to assume that the user has changed their
mind (or the client has gone offline), and may clean up the uploaded media.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"may clean up" means servers could decide to not do so. This would create dangling media that are not viewable by other users but still stored on the server which, I think, could still be an issue with respect to GDPR erasure. Should we make clean-up RECOMMENDED at least?

The new `/download` and `/thumbnail` endpoints added in
[MSC3916](https://github.com/matrix-org/matrix-spec-proposals/pull/3916) are
updated the server must check if the requesting user or server has
permission to see the corresponding event. If not, the server responds with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec allows servers to restrict profile look-ups (the minimum being users that share rooms). A server that does this should probably also apply this check when user avatars are downloaded?

Suggested change
permission to see the corresponding event. If not, the server responds with
permission to see the corresponding event or profile. If not, the server responds with


## Potential issues

* Since each `m.room.member` references the avatar separately, changing your
Copy link
Contributor

@benkuly benkuly Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a big issue to me, that should be adressed in this MSC. From a client perspecive it means, that a simple user avatar change would force clients to download the same file n times. Why not just allow multiple restrictions per mxc uri?

* The methods for sending events
([`PUT /_matrix/client/v3/rooms/{roomId}/state/{eventType}/{stateKey}`](https://spec.matrix.org/v1.4/client-server-api/#put_matrixclientv3roomsroomidstateeventtypestatekey)
and [`PUT /_matrix/client/v3/rooms/{roomId}/send/{eventType}/{txnId}`](https://spec.matrix.org/v1.4/client-server-api/#put_matrixclientv3roomsroomidsendeventtypetxnid))
are extended to take a query parameter `attach_media`, whose value must be a complete `mxc:` URI.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree with this. We're going to be hitting URL length limits, query escaping issues, etc if we dump MXC URIs as query params. I would be much more happy if we just used the media id component, given the server name and mxc:// are redundant. This makes it:

  • shorter
  • less likely to cause escaping issues

..at the cost of needing clients to split the MXC URI, though clients already need to do some amount of introspection of the URI so this feels fine.

[MSC3916](https://github.com/matrix-org/matrix-spec-proposals/pull/3916) are
updated the server must check if the requesting user or server has
permission to see the corresponding event. If not, the server responds with
a 403 error and `M_UNAUTHORIZED`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the server must check if the requesting user or server has permission to see the corresponding event. If not, the server responds with a 403 error and M_UNAUTHORIZED.

This needs clarification. It's either:

  • based on history visibility, or
  • based on current room membership

I would advocate for it to be based on current room membership, not history visibility because:

  • history visibility makes no sense for room avatars, per-room profile avatars, etc which are room state. It would mean newly joined users would be unable to see the room avatar / etc because the room avatar event was sent prior to their join.
  • history visibility is not enforceable over federation, providing a false sense of security. At a federation level we rely on simply "was there at least 1 user on your server joined at that time", which is not the same as "were you joined at that time".
  • history visibility is hard to cache, as concurrent changes to that event can cause previously visible events to become hidden or vice versa. This is hard to explain to clients.
  • history visibility interacts poorly with E2EE rooms, where "visibility" is not just whether you see the event JSON, but whether you have the keys to decrypt said JSON. This poor interaction means you could be sent event JSON which you can't decrypt, or not be sent event JSON which you have the keys to decrypt.

In comparison, current room membership:

  • works for room avatar and per-room profiles.
  • is much easier to cache as it doesn't rely on historical room state, and is easy to explain to clients.
  • doesn't interfere with E2EE as badly, since it's a more permissive approach (we err on the side of letting clients fetch media and if they don't have the key too bad, but we never end up in a situation where we fail to let clients fetch media they can legitimately decrypt).

that can be attached to a single event is implementation-defined by servers.

If any of the `attach_media` parameters do not correspond to known
restricted media items, or they refer to restricted media items that have
Copy link
Member

@kegsay kegsay Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If any of the attach_media parameters do not correspond to known restricted media items, or they refer to restricted media items that have already been attached, the server responds with a 400 error

This enforces a 1:1 relationship between MXC ID and event ID. This is problematic, and is acknowledged throughout this proposal:

  • Since each m.room.member references the avatar separately, changing your avatar will cause an even bigger traffic storm if you're in a lot of rooms.

  • this could cause duplication of media in the remote media cache, if the implementation does not take steps to deduplicate (eg, storing media by content hash rather than media id).

  • The copy API entirely.
  • The inability for clients to cache repeated content e.g stickers, resulting in suggestions like adding HTTP headers which contain the content hash.

It is poorly justified why it is worth it to keep a 1:1 mapping despite all these issues. The only relevant comment I see is justification for the copy API:

(This mechanism, rather than just allowing clients to attach media to multiple events, is necessary to ensure that the list of events attached a given piece of media does not grow over time, which would make it difficult for servers to reliably cache media and impose the correct access restrictions.)

which doesn't make sense to me. In fact, the inability to attach the same media to multiple events makes it hard to cache, hence people asking for a content hash.

To me, it makes far more sense to say:

  • The media ID in the MXC URI is a content hash and we spec it so clients can introspect it.
  • When you ?attach_media= a row is added in an attached_media table which has event_id and content_hash columns. This table is UNIQUE(event_id, content_hash), and indexed on both event_id and content_hash independently.
  • This gives you a 1:many mapping for content hashes, whilst still allowing servers to "impose the correct access restrictions".
  • Rows in this table will grow over time if there are genuinely more events referencing the same piece of content.
  • Illegal material can be nuked by content hash.
  • GDPR erasure can be achieved by JOINing on event_id to the event's sender and deleting those rows.
  • All content in a room can be deleted by JOINing on event_id to the event's room_id and deleting those rows.
  • Content can be deleted if there exists no content hash in this table.

It all just makes life significantly easier imo. The only reason I can think of why you wouldn't just do this is if you've outsourced your media up/downloading to a 3rd party who doesn't provide a way to add custom code. If you can only nuke individual URLs then that becomes your poor-man's "deleting rows from the attached media table". Whilst that may make it easier for a specific server setup, it negatively impacts everyone else who can no longer de-duplicate in a sane way.

Comment on lines +142 to +146
The response is a json object with a required `content_uri` property,
giving a new MXC URI referring to the media.

The new media item can be attached to a new event, and generally functions
in every way the same as uploading a brand new media item.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this address cases where ie. I intentionally want to reuse the same media item for client deduplication reasons, ie. reusing the same 2MB room icon across all 100 rooms in my space? Could this be changed to some way where a client can inherently (without extra API calls) know it's downloading the same item 100 times for a /hierarchy listing or similar?
Similar case for custom emojis, where you might over time end up with thousands if not millions of copies of the same exact media item.

Idea: can we pass the event ID to the download/thumbnail endpoints instead, so it doesn't have to completely change semantics and break existing client media storage optimisations?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API hacktoberfest-accepted kind:core MSC which is critical to the protocol's success needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal s2s Server-to-Server API (federation) security
Projects
Status: Scheduled - v1.10
Development

Successfully merging this pull request may close these issues.