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.

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

We should spec the max size of an EDU over federation #807

Open
richvdh opened this issue Apr 14, 2021 · 13 comments
Open

We should spec the max size of an EDU over federation #807

richvdh opened this issue Apr 14, 2021 · 13 comments
Labels
A-S2S Server-to-Server API (federation)

Comments

@richvdh
Copy link
Member

richvdh commented Apr 14, 2021

Currently, there is effectively no limit to the size of a /_matrix/federation/v1/send/{txnId} request, because some EDUs (eg presence, receipts) can reasonably contain an unlimited quantity of data. I think we should specify some limits to the amount of data such EDUs can contain.

@turt2live
Copy link
Member

EDUs are events and thus are subject to the 65K limit?

@richvdh
Copy link
Member Author

richvdh commented Apr 15, 2021

They really aren't events: they don't follow the same structure in any other way.

Using the same limit as PDUs seems reasonable.

@turt2live
Copy link
Member

I mean the spec calls them events and already applies that limit.

@richvdh
Copy link
Member Author

richvdh commented Apr 15, 2021

what do you mean "applies that limit"?

I think it's a mistake to call them events. They don't have event_ids, room_ids, or anything else that makes an event an event.

@turt2live
Copy link
Member

from memory, there's a ton of mentions of them being events. It's a similar situation for account data. Events are only defined to have content and type - we have a different concept of Room Events which have an ID and other fields.

@richvdh
Copy link
Member Author

richvdh commented Apr 15, 2021

@turt2live
Copy link
Member

turt2live commented Apr 15, 2021

<insert sobbing>

They have a type at the client-server level. We should fix this...

@richvdh
Copy link
Member Author

richvdh commented Apr 15, 2021

they don't exist at the client-server level. They might map onto something else at that level, but they are a purely server-server concept.

@KitsuneRal
Copy link
Member

KitsuneRal commented Apr 15, 2021

I understand that they might not look like "proper events" in the federation API, but CS API really treats them like those throughout. Not that they can be interspersed with PDUs but, e.g., /sync sends them just as another bunch of "events" (and even room events don't have room_id because it's resolved from the context both for PDUs and EDUs, btw).

@richvdh
Copy link
Member Author

richvdh commented Apr 15, 2021

/sync sends them just as another bunch of "events"

/sync sends things like "read receipts" and "presence updates". It calls them "Ephemeral events", but part of the entire problem here is that a single EDU can include multiple read receipts and presence updates. It's definitely not the case that these are then sent in their entirety on to clients.

In short: the "ephemeral events" that the C-S spec talks about are not the same as EDUs that happen over the S-S API. That may be less than satisfactory, but changing it is a bigger task.

@turt2live turt2live added the A-S2S Server-to-Server API (federation) label Apr 15, 2021
@turt2live turt2live changed the title We should spec the max size of an EDU We should spec the max size of an EDU over federation Apr 15, 2021
@KitsuneRal
Copy link
Member

By the way, CS API applies the same restriction on all events, referring to S-S API as a source of canonicalisation algorithm; I wonder if that section should probably be updated as well.

@richvdh
Copy link
Member Author

richvdh commented Nov 28, 2024

Whether they are an "event" or not (for which, see #897), I still think the spec should give an explicit limit for the size of an EDU (and also the size of a to-device message).

@ansiwen
Copy link

ansiwen commented Jan 3, 2025

See element-hq/synapse#17035 (comment) for an example that results in 32MB to-device EDU transactions that are blocked by matrix.org (or cloudflare I guess).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-S2S Server-to-Server API (federation)
Projects
None yet
Development

No branches or pull requests

4 participants