-
-
Notifications
You must be signed in to change notification settings - Fork 109
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
Move size limits for user, room and event IDs into the appendix and clarify that the length is to be measured in bytes #1850
Conversation
…larify that the length is to be measured in bytes Fixes: matrix-org#1826 Relates to: matrix-org#1001 Signed-off-by: Johannes Marbach <[email protected]>
68a8ff8
to
c35ba8f
Compare
@@ -556,7 +556,7 @@ The `domain` of a user ID is the [server name](#server-name) of the | |||
homeserver which allocated the account. | |||
|
|||
The length of a user ID, including the `@` sigil and the domain, MUST | |||
NOT exceed 255 characters. | |||
NOT exceed 255 bytes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For future reference: this restriction comes from the fact that a user ID must fit inside a state key (in the m.room.member
event), and state keys have always been limited to 255 bytes (since 6a0595b)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think this and #1850 (comment) should also go into the spec?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, that's fine. I just wanted to make a record so that when, in a few months, we come to look at this PR, we remember the thought process.
The length of a room ID, including the `!` sigil and the domain, MUST | ||
NOT exceed 255 bytes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was also previously specified in the C-S API, also going back to 6a0595b.
Co-authored-by: Richard van der Hoff <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes: #1826
Relates to: #1001
Pull Request Checklist
Preview: https://pr1850--matrix-spec-previews.netlify.app