Skip to content
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.

Sign up for GitHub

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

MSC3896: Appservice media #3896

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

MSC3896: Appservice media #3896

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1cfbd09
Propose appservice media
tezlm Sep 24, 2022
b2973bf
Add msc number
tezlm Sep 24, 2022
4e14b5b
Fix wording
tezlm Sep 24, 2022
5454061
clarify unstable prefixes
Kladki Jun 13, 2024
6e79d15
further clarify response and request format
Kladki Jun 13, 2024
af67a41
clarify that server_name must be own
Kladki Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions proposals/3896-appservice-media.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# MSC3896: Appservice media

Appservices often need to bridge media, and do so by reuploading files to the homeserver. This
can cause a lot of storage usage. This MSC proposes a way for appservices to register media
(`mxc://`) namespaces.

## Proposal

This proposal adds a new key (`media`) to the namespaces block of appservice registration.

Whenever the homeserver gets a request that matches the regex, it should make a http GET request
to `/_matrix/app/v1/media/{mediaId}`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would the timeout_ms parameter from the client request be forwarded to the appservice?

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't really make sense in this context, since this is for async uploads, which doesn't apply for media appservices. Not sure why I even asked this (this can be marked as resolved).

Copy link
Contributor

@Kladki Kladki Apr 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is the server expected to always follow redirects, or should it use the allow_redirect param like the C-S API?

Copy link
Contributor

Choose a reason for hiding this comment

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

Considering that this was added after the fact to ensure compatibility, I think this should be assumed for new endpoints.


For example:

```yaml
namespaces:
media:
- exclusive: true
regex: "foobar-.*"
```

In this case, fetching `mxc://server.tld/foobar-bazqux` leads to a request to
`/_matrix/app/v1/media/foobar-bazqux`.

Appservices may set `Cache-Control` on their response. Homeservers should cache the response, though
they may remove cached remote media to save space.

## 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.

Since currently media ids are largely generated at random, it is very possible that existing media could collide with new namespaced media. Because of this, there needs to be some sort of way to disambiguate this media, either by adding some sort of prefix to media by appservices that homeservers cannot use, vice versa, or something completely different.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not to sure how much of an issue this is, considering:

  • Dendrite uses hex for media IDs ([0-9a-f])
  • Conduit uses alphanumeric chars for media IDs ([0-9a-zA-Z])
  • Synapse uses alphabetical chars ([a-zA-Z])

So perhaps something should be added to the spec stating that for automatically generated media IDs, only alphanumeric characters should be used, so that appservices can use other characters to not collide.

Copy link
Member

Choose a reason for hiding this comment

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

This feels like a similar problem to users: if users already exist under the appservice namespace when the appservice is introduced, they are adopted into the namespace as well. For media, we'd do the same: media which matches the namespace is adopted by the appservice, regardless of when it was created.

The grammar concern is best handled by another MSC (#1597, primarily)

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I will bring this up in the aforementioned MSC then, thanks!


Media may not be able to load if the appservice is unable to reach wherever remote media is stored.

## Alternatives
Copy link
Contributor

@Kladki Kladki May 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

mautrix-discord sets up what is essentially a homeserver which only serves media, so perhaps that could be mentioned, and then explain how that is sub-optimal.

In the past it also intercepted requests to your homeserver with regex in a reverse proxy, so that can also be mentioned.


*none*

## Security considerations

*none*

## Unstable prefix
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably specify that for the endpoint, the unstable prefix is /_matrix/app/unstable/org.eu.celery.msc3896/media/{mediaId}


`org.eu.celery.msc3896.media` should be used instead of `media`

## Dependencies

*none*